From 4b28157865ee37d47b6fb12d1dec47a7756e1d28 Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Fri, 21 Oct 2022 16:10:46 +1030
Subject: [PATCH 1/8] Add detailed_merge_status to REST API

Changelog: added
---
 app/graphql/types/merge_request_type.rb          |  4 ----
 app/models/merge_request.rb                      | 10 ++++++++--
 ee/app/models/ee/project.rb                      |  3 ++-
 lib/api/entities/merge_request_basic.rb          |  1 +
 .../lib/api/entities/merge_request_basic_spec.rb | 16 ++++++++++------
 5 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb
index c98cfed74938e0..a7f40854f87e80 100644
--- a/app/graphql/types/merge_request_type.rb
+++ b/app/graphql/types/merge_request_type.rb
@@ -291,10 +291,6 @@ def security_auto_fix
     def merge_user
       object.metrics&.merged_by || object.merge_user
     end
-
-    def detailed_merge_status
-      ::MergeRequests::Mergeability::DetailedMergeStatusService.new(merge_request: object).execute
-    end
   end
 end
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 220dd10433543c..81c87a23497777 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -274,6 +274,12 @@ def public_merge_status
     cannot_be_merged_rechecking? || preparing? ? 'checking' : merge_status
   end
 
+  # rubocop: disable CodeReuse/ServiceClass
+  def detailed_merge_status
+    MergeRequests::Mergeability::DetailedMergeStatusService.new(merge_request: self).execute
+  end
+  # rubocop: enable CodeReuse/ServiceClass
+
   validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_or_merged_without_fork?]
   validates :source_branch, presence: true
   validates :target_project, presence: true
@@ -343,8 +349,8 @@ def public_merge_status
   scope :with_api_entity_associations, -> {
     preload_routables
       .preload(:assignees, :author, :unresolved_notes, :labels, :milestone,
-               :timelogs, :latest_merge_request_diff, :reviewers,
-               target_project: :project_feature,
+               :timelogs, :latest_merge_request_diff, :reviewers, :approval_rules,
+               target_project: [:project_feature, :regular_or_any_approver_approval_rules],
                metrics: [:latest_closed_by, :merged_by])
   }
 
diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index edd90fb829465f..0f5cbc9a1e0203 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -62,6 +62,7 @@ def inapplicable_to_branch(branch)
           includes(:protected_branches).reject { |rule| rule.applies_to_branch?(branch) }
         end
       end
+      has_many :regular_or_any_approver_approval_rules, -> { regular_or_any_approver.order(rule_type: :desc, id: :asc) }, class_name: 'ApprovalProjectRule'
       has_many :external_status_checks, class_name: 'MergeRequests::ExternalStatusCheck'
       has_many :approval_merge_request_rules, through: :merge_requests, source: :approval_rules
       has_many :audit_events, as: :entity
@@ -1015,7 +1016,7 @@ def open_source_license_granted?
     def user_defined_rules
       strong_memoize(:user_defined_rules) do
         # Loading the relation in order to memoize it loaded
-        approval_rules.regular_or_any_approver.order(rule_type: :desc, id: :asc).load
+        regular_or_any_approver_approval_rules.load
       end
     end
 
diff --git a/lib/api/entities/merge_request_basic.rb b/lib/api/entities/merge_request_basic.rb
index 55d58166590ac4..3ff91cf1e50536 100644
--- a/lib/api/entities/merge_request_basic.rb
+++ b/lib/api/entities/merge_request_basic.rb
@@ -58,6 +58,7 @@ class MergeRequestBasic < IssuableEntity
         merge_request.check_mergeability(async: true) unless options[:skip_merge_status_recheck]
         merge_request.public_merge_status
       end
+      expose :detailed_merge_status
       expose :diff_head_sha, as: :sha
       expose :merge_commit_sha
       expose :squash_commit_sha
diff --git a/spec/lib/api/entities/merge_request_basic_spec.rb b/spec/lib/api/entities/merge_request_basic_spec.rb
index 40f259b86e2e3b..bb0e25d2613211 100644
--- a/spec/lib/api/entities/merge_request_basic_spec.rb
+++ b/spec/lib/api/entities/merge_request_basic_spec.rb
@@ -18,12 +18,16 @@ def present(obj)
 
   subject { entity.as_json }
 
