Add max_name_length to Assignment

Refs: SIS-2692

Test plan:
- Verify that an assignment requires name to be shorter or equal
  to the name length setting on the account with post to sis
  checked, the sis sync settings is checked, the assignment name
  length setting is checked on the account, there's a value for
  the name length setting and the new_sis_integrations feature
  flag is on

Change-Id: I8eea8c1087d9798f3893776e358e5cfb966717f5
Reviewed-on: https://gerrit.instructure.com/100056
Tested-by: Jenkins
Reviewed-by: Stewie aka Nicholas Stewart <nstewart@instructure.com>
Product-Review: Brad Humphrey <brad@instructure.com>
QA-Review: Steven Shepherd <sshepherd@instructure.com>

app/models/assignment.rb

@@ -69,7 +69,6 @@ class Assignment < ActiveRecord::Base
   has_many :ignores, :as => :asset
   has_many :moderated_grading_selections, class_name: 'ModeratedGrading::Selection'
   belongs_to :context, polymorphic: [:course]
-  validates_length_of :title, :maximum => maximum_string_length, :allow_nil => false, :allow_blank => true
   belongs_to :grading_standard
   belongs_to :group_category
 
@@ -145,12 +144,11 @@ class Assignment < ActiveRecord::Base
     AssignmentUtil.due_date_required?(self)
   end
 
-  def assignment_name_length_ok?
-    name_length = self.try(:context).try(:account).try(:sis_assignment_name_length_input).try(:[], :value) ||
-                  Assignment.maximum_string_length
-    if sis_require_assignment_name_length? && self.post_to_sis.present? && self.title.length > name_length
-      errors.add(:title, I18n.t('The title cannot be longer than %{length} characters', length: name_length))
+  def max_name_length
+    if AssignmentUtil.assignment_name_length_required?(self)
+      return self.try(:context).try(:account).try(:sis_assignment_name_length_input).try(:[], :value).to_i
     end
+    Assignment.maximum_string_length
   end
 
   def secure_params
@@ -248,7 +246,6 @@ class Assignment < ActiveRecord::Base
   validates_presence_of :context_id, :context_type, :workflow_state
 
   validates_presence_of :title, if: :title_changed?
-  validates_length_of :title, :maximum => maximum_string_length, :allow_nil => true
   validates_length_of :description, :maximum => maximum_long_text_length, :allow_nil => true, :allow_blank => true
   validates_length_of :allowed_extensions, :maximum => maximum_long_text_length, :allow_nil => true, :allow_blank => true
   validate :frozen_atts_not_altered, :if => :frozen?, :on => :update
@@ -2339,9 +2336,16 @@ class Assignment < ActiveRecord::Base
     end
   end
 
-  def sis_require_assignment_name_length?
-    self.try(:context).try(:account).try(:sis_assignment_name_length).try(:[], :value) &&
-    self.try(:context).try(:feature_enabled?, 'new_sis_integrations').present?
+  def assignment_name_length_ok?
+    name_length = max_name_length
+
+    # Due to the removal of the multiple `validates_length_of :title` validations we need this nil check
+    # here to act as those validations so we can reduce the number of validations for this attribute
+    # to just one single check
+    return if self.nil? || self.title.nil?
+
+    if self.title.to_s.length > name_length
+      errors.add(:title, I18n.t('The title cannot be longer than %{length} characters', length: name_length))
+    end
   end
 end
-

lib/assignment_util.rb

@@ -8,5 +8,11 @@ module AssignmentUtil
   def self.due_date_ok?(assignment)
     !due_date_required?(assignment) || assignment.due_at.present?
   end
-end
 
+  def self.assignment_name_length_required?(assignment)
+    assignment.post_to_sis.present? &&
+    assignment.try(:context).try(:account).try(:sis_syncing).try(:[], :value) &&
+    assignment.try(:context).try(:account).try(:sis_assignment_name_length).try(:[], :value) &&
+    assignment.try(:context).try(:account).try(:feature_enabled?, 'new_sis_integrations').present?
+  end
+end

spec/lib/assignment_util_spec.rb

@@ -10,6 +10,8 @@ describe AssignmentUtil do
     @course.assignments.create!(assignment_valid_attributes)
   end
 
+  let(:assignment_name_length_value){ 15 }
+
   describe "due_date_required?" do
     it "returns true when all 3 are set to true" do
       assignment.post_to_sis = true
@@ -65,5 +67,46 @@ describe AssignmentUtil do
       expect(described_class.due_date_ok?(assignment)).to eq(true)
     end
   end
-end
 
