diff options
40 files changed, 778 insertions, 101 deletions
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index a753d5705aa..6f3c96fa654 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -177,10 +177,8 @@ class Projects::BranchesController < Projects::ApplicationController def fetch_branches_by_mode return fetch_branches_for_overview if @mode == 'overview' - # active/stale/all view mode - @branches = BranchesFinder.new(@repository, params.merge(sort: @sort)).execute - @branches = @branches.select { |b| b.state.to_s == @mode } if %w[active stale].include?(@mode) - @branches = Kaminari.paginate_array(@branches).page(params[:page]) + @branches, @prev_path, @next_path = + Projects::BranchesByModeService.new(@project, params.merge(sort: @sort, mode: @mode)).execute end def fetch_branches_for_overview diff --git a/app/graphql/types/projects/service_type_enum.rb b/app/graphql/types/projects/service_type_enum.rb index 34e06c67be6..fcb36fc233d 100644 --- a/app/graphql/types/projects/service_type_enum.rb +++ b/app/graphql/types/projects/service_type_enum.rb @@ -6,7 +6,7 @@ module Types graphql_name 'ServiceType' ::Service.available_services_types(include_dev: false).each do |service_type| - value service_type.underscore.upcase, value: service_type + value service_type.underscore.upcase, value: service_type, description: "#{service_type} type" end end end diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index 150ec85fca9..59d8f553eff 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -14,6 +14,7 @@ module MergeRequests create_approval_note(merge_request) mark_pending_todos_as_done(merge_request) execute_approval_hooks(merge_request, current_user) + merge_request_activity_counter.track_approve_mr_action(user: current_user) success end diff --git a/app/services/merge_requests/remove_approval_service.rb b/app/services/merge_requests/remove_approval_service.rb index 3164d0b4069..f2bf5de61c1 100644 --- a/app/services/merge_requests/remove_approval_service.rb +++ b/app/services/merge_requests/remove_approval_service.rb @@ -16,6 +16,7 @@ module MergeRequests reset_approvals_cache(merge_request) create_note(merge_request) + merge_request_activity_counter.track_unapprove_mr_action(user: current_user) end success diff --git a/app/services/projects/branches_by_mode_service.rb b/app/services/projects/branches_by_mode_service.rb new file mode 100644 index 00000000000..fb66bfa073b --- /dev/null +++ b/app/services/projects/branches_by_mode_service.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +# Projects::BranchesByModeService uses Gitaly page-token pagination +# in order to optimally fetch branches. +# The drawback of the page-token pagination is that it doesn't provide +# an option of going to the previous page of the collection. +# That's why we need to fall back to offset pagination when previous page +# is requested. +class Projects::BranchesByModeService + include Gitlab::Routing + + attr_reader :project, :params + + def initialize(project, params = {}) + @project = project + @params = params + end + + def execute + return fetch_branches_via_gitaly_pagination if use_gitaly_pagination? + + fetch_branches_via_offset_pagination + end + + private + + def mode + params[:mode] + end + + def by_mode(branches) + return branches unless %w[active stale].include?(mode) + + branches.select { |b| b.state.to_s == mode } + end + + def use_gitaly_pagination? + return false if params[:page].present? || params[:search].present? + + Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: true) + end + + def fetch_branches_via_offset_pagination + branches = BranchesFinder.new(project.repository, params).execute + branches = Kaminari.paginate_array(by_mode(branches)).page(params[:page]) + + branches_with_links(branches, last_page: branches.last_page?) + end + + def fetch_branches_via_gitaly_pagination + per_page = Kaminari.config.default_per_page + options = params.merge(per_page: per_page + 1, page_token: params[:page_token]) + + branches = BranchesFinder.new(project.repository, options).execute(gitaly_pagination: true) + + # Branch is stale if it hasn't been updated for 3 months + # This logic is specified in Gitlab Rails and isn't specified in Gitaly + # To display stale branches we fetch branches sorted as most-stale-at-the-top + # If the result contains active branches we filter them out and define that no more stale branches left + # Same logic applies to fetching active branches + branches = by_mode(branches) + last_page = branches.size <= per_page + + branches = branches.take(per_page) # rubocop:disable CodeReuse/ActiveRecord + + branches_with_links(branches, last_page: last_page) + end + + def branches_with_links(branches, last_page:) + # To fall back to offset pagination we need to track current page via offset param + # And increase it whenever we go to the next page + previous_offset = params[:offset].to_i + + previous_path, next_path = nil, nil + + return [branches, previous_path, next_path] if branches.blank? + + unless last_page + next_path = project_branches_filtered_path(project, state: mode, page_token: branches.last.name, sort: params[:sort], offset: previous_offset + 1) + end + + if previous_offset > 0 + previous_path = project_branches_filtered_path(project, state: mode, sort: params[:sort], page: previous_offset, offset: previous_offset - 1) + end + + [branches, previous_path, next_path] + end +end diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index f383674eb6c..822fe3fea88 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -439,8 +439,6 @@ %span = _('Pages') - = render 'shared/sidebar_toggle_button' - -# Shortcut to Project > Activity %li.hidden = link_to activity_project_path(@project), title: _('Activity'), class: 'shortcuts-project-activity' do @@ -475,3 +473,5 @@ - if project_nav_tab?(:issues) %li.hidden = link_to _('Issue Boards'), project_boards_path(@project), title: _('Issue Boards'), class: 'shortcuts-issue-boards' + + = render 'shared/sidebar_toggle_button' diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index 78f7d1af60f..beccf458138 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -61,7 +61,7 @@ - @branches.each do |branch| = render "projects/branches/branch", branch: branch, merged: @merged_branch_names.include?(branch.name), commit_status: @branch_pipeline_statuses[branch.name], show_commit_status: @branch_pipeline_statuses.any? - if Feature.enabled?(:branches_pagination_without_count, @project, default_enabled: true) - = paginate_without_count @branches + = render('kaminari/gitlab/without_count', previous_path: @prev_path, next_path: @next_path) - else = paginate @branches, theme: 'gitlab' - else diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml index 008340a3fe7..d299d2846c6 100644 --- a/app/views/projects/issues/_new_branch.html.haml +++ b/app/views/projects/issues/_new_branch.html.haml @@ -47,17 +47,17 @@ .form-group %label{ for: 'new-branch-name' } = _('Branch name') - %input#new-branch-name.js-branch-name.form-control{ type: 'text', placeholder: "#{@issue.to_branch_name}", value: "#{@issue.to_branch_name}" } + %input#new-branch-name.js-branch-name.form-control.gl-form-input{ type: 'text', placeholder: "#{@issue.to_branch_name}", value: "#{@issue.to_branch_name}" } %span.js-branch-message.form-text .form-group %label{ for: 'source-name' } = _('Source (branch or tag)') - %input#source-name.js-ref.ref.form-control{ type: 'text', placeholder: "#{@project.default_branch}", value: "#{@project.default_branch}", data: { value: "#{@project.default_branch}" } } + %input#source-name.js-ref.ref.form-control.gl-form-input{ type: 'text', placeholder: "#{@project.default_branch}", value: "#{@project.default_branch}", data: { value: "#{@project.default_branch}" } } %span.js-ref-message.form-text.text-muted .form-group - %button.btn.btn-success.js-create-target{ type: 'button', data: { action: 'create-mr' } } + %button.btn.gl-button.btn-success.js-create-target{ type: 'button', data: { action: 'create-mr' } } = create_mr_text - if can_create_confidential_merge_request? diff --git a/changelogs/unreleased/292825-track-approver-metrics-for-a-merge-request.yml b/changelogs/unreleased/292825-track-approver-metrics-for-a-merge-request.yml new file mode 100644 index 00000000000..7b5e253b7e0 --- /dev/null +++ b/changelogs/unreleased/292825-track-approver-metrics-for-a-merge-request.yml @@ -0,0 +1,5 @@ +--- +title: Add metrics for merge request approvals and revoking approvals +merge_request: 53201 +author: +type: added diff --git a/changelogs/unreleased/gl-button-create-mr.yml b/changelogs/unreleased/gl-button-create-mr.yml new file mode 100644 index 00000000000..850385f4c45 --- /dev/null +++ b/changelogs/unreleased/gl-button-create-mr.yml @@ -0,0 +1,5 @@ +--- +title: Apply new GitLab UI for create mr button from issue +merge_request: 53467 +author: Yogi (@yo) +type: other diff --git a/changelogs/unreleased/id-optimize-branches-index.yml b/changelogs/unreleased/id-optimize-branches-index.yml new file mode 100644 index 00000000000..e79b89d188c --- /dev/null +++ b/changelogs/unreleased/id-optimize-branches-index.yml @@ -0,0 +1,5 @@ +--- +title: Use Gitaly keyset pagination to optimize branches page +merge_request: 53409 +author: +type: performance diff --git a/config/feature_flags/development/batch_suggestions.yml b/config/feature_flags/development/batch_suggestions.yml index b6180a6990b..6a13094f0ab 100644 --- a/config/feature_flags/development/batch_suggestions.yml +++ b/config/feature_flags/development/batch_suggestions.yml @@ -1,7 +1,7 @@ --- name: batch_suggestions introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34782 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320755 milestone: '13.1' type: development group: group::code review diff --git a/config/feature_flags/development/context_commits.yml b/config/feature_flags/development/context_commits.yml index bd71cf49da2..521ab5d0a6f 100644 --- a/config/feature_flags/development/context_commits.yml +++ b/config/feature_flags/development/context_commits.yml @@ -1,7 +1,7 @@ --- name: context_commits introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23701 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320757 milestone: '12.8' type: development group: group::code review diff --git a/config/feature_flags/development/merge_request_draft_filter.yml b/config/feature_flags/development/merge_request_draft_filter.yml index 0614485d37c..3c08e0304d2 100644 --- a/config/feature_flags/development/merge_request_draft_filter.yml +++ b/config/feature_flags/development/merge_request_draft_filter.yml @@ -1,7 +1,7 @@ --- name: merge_request_draft_filter introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/35942 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/298776 milestone: '13.3' type: development group: group::code review diff --git a/config/feature_flags/development/merge_request_rebase_nowait_lock.yml b/config/feature_flags/development/merge_request_rebase_nowait_lock.yml index 73b8fe9ab63..e0d837a3757 100644 --- a/config/feature_flags/development/merge_request_rebase_nowait_lock.yml +++ b/config/feature_flags/development/merge_request_rebase_nowait_lock.yml @@ -1,7 +1,7 @@ --- name: merge_request_rebase_nowait_lock introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/18481 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320758 milestone: '12.5' type: development group: group::code review diff --git a/config/feature_flags/development/remove_resolve_note.yml b/config/feature_flags/development/remove_resolve_note.yml index 0f792dffd0b..008a469e16d 100644 --- a/config/feature_flags/development/remove_resolve_note.yml +++ b/config/feature_flags/development/remove_resolve_note.yml @@ -1,7 +1,7 @@ --- name: remove_resolve_note introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45549 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320756 milestone: '13.6' type: development group: group::code review diff --git a/config/feature_flags/development/usage_data_i_code_review_user_approve_mr.yml b/config/feature_flags/development/usage_data_i_code_review_user_approve_mr.yml new file mode 100644 index 00000000000..c20d201de7c --- /dev/null +++ b/config/feature_flags/development/usage_data_i_code_review_user_approve_mr.yml @@ -0,0 +1,8 @@ +--- +name: usage_data_i_code_review_user_approve_mr +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53201 +rollout_issue_url: +milestone: '13.9' +type: development +group: group::code review +default_enabled: true diff --git a/config/feature_flags/development/usage_data_i_code_review_user_unapprove_mr.yml b/config/feature_flags/development/usage_data_i_code_review_user_unapprove_mr.yml new file mode 100644 index 00000000000..9d31ad6a482 --- /dev/null +++ b/config/feature_flags/development/usage_data_i_code_review_user_unapprove_mr.yml @@ -0,0 +1,8 @@ +--- +name: usage_data_i_code_review_user_unapprove_mr +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53201 +rollout_issue_url: +milestone: '13.9' +type: development +group: group::code review +default_enabled: true diff --git a/doc/.vale/gitlab/Substitutions.yml b/doc/.vale/gitlab/Substitutions.yml index 987785c7a22..ccfbaf351c4 100644 --- a/doc/.vale/gitlab/Substitutions.yml +++ b/doc/.vale/gitlab/Substitutions.yml @@ -20,7 +20,7 @@ swap: param: parameter params: parameters pg: PostgreSQL - postgres: PostgreSQL + 'postgres$': PostgreSQL raketask: Rake task raketasks: Rake tasks rspec: RSpec diff --git a/doc/.vale/gitlab/spelling-exceptions.txt b/doc/.vale/gitlab/spelling-exceptions.txt index c3dc596acb7..29f7bce9a96 100644 --- a/doc/.vale/gitlab/spelling-exceptions.txt +++ b/doc/.vale/gitlab/spelling-exceptions.txt @@ -396,6 +396,7 @@ Poedit polyfill polyfills pooler +postgres.ai PostgreSQL precompile preconfigure diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index fa4b84eb3b7..d276a0a4109 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -23796,41 +23796,184 @@ type ServiceEdge { } enum ServiceType { + """ + AsanaService type + """ ASANA_SERVICE + + """ + AssemblaService type + """ ASSEMBLA_SERVICE + + """ + BambooService type + """ BAMBOO_SERVICE + + """ + BugzillaService type + """ BUGZILLA_SERVICE + + """ + BuildkiteService type + """ BUILDKITE_SERVICE + + """ + CampfireService type + """ CAMPFIRE_SERVICE + + """ + ConfluenceService type + """ CONFLUENCE_SERVICE + + """ + CustomIssueTrackerService type + """ CUSTOM_ISSUE_TRACKER_SERVICE + + """ + DatadogService type + """ DATADOG_SERVICE + + """ + DiscordService type + """ DISCORD_SERVICE + + """ + DroneCiService type + """ DRONE_CI_SERVICE + + """ + EmailsOnPushService type + """ EMAILS_ON_PUSH_SERVICE + + """ + EwmService type + """ EWM_SERVICE + + """ + ExternalWikiService type + """ EXTERNAL_WIKI_SERVICE + + """ + FlowdockService type + """ FLOWDOCK_SERVICE + + """ + GithubService type + """ GITHUB_SERVICE + + """ + HangoutsChatService type + """ HANGOUTS_CHAT_SERVICE + + """ + HipchatService type + """ HIPCHAT_SERVICE + + """ + IrkerService type + """ IRKER_SERVICE + + """ + JenkinsService type + """ JENKINS_SERVICE + + """ + JiraService type + """ JIRA_SERVICE + + """ + MattermostService type + """ MATTERMOST_SERVICE + + """ + MattermostSlashCommandsService type + """ MATTERMOST_SLASH_COMMANDS_SERVICE + + """ + MicrosoftTeamsService type + """ MICROSOFT_TEAMS_SERVICE + + """ + PackagistService type + """ PACKAGIST_SERVICE + + """ + PipelinesEmailService type + """ PIPELINES_EMAIL_SERVICE + + """ + PivotaltrackerService type + """ PIVOTALTRACKER_SERVICE + + """ + PrometheusService type + """ PROMETHEUS_SERVICE + + """ + PushoverService type + """ PUSHOVER_SERVICE + + """ + RedmineService type + """ REDMINE_SERVICE + + """ + SlackService type + """ SLACK_SERVICE + + """ + SlackSlashCommandsService type + """ SLACK_SLASH_COMMANDS_SERVICE + + """ + TeamcityService type + """ TEAMCITY_SERVICE + + """ + UnifyCircuitService type + """ UNIFY_CIRCUIT_SERVICE + + """ + WebexTeamsService type + """ WEBEX_TEAMS_SERVICE + + """ + YoutrackService type + """ YOUTRACK_SERVICE } diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 1101990e0fa..c9cb59dec79 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -68885,217 +68885,217 @@ "enumValues": [ { "name": "ASANA_SERVICE", - "description": null, + "description": "AsanaService type", "isDeprecated": false, "deprecationReason": null }, { "name": "ASSEMBLA_SERVICE", - "description": null, + "description": "AssemblaService type", "isDeprecated": false, "deprecationReason": null }, { "name": "BAMBOO_SERVICE", - "description": null, + "description": "BambooService type", "isDeprecated": false, "deprecationReason": null }, { "name": "BUGZILLA_SERVICE", - "description": null, + "description": "BugzillaService type", "isDeprecated": false, "deprecationReason": null }, { "name": "BUILDKITE_SERVICE", - "description": null, + "description": "BuildkiteService type", "isDeprecated": false, "deprecationReason": null }, { "name": "CAMPFIRE_SERVICE", - "description": null, + "description": "CampfireService type", "isDeprecated": false, "deprecationReason": null }, { "name": "CONFLUENCE_SERVICE", - "description": null, + "description": "ConfluenceService type", "isDeprecated": false, "deprecationReason": null }, { "name": "CUSTOM_ISSUE_TRACKER_SERVICE", - "description": null, + "description": "CustomIssueTrackerService type", "isDeprecated": false, "deprecationReason": null }, { "name": "DATADOG_SERVICE", - "description": null, + "description": "DatadogService type", "isDeprecated": false, "deprecationReason": null }, { "name": "DISCORD_SERVICE", - "description": null, + "description": "DiscordService type", "isDeprecated": false, "deprecationReason": null }, { "name": "DRONE_CI_SERVICE", - "description": null, + "description": "DroneCiService type", "isDeprecated": false, "deprecationReason": null }, { "name": "EMAILS_ON_PUSH_SERVICE", - "description": null, + "description": "EmailsOnPushService type", "isDeprecated": false, "deprecationReason": null }, { "name": "EWM_SERVICE", - "description": null, + "description": "EwmService type", "isDeprecated": false, "deprecationReason": null }, { "name": "EXTERNAL_WIKI_SERVICE", - "description": null, + "description": "ExternalWikiService type", "isDeprecated": false, "deprecationReason": null }, { "name": "FLOWDOCK_SERVICE", - "description": null, + "description": "FlowdockService type", "isDeprecated": false, "deprecationReason": null }, { "name": "GITHUB_SERVICE", - "description": null, + "description": "GithubService type", "isDeprecated": false, "deprecationReason": null }, { "name": "HANGOUTS_CHAT_SERVICE", - "description": null, + "description": "HangoutsChatService type", "isDeprecated": false, "deprecationReason": null }, { "name": "HIPCHAT_SERVICE", - "description": null, + "description": "HipchatService type", "isDeprecated": false, "deprecationReason": null }, { "name": "IRKER_SERVICE", - "description": null, + "description": "IrkerService type", "isDeprecated": false, "deprecationReason": null }, { "name": "JENKINS_SERVICE", - "description": null, + "description": "JenkinsService type", "isDeprecated": false, "deprecationReason": null }, { "name": "JIRA_SERVICE", - "description": null, + "description": "JiraService type", "isDeprecated": false, "deprecationReason": null }, { "name": "MATTERMOST_SERVICE", - "description": null, + "description": "MattermostService type", "isDeprecated": false, "deprecationReason": null }, { "name": "MATTERMOST_SLASH_COMMANDS_SERVICE", - "description": null, + "description": "MattermostSlashCommandsService type", "isDeprecated": false, "deprecationReason": null }, { "name": "MICROSOFT_TEAMS_SERVICE", - "description": null, + "description": "MicrosoftTeamsService type", "isDeprecated": false, "deprecationReason": null }, { "name": "PACKAGIST_SERVICE", - "description": null, + "description": "PackagistService type", "isDeprecated": false, "deprecationReason": null }, { "name": "PIPELINES_EMAIL_SERVICE", - "description": null, + "description": "PipelinesEmailService type", "isDeprecated": false, "deprecationReason": null }, { "name": "PIVOTALTRACKER_SERVICE", - "description": null, + "description": "PivotaltrackerService type", "isDeprecated": false, "deprecationReason": null }, { "name": "PROMETHEUS_SERVICE", - "description": null, + "description": "PrometheusService type", "isDeprecated": false, "deprecationReason": null }, { "name": "PUSHOVER_SERVICE", - "description": null, + "description": "PushoverService type", "isDeprecated": false, "deprecationReason": null }, { "name": "REDMINE_SERVICE", - "description": null, + "description": "RedmineService type", "isDeprecated": false, "deprecationReason": null }, { "name": "SLACK_SERVICE", - "description": null, + "description": "SlackService type", "isDeprecated": false, "deprecationReason": null }, { "name": "SLACK_SLASH_COMMANDS_SERVICE", - "description": null, + "description": "SlackSlashCommandsService type", "isDeprecated": false, "deprecationReason": null }, { "name": "TEAMCITY_SERVICE", - "description": null, + "description": "TeamcityService type", "isDeprecated": false, "deprecationReason": null }, { "name": "UNIFY_CIRCUIT_SERVICE", - "description": null, + "description": "UnifyCircuitService type", "isDeprecated": false, "deprecationReason": null }, { "name": "WEBEX_TEAMS_SERVICE", - "description": null, + "description": "WebexTeamsService type", "isDeprecated": false, "deprecationReason": null }, { "name": "YOUTRACK_SERVICE", - "description": null, + "description": "YoutrackService type", "isDeprecated": false, "deprecationReason": null } diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 95ed4508ba4..7e060b43d54 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -5210,42 +5210,42 @@ State of a Sentry error. | Value | Description | | ----- | ----------- | -| `ASANA_SERVICE` | | -| `ASSEMBLA_SERVICE` | | -| `BAMBOO_SERVICE` | | -| `BUGZILLA_SERVICE` | | -| `BUILDKITE_SERVICE` | | -| `CAMPFIRE_SERVICE` | | -| `CONFLUENCE_SERVICE` | | -| `CUSTOM_ISSUE_TRACKER_SERVICE` | | -| `DATADOG_SERVICE` | | -| `DISCORD_SERVICE` | | -| `DRONE_CI_SERVICE` | | -| `EMAILS_ON_PUSH_SERVICE` | | -| `EWM_SERVICE` | | -| `EXTERNAL_WIKI_SERVICE` | | -| `FLOWDOCK_SERVICE` | | -| `GITHUB_SERVICE` | | -| `HANGOUTS_CHAT_SERVICE` | | -| `HIPCHAT_SERVICE` | | -| `IRKER_SERVICE` | | -| `JENKINS_SERVICE` | | -| `JIRA_SERVICE` | | -| `MATTERMOST_SERVICE` | | -| `MATTERMOST_SLASH_COMMANDS_SERVICE` | | -| `MICROSOFT_TEAMS_SERVICE` | | -| `PACKAGIST_SERVICE` | | -| `PIPELINES_EMAIL_SERVICE` | | -| `PIVOTALTRACKER_SERVICE` | | -| `PROMETHEUS_SERVICE` | | -| `PUSHOVER_SERVICE` | | -| `REDMINE_SERVICE` | | -| `SLACK_SERVICE` | | -| `SLACK_SLASH_COMMANDS_SERVICE` | | -| `TEAMCITY_SERVICE` | | -| `UNIFY_CIRCUIT_SERVICE` | | -| `WEBEX_TEAMS_SERVICE` | | -| `YOUTRACK_SERVICE` | | +| `ASANA_SERVICE` | AsanaService type | +| `ASSEMBLA_SERVICE` | AssemblaService type | +| `BAMBOO_SERVICE` | BambooService type | +| `BUGZILLA_SERVICE` | BugzillaService type | +| `BUILDKITE_SERVICE` | BuildkiteService type | +| `CAMPFIRE_SERVICE` | CampfireService type | +| `CONFLUENCE_SERVICE` | ConfluenceService type | +| `CUSTOM_ISSUE_TRACKER_SERVICE` | CustomIssueTrackerService type | +| `DATADOG_SERVICE` | DatadogService type | +| `DISCORD_SERVICE` | DiscordService type | +| `DRONE_CI_SERVICE` | DroneCiService type | +| `EMAILS_ON_PUSH_SERVICE` | EmailsOnPushService type | +| `EWM_SERVICE` | EwmService type | +| `EXTERNAL_WIKI_SERVICE` | ExternalWikiService type | +| `FLOWDOCK_SERVICE` | FlowdockService type | +| `GITHUB_SERVICE` | GithubService type | +| `HANGOUTS_CHAT_SERVICE` | HangoutsChatService type | +| `HIPCHAT_SERVICE` | HipchatService type | +| `IRKER_SERVICE` | IrkerService type | +| `JENKINS_SERVICE` | JenkinsService type | +| `JIRA_SERVICE` | JiraService type | +| `MATTERMOST_SERVICE` | MattermostService type | +| `MATTERMOST_SLASH_COMMANDS_SERVICE` | MattermostSlashCommandsService type | +| `MICROSOFT_TEAMS_SERVICE` | MicrosoftTeamsService type | +| `PACKAGIST_SERVICE` | PackagistService type | +| `PIPELINES_EMAIL_SERVICE` | PipelinesEmailService type | +| `PIVOTALTRACKER_SERVICE` | PivotaltrackerService type | +| `PROMETHEUS_SERVICE` | PrometheusService type | +| `PUSHOVER_SERVICE` | PushoverService type | +| `REDMINE_SERVICE` | RedmineService type | +| `SLACK_SERVICE` | SlackService type | +| `SLACK_SLASH_COMMANDS_SERVICE` | SlackSlashCommandsService type | +| `TEAMCITY_SERVICE` | TeamcityService type | +| `UNIFY_CIRCUIT_SERVICE` | UnifyCircuitService type | +| `WEBEX_TEAMS_SERVICE` | WebexTeamsService type | +| `YOUTRACK_SERVICE` | YoutrackService type | ### SnippetBlobActionEnum diff --git a/doc/architecture/blueprints/database_testing/index.md b/doc/architecture/blueprints/database_testing/index.md new file mode 100644 index 00000000000..ab4b3837c2f --- /dev/null +++ b/doc/architecture/blueprints/database_testing/index.md @@ -0,0 +1,145 @@ +--- +comments: false +description: 'Database Testing' +--- + +# Database Testing + +We have identified [common themes of reverted migrations](https://gitlab.com/gitlab-org/gitlab/-/issues/233391) and discovered failed migrations breaking in both production and staging even when successfully tested in a developer environment. We have also experienced production incidents even with successful testing in staging. These failures are quite expensive: they can have a significant effect on availability, block deployments, and generate incident escalations. These escalations must be triaged and either reverted or fixed forward. Often, this can take place without the original author's involvement due to time zones and/or the criticality of the escalation. With our increased deployment speeds and stricter uptime requirements, the need for improving database testing is critical, particularly earlier in the development process (shift left). + +From a developer's perspective, it is hard, if not unfeasible, to validate a migration on a large enough dataset before it goes into production. + +Our primary goal is to **provide developers with immediate feedback for new migrations and other database-related changes tested on a full copy of the production database**, and to do so with high levels of efficiency (particularly in terms of infrastructure costs) and security. + +## Current day + +Developers are expected to test database migrations prior to deploying to any environment, but we lack the ability to perform testing against large environments such as GitLab.com. The [developer database migration style guide](../../../development/migration_style_guide.md) provides guidelines on migrations, and we focus on validating migrations during code review and testing in CI and staging. + +The [code review phase](../../../development/database_review.md) involves Database Reviewers and Maintainers to manually check the migrations committed. This often involves knowing and spotting problematic patterns and their particular behavior on GitLab.com from experience. There is no large-scale environment available that allows us to test database migrations before they are being merged. + +Testing in CI is done on a very small database. We mainly check forward/backward migration consistency, evaluate rubocop rules to detect well-known problematic behaviors (static code checking) and have a few other, rather technical checks in place (adding the right files etc). That is, we typically find code or other rather simple errors, but cannot surface any data related errors - which are also typically not covered by unit tests either. + +Once merged, migrations are being deployed to the staging environment. Its database size is less than 5% of the production database size as of January 2021 and its recent data distribution does not resemble the production site. Oftentimes, we see migrations succeed in staging but then fail in production due to query timeouts or other unexpected problems. Even if we caught problems in staging, this is still expensive to reconcile and ideally we want to catch those problems as early as possible in the development cycle. + +Today, we have gained experience with working on a thin-cloned production database (more on this below) and already use it to provide developers with access to production query plans, automated query feedback and suggestions with optimizations. This is built around [Database Lab](https://gitlab.com/postgres-ai/database-lab) and [Joe](https://gitlab.com/postgres-ai/joe), both available through Slack (using chatops) and [postgres.ai](https://postgres.ai/). + +## Vision + +As a developer: + +1. I am working on a GitLab code change that includes a data migration and changes a heavy database query. +1. I push my code, create a merge request, and provide an example query in the description. +1. The pipeline executes the data migration and examines the query in a large-scale environment (a copy of GitLab.com). +1. Once the pipeline finishes, the merge request gets detailed feedback and information about the migration and the query I provided. This is based on a full clone of the production database with a state that is very close to production (minutes). + +For database migrations, the information gathered from execution on the clone includes: + +- Overall runtime. +- Detailed statistics for queries being executed in the migration (normalizing queries and showing their frequencies and execution times as plots). +- Dangerous locks held during the migration (which would cause blocking situations in production). + +For database queries, we can automatically gather: + +- Query plans along with visualization. +- Execution times and predictions for production. +- Suggestions on optimizations from Joe. +- Memory and IO statistics. + +After having gotten that feedback: + +1. I can go back and investigate a performance problem with the data migration. +1. Once I have a fix pushed, I can repeat the above cycle and eventually send my merge request for database review. During the database review, the database reviewer and maintainer have all the additional generated information available to them to make an informed decision on the performance of the introduced changes. + +This information gathering is done in a protected and safe environment, making sure that there is no unauthorized access to production data and we can safely execute code in this environment. + +The intended benefits include: + +- Shifting left: Allow developers to understand large-scale database performance and what to expect to happen on GitLab.com in a self-service manner +- Identify errors that are only generated when working against a production scale dataset with real data (with inconsistencies or unexpected patterns) +- Automate the information gathering phase to make it easier for everybody involved in code review (developer, reviewer, maintainer) by providing relevant details automatically and upfront. + +## Technology and next steps + +We already use Database Lab from [postgres.ai](https://postgres.ai/), which is a thin-cloning technology. We maintain a PostgreSQL replica which is up to date with production data but does not serve any production traffic. This runs Database Lab which allows us to quickly create a full clone of the production dataset (in the order of seconds). + +Internally, this is based on ZFS and implements a "thin-cloning technology". That is, ZFS snapshots are being used to clone the data and it exposes a full read/write PostgreSQL cluster based on the cloned data. This is called a *thin clone*. It is rather short lived and is going to be destroyed again shortly after we are finished using it. + +It is important to note that a thin clone is fully read/write. This allows us to execute migrations on top of it. + +Database Lab provides an API we can interact with to manage thin clones. In order to automate the migration and query testing, we add steps to the `gitlab/gitlab-org` CI pipeline. This triggers automation that performs the following steps for a given merge request: + +1. Create a thin-clone with production data for this testing session. +1. Pull GitLab code from the merge request. +1. Execute migrations and gather all necessary information from it. +1. Execute query testing and gather all necessary information from it. +1. Post back the results of the migration and query testing to the merge request. +1. Destroy the thin-clone. + +### Short-term + +The short-term focus is on testing regular migrations (typically schema changes) and using the existing Database Lab instance from postgres.ai for it. + +In order to secure this process and meet compliance goals, the runner environment will be treated as a *production* environment and similarly locked down, monitored and audited. Only Database Maintainers will have access to the CI pipeline and its job output. Everyone else will only be able to see the results and statistics posted back on the merge request. + +We implement a secured CI pipeline on <https://ops.gitlab.net> that adds the execution steps outlined above. The goal is to secure this pipeline in order to solve the following problem: + +Make sure we strongly protect production data, even though we allow everyone (GitLab team/developers) to execute arbitrary code on the thin-clone which contains production data. + +This is in principle achieved by locking down the GitLab Runner instance executing the code and its containers on a network level, such that no data can escape over the network. We make sure no communication can happen to the outside world from within the container executing the GitLab Rails code (and its database migrations). + +Furthermore, we limit the ability to view the results of the jobs (including the output printed from code) to Maintainer and Owner level on the <https://ops.gitlab.net> pipeline and provide only a high level summary back to the original MR. If there are issues or errors in one of the jobs run, the database Maintainer assigned to review the MR can check the original job for more details. + +With this step implemented, we already have the ability to execute database migrations on the thin-cloned GitLab.com database automatically from GitLab CI and provide feedback back to the merge request and the developer. The content of that feedback is expected to evolve over time and we can continuously add to this. + +We already have an [MVC-style implementation for the pipeline](https://gitlab.com/gitlab-org/database-team/gitlab-com-migrations) for reference and an [example merge request with feedback](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50793#note_477815261) from the pipeline. + +The short-term goal is detailed in [this epic](https://gitlab.com/groups/gitlab-org/database-team/-/epics/6). + +### Mid-term - Improved feedback, query testing and background migration testing + +Mid-term, we plan to expand the level of detail the testing pipeline reports back to the Merge Request and expand its scope to cover query testing, too. By doing so, we use our experience from database code reviews and using thin-clone technology and bring this back closer to the GitLab workflow. Instead of reaching out to different tools (postgres.ai, joe, Slack, plan visualizations etc.) we bring this back to GitLab and working directly on the Merge Request. + +Secondly, we plan to cover background migrations testing, too. These are typically data migrations that are scheduled to run over a long period of time. The success of both the scheduling phase and the job execution phase typically depends a lot on data distribution - which only surfaces when running these migrations on actual production data. In order to become confident about a background migration, we plan to provide the following feedback: + +1. Scheduling phase - query statistics (for example a histogram of query execution times), job statistics (how many jobs, overall duration etc.), batch sizes. +1. Execution phase - using a few instances of a job as examples, we execute those to gather query and runtime statistics. + +### Long-term - incorporate into GitLab product + +There are opportunities to discuss for extracting features from this into GitLab itself. For example, annotating the Merge Request with query examples and attaching feedback gathered from the testing run can become a first-class citizen instead of using Merge Request description and comments for it. We plan to evaluate those ideas as we see those being used in earlier phases and bring our experience back into the product. + +## An alternative discussed: Anonymization + +At the core of this problem lies the concern about executing (potentially arbitrary) code on a production dataset and making sure the production data is well protected. The approach discussed above solves this by strongly limiting access to the output of said code. + +An alternative approach we have discussed and abandoned is to "scrub" and anonymize production data. The idea is to remove any sensitive data from the database and use the resulting dataset for database testing. This has a lot of downsides which led us to abandon the idea: + +- Anonymization is complex by nature - it is a hard problem to call a "scrubbed clone" actually safe to work with in public. Different data types may require different anonymization techniques (e.g. anonymizing sensitive information inside a JSON field) and only focusing on one attribute at a time does not guarantee that a dataset is fully anonymized (for example join attacks or using timestamps in conjunction to public profiles/projects to de-anonymize users by there activity). +- Anonymization requires an additional process to keep track and update the set of attributes considered as sensitive, ongoing maintenance and security reviews every time the database schema changes. +- Annotating data as "sensitive" is error prone, with the wrong anonymization approach used for a data type or one sensitive attribute accidentally not marked as such possibly leading to a data breach. +- Scrubbing not only removes sensitive data, but also changes data distribution, which greatly affects performance of migrations and queries. +- Scrubbing heavily changes the database contents, potentially updating a lot of data, which leads to different data storage details (think MVC bloat), affecting performance of migrations and queries. + +## Who + +This effort is owned and driven by the [GitLab Database Team](https://about.gitlab.com/handbook/engineering/development/enablement/database/) with support from the [GitLab.com Reliability Datastores](https://about.gitlab.com/handbook/engineering/infrastructure/team/reliability/datastores/) team. + +<!-- vale gitlab.Spelling = NO --> + +| Role | Who +|------------------------------|-------------------------| +| Author | Andreas Brandl | +| Architecture Evolution Coach | Gerardo Lopez-Fernandez | +| Engineering Leader | Craig Gomes | +| Domain Expert | Yannis Roussos | +| Domain Expert | Pat Bair | + +DRIs: + +| Role | Who +|------------------------------|------------------------| +| Product | Fabian Zimmer | +| Leadership | Craig Gomes | +| Engineering | Andreas Brandl | + +<!-- vale gitlab.Spelling = YES --> diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 2d865ec3a7e..29a6873d1b6 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -96,18 +96,18 @@ belongs to, as well as an information block as described below: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments ``` -For example, the following metadata would be at the beginning of a product -documentation page whose content is primarily associated with the Audit Events -feature: +For example: ```yaml --- -stage: Monitor -group: APM +stage: Example Stage +group: Example Group info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- ``` +If you need help determining the correct stage, read [Ask for help](workflow.md#ask-for-help). + ### Document type metadata Originally discussed in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/1280), diff --git a/doc/development/documentation/workflow.md b/doc/development/documentation/workflow.md index e809ca84707..8e2028532e4 100644 --- a/doc/development/documentation/workflow.md +++ b/doc/development/documentation/workflow.md @@ -65,13 +65,15 @@ To update GitLab documentation: NOTE: Work in a fork if you do not have Developer access to the GitLab project. -Request help from the Technical Writing team if you: +### Ask for help + +Ask for help from the Technical Writing team if you: - Need help to choose the correct place for documentation. - Want to discuss a documentation idea or outline. - Want to request any other help. -To request help: +To identify someone who can help you: 1. Locate the Technical Writer for the relevant [DevOps stage group](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments). diff --git a/doc/development/usage_ping.md b/doc/development/usage_ping.md index 02a070bc3ad..4b081f59abd 100644 --- a/doc/development/usage_ping.md +++ b/doc/development/usage_ping.md @@ -638,7 +638,7 @@ Next, get the unique events for the current week. ```ruby # Get unique events for metric for current_week Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: 'g_compliance_audit_events', - start_date: Date.current.beginning_of_week, end_date: Date.current.end_of_week) + start_date: Date.current.beginning_of_week, end_date: Date.current.next_week) ``` ##### Recommendations diff --git a/doc/security/two_factor_authentication.md b/doc/security/two_factor_authentication.md index 4911cf63489..f1a7abbfd33 100644 --- a/doc/security/two_factor_authentication.md +++ b/doc/security/two_factor_authentication.md @@ -149,3 +149,8 @@ To disable it: ```ruby Feature.disable(:two_factor_for_cli) ``` + +The feature flag affects these features: + +- [Two-factor Authentication (2FA) for Git over SSH operations](#two-factor-authentication-2fa-for-git-over-ssh-operations). +- [Customize session duration for Git Operations when 2FA is enabled](../user/admin_area/settings/account_and_limit_settings.md#customize-session-duration-for-git-operations-when-2fa-is-enabled). diff --git a/doc/topics/autodevops/stages.md b/doc/topics/autodevops/stages.md index e2be3d59f10..f1244a1ad1b 100644 --- a/doc/topics/autodevops/stages.md +++ b/doc/topics/autodevops/stages.md @@ -378,6 +378,12 @@ deploys with Auto DevOps can undo your changes. Also, if you change something and want to undo it by deploying again, Helm may not detect that anything changed in the first place, and thus not realize that it needs to re-apply the old configuration. +WARNING: +GitLab 14.0 [renews the Auto Deploy template](https://gitlab.com/gitlab-org/gitlab/-/issues/232788). +This might cause an unexpected failure on your Auto DevOps project due to the breaking changes on +the v2 `auto-deploy-image`. Follow [the upgrade guide](upgrading_auto_deploy_dependencies.md#upgrade-guide) +to upgrade your environments before upgrading to GitLab 14.0. + ### GitLab deploy tokens > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/19507) in GitLab 11.0. diff --git a/doc/topics/autodevops/upgrading_auto_deploy_dependencies.md b/doc/topics/autodevops/upgrading_auto_deploy_dependencies.md index bf07743fb0e..5f8dfcdfc05 100644 --- a/doc/topics/autodevops/upgrading_auto_deploy_dependencies.md +++ b/doc/topics/autodevops/upgrading_auto_deploy_dependencies.md @@ -181,7 +181,7 @@ the latest Auto Deploy template into your `.gitlab-ci.yml`: ```yaml include: - template: Auto-DevOps.gitlab-ci.yml - - remote: https://gitlab.com/gitlab-org/gitlab/-/raw/master/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml + - template: Jobs/Deploy.latest.gitlab-ci.yml ``` WARNING: diff --git a/doc/user/application_security/coverage_fuzzing/index.md b/doc/user/application_security/coverage_fuzzing/index.md index 09f17f24470..af2d0a9464a 100644 --- a/doc/user/application_security/coverage_fuzzing/index.md +++ b/doc/user/application_security/coverage_fuzzing/index.md @@ -33,6 +33,7 @@ Docker image with the fuzz engine to run your app. | Java | [JQF](https://github.com/rohanpadhye/JQF) (not preferred) | [jqf-fuzzing-example](https://gitlab.com/gitlab-org/security-products/demos/coverage-fuzzing/java-fuzzing-example) | | JavaScript | [`jsfuzz`](https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/jsfuzz)| [jsfuzz-fuzzing-example](https://gitlab.com/gitlab-org/security-products/demos/coverage-fuzzing/jsfuzz-fuzzing-example) | | Python | [`pythonfuzz`](https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/pythonfuzz)| [pythonfuzz-fuzzing-example](https://gitlab.com/gitlab-org/security-products/demos/coverage-fuzzing/pythonfuzz-fuzzing-example) | +| AFL (any language that works on top of AFL) | [AFL](https://lcamtuf.coredump.cx/afl/)| [afl-fuzzing-example](https://gitlab.com/gitlab-org/security-products/demos/coverage-fuzzing/afl-fuzzing-example) | ## Configuration diff --git a/doc/user/project/merge_requests/getting_started.md b/doc/user/project/merge_requests/getting_started.md index 11d7dcad71b..60d041e74fd 100644 --- a/doc/user/project/merge_requests/getting_started.md +++ b/doc/user/project/merge_requests/getting_started.md @@ -132,7 +132,7 @@ To request it, open the **Reviewers** drop-down box to search for the user you w #### Approval Rule information for Reviewers **(PREMIUM)** -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/233736) in GitLab 13.8. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/233736) in GitLab 13.8. For this version only, GitLab administrators can opt to [enable it](#enable-or-disable-approval-rule-information). > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/293742) in GitLab 13.9. When editing the **Reviewers** field in a new or existing merge request, GitLab @@ -147,6 +147,21 @@ This example shows reviewers and approval rules in a merge request sidebar:  +#### Requesting a new review + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/293933) in GitLab 13.9. + +After a reviewer completes their [merge request reviews](../../discussions/index.md), +the author of the merge request can request a new review from the reviewer: + +1. If the right sidebar in the merge request is collapsed, click the + **{chevron-double-lg-left}** **Expand Sidebar** icon to expand it. +1. In the **Reviewers** section, click the **Re-request a review** icon (**{redo}**) + next to the reviewer's name. + +GitLab creates a new [to-do item](../../todos.md) for the reviewer, and sends +them a notification email. + ### Merge requests to close issues If the merge request is being created to resolve an issue, you can @@ -188,3 +203,30 @@ is set for deletion, the merge request widget displays the - Take one thing at a time and ship the smallest changes possible. By doing so, reviews are faster and your changes are less prone to errors. - Do not use capital letters nor special chars in branch names. + +## Enable or disable Approval Rule information **(PREMIUM SELF)** + +> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/293742) in GitLab 13.9. + +Merge Request Reviewers is under development and ready for production use. +It is deployed behind a feature flag that is **enabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md) +can opt to disable it. + +To enable it: + +```ruby +# For the instance +Feature.enable(:reviewer_approval_rules) +# For a single project +Feature.enable(:reviewer_approval_rules, Project.find(<project id>)) +``` + +To disable it: + +```ruby +# For the instance +Feature.disable(:reviewer_approval_rules) +# For a single project +Feature.disable(:reviewer_approval_rules, Project.find(<project id>)) +``` diff --git a/lib/gitlab/usage_data_counters/known_events/common.yml b/lib/gitlab/usage_data_counters/known_events/common.yml index 7c3d0ea6e42..c4ae67e7861 100644 --- a/lib/gitlab/usage_data_counters/known_events/common.yml +++ b/lib/gitlab/usage_data_counters/known_events/common.yml @@ -486,6 +486,16 @@ category: code_review aggregation: weekly feature_flag: usage_data_i_code_review_user_reopen_mr +- name: i_code_review_user_approve_mr + redis_slot: code_review + category: code_review + aggregation: weekly + feature_flag: usage_data_i_code_review_user_approve_mr +- name: i_code_review_user_unapprove_mr + redis_slot: code_review + category: code_review + aggregation: weekly + feature_flag: usage_data_i_code_review_user_unapprove_mr - name: i_code_review_user_resolve_thread redis_slot: code_review category: code_review diff --git a/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb index a8c0cbcc1cc..b5227f79261 100644 --- a/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb +++ b/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb @@ -10,6 +10,8 @@ module Gitlab MR_CLOSE_ACTION = 'i_code_review_user_close_mr' MR_REOPEN_ACTION = 'i_code_review_user_reopen_mr' MR_MERGE_ACTION = 'i_code_review_user_merge_mr' + MR_APPROVE_ACTION = 'i_code_review_user_approve_mr' + MR_UNAPPROVE_ACTION = 'i_code_review_user_unapprove_mr' MR_CREATE_COMMENT_ACTION = 'i_code_review_user_create_mr_comment' MR_EDIT_COMMENT_ACTION = 'i_code_review_user_edit_mr_comment' MR_REMOVE_COMMENT_ACTION = 'i_code_review_user_remove_mr_comment' @@ -53,6 +55,14 @@ module Gitlab track_unique_action_by_user(MR_REOPEN_ACTION, user) end + def track_approve_mr_action(user:) + track_unique_action_by_user(MR_APPROVE_ACTION, user) + end + + def track_unapprove_mr_action(user:) + track_unique_action_by_user(MR_UNAPPROVE_ACTION, user) + end + def track_resolve_thread_action(user:) track_unique_action_by_user(MR_RESOLVE_THREAD_ACTION, user) end diff --git a/scripts/lint-doc.sh b/scripts/lint-doc.sh index d6dde5744d7..061af911839 100755 --- a/scripts/lint-doc.sh +++ b/scripts/lint-doc.sh @@ -18,6 +18,18 @@ then ((ERRORCODE++)) fi +# Documentation pages need front matter for tracking purposes. +echo '=> Checking documentation for front matter...' +echo +no_frontmatter=$(find doc -name "*.md" -exec head -n1 {} \; | grep -v --count -- ---) +if [ $no_frontmatter -ne 0 ] +then + echo '✖ ERROR: These documentation pages need front matter. See https://docs.gitlab.com/ee/development/documentation/index.html#stage-and-group-metadata for how to add it.' >&2 + find doc -name "*.md" -exec sh -c 'if (head -n 1 "{}" | grep -v -- --- >/dev/null); then echo "{}"; fi' \; 2>&1 + echo + ((ERRORCODE++)) +fi + # Test for non-standard spaces (NBSP, NNBSP) in documentation. echo '=> Checking for non-standard spaces...' echo diff --git a/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb index f5fc2e71097..85ea23a8a48 100644 --- a/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb @@ -73,6 +73,22 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl end end + describe '.track_approve_mr_action' do + subject { described_class.track_approve_mr_action(user: user) } + + it_behaves_like 'a tracked merge request unique event' do + let(:action) { described_class::MR_APPROVE_ACTION } + end + end + + describe '.track_unapprove_mr_action' do + subject { described_class.track_unapprove_mr_action(user: user) } + + it_behaves_like 'a tracked merge request unique event' do + let(:action) { described_class::MR_UNAPPROVE_ACTION } + end + end + describe '.track_resolve_thread_action' do subject { described_class.track_resolve_thread_action(user: user) } diff --git a/spec/services/merge_requests/approval_service_spec.rb b/spec/services/merge_requests/approval_service_spec.rb index 124501f17d5..df9a98c5540 100644 --- a/spec/services/merge_requests/approval_service_spec.rb +++ b/spec/services/merge_requests/approval_service_spec.rb @@ -31,6 +31,13 @@ RSpec.describe MergeRequests::ApprovalService do expect(todo.reload).to be_pending end + + it 'does not track merge request approve action' do + expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) + .not_to receive(:track_approve_mr_action).with(user: user) + + service.execute(merge_request) + end end context 'with valid approval' do @@ -59,6 +66,13 @@ RSpec.describe MergeRequests::ApprovalService do service.execute(merge_request) end end + + it 'tracks merge request approve action' do + expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) + .to receive(:track_approve_mr_action).with(user: user) + + service.execute(merge_request) + end end context 'user cannot update the merge request' do diff --git a/spec/services/merge_requests/remove_approval_service_spec.rb b/spec/services/merge_requests/remove_approval_service_spec.rb index 40da928e832..4ef2da290e1 100644 --- a/spec/services/merge_requests/remove_approval_service_spec.rb +++ b/spec/services/merge_requests/remove_approval_service_spec.rb @@ -32,6 +32,13 @@ RSpec.describe MergeRequests::RemoveApprovalService do execute! end + + it 'tracks merge request unapprove action' do + expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) + .to receive(:track_unapprove_mr_action).with(user: user) + + execute! + end end context 'with a user who has not approved' do @@ -41,6 +48,13 @@ RSpec.describe MergeRequests::RemoveApprovalService do execute! end + + it 'does not track merge request unapprove action' do + expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) + .not_to receive(:track_unapprove_mr_action).with(user: user) + + execute! + end end end end diff --git a/spec/services/projects/branches_by_mode_service_spec.rb b/spec/services/projects/branches_by_mode_service_spec.rb new file mode 100644 index 00000000000..9199c3e0b3a --- /dev/null +++ b/spec/services/projects/branches_by_mode_service_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::BranchesByModeService do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + + let(:finder) { described_class.new(project, params) } + let(:params) { { mode: 'all' } } + + subject { finder.execute } + + describe '#execute' do + context 'page is passed' do + let(:params) { { page: 4, mode: 'all', offset: 3 } } + + it 'uses offset pagination' do + expect(finder).to receive(:fetch_branches_via_offset_pagination).and_call_original + + branches, prev_page, next_page = subject + + expect(branches.size).to eq(10) + expect(next_page).to be_nil + expect(prev_page).to eq("/#{project.full_path}/-/branches/all?offset=2&page=3") + end + + context 'but the page does not contain any branches' do + let(:params) { { page: 10, mode: 'all' } } + + it 'uses offset pagination' do + expect(finder).to receive(:fetch_branches_via_offset_pagination).and_call_original + + branches, prev_page, next_page = subject + + expect(branches).to eq([]) + expect(next_page).to be_nil + expect(prev_page).to be_nil + end + end + end + + context 'search is passed' do + let(:params) { { search: 'feature' } } + + it 'uses offset pagination' do + expect(finder).to receive(:fetch_branches_via_offset_pagination).and_call_original + + branches, prev_page, next_page = subject + + expect(branches.map(&:name)).to match_array(%w(feature feature_conflict)) + expect(next_page).to be_nil + expect(prev_page).to be_nil + end + end + + context 'branch_list_keyset_pagination is disabled' do + it 'uses offset pagination' do + stub_feature_flags(branch_list_keyset_pagination: false) + + expect(finder).to receive(:fetch_branches_via_offset_pagination).and_call_original + + branches, prev_page, next_page = subject + + expect(branches.size).to eq(20) + expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=1&page_token=conflict-resolvable") + expect(prev_page).to be_nil + end + end + + context 'uses gitaly pagination' do + before do + expect(finder).to receive(:fetch_branches_via_gitaly_pagination).and_call_original + end + + it 'returns branches for the first page' do + branches, prev_page, next_page = subject + + expect(branches.size).to eq(20) + expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=1&page_token=conflict-resolvable") + expect(prev_page).to be_nil + end + + context 'when second page is requested' do + let(:params) { { page_token: 'conflict-resolvable', mode: 'all', sort: 'name_asc', offset: 1 } } + + it 'returns branches for the first page' do + branches, prev_page, next_page = subject + + expect(branches.size).to eq(20) + expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=2&page_token=improve%2Fawesome&sort=name_asc") + expect(prev_page).to eq("/#{project.full_path}/-/branches/all?offset=0&page=1&sort=name_asc") + end + end + + context 'when last page is requested' do + let(:params) { { page_token: 'signed-commits', mode: 'all', sort: 'name_asc', offset: 4 } } + + it 'returns branches after the specified branch' do + branches, prev_page, next_page = subject + + expect(branches.size).to eq(14) + expect(next_page).to be_nil + expect(prev_page).to eq("/#{project.full_path}/-/branches/all?offset=3&page=4&sort=name_asc") + end + end + end + + context 'filter by mode' do + let(:stale) { double(state: 'stale') } + let(:active) { double(state: 'active') } + + before do + allow_next_instance_of(BranchesFinder) do |instance| + allow(instance).to receive(:execute).and_return([stale, active]) + end + end + + context 'stale' do + let(:params) { { mode: 'stale' } } + + it 'returns stale branches' do + is_expected.to eq([[stale], nil, nil]) + end + end + + context 'active' do + let(:params) { { mode: 'active' } } + + it 'returns active branches' do + is_expected.to eq([[active], nil, nil]) + end + end + end + end +end diff --git a/spec/support/shared_examples/features/navbar_shared_examples.rb b/spec/support/shared_examples/features/navbar_shared_examples.rb index c768e95c45a..9b89a3b5e54 100644 --- a/spec/support/shared_examples/features/navbar_shared_examples.rb +++ b/spec/support/shared_examples/features/navbar_shared_examples.rb @@ -8,12 +8,13 @@ RSpec.shared_examples 'verified navigation bar' do end it 'renders correctly' do - current_structure = page.all('.sidebar-top-level-items > li', class: ['!hidden']).map do |item| + # we are using * here in the selectors to prevent a regression where we added a non 'li' inside an 'ul' + current_structure = page.all('.sidebar-top-level-items > *', class: ['!hidden']).map do |item| next if item.find_all('a').empty? nav_item = item.find_all('a').first.text.gsub(/\s+\d+$/, '') # remove counts at the end - nav_sub_items = item.all('.sidebar-sub-level-items > li', class: ['!fly-out-top-item']).map do |list_item| + nav_sub_items = item.all('.sidebar-sub-level-items > *', class: ['!fly-out-top-item']).map do |list_item| list_item.all('a').first.text end |