-  it 'includes basic fields' do
-    is_expected.to include(
-      draft: merge_request.draft?,
-      work_in_progress: merge_request.draft?,
-      merge_user: nil
-    )
+  it 'includes expected fields' do
+    expected_fields = %i[
+      merged_by merge_user merged_at closed_by closed_at target_branch user_notes_count upvotes downvotes
+      author assignees assignee reviewers source_project_id target_project_id labels draft work_in_progress
+      milestone merge_when_pipeline_succeeds merge_status detailed_merge_status sha merge_commit_sha
+      squash_commit_sha discussion_locked should_remove_source_branch force_remove_source_branch
+      reference references web_url time_stats squash task_completion_status has_conflicts blocking_discussions_resolved
+    ]
+
+    is_expected.to include(*expected_fields)
   end
 
   context "with :with_api_entity_associations scope" do
-- 
GitLab


From f06362aeeb30d4031fdc8b247547eb719892d8f8 Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Tue, 25 Oct 2022 20:22:18 +1030
Subject: [PATCH 2/8] Reuse extensions with the new association as well

---
 ee/app/models/ee/project.rb | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index 0f5cbc9a1e0203..b3308c5d1d2164 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -16,6 +16,16 @@ module Project
     GIT_LFS_DOWNLOAD_OPERATION = 'download'
     ISSUE_BATCH_SIZE = 500
 
+    module FilterByBranch
+      def applicable_to_branch(branch)
+        includes(:protected_branches).select { |rule| rule.applies_to_branch?(branch) }
+      end
+
+      def inapplicable_to_branch(branch)
+        includes(:protected_branches).reject { |rule| rule.applies_to_branch?(branch) }
+      end
+    end
+
     prepended do
       include Elastic::ProjectsSearch
       include EachBatch
@@ -53,16 +63,9 @@ module Project
       has_many :approvers, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
       has_many :approver_users, through: :approvers, source: :user
       has_many :approver_groups, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-      has_many :approval_rules, class_name: 'ApprovalProjectRule' do
-        def applicable_to_branch(branch)
-          includes(:protected_branches).select { |rule| rule.applies_to_branch?(branch) }
-        end
-
-        def inapplicable_to_branch(branch)
-          includes(:protected_branches).reject { |rule| rule.applies_to_branch?(branch) }
-        end
-      end
-      has_many :regular_or_any_approver_approval_rules, -> { regular_or_any_approver.order(rule_type: :desc, id: :asc) }, class_name: 'ApprovalProjectRule'
+      has_many :approval_rules, class_name: 'ApprovalProjectRule', extend: FilterByBranch
+      # NOTE: This was added to avoid N+1 queries when we load list of MergeRequests
+      has_many :regular_or_any_approver_approval_rules, -> { regular_or_any_approver.order(rule_type: :desc, id: :asc) }, class_name: 'ApprovalProjectRule', extend: FilterByBranch
       has_many :external_status_checks, class_name: 'MergeRequests::ExternalStatusCheck'
       has_many :approval_merge_request_rules, through: :merge_requests, source: :approval_rules
       has_many :audit_events, as: :entity
-- 
GitLab


From 1ce0bb78a76c43895e47653e30387eddd121ba75 Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Wed, 26 Oct 2022 17:13:03 +1030
Subject: [PATCH 3/8] Move detailed_merge_status out of merge_request model

---
 app/graphql/types/merge_request_type.rb | 4 ++++
 app/models/merge_request.rb             | 6 ------
 lib/api/entities/merge_request_basic.rb | 6 ++++++
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb
index a7f40854f87e80..c98cfed74938e0 100644
--- a/app/graphql/types/merge_request_type.rb
+++ b/app/graphql/types/merge_request_type.rb
@@ -291,6 +291,10 @@ def security_auto_fix
     def merge_user
       object.metrics&.merged_by || object.merge_user
     end
+
+    def detailed_merge_status
+      ::MergeRequests::Mergeability::DetailedMergeStatusService.new(merge_request: object).execute
+    end
   end
 end
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 81c87a23497777..bc018a6ac5bdf8 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -274,12 +274,6 @@ def public_merge_status
     cannot_be_merged_rechecking? || preparing? ? 'checking' : merge_status
   end
 