+  describe "assignment_name_length_required?" do
+    it "returns true when all 4 are set to true" do
+      assignment.post_to_sis = true
+      assignment.context.account.stubs(:sis_syncing).returns({value: true})
+      assignment.context.account.stubs(:sis_assignment_name_length).returns({value: true})
+      assignment.context.account.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
+      expect(described_class.assignment_name_length_required?(assignment)).to eq(true)
+    end
+
+    it "returns false when sis_sycning is set to false" do
+      assignment.post_to_sis = true
+      assignment.context.account.stubs(:sis_syncing).returns({value: false})
+      assignment.context.account.stubs(:sis_assignment_name_length).returns({value: true})
+      assignment.context.account.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
+      expect(described_class.assignment_name_length_required?(assignment)).to eq(false)
+    end
+
+    it "returns false when post_to_sis is false" do
+      assignment.post_to_sis = false
+      assignment.context.account.stubs(:sis_syncing).returns({value: true})
+      assignment.context.account.stubs(:sis_assignment_name_length).returns({value: true})
+      assignment.context.account.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
+      expect(described_class.assignment_name_length_required?(assignment)).to eq(false)
+    end
+
+    it "returns false when sis_assignment_name_length is false" do
+      assignment.post_to_sis = true
+      assignment.context.account.stubs(:sis_syncing).returns({value: false})
+      assignment.context.account.stubs(:sis_assignment_name_length).returns({value: false})
+      assignment.context.account.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
+      expect(described_class.assignment_name_length_required?(assignment)).to eq(false)
+    end
+
+    it "returns false when new_sis_integrations is false" do
+      assignment.post_to_sis = true
+      assignment.context.account.stubs(:sis_syncing).returns({value: false})
+      assignment.context.account.stubs(:sis_assignment_name_length).returns({value: true})
+      assignment.context.account.stubs(:feature_enabled?).with('new_sis_integrations').returns(false)
+      expect(described_class.assignment_name_length_required?(assignment)).to eq(false)
+    end
+  end
+end

spec/models/assignment_spec.rb

@@ -3384,6 +3384,11 @@ describe Assignment do
       @assignment = assignment_model(course: @course)
     end
 
+    let(:errors) do
+      @assignment.valid?
+      @assignment.errors
+    end
+
     it "should hard truncate at 30 characters" do
       @assignment.title = "a" * 31
       expect(@assignment.title.length).to eq 31
@@ -3410,7 +3415,7 @@ describe Assignment do
                            qwertyuiopasdfghjklzxcvbnmqwertyuiopasdfghjklzxcvbnm
                            qwertyuiopasdfghjklzxcvbnmqwertyuiopasdfghjklzxcvbnm'
 
-      expect(lambda { @assignment.save! }).to raise_error("Validation failed: Title is too long (maximum is 255 characters), Title is too long (maximum is 255 characters)")
+      expect(errors[:title]).not_to be_empty
     end
   end
 
@@ -3813,60 +3818,24 @@ describe Assignment do
       errors = assignment.errors
       expect(errors[:title]).not_to be_empty
     end
+  end
 
-    describe "when sis_assignment_name_length is true" do
-      it "is required when post_to_sis is true and feature_enabled(post_grades) is true" do
-        assignment.post_to_sis = true
-        assignment.title = "Too much tuna fish"
-        assignment.context.account.stubs(:sis_assignment_name_length).returns({value: true})
-        assignment.context.account.stubs(:sis_assignment_name_length_input).returns({value: 15})
-        assignment.context.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
-        expect(assignment.valid?).to eq(false)
-      end
-
-      it "is valid when post_to_sis is true, feature_enabled(post_grades) is true and there is no name length value" do
-        assignment.post_to_sis = true
-        assignment.title = "Too much tuna fish"
-        assignment.context.account.stubs(:sis_assignment_name_length).returns({value: true})
-        assignment.context.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
-        expect(assignment.valid?).to eq(true)
-      end
+  describe "max_name_length" do
+    let(:assignment) do
+      @course.assignments.new(assignment_valid_attributes)
     end
 
-    describe "when sis_assignment_name_length is false" do
-      before do
-        assignment.context.account.stubs(:sis_assignment_name_length).returns({value: false})
-      end
+    it "returns custom name length if sis_assignment_name_length_input is present" do
+      assignment.post_to_sis = true
+      assignment.context.account.stubs(:sis_syncing).returns({value: true})
+      assignment.context.account.stubs(:sis_assignment_name_length).returns({value: true})
+      assignment.context.account.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
+      assignment.context.account.stubs(:sis_assignment_name_length_input).returns({value: 15})
+      expect(assignment.max_name_length).to eq(15)
+    end
 
-      it "is not required when post_to_sis is true and feature_enabled(post_grades) is true" do
-        assignment.post_to_sis = true
-        assignment.title = "Too much tuna fish"
-        assignment.context.account.stubs(:sis_assignment_name_length).returns({value: false})
-        assignment.context.account.stubs(:sis_assignment_name_length_input).returns({value: 15})
-        assignment.context.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
-        expect(assignment.valid?).to eq(true)
-      end
-
-      it "is not required when post_to_sis is true and feature_enabled(post_grades) is false" do
-        assignment.post_to_sis = true
-        assignment.title = "some new assignment"
-        assignment.context.stubs(:feature_enabled?).with('new_sis_integrations').returns(false)
-        expect(assignment.valid?).to eq(true)
-      end
-
-      it "is not required when post_to_sis is false and feature_enabled(post_grades) is true" do
-        assignment.post_to_sis = false
-        assignment.title = "oh hello"
-        assignment.context.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
-        expect(assignment.valid?).to eq(true)
-      end
-
-      it "is not required when post_to_sis is false and feature_enabled(post_grades) is false" do
-        assignment.post_to_sis = false
-        assignment.title = "tuna"
-        assignment.context.stubs(:feature_enabled?).with('new_sis_integrations').returns(false)
-        expect(assignment.valid?).to eq(true)
-      end
+    it "returns default of 255 if sis_assignment_name_length_input is not present " do
+      expect(assignment.max_name_length).to eq(255)
     end
   end