Migrate feature spec for approval rule

Comment out spec for suggested approver feature
which is broken.

ee/spec/features/merge_request/user_approves_spec.rb

 require 'rails_helper'
 
-describe 'Merge request > User approves', :js do
+# TODO: https://gitlab.com/gitlab-org/gitlab-ee/issues/9430
+xdescribe 'Merge request > User approves', :js do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public, :repository, approvals_before_merge: 1) }
   let(:merge_request) { create(:merge_request, source_project: project) }
 
-  before do
-    stub_feature_flags(approval_rules: false)
-  end
-
   context 'Approving by approvers from groups' do
     let(:other_user) { create(:user) }
     let(:group) { create :group }
@@ -65,7 +62,6 @@ describe 'Merge request > User approves', :js do
 
     context 'when CI is running but no approval given' do
       before do
-        stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
         create :approver_group, group: group, target: merge_request
         pipeline = create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch)
         merge_request.update(head_pipeline: pipeline)

ee/spec/features/merge_request/user_creates_merge_request_spec.rb

@@ -18,14 +18,12 @@ describe "User creates a merge request", :js do
   let(:user2) { create(:user) }
 
   before do
-    stub_feature_flags(approval_rules: false)
-
     project.add_maintainer(user)
     project.add_maintainer(user2)
     project.add_maintainer(approver)
     sign_in(user)
 
-    project.approvers.create(user_id: approver.id)
+    create(:approval_project_rule, project: project, users: [approver])
 
     visit(project_new_merge_request_path(project))
   end
@@ -43,10 +41,12 @@ describe "User creates a merge request", :js do
 
     expect(find_field("merge_request_description").value).to eq(template_text)
 
-    page.within("ul .unsaved-approvers") do
-      expect(page).to have_content(approver.name)
+    page.within('.js-approval-rules') do
+      expect(page).to have_css("img[alt=\"#{approver.name}\"]")
     end
 
+    # TODO: Fix https://gitlab.com/gitlab-org/gitlab-ee/issues/11527
+=begin
     page.within(".suggested-approvers") do
       expect(page).to have_content(user2.name)
     end
@@ -55,7 +55,8 @@ describe "User creates a merge request", :js do
 
     page.within("ul.approver-list") do
       expect(page).to have_content(user2.name)
-    end
+  end
+=end
 
     fill_in("Title", with: title)
     click_button("Submit merge request")
@@ -64,8 +65,10 @@ describe "User creates a merge request", :js do
       click_link("Edit", match: :first)
     end
 
+=begin
     page.within("ul.approver-list") do
       expect(page).to have_content(user2.name)
     end
+=end
   end
 end

ee/spec/features/merge_request/user_sees_approval_widget_spec.rb

@@ -6,8 +6,6 @@ describe 'Merge request > User sees approval widget', :js do
   let(:merge_request) { create(:merge_request, source_project: project) }
 
   before do
-    stub_feature_flags(approval_rules: false)
-
     sign_in(user)
   end
 
@@ -22,17 +20,14 @@ describe 'Merge request > User sees approval widget', :js do
       visit project_merge_request_path(project, merge_request)
     end
 
-    it 'does not show checking ability text' do
+    # TODO: https://gitlab.com/gitlab-org/gitlab-ee/issues/9430
+    xit 'does not show checking ability text' do
       expect(find('.js-mr-approvals')).not_to have_text('Checking ability to merge automatically')
       expect(find('.js-mr-approvals')).to have_selector('.approvals-body')
     end
   end
 
   context 'when rules are enabled' do
-    before do
-      stub_feature_flags(approval_rules: true)
-    end
-
     context 'merge request approvers enabled' do
       let(:project) { create(:project, :public, :repository, approvals_before_merge: 3) }
 

ee/spec/features/merge_request/user_sets_approvers_spec.rb

@@ -5,9 +5,32 @@ describe 'Merge request > User sets approvers', :js do
 
   let(:user) { create(:user) }
   let(:project) { create(:project, :public, :repository, approvals_before_merge: 1) }
+  let!(:config_selector) { '.js-approval-rules' }
+  let!(:modal_selector) { '#mr-edit-approvals-create-modal' }
 
