From 5f2090172d58b1941f919b698685c5d3e9de6be0 Mon Sep 17 00:00:00 2001 From: James Williams Date: Tue, 23 Aug 2016 09:32:15 -0600 Subject: [PATCH] don't allow nil rubric titles also fixup current rubrics with nil titles test plan: * should not be able to create a rubric with a nil title closes #OUT-8 Change-Id: Ic58b0c6f513ad54e11fed1b95342be16f6fd6e3f Reviewed-on: https://gerrit.instructure.com/88496 Tested-by: Jenkins Reviewed-by: Jeremy Stanley Product-Review: James Williams QA-Review: James Williams --- app/models/importers/rubric_importer.rb | 1 + app/models/rubric.rb | 20 ++++++++++++++----- .../20160823152519_fix_null_rubric_titles.rb | 11 ++++++++++ lib/data_fixup/fix_null_rubric_titles.rb | 12 +++++++++++ 4 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20160823152519_fix_null_rubric_titles.rb create mode 100644 lib/data_fixup/fix_null_rubric_titles.rb diff --git a/app/models/importers/rubric_importer.rb b/app/models/importers/rubric_importer.rb index 956f15677f1..9916b715425 100644 --- a/app/models/importers/rubric_importer.rb +++ b/app/models/importers/rubric_importer.rb @@ -42,6 +42,7 @@ module Importers item.migration_id = hash[:migration_id] item.workflow_state = 'active' if item.deleted? item.title = hash[:title] + item.populate_rubric_title # just in case item.description = hash[:description] item.points_possible = hash[:points_possible].to_f item.read_only = hash[:read_only] unless hash[:read_only].nil? diff --git a/app/models/rubric.rb b/app/models/rubric.rb index 8bcaa4d69c4..14954889777 100644 --- a/app/models/rubric.rb +++ b/app/models/rubric.rb @@ -28,7 +28,7 @@ class Rubric < ActiveRecord::Base validates_presence_of :context_id, :context_type, :workflow_state validates_length_of :description, :maximum => maximum_text_length, :allow_nil => true, :allow_blank => true - validates_length_of :title, :maximum => maximum_string_length, :allow_nil => true, :allow_blank => true + validates_length_of :title, :maximum => maximum_string_length, :allow_nil => false, :allow_blank => false before_validation :default_values after_save :update_alignments @@ -75,13 +75,19 @@ class Rubric < ActiveRecord::Base end def default_values - original_title = self.title + if Rails.env.test? + populate_rubric_title # there are too many specs to change and i'm too lazy + end + cnt = 0 siblings = Rubric.where(context_id: self.context_id, context_type: self.context_type).where("workflow_state<>'deleted'") siblings = siblings.where("id<>?", self.id) unless new_record? - while siblings.where(title: self.title).exists? - cnt += 1 - self.title = "#{original_title} (#{cnt})" + if self.title.present? + original_title = self.title + while siblings.where(title: self.title).exists? + cnt += 1 + self.title = "#{original_title} (#{cnt})" + end end self.context_code = "#{self.context_type.underscore}_#{self.context_id}" rescue nil end @@ -211,6 +217,10 @@ class Rubric < ActiveRecord::Base false end + def populate_rubric_title + self.title ||= t('context_name_rubric', "%{course_name} Rubric", :course_name => context.name) + end + CriteriaData = Struct.new(:criteria, :points_possible, :title) def generate_criteria(params) @used_ids = {} diff --git a/db/migrate/20160823152519_fix_null_rubric_titles.rb b/db/migrate/20160823152519_fix_null_rubric_titles.rb new file mode 100644 index 00000000000..f663ecc960e --- /dev/null +++ b/db/migrate/20160823152519_fix_null_rubric_titles.rb @@ -0,0 +1,11 @@ +class FixNullRubricTitles < ActiveRecord::Migration + tag :postdeploy + + def up + DataFixup::FixNullRubricTitles.send_later_if_production_enqueue_args( + :run, :priority => Delayed::LOW_PRIORITY, :max_attempts => 1) + end + + def down + end +end diff --git a/lib/data_fixup/fix_null_rubric_titles.rb b/lib/data_fixup/fix_null_rubric_titles.rb new file mode 100644 index 00000000000..d3cd6a240eb --- /dev/null +++ b/lib/data_fixup/fix_null_rubric_titles.rb @@ -0,0 +1,12 @@ +module DataFixup + module FixNullRubricTitles + def self.run + Rubric.find_ids_in_ranges(:batch_size => 10_000) do |min_id, max_id| + Rubric.where(:id => min_id..max_id).where(:title => nil).each do |rubric| + rubric.populate_rubric_title + rubric.save + end + end + end + end +end