-  # rubocop: disable CodeReuse/ServiceClass
-  def detailed_merge_status
-    MergeRequests::Mergeability::DetailedMergeStatusService.new(merge_request: self).execute
-  end
-  # rubocop: enable CodeReuse/ServiceClass
-
   validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_or_merged_without_fork?]
   validates :source_branch, presence: true
   validates :target_project, presence: true
diff --git a/lib/api/entities/merge_request_basic.rb b/lib/api/entities/merge_request_basic.rb
index 3ff91cf1e50536..b4742fc841e5cd 100644
--- a/lib/api/entities/merge_request_basic.rb
+++ b/lib/api/entities/merge_request_basic.rb
@@ -94,6 +94,12 @@ class MergeRequestBasic < IssuableEntity
       expose :task_completion_status
       expose :cannot_be_merged?, as: :has_conflicts
       expose :mergeable_discussions_state?, as: :blocking_discussions_resolved
+
+      private
+
+      def detailed_merge_status
+        ::MergeRequests::Mergeability::DetailedMergeStatusService.new(merge_request: merge_request).execute
+      end
     end
   end
 end
-- 
GitLab


From 461a0d46ae16c0b9d09ffde3b99cfd97f00acfba Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Wed, 26 Oct 2022 17:29:09 +1030
Subject: [PATCH 4/8] Update REST API doc to include detailed_merge_status

---
 doc/api/merge_requests.md | 41 +++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 4667e48f233dfb..c6169251bdafcc 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -160,6 +160,7 @@ Supported attributes:
     },
     "merge_when_pipeline_succeeds": true,
     "merge_status": "can_be_merged",
+    "detailed_merge_status": "not_open",
     "sha": "8888888888888888888888888888888888888888",
     "merge_commit_sha": null,
     "squash_commit_sha": null,
@@ -360,6 +361,7 @@ Supported attributes:
     },
     "merge_when_pipeline_succeeds": true,
     "merge_status": "can_be_merged",
+    "detailed_merge_status": "not_open",
     "sha": "8888888888888888888888888888888888888888",
     "merge_commit_sha": null,
     "squash_commit_sha": null,
@@ -547,6 +549,7 @@ Supported attributes:
     },
     "merge_when_pipeline_succeeds": true,
     "merge_status": "can_be_merged",
+    "detailed_merge_status": "not_open",
     "sha": "8888888888888888888888888888888888888888",
     "merge_commit_sha": null,
     "squash_commit_sha": null,
@@ -655,6 +658,7 @@ Supported attributes:
   "milestone": null,
   "merge_when_pipeline_succeeds": false,
   "merge_status": "can_be_merged",
+  "detailed_merge_status": "can_be_merged",
   "sha": "e82eb4a098e32c796079ca3915e07487fc4db24c",
   "merge_commit_sha": null,
   "squash_commit_sha": null,
@@ -766,12 +770,6 @@ the `approvals_before_merge` parameter:
 
 ### Single merge request response notes
 
-- The `merge_status` field may hold one of the following values:
-  - `unchecked`: This merge request has not yet been checked.
-  - `checking`: This merge request is currently being checked to see if it can be merged.
-  - `can_be_merged`: This merge request can be merged without conflict.
-  - `cannot_be_merged`: There are merge conflicts between the source and target branches.
-  - `cannot_be_merged_recheck`: Currently unchecked. Before the current changes, there were conflicts.
 - The `diff_refs` in the response correspond to the latest diff version of the merge request.
 - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/29984) in GitLab 12.8, the mergeability (`merge_status`)
   of each merge request is checked asynchronously when a request is made to this endpoint. Poll this API endpoint
@@ -787,6 +785,29 @@ the `approvals_before_merge` parameter:
 - `pipeline` is an old parameter and should not be used. Use `head_pipeline` instead,
   as it is faster and returns more information.
 
