add pending_review submissions to todo list for grading

quiz submissions that have essay questions are marked as pending_review, but
they have a grade, so the previous trigger was ignoring them when incrementing
the needs_grading counts. this changes increments the counter for
pending_review even if there is already a grade.

test-plan:
- create a quiz with an essay question
- as a student, submit the quiz
- as a teacher, your to do list should indicate that you have a quiz that needs
  grading.

fixes #5326
fixes #7466

Change-Id: I7cf2293bb43df882000902109124810e896e66ad
Reviewed-on: https://gerrit.instructure.com/9095
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Hudson <hudson@instructure.com>

app/models/submission.rb

@@ -77,10 +77,11 @@ class Submission < ActiveRecord::Base
   }
 
   named_scope :needs_grading, :conditions => <<-SQL
-    submissions.submission_type IS NOT NULL AND
-    submissions.workflow_state IN ('submitted', 'pending_review') AND (
-      submissions.score IS NULL OR
-      NOT submissions.grade_matches_current_submission
+    submissions.submission_type IS NOT NULL 
+    AND (submissions.workflow_state = 'pending_review'
+      OR (submissions.workflow_state = 'submitted' 
+        AND (submissions.score IS NULL OR NOT submissions.grade_matches_current_submission)
+      )
     )
     SQL
   def self.needs_grading_conditions(prefix = nil)

app/views/courses/_recent_feedback.html.erb

@@ -2,7 +2,7 @@
   show_context ||= false
   comment = nil
   grade = nil
-   score = nil
+  score = nil
 
   cache(['recent_feedback_render', recent_feedback || 'blank_feedback', Time.zone.utc_offset].cache_key) do 
     context = recent_feedback.context
@@ -16,6 +16,8 @@
     icon_explanation = case recent_feedback.workflow_state
     when 'submitted'
       t 'submitted', 'Submitted'
+    when 'pending_review'
+      t 'pending_review', 'Pending review'
     when 'graded'
       t 'graded', 'Graded'
     else

db/migrate/20120229203255_create_triggers_enrollments_insert_and_enrollments_update_and_submissions_update_and_submissions_insert1.rb

@@ -0,0 +1,111 @@
+# This migration was auto-generated via `rake db:generate_trigger_migration'.
+# While you can edit this file, any changes you make to the definitions here
+# will be undone by the next auto-generated trigger migration.
+
+class CreateTriggersEnrollmentsInsertAndEnrollmentsUpdateAndSubmissionsUpdateAndSubmissionsInsert1 < ActiveRecord::Migration
+  tag :predeploy
+
+  def self.up
+    drop_trigger("enrollments_after_insert_row_when_new_workflow_state_active__tr", "enrollments", :generated => true)
+
+    drop_trigger("enrollments_after_update_row_when_new_workflow_state_old_wor_tr", "enrollments", :generated => true)
+
+    drop_trigger("submissions_after_update_row_tr", "submissions", :generated => true)
+
+    drop_trigger("submissions_after_update_row_when_old_submission_type_is_not_tr", "submissions", :generated => true)
+
+    drop_trigger("submissions_after_insert_row_tr", "submissions", :generated => true)
+
+    drop_trigger("submissions_after_insert_row_when_new_submission_type_is_not_tr", "submissions", :generated => true)
+
+    create_trigger("enrollments_after_insert_row_when_new_workflow_state_active__tr", :generated => true, :compatibility => 1).
+        on("enrollments").
+        after(:insert).
+        where("NEW.workflow_state = 'active'") do
+      {:mysql=>"\n    IF NOT EXISTS (SELECT 1 FROM enrollments WHERE workflow_state = 'active' AND user_id = NEW.user_id AND course_id = NEW.course_id AND id <> NEW.id LIMIT 1) THEN\n      UPDATE assignments, submissions SET needs_grading_count = needs_grading_count + 1\n      WHERE context_id = NEW.course_id\n        AND context_type = 'Course'\n        AND assignments.id = submissions.assignment_id\n        AND submissions.user_id = NEW.user_id\n        AND ( submissions.submission_type IS NOT NULL AND (submissions.workflow_state = 'pending_review' OR (submissions.workflow_state = 'submitted' AND (submissions.score IS NULL OR NOT submissions.grade_matches_current_submission) ) ) );\n    END IF;", :default=>"    UPDATE assignments SET needs_grading_count = needs_grading_count + 1\n    WHERE context_id = NEW.course_id\n      AND context_type = 'Course'\n      AND EXISTS (\n        SELECT 1\n        FROM submissions\n        WHERE user_id = NEW.user_id\n          AND assignment_id = assignments.id\n          AND ( submissions.submission_type IS NOT NULL AND (submissions.workflow_state = 'pending_review' OR (submissions.workflow_state = 'submitted' AND (submissions.score IS NULL OR NOT submissions.grade_matches_current_submission) ) ) )\n        LIMIT 1\n      )\n      AND NOT EXISTS (SELECT 1 FROM enrollments WHERE workflow_state = 'active' AND user_id = NEW.user_id AND course_id = NEW.course_id AND id <> NEW.id LIMIT 1);"}
+    end
+
+    create_trigger("enrollments_after_update_row_when_new_workflow_state_old_wor_tr", :generated => true, :compatibility => 1).
+        on("enrollments").
+        after(:update).
+        where("NEW.workflow_state <> OLD.workflow_state AND (NEW.workflow_state = 'active' OR OLD.workflow_state = 'active')") do
+      {:mysql=>"\n    IF NOT EXISTS (SELECT 1 FROM enrollments WHERE workflow_state = 'active' AND user_id = NEW.user_id AND course_id = NEW.course_id AND id <> NEW.id LIMIT 1) THEN\n      UPDATE assignments, submissions SET needs_grading_count = needs_grading_count + CASE WHEN NEW.workflow_state = 'active' THEN 1 ELSE -1 END\n      WHERE context_id = NEW.course_id\n        AND context_type = 'Course'\n        AND assignments.id = submissions.assignment_id\n        AND submissions.user_id = NEW.user_id\n        AND ( submissions.submission_type IS NOT NULL AND (submissions.workflow_state = 'pending_review' OR (submissions.workflow_state = 'submitted' AND (submissions.score IS NULL OR NOT submissions.grade_matches_current_submission) ) ) );\n    END IF;", :default=>"    UPDATE assignments SET needs_grading_count = needs_grading_count + CASE WHEN NEW.workflow_state = 'active' THEN 1 ELSE -1 END\n    WHERE context_id = NEW.course_id\n      AND context_type = 'Course'\n      AND EXISTS (\n        SELECT 1\n        FROM submissions\n        WHERE user_id = NEW.user_id\n          AND assignment_id = assignments.id\n          AND ( submissions.submission_type IS NOT NULL AND (submissions.workflow_state = 'pending_review' OR (submissions.workflow_state = 'submitted' AND (submissions.score IS NULL OR NOT submissions.grade_matches_current_submission) ) ) )\n        LIMIT 1\n      )\n      AND NOT EXISTS (SELECT 1 FROM enrollments WHERE workflow_state = 'active' AND user_id = NEW.user_id AND course_id = NEW.course_id AND id <> NEW.id LIMIT 1);"}
+    end
+
+    create_trigger("submissions_after_update_row_tr", :generated => true, :compatibility => 1).
+        on("submissions").
+        after(:update) do |t|
+      t.where("( OLD.submission_type IS NOT NULL AND (OLD.workflow_state = 'pending_review' OR (OLD.workflow_state = 'submitted' AND (OLD.score IS NULL OR NOT OLD.grade_matches_current_submission) ) ) ) <> ( NEW.submission_type IS NOT NULL AND (NEW.workflow_state = 'pending_review' OR (NEW.workflow_state = 'submitted' AND (NEW.score IS NULL OR NOT NEW.grade_matches_current_submission) ) ) )") do
+        <<-SQL_ACTIONS
+      UPDATE assignments
+      SET needs_grading_count = needs_grading_count + CASE WHEN ( NEW.submission_type IS NOT NULL AND (NEW.workflow_state = 'pending_review' OR (NEW.workflow_state = 'submitted' AND (NEW.score IS NULL OR NOT NEW.grade_matches_current_submission) ) ) ) THEN 1 ELSE -1 END
+      WHERE id = NEW.assignment_id;
+        SQL_ACTIONS
+      end
+    end
+
+    create_trigger("submissions_after_insert_row_tr", :generated => true, :compatibility => 1).
+        on("submissions").
+        after(:insert) do |t|
+      t.where(" NEW.submission_type IS NOT NULL AND (NEW.workflow_state = 'pending_review' OR (NEW.workflow_state = 'submitted' AND (NEW.score IS NULL OR NOT NEW.grade_matches_current_submission) ) ) ") do
+        <<-SQL_ACTIONS
+      UPDATE assignments
+      SET needs_grading_count = needs_grading_count + 1
+      WHERE id = NEW.assignment_id;
+        SQL_ACTIONS
+      end
+    end
+  end
+
+  def self.down
+    drop_trigger("enrollments_after_insert_row_when_new_workflow_state_active__tr", "enrollments", :generated => true)
+
+    drop_trigger("enrollments_after_update_row_when_new_workflow_state_old_wor_tr", "enrollments", :generated => true)
+
+    drop_trigger("submissions_after_update_row_tr", "submissions", :generated => true)
+
+    drop_trigger("submissions_after_update_row_when_old_submission_type_is_not_tr", "submissions", :generated => true)
+
+    drop_trigger("submissions_after_insert_row_tr", "submissions", :generated => true)
+
+    drop_trigger("submissions_after_insert_row_when_new_submission_type_is_not_tr", "submissions", :generated => true)
+
+    create_trigger("enrollments_after_insert_row_when_new_workflow_state_active__tr", :generated => true, :compatibility => 1).
+        on("enrollments").
+        after(:insert).
+        where("NEW.workflow_state = 'active'") do
+      {:mysql=>"\n    IF NOT EXISTS (SELECT 1 FROM enrollments WHERE workflow_state = 'active' AND user_id = NEW.user_id AND course_id = NEW.course_id AND id <> NEW.id LIMIT 1) THEN\n      UPDATE assignments, submissions SET needs_grading_count = needs_grading_count + 1\n      WHERE context_id = NEW.course_id\n        AND context_type = 'Course'\n        AND assignments.id = submissions.assignment_id\n        AND submissions.user_id = NEW.user_id\n        AND ( submissions.submission_type IS NOT NULL AND submissions.workflow_state IN ('submitted', 'pending_review') AND ( submissions.score IS NULL OR NOT submissions.grade_matches_current_submission ) );\n    END IF;", :default=>"    UPDATE assignments SET needs_grading_count = needs_grading_count + 1\n    WHERE context_id = NEW.course_id\n      AND context_type = 'Course'\n      AND EXISTS (\n        SELECT 1\n        FROM submissions\n        WHERE user_id = NEW.user_id\n          AND assignment_id = assignments.id\n          AND ( submissions.submission_type IS NOT NULL AND submissions.workflow_state IN ('submitted', 'pending_review') AND ( submissions.score IS NULL OR NOT submissions.grade_matches_current_submission ) )\n        LIMIT 1\n      )\n      AND NOT EXISTS (SELECT 1 FROM enrollments WHERE workflow_state = 'active' AND user_id = NEW.user_id AND course_id = NEW.course_id AND id <> NEW.id LIMIT 1);"}
+    end
+
+    create_trigger("enrollments_after_update_row_when_new_workflow_state_old_wor_tr", :generated => true, :compatibility => 1).
+        on("enrollments").
+        after(:update).
+        where("NEW.workflow_state <> OLD.workflow_state AND (NEW.workflow_state = 'active' OR OLD.workflow_state = 'active')") do
+      {:mysql=>"\n    IF NOT EXISTS (SELECT 1 FROM enrollments WHERE workflow_state = 'active' AND user_id = NEW.user_id AND course_id = NEW.course_id AND id <> NEW.id LIMIT 1) THEN\n      UPDATE assignments, submissions SET needs_grading_count = needs_grading_count + CASE WHEN NEW.workflow_state = 'active' THEN 1 ELSE -1 END\n      WHERE context_id = NEW.course_id\n        AND context_type = 'Course'\n        AND assignments.id = submissions.assignment_id\n        AND submissions.user_id = NEW.user_id\n        AND ( submissions.submission_type IS NOT NULL AND submissions.workflow_state IN ('submitted', 'pending_review') AND ( submissions.score IS NULL OR NOT submissions.grade_matches_current_submission ) );\n    END IF;", :default=>"    UPDATE assignments SET needs_grading_count = needs_grading_count + CASE WHEN NEW.workflow_state = 'active' THEN 1 ELSE -1 END\n    WHERE context_id = NEW.course_id\n      AND context_type = 'Course'\n      AND EXISTS (\n        SELECT 1\n        FROM submissions\n        WHERE user_id = NEW.user_id\n          AND assignment_id = assignments.id\n          AND ( submissions.submission_type IS NOT NULL AND submissions.workflow_state IN ('submitted', 'pending_review') AND ( submissions.score IS NULL OR NOT submissions.grade_matches_current_submission ) )\n        LIMIT 1\n      )\n      AND NOT EXISTS (SELECT 1 FROM enrollments WHERE workflow_state = 'active' AND user_id = NEW.user_id AND course_id = NEW.course_id AND id <> NEW.id LIMIT 1);"}
+    end
+
+    create_trigger("submissions_after_update_row_tr", :generated => true, :compatibility => 1).
+        on("submissions").
+        after(:update) do |t|
+      t.where("( OLD.submission_type IS NOT NULL AND OLD.workflow_state IN ('submitted', 'pending_review') AND ( OLD.score IS NULL OR NOT OLD.grade_matches_current_submission ) ) <> ( NEW.submission_type IS NOT NULL AND NEW.workflow_state IN ('submitted', 'pending_review') AND ( NEW.score IS NULL OR NOT NEW.grade_matches_current_submission ) )") do
+        <<-SQL_ACTIONS
+      UPDATE assignments
+      SET needs_grading_count = needs_grading_count + CASE WHEN ( NEW.submission_type IS NOT NULL AND NEW.workflow_state IN ('submitted', 'pending_review') AND ( NEW.score IS NULL OR NOT NEW.grade_matches_current_submission ) ) THEN 1 ELSE -1 END
+      WHERE id = NEW.assignment_id;
+        SQL_ACTIONS
+      end
+    end
+
+    create_trigger("submissions_after_insert_row_tr", :generated => true, :compatibility => 1).
+        on("submissions").
+        after(:insert) do |t|
+      t.where(" NEW.submission_type IS NOT NULL AND NEW.workflow_state IN ('submitted', 'pending_review') AND ( NEW.score IS NULL OR NOT NEW.grade_matches_current_submission ) ") do
+        <<-SQL_ACTIONS
+      UPDATE assignments
+      SET needs_grading_count = needs_grading_count + 1
+      WHERE id = NEW.assignment_id;
+        SQL_ACTIONS
+      end
+    end
+  end
+end

db/migrate/20120302175325_fix_ungraded_counts_include_quiz_essays.rb

@@ -0,0 +1,24 @@
+class FixUngradedCountsIncludeQuizEssays < ActiveRecord::Migration
+  tag :postdeploy
+
+  def self.up
+    execute <<-SQL
+      UPDATE assignments SET needs_grading_count = COALESCE((
+        SELECT COUNT(DISTINCT s.id)
+        FROM submissions s
+        INNER JOIN enrollments e ON e.user_id = s.user_id AND e.workflow_state = 'active'
+        WHERE s.assignment_id = assignments.id
+          AND e.course_id = assignments.context_id
+          AND s.submission_type IS NOT NULL
+          AND (s.workflow_state = 'pending_review'
+            OR (s.workflow_state = 'submitted' 
+              AND (s.score IS NULL OR NOT s.grade_matches_current_submission)
+            )
+          )
+      ), 0)
+      SQL
+  end
+
+  def self.down
+  end
+end

spec/models/quiz_submission_spec.rb

@@ -191,6 +191,10 @@ describe QuizSubmission do
       })
       @quiz_submission.submission.workflow_state.should eql 'graded'
     end
