Automatic merge of gitlab-org/gitlab master

GITALY_SERVER_VERSION

-21e7c85471a8d7401fad69be300eaff1c0384577
+82a7a8e90f5bf3f0cae18d158a28eb8a7a1693c6

app/controllers/projects/commit_controller.rb

@@ -52,7 +52,8 @@ class Projects::CommitController < Projects::ApplicationController
   # rubocop: disable CodeReuse/ActiveRecord
   def pipelines
     @pipelines = @commit.pipelines.order(id: :desc)
-    @pipelines = @pipelines.where(ref: params[:ref]).page(params[:page]).per(30) if params[:ref]
+    @pipelines = @pipelines.where(ref: params[:ref]) if params[:ref]
+    @pipelines = @pipelines.page(params[:page])
 
     respond_to do |format|
       format.html

app/controllers/projects/merge_requests_controller.rb

@@ -167,7 +167,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
 
   def pipelines
     set_pipeline_variables
-    @pipelines = @pipelines.page(params[:page]).per(30)
+    @pipelines = @pipelines.page(params[:page])
 
     Gitlab::PollingInterval.set_header(response, interval: 10_000)
 

app/controllers/projects/pipelines_controller.rb

@@ -42,7 +42,6 @@ class Projects::PipelinesController < Projects::ApplicationController
       .new(project, current_user, index_params)
       .execute
       .page(params[:page])
-      .per(20)
 
     @pipelines_count = limited_pipelines_count(project)
 

app/models/ci/pipeline.rb

@@ -29,6 +29,8 @@ module Ci
 
     BridgeStatusError = Class.new(StandardError)
 
+    paginates_per 15
+
     sha_attribute :source_sha
     sha_attribute :target_sha
 

doc/user/discussions/index.md

@@ -65,7 +65,7 @@ You can create a thread without replying to a standard comment.
 Prerequisites:
 
 - You must have at least the [Guest role](../permissions.md#project-members-permissions).
-- You must be in an issue, commit, snippet, or merge request.
+- You must be in an issue, merge request, commit, or snippet.
 
 To create a thread:
 
@@ -95,25 +95,30 @@ You can edit your own comment at any time.
 Anyone with the [Maintainer role](../permissions.md) or
 higher can also edit a comment made by someone else.
 
-## Resolvable comments and threads
+## Resolve a thread
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/5022) in GitLab 8.11.
 > - Resolvable threads can be added only to merge request diffs.
 > - Resolving comments individually was [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/28750) in GitLab 13.6.
 
-Thread resolution helps keep track of progress during planning or code review.
+You can resolve a thread when you want to finish a conversation.
 
