Merge branch 'nd/extend-validation-checks' into 'master'

Extend epic work item validation checks

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145916



Merged-by: Aaron Huntsman <ahuntsman@gitlab.com>
Approved-by: Joseph Wambua <jjoshua@gitlab.com>
Approved-by: Aaron Huntsman <ahuntsman@gitlab.com>
Reviewed-by: Nicolas Dular <ndular@gitlab.com>
Co-authored-by: Nicolas Dular <ndular@gitlab.com>

ee/lib/gitlab/epic_work_item_sync/diff.rb

@@ -22,9 +22,15 @@ class Diff
 
       ALLOWED_TIME_RANGE_S = 1.second
 
-      def initialize(epic, work_item)
+      # strict_equal: true
+      #   We expect that a work item is fully synced with the epic, including all relations.
+      # strict_equal: false
+      #   Allows that relations are partially synced. For example when the backfill did not run yet but we already start
+      #   creating related links. We only check against links that have a work item.
+      def initialize(epic, work_item, strict_equal: false)
         @epic = epic
         @work_item = work_item
+        @strict_equal = strict_equal
         @mismatched_attributes = []
       end
 
@@ -33,6 +39,13 @@ def attributes
         check_updated_at
         check_namespace
         check_color
+        check_hierarchy
+        check_relative_position
+        check_start_date_is_fixed
+        check_start_date_fixed
+        check_due_date_is_fixed
+        check_due_date_fixed
+        check_related_epic_links
 
         mismatched_attributes
       end
@@ -64,7 +77,57 @@ def check_color
         mismatched_attributes.push("color")
       end
 
-      attr_reader :epic, :work_item
+      def check_hierarchy
+        return if epic.parent.nil? && work_item.work_item_parent.nil?
+        return if epic.parent&.issue_id == work_item.work_item_parent&.id
+
+        mismatched_attributes.push("parent_id")
+      end
+
+      def check_relative_position
+        return if epic.relative_position.nil? && work_item.parent_link&.relative_position.nil?
+        return if epic.relative_position == work_item.parent_link&.relative_position
+
+        mismatched_attributes.push("relative_position")
+      end
+
+      def check_start_date_is_fixed
+        return if epic.start_date_is_fixed == work_item.dates_source&.start_date_is_fixed
+
+        mismatched_attributes.push("start_date_is_fixed")
+      end
+
+      def check_start_date_fixed
+        return if epic.start_date_fixed == work_item.dates_source&.start_date_fixed
+
+        mismatched_attributes.push("start_date_fixed")
+      end
+
+      def check_due_date_is_fixed
+        return if epic.due_date_is_fixed == work_item.dates_source&.due_date_is_fixed
+
+        mismatched_attributes.push("due_date_is_fixed")
+      end
+
+      def check_due_date_fixed
+        return if epic.due_date_fixed == work_item.dates_source&.due_date_fixed
+
+        mismatched_attributes.push("due_date_fixed")
+      end
+
+      def check_related_epic_links
+        related_epic_issues = epic.unauthorized_related_epics
+        related_epic_issues = related_epic_issues.has_work_item unless strict_equal
+
+        related_epic_issue_ids = related_epic_issues.map(&:issue_id)
+        related_work_item_ids = work_item.related_issues(authorize: false).map(&:id)
+
+        return if related_work_item_ids == related_epic_issue_ids
+
+        mismatched_attributes.push("related_links")
+      end
+
+      attr_reader :epic, :work_item, :strict_equal
       attr_accessor :mismatched_attributes
     end
   end

ee/spec/lib/gitlab/epic_work_item_sync/diff_spec.rb

@@ -3,10 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::EpicWorkItemSync::Diff, feature_category: :team_planning do
+  let(:strict_equal) { false }
+
   describe '#attributes' do
-    subject(:attributes) { described_class.new(epic, work_item).attributes }
+    subject(:attributes) { described_class.new(epic, work_item, strict_equal: strict_equal).attributes }
 
