ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

assignments.needs_grading_count counter to speed things up, closes #3945 

Change-Id: I3748a7e0551e49720d06be302df29a8e33528007
Reviewed-on: https://gerrit.instructure.com/2573
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
This commit is contained in:
Jon Jensen 2011-03-08 15:00:41 -07:00
parent 040a430eff
commit 63d09992d4
16 changed files with 239 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Gemfile
View File

@ -36,6 +36,7 @@ gem 'sanitize', '1.2.1'
gem 'uuid', '2.3.1'
gem 'xml-simple', '1.0.12', :require => 'xmlsimple'
gem 'will_paginate', '2.3.15'
gem 'hairtrigger', '0.1.1'
group :test do
gem 'sqlite3-ruby', '1.3.2'

2
app/controllers/application_controller.rb
View File

@ -333,7 +333,7 @@ class ApplicationController < ActionController::Base
@ungraded_assignments = @assignments.select{|a|
a.grants_right?(@current_user, session, :grade) &&
a.expects_submission? &&
a.submissions.having_submission.ungraded.count > 0
a.needs_grading_count > 0
}
@assignment_groups = @groups
@past_assignments = @assignments.select{ |a| a.due_at && a.due_at < Time.now }

54
app/models/assignment.rb
View File

@ -599,17 +599,10 @@ class Assignment < ActiveRecord::Base
submission_types && self.submission_types != "" && self.submission_types != "none" && self.submission_types != 'not_graded' && self.submission_types != "online_quiz" && self.submission_types != 'discussion_topic' && self.submission_types != 'attendance'
end
def ungraded_count
return read_attribute(:ungraded_count).to_i if read_attribute(:ungraded_count)
Rails.cache.fetch(['ungraded_count', self].cache_key) do
self.submissions.select{|s| s.ungraded? && s.has_submission? }.length #having_submission.ungraded.count
end
end
def graded_count
return read_attribute(:graded_count).to_i if read_attribute(:graded_count)
Rails.cache.fetch(['graded_count', self].cache_key) do
self.submissions.select{|s| !s.ungraded? }.length
self.submissions.select(&:graded?).length
end
end
@ -1067,13 +1060,6 @@ class Assignment < ActiveRecord::Base
}
}
named_scope :include_ungraded_count, lambda {
{:select => "assignments.*, (SELECT COUNT(*) FROM submissions
WHERE assignments.id = submissions.assignment_id
AND submissions.submission_type IS NOT NULL AND submissions.grade IS NULL) AS ungraded_count"
}
}
# don't really need this scope anymore since we are doing the auto_peer_reviews assigning as a delayed job instead of a poller, but I'll leave it here if it is useful to anyone. -RS
named_scope :to_be_auto_peer_reviewed, lambda {
{:conditions => ['assignments.peer_reviews_assigned != ? AND assignments.peer_reviews = ? AND assignments.due_at < ? AND assignments.automatic_peer_reviews = ?', true, true, Time.now.utc, true], :order => 'assignments.updated_at, assignments.peer_reviews_due_at' }
@ -1132,35 +1118,15 @@ class Assignment < ActiveRecord::Base
}
}
# This should only be used in the course drop down to show assignments recently graded.
# This should only be used in the course drop down to show assignments not yet graded.
named_scope :need_grading_info, lambda{|limit, ignore_ids|
ignore_ids ||= []
# TODO: this could be faster using:
# SELECT assignments.id, title, points_possible, due_at, context_id, context_type, COUNT(submissions.id) AS need_graded_count
# FROM assignments
# JOIN submissions ON submissions.assignment_id = assignments.id
# JOIN enrollments ON enrollments.course_id = assignments.context_id AND enrollments.user_id = submissions.user_id AND enrollments.workflow_state != 'deleted'
# WHERE submission_types IS NOT NULL AND (submission_types NOT IN ('', 'none', 'not_graded', 'on_paper') AND (assignments.workflow_state != 'deleted')) AND (assignments.context_code IN ('course_20033','course_20036','course_20222','course_20237','course_20246','course_20247','course_20252','course_20257','course_20262','course_20263','course_20264','course_20265','course_20267','course_20268','course_20269','course_20270','course_20271','course_20272','course_20273','course_20276','course_20277','course_20281','course_20320','course_20325','course_20331','course_20341','course_20355','course_20363','course_20370','course_20428'))
# AND submissions.submitted_at > '2010-04-15 21:46:19' AND submissions.score IS NULL AND submissions.workflow_state != 'graded'
# GROUP BY assignments.id
# ORDER BY due_at DESC LIMIT 15
sql_for_need_graded_count = "(SELECT COUNT(submissions.id) FROM submissions
JOIN enrollments ON enrollments.user_id = submissions.user_id AND enrollments.workflow_state = 'active'
WHERE assignment_id = assignments.id
AND enrollments.course_id = assignments.context_id
AND (
(score IS NULL AND submissions.workflow_state != 'graded')
OR submissions.workflow_state = 'submitted'
OR submissions.workflow_state = 'pending_review'
)
AND submission_type IS NOT NULL)"
{:select => 'assignments.id, title, points_possible, due_at, context_id, context_type, submission_types, ' +
'(SELECT name FROM courses WHERE id = assignments.context_id) AS context_name,' +
"#{sql_for_need_graded_count} AS need_graded_count",
:conditions => "#{sql_for_need_graded_count} > 0 #{ignore_ids.empty? ? "" : "AND id NOT IN (#{ignore_ids.join(',')})"}",
:limit => limit,
:order=>'due_at ASC'
{
:select => 'assignments.id, title, points_possible, due_at, context_id, context_type, submission_types, ' +
'(SELECT name FROM courses WHERE id = assignments.context_id) AS context_name, needs_grading_count',
:conditions => "needs_grading_count > 0 #{ignore_ids.empty? ? "" : "AND id NOT IN (#{ignore_ids.join(',')})"}",
:limit => limit,
:order=>'due_at ASC'
}
}
@ -1434,10 +1400,6 @@ class Assignment < ActiveRecord::Base
end
end
def submissions_needing_grading
self.submissions.select{|s| !s.graded? }
end
def special_class; nil; end
def submission_action_string

28
app/models/enrollment.rb
View File

@ -38,6 +38,32 @@ class Enrollment < ActiveRecord::Base
after_save :touch_user
after_create :update_user_account_associations
trigger.after(:insert).where("NEW.workflow_state = 'active'") do
<<-SQL
UPDATE assignments
SET needs_grading_count = needs_grading_count + 1
WHERE id IN (SELECT assignment_id
FROM submissions
WHERE user_id = NEW.user_id
AND context_code = 'course_' || NEW.course_id
AND (#{Submission.needs_grading_conditions})
);
SQL
end
trigger.after(:update).where("NEW.workflow_state <> OLD.workflow_state AND (NEW.workflow_state = 'active' OR OLD.workflow_state = 'active')") do
<<-SQL
UPDATE assignments
SET needs_grading_count = needs_grading_count + CASE WHEN NEW.workflow_state = 'active' THEN 1 ELSE -1 END
WHERE id IN (SELECT assignment_id
FROM submissions
WHERE user_id = NEW.user_id
AND context_code = 'course_' || NEW.course_id
AND (#{Submission.needs_grading_conditions})
);
SQL
end
adheres_to_policy
@ -111,7 +137,7 @@ class Enrollment < ActiveRecord::Base
def update_user_account_associations
self.user.send_later(:update_account_associations)
end
def conclude
self.workflow_state = "completed"
self.completed_at = Time.now

40
app/models/submission.rb
View File

@ -71,6 +71,19 @@ class Submission < ActiveRecord::Base
}
}
named_scope :needs_grading, :conditions => <<-SQL
submissions.submission_type IS NOT NULL
AND (
submissions.score IS NULL
OR NOT submissions.grade_matches_current_submission
OR submissions.workflow_state IN ('submitted', 'pending_review')
)
SQL
def self.needs_grading_conditions(prefix = nil)
conditions = needs_grading.proxy_options[:conditions].gsub(/\s+/, ' ')
conditions.gsub!("submissions.", prefix + ".") if prefix
conditions
end
sanitize_field :body, Instructure::SanitizeField::SANITIZE
@ -86,6 +99,16 @@ class Submission < ActiveRecord::Base
after_save :update_final_score
after_save :submit_to_turnitin_later
after_save :update_admins_if_just_submitted
trigger.after(:update) do |t|
t.where('(#{Submission.needs_grading_conditions("OLD")}) <> (#{Submission.needs_grading_conditions("NEW")})') do
<<-SQL
UPDATE assignments
SET needs_grading_count = needs_grading_count + CASE WHEN (#{needs_grading_conditions('NEW')}) THEN 1 ELSE -1 END
WHERE id = NEW.assignment_id;
SQL
end
end
attr_reader :suppress_broadcast
attr_reader :group_broadcast_submission
@ -343,7 +366,7 @@ class Submission < ActiveRecord::Base
true
end
attr_accessor :created_correctly_from_assignment_rb
def update_admins_if_just_submitted
if @just_submitted
context.send_later_if_production(:resubmission_for, "assignment_#{assignment_id}")
@ -537,11 +560,7 @@ class Submission < ActiveRecord::Base
workflow do
state :submitted do
event :grade_it, :transitions_to => :graded do
set_broadcast_flags
self.workflow_state = :graded
broadcast_notifications
end
event :grade_it, :transitions_to => :graded
end
state :unsubmitted
state :pending_review
@ -555,9 +574,6 @@ class Submission < ActiveRecord::Base
named_scope :ungraded, lambda {
{:conditions => ['submissions.grade IS NULL'], :include => :assignment}
}
named_scope :ungraded_or_needing_regrade, lambda {
{:conditions => ['submissions.grade IS NULL OR grade_matches_current_submission = ?', false], :include => :assignment }
}
named_scope :having_submission, lambda {
{:conditions => ['submissions.submission_type IS NOT NULL'] }
}
@ -590,10 +606,10 @@ class Submission < ActiveRecord::Base
{:conditions => ['submissions.submission_type = ? AND submissions.attachment_id IS NULL AND submissions.process_attempts < 3', 'online_url'], :order => :updated_at}
}
def ungraded?
!self.grade
def needs_regrading?
graded? && !grade_matches_current_submission?
end
def readable_state
if workflow_state == 'pending_review'
'pending review'

8
app/views/assignments/_assignments_list_content.html.erb
View File

@ -69,9 +69,9 @@ $(document).ready(function() {
}
if(submission && submission.submission_type && !submission.score) {
$assignment.addClass('group_assignment_ungraded');
var cnt = parseInt($assignment.find(".ungraded_count").text(), 10) || 0;
var cnt = parseInt($assignment.find(".needs_grading_count").text(), 10) || 0;
cnt++;
$assignment.find(".ungraded_count").text(cnt);
$assignment.find(".needs_grading_count").text(cnt);
}
$assignment.fillTemplateData({
data: submission,
@ -114,8 +114,8 @@ $(document).ready(function() {
if($(this).hasClass('group_assignment_overdue')) {
$(this).attr('title', "This assignment is overdue");
} else if($(this).hasClass('group_assignment_ungraded')) {
var ungraded_count = $(this).getTemplateData({textValues: ['ungraded_count']}).ungraded_count;
$(this).attr('title', ungraded_count + " submissions for this assignment still need grading");
var needs_grading_count = $(this).getTemplateData({textValues: ['needs_grading_count']}).needs_grading_count;
$(this).attr('title', needs_grading_count + " submissions for this assignment still need grading");
} else if($(this).hasClass('group_assignment_graded')) {
$(this).attr('title', "This assignment has been graded");
}

6
app/views/assignments/_grade_assignment.html.erb
View File

@ -5,11 +5,15 @@
<% subs = @current_student_submissions %>
<% unless @current_student_submissions.blank? %>
<div style="margin-bottom: 0px;">
<% graded = subs.select{|s| s.graded? }.length %>
<% graded = subs.select(&:graded?).length # this includes resubmissions that were previously graded %>
<% resubmitted = subs.select(&:needs_regrading?).length %>
<% submitted = subs.length %>
<span style="<%= 'font-weight: bold;' if graded < submitted %>">
<%= graded %> <span style="font-size: 0.8em;">out of</span> <%= submitted %> Submission<%= graded == 1 ? "" : "s" %> Graded
</span>
<% if resubmitted > 0 %>
<b><br /><%= pluralize(resubmitted, "Ungraded Re-submission") %></b>
<% end %>
</div>
<% end %>
<a target="_blank" class="button button-sidebar-wide" href="<%= context_url(@context, :speed_grader_context_gradebook_url, :assignment_id => @assignment.id) %>"><%= image_tag "grading_icon.png" %> SpeedGrader</a><br/>

2
app/views/courses/_assignments_needing_grading.html.erb
View File

@ -34,7 +34,7 @@ cache(safe_cache_key([@current_user, contexts, 'a_need_grading'])) do
</span>
<% end %>
<b>Grade <%= assignment.title %></b>
<em><%= assignment.need_graded_count %> need<%= 's' if assignment.need_graded_count == 1 %> grading</em>
<em><%= assignment.needs_grading_count %> need<%= 's' if assignment.needs_grading_count == 1 %> grading</em>
</a>
<a class='disable_item_link grading' title="Ignore this assignment" href="<%= dashboard_ignore_item_url(assignment.asset_string, 'grading') %>"><%= image_tag "earmark_hover.png", :alt => 'ignore' %></a>
</li>

2
app/views/shared/_assignment.html.erb
View File

@ -33,7 +33,7 @@
</div>
<div class="links data">
<div style="display: none;">
<span class="ungraded_count">&nbsp;</span>
<span class="needs_grading_count">&nbsp;</span>
<span class="timestamp"><%= (assignment.try_rescue(:due_at).try_rescue(:to_i) || "0") %></span>
<span class="submission_types"><%= assignment.try_rescue(:submission_types) || nbsp %></span>
<span class="due_date_string"><%= assignment.try_rescue(:due_at).try_rescue(:strftime, "%m/%d/%Y") || nbsp %></span>

2
app/views/shared/_menu_assignment.html.erb
View File

@ -9,7 +9,7 @@
<% if title == "To Turn In" %>
due: <%= datetime_string(menu_assignment.due_at, :due_date, nil, true) %>
<% elsif title == "Needing Grading" %>
<%= menu_assignment.need_graded_count %> need<%= 's' if menu_assignment.need_graded_count.to_i == 1 %> grading
<%= menu_assignment.needs_grading_count %> need<%= 's' if menu_assignment.needs_grading_count.to_i == 1 %> grading
<% elsif title == "Recently Graded" %>
<% if menu_assignment.grading_type == 'points' %>
<%= "#{menu_assignment.score}/#{menu_assignment.points_possible}" %>

27
config/initializers/active_record.rb
View File

@ -315,4 +315,31 @@ ActiveRecord::Associations::AssociationCollection.class_eval do
end
end
alias_method_chain :method_missing, :splat_fix
end
# See https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/66-true-false-conditions-broken-for-sqlite#ticket-66-9
# The default 't' and 'f' are no good, since sqlite treats them both as 0 in boolean logic.
# This patch makes it so you can do stuff like:
# :conditions => "active"
# instead of having to do:
# :conditions => ["active = ?", true]
if defined?(ActiveRecord::ConnectionAdapters::SQLiteAdapter)
ActiveRecord::ConnectionAdapters::SQLiteAdapter.class_eval do
def quoted_true
'1'
end
def quoted_false
'0'
end
end
end
if defined?(ActiveRecord::ConnectionAdapters::MysqlAdapter)
ActiveRecord::ConnectionAdapters::MysqlAdapter.class_eval do
def configure_connection_with_pg_compat
configure_connection_without_pg_compat
execute "SET SESSION SQL_MODE='PIPES_AS_CONCAT'"
end
alias_method_chain :configure_connection, :pg_compat
end
end

25
db/migrate/20110307163027_add_ungraded_count_to_assignments.rb Normal file
View File

@ -0,0 +1,25 @@
class AddUngradedCountToAssignments < ActiveRecord::Migration
def self.up
add_column :assignments, :needs_grading_count, :integer, :default => 0
execute <<-SQL
UPDATE assignments SET needs_grading_count = COALESCE((
SELECT COUNT(*)
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.score IS NULL
OR NOT grade_matches_current_submission
OR s.workflow_state = 'submitted'
OR s.workflow_state = 'pending_review'
)
AND s.submission_type IS NOT NULL
), 0)
SQL
end
def self.down
remove_column :assignments, :needs_grading_count
end
end

56
db/migrate/20110321234519_ungraded_count_triggers.rb Normal file
View File

@ -0,0 +1,56 @@
# 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 UngradedCountTriggers < ActiveRecord::Migration
def self.up
create_trigger("enrollments_after_insert_row_when_new_workflow_state_active__tr", :generated => true).
on("enrollments").
after(:insert).
where("NEW.workflow_state = 'active'") do
<<-SQL_ACTIONS
UPDATE assignments
SET needs_grading_count = needs_grading_count + 1
WHERE id IN (SELECT assignment_id
FROM submissions
WHERE user_id = NEW.user_id
AND context_code = 'course_' || NEW.course_id
AND ( submissions.submission_type IS NOT NULL AND ( submissions.score IS NULL OR NOT submissions.grade_matches_current_submission OR submissions.workflow_state IN ('submitted', 'pending_review') ) )
);
SQL_ACTIONS
end
create_trigger("enrollments_after_update_row_when_new_workflow_state_old_wor_tr", :generated => true).
on("enrollments").
after(:update).
where("NEW.workflow_state <> OLD.workflow_state AND (NEW.workflow_state = 'active' OR OLD.workflow_state = 'active')") do
<<-SQL_ACTIONS
UPDATE assignments
SET needs_grading_count = needs_grading_count + CASE WHEN NEW.workflow_state = 'active' THEN 1 ELSE -1 END
WHERE id IN (SELECT assignment_id
FROM submissions
WHERE user_id = NEW.user_id
AND context_code = 'course_' || NEW.course_id
AND ( submissions.submission_type IS NOT NULL AND ( submissions.score IS NULL OR NOT submissions.grade_matches_current_submission OR submissions.workflow_state IN ('submitted', 'pending_review') ) )
);
SQL_ACTIONS
end
create_trigger("submissions_after_update_row_tr", :generated => true).
on("submissions").
after(:update) do |t|
t.where("( OLD.submission_type IS NOT NULL AND ( OLD.score IS NULL OR NOT OLD.grade_matches_current_submission OR OLD.workflow_state IN ('submitted', 'pending_review') ) ) <> ( NEW.submission_type IS NOT NULL AND ( NEW.score IS NULL OR NOT NEW.grade_matches_current_submission OR NEW.workflow_state IN ('submitted', 'pending_review') ) )") do
<<-SQL_ACTIONS
UPDATE assignments
SET needs_grading_count = needs_grading_count + CASE WHEN ( NEW.submission_type IS NOT NULL AND ( NEW.score IS NULL OR NOT NEW.grade_matches_current_submission OR NEW.workflow_state IN ('submitted', 'pending_review') ) ) THEN 1 ELSE -1 END
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)
end
end

2
lib/tasks/hair_trigger.rake Normal file
View File

@ -0,0 +1,2 @@
$VERBOSE = nil
Dir["#{Gem.searcher.find('hair_trigger').full_gem_path}/lib/tasks/*.rake"].each { |ext| load ext }

27
spec/integration/hair_trigger_spec.rb Normal file
View File

@ -0,0 +1,27 @@
#
# Copyright (C) 2011 Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
describe "hair_trigger" do
describe "migrations" do
it "should be current for all models" do
HairTrigger::migrations_current?.should be_true
end
end
end

27
spec/models/assignment_spec.rb
View File

@ -50,6 +50,27 @@ describe Assignment do
@submission.score.should eql(10.0)
@submission.user_id.should eql(@user.id)
end
it "should update needs_grading_count when submissions transition state" do
setup_assignment_with_homework
@assignment.needs_grading_count.should eql(1)
@assignment.grade_student(@user, :grade => "0")
@assignment.reload
@assignment.needs_grading_count.should eql(0)
end
it "should update needs_grading_count when enrollment changes" do
setup_assignment_with_homework
@assignment.needs_grading_count.should eql(1)
@course.enrollments.find_by_user_id(@user.id).destroy
@assignment.reload
@assignment.needs_grading_count.should eql(0)
e = @course.enroll_student(@user)
e.invite
e.accept
@assignment.reload
@assignment.needs_grading_count.should eql(1)
end
it "should not override the grade if the assignment has no points possible" do
setup_assignment_without_submission
@ -910,12 +931,14 @@ def setup_assignment_without_submission
# Established course too, as a context
assignment_model
user_model
@course.enroll_student(@user)
e = @course.enroll_student(@user)
e.invite
e.accept
end
def setup_assignment_with_homework
setup_assignment_without_submission
res = @assignment.submit_homework(@user)
res = @assignment.submit_homework(@user, {:submission_type => 'online_text_entry'})
res.should_not be_nil
res.should be_is_a(Submission)
@assignment.reload