+
+    it "should increment the assignment needs_grading_count for pending_review state" do
+      @quiz.assignment.reload.needs_grading_count.should == 1
+    end
   end
 
   describe "with multiple essay questions" do

spec/selenium/dashboard_spec.rb

@@ -264,7 +264,6 @@ describe "dashboard" do
     end
 
     it "should display assignment to grade in to do list and assignments menu for a teacher" do
-      course_with_teacher_logged_in
       assignment = assignment_model({:submission_types => 'online_text_entry', :course => @course})
       student = user_with_pseudonym(:active_user => true, :username => 'student@example.com', :password => 'qwerty')
       @course.enroll_user(student, "StudentEnrollment", :enrollment_state => 'active')
@@ -284,26 +283,23 @@ describe "dashboard" do
     end
 
     it "should show submitted essay quizzes in the todo list" do
-      pending("bug 7466 - Essay's in quizzes that have been taken are not showing up in the To Do list for a Teacher") do
-        quiz_title = 'new quiz'
-        course_with_teacher_logged_in
-        student_in_course
-        q = @course.quizzes.create!(:title => quiz_title)
-        q.quiz_questions.create!(:question_data => {:name => "Quiz Essay Question 1", :question_type=>'essay_question', :question_text=>'qq1', :points_possible=>10})
-        q.generate_quiz_data
-        q.workflow_state = 'available'
-        q.save
-        q.reload
-        qs = q.generate_submission(@user)
-        qs.workflow_state = 'complete'
-        qs.save
-        qs.reload
-        get "/"
+      quiz_title = 'new quiz'
+      student_in_course
+      q = @course.quizzes.create!(:title => quiz_title)
+      q.quiz_questions.create!(:question_data => {:id => 31, :name => "Quiz Essay Question 1", :question_type=>'essay_question', :question_text=>'qq1', :points_possible=>10})
+      q.generate_quiz_data
+      q.workflow_state = 'available'
+      q.save
+      q.reload
+      qs = q.generate_submission(@user)
+      qs.mark_completed
+      qs.submission_data = {"question_31"=>"<p>abeawebawebae</p>", "question_text"=>"qq1"}
+      qs.grade_submission
+      get "/"
 
-        todo_list = f('.right-side-list .to-do-list')
-        todo_list.should_not be_nil
-        todo_list.should include_text(quiz_title)
-      end
+      todo_list = f('.to-do-list')
+      todo_list.should_not be_nil
+      todo_list.should include_text(quiz_title)
     end
 
     context "course menu customization" do

spec/spec_helper.rb

@@ -369,6 +369,7 @@ Spec::Runner.configure do |config|
     @quiz = Quiz.find_by_assignment_id(@assignment.id)
     @questions = questions.map { |q| @quiz.quiz_questions.create!(q) }
     @quiz.generate_quiz_data
+    @quiz.workflow_state = "available"
     @quiz.save!
     @quiz_submission = @quiz.generate_submission(@user)
     @quiz_submission.mark_completed