Merge branch 'id-filter-by-draft-wip' into 'master'

Filter by draft merge requests

See merge request gitlab-org/gitlab!35942

app/finders/merge_requests_finder.rb

@@ -31,7 +31,7 @@
 #
 class MergeRequestsFinder < IssuableFinder
   def self.scalar_params
-    @scalar_params ||= super + [:wip, :target_branch]
+    @scalar_params ||= super + [:wip, :draft, :target_branch]
   end
 
   def klass
@@ -42,7 +42,7 @@ class MergeRequestsFinder < IssuableFinder
     items = by_commit(super)
     items = by_deployment(items)
     items = by_source_branch(items)
-    items = by_wip(items)
+    items = by_draft(items)
     items = by_target_branch(items)
     by_source_project_id(items)
   end
@@ -88,20 +88,32 @@ class MergeRequestsFinder < IssuableFinder
     items.where(source_project_id: source_project_id)
   end
 
-  def by_wip(items)
-    if params[:wip] == 'yes'
+  def by_draft(items)
+    draft_param = params[:draft] || params[:wip]
+
+    if draft_param == 'yes'
       items.where(wip_match(items.arel_table))
-    elsif params[:wip] == 'no'
+    elsif draft_param == 'no'
       items.where.not(wip_match(items.arel_table))
     else
       items
     end
   end
 
+  # WIP is deprecated in favor of Draft. Currently both options are supported
   def wip_match(table)
+    items =
       table[:title].matches('WIP:%')
         .or(table[:title].matches('WIP %'))
         .or(table[:title].matches('[WIP]%'))
+
+    return items unless Feature.enabled?(:merge_request_draft_filter)
+
+    items
+      .or(table[:title].matches('Draft - %'))
+      .or(table[:title].matches('Draft:%'))
+      .or(table[:title].matches('[Draft]%'))
+      .or(table[:title].matches('(Draft)%'))
   end
 
   def by_deployment(items)

spec/finders/merge_requests_finder_spec.rb

@@ -192,43 +192,59 @@ RSpec.describe MergeRequestsFinder do
         expect(merge_requests).to contain_exactly(merge_request3)
       end
 
-      describe 'WIP state' do
+      describe 'draft state' do
         let!(:wip_merge_request1) { create(:merge_request, :simple, author: user, source_project: project5, target_project: project5, title: 'WIP: thing') }
         let!(:wip_merge_request2) { create(:merge_request, :simple, author: user, source_project: project6, target_project: project6, title: 'wip thing') }
         let!(:wip_merge_request3) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project1, title: '[wip] thing') }
         let!(:wip_merge_request4) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project2, title: 'wip: thing') }
+        let!(:draft_merge_request1) { create(:merge_request, :simple, author: user, source_branch: 'draft1', source_project: project5, target_project: project5, title: 'Draft: thing') }
+        let!(:draft_merge_request2) { create(:merge_request, :simple, author: user, source_branch: 'draft2', source_project: project6, target_project: project6, title: '[draft] thing') }
+        let!(:draft_merge_request3) { create(:merge_request, :simple, author: user, source_branch: 'draft3', source_project: project1, target_project: project1, title: '(draft) thing') }
+        let!(:draft_merge_request4) { create(:merge_request, :simple, author: user, source_branch: 'draft4', source_project: project1, target_project: project2, title: 'Draft - thing') }
 
-        it 'filters by wip' do
-          params = { wip: 'yes' }
+        [:wip, :draft].each do |draft_param_key|
+          it "filters by #{draft_param_key}" do
+            params = { draft_param_key => 'yes' }
 
             merge_requests = described_class.new(user, params).execute
 
-          expect(merge_requests).to contain_exactly(merge_request4, merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3, wip_merge_request4)
+            expect(merge_requests).to contain_exactly(
+              merge_request4, merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3, wip_merge_request4,
+              draft_merge_request1, draft_merge_request2, draft_merge_request3, draft_merge_request4
+            )
+          end
+
+          context 'when merge_request_draft_filter is disabled' do
+            it 'does not include draft merge requests' do
+              stub_feature_flags(merge_request_draft_filter: false)
+
+              merge_requests = described_class.new(user, { draft_param_key => 'yes' }).execute
+
+              expect(merge_requests).to contain_exactly(
+                merge_request4, merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3, wip_merge_request4
+              )
+            end
           end
 
-        it 'filters by not wip' do
-          params = { wip: 'no' }
+          it "filters by not #{draft_param_key}" do
+            params = { draft_param_key => 'no' }
 
             merge_requests = described_class.new(user, params).execute
 
             expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3)
           end
 
-        it 'returns all items if no valid wip param exists' do
-          params = { wip: '' }
+          it "returns all items if no valid #{draft_param_key} param exists" do
+            params = { draft_param_key => '' }
 
             merge_requests = described_class.new(user, params).execute
 
             expect(merge_requests).to contain_exactly(
               merge_request1, merge_request2, merge_request3, merge_request4,
-            merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3,
-            wip_merge_request4)
+              merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3, wip_merge_request4,
+              draft_merge_request1, draft_merge_request2, draft_merge_request3, draft_merge_request4
+            )
           end
-
-        it 'adds wip to scalar params' do
-          scalar_params = described_class.scalar_params
-
-          expect(scalar_params).to include(:wip, :assignee_id)
         end
 
         context 'filter by deployment' do
@@ -265,6 +281,14 @@ RSpec.describe MergeRequestsFinder do
         end
       end
 
+      describe '.scalar_params' do
+        it 'contains scalar params related to merge requests' do
+          scalar_params = described_class.scalar_params
+
+          expect(scalar_params).to include(:wip, :draft, :assignee_id)
+        end
+      end
+
       context 'assignee filtering' do
         let(:issuables) { described_class.new(user, params).execute }