Improve of required validation(master) (agileware-jp#59)

* Add unit test for IssueTemplate

* Add unit test for GlobalIssueTemplate

* Add unit test for NoteTemplate

* Add fixture global_note_templates

* Add unit test for GlobalNoteTemplate

* Add validation of description

* Fixed tests(set description)

* Set text formatting to 'textile'

* Refactor: ActiveRecord::Enum references via a class method

* Remove unnecessary include

* Revert "Set text formatting to 'textile'"

b2f6495

* Set text formatting to 'textile' in the callback

app/models/concerns/issue_template_common.rb

@@ -15,6 +15,7 @@ module IssueTemplateCommon
 
     validates :title, presence: true
     validates :tracker, presence: true
+    validates :description, presence: true
     validates :related_link, format: { with: URI::DEFAULT_PARSER.make_regexp }, allow_blank: true
 
     scope :enabled, -> { where(enabled: true) }
@@ -105,4 +106,4 @@ def confirm_disabled
   def copy_title
     "copy_of_#{title}"
   end
-end
+end

app/models/global_note_template.rb

@@ -2,7 +2,6 @@
 
 class GlobalNoteTemplate < ActiveRecord::Base
   include Redmine::SafeAttributes
-  include ActiveModel::Validations
 
   # author and project should be stable.
   safe_attributes 'name',
@@ -28,6 +27,7 @@ class GlobalNoteTemplate < ActiveRecord::Base
 
   validates :name, presence: true
   validates :tracker, presence: true
+  validates :description, presence: true
   acts_as_positioned scope: %i[tracker_id]
 
   enum visibility: { roles: 1, open: 2 }

app/models/note_template.rb

@@ -2,7 +2,6 @@
 
 class NoteTemplate < ActiveRecord::Base
   include Redmine::SafeAttributes
-  include ActiveModel::Validations
 
   class NoteTemplateError < StandardError; end
 
@@ -24,6 +23,7 @@ class NoteTemplateError < StandardError; end
   validates :name, uniqueness: { scope: :project_id }
   validates :name, presence: true
   validates :tracker, presence: true
+  validates :description, presence: true
   acts_as_positioned scope: %i[project_id tracker_id]
 
   enum visibility: { mine: 0, roles: 1, open: 2 }

spec/factories/note_templates.rb

@@ -8,6 +8,6 @@
     sequence(:memo) { |n| "note-template-memo: #{n}" }
     enabled { true }
     sequence(:position) { |n| n }
-    visibility { NoteTemplate::visibilities[:open] }
+    visibility { NoteTemplate.visibilities[:open] }
   end
 end

spec/features/drag_and_drop_spec.rb

@@ -91,7 +91,7 @@
       template_list =
         FactoryBot.create_list(:note_template, 2,
           project_id: project.id, tracker_id: tracker.id,
-          visibility: NoteTemplate::visibilities[:roles],
+          visibility: NoteTemplate.visibilities[:roles],
           role_ids: [role.id, developer_role.id],
         )
       template_list.each(&:reload)

spec/models/global_issue_template_spec.rb

@@ -4,7 +4,7 @@
 
 describe GlobalIssueTemplate do
   describe '#valid?' do
-    let(:instance) { GlobalIssueTemplate.new(tracker_id: tracker.id, title: 'sample') }
+    let(:instance) { GlobalIssueTemplate.new(tracker_id: tracker.id, title: 'sample', description: 'description1') }
     let(:tracker) { FactoryBot.create(:tracker, :with_default_status) }
     subject { instance.valid? }
 

spec/models/issue_template_spec.rb

@@ -56,7 +56,7 @@
   end
 
   describe '#valid?' do
-    let(:instance) { described_class.new(tracker_id: tracker.id, project_id: project.id, title: 'sample') }
+    let(:instance) { described_class.new(tracker_id: tracker.id, project_id: project.id, title: 'sample', description: 'description1') }
     subject { instance.valid? }
 
     it 'related_link in invalid format' do

test/fixtures/global_note_templates.yml

@@ -0,0 +1,12 @@
+---
+global_note_templates_001:
+  id: 1
+  name: global note template 1
+  description: |-
+    global description 1-1
+    global description 1-2
+  tracker_id: 1
+  author_id: 2
+  enabled: true
+  position: 1
+  visibility: open

test/test_helper.rb

@@ -30,4 +30,18 @@
 ActiveRecord::FixtureSet.create_fixtures(File.dirname(__FILE__) + '/fixtures/',
                                          %i[issue_templates issue_template_settings
                                             global_issue_templates global_issue_templates_projects
-                                            note_templates note_visible_roles])
+                                            note_templates note_visible_roles
+                                            global_note_templates])
+
+module Redmine
+  class ControllerTest
+    setup do
+      Setting.text_formatting = 'textile'
+    end
+
+    teardown do
+      Setting.delete_all
+      Setting.clear_cache
+    end
+  end
+end

test/unit/global_issue_templates_test.rb