-    let(:epic) { create(:epic, :with_synced_work_item) }
+    let_it_be(:group) { create(:group) }
+    let_it_be_with_reload(:epic) { create(:epic, :with_synced_work_item, group: group) }
     let(:work_item) { epic.work_item }
 
     context 'when epic and work item are equal' do
@@ -86,5 +89,127 @@
         it { is_expected.to include("color") }
       end
     end
+
+    describe 'hierarchy' do
+      let_it_be(:parent_epic) { create(:epic, :with_synced_work_item, group: group) }
+      let_it_be_with_reload(:epic) { create(:epic, :with_synced_work_item, group: group, parent: parent_epic) }
+
+      context 'when epic and work item hierarchy are equal' do
+        before do
+          create(:parent_link, work_item_parent: parent_epic.work_item, work_item: epic.work_item)
+        end
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'when epic and work item hierarchy are not equal' do
+        before do
+          create(:parent_link, work_item_parent: create(:work_item, :epic), work_item: epic.work_item)
+        end
+
+        it { is_expected.to include("parent_id") }
+      end
+
+      context 'when relative_position is equal' do
+        let_it_be_with_reload(:epic) do
+          create(:epic, :with_synced_work_item, group: group, parent: parent_epic, relative_position: 10)
+        end
+
+        before do
+          create(:parent_link, work_item_parent: parent_epic.work_item, work_item: epic.work_item,
+            relative_position: 10)
+        end
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'when relative_position is not equal' do
+        let_it_be_with_reload(:epic) do
+          create(:epic, :with_synced_work_item, group: group, parent: parent_epic, relative_position: 9)
+        end
+
+        before do
+          create(:parent_link, work_item_parent: parent_epic.work_item, work_item: epic.work_item,
+            relative_position: 10)
+        end
+
+        it { is_expected.to include("relative_position") }
+      end
+    end
+
+    describe 'start dates' do
+      let_it_be(:expected_start_date) { Time.current.to_date }
+      let_it_be(:expected_due_date) { expected_start_date + 2.days }
+      let_it_be_with_reload(:epic) do
+        create(:epic, :with_synced_work_item, group: group, start_date_fixed: expected_start_date,
+          due_date_fixed: expected_due_date, start_date_is_fixed: true, due_date_is_fixed: true
+        )
+      end
+
+      context 'when it is equal' do
+        before do
+          create(
+            :work_items_dates_source,
+            :fixed,
+            work_item: work_item,
+            start_date: expected_start_date,
+            due_date: expected_due_date)
+        end
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'when it is different' do
+        it { is_expected.to include("start_date_fixed", "due_date_fixed", "start_date_is_fixed", "due_date_is_fixed") }
+      end
+    end
+
+    describe 'related epic links' do
+      let_it_be_with_reload(:target) { create(:epic, :with_synced_work_item, group: group) }
+      let(:source) { epic }
+
+      context 'when epic and work item related epic links are equal' do
+        before do
+          create(:related_epic_link, source: source, target: target)
+          create(:work_item_link, source: source.work_item, target: target.work_item)
+        end
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'when epic has links without a synced work item and work items has no links' do
+        let_it_be_with_reload(:target) { create(:epic, group: group) }
+
+        before do
+          create(:related_epic_link, source: source, target: target)
+        end
+
+        context 'when strict_equal is false' do
+          it { is_expected.to be_empty }
+        end
+
+        context 'when strict_equal is true' do
+          let(:strict_equal) { true }
+
+          it { is_expected.to include("related_links") }
+        end
+      end
+
+      context 'when work item has no related link but epic has' do
+        before do
+          create(:related_epic_link, source: source, target: target)
+        end
+
+        it { is_expected.to include("related_links") }
+      end
+
+      context 'when eipc has no related link but the work item has' do
+        before do
+          create(:work_item_link, source: source.work_item, target: target.work_item)
+        end
+
+        it { is_expected.to include("related_links") }
+      end
+    end
   end
 end