+### Merge Status
+
+- The `merge_status` field may hold one of the following values:
+  - `unchecked`: This merge request has not yet been checked.
+  - `checking`: This merge request is currently being checked to see if it can be merged.
+  - `can_be_merged`: This merge request can be merged without conflict.
+  - `cannot_be_merged`: There are merge conflicts between the source and target branches.
+  - `cannot_be_merged_recheck`: Currently unchecked. Before the current changes, there were conflicts.
+- The `detailed_merge_status` field may hold one of the following values:
+  - `blocked_status`: Merge request is blocked by another merge request.
+  - `broken_status`: Can not merge the source into the target branch, potential conflict.
+  - `checking`: currently checking for mergeability.
+  - `ci_must_pass`: Pipeline must succeed before merging.
+  - `ci_still_runniNG`: Pipeline is still running.
+  - `discussions_not_resolved`: Discussions must be resolved before merging.
+  - `draft_status`: Merge request must not be draft before merging.
+  - `external_status_checks`: Status checks must pass.
+  - `mergeable`: branch can be merged.
+  - `not_approved`: Merge request must be approved before merging.
+  - `not_open`: merge request must be open before merging.
+  - `policies_denied`: There are denied policies for the merge request.
+  - `unchecked`: merge status has not been checked.
+
 ## Get single MR participants
 
 Get a list of merge request participants.
@@ -994,6 +1015,7 @@ Supported attributes:
   },
   "merge_when_pipeline_succeeds": true,
   "merge_status": "can_be_merged",
+  "detailed_merge_status": "can_be_merged",
   "subscribed" : true,
   "sha": "8888888888888888888888888888888888888888",
   "merge_commit_sha": null,
@@ -1211,6 +1233,7 @@ If `approvals_before_merge` is not provided, it inherits the value from the targ
   },
   "merge_when_pipeline_succeeds": true,
   "merge_status": "can_be_merged",
+  "detailed_merge_status": "not_open",
   "merge_error": null,
   "sha": "8888888888888888888888888888888888888888",
   "merge_commit_sha": null,
@@ -1392,6 +1415,7 @@ Must include at least one non-required attribute from above.
   },
   "merge_when_pipeline_succeeds": true,
   "merge_status": "can_be_merged",
+  "detailed_merge_status": "not_open",
   "merge_error": null,
   "sha": "8888888888888888888888888888888888888888",
   "merge_commit_sha": null,
@@ -1580,6 +1604,7 @@ Supported attributes:
   },
   "merge_when_pipeline_succeeds": true,
   "merge_status": "can_be_merged",
+  "detailed_merge_status": "not_open",
   "merge_error": null,
   "sha": "8888888888888888888888888888888888888888",
   "merge_commit_sha": null,
@@ -1792,6 +1817,7 @@ Supported attributes:
   },
   "merge_when_pipeline_succeeds": false,
   "merge_status": "can_be_merged",
+  "detailed_merge_status": "not_open",
   "merge_error": null,
   "sha": "8888888888888888888888888888888888888888",
   "merge_commit_sha": null,
@@ -2124,6 +2150,7 @@ Example response:
   },
   "merge_when_pipeline_succeeds": true,
   "merge_status": "can_be_merged",
+  "detailed_merge_status": "not_open",
   "sha": "8888888888888888888888888888888888888888",
   "merge_commit_sha": null,
   "squash_commit_sha": null,
@@ -2294,6 +2321,7 @@ Example response:
   },
   "merge_when_pipeline_succeeds": true,
   "merge_status": "can_be_merged",
+  "detailed_merge_status": "not_open",
   "sha": "8888888888888888888888888888888888888888",
   "merge_commit_sha": null,
   "squash_commit_sha": null,
@@ -2479,6 +2507,7 @@ Example response:
     },
     "merge_when_pipeline_succeeds": false,
     "merge_status": "unchecked",
+    "detailed_merge_status": "not_open",
     "subscribed": true,
     "sha": "8888888888888888888888888888888888888888",
     "merge_commit_sha": null,
-- 
GitLab


From 1e0df264cf6d9c826bff30029aedbcb985a1457f Mon Sep 17 00:00:00 2001
From: Amy Qualls <aqualls@gitlab.com>
Date: Thu, 27 Oct 2022 00:39:02 +0000
Subject: [PATCH 5/8] Apply 1 suggestion(s) to 1 file(s)

---
 doc/api/merge_requests.md | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index c6169251bdafcc..ba4a3292ce8468 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -785,7 +785,9 @@ the `approvals_before_merge` parameter:
 - `pipeline` is an old parameter and should not be used. Use `head_pipeline` instead,
   as it is faster and returns more information.
 
-### Merge Status
+### Merge status
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/101724) in GitLab 15.6.
 
 - The `merge_status` field may hold one of the following values:
   - `unchecked`: This merge request has not yet been checked.
