Merge branch 'board_model_dry' into 'master'

Added board list concern

See merge request gitlab-org/gitlab!51328

app/models/concerns/boards/listable.rb

+# frozen_string_literal: true
+
+module Boards
+  module Listable
+    extend ActiveSupport::Concern
+
+    included do
+      validates :label, :position, presence: true, if: :label?
+      validates :position, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, if: :movable?
+
+      before_destroy :can_be_destroyed
+
+      scope :ordered, -> { order(:list_type, :position) }
+      scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) }
+      scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
+    end
+
+    class_methods do
+      def destroyable_types
+        [:label]
+      end
+
+      def movable_types
+        [:label]
+      end
+    end
+
+    def destroyable?
+      self.class.destroyable_types.include?(list_type&.to_sym)
+    end
+
+    def movable?
+      self.class.movable_types.include?(list_type&.to_sym)
+    end
+
+    def title
+      label? ? label.name : list_type.humanize
+    end
+
+    private
+
+    def can_be_destroyed
+      throw(:abort) unless destroyable? # rubocop:disable Cop/BanCatchThrow
+    end
+  end
+end

app/models/list.rb

 # frozen_string_literal: true
 
 class List < ApplicationRecord
+  include Boards::Listable
   include Importable
 
   belongs_to :board
@@ -10,30 +11,13 @@ class List < ApplicationRecord
   enum list_type: { backlog: 0, label: 1, closed: 2, assignee: 3, milestone: 4, iteration: 5 }
 
   validates :board, :list_type, presence: true, unless: :importing?
-  validates :label, :position, presence: true, if: :label?
   validates :label_id, uniqueness: { scope: :board_id }, if: :label?
-  validates :position, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, if: :movable?
-
-  before_destroy :can_be_destroyed
-
-  scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) }
-  scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
 
   scope :preload_associated_models, -> { preload(:board, label: :priorities) }
 
-  scope :ordered, -> { order(:list_type, :position) }
-
   alias_method :preferences, :list_user_preferences
 
   class << self
-    def destroyable_types
-      [:label]
-    end
-
-    def movable_types
-      [:label]
-    end
-
     def preload_preferences_for_user(lists, user)
       return unless user
 
@@ -60,18 +44,6 @@ class List < ApplicationRecord
     preferences_for(user).update(preferences)
   end
 
-  def destroyable?
-    self.class.destroyable_types.include?(list_type&.to_sym)
-  end
-
-  def movable?
-    self.class.movable_types.include?(list_type&.to_sym)
-  end
-
-  def title
-    label? ? label.name : list_type.humanize
-  end
-
   def collapsed?(user)
     preferences = preferences_for(user)
 
@@ -95,12 +67,6 @@ class List < ApplicationRecord
       end
     end
   end
-
-  private
-
-  def can_be_destroyed
-    throw(:abort) unless destroyable? # rubocop:disable Cop/BanCatchThrow
-  end
 end
 
 List.prepend_if_ee('::EE::List')

ee/app/models/boards/epic_list.rb

@@ -2,30 +2,13 @@
 
 module Boards
   class EpicList < ApplicationRecord
-    # TODO: we can move logic shared with with List model to
-    # a module. https://gitlab.com/gitlab-org/gitlab/-/issues/296559
+    include ::Boards::Listable
+
     belongs_to :epic_board, optional: false, inverse_of: :epic_lists
     belongs_to :label, inverse_of: :epic_lists
 
-    enum list_type: { backlog: 0, label: 1, closed: 2 }
-
-    validates :label, :position, presence: true, if: :label?
     validates :label_id, uniqueness: { scope: :epic_board_id }, if: :label?
-    validates :position, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, if: :label?
-
-    scope :ordered, -> { order(:list_type, :position) }
-    scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
-
-    def self.movable_types
-      [:label]
-    end
-
-    def title
-      label? ? label.name : list_type.humanize
-    end
 
-    def movable?
-      label?
-    end
+    enum list_type: { backlog: 0, label: 1, closed: 2 }
   end
 end

ee/spec/models/boards/epic_list_spec.rb

@@ -3,44 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe Boards::EpicList do
+  it_behaves_like 'boards listable model', :epic_list
+
   describe 'associations' do
     subject { build(:epic_list) }
 
     it { is_expected.to belong_to(:epic_board).required.inverse_of(:epic_lists) }
     it { is_expected.to belong_to(:label).inverse_of(:epic_lists) }
-    it { is_expected.to validate_presence_of(:position) }
-    it { is_expected.to validate_numericality_of(:position).only_integer.is_greater_than_or_equal_to(0) }
     it { is_expected.to validate_uniqueness_of(:label_id).scoped_to(:epic_board_id) }
-
-    context 'when list_type is set to closed' do
-      subject { build(:epic_list, list_type: :closed) }
-
-      it { is_expected.not_to validate_presence_of(:label) }
-      it { is_expected.not_to validate_presence_of(:position) }
-    end
-  end
-
-  describe 'scopes' do
-    describe '.ordered' do
-      it 'returns lists ordered by type and position' do
-        list1 = create(:epic_list, list_type: :backlog)
-        list2 = create(:epic_list, list_type: :closed)
-        list3 = create(:epic_list, position: 1)
-        list4 = create(:epic_list, position: 2)
-
-        expect(described_class.ordered).to eq([list1, list3, list4, list2])
-      end
-    end
-  end
-
-  describe '#title' do
-    it 'returns label name for label lists' do
-      list = build(:epic_list)
-      expect(list.title).to eq(list.label.name)
-    end
-
-    it 'returns list type for non-label lists' do
-      expect(build(:epic_list, list_type: ::Boards::EpicList.list_types[:backlog]).title).to eq('Backlog')
-    end
   end
 end