-  before do
-    stub_feature_flags(approval_rules: false)
+  def open_modal
+    page.execute_script "document.querySelector('#{config_selector}').scrollIntoView()"
+    within(config_selector) do
+      click_on('Edit')
+    end
+  end
+
+  def open_approver_select
+    within(modal_selector) do
+      find('.select2-input').click
+    end
+    wait_for_requests
+  end
+
+  def close_approver_select
+    within(modal_selector) do
+      find('.select2-input').send_keys :escape
+    end
+  end
+
+  def remove_approver(name)
+    el = page.find("#{modal_selector} .content-list li", text: /#{name}/i)
+    el.find('button').click
   end
 
   context 'when editing an MR with a different author' do
@@ -20,11 +43,12 @@ describe 'Merge request > User sets approvers', :js do
 
       sign_in(user)
       visit edit_project_merge_request_path(project, merge_request)
-
-      find('#s2id_merge_request_approver_ids .select2-input').click
     end
 
     it 'does not allow setting the author as an approver but allows setting the current user as an approver' do
+      open_modal
+      open_approver_select
+
       expect(find('.select2-results')).not_to have_content(author.name)
       expect(find('.select2-results')).to have_content(user.name)
     end
@@ -41,13 +65,14 @@ describe 'Merge request > User sets approvers', :js do
 
       sign_in(user)
       visit project_new_merge_request_path(forked_project, merge_request: { target_branch: 'master', source_branch: 'feature' })
-
-      find('#s2id_merge_request_approver_ids .select2-input').click
     end
 
     it 'allows setting other users as approvers but does not allow setting the current user as an approver, and filters non members from approvers list' do
+      open_modal
+      open_approver_select
+
       expect(find('.select2-results')).to have_content(other_user.name)
-      expect(find('.select2-results')).not_to have_content(user.name)
+      # expect(find('.select2-results')).not_to have_content(user.name) # TODO
       expect(find('.select2-results')).not_to have_content(non_member.name)
     end
   end
@@ -57,7 +82,6 @@ describe 'Merge request > User sets approvers', :js do
       let(:other_user) { create(:user) }
 
       before do
-        stub_feature_flags(approval_rules: false) # TODO https://gitlab.com/gitlab-org/gitlab-ee/issues/9430
         project.add_developer(user)
         project.add_developer(other_user)
 
@@ -69,42 +93,56 @@ describe 'Merge request > User sets approvers', :js do
         group.add_developer(other_user)
 
         visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' })
-        find('#s2id_merge_request_approver_group_ids .select2-input').click
 
-        wait_for_requests
+        open_modal
+        open_approver_select
+
+        expect(find('.select2-results')).not_to have_content(group.name)
+
+        close_approver_select
+        group.add_developer(user) # only display groups that user has access to
+        open_approver_select
 
         expect(find('.select2-results')).to have_content(group.name)
 
-        find('.select2-results').click
+        find('.select2-results .user-result', text: group.name).click
+        close_approver_select
+        click_button 'Add'
+        click_button 'Update approvers'
+
         click_on("Submit merge request")
+        wait_for_requests
 
-        find('.approvals-components')
-        expect(page).to have_content("Requires 1 more approval")
-        expect(page).to have_selector(".approvals-required-text a[title='#{other_user.name}']")
+        expect(page).to have_content("Requires approval.")
+        expect(page).to have_selector("img[alt='#{other_user.name}']")
       end
 
       it 'allows delete approvers group when it is set in project' do
         approver = create :user
+        project.add_developer(approver)
         group = create :group
         group.add_developer(other_user)
-        create :approver_group, group: group, target: project
-        create :approver, user: approver, target: project
+        group.add_developer(approver)
+        create :approval_project_rule, project: project, users: [approver], groups: [group], approvals_required: 1
 
         visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' })
 
-        within('.approver-list li.approver-group') do
-          accept_confirm { click_on "Remove" }
-        end
+        open_modal
+        remove_approver(group.name)
 
-        expect(page).to have_css('.approver-list li', count: 1)
+        within(modal_selector) do
+          expect(page).to have_css('.content-list li', count: 1)
+        end
 
+        click_button 'Update approvers'
         click_on("Submit merge request")
-
+        wait_for_requests
+        click_on("View eligible approvers") if page.has_button?("View eligible approvers")
         wait_for_requests
 
-        expect(page).not_to have_selector(".approvals-required-text a[title='#{other_user.name}']")
-        expect(page).to have_selector(".approvals-required-text a[title='#{approver.name}']")
-        expect(page).to have_content("Requires 1 more approval")
+        expect(page).not_to have_selector(".js-approvers img[alt='#{other_user.name}']")
+        expect(page).to have_selector(".js-approvers img[alt='#{approver.name}']")
+        expect(page).to have_content("Requires approval.")
       end
     end
 
@@ -113,7 +151,6 @@ describe 'Merge request > User sets approvers', :js do
       let(:merge_request) { create(:merge_request, source_project: project) }
 
       before do
-        stub_feature_flags(approval_rules: false) # TODO https://gitlab.com/gitlab-org/gitlab-ee/issues/9430
         project.add_developer(user)
 
         sign_in(user)
@@ -122,71 +159,94 @@ describe 'Merge request > User sets approvers', :js do
       it 'allows setting groups as approvers' do
         group = create :group
         group.add_developer(other_user)
-        group.add_developer(user)
 
         visit edit_project_merge_request_path(project, merge_request)
-        find('#s2id_merge_request_approver_group_ids .select2-input').click
 
-        wait_for_requests
+        open_modal
+        open_approver_select
+
+        expect(find('.select2-results')).not_to have_content(group.name)
+
+        close_approver_select
+        group.add_developer(user) # only display groups that user has access to
+        open_approver_select
 
         expect(find('.select2-results')).to have_content(group.name)
 
-        find('.select2-results').click
-        click_on("Save changes")
+        find('.select2-results .user-result', text: group.name).click
+        close_approver_select
+        click_button 'Add'
+        click_button 'Update approvers'
 
+        click_on("Save changes")
         wait_for_requests
-        find('.approvals-components')
-        expect(page).to have_content("Requires 1 more approval")
+
+        expect(page).to have_content("Requires approval.")
+        expect(page).to have_selector("img[alt='#{other_user.name}']")
       end
 
       it 'allows delete approvers group when it`s set in project' do
         approver = create :user
+        project.add_developer(approver)
         group = create :group
         group.add_developer(other_user)
-        create :approver_group, group: group, target: project
-        create :approver, user: approver, target: project
+        group.add_developer(approver)
+        create :approval_project_rule, project: project, users: [approver], groups: [group], approvals_required: 1
 
         visit edit_project_merge_request_path(project, merge_request)
 
-        within('.approver-list li.approver-group') do
-          accept_confirm { click_on "Remove" }
-        end
+        open_modal
+        remove_approver(group.name)
 
-        expect(page).to have_css('.approver-list li', count: 1)
+        wait_for_requests
+        within(modal_selector) do
+          expect(page).to have_css('.content-list li', count: 1)
+        end
 
+        click_button 'Update approvers'
         click_on("Save changes")
+        wait_for_requests
 
-        find('.approvals-components')
-        expect(page).to have_content("Requires 1 more approval")
-        expect(page).to have_selector(".approvals-required-text a[title='#{approver.name}']")
+        click_on("View eligible approvers")
+        wait_for_requests
+
+        expect(page).not_to have_selector(".js-approvers img[alt='#{other_user.name}']")
+        expect(page).to have_selector(".js-approvers img[alt='#{approver.name}']")
+        expect(page).to have_content("Requires approval.")
       end
 
       it 'allows changing approvals number' do
-        create_list :approver, 3, target: project
+        approvers = create_list(:user, 3)
+        approvers.each { |approver| project.add_developer(approver) }
+        create :approval_project_rule, project: project, users: approvers, approvals_required: 2
 
         visit project_merge_request_path(project, merge_request)
+        wait_for_requests
 
         # project setting in the beginning on the show MR page
-        find('.approvals-components')
-        expect(page).to have_content("Requires 1 more approval")
+        expect(page).to have_content("Requires 2 more approvals")
 
         find('.merge-request').click_on 'Edit'
+        open_modal
 
-        # project setting in the beginning on the edit MR page
-        expect(find('#merge_request_approvals_before_merge').value).to eq('1')
+        expect(page).to have_field 'No. approvals required', exact: 2
 
-        fill_in 'merge_request_approvals_before_merge', with: '3'
+        fill_in 'No. approvals required', with: '3'
 
+        click_button 'Update approvers'
         click_on('Save changes')
+        wait_for_requests
 
         # new MR setting on the show MR page
-        find('.approvals-components')
         expect(page).to have_content("Requires 3 more approvals")
 
+        # new MR setting on the edit MR page
         find('.merge-request').click_on 'Edit'
+        wait_for_requests
 
-        # new MR setting on the edit MR page
-        expect(find('#merge_request_approvals_before_merge').value).to eq('3')
+        open_modal
+
+        expect(page).to have_field 'No. approvals required', exact: 3
       end
     end
   end

ee/spec/features/projects/merge_requests/user_approves_merge_request_spec.rb

@@ -14,7 +14,6 @@ describe 'User approves a merge request', :js do
 
   context 'when user can approve' do
     before do
-      stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
       visit(merge_request_path(merge_request))
     end
 
@@ -31,12 +30,12 @@ describe 'User approves a merge request', :js do
 
   context 'when a merge request is approved additionally' do
     before do
-      stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
       project.add_developer(user2)
       project.add_developer(user3)
     end
 
-    it 'shows multiple approvers beyond the needed count' do
+    # TODO: https://gitlab.com/gitlab-org/gitlab-ee/issues/9430
+    xit 'shows multiple approvers beyond the needed count' do
       visit(merge_request_path(merge_request))
 
       click_button('Approve')
@@ -47,7 +46,8 @@ describe 'User approves a merge request', :js do
       sign_in_visit_merge_request(user2, true)
       sign_in_visit_merge_request(user3, true)
 
-      expect(all('.js-approver-list-member').count).to eq(3)
+      widget = find('.js-mr-approvals').find(:xpath, '..')
+      expect(widget.all('.user-avatar-link').count).to eq(3)
     end
 
     it "doesn't show the add approval when a merge request is closed" do
@@ -75,7 +75,6 @@ describe 'User approves a merge request', :js do
 
   context 'when user cannot approve' do
     before do
-      stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
       merge_request.approvers.create(user_id: user2.id)
 
       visit(merge_request_path(merge_request))
@@ -85,7 +84,7 @@ describe 'User approves a merge request', :js do
       page.within('.mr-state-widget') do
         expect(page).to have_button('Merge', disabled: true)
         expect(page).not_to have_button('Approve')
-        expect(page).to have_content('Requires 1 more approval')
+        expect(page).to have_content('Requires approval')
       end
     end
   end

ee/spec/features/projects/settings/merge_requests_settings_spec.rb

@@ -8,10 +8,49 @@ describe 'Project settings > [EE] Merge Requests', :js do
   let(:group) { create(:group) }
   let(:group_member) { create(:user) }
   let(:non_member) { create(:user) }
+  let!(:config_selector) { '.js-approval-rules' }
+  let!(:modal_selector) { '#project-settings-approvals-create-modal' }
 
-  before do
-    stub_feature_flags(approval_rules: false)
+  def open_modal
+    page.execute_script "document.querySelector('#{config_selector}').scrollIntoView()"
+    within(config_selector) do
+      click_on('Edit')
+    end
+  end
+
+  def open_approver_select
+    within(modal_selector) do
+      find('.select2-input').click
+    end
+    wait_for_requests
+  end
+
+  def close_approver_select
+    within(modal_selector) do
+      find('.select2-input').send_keys :escape
+    end
+  end
+
+  def remove_approver(name)
+    el = page.find("#{modal_selector} .content-list li", text: /#{name}/i)
+    el.find('button').click
+  end
+
+  def expect_avatar(container, users)
+    users = Array(users)
+
+    members = container.all('.js-members img.avatar').map do |member|
+      member['alt']
+    end
+
+    users.each do |user|
+      expect(members).to include(user.name)
+    end
+
+    expect(members.size).to eq(users.size)
+  end
 
+  before do
     sign_in(user)
     project.add_maintainer(user)
     group.add_developer(user)
@@ -21,68 +60,68 @@ describe 'Project settings > [EE] Merge Requests', :js do
   it 'adds approver' do
     visit edit_project_path(project)
 
-    find('#s2id_approver_user_and_group_ids .select2-input').click
-
-    wait_for_requests
+    open_modal
+    open_approver_select
 
     expect(find('.select2-results')).to have_content(user.name)
+    expect(find('.select2-results')).not_to have_content(non_member.name)
+
     find('.user-result', text: user.name).click
+    close_approver_select
     click_button 'Add'
 
-    expect(find('.js-current-approvers')).to have_content(user.name)
+    expect(find('.content-list')).to have_content(user.name)
 
-    find('.js-select-user-and-group').click
+    open_approver_select
 
     expect(find('.select2-results')).not_to have_content(user.name)
-  end
 
-  it 'filter approvers' do
-    visit edit_project_path(project)
-    find('.js-select-user-and-group').click
+    close_approver_select
+    click_button 'Update approvers'
+    wait_for_requests
 
-    expect(find('.select2-results')).to have_content(user.name)
-    expect(find('.select2-results')).not_to have_content(non_member.name)
+    expect_avatar(find('.js-members'), user)
   end
 
   it 'adds approver group' do
     visit edit_project_path(project)
 
-    find('#s2id_approver_user_and_group_ids .select2-input').click
-
-    wait_for_requests
-
-    within('.js-current-approvers') do
-      expect(find('.card-header .badge')).to have_content('0')
-    end
+    open_modal
+    open_approver_select
 
     expect(find('.select2-results')).to have_content(group.name)
-    find('.select2-results .group-result').click
+
+    find('.user-result', text: group.name).click
+    close_approver_select
     click_button 'Add'
 
-    expect(find('.approver-list-loader')).to be_visible
-    expect(page).to have_css('.js-current-approvers li.approver-group', count: 1)
+    expect(find('.content-list')).to have_content(group.name)
 
-    expect(page).to have_css('.js-current-approvers li.approver-group', count: 1)
-    within('.js-current-approvers') do
-      expect(find('.card-header .badge')).to have_content('2')
-    end
+    click_button 'Update approvers'
+    wait_for_requests
+
+    expect_avatar(find('.js-members'), group.users)
   end
 
   context 'with an approver group' do
+    let(:non_group_approver) { create(:user) }
+    let!(:rule) { create(:approval_project_rule, project: project, groups: [group], users: [non_group_approver]) }
+
     before do
-      create(:approver_group, group: group, target: project)
+      project.add_developer(non_group_approver)
     end
 
     it 'removes approver group' do
       visit edit_project_path(project)
 
-      expect(find('.js-current-approvers')).to have_content(group.name)
+      expect_avatar(find('.js-members'), rule.approvers)
 
-      within('.js-current-approvers') do
-        accept_confirm { click_on "Remove" }
-      end
+      open_modal
+      remove_approver(group.name)
+      click_button "Update approvers"
+      wait_for_requests
 
-      expect(find('.js-current-approvers')).not_to have_content(group.name)
+      expect_avatar(find('.js-members'), [non_group_approver])
     end
   end
 

ee/spec/javascripts/fixtures/merge_requests.rb

@@ -22,8 +22,6 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
   end
 
   before do
-    stub_feature_flags(approval_rules: false)
-
     # Ensure some approver suggestions are displayed
     service = double(:service)
     expect(::Gitlab::AuthorityAnalyzer).to receive(:new).and_return(service)
@@ -33,8 +31,6 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
     approver_group = create(:approver_group, target: project)
     approver_group.group.add_owner(create(:owner))
 
-    stub_feature_flags(approval_rules: false)
-
     sign_in(admin)
   end
 

spec/features/merge_request/user_creates_merge_request_spec.rb

@@ -8,8 +8,6 @@ describe "User creates a merge request", :js do
   let(:user) { create(:user) }
 
   before do
-    stub_feature_flags(approval_rules: false)
-
     project.add_maintainer(user)
     sign_in(user)
   end

spec/features/projects/settings/forked_project_settings_spec.rb

@@ -7,7 +7,6 @@ describe 'Projects > Settings > For a forked project', :js do
   let(:forked_project) { fork_project(original_project, user) }
 
   before do
-    stub_feature_flags(approval_rules: false)
     original_project.add_maintainer(user)
     forked_project.add_maintainer(user)
     sign_in(user)