-Every thread in merge requests, commits, commit diffs, and
-snippets is initially displayed as unresolved. They can then be individually resolved by anyone
-with at least the Developer role to the project or by the author of the change being reviewed.
-If the thread has been resolved and a non-member un-resolves their own response,
-this also unresolves the discussion thread.
-If the non-member then resolves this same response, this resolves the discussion thread.
+Prerequisites:
+
+- You must have at least the [Developer role](../permissions.md#project-members-permissions)
+  or be the author of the change being reviewed.
+- You must be in an issue, merge request, commit, or snippet.
+
+To resolve a thread:
+
+1. Go to the thread.
+1. Below the last reply, in the **Reply** field, either:
+   - Select **Resolve thread**.
+   - Enter text, select the **Resolve thread** checkbox, and select **Add comment now**.
 
-The need to resolve threads prevents you from forgetting to address feedback and lets you
-hide threads that are no longer relevant.
+At the top of the page, the number of unresolved threads is updated.
 
-!["A thread between two people on a piece of code"](img/thread_view.png)
+![Count of unresolved threads](img/unresolved_threads_v14_1.png)
 
 ### Commit threads in the context of a merge request
 

doc/user/index.md

@@ -136,7 +136,7 @@ merge requests, code snippets, and commits.
 
 When performing inline reviews to implementations
 to your codebase through merge requests you can
-gather feedback through [resolvable threads](discussions/index.md#resolvable-comments-and-threads).
+gather feedback through [resolvable threads](discussions/index.md#resolve-a-thread).
 
 ### GitLab Flavored Markdown (GFM)
 

doc/user/project/merge_requests/index.md

@@ -27,7 +27,7 @@ important parts of the merge request:
 ![Merge request tab positions](img/merge_request_tab_position_v13_11.png)
 
 - **Overview**: Contains the description, notifications from pipelines, and a
-  discussion area for [comment threads](../../discussions/index.md#resolvable-comments-and-threads)
+  discussion area for [comment threads](../../discussions/index.md#resolve-a-thread))
   and [code suggestions](reviews/suggestions.md). The right sidebar provides fields
   to add assignees, reviewers, labels, and a milestone to your work, and the
   [merge request widgets area](widgets.md) reports results from pipelines and tests.

doc/user/project/merge_requests/reviews/index.md

@@ -100,7 +100,7 @@ When you submit your review, GitLab:
 
 ### Resolving/Unresolving threads
 
-Review comments can also resolve or unresolve [resolvable threads](../../../discussions/index.md#resolvable-comments-and-threads).
+Review comments can also resolve or unresolve [resolvable threads](../../../discussions/index.md#resolve-a-thread)).
 When replying to a comment, a checkbox is displayed to resolve or unresolve
 the thread after publication.
 

ee/app/views/admin/audit_logs/index.html.haml

 - page_title _('Audit Events')
 
+%h3.page-title= _('Instance audit events')
+%p.light
+  = _('Track important events in your GitLab instance.')
+  = link_to _('What are instance audit events?'), help_page_path('administration/audit_events.md', anchor: 'instance-events'), target: '_blank', rel: 'noopener noreferrer'
+
 #js-audit-log-app{ data: { form_path: admin_audit_logs_path,
   events: @events.to_json,
   is_last_page: @is_last_page.to_json,

ee/app/views/groups/audit_events/index.html.haml

 - page_title _('Audit Events')
 
-%h3.page-title= _('Group Audit Events')
-%p.light= _('Group-level events in %{group_name} (no project-level events)') % { group_name: @group.name }
+%h3.page-title= _('Group audit events')
+%p.light
+  = _("Track important events in your group.")
+  = link_to _('What are group audit events?'), help_page_path('administration/audit_events.md', anchor: 'group-events'), target: '_blank', rel: 'noopener noreferrer'
 
 #js-group-audit-events-app{ data: { form_path: group_audit_events_path(@group),
   events: @events.to_json,

ee/app/views/projects/audit_events/index.html.haml

@@ -3,9 +3,10 @@
 
 - if feature_available
   %h3.page-title
-    = _('Project Audit Events')
+    = _('Project audit events')
   %p.light
-    = _('Events in %{project_path}') % { project_path: @project.full_path }
+    = _('Track important events in your project.')
+    = link_to _('What are project audit events?'), help_page_path('administration/audit_events.md', anchor: 'project-events'), target: '_blank', rel: 'noopener noreferrer'
 
   #js-project-audit-events-app{ data: { form_path: project_audit_events_path(@project),
   events: @events.to_json,

ee/spec/features/projects/audit_events_spec.rb

@@ -60,10 +60,10 @@ RSpec.describe 'Projects > Audit Events', :js do
       expect(page).to have_link('Audit Events')
     end
 
-    it 'does not have Project Audit Events in the header' do
+    it 'does not have Project audit events in the header' do
       visit project_audit_events_path(project)
 
-      expect(page).not_to have_content('Project Audit Events')
+      expect(page).not_to have_content('Project audit events')
     end
   end
 
@@ -73,10 +73,10 @@ RSpec.describe 'Projects > Audit Events', :js do
     expect(page).to have_link('Audit Events')
   end
 
-  it 'has Project Audit Events in the header' do
+  it 'has Project audit events in the header' do
     visit project_audit_events_path(project)
 
-    expect(page).to have_content('Project Audit Events')
+    expect(page).to have_content('Project audit events')
   end
 
   describe 'adding an SSH key' do

lib/gitlab/pagination/offset_pagination.rb

@@ -19,11 +19,10 @@ module Gitlab
       private
 
       def paginate_with_limit_optimization(relation)
-        # do not paginate relation if it is already paginated
-        pagination_data = if relation.respond_to?(:current_page) && relation.current_page == params[:page] && relation.limit_value == params[:per_page]
-                            relation
-                          else
+        pagination_data = if needs_pagination?(relation)
                             relation.page(params[:page]).per(params[:per_page])
+                          else
+                            relation
                           end
 
         return pagination_data unless pagination_data.is_a?(ActiveRecord::Relation)
@@ -39,6 +38,14 @@ module Gitlab
         end
       end
 
+      def needs_pagination?(relation)
+        return true unless relation.respond_to?(:current_page)
+        return true if params[:page].present? && relation.current_page != params[:page].to_i
+        return true if params[:per_page].present? && relation.limit_value != params[:per_page].to_i
+
+        false
+      end
+
       def add_default_order(relation)
         if relation.is_a?(ActiveRecord::Relation) && relation.order_values.empty?
           relation = relation.order(:id) # rubocop: disable CodeReuse/ActiveRecord

locale/gitlab.pot

@@ -12979,9 +12979,6 @@ msgstr ""
 msgid "Events"
 msgstr ""
 
-msgid "Events in %{project_path}"
-msgstr ""
-
 msgid "Every %{action} attempt has failed: %{job_error_message}. Please try again."
 msgstr ""
 
@@ -15256,9 +15253,6 @@ msgstr ""
 msgid "Group %{group_name} was successfully created."
 msgstr ""
 
-msgid "Group Audit Events"
-msgstr ""
-
 msgid "Group Git LFS status:"
 msgstr ""
 
@@ -15289,6 +15283,9 @@ msgstr ""
 msgid "Group applications"
 msgstr ""
 
+msgid "Group audit events"
+msgstr ""
+
 msgid "Group avatar"
 msgstr ""
 
@@ -15397,9 +15394,6 @@ msgstr ""
 msgid "Group was successfully updated."
 msgstr ""
 
-msgid "Group-level events in %{group_name} (no project-level events)"
-msgstr ""
-
 msgid "Group: %{group_name}"
 msgstr ""
 
@@ -17380,6 +17374,9 @@ msgstr ""
 msgid "Instance administrators group already exists"
 msgstr ""
 
+msgid "Instance audit events"
+msgstr ""
+
 msgid "Instance overview"
 msgstr ""
 
@@ -25211,9 +25208,6 @@ msgstr ""
 msgid "Project Access Tokens"
 msgstr ""
 
-msgid "Project Audit Events"
-msgstr ""
-
 msgid "Project Badges"
 msgstr ""
 
@@ -25241,6 +25235,9 @@ msgstr ""
 msgid "Project and wiki repositories"
 msgstr ""
 
+msgid "Project audit events"
+msgstr ""
+
 msgid "Project avatar"
 msgstr ""
 
@@ -34189,6 +34186,15 @@ msgstr ""
 msgid "Track groups of issues that share a theme, across projects and milestones"
 msgstr ""
 
+msgid "Track important events in your GitLab instance."
+msgstr ""
+
+msgid "Track important events in your group."
+msgstr ""
+
+msgid "Track important events in your project."
+msgstr ""
+
 msgid "Track time with quick actions"
 msgstr ""
 
@@ -36550,6 +36556,15 @@ msgstr ""
 msgid "Welcome, %{name}!"
 msgstr ""
 
+msgid "What are group audit events?"
+msgstr ""
+
+msgid "What are instance audit events?"
+msgstr ""
+
+msgid "What are project audit events?"
+msgstr ""
+
 msgid "What are you searching for?"
 msgstr ""
 

spec/controllers/projects/commit_controller_spec.rb

@@ -483,7 +483,7 @@ RSpec.describe Projects::CommitController do
         end
 
         context 'when rendering a JSON format' do
-          it 'responds with serialized pipelines' do
+          it 'responds with serialized pipelines', :aggregate_failures do
             get_pipelines(id: commit.id, format: :json)
 
             expect(response).to be_ok
@@ -491,6 +491,26 @@ RSpec.describe Projects::CommitController do
             expect(json_response['count']['all']).to eq 1
             expect(response).to include_pagination_headers
           end
+
+          context 'with pagination' do
+            let!(:extra_pipeline) { create(:ci_pipeline, project: project, ref: project.default_branch, sha: commit.sha, status: :running) }
+
+            it 'paginates the result when ref is blank' do
+              allow(Ci::Pipeline).to receive(:default_per_page).and_return(1)
+
+              get_pipelines(id: commit.id, format: :json)
+
+              expect(json_response['pipelines'].count).to eq(1)
+            end
+
+            it 'paginates the result when ref is present' do
+              allow(Ci::Pipeline).to receive(:default_per_page).and_return(1)
+
+              get_pipelines(id: commit.id, ref: project.default_branch, format: :json)
+
+              expect(json_response['pipelines'].count).to eq(1)
+            end
+          end
         end
       end
     end

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -860,6 +860,20 @@ RSpec.describe Projects::MergeRequestsController do
         end
       end
     end
+
+    context 'with pagination' do
+      before do
+        create(:ci_pipeline, project: merge_request.source_project, ref: merge_request.source_branch, sha: merge_request.diff_head_sha)
+      end
+
+      it 'paginates the result' do
+        allow(Ci::Pipeline).to receive(:default_per_page).and_return(1)
+
+        get :pipelines, params: { namespace_id: project.namespace.to_param, project_id: project, id: merge_request.iid }, format: :json
+
+        expect(json_response['pipelines'].count).to eq(1)
+      end
+    end
   end
 
   describe 'GET context commits' do

spec/controllers/projects/pipelines_controller_spec.rb

@@ -66,6 +66,14 @@ RSpec.describe Projects::PipelinesController do
       expect(json_response['pipelines'][0]).not_to include('coverage')
     end
 
+    it 'paginates the result' do
+      allow(Ci::Pipeline).to receive(:default_per_page).and_return(2)
+
+      get_pipelines_index_json
+
+      check_pipeline_response(returned: 2, all: 6)
+    end
+
     context 'when performing gitaly calls', :request_store do
       it 'limits the Gitaly requests' do
         # Isolate from test preparation (Repository#exists? is also cached in RequestStore)

spec/lib/gitlab/pagination/offset_pagination_spec.rb

@@ -130,6 +130,80 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do
           end
         end
 
+        context 'when resource already paginated' do
+          let(:resource) { Project.all.page(1).per(1) }
+
+          context 'when per_page param is specified' do
+            let(:query) { base_query.merge(page: 1, per_page: 2) }
+
+            it 'returns appropriate amount of resources based on per_page param' do
+              expect(subject.paginate(resource).count).to eq 2
+            end
+          end
+
+          context 'when page and per page params are strings' do
+            let(:query) { base_query.merge(page: '1', per_page: '1') }
+
+            it 'returns appropriate amount of resources' do
+              expect(subject.paginate(resource).count).to eq 1
+            end
+          end
+
+          context 'when per_page param is blank' do
+            let(:query) { base_query.merge(page: 1) }
+
+            it 'returns appropriate amount of resources' do
+              expect(subject.paginate(resource).count).to eq 1
+            end
+          end
+
+          context 'when page param is blank' do
+            let(:query) { base_query }
+
+            it 'returns appropriate amount of resources based on resource per(N)' do
+              expect(subject.paginate(resource).count).to eq 1
+            end
+          end
+        end
+
+        context 'when resource does not respond to limit_value' do
+          let(:custom_collection) do
+            Class.new do
+              include Enumerable
+
+              def initialize(items)
+                @collection = items
+              end
+
+              def each
+                @collection.each { |item| yield item }
+              end
+
+              def page(number)
+                Kaminari.paginate_array(@collection).page(number)
+              end
+            end
+          end
+
+          let(:resource) { custom_collection.new(Project.all).page(query[:page]) }
+
+          context 'when page param is blank' do
+            let(:query) { base_query }
+
+            it 'returns appropriate amount of resources' do
+              expect(subject.paginate(resource).count).to eq 3
+            end
+          end
+
+          context 'when per_page param is blank' do
+            let(:query) { base_query.merge(page: 1) }
+
+            it 'returns appropriate amount of resources with default per page value' do
+              expect(subject.paginate(resource).count).to eq 3
+            end
+          end
+        end
+
         context 'when resource is a paginatable array' do
           let(:resource) { Kaminari.paginate_array(Project.all.to_a) }
 

spec/models/ci/pipeline_spec.rb

@@ -11,6 +11,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   let_it_be(:namespace) { create_default(:namespace).freeze }
   let_it_be(:project) { create_default(:project, :repository).freeze }
 
+  it 'paginates 15 pipeleines per page' do
+    expect(described_class.default_per_page).to eq(15)
+  end
+
   it_behaves_like 'having unique enum values'
 
   it { is_expected.to belong_to(:project) }