spec/models/list_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe List do
   it_behaves_like 'having unique enum values'
+  it_behaves_like 'boards listable model', :list
 
   describe 'relationships' do
     it { is_expected.to belong_to(:board) }
@@ -14,72 +15,6 @@ RSpec.describe List do
     it { is_expected.to validate_presence_of(:board) }
     it { is_expected.to validate_presence_of(:label) }
     it { is_expected.to validate_presence_of(:list_type) }
-    it { is_expected.to validate_presence_of(:position) }
-    it { is_expected.to validate_numericality_of(:position).only_integer.is_greater_than_or_equal_to(0) }
-
-    context 'when list_type is set to closed' do
-      subject { described_class.new(list_type: :closed) }
-
-      it { is_expected.not_to validate_presence_of(:label) }
-      it { is_expected.not_to validate_presence_of(:position) }
-    end
-  end
-
-  describe '#destroy' do
-    it 'can be destroyed when list_type is set to label' do
-      subject = create(:list)
-
-      expect(subject.destroy).to be_truthy
-    end
-
-    it 'can not be destroyed when list_type is set to closed' do
-      subject = create(:closed_list)
-
-      expect(subject.destroy).to be_falsey
-    end
-  end
-
-  describe '#destroyable?' do
-    it 'returns true when list_type is set to label' do
-      subject.list_type = :label
-
-      expect(subject).to be_destroyable
-    end
-
-    it 'returns false when list_type is set to closed' do
-      subject.list_type = :closed
-
-      expect(subject).not_to be_destroyable
-    end
-  end
-
-  describe '#movable?' do
-    it 'returns true when list_type is set to label' do
-      subject.list_type = :label
-
-      expect(subject).to be_movable
-    end
-
-    it 'returns false when list_type is set to closed' do
-      subject.list_type = :closed
-
-      expect(subject).not_to be_movable
-    end
-  end
-
-  describe '#title' do
-    it 'returns label name when list_type is set to label' do
-      subject.list_type = :label
-      subject.label = Label.new(name: 'Development')
-
-      expect(subject.title).to eq 'Development'
-    end
-
-    it 'returns Closed when list_type is set to closed' do
-      subject.list_type = :closed
-
-      expect(subject.title).to eq 'Closed'
-    end
   end
 
   describe '#update_preferences_for' do

spec/support/shared_examples/models/boards/listable_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'boards listable model' do |list_factory|
+  subject { build(list_factory) }
+
+  describe 'associations' do
+    it { is_expected.to validate_presence_of(:position) }
+    it { is_expected.to validate_numericality_of(:position).only_integer.is_greater_than_or_equal_to(0) }
+
+    context 'when list_type is set to closed' do
+      subject { build(list_factory, list_type: :closed) }
+
+      it { is_expected.not_to validate_presence_of(:label) }
+      it { is_expected.not_to validate_presence_of(:position) }
+    end
+  end
+
+  describe 'scopes' do
+    describe '.ordered' do
+      it 'returns lists ordered by type and position' do
+        # rubocop:disable Rails/SaveBang
+        lists = [
+          create(list_factory, list_type: :backlog),
+          create(list_factory, list_type: :closed),
+          create(list_factory, position: 1),
+          create(list_factory, position: 2)
+        ]
+        # rubocop:enable Rails/SaveBang
+
+        expect(described_class.where(id: lists).ordered).to eq([lists[0], lists[2], lists[3], lists[1]])
+      end
+    end
+  end
+
+  describe '#destroyable?' do
+    it 'returns true when list_type is set to label' do
+      subject.list_type = :label
+
+      expect(subject).to be_destroyable
+    end
+
+    it 'returns false when list_type is set to closed' do
+      subject.list_type = :closed
+
+      expect(subject).not_to be_destroyable
+    end
+  end
+
+  describe '#movable?' do
+    it 'returns true when list_type is set to label' do
+      subject.list_type = :label
+
+      expect(subject).to be_movable
+    end
+
+    it 'returns false when list_type is set to closed' do
+      subject.list_type = :closed
+
+      expect(subject).not_to be_movable
+    end
+  end
+
+  describe '#title' do
+    it 'returns label name when list_type is set to label' do
+      subject.list_type = :label
+      subject.label = Label.new(name: 'Development')
+
+      expect(subject.title).to eq 'Development'
+    end
+
+    it 'returns Closed when list_type is set to closed' do
+      subject.list_type = :closed
+
+      expect(subject.title).to eq 'Closed'
+    end
+  end
+
+  describe '#destroy' do
+    it 'can be destroyed when list_type is set to label' do
+      subject = create(list_factory) # rubocop:disable Rails/SaveBang
+
+      expect(subject.destroy).to be_truthy
+    end
+
+    it 'can not be destroyed when list_type is set to closed' do
+      subject = create(list_factory, list_type: :closed) # rubocop:disable Rails/SaveBang
+
+      expect(subject.destroy).to be_falsey
+    end
+  end
+end