@@ -26,4 +26,21 @@ def test_sort_by_position
     b = GlobalIssueTemplate.new(title: 'Template5', position: 1, tracker_id: 1)
     assert_equal [b, a], [a, b].sort
   end
+
+  def test_required_attributes_should_be_validated
+    {
+      title: ' ',
+      tracker: nil,
+      description: " \n\n ",
+    }.each do |attr, val|
+      @global_issue_template.reload
+      @global_issue_template.__send__("#{attr}=", val)
+
+      assert_raises ActiveRecord::RecordInvalid do
+        @global_issue_template.save!
+      end
+
+      assert_includes @global_issue_template.errors[attr], 'cannot be blank'
+    end
+  end
 end

test/unit/global_note_template_test.rb

@@ -3,18 +3,36 @@
 require File.expand_path(File.dirname(__FILE__) + '/../test_helper')
 
 class GlobalNoteTemplateTest < ActiveSupport::TestCase
-  fixtures :projects, :users, :trackers, :roles
+  fixtures :projects, :users, :trackers, :roles, :global_note_templates
 
   def setup; end
   def teardown; end
 
   def test_create_should_require_tracker
-    template = GlobalNoteTemplate.new(name: 'GlobalNoteTemplate1', visibility: 'open')
+    template = GlobalNoteTemplate.new(name: 'GlobalNoteTemplate1', visibility: 'open', description: 'description1')
     assert_no_difference 'GlobalNoteTemplate.count' do
       assert_raises ActiveRecord::RecordInvalid do
         template.save!
       end
     end
     assert_equal ['Tracker cannot be blank'], template.errors.full_messages
   end
+
+  def test_required_attributes_should_be_validated
+    template = GlobalNoteTemplate.find(1)
+    {
+      name: ' ',
+      tracker: nil,
+      description: " \n\n ",
+    }.each do |attr, val|
+      template.reload
+      template.__send__("#{attr}=", val)
+
+      assert_raises ActiveRecord::RecordInvalid do
+        template.save!
+      end
+
+      assert_includes template.errors[attr], 'cannot be blank'
+    end
+  end
 end

test/unit/issue_template_test.rb

@@ -41,4 +41,22 @@ def test_is_default
       assert !template.is_default?
     end
   end
+
+  def test_required_attributes_should_be_validated
+    {
+      project_id: nil,
+      title: ' ',
+      tracker: nil,
+      description: " \n\n ",
+    }.each do |attr, val|
+      @issue_template.reload
+      @issue_template.__send__("#{attr}=", val)
+
+      assert_raises ActiveRecord::RecordInvalid do
+        @issue_template.save!
+      end
+
+      assert_includes @issue_template.errors[attr], 'cannot be blank'
+    end
+  end
 end

test/unit/note_template_test.rb

@@ -44,18 +44,18 @@ def test_sort_by_position
   def test_visibility_with_success
     a = NoteTemplate.create(name: 'Template1', position: 2, project_id: 1, tracker_id: 1,
                             visibility: 'roles', role_ids: [Role.first.id])
-    assert_equal 1, a.visibility_before_type_cast
+    assert_equal 1, NoteTemplate.visibilities[a.visibility]
 
     a.visibility = 'mine'
     a.save
-    assert_equal 0, a.visibility_before_type_cast
+    assert_equal 0, NoteTemplate.visibilities[a.visibility]
   end
 
   def test_visibility_without_role_ids
     # When enable validation: Raise ActiveRecord::RecordInvalid
     e = assert_raises ActiveRecord::RecordInvalid do
       NoteTemplate.create!(name: 'Template1', position: 2, project_id: 1, tracker_id: 1,
-                           visibility: 'roles')
+                           visibility: 'roles', description: 'description1')
     end
 
     # Check error message.
@@ -77,7 +77,7 @@ def test_visibility_from_mine_to_roles
   end
 
   def test_create_should_require_tracker
-    template = NoteTemplate.new(name: 'NoteTemplate1', project_id: 1, visibility: 'open')
+    template = NoteTemplate.new(name: 'NoteTemplate1', project_id: 1, visibility: 'open', description: 'description1')
     assert_no_difference 'NoteTemplate.count' do
       assert_raises ActiveRecord::RecordInvalid do
         template.save!
@@ -86,6 +86,25 @@ def test_create_should_require_tracker
     assert_equal ['Tracker cannot be blank'], template.errors.full_messages
   end
 
+  def test_required_attributes_should_be_validated
+    template = NoteTemplate.find(1)
+    {
+      project_id: nil,
+      name: ' ',
+      tracker: nil,
+      description: " \n\n ",
+    }.each do |attr, val|
+      template.reload
+      template.__send__("#{attr}=", val)
+
+      assert_raises ActiveRecord::RecordInvalid do
+        template.save!
+      end
+
+      assert_includes template.errors[attr], 'cannot be blank'
+    end
+  end
+
   def test_loadable_with_admin_user
     user = User.find_by_login('admin')
     assert_equal true, user.admin?