Automatic merge of gitlab-org/gitlab master

app/assets/javascripts/batch_comments/components/review_bar.vue

@@ -26,7 +26,7 @@ export default {
 </script>
 <template>
   <div v-show="draftsCount > 0">
-    <nav class="review-bar-component">
+    <nav class="review-bar-component" data-testid="review_bar_component">
       <div
         class="review-bar-content d-flex gl-justify-content-end"
         data-qa-selector="review_bar_content"

app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js

@@ -75,6 +75,13 @@ export default (IssuableTokenKeys, disableTargetBranchFilter = false) => {
       icon: 'approval',
       tag: '@approved-by',
     },
+    tokenAlternative: {
+      formattedKey: __('Approved-By'),
+      key: 'approved-by',
+      type: 'string',
+      param: 'usernames',
+      symbol: '@',
+    },
     condition: [
       {
         url: 'approved_by_usernames[]=None',
@@ -105,7 +112,11 @@ export default (IssuableTokenKeys, disableTargetBranchFilter = false) => {
 
   const tokenPosition = 3;
   IssuableTokenKeys.tokenKeys.splice(tokenPosition, 0, ...[approvedBy.token]);
-  IssuableTokenKeys.tokenKeysWithAlternative.splice(tokenPosition, 0, ...[approvedBy.token]);
+  IssuableTokenKeys.tokenKeysWithAlternative.splice(
+    tokenPosition,
+    0,
+    ...[approvedBy.token, approvedBy.tokenAlternative],
+  );
   IssuableTokenKeys.conditions.push(...approvedBy.condition);
 
   const environmentToken = {

app/assets/stylesheets/components/batch_comments/review_bar.scss

@@ -2,13 +2,15 @@
   position: fixed;
   bottom: 0;
   left: 0;
-  width: 100%;
-  background: $white;
   z-index: $zindex-dropdown-menu;
-  padding: 7px 0 6px; // to keep aligned with "collapse sidebar" button on the left sidebar
-  border-top: 1px solid $border-color;
+  display: flex;
+  align-items: center;
+  width: 100%;
+  height: $toggle-sidebar-height;
   padding-left: $contextual-sidebar-width;
   padding-right: $gutter_collapsed_width;
+  background: $white;
+  border-top: 1px solid $border-color;
   transition: padding $sidebar-transition-duration;
 
   .page-with-icon-sidebar & {

app/controllers/projects/forks_controller.rb

@@ -17,7 +17,7 @@ class Projects::ForksController < Projects::ApplicationController
   feature_category :source_code_management
 
   before_action do
-    push_frontend_feature_flag(:fork_project_form)
+    push_frontend_feature_flag(:fork_project_form, @project, default_enabled: :yaml)
   end
 
   def index

app/finders/merge_requests_finder.rb

@@ -76,6 +76,7 @@ class MergeRequestsFinder < IssuableFinder
   def filter_negated_items(items)
     items = super(items)
     items = by_negated_reviewer(items)
+    items = by_negated_approved_by(items)
     by_negated_target_branch(items)
   end
 
@@ -119,6 +120,12 @@ class MergeRequestsFinder < IssuableFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  def by_negated_approved_by(items)
+    return items unless not_params[:approved_by_usernames]
+
+    items.not_approved_by_users_with_usernames(not_params[:approved_by_usernames])
+  end
+
   def source_project_id
     @source_project_id ||= params[:source_project_id].presence
   end

app/models/concerns/approvable_base.rb

@@ -24,6 +24,19 @@ module ApprovableBase
         .group(:id)
         .having("COUNT(users.id) = ?", usernames.size)
     end
+
+    scope :not_approved_by_users_with_usernames, -> (usernames) do
+      users = User.where(username: usernames).select(:id)
+      self_table = self.arel_table
+      app_table = Approval.arel_table
+
+      where(
+        Approval.where(approvals: { user_id: users })
+        .where(app_table[:merge_request_id].eq(self_table[:id]))
+        .select('true')
+        .arel.exists.not
+      )
+    end
   end
 
   class_methods do

app/services/projects/protect_default_branch_service.rb

@@ -22,7 +22,7 @@ module Projects
       # Ensure HEAD points to the default branch in case it is not master
       project.change_head(default_branch)
 
-      create_protected_branch if protect_branch?
+      create_protected_branch if protect_branch? && !protected_branch_exists?
     end
 
     def create_protected_branch
@@ -44,6 +44,10 @@ module Projects
         !ProtectedBranch.protected?(project, default_branch)
     end
 
+    def protected_branch_exists?
+      project.protected_branches.find_by_name(default_branch).present?
+    end
+
     def default_branch
       project.default_branch
     end

app/views/groups/_home_panel.html.haml

@@ -23,6 +23,10 @@
     .home-panel-buttons.col-md-12.col-lg-6
       - if current_user
         .gl-display-flex.gl-flex-wrap.gl-lg-justify-content-end.gl-mx-n2{ data: { testid: 'group-buttons' } }
+          - if current_user.admin?
+            = link_to [:admin, @group], class: 'btn btn-default gl-button btn-icon gl-mt-3 gl-mr-2', title: s_('View group in admin area'),
+              data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
+              = sprite_icon('admin')
           - if @notification_setting
             .js-vue-notification-dropdown{ data: { disabled: emails_disabled.to_s, dropdown_items: notification_dropdown_items(@notification_setting).to_json, notification_level: @notification_setting.level, help_page_path: help_page_path('user/profile/notifications'), group_id: @group.id, container_class: 'gl-mx-2 gl-mt-3 gl-vertical-align-top' } }
           - if can_create_subgroups

app/views/projects/_home_panel.html.haml

@@ -47,6 +47,10 @@
     = cache_if(cache_enabled, [@project, :buttons, current_user, @notification_setting], expires_in: 1.day) do
       .project-repo-buttons.gl-display-flex.gl-justify-content-md-end.gl-align-items-start.gl-flex-wrap.gl-mt-5
         - if current_user
+          - if current_user.admin?
+            = link_to [:admin, @project], class: 'btn gl-button btn-icon gl-align-self-start gl-py-2! gl-mr-3', title: s_('View project in admin area'),
+              data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
+              = sprite_icon('admin')
           .gl-display-flex.gl-align-items-start.gl-mr-3
             - if @notification_setting
               .js-vue-notification-dropdown{ data: { button_size: "small", disabled: emails_disabled.to_s, dropdown_items: notification_dropdown_items(@notification_setting).to_json, notification_level: @notification_setting.level, help_page_path: help_page_path('user/profile/notifications'), project_id: @project.id } }

app/views/projects/forks/new.html.haml

 - page_title s_("ForkProject|Fork project")
 
-- if Feature.enabled?(:fork_project_form)
+- if Feature.enabled?(:fork_project_form, @project, default_enabled: :yaml)
   #fork-groups-mount-element{ data: { fork_illustration: image_path('illustrations/project-create-new-sm.svg'),
     endpoint: new_project_fork_path(@project, format: :json),
     new_group_path: new_group_path,

config/feature_flags/development/fork_project_form.yml

@@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/321387
 milestone: '13.10'
 type: development
 group: group::source code
-default_enabled: false
+default_enabled: true

ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js

@@ -37,6 +37,13 @@ const approvers = {
     icon: 'approval',
     tag: '@approver',
   },
+  tokenAlternative: {
+    formattedKey: __('Approver'),
+    key: 'approver',
+    type: 'string',
+    param: 'usernames',
+    symbol: '@',
+  },
 };
 
 export default (IssuableTokenKeys, disableTargetBranchFilter = false) => {
@@ -44,6 +51,10 @@ export default (IssuableTokenKeys, disableTargetBranchFilter = false) => {
   const tokenPosition = 3;
 
   IssuableTokenKeys.tokenKeys.splice(tokenPosition, 0, ...[approvers.token]);
-  IssuableTokenKeys.tokenKeysWithAlternative.splice(tokenPosition, 0, ...[approvers.token]);
+  IssuableTokenKeys.tokenKeysWithAlternative.splice(
+    tokenPosition,
+    0,
+    ...[approvers.token, approvers.tokenAlternative],
+  );
   IssuableTokenKeys.conditions.push(...approvers.condition);
 };

ee/app/services/security/security_orchestration_policies/project_create_service.rb

@@ -13,7 +13,7 @@ module Security
         project.create_security_orchestration_policy_configuration! do |p|
           p.security_policy_management_project_id = policy_project.id
         end
-        create_protected_branch(policy_project)
+        create_or_update_protected_branch(policy_project)
 
         members = add_members(policy_project)
         errors = members.flat_map { |member| member.errors.full_messages }
@@ -25,13 +25,21 @@ module Security
 
       private
 
-      def create_protected_branch(policy_project)
+      def create_or_update_protected_branch(policy_project)
+        protected_branch = policy_project.protected_branches.find_by_name(policy_project.default_branch_or_main)
         params = {
           name: policy_project.default_branch_or_main,
           push_access_levels_attributes: [{ access_level: Gitlab::Access::NO_ACCESS }],
           merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]
         }
 
+        if protected_branch.present?
+          ProtectedBranch::UpdateService
+            .new(policy_project, current_user, params)
+            .execute(protected_branch)
+          return
+        end
+
         ProtectedBranches::CreateService
           .new(policy_project, current_user, params)
           .execute(skip_authorization: true)

ee/spec/services/security/security_orchestration_policies/project_create_service_spec.rb

@@ -23,6 +23,8 @@ RSpec.describe Security::SecurityOrchestrationPolicies::ProjectCreateService do
         expect(project.security_orchestration_policy_configuration.security_policy_management_project).to eq(policy_project)
         expect(policy_project.namespace).to eq(project.namespace)
         expect(policy_project.protected_branches.map(&:name)).to contain_exactly(project.default_branch_or_main)
+        expect(policy_project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER])
+        expect(policy_project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS])
         expect(policy_project.team.developers).to contain_exactly(maintainer)
       end
     end

lib/api/admin/ci/variables.rb

@@ -8,7 +8,7 @@ module API
 
         before { authenticated_as_admin! }
 
-        feature_category :continuous_integration
+        feature_category :pipeline_authoring
 
         namespace 'admin' do
           namespace 'ci' do

lib/api/ci/runner.rb

@@ -7,7 +7,7 @@ module API
 
       content_type :txt, 'text/plain'
 
-      feature_category :continuous_integration
+      feature_category :runner
 
       resource :runners do
         desc 'Registers a new Runner' do

lib/api/variables.rb

@@ -7,7 +7,7 @@ module API
     before { authenticate! }
     before { authorize! :admin_build, user_project }
 
-    feature_category :continuous_integration
+    feature_category :pipeline_authoring
 
     helpers Helpers::VariablesHelpers
 

locale/gitlab.pot

@@ -35860,6 +35860,9 @@ msgstr ""
 msgid "View full log"
 msgstr ""
 
+msgid "View group in admin area"
+msgstr ""
+
 msgid "View group labels"
 msgstr ""
 
@@ -35911,6 +35914,9 @@ msgstr ""
 msgid "View project"
 msgstr ""
 
+msgid "View project in admin area"
+msgstr ""
+
 msgid "View project labels"
 msgstr ""
 

spec/features/merge_request/batch_comments_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe 'Merge request > Batch comments', :js do
     end
 
     it 'has review bar' do
-      expect(page).to have_css('.review-bar-component', visible: false)
+      expect(page).to have_selector('[data-testid="review_bar_component"]', visible: false)
     end
 
     it 'adds draft note' do
@@ -32,7 +32,7 @@ RSpec.describe 'Merge request > Batch comments', :js do
 
       expect(find('.draft-note-component')).to have_content('Line is wrong')
 
-      expect(page).to have_css('.review-bar-component')
+      expect(page).to have_selector('[data-testid="review_bar_component"]')
 
       expect(find('.review-bar-content .btn-confirm')).to have_content('1')
     end

spec/finders/merge_requests_finder_spec.rb

@@ -520,6 +520,44 @@ RSpec.describe MergeRequestsFinder do
         end
       end
 
+      context 'filtering by approved by' do
+        let(:params) { { approved_by_usernames: user2.username } }
+
+        before do
+          create(:approval, merge_request: merge_request3, user: user2)
+        end
+
+        it 'returns merge requests approved by that user' do
+          merge_requests = described_class.new(user, params).execute
+
+          expect(merge_requests).to contain_exactly(merge_request3)
+        end
+
+        context 'not filter' do
+          let(:params) { { not: { approved_by_usernames: user2.username } } }
+
+          it 'returns merge requests not approved by that user' do
+            merge_requests = described_class.new(user, params).execute
+
+            expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request4, merge_request5)
+          end
+        end
+
+        context 'when filtering by author and not approved by' do
+          let(:params) { { not: { approved_by_usernames: user2.username }, author_username: user.username } }
+
+          before do
+            merge_request4.update!(author: user2)
+          end
+
+          it 'returns merge requests authored by user and not approved by user2' do
+            merge_requests = described_class.new(user, params).execute
+
+            expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request5)
+          end
+        end
+      end
+
       context 'filtering by created_at/updated_at' do
         let(:new_project) { create(:project, forked_from_project: project1) }
 

spec/models/concerns/approvable_base_spec.rb

@@ -59,4 +59,25 @@ RSpec.describe ApprovableBase do
       end
     end
   end
+
+  describe '.not_approved_by_users_with_usernames' do
+    subject { MergeRequest.not_approved_by_users_with_usernames([user.username, user2.username]) }
+
+    let!(:merge_request2) { create(:merge_request) }
+    let!(:merge_request3) { create(:merge_request) }
+    let!(:merge_request4) { create(:merge_request) }
+    let(:user2) { create(:user) }
+    let(:user3) { create(:user) }
+
+    before do
+      create(:approval, merge_request: merge_request, user: user)
+      create(:approval, merge_request: merge_request2, user: user2)
+      create(:approval, merge_request: merge_request2, user: user3)
+      create(:approval, merge_request: merge_request4, user: user3)
+    end
+
+    it 'has the merge request that is not approved at all and not approved by either user' do
+      expect(subject).to contain_exactly(merge_request3, merge_request4)
+    end
+  end
 end

spec/services/projects/protect_default_branch_service_spec.rb

@@ -99,6 +99,53 @@ RSpec.describe Projects::ProtectDefaultBranchService do
           .not_to have_received(:create_protected_branch)
       end
     end
+
+    context 'when protected branch does not exist' do
+      before do
+        allow(service)
+          .to receive(:protected_branch_exists?)
+                .and_return(false)
+        allow(service)
+          .to receive(:protect_branch?)
+                .and_return(true)
+      end
+
+      it 'changes the HEAD of the project' do
+        service.protect_default_branch
+
+        expect(project)
+          .to have_received(:change_head)
+      end
+
+      it 'protects the default branch' do
+        service.protect_default_branch
+
+        expect(service)
+          .to have_received(:create_protected_branch)
+      end
+    end
+
+    context 'when protected branch already exists' do
+      before do
+        allow(service)
+          .to receive(:protected_branch_exists?)
+                .and_return(true)
+      end
+
+      it 'changes the HEAD of the project' do
+        service.protect_default_branch
+
+        expect(project)
+          .to have_received(:change_head)
+      end
+
+      it 'does not protect the default branch' do
+        service.protect_default_branch
+
+        expect(service)
+          .not_to have_received(:create_protected_branch)
+      end
+    end
   end
 
   describe '#create_protected_branch' do

spec/views/groups/_home_panel.html.haml_spec.rb

@@ -14,4 +14,30 @@ RSpec.describe 'groups/_home_panel' do
 
     expect(rendered).to have_content("Group ID: #{group.id}")
   end
+
+  context 'admin area link' do
+    it 'renders admin area link for admin' do
+      allow(view).to receive(:current_user).and_return(create(:admin))
+
+      render
+
+      expect(rendered).to have_link(href: admin_group_path(group))
+    end
+
+    it 'does not render admin area link for non-admin' do
+      allow(view).to receive(:current_user).and_return(create(:user))
+
+      render
+
+      expect(rendered).not_to have_link(href: admin_group_path(group))
+    end
+
+    it 'does not render admin area link for anonymous' do
+      allow(view).to receive(:current_user).and_return(nil)
+
+      render
+
+      expect(rendered).not_to have_link(href: admin_group_path(group))
+    end
+  end
 end

spec/views/projects/_home_panel.html.haml_spec.rb

@@ -5,6 +5,38 @@ require 'spec_helper'
 RSpec.describe 'projects/_home_panel' do
   include ProjectForksHelper
 
+  context 'admin area link' do
+    let(:project) { create(:project) }
+
+    before do
+      assign(:project, project)
+    end
+
+    it 'renders admin area link for admin' do
+      allow(view).to receive(:current_user).and_return(create(:admin))
+
+      render
+
+      expect(rendered).to have_link(href: admin_project_path(project))
+    end
+
+    it 'does not render admin area link for non-admin' do
+      allow(view).to receive(:current_user).and_return(create(:user))
+
+      render
+
+      expect(rendered).not_to have_link(href: admin_project_path(project))
+    end
+
+    it 'does not render admin area link for anonymous' do
+      allow(view).to receive(:current_user).and_return(nil)
+
+      render
+
+      expect(rendered).not_to have_link(href: admin_project_path(project))
+    end
+  end
+
   context 'notifications' do
     let(:project) { create(:project) }