@@ -798,7 +800,7 @@ the `approvals_before_merge` parameter:
   - `broken_status`: Can not merge the source into the target branch, potential conflict.
   - `checking`: currently checking for mergeability.
   - `ci_must_pass`: Pipeline must succeed before merging.
-  - `ci_still_runniNG`: Pipeline is still running.
+  - `ci_still_running`: Pipeline is still running.
   - `discussions_not_resolved`: Discussions must be resolved before merging.
   - `draft_status`: Merge request must not be draft before merging.
   - `external_status_checks`: Status checks must pass.
-- 
GitLab


From 62fb3bc889b3d13553dc66213eccc12d2c28d55e Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Thu, 27 Oct 2022 11:17:21 +1030
Subject: [PATCH 6/8] Fix an incorrect variable name

---
 lib/api/entities/merge_request_basic.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/api/entities/merge_request_basic.rb b/lib/api/entities/merge_request_basic.rb
index b4742fc841e5cd..27f6e6ade062e2 100644
--- a/lib/api/entities/merge_request_basic.rb
+++ b/lib/api/entities/merge_request_basic.rb
@@ -98,7 +98,7 @@ class MergeRequestBasic < IssuableEntity
       private
 
       def detailed_merge_status
-        ::MergeRequests::Mergeability::DetailedMergeStatusService.new(merge_request: merge_request).execute
+        ::MergeRequests::Mergeability::DetailedMergeStatusService.new(merge_request: object).execute
       end
     end
   end
-- 
GitLab


From ef6f8d1630130ae586f48e1b451052b30412a0f5 Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Thu, 27 Oct 2022 13:47:23 +1030
Subject: [PATCH 7/8] Fix FOSS issue

---
 app/models/merge_request.rb       | 4 ++--
 ee/app/models/ee/merge_request.rb | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index bc018a6ac5bdf8..220dd10433543c 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -343,8 +343,8 @@ def public_merge_status
   scope :with_api_entity_associations, -> {
     preload_routables
       .preload(:assignees, :author, :unresolved_notes, :labels, :milestone,
-               :timelogs, :latest_merge_request_diff, :reviewers, :approval_rules,
-               target_project: [:project_feature, :regular_or_any_approver_approval_rules],
+               :timelogs, :latest_merge_request_diff, :reviewers,
+               target_project: :project_feature,
                metrics: [:latest_closed_by, :merged_by])
   }
 
diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb
index 4b2c7920005118..5d574455705699 100644
--- a/ee/app/models/ee/merge_request.rb
+++ b/ee/app/models/ee/merge_request.rb
@@ -95,7 +95,8 @@ def merge_requests_disable_committers_approval?
     class_methods do
       # This is an ActiveRecord scope in CE
       def with_api_entity_associations
-        super.preload(:blocking_merge_requests, target_project: [group: :saml_provider])
+        super.preload(:blocking_merge_requests, :approval_rules,
+                      target_project: [:regular_or_any_approver_approval_rules, group: :saml_provider])
       end
 
       def sort_by_attribute(method, *args, **kwargs)
-- 
GitLab


From 1645692d4273c2264eec3b4c80c08078946fc713 Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Thu, 27 Oct 2022 17:40:44 +1030
Subject: [PATCH 8/8] Fix failing spec

---
 spec/lib/gitlab/import_export/all_models.yml | 1 +
 spec/requests/api/todos_spec.rb              | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 7a0d25960490b0..c11927150e439d 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -540,6 +540,7 @@ project:
 - jenkins_integration
 - index_status
 - feature_usage
+- regular_or_any_approver_approval_rules
 - approval_rules
 - approval_merge_request_rules
 - approval_merge_request_rule_sources
diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb
index 0fcb6412a2d31c..7a626ee4d2974e 100644
--- a/spec/requests/api/todos_spec.rb
+++ b/spec/requests/api/todos_spec.rb
@@ -231,7 +231,7 @@
       create(:on_commit_todo, project: new_todo.project, author: author_1, user: john_doe, target: merge_request_3)
       create(:todo, project: new_todo.project, author: author_2, user: john_doe, target: merge_request_3)
 
-      expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control1).with_threshold(4)
+      expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control1).with_threshold(5)
       control2 = ActiveRecord::QueryRecorder.new { get api('/todos', john_doe) }
 
       create_issue_todo_for(john_doe)
-- 
GitLab