Strip attributes for Milestone and Issuable. #3428

CHANGELOG

@@ -4,6 +4,7 @@ v 8.3.0 (unreleased)
   - Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera)
   - Fix 500 error when update group member permission
   - Fix: Raw private snippets access workflow
+  - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
 
 v 8.2.1
   - Forcefully update builds that didn't want to update with state machine

app/controllers/projects/issues_controller.rb

@@ -158,12 +158,10 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def issue_params
-    permitted = params.require(:issue).permit(
+    params.require(:issue).permit(
       :title, :assignee_id, :position, :description,
       :milestone_id, :state_event, :task_num, label_ids: []
     )
-    params[:issue][:title].strip! if params[:issue][:title]
-    permitted
   end
 
   def bulk_update_params

app/controllers/projects/merge_requests_controller.rb

@@ -276,13 +276,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def merge_request_params
-    permitted = params.require(:merge_request).permit(
+    params.require(:merge_request).permit(
       :title, :assignee_id, :source_project_id, :source_branch,
       :target_project_id, :target_branch, :milestone_id,
       :state_event, :description, :task_num, label_ids: []
     )
-    params[:merge_request][:title].strip! if params[:merge_request][:title]
-    permitted
   end
 
   # Make sure merge requests created before 8.0

app/models/concerns/issuable.rb

@@ -8,6 +8,7 @@ module Issuable
   extend ActiveSupport::Concern
   include Participable
   include Mentionable
+  include StripAttribute
 
   included do
     belongs_to :author, class_name: "User"
@@ -51,6 +52,7 @@ module Issuable
 
     attr_mentionable :title, :description
     participant :author, :assignee, :notes_with_associations
+    strip_attributes :title
   end
 
   module ClassMethods

app/models/concerns/strip_attribute.rb

@@ -0,0 +1,34 @@
+# == Strip Attribute module
+#
+# Contains functionality to clean attributes before validation
+#
+# Usage:
+#
+#     class Milestone < ActiveRecord::Base
+#       strip_attributes :title
+#     end
+#
+#
+module StripAttribute
+  extend ActiveSupport::Concern
+
+  module ClassMethods
+    def strip_attributes(*attrs)
+      strip_attrs.concat(attrs)
+    end
+
+    def strip_attrs
+      @strip_attrs ||= []
+    end
+  end
+
+  included do
+    before_validation :strip_attributes
+  end
+
+  def strip_attributes
+    self.class.strip_attrs.each do |attr|
+      self[attr].strip! if self[attr] && self[attr].respond_to?(:strip!)
+    end
+  end
+end

app/models/milestone.rb

@@ -22,6 +22,7 @@ class Milestone < ActiveRecord::Base
 
   include InternalId
   include Sortable
+  include StripAttribute
 
   belongs_to :project
   has_many :issues
@@ -35,6 +36,8 @@ class Milestone < ActiveRecord::Base
   validates :title, presence: true
   validates :project, presence: true
 
+  strip_attributes :title
+
   state_machine :state, initial: :active do
     event :close do
       transition active: :closed

spec/models/concerns/strip_attribute_spec.rb

@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Milestone, "StripAttribute" do
+  let(:milestone) { create(:milestone) }
+
+  describe ".strip_attributes" do
+    it { expect(Milestone).to respond_to(:strip_attributes) }
+    it { expect(Milestone.strip_attrs).to include(:title) }
+  end
+
+  describe "#strip_attributes" do
+    before do
+      milestone.title = '    8.3   '
+      milestone.valid?
+    end
+
+    it { expect(milestone.title).to eq('8.3') }
+  end
+
+end