diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 5dd6dda0824..ee06cab06e8 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -873,7 +873,7 @@ class Assignment < ActiveRecord::Base homeworks = [] ts = Time.now.to_s students.each do |student| - homework = self.find_or_create_submission(student) + homework = Submission.find_or_initialize_by_assignment_id_and_user_id(self.id, student.id) homework.grade_matches_current_submission = homework.score ? false : true homework.attributes = opts.merge({ :attachment => nil, @@ -884,7 +884,7 @@ class Assignment < ActiveRecord::Base }) homework.submitted_at = Time.now - homework.with_versioning(true) do + homework.with_versioning(:explicit => true) do group ? homework.save_without_broadcast : homework.save end context_module_action(student, :submitted) diff --git a/app/models/submission.rb b/app/models/submission.rb index bfd9938be6c..ed874aac811 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -109,6 +109,15 @@ class Submission < ActiveRecord::Base SQL end end + trigger.after(:insert) do |t| + t.where('#{Submission.needs_grading_conditions("NEW")}') do + <<-SQL + UPDATE assignments + SET needs_grading_count = needs_grading_count + 1 + WHERE id = NEW.assignment_id; + SQL + end + end attr_reader :suppress_broadcast attr_reader :group_broadcast_submission @@ -117,7 +126,7 @@ class Submission < ActiveRecord::Base has_a_broadcast_policy - simply_versioned + simply_versioned :explicit => true set_policy do given {|user| user && user.id == self.user_id } diff --git a/db/migrate/20110527155754_create_trigger_submissions_insert.rb b/db/migrate/20110527155754_create_trigger_submissions_insert.rb new file mode 100644 index 00000000000..f661bcb8bf8 --- /dev/null +++ b/db/migrate/20110527155754_create_trigger_submissions_insert.rb @@ -0,0 +1,25 @@ +# 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 CreateTriggerSubmissionsInsert < ActiveRecord::Migration + def self.up + 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.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 + 1 + WHERE id = NEW.assignment_id; + SQL_ACTIONS + end + end + end + + def self.down + drop_trigger("submissions_after_insert_row_tr", "submissions") + + drop_trigger("submissions_after_insert_row_when_new_submission_type_is_not_tr", "submissions") + end +end diff --git a/spec/apis/v1/submissions_api_spec.rb b/spec/apis/v1/submissions_api_spec.rb index 1d08dfd1935..527dd3494d7 100644 --- a/spec/apis/v1/submissions_api_spec.rb +++ b/spec/apis/v1/submissions_api_spec.rb @@ -29,7 +29,9 @@ describe SubmissionsApiController, :type => :integration do end sub.workflow_state = 'submitted' yield(sub) if block_given? - update_with_protected_attributes!(sub, { :submitted_at => Time.at(@submit_homework_time), :created_at => Time.at(@submit_homework_time) }.merge(opts)) + sub.with_versioning(:explicit => true) do + update_with_protected_attributes!(sub, { :submitted_at => Time.at(@submit_homework_time), :created_at => Time.at(@submit_homework_time) }.merge(opts)) + end sub.versions(true).each { |v| Version.update_all({ :created_at => v.model.created_at }, { :id => v.id }) } sub end diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb index e24e19d7312..8805f24558c 100644 --- a/spec/models/assignment_spec.rb +++ b/spec/models/assignment_spec.rb @@ -36,6 +36,7 @@ describe Assignment do @assignment.submissions.size.should eql(1) @submission = @assignment.submissions.first @submission.user_id.should eql(@user.id) + @submission.versions.length.should eql(1) end it "should be able to grade a submission" do @@ -49,6 +50,7 @@ describe Assignment do @submission.should eql(s[0]) @submission.score.should eql(10.0) @submission.user_id.should eql(@user.id) + @submission.versions.length.should eql(1) end it "should update needs_grading_count when submissions transition state" do @@ -96,12 +98,18 @@ describe Assignment do s2 = @a.grade_student(@user, :grade => "10") s.reload s.should eql(s2[0]) + # there should only be one version, even though the grade changed + s.versions.length.should eql(1) s2[0].state.should eql(:graded) end - it "should be versioned" do - assignment_model - @a.should be_respond_to(:versions) + it "should create a new version for each submission" do + setup_assignment_without_submission + @a.submit_homework(@user) + @a.submit_homework(@user) + @a.submit_homework(@user) + @a.reload + @a.submissions.first.versions.length.should eql(3) end describe "infer_due_at" do diff --git a/spec/models/general_model_spec.rb b/spec/models/general_model_spec.rb index 7d4a9dfffde..b9c93e635cd 100644 --- a/spec/models/general_model_spec.rb +++ b/spec/models/general_model_spec.rb @@ -49,7 +49,8 @@ describe 'Models' do ActiveRecord::SessionStore::Session, Delayed::Backend::ActiveRecord::Job, Version, - Story + Story, + Woozel ] (ignore_classes << AddThumbnailUuid::Thumbnail) rescue nil (ignore_classes << CustomField) rescue nil diff --git a/vendor/plugins/simply_versioned/lib/simply_versioned.rb b/vendor/plugins/simply_versioned/lib/simply_versioned.rb index 26abc8d6329..ed0c7167b79 100644 --- a/vendor/plugins/simply_versioned/lib/simply_versioned.rb +++ b/vendor/plugins/simply_versioned/lib/simply_versioned.rb @@ -27,6 +27,8 @@ module SoftwareHeretics # +limit+ - specifies the number of old versions to keep (default = nil, never delete old versions) # +automatic+ - controls whether versions are created automatically (default = true, save versions) # +exclude+ - specify columns that will not be saved (default = [], save all columns) + # +explicit+ - INSTRUCTURE: explicit versioning keeps the last version up to date, but doesn't + # automatically create new versions (default = false) # # To save the record without creating a version either set +versioning_enabled+ to false # on the model before calling save or, alternatively, use +without_versioning+ and save @@ -34,13 +36,14 @@ module SoftwareHeretics # def simply_versioned( options = {} ) - bad_keys = options.keys - [:keep,:automatic,:exclude] + bad_keys = options.keys - [:keep,:automatic,:exclude,:explicit] raise SimplyVersioned::BadOptions.new( bad_keys ) unless bad_keys.empty? options.reverse_merge!( { :keep => nil, :automatic => true, - :exclude => [] + :exclude => [], + :explicit => false }) has_many :versions, :order => 'number DESC', :as => :versionable, :dependent => :destroy, :extend => VersionsProxyMethods @@ -58,6 +61,9 @@ module SoftwareHeretics cattr_accessor :simply_versioned_excluded_columns self.simply_versioned_excluded_columns = Array( options[ :exclude ] ).map( &:to_s ) + + cattr_accessor :simply_versioned_explicitly_versioned + self.simply_versioned_explicitly_versioned = options[:explicit] class_eval do def versioning_enabled=( enabled ) @@ -107,18 +113,31 @@ module SoftwareHeretics # Invoke the supplied block passing the receiver as the sole block argument with # versioning enabled or disabled depending upon the value of the +enabled+ parameter # for the duration of the block. - def with_versioning( enabled, &block ) + def with_versioning( enabled = true) versioning_was_enabled = self.versioning_enabled? + explicit_versioning_was_enabled = @simply_versioned_explicit_enabled + explicit_enabled = false + if enabled.is_a?(Hash) + opts = enabled + enabled = true + explicit_enabled = true if opts[:explicit] + end self.versioning_enabled = enabled + @simply_versioned_explicit_enabled = explicit_enabled # INSTRUCTURE: always create a version if explicitly told to do so @versioning_explicitly_enabled = enabled == true begin - block.call( self ) + yield self ensure @versioning_explicitly_enabled = nil self.versioning_enabled = versioning_was_enabled + @simply_versioned_explicit_enabled = explicit_enabled end end + + def without_versioning(&block) + with_versioning(false, &block) + end def unversioned? self.versions.nil? || self.versions.size == 0 @@ -144,7 +163,7 @@ module SoftwareHeretics self.versions.current.number end end - + protected # INSTRUCTURE: If defined on a method, allow a check @@ -161,7 +180,11 @@ module SoftwareHeretics # INSTRUCTURE if @versioning_explicitly_enabled || @changes_are_worth_versioning @changes_are_worth_versioning = nil - if self.versions.create( :yaml => self.attributes.except( *simply_versioned_excluded_columns ).to_yaml ) + if simply_versioned_explicitly_versioned && !@simply_versioned_explicit_enabled && versioned? + version = self.versions.current + version.yaml = self.attributes.except( *simply_versioned_excluded_columns ).to_yaml + version.save + elsif self.versions.create( :yaml => self.attributes.except( *simply_versioned_excluded_columns ).to_yaml ) self.versions.clean_old_versions( simply_versioned_keep_limit.to_i ) if simply_versioned_keep_limit end end diff --git a/vendor/plugins/simply_versioned/spec_canvas/simply_versioned_spec.rb b/vendor/plugins/simply_versioned/spec_canvas/simply_versioned_spec.rb new file mode 100644 index 00000000000..4be03c87fc0 --- /dev/null +++ b/vendor/plugins/simply_versioned/spec_canvas/simply_versioned_spec.rb @@ -0,0 +1,65 @@ +require File.expand_path(File.dirname(__FILE__)+'/../../../../spec/apis/api_spec_helper') + +class Woozel< ActiveRecord::Base + simply_versioned :explicit => true +end + +Woozel.establish_connection(:adapter => 'sqlite3', :database => ':memory:') + +describe 'simply_versioned' do + describe "explicit versions" do + before do + Woozel.connection.create_table :woozels, :force => true do |t| + t.string :name + end + end + after do + Woozel.connection.drop_table :woozels + end + + it "should create the first version on save" do + woozel = Woozel.new(:name => 'Eeyore') + woozel.should_not be_versioned + woozel.save! + woozel.should be_versioned + woozel.versions.length.should eql(1) + woozel.versions.current.model.name.should eql('Eeyore') + end + + it "should keep the last version up to date for each save" do + woozel = Woozel.create!(:name => 'Eeyore') + woozel.should be_versioned + woozel.versions.length.should eql(1) + woozel.versions.current.model.name.should eql('Eeyore') + woozel.name = 'Piglet' + woozel.save! + woozel.versions.length.should eql(1) + woozel.versions.current.model.name.should eql('Piglet') + end + + it "should create a new version when asked to" do + woozel = Woozel.create!(:name => 'Eeyore') + woozel.name = 'Piglet' + woozel.with_versioning(:explicit => true, &:save!) + woozel.versions.length.should eql(2) + woozel.versions.first.model.name.should eql('Eeyore') + woozel.versions.current.model.name.should eql('Piglet') + end + + it 'should not create a new version when not explicitly asked to' do + woozel = Woozel.create!(:name => 'Eeyore') + woozel.name = 'Piglet' + woozel.with_versioning(&:save!) + woozel.versions.length.should eql(1) + woozel.versions.current.model.name.should eql('Piglet') + end + + it 'should not update the last version when not versioning' do + woozel = Woozel.create!(:name => 'Eeyore') + woozel.name = 'Piglet' + woozel.without_versioning(&:save!) + woozel.versions.length.should eql(1) + woozel.versions.current.model.name.should eql('Eeyore') + end + end +end \ No newline at end of file