Add latest changes from gitlab-org/gitlab@master

app/assets/stylesheets/framework/typography.scss

@@ -427,10 +427,10 @@
       padding-inline-start: 28px;
       margin-inline-start: 0 !important;
 
-      input.task-list-item-checkbox {
+      > input.task-list-item-checkbox {
         position: absolute;
-        inset-inline-start: $gl-padding-8;
+        inset-inline-start: 8px;
-        inset-block-start: 5px;
+        top: 5px;
       }
     }
   }

app/assets/stylesheets/page_bundles/issues_show.scss

@@ -1,24 +1,77 @@
 @import 'mixins_and_variables_and_functions';
 
 .description {
-  ul.task-list > li.task-list-item {
+  ul,
-    margin-inline-start: 0.5rem !important; /* Override typography.scss */
+  ol {
+    /* We're changing list-style-position to inside because the default of
+     * outside doesn't move negative margin to the left of the bullet. */
+    list-style-position: inside;
   }
 
   li {
     position: relative;
-    margin-inline-start: 2.25rem;
+    /* In the browser, the li element comes after (to the right of) the bullet point, so hovering
-
+     * over the left of the bullet point doesn't trigger a row hover. To trigger hovering on the
-    &.task-list-item > .drag-icon {
+     * left, we're applying negative margin here to shift the li element left. */
-      inset-inline-start: -0.6rem;
+    margin-inline-start: -1rem;
-    }
+    padding-inline-start: 2.5rem;
 
     .drag-icon {
       position: absolute;
       inset-block-start: 0.3rem;
-      inset-inline-start: -2.3rem;
+      inset-inline-start: 1rem;
-      padding-inline-end: 1rem;
+    }
-      width: 2rem;
+
+    /* The inside bullet aligns itself to the bottom, which we see when text to the right of
+     * a multi-line list item wraps. We fix this by aligning it to the top, and excluding
+     * other elements. Targeting ::marker doesn't seem to work, instead we exclude custom elements
+     * or anything with a class */
+    > *:not(gl-emoji, code, [class]) {
+      vertical-align: top;
+    }
+
+    /* The inside bullet is treated like an element inside the li element, so when we have a
+     * multi-paragraph list item, the text doesn't start on the right of the bullet because
+     * it is a block level p element. We make it inline to fix this. */
+    > p:first-of-type {
+      display: inline-block;
+      max-width: calc(100% - 1.5rem);
+    }
+
+    /* We fix the other paragraphs not indenting to the
+     * right of the bullet due to the inside bullet. */
+    p ~ a,
+    p ~ blockquote,
+    p ~ code,
+    p ~ details,
+    p ~ dl,
+    p ~ h1,
+    p ~ h2,
+    p ~ h3,
+    p ~ h4,
+    p ~ h5,
+    p ~ h6,
+    p ~ hr,
+    p ~ ol,
+    p ~ p,
+    p ~ table:not(.code), /* We need :not(.code) to override typography.scss */
+    p ~ ul,
+    p ~ .markdown-code-block {
+      margin-inline-start: 1rem;
+    }
+  }
+
+  ul.task-list {
+    > li.task-list-item {
+      /* We're using !important to override the same selector in typography.scss */
+      margin-inline-start: -1rem !important;
+      padding-inline-start: 2.5rem;
+
+      > input.task-list-item-checkbox {
+        position: static;
+        vertical-align: middle;
+        margin-block-start: -2px;
+      }
     }
   }
 }

app/graphql/mutations/merge_requests/remove_attention_request.rb

@@ -1,40 +0,0 @@
-# frozen_string_literal: true
-
-module Mutations
-  module MergeRequests
-    class RemoveAttentionRequest < Base
-      graphql_name 'MergeRequestRemoveAttentionRequest'
-
-      argument :user_id, ::Types::GlobalIDType[::User],
-               loads: Types::UserType,
-               required: true,
-               description: <<~DESC
-                            User ID of the user for attention request removal.
-               DESC
-
-      def resolve(project_path:, iid:, user:)
-        raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless feature_enabled?
-
-        merge_request = authorized_find!(project_path: project_path, iid: iid)
-
-        result = ::MergeRequests::RemoveAttentionRequestedService.new(
-          project: merge_request.project,
-          current_user: current_user,
-          merge_request: merge_request,
-          user: user
-        ).execute
-
-        {
-          merge_request: merge_request,
-          errors: Array(result[:message])
-        }
-      end
-
-      private
-
-      def feature_enabled?
-        current_user&.mr_attention_requests_enabled?
-      end
-    end
-  end
-end

app/graphql/mutations/merge_requests/request_attention.rb

@@ -1,40 +0,0 @@
-# frozen_string_literal: true
-
-module Mutations
-  module MergeRequests
-    class RequestAttention < Base
-      graphql_name 'MergeRequestRequestAttention'
-
-      argument :user_id, ::Types::GlobalIDType[::User],
-               loads: Types::UserType,
-               required: true,
-               description: <<~DESC
-                            User ID of the user to request attention.
-               DESC
-
-      def resolve(project_path:, iid:, user:)
-        raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless feature_enabled?
-
-        merge_request = authorized_find!(project_path: project_path, iid: iid)
-
-        result = ::MergeRequests::RequestAttentionService.new(
-          project: merge_request.project,
-          current_user: current_user,
-          merge_request: merge_request,
-          user: user
-        ).execute
-
-        {
-          merge_request: merge_request,
-          errors: Array(result[:message])
-        }
-      end
-
-      private
-
-      def feature_enabled?
-        current_user&.mr_attention_requests_enabled?
-      end
-    end
-  end
-end

app/graphql/mutations/merge_requests/toggle_attention_requested.rb

@@ -1,29 +0,0 @@
-# frozen_string_literal: true
-
-module Mutations
-  module MergeRequests
-    class ToggleAttentionRequested < Base
-      graphql_name 'MergeRequestToggleAttentionRequested'
-
-      argument :user_id, ::Types::GlobalIDType[::User],
-               loads: Types::UserType,
-               required: true,
-               description: <<~DESC
-                            User ID for the user to toggle attention requested.
-               DESC
-
-      def resolve(project_path:, iid:, user:)
-        raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless current_user&.mr_attention_requests_enabled?
-
-        merge_request = authorized_find!(project_path: project_path, iid: iid)
-
-        result = ::MergeRequests::ToggleAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: user).execute
-
-        {
-          merge_request: merge_request,
-          errors: Array(result[:message])
-        }
-      end
-    end
-  end
-end

app/graphql/types/mutation_type.rb

@@ -73,9 +73,6 @@ module Types
     mount_mutation Mutations::MergeRequests::SetDraft, calls_gitaly: true
     mount_mutation Mutations::MergeRequests::SetAssignees
     mount_mutation Mutations::MergeRequests::ReviewerRereview
-    mount_mutation Mutations::MergeRequests::RequestAttention
-    mount_mutation Mutations::MergeRequests::RemoveAttentionRequest
-    mount_mutation Mutations::MergeRequests::ToggleAttentionRequested
     mount_mutation Mutations::Metrics::Dashboard::Annotations::Create
     mount_mutation Mutations::Metrics::Dashboard::Annotations::Delete
     mount_mutation Mutations::Notes::Create::Note, calls_gitaly: true

app/helpers/system_note_helper.rb

@@ -30,6 +30,7 @@ module SystemNoteHelper
     'locked' => 'lock',
     'unlocked' => 'lock-open',
     'due_date' => 'calendar',
+    'start_date_or_due_date' => 'calendar',
     'health_status' => 'status-health',
     'designs_added' => 'doc-image',
     'designs_modified' => 'doc-image',

app/models/concerns/issuable.rb

@@ -515,11 +515,23 @@ module Issuable
     changes
   end
 
+  def hook_reviewer_changes(old_associations)
+    changes = {}
+    old_reviewers = old_associations.fetch(:reviewers, reviewers)
+
+    if old_reviewers != reviewers
+      changes[:reviewers] = [old_reviewers.map(&:hook_attrs), reviewers.map(&:hook_attrs)]
+    end
+
+    changes
+  end
+
   def to_hook_data(user, old_associations: {})
     changes = previous_changes
 
     if old_associations.present?
       changes.merge!(hook_association_changes(old_associations))
+      changes.merge!(hook_reviewer_changes(old_associations)) if allows_reviewers?
     end
 
     Gitlab::DataBuilder::Issuable.new(self).build(user: user, changes: changes)

app/models/members/last_group_owner_assigner.rb

@@ -8,7 +8,7 @@ class LastGroupOwnerAssigner
   end
 
   def execute
-    @last_blocked_owner = no_owners_in_heirarchy? && group.single_blocked_owner?
+    @last_blocked_owner = no_owners_in_hierarchy? && group.single_blocked_owner?
     @group_single_owner = owners.size == 1
 
     members.each { |member| set_last_owner(member) }
@@ -18,7 +18,7 @@ class LastGroupOwnerAssigner
 
   attr_reader :group, :members, :last_blocked_owner, :group_single_owner
 
-  def no_owners_in_heirarchy?
+  def no_owners_in_hierarchy?
     owners.empty?
   end
 

app/models/protected_branch.rb

@@ -4,8 +4,6 @@ class ProtectedBranch < ApplicationRecord
   include ProtectedRef
   include Gitlab::SQL::Pattern
 
-  CACHE_EXPIRE_IN = 1.hour
-
   scope :requiring_code_owner_approval,
         -> { where(code_owner_approval_required: true) }
 
@@ -27,10 +25,30 @@ class ProtectedBranch < ApplicationRecord
   end
 
   # Check if branch name is marked as protected in the system
-  def self.protected?(project, ref_name)
+  def self.protected?(project, ref_name, dry_run: true)
     return true if project.empty_repo? && project.default_branch_protected?
     return false if ref_name.blank?
 
+    new_cache_result = new_cache(project, ref_name, dry_run: dry_run)
+
+    return new_cache_result unless new_cache_result.nil?
+
+    deprecated_cache(project, ref_name)
+  end
+
+  def self.new_cache(project, ref_name, dry_run: true)
+    if Feature.enabled?(:hash_based_cache_for_protected_branches, project)
+      ProtectedBranches::CacheService.new(project).fetch(ref_name, dry_run: dry_run) do # rubocop: disable CodeReuse/ServiceClass
+        self.matching(ref_name, protected_refs: protected_refs(project)).present?
+      end
+    end
+  end
+
+  # Deprecated: https://gitlab.com/gitlab-org/gitlab/-/issues/368279
+  # ----------------------------------------------------------------
+  CACHE_EXPIRE_IN = 1.hour
+
+  def self.deprecated_cache(project, ref_name)
     Rails.cache.fetch(protected_ref_cache_key(project, ref_name), expires_in: CACHE_EXPIRE_IN) do
       self.matching(ref_name, protected_refs: protected_refs(project)).present?
     end
@@ -39,6 +57,7 @@ class ProtectedBranch < ApplicationRecord
   def self.protected_ref_cache_key(project, ref_name)
     "protected_ref-#{project.cache_key}-#{Digest::SHA1.hexdigest(ref_name)}"
   end
+  # End of deprecation --------------------------------------------
 
   def self.allow_force_push?(project, ref_name)
     project.protected_branches.allowing_force_push.matching(ref_name).any?

app/models/system_note_metadata.rb

@@ -22,7 +22,7 @@ class SystemNoteMetadata < ApplicationRecord
     designs_added designs_modified designs_removed designs_discussion_added
     title time_tracking branch milestone discussion task moved cloned
     opened closed merged duplicate locked unlocked outdated reviewer
-    tag due_date pinned_embed cherry_pick health_status approved unapproved
+    tag due_date start_date_or_due_date pinned_embed cherry_pick health_status approved unapproved
     status alert_issue_added relate unrelate new_alert_added severity
     attention_requested attention_request_removed contact timeline_event
   ].freeze

app/serializers/merge_request_user_entity.rb

@@ -20,10 +20,6 @@ class MergeRequestUserEntity < ::API::Entities::UserBasic
     find_reviewer_or_assignee(user, options)&.reviewed?
   end
 
-  expose :attention_requested, if: ->(_, options) { options[:merge_request].present? && options[:merge_request].allows_reviewers? && request.current_user&.mr_attention_requests_enabled? } do |user, options|
-    find_reviewer_or_assignee(user, options)&.attention_requested?
-  end
-
   expose :approved, if: satisfies(:present?) do |user, options|
     # This approach is preferred over MergeRequest#approved_by? since this
     # makes one query per merge request, whereas #approved_by? makes one per user

app/services/issuable/common_system_notes_service.rb

@@ -21,7 +21,7 @@ module Issuable
           create_discussion_lock_note if issuable.previous_changes.include?('discussion_locked')
         end
 
-        create_due_date_note if issuable.previous_changes.include?('due_date')
+        handle_start_date_or_due_date_change_note
         create_milestone_change_event(old_milestone) if issuable.previous_changes.include?('milestone_id')
         create_labels_note(old_labels) if old_labels && issuable.labels != old_labels
       end
@@ -29,6 +29,13 @@ module Issuable
 
     private
 
+    def handle_start_date_or_due_date_change_note
+      # Type check needed as some issuables do their own date change handling for date fields other than due_date
+      change_date_fields = issuable.is_a?(Issue) ? %w[due_date start_date] : %w[due_date]
+      changed_dates = issuable.previous_changes.slice(*change_date_fields)
+      create_start_date_or_due_date_note(changed_dates)
+    end
+
     def handle_time_tracking_note
       if issuable.previous_changes.include?('time_estimate')
         create_time_estimate_note
@@ -99,8 +106,10 @@ module Issuable
         .execute
     end
 
-    def create_due_date_note
+    def create_start_date_or_due_date_note(changed_dates)
-      SystemNoteService.change_due_date(issuable, issuable.project, current_user, issuable.due_date)
+      return if changed_dates.blank?
+
+      SystemNoteService.change_start_date_or_due_date(issuable, issuable.project, current_user, changed_dates)
     end
 
     def create_discussion_lock_note

app/services/protected_branches/cache_service.rb

@@ -7,17 +7,24 @@ module ProtectedBranches
     CACHE_EXPIRE_IN = 1.day
     CACHE_LIMIT = 1000
 
-    def fetch(ref_name)
+    def fetch(ref_name, dry_run: false)
       record = OpenSSL::Digest::SHA256.hexdigest(ref_name)
 
       Gitlab::Redis::Cache.with do |redis|
         cached_result = redis.hget(redis_key, record)
 
-        break Gitlab::Redis::Boolean.decode(cached_result) unless cached_result.nil?
+        decoded_result = Gitlab::Redis::Boolean.decode(cached_result) unless cached_result.nil?
 
-        value = yield
+        # If we're dry-running, don't break because we need to check against
+        # the real value to ensure the cache is working properly.
+        # If the result is nil we'll need to run the block, so don't break yet.
+        break decoded_result unless dry_run || decoded_result.nil?
 
-        redis.hset(redis_key, record, Gitlab::Redis::Boolean.encode(value))
+        calculated_value = yield
+
+        check_and_log_discrepancy(decoded_result, calculated_value, ref_name) if dry_run
+
+        redis.hset(redis_key, record, Gitlab::Redis::Boolean.encode(calculated_value))
 
         # We don't want to extend cache expiration time
         if redis.ttl(redis_key) == TTL_UNSET
@@ -30,7 +37,7 @@ module ProtectedBranches
           redis.unlink(redis_key)
         end
 
-        value
+        calculated_value
       end
     end
 
@@ -40,6 +47,20 @@ module ProtectedBranches
 
     private
 
+    def check_and_log_discrepancy(cached_value, real_value, ref_name)
+      return if cached_value.nil?
+      return if cached_value == real_value
+
+      encoded_ref_name = Gitlab::EncodingHelper.encode_utf8_with_replacement_character(ref_name)
+
+      log_error(
+        'class' => self.class.name,
+        'message' => "Cache mismatch '#{encoded_ref_name}': cached value: #{cached_value}, real value: #{real_value}",
+        'project_id' => @project.id,
+        'project_path' => @project.full_path
+      )
+    end
+
     def redis_key
       @redis_key ||= [CACHE_ROOT_KEY, @project.id].join(':')
     end

app/services/system_note_service.rb

@@ -57,7 +57,7 @@ module SystemNoteService
     ::SystemNotes::IssuablesService.new(noteable: noteable, project: noteable.project, author: user).unrelate_issuable(noteable_ref)
   end
 
-  # Called when the due_date of a Noteable is changed
+  # Called when the due_date or start_date of a Noteable is changed
   #
   # noteable  - Noteable object
   # project   - Project owning noteable
@@ -68,11 +68,15 @@ module SystemNoteService
   #
   #   "removed due date"
   #
-  #   "changed due date to September 20, 2018"
+  #   "changed due date to September 20, 2018 and changed start date to September 25, 2018"
   #
   # Returns the created Note object
-  def change_due_date(noteable, project, author, due_date)
+  def change_start_date_or_due_date(noteable, project, author, changed_dates)
-    ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_due_date(due_date)
+    ::SystemNotes::TimeTrackingService.new(
+      noteable: noteable,
+      project: project,
+      author: author
+    ).change_start_date_or_due_date(changed_dates)
   end
 
   # Called when the estimated time of a Noteable is changed

app/services/system_notes/time_tracking_service.rb

@@ -2,8 +2,9 @@
 
 module SystemNotes
   class TimeTrackingService < ::SystemNotes::BaseService
-    # Called when the due_date of a Noteable is changed
+    # Called when the start_date or due_date of an Issue/WorkItem is changed
     #
+    # start_date  - Start date being assigned, or nil
     # due_date  - Due date being assigned, or nil
     #
     # Example Note text:
@@ -11,14 +12,20 @@ module SystemNotes
     #   "removed due date"
     #
     #   "changed due date to September 20, 2018"
+
+    #   "changed start date to September 20, 2018 and changed due date to September 25, 2018"
     #
     # Returns the created Note object
-    def change_due_date(due_date)
+    def change_start_date_or_due_date(changed_dates = {})
-      body = due_date ? "changed due date to #{due_date.to_s(:long)}" : 'removed due date'
+      return if changed_dates.empty?
 
-      issue_activity_counter.track_issue_due_date_changed_action(author: author) if noteable.is_a?(Issue)
+      if noteable.is_a?(Issue) && changed_dates.key?('due_date')
+        issue_activity_counter.track_issue_due_date_changed_action(author: author)
+      end
 
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'due_date'))
+      create_note(
+        NoteSummary.new(noteable, project, author, changed_date_body(changed_dates), action: 'start_date_or_due_date')
+      )
     end
 
     # Called when the estimated time of a Noteable is changed
@@ -116,6 +123,27 @@ module SystemNotes
 
     private
 
+    def changed_date_body(changed_dates)
+      %w[start_date due_date].each_with_object([]) do |date_field, word_array|
+        next unless changed_dates.key?(date_field)
+
+        word_array << 'and' if word_array.any?
+
+        word_array << message_for_changed_date(changed_dates, date_field)
+      end.join(' ')
+    end
+
+    def message_for_changed_date(changed_dates, date_key)
+      changed_date = changed_dates[date_key].last
+      readable_date = date_key.humanize.downcase
+
+      if changed_date.nil?
+        "removed #{readable_date}"
+      else
+        "changed #{readable_date} to #{changed_date.to_s(:long)}"
+      end
+    end
+
     def issue_activity_counter
       Gitlab::UsageDataCounters::IssueActivityUniqueCounter
     end

app/views/projects/_home_panel.html.haml

@@ -29,7 +29,7 @@
         - 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: _('View project in admin area'),
-              data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
+              data: {toggle: 'tooltip', placement: 'top', container: 'body'} do
               = sprite_icon('admin')
           .gl-display-flex.gl-align-items-start.gl-mr-3
             - if @notification_setting

config/feature_flags/development/hash_based_cache_for_protected_branches.yml

@@ -0,0 +1,8 @@
+---
+name: hash_based_cache_for_protected_branches
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92934
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/368279
+milestone: '15.3'
+type: development
+group: group::source code
+default_enabled: false

doc/administration/gitaly/troubleshooting.md

@@ -360,6 +360,17 @@ push, which causes a significant delay.
 
 If Git pushes are too slow when Dynatrace is enabled, disable Dynatrace.
 
+### Find storage resource details
+
+You can run the following commands in a [Rails conosole](../operations/rails_console.md#starting-a-rails-console-session) to determine the available and used space on a
+Gitaly storage:
+
+```ruby
+Gitlab::GitalyClient::ServerService.new("default").storage_disk_statistics
+# For Gitaly Cluster
+Gitlab::GitalyClient::ServerService.new("<storage name>").disk_statistics
+```
+
 ## Troubleshoot Praefect (Gitaly Cluster)
 
 The following sections provide possible solutions to Gitaly Cluster errors.

doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md

@@ -1392,16 +1392,6 @@ registry = Geo::SnippetRepositoryRegistry.find(registry_id)
 registry.replicator.send(:sync_repository)
 ```
 
-## Gitaly
-
-### Find available and used space
-
-A Gitaly storage resource can be polled through Rails to determine the available and used space.
-
-```ruby
-Gitlab::GitalyClient::ServerService.new("default").storage_disk_statistics
-```
-
 ## Generate Service Ping
 
 The [Service Ping Guide](../../development/service_ping/index.md) in our developer documentation

doc/api/graphql/reference/index.md

@@ -3634,48 +3634,6 @@ Input type: `MergeRequestCreateInput`
 | <a id="mutationmergerequestcreateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
 | <a id="mutationmergerequestcreatemergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
 
-### `Mutation.mergeRequestRemoveAttentionRequest`
-
-Input type: `MergeRequestRemoveAttentionRequestInput`
-
-#### Arguments
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mutationmergerequestremoveattentionrequestclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationmergerequestremoveattentionrequestiid"></a>`iid` | [`String!`](#string) | IID of the merge request to mutate. |
-| <a id="mutationmergerequestremoveattentionrequestprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the merge request to mutate is in. |
-| <a id="mutationmergerequestremoveattentionrequestuserid"></a>`userId` | [`UserID!`](#userid) | User ID of the user for attention request removal. |
-
-#### Fields
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mutationmergerequestremoveattentionrequestclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationmergerequestremoveattentionrequesterrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
-| <a id="mutationmergerequestremoveattentionrequestmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
-
-### `Mutation.mergeRequestRequestAttention`
-
-Input type: `MergeRequestRequestAttentionInput`
-
-#### Arguments
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mutationmergerequestrequestattentionclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationmergerequestrequestattentioniid"></a>`iid` | [`String!`](#string) | IID of the merge request to mutate. |
-| <a id="mutationmergerequestrequestattentionprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the merge request to mutate is in. |
-| <a id="mutationmergerequestrequestattentionuserid"></a>`userId` | [`UserID!`](#userid) | User ID of the user to request attention. |
-
-#### Fields
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mutationmergerequestrequestattentionclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationmergerequestrequestattentionerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
-| <a id="mutationmergerequestrequestattentionmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
-
 ### `Mutation.mergeRequestReviewerRereview`
 
 Input type: `MergeRequestReviewerRereviewInput`
@@ -3825,27 +3783,6 @@ Input type: `MergeRequestSetSubscriptionInput`
 | <a id="mutationmergerequestsetsubscriptionerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
 | <a id="mutationmergerequestsetsubscriptionmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
 
-### `Mutation.mergeRequestToggleAttentionRequested`
-
-Input type: `MergeRequestToggleAttentionRequestedInput`
-
-#### Arguments
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mutationmergerequesttoggleattentionrequestedclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationmergerequesttoggleattentionrequestediid"></a>`iid` | [`String!`](#string) | IID of the merge request to mutate. |
-| <a id="mutationmergerequesttoggleattentionrequestedprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the merge request to mutate is in. |
-| <a id="mutationmergerequesttoggleattentionrequesteduserid"></a>`userId` | [`UserID!`](#userid) | User ID for the user to toggle attention requested. |
-
-#### Fields
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mutationmergerequesttoggleattentionrequestedclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationmergerequesttoggleattentionrequestederrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
-| <a id="mutationmergerequesttoggleattentionrequestedmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
-
 ### `Mutation.mergeRequestUpdate`
 
 Update attributes of a merge request.

doc/security/webhooks.md

@@ -74,7 +74,7 @@ allowlist:
 The allowed entries can be separated by semicolons, commas or whitespaces
 (including newlines) and be in different formats like hostnames, IP addresses and/or
 IP ranges. IPv6 is supported. Hostnames that contain Unicode characters should
-use [Internationalized Domain Names in Applications](https://www.icann.org/resources/pages/glossary-2014-02-04-en#i)
+use [Internationalized Domain Names in Applications](https://www.icann.org/en/icann-acronyms-and-terms/internationalized-domain-names-in-applications-en)
 (IDNA) encoding.
 
 The allowlist can hold a maximum of 1000 entries. Each entry can be a maximum of

doc/subscriptions/gitlab_com/index.md

@@ -297,7 +297,7 @@ for your personal or group namespace. CI/CD minutes are a **one-time purchase**,
 ## Add-on subscription for additional Storage and Transfer
 
 NOTE:
-Free namespaces are subject to a 5GB storage and 10GB transfer [soft limit](https://about.gitlab.com/pricing). Once all storage is available to view in the usage quota workflow, GitLab will automatically enforce the namespace storage limit and the project limit will be removed. This change will be announced separately. The storage and transfer add-on can be purchased to increase the limits.
+Free namespaces are subject to a 5GB storage and 10GB transfer [soft limit](https://about.gitlab.com/pricing/). Once all storage is available to view in the usage quota workflow, GitLab will automatically enforce the namespace storage limit and the project limit will be removed. This change will be announced separately. The storage and transfer add-on can be purchased to increase the limits.
 
 Projects have a free storage quota of 10 GB. To exceed this quota you must first 
 [purchase one or more storage subscription units](#purchase-more-storage-and-transfer). Each unit provides 10 GB of additional

doc/topics/autodevops/cloud_deployments/auto_devops_with_gke.md

@@ -35,7 +35,7 @@ you need a [Google Cloud Platform account](https://console.cloud.google.com).
 Sign in with an existing Google account, such as the one you use to access Gmail
 or Google Drive, or create a new one.
 
-1. Follow the steps described in the ["Before you begin" section](https://cloud.google.com/kubernetes-engine/docs/quickstart#before-you-begin)
+1. Follow the steps described in the ["Before you begin" section](https://cloud.google.com/kubernetes-engine/docs/deploy-app-cluster#before-you-begin)
    of the Kubernetes Engine documentation to enable the required APIs and related services.
 1. Ensure you've created a [billing account](https://cloud.google.com/billing/docs/how-to/manage-billing-account)
    with Google Cloud Platform.

doc/update/plan_your_upgrade.md

@@ -14,7 +14,7 @@ General notes:
 - If possible, we recommend you test out the upgrade in a test environment before
   updating your production instance. Ideally, your test environment should mimic
   your production environment as closely as possible.
-- If [working with Support](https://about.gitlab.com/support/scheduling-upgrade-assistance.html)
+- If [working with Support](https://about.gitlab.com/support/scheduling-upgrade-assistance/)
   to create your plan, share details of your architecture, including:
   - How is GitLab installed?
   - What is the operating system of the node?

doc/update/zero_downtime.md

@@ -395,7 +395,7 @@ HA.
 
 #### In the application node
 
-According to [official Redis documentation](https://redis.io/topics/admin#upgrading-or-restarting-a-redis-instance-without-downtime),
+According to [official Redis documentation](https://redis.io/docs/manual/admin/#upgrading-or-restarting-a-redis-instance-without-downtime),
 the easiest way to update an HA instance using Sentinel is to upgrade the
 secondaries one after the other, perform a manual failover from current
 primary (running old version) to a recently upgraded secondary (running a new

doc/user/application_security/api_fuzzing/index.md

@@ -1802,7 +1802,7 @@ To detect and correct elements that don't comply with the OpenAPI specifications
 | Editor | OpenAPI 2.0 | OpenAPI 3.0.x | OpenAPI 3.1.x |
 | -- | -- | -- | -- |
 | [Swagger Editor](https://editor.swagger.io/) | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON | **{dotted-circle}** YAML, JSON |
-| [Stoplight Studio](https://stoplight.io/studio/) | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON |
+| [Stoplight Studio](https://stoplight.io/studio) | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON |
 
 If your OpenAPI document is generated manually, load your document in the editor and fix anything that is non-compliant. If your document is generated automatically, load it in your editor to identify the issues in the schema, then go to the application and perform the corrections based on the framework you are using.
 

doc/user/application_security/container_scanning/index.md

@@ -680,7 +680,7 @@ It's possible to run the [GitLab container scanning tool](https://gitlab.com/git
 against a Docker container without needing to run it within the context of a CI job. To scan an
 image directly, follow these steps:
 
-1. Run [Docker Desktop](https://www.docker.com/products/docker-desktop)
+1. Run [Docker Desktop](https://www.docker.com/products/docker-desktop/)
    or [Docker Machine](https://github.com/docker/machine).
 
 1. Run the analyzer's Docker image, passing the image and tag you want to analyze in the

doc/user/application_security/dast_api/index.md

@@ -1612,7 +1612,7 @@ To detect and correct elements that don't comply with the OpenAPI specifications
 | Editor | OpenAPI 2.0 | OpenAPI 3.0.x | OpenAPI 3.1.x |
 | -- | -- | -- | -- |
 | [Swagger Editor](https://editor.swagger.io/) | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON | **{dotted-circle}** YAML, JSON |
-| [Stoplight Studio](https://stoplight.io/studio/) | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON |
+| [Stoplight Studio](https://stoplight.io/studio) | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON | **{check-circle}** YAML, JSON |
 
 If your OpenAPI document is generated manually, load your document in the editor and fix anything that is non-compliant. If your document is generated automatically, load it in your editor to identify the issues in the schema, then go to the application and perform the corrections based on the framework you are using.
 

doc/user/clusters/agent/install/index.md

@@ -17,7 +17,7 @@ To connect a Kubernetes cluster to GitLab, you must install an agent in your clu
 Before you can install the agent in your cluster, you need:
 
 - An existing Kubernetes cluster. If you don't have a cluster, you can create one on a cloud provider, like:
-  - [Google Kubernetes Engine (GKE)](https://cloud.google.com/kubernetes-engine/docs/quickstart)
+  - [Google Kubernetes Engine (GKE)](https://cloud.google.com/kubernetes-engine/docs/deploy-app-cluster)
   - [Amazon Elastic Kubernetes Service (EKS)](https://docs.aws.amazon.com/eks/latest/userguide/getting-started.html)
   - [Digital Ocean](https://docs.digitalocean.com/products/kubernetes/quickstart/)
 - On self-managed GitLab instances, a GitLab administrator must set up the

doc/user/group/saml_sso/scim_setup.md

@@ -106,7 +106,7 @@ Before you start this section:
 
 - Check that you are using Okta [Lifecycle Management](https://www.okta.com/products/lifecycle-management/) product. This product tier is required to use SCIM on Okta. To check which Okta product you are using, check your signed Okta contract, contact your Okta AE, CSM, or Okta support.
 - Complete the [GitLab configuration](#gitlab-configuration) process.
-- Complete the setup for SAML application for [Okta](https://developer.okta.com/docs/guides/build-sso-integration/saml2/overview/), as described in the [Okta setup notes](index.md#okta-setup-notes).
+- Complete the setup for SAML application for [Okta](https://developer.okta.com/docs/guides/build-sso-integration/saml2/main/), as described in the [Okta setup notes](index.md#okta-setup-notes).
 - Check that your Okta SAML setup matches our documentation exactly, especially the NameID configuration. Otherwise, the Okta SCIM app may not work properly.
 
 After the above steps are complete:

doc/user/project/clusters/add_gke_clusters.md

@@ -37,7 +37,7 @@ Prerequisites:
   set up with access.
 - Kubernetes Engine API and related services enabled. It should work immediately but may
   take up to 10 minutes after you create a project. For more information see the
-  ["Before you begin" section of the Kubernetes Engine docs](https://cloud.google.com/kubernetes-engine/docs/quickstart#before-you-begin).
+  ["Before you begin" section of the Kubernetes Engine docs](https://cloud.google.com/kubernetes-engine/docs/deploy-app-cluster#before-you-begin).
 
 Note the following:
 

doc/user/project/import/github.md

@@ -241,3 +241,35 @@ Feature.disable(:github_importer_lower_per_page_limit, group)
 
 For information on automating user, group, and project import API calls, see
 [Automate group and project import](index.md#automate-group-and-project-import).
+
+## Troubleshooting
+
+### Manually continue a previously failed import process
+
+In some cases, the GitHub import process can fail to import the repository. This causes GitLab to abort the project import process and requires the
+repository to be imported manually. Administrators can manually import the repository for a failed import process:
+
+1. Open a Rails console.
+1. Run the following series of commands in the console:
+
+   ```ruby
+   project_id = <PROJECT_ID> 
+   github_access_token =  <GITHUB_ACCESS_TOKEN> 
+   github_repository_path = '<GROUP>/<REPOSITORY>'
+
+   github_repository_url = "https://#{github_access_token}@github.com/#{github_repository_path}.git"
+
+   # Find project by ID
+   project = Project.find(project_id)
+   # Set import URL and credentials
+   project.import_url = github_repository_url
+   project.import_type = 'github'
+   project.import_source = github_repository_path
+   project.save!
+   # Create an import state if the project was created manually and not from a failed import
+   project.create_import_state if project.import_state.blank?
+   # Set state to start
+   project.import_state.force_start
+   # Trigger import from second step
+   Gitlab::GithubImport::Stage::ImportRepositoryWorker.perform_async(project.id)
+   ```

doc/user/project/integrations/webhook_events.md

@@ -878,7 +878,9 @@ Payload example:
     "source_branch": "ms-viewport",
     "source_project_id": 14,
     "author_id": 51,
+    "assignee_ids": [6],
     "assignee_id": 6,
+    "reviewer_ids": [6],
     "title": "MS-Viewport",
     "created_at": "2013-12-03T17:23:34Z",
     "updated_at": "2013-12-03T17:23:34Z",
@@ -945,12 +947,7 @@ Payload example:
       "type": "ProjectLabel",
       "group_id": 41
     }],
-    "action": "open",
+    "action": "open"
-    "assignee": {
-      "name": "User1",
-      "username": "user1",
-      "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
-    }
   },
   "labels": [{
     "id": 206,
@@ -999,7 +996,23 @@ Payload example:
         "group_id": 41
       }]
     }
-  }
+  },
+  "assignees": [
+    {
+      "id": 6,
+      "name": "User1",
+      "username": "user1",
+      "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
+    }
+  ],
+  "reviewers": [
+    {
+      "id": 6,
+      "name": "User1",
+      "username": "user1",
+      "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
+    }
+  ]
 }
 ```
 

lib/api/user_counts.rb

@@ -12,19 +12,13 @@ module API
       get do
         unauthorized! unless current_user
 
-        counts = {
+        {
           merge_requests: current_user.assigned_open_merge_requests_count, # @deprecated
           assigned_issues: current_user.assigned_open_issues_count,
           assigned_merge_requests: current_user.assigned_open_merge_requests_count,
           review_requested_merge_requests: current_user.review_requested_open_merge_requests_count,
           todos: current_user.todos_pending_count
         }
-
-        if current_user&.mr_attention_requests_enabled?
-          counts[:attention_requests] = current_user.attention_requested_open_merge_requests_count
-        end
-
-        counts
       end
     end
   end

lib/gitlab/data_builder/issuable.rb

@@ -26,6 +26,10 @@ module Gitlab
 
         hook_data[:assignees] = issuable.assignees.map(&:hook_attrs) if issuable.assignees.any?
 
+        if issuable.allows_reviewers? && issuable.reviewers.any?
+          hook_data[:reviewers] = issuable.reviewers.map(&:hook_attrs)
+        end
+
         hook_data
       end
 

lib/gitlab/hook_data/merge_request_builder.rb

@@ -22,6 +22,7 @@ module Gitlab
           merge_user_id
           merge_when_pipeline_succeeds
           milestone_id
+          reviewer_ids
           source_branch
           source_project_id
           state_id
@@ -38,6 +39,7 @@ module Gitlab
         %i[
           assignees
           labels
+          reviewers
           total_time_spent
           time_change
         ].freeze
@@ -60,6 +62,7 @@ module Gitlab
           human_time_estimate: merge_request.human_time_estimate,
           assignee_ids: merge_request.assignee_ids,
           assignee_id: merge_request.assignee_ids.first, # This key is deprecated
+          reviewer_ids: merge_request.reviewer_ids,
           labels: merge_request.labels_hook_attrs,
           state: merge_request.state, # This key is deprecated
           blocking_discussions_resolved: merge_request.mergeable_discussions_state?,

lib/gitlab/quick_actions/merge_request_actions.rb

@@ -292,76 +292,6 @@ module Gitlab
             @updates[:reviewer_ids] = []
           end
         end
-
-        desc do
-          if quick_action_target.allows_multiple_reviewers?
-            _('Request attention from assignee(s) or reviewer(s)')
-          else
-            _('Request attention from assignee or reviewer')
-          end
-        end
-        explanation do |users|
-          _('Request attention from %{users_sentence}.') % { users_sentence: reviewer_users_sentence(users) }
-        end
-        execution_message do |users = nil|
-          if users.blank?
-            _("Failed to request attention because no user was found.")
-          else
-            _('Requested attention from %{users_sentence}.') % { users_sentence: reviewer_users_sentence(users) }
-          end
-        end
-        params do
-          quick_action_target.allows_multiple_reviewers? ? '@user1 @user2' : '@user'
-        end
-        types MergeRequest
-        condition do
-          current_user.mr_attention_requests_enabled? &&
-            current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
-        end
-        parse_params do |attention_param|
-          extract_users(attention_param)
-        end
-        command :attention, :attn do |users|
-          next if users.empty?
-
-          users.each do |user|
-            ::MergeRequests::ToggleAttentionRequestedService.new(project: quick_action_target.project, merge_request: quick_action_target, current_user: current_user, user: user).execute
-          end
-        end
-
-        desc do
-          if quick_action_target.allows_multiple_reviewers?
-            _('Remove attention request(s)')
-          else
-            _('Remove attention request')
-          end
-        end
-        explanation do |users|
-          _('Removes attention from %{users_sentence}.') % { users_sentence: reviewer_users_sentence(users) }
-        end
-        execution_message do |users = nil|
-          if users.blank?
-            _("Failed to remove attention because no user was found.")
-          else
-            _('Removed attention from %{users_sentence}.') % { users_sentence: reviewer_users_sentence(users) }
-          end
-        end
-        params do
-          quick_action_target.allows_multiple_reviewers? ? '@user1 @user2' : '@user'
-        end
-        types MergeRequest
-        condition do
-          current_user.mr_attention_requests_enabled? &&
-            current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
-        end
-        parse_params do |attention_param|
-          extract_users(attention_param)
-        end
-        command :remove_attention do |users|
-          next if users.empty?
-
-          ::MergeRequests::BulkRemoveAttentionRequestedService.new(project: quick_action_target.project, merge_request: quick_action_target, current_user: current_user, users: users).execute
-        end
       end
 
       def reviewer_users_sentence(users)

locale/gitlab.pot

@@ -16030,9 +16030,6 @@ msgstr ""
 msgid "Failed to remove a to-do item for the design."
 msgstr ""
 
-msgid "Failed to remove attention because no user was found."
-msgstr ""
-
 msgid "Failed to remove mirror."
 msgstr ""
 
@@ -16048,9 +16045,6 @@ msgstr ""
 msgid "Failed to remove user key."
 msgstr ""
 
-msgid "Failed to request attention because no user was found."
-msgstr ""
-
 msgid "Failed to retrieve page"
 msgstr ""
 
@@ -32514,12 +32508,6 @@ msgstr ""
 msgid "Remove assignee"
 msgstr ""
 
-msgid "Remove attention request"
-msgstr ""
-
-msgid "Remove attention request(s)"
-msgstr ""
-
 msgid "Remove avatar"
 msgstr ""
 
@@ -32661,9 +32649,6 @@ msgstr ""
 msgid "Removed an issue from an epic."
 msgstr ""
 
-msgid "Removed attention from %{users_sentence}."
-msgstr ""
-
 msgid "Removed group can not be restored!"
 msgstr ""
 
@@ -32712,9 +32697,6 @@ msgstr ""
 msgid "Removes an issue from an epic."
 msgstr ""
 
-msgid "Removes attention from %{users_sentence}."
-msgstr ""
-
 msgid "Removes parent epic %{epic_ref}."
 msgstr ""
 
@@ -33191,15 +33173,6 @@ msgstr ""
 msgid "Request a new one"
 msgstr ""
 
-msgid "Request attention from %{users_sentence}."
-msgstr ""
-
-msgid "Request attention from assignee or reviewer"
-msgstr ""
-
-msgid "Request attention from assignee(s) or reviewer(s)"
-msgstr ""
-
 msgid "Request data is too large"
 msgstr ""
 
@@ -33224,9 +33197,6 @@ msgstr ""
 msgid "Requested %{time_ago}"
 msgstr ""
 
-msgid "Requested attention from %{users_sentence}."
-msgstr ""
-
 msgid "Requested design version does not exist."
 msgstr ""
 

spec/frontend/sidebar/reviewer_title_spec.js

@@ -47,7 +47,7 @@ describe('ReviewerTitle component', () => {
       editable: false,
     });
 
-    expect(wrapper.find(GlLoadingIcon).exists()).toBeFalsy();
+    expect(wrapper.find(GlLoadingIcon).exists()).toBe(false);
   });
 
   it('renders spinner when loading', () => {
@@ -57,7 +57,7 @@ describe('ReviewerTitle component', () => {
       editable: false,
     });
 
-    expect(wrapper.find(GlLoadingIcon).exists()).toBeTruthy();
+    expect(wrapper.find(GlLoadingIcon).exists()).toBe(true);
   });
 
   it('does not render edit link when not editable', () => {

spec/frontend/vue_shared/components/sidebar/todo_button_spec.js

@@ -37,7 +37,7 @@ describe('Todo Button', () => {
     createComponent({}, mount);
     wrapper.findComponent(GlButton).trigger('click');
 
-    expect(wrapper.emitted().click).toBeTruthy();
+    expect(wrapper.emitted().click).toHaveLength(1);
   });
 
   it('calls dispatchDocumentEvent to update global To-Do counter correctly', () => {

spec/lib/gitlab/data_builder/issuable_spec.rb

@@ -113,6 +113,7 @@ RSpec.describe Gitlab::DataBuilder::Issuable do
         expect(data[:object_attributes]['assignee_id']).to eq(user.id)
         expect(data[:assignees].first).to eq(user.hook_attrs)
         expect(data).not_to have_key(:assignee)
+        expect(data).not_to have_key(:reviewers)
       end
     end
 
@@ -126,5 +127,25 @@ RSpec.describe Gitlab::DataBuilder::Issuable do
         expect(data).not_to have_key(:assignee)
       end
     end
+
+    context 'merge_request is assigned reviewers' do
+      let(:merge_request) { create(:merge_request, reviewers: [user]) }
+      let(:data) { described_class.new(merge_request).build(user: user) }
+
+      it 'returns correct hook data' do
+        expect(data[:object_attributes]['reviewer_ids']).to match_array([user.id])
+        expect(data[:reviewers].first).to eq(user.hook_attrs)
+      end
+    end
+
+    context 'when merge_request does not have reviewers and assignees' do
+      let(:merge_request) { create(:merge_request) }
+      let(:data) { described_class.new(merge_request).build(user: user) }
+
+      it 'returns correct hook data' do
+        expect(data).not_to have_key(:assignees)
+        expect(data).not_to have_key(:reviewers)
+      end
+    end
   end
 end

spec/lib/gitlab/hook_data/merge_request_builder_spec.rb

@@ -29,6 +29,7 @@ RSpec.describe Gitlab::HookData::MergeRequestBuilder do
         merge_user_id
         merge_when_pipeline_succeeds
         milestone_id
+        reviewer_ids
         source_branch
         source_project_id
         state_id
@@ -72,6 +73,7 @@ RSpec.describe Gitlab::HookData::MergeRequestBuilder do
         human_time_estimate
         assignee_ids
         assignee_id
+        reviewer_ids
         labels
         state
         blocking_discussions_resolved

spec/models/concerns/issuable_spec.rb

@@ -569,6 +569,27 @@ RSpec.describe Issuable do
       end
     end
 
+    context 'merge_request update reviewers' do
+      let(:merge_request) { create(:merge_request) }
+      let(:user2) { create(:user) }
+
+      before do
+        merge_request.update!(reviewers: [user])
+        merge_request.update!(reviewers: [user, user2])
+        expect(Gitlab::DataBuilder::Issuable)
+          .to receive(:new).with(merge_request).and_return(builder)
+      end
+
+      it 'delegates to Gitlab::DataBuilder::Issuable#build' do
+        expect(builder).to receive(:build).with(
+          user: user,
+          changes: hash_including(
+            'reviewers' => [[user.hook_attrs], [user.hook_attrs, user2.hook_attrs]]
+          ))
+        merge_request.to_hook_data(user, old_associations: { reviewers: [user] })
+      end
+    end
+
     context 'incident severity is updated' do
       let(:issue) { create(:incident) }
 

spec/models/protected_branch_spec.rb

@@ -167,36 +167,130 @@ RSpec.describe ProtectedBranch do
         expect(described_class.protected?(project, nil)).to eq(false)
       end
 
-      context 'with caching', :use_clean_rails_memory_store_caching do
+      context 'with caching', :use_clean_rails_redis_caching do
         let_it_be(:project) { create(:project, :repository) }
         let_it_be(:protected_branch) { create(:protected_branch, project: project, name: "“jawn”") }
 
+        let(:feature_flag) { true }
+        let(:dry_run) { true }
+
+        shared_examples_for 'hash based cache implementation' do
+          it 'calls only hash based cache implementation' do
+            expect_next_instance_of(ProtectedBranches::CacheService) do |instance|
+              expect(instance).to receive(:fetch).with('missing-branch', anything).and_call_original
+            end
+
+            expect(Rails.cache).not_to receive(:fetch)
+
+            described_class.protected?(project, 'missing-branch', dry_run: dry_run)
+          end
+        end
+
         before do
-          allow(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).once.and_call_original
+          stub_feature_flags(hash_based_cache_for_protected_branches: feature_flag)
+          allow(described_class).to receive(:matching).and_call_original
 
           # the original call works and warms the cache
-          described_class.protected?(project, protected_branch.name)
+          described_class.protected?(project, protected_branch.name, dry_run: dry_run)
         end
 
-        it 'correctly invalidates a cache' do
+        context 'Dry-run: true' do
-          expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).once.and_call_original
+          it 'recalculates a fresh value every time in order to check the cache is not returning stale data' do
+            expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).twice
 
-          create(:protected_branch, project: project, name: "bar")
+            2.times { described_class.protected?(project, protected_branch.name) }
-          # the cache is invalidated because the project has been "updated"
+          end
-          expect(described_class.protected?(project, protected_branch.name)).to eq(true)
+
+          it_behaves_like 'hash based cache implementation'
         end
 
-        it 'correctly uses the cached version' do
+        context 'Dry-run: false' do
-          expect(described_class).not_to receive(:matching)
+          let(:dry_run) { false }
-          expect(described_class.protected?(project, protected_branch.name)).to eq(true)
+
+          it 'correctly invalidates a cache' do
+            expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).exactly(3).times.and_call_original
+
+            create_params = { name: 'bar', merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }] }
+            branch = ProtectedBranches::CreateService.new(project, project.owner, create_params).execute
+            expect(described_class.protected?(project, protected_branch.name, dry_run: dry_run)).to eq(true)
+
+            ProtectedBranches::UpdateService.new(project, project.owner, name: 'ber').execute(branch)
+            expect(described_class.protected?(project, protected_branch.name, dry_run: dry_run)).to eq(true)
+
+            ProtectedBranches::DestroyService.new(project, project.owner).execute(branch)
+            expect(described_class.protected?(project, protected_branch.name, dry_run: dry_run)).to eq(true)
+          end
+
+          it_behaves_like 'hash based cache implementation'
+
+          context 'when project is updated' do
+            it 'does not invalidate a cache' do
+              expect(described_class).not_to receive(:matching).with(protected_branch.name, protected_refs: anything)
+
+              project.touch
+
+              described_class.protected?(project, protected_branch.name, dry_run: dry_run)
+            end
+          end
+
+          context 'when other project protected branch is updated' do
+            it 'does not invalidate the current project cache' do
+              expect(described_class).not_to receive(:matching).with(protected_branch.name, protected_refs: anything)
+
+              another_project = create(:project)
+              ProtectedBranches::CreateService.new(another_project, another_project.owner, name: 'bar').execute
+
+              described_class.protected?(project, protected_branch.name, dry_run: dry_run)
+            end
+          end
+
+          it 'correctly uses the cached version' do
+            expect(described_class).not_to receive(:matching)
+
+            expect(described_class.protected?(project, protected_branch.name, dry_run: dry_run)).to eq(true)
+          end
         end
 
-        it 'sets expires_in for a cache key' do
+        context 'when feature flag hash_based_cache_for_protected_branches is off' do
-          cache_key = described_class.protected_ref_cache_key(project, protected_branch.name)
+          let(:feature_flag) { false }
 
-          expect(Rails.cache).to receive(:fetch).with(cache_key, expires_in: 1.hour)
+          it 'does not call hash based cache implementation' do
+            expect(ProtectedBranches::CacheService).not_to receive(:new)
+            expect(Rails.cache).to receive(:fetch).and_call_original
 
-          described_class.protected?(project, protected_branch.name)
+            described_class.protected?(project, 'missing-branch')
+          end
+
+          it 'correctly invalidates a cache' do
+            expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).once.and_call_original
+
+            create(:protected_branch, project: project, name: "bar")
+            # the cache is invalidated because the project has been "updated"
+            expect(described_class.protected?(project, protected_branch.name)).to eq(true)
+          end
+
+          it 'sets expires_in of 1 hour for the Rails cache key' do
+            cache_key = described_class.protected_ref_cache_key(project, protected_branch.name)
+
+            expect(Rails.cache).to receive(:fetch).with(cache_key, expires_in: 1.hour)
+
+            described_class.protected?(project, protected_branch.name)
+          end
+
+          context 'when project is updated' do
+            it 'invalidates Rails cache' do
+              expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).once.and_call_original
+
+              project.touch
+
+              described_class.protected?(project, protected_branch.name)
+            end
+          end
+
+          it 'correctly uses the cached version' do
+            expect(described_class).not_to receive(:matching)
+            expect(described_class.protected?(project, protected_branch.name)).to eq(true)
+          end
         end
       end
     end

spec/requests/api/graphql/mutations/merge_requests/request_attention_spec.rb

@@ -1,79 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe 'Request attention' do
-  include GraphqlHelpers
-
-  let_it_be(:current_user) { create(:user) }
-  let_it_be(:user) { create(:user) }
-  let_it_be(:merge_request) { create(:merge_request, reviewers: [user]) }
-  let_it_be(:project) { merge_request.project }
-
-  let(:input) { { user_id: global_id_of(user) } }
-
-  let(:mutation) do
-    variables = {
-      project_path: project.full_path,
-      iid: merge_request.iid.to_s
-    }
-    graphql_mutation(:merge_request_request_attention, variables.merge(input),
-                     <<-QL.strip_heredoc
-                       clientMutationId
-                       errors
-    QL
-    )
-  end
-
-  def mutation_response
-    graphql_mutation_response(:merge_request_request_attention)
-  end
-
-  def mutation_errors
-    mutation_response['errors']
-  end
-
-  before_all do
-    project.add_developer(current_user)
-    project.add_developer(user)
-  end
-
-  it 'is successful' do
-    post_graphql_mutation(mutation, current_user: current_user)
-
-    expect(response).to have_gitlab_http_status(:success)
-    expect(mutation_errors).to be_empty
-  end
-
-  context 'when current user is not allowed to update the merge request' do
-    it 'returns an error' do
-      post_graphql_mutation(mutation, current_user: create(:user))
-
-      expect(graphql_errors).not_to be_empty
-    end
-  end
-
-  context 'when user is not a reviewer' do
-    let(:input) { { user_id: global_id_of(create(:user)) } }
-
-    it 'returns an error' do
-      post_graphql_mutation(mutation, current_user: current_user)
-
-      expect(response).to have_gitlab_http_status(:success)
-      expect(mutation_errors).not_to be_empty
-    end
-  end
-
-  context 'feature flag is disabled' do
-    before do
-      stub_feature_flags(mr_attention_requests: false)
-    end
-
-    it 'returns an error' do
-      post_graphql_mutation(mutation, current_user: current_user)
-
-      expect(response).to have_gitlab_http_status(:success)
-      expect(graphql_errors[0]["message"]).to eq "Feature disabled"
-    end
-  end
-end

spec/requests/api/graphql/mutations/merge_requests/update_reviewer_state_spec.rb

@@ -1,65 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe 'Toggle attention requested for reviewer' do
-  include GraphqlHelpers
-
-  let(:current_user) { create(:user) }
-  let(:merge_request) { create(:merge_request, reviewers: [user]) }
-  let(:project) { merge_request.project }
-  let(:user) { create(:user) }
-  let(:input) { { user_id: global_id_of(user) } }
-
-  let(:mutation) do
-    variables = {
-      project_path: project.full_path,
-      iid: merge_request.iid.to_s
-    }
-    graphql_mutation(:merge_request_toggle_attention_requested, variables.merge(input),
-                     <<-QL.strip_heredoc
-                       clientMutationId
-                       errors
-    QL
-    )
-  end
-
-  def mutation_response
-    graphql_mutation_response(:merge_request_toggle_attention_requested)
-  end
-
-  def mutation_errors
-    mutation_response['errors']
-  end
-
-  before do
-    project.add_developer(current_user)
-    project.add_developer(user)
-  end
-
-  it 'returns an error if the user is not allowed to update the merge request' do
-    post_graphql_mutation(mutation, current_user: create(:user))
-
-    expect(graphql_errors).not_to be_empty
-  end
-
-  describe 'reviewer does not exist' do
-    let(:input) { { user_id: global_id_of(create(:user)) } }
-
-    it 'returns an error' do
-      post_graphql_mutation(mutation, current_user: current_user)
-
-      expect(response).to have_gitlab_http_status(:success)
-      expect(mutation_errors).not_to be_empty
-    end
-  end
-
-  describe 'reviewer exists' do
-    it 'does not return an error' do
-      post_graphql_mutation(mutation, current_user: current_user)
-
-      expect(response).to have_gitlab_http_status(:success)
-      expect(mutation_errors).to be_empty
-    end
-  end
-end

spec/requests/api/graphql/mutations/remove_attention_request_spec.rb

@@ -1,79 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe 'Remove attention request' do
-  include GraphqlHelpers
-
-  let_it_be(:current_user) { create(:user) }
-  let_it_be(:user) { create(:user) }
-  let_it_be(:merge_request) { create(:merge_request, reviewers: [user]) }
-  let_it_be(:project) { merge_request.project }
-
-  let(:input) { { user_id: global_id_of(user) } }
-
-  let(:mutation) do
-    variables = {
-      project_path: project.full_path,
-      iid: merge_request.iid.to_s
-    }
-    graphql_mutation(:merge_request_remove_attention_request, variables.merge(input),
-                     <<-QL.strip_heredoc
-                       clientMutationId
-                       errors
-    QL
-    )
-  end
-
-  def mutation_response
-    graphql_mutation_response(:merge_request_remove_attention_request)
-  end
-
-  def mutation_errors
-    mutation_response['errors']
-  end
-
-  before_all do
-    project.add_developer(current_user)
-    project.add_developer(user)
-  end
-
-  it 'is successful' do
-    post_graphql_mutation(mutation, current_user: current_user)
-
-    expect(response).to have_gitlab_http_status(:success)
-    expect(mutation_errors).to be_empty
-  end
-
-  context 'when current user is not allowed to update the merge request' do
-    it 'returns an error' do
-      post_graphql_mutation(mutation, current_user: create(:user))
-
-      expect(graphql_errors).not_to be_empty
-    end
-  end
-
-  context 'when user is not a reviewer' do
-    let(:input) { { user_id: global_id_of(create(:user)) } }
-
-    it 'returns an error' do
-      post_graphql_mutation(mutation, current_user: current_user)
-
-      expect(response).to have_gitlab_http_status(:success)
-      expect(mutation_errors).not_to be_empty
-    end
-  end
-
-  context 'feature flag is disabled' do
-    before do
-      stub_feature_flags(mr_attention_requests: false)
-    end
-
-    it 'returns an error' do
-      post_graphql_mutation(mutation, current_user: current_user)
-
-      expect(response).to have_gitlab_http_status(:success)
-      expect(graphql_errors[0]["message"]).to eq "Feature disabled"
-    end
-  end
-end

spec/requests/api/user_counts_spec.rb

@@ -43,21 +43,6 @@ RSpec.describe API::UserCounts do
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response).to be_a Hash
           expect(json_response['merge_requests']).to eq(2)
-          expect(json_response['attention_requests']).to eq(0)
-        end
-
-        describe 'mr_attention_requests is disabled' do
-          before do
-            stub_feature_flags(mr_attention_requests: false)
-          end
-
-          it 'does not include attention_requests count' do
-            create(:merge_request, source_project: project, author: user, assignees: [user])
-
-            get api('/user_counts', user)
-
-            expect(json_response.key?('attention_requests')).to be(false)
-          end
         end
       end
 

spec/serializers/merge_request_user_entity_spec.rb

@@ -18,8 +18,7 @@ RSpec.describe MergeRequestUserEntity do
     it 'exposes needed attributes' do
       is_expected.to include(
         :id, :name, :username, :state, :avatar_url, :web_url,
-        :can_merge, :can_update_merge_request, :reviewed, :approved,
+        :can_merge, :can_update_merge_request, :reviewed, :approved
-        :attention_requested
       )
     end
 
@@ -57,14 +56,6 @@ RSpec.describe MergeRequestUserEntity do
       end
     end
 
-    context 'attention_requested' do
-      before do
-        merge_request.find_assignee(user).update!(state: :attention_requested)
-      end
-
-      it { is_expected.to include(attention_requested: true ) }
-    end
-
     describe 'performance' do
       let_it_be(:user_a) { create(:user) }
       let_it_be(:user_b) { create(:user) }

spec/services/issuable/common_system_notes_service_spec.rb

@@ -8,6 +8,37 @@ RSpec.describe Issuable::CommonSystemNotesService do
 
   let(:issuable) { create(:issue, project: project) }
 
+  shared_examples 'system note for issuable date changes' do
+    it 'creates a system note for due_date set' do
+      issuable.update!(due_date: Date.today)
+
+      expect { subject }.to change(issuable.notes, :count).from(0).to(1)
+      expect(issuable.notes.last.note).to match('changed due date to')
+    end
+
+    it 'creates a system note for start_date set' do
+      issuable.update!(start_date: Date.today)
+
+      expect { subject }.to change(issuable.notes, :count).from(0).to(1)
+      expect(issuable.notes.last.note).to match('changed start date to')
+    end
+
+    it 'creates a note when both start and due date are changed' do
+      issuable.update!(start_date: Date.today, due_date: 1.week.from_now)
+
+      expect { subject }.to change { issuable.notes.count }.from(0).to(1)
+      expect(issuable.notes.last.note).to match(/changed start date to.+and changed due date to/)
+    end
+
+    it 'does not call SystemNoteService if no dates are changed' do
+      issuable.update!(title: 'new title')
+
+      expect(SystemNoteService).not_to receive(:change_start_date_or_due_date)
+
+      subject
+    end
+  end
+
   context 'on issuable update' do
     it_behaves_like 'system note creation', { title: 'New title' }, 'changed title'
     it_behaves_like 'system note creation', { description: 'New description' }, 'changed the description'
@@ -61,6 +92,12 @@ RSpec.describe Issuable::CommonSystemNotesService do
         end
       end
     end
+
+    context 'when changing dates' do
+      it_behaves_like 'system note for issuable date changes' do
+        subject { described_class.new(project: project, current_user: user).execute(issuable) }
+      end
+    end
   end
 
   context 'on issuable create' do
@@ -102,12 +139,8 @@ RSpec.describe Issuable::CommonSystemNotesService do
       end
     end
 
-    it 'creates a system note for due_date set' do
+    context 'when changing dates' do
-      issuable.due_date = Date.today
+      it_behaves_like 'system note for issuable date changes'
-      issuable.save!
-
-      expect { subject }.to change { issuable.notes.count }.from(0).to(1)
-      expect(issuable.notes.last.note).to match('changed due date')
     end
   end
 end

spec/services/issues/clone_service_spec.rb

@@ -182,19 +182,21 @@ RSpec.describe Issues::CloneService do
 
       context 'issue with due date' do
         let(:date) { Date.parse('2020-01-10') }
+        let(:new_date) { date + 1.week }
 
         let(:old_issue) do
           create(:issue, title: title, description: description, project: old_project, author: author, due_date: date)
         end
 
         before do
-          SystemNoteService.change_due_date(old_issue, old_project, author, old_issue.due_date)
+          old_issue.update!(due_date: new_date)
+          SystemNoteService.change_start_date_or_due_date(old_issue, old_project, author, old_issue.previous_changes.slice('due_date'))
         end
 
         it 'keeps the same due date' do
           new_issue = clone_service.execute(old_issue, new_project)
 
-          expect(new_issue.due_date).to eq(date)
+          expect(new_issue.due_date).to eq(old_issue.due_date)
         end
       end
 

spec/services/issues/move_service_spec.rb

@@ -140,7 +140,8 @@ RSpec.describe Issues::MoveService do
         end
 
         before do
-          SystemNoteService.change_due_date(old_issue, old_project, author, old_issue.due_date)
+          old_issue.update!(due_date: Date.today)
+          SystemNoteService.change_start_date_or_due_date(old_issue, old_project, author, old_issue.previous_changes.slice('due_date'))
         end
 
         it 'does not create extra system notes' do

spec/services/protected_branches/cache_service_spec.rb

@@ -63,6 +63,38 @@ RSpec.describe ProtectedBranches::CacheService, :clean_gitlab_redis_cache do
         expect(service.fetch('not-found') { true }).to eq(true)
       end
     end
+
+    context 'when dry_run is on' do
+      it 'does not use cached value' do
+        expect(service.fetch('main', dry_run: true) { true }).to eq(true)
+        expect(service.fetch('main', dry_run: true) { false }).to eq(false)
+      end
+
+      context 'when cache mismatch' do
+        it 'logs an error' do
+          expect(service.fetch('main', dry_run: true) { true }).to eq(true)
+
+          expect(Gitlab::AppLogger).to receive(:error).with(
+            'class' => described_class.name,
+            'message' => /Cache mismatch/,
+            'project_id' => project.id,
+            'project_path' => project.full_path
+          )
+
+          expect(service.fetch('main', dry_run: true) { false }).to eq(false)
+        end
+      end
+
+      context 'when cache matches' do
+        it 'does not log an error' do
+          expect(service.fetch('main', dry_run: true) { true }).to eq(true)
+
+          expect(Gitlab::AppLogger).not_to receive(:error)
+
+          expect(service.fetch('main', dry_run: true) { true }).to eq(true)
+        end
+      end
+    end
   end
 
   describe '#refresh' do

spec/services/quick_actions/interpret_service_spec.rb

@@ -810,38 +810,6 @@ RSpec.describe QuickActions::InterpretService do
       end
     end
 
-    shared_examples 'attention command' do
-      it 'updates reviewers attention status' do
-        _, _, message = service.execute(content, issuable)
-
-        expect(message).to eq("Requested attention from #{developer.to_reference}.")
-
-        reviewer.reload
-
-        expect(reviewer).to be_attention_requested
-      end
-
-      it 'supports attn alias' do
-        attn_cmd = content.gsub(/attention/, 'attn')
-        _, _, message = service.execute(attn_cmd, issuable)
-
-        expect(message).to eq("Requested attention from #{developer.to_reference}.")
-
-        reviewer.reload
-
-        expect(reviewer).to be_attention_requested
-      end
-    end
-
-    shared_examples 'remove attention command' do
-      it 'updates reviewers attention status' do
-        _, _, message = service.execute(content, issuable)
-
-        expect(message).to eq("Removed attention from #{developer.to_reference}.")
-        expect(reviewer).not_to be_attention_requested
-      end
-    end
-
     it_behaves_like 'reopen command' do
       let(:content) { '/reopen' }
       let(:issuable) { issue }
@@ -2481,82 +2449,6 @@ RSpec.describe QuickActions::InterpretService do
         expect(message).to eq('One or more contacts were successfully removed.')
       end
     end
-
-    describe 'attention command' do
-      let(:issuable) { create(:merge_request, reviewers: [developer], source_project: project) }
-      let(:reviewer) { issuable.merge_request_reviewers.find_by(user_id: developer.id) }
-      let(:content) { "/attention @#{developer.username}" }
-
-      context 'with one user' do
-        before do
-          reviewer.update!(state: :reviewed)
-        end
-
-        it_behaves_like 'attention command'
-      end
-
-      context 'with no user' do
-        let(:content) { "/attention" }
-
-        it_behaves_like 'failed command', 'Failed to request attention because no user was found.'
-      end
-
-      context 'with incorrect permissions' do
-        let(:service) { described_class.new(project, create(:user)) }
-
-        it_behaves_like 'failed command', 'Could not apply attention command.'
-      end
-
-      context 'with feature flag disabled' do
-        before do
-          stub_feature_flags(mr_attention_requests: false)
-        end
-
-        it_behaves_like 'failed command', 'Could not apply attention command.'
-      end
-
-      context 'with an issue instead of a merge request' do
-        let(:issuable) { issue }
-
-        it_behaves_like 'failed command', 'Could not apply attention command.'
-      end
-    end
-
-    describe 'remove attention command' do
-      let(:issuable) { create(:merge_request, reviewers: [developer], source_project: project) }
-      let(:reviewer) { issuable.merge_request_reviewers.find_by(user_id: developer.id) }
-      let(:content) { "/remove_attention @#{developer.username}" }
-
-      context 'with one user' do
-        it_behaves_like 'remove attention command'
-      end
-
-      context 'with no user' do
-        let(:content) { "/remove_attention" }
-
-        it_behaves_like 'failed command', 'Failed to remove attention because no user was found.'
-      end
-
-      context 'with incorrect permissions' do
-        let(:service) { described_class.new(project, create(:user)) }
-
-        it_behaves_like 'failed command', 'Could not apply remove_attention command.'
-      end
-
-      context 'with feature flag disabled' do
-        before do
-          stub_feature_flags(mr_attention_requests: false)
-        end
-
-        it_behaves_like 'failed command', 'Could not apply remove_attention command.'
-      end
-
-      context 'with an issue instead of a merge request' do
-        let(:issuable) { issue }
-
-        it_behaves_like 'failed command', 'Could not apply remove_attention command.'
-      end
-    end
   end
 
   describe '#explain' do

spec/services/system_note_service_spec.rb

@@ -134,15 +134,15 @@ RSpec.describe SystemNoteService do
     end
   end
 
-  describe '.change_due_date' do
+  describe '.change_start_date_or_due_date' do
-    let(:due_date) { double }
+    let(:changed_dates) { double }
 
     it 'calls TimeTrackingService' do
       expect_next_instance_of(::SystemNotes::TimeTrackingService) do |service|
-        expect(service).to receive(:change_due_date).with(due_date)
+        expect(service).to receive(:change_start_date_or_due_date).with(changed_dates)
       end
 
-      described_class.change_due_date(noteable, project, author, due_date)
+      described_class.change_start_date_or_due_date(noteable, project, author, changed_dates)
     end
   end
 

spec/services/system_notes/time_tracking_service_spec.rb

@@ -6,38 +6,94 @@ RSpec.describe ::SystemNotes::TimeTrackingService do
   let_it_be(:author)   { create(:user) }
   let_it_be(:project)  { create(:project, :repository) }
 
-  describe '#change_due_date' do
+  describe '#change_start_date_or_due_date' do
-    subject { described_class.new(noteable: noteable, project: project, author: author).change_due_date(due_date) }
+    subject(:note) { described_class.new(noteable: noteable, project: project, author: author).change_start_date_or_due_date(changed_dates) }
 
-    let(:due_date) { Date.today }
+    let(:start_date) { Date.today }
+    let(:due_date) { 1.week.from_now.to_date }
+    let(:changed_dates) { { 'due_date' => [nil, due_date], 'start_date' => [nil, start_date] } }
+
+    let_it_be(:noteable) { create(:issue, project: project) }
 
     context 'when noteable is an issue' do
-      let_it_be(:noteable) { create(:issue, project: project) }
-
       it_behaves_like 'a note with overridable created_at'
 
       it_behaves_like 'a system note' do
-        let(:action) { 'due_date' }
+        let(:action) { 'start_date_or_due_date' }
       end
 
-      context 'when due date added' do
+      context 'when both dates are added' do
-        it 'sets the note text' do
+        it 'sets the correct note message' do
-          expect(subject.note).to eq "changed due date to #{due_date.to_s(:long)}"
+          expect(note.note).to eq("changed start date to #{start_date.to_s(:long)} and changed due date to #{due_date.to_s(:long)}")
         end
       end
 
-      context 'when due date removed' do
+      context 'when both dates are removed' do
-        let(:due_date) { nil }
+        let(:changed_dates) { { 'due_date' => [due_date, nil], 'start_date' => [start_date, nil] } }
 
-        it 'sets the note text' do
+        before do
-          expect(subject.note).to eq 'removed due date'
+          noteable.update!(start_date: start_date, due_date: due_date)
+        end
+
+        it 'sets the correct note message' do
+          expect(note.note).to eq('removed start date and removed due date')
         end
       end
 
-      it 'tracks the issue event in usage ping' do
+      context 'when due date is added' do
-        expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_due_date_changed_action).with(author: author)
+        let(:changed_dates) { { 'due_date' => [nil, due_date] } }
 
-        subject
+        it 'tracks the issue event in usage ping' do
+          expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_due_date_changed_action).with(author: author)
+
+          subject
+        end
+
+        it 'sets the correct note message' do
+          expect(note.note).to eq("changed due date to #{due_date.to_s(:long)}")
+        end
+
+        context 'and start date removed' do
+          let(:changed_dates) { { 'due_date' => [nil, due_date], 'start_date' => [start_date, nil] } }
+
+          it 'sets the correct note message' do
+            expect(note.note).to eq("removed start date and changed due date to #{due_date.to_s(:long)}")
+          end
+        end
+      end
+
+      context 'when start_date is added' do
+        let(:changed_dates) { { 'start_date' => [nil, start_date] } }
+
+        it 'does not track the issue event in usage ping' do
+          expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_due_date_changed_action)
+
+          subject
+        end
+
+        it 'sets the correct note message' do
+          expect(note.note).to eq("changed start date to #{start_date.to_s(:long)}")
+        end
+
+        context 'and due date removed' do
+          let(:changed_dates) { { 'due_date' => [due_date, nil], 'start_date' => [nil, start_date] } }
+
+          it 'sets the correct note message' do
+            expect(note.note).to eq("changed start date to #{start_date.to_s(:long)} and removed due date")
+          end
+        end
+      end
+
+      context 'when no dates are changed' do
+        let(:changed_dates) { {} }
+
+        it 'does not create a note and returns nil' do
+          expect do
+            note
+          end.to not_change(Note, :count)
+
+          expect(note).to be_nil
+        end
       end
     end
 
@@ -45,7 +101,7 @@ RSpec.describe ::SystemNotes::TimeTrackingService do
       let_it_be(:noteable) { create(:merge_request, source_project: project) }
 
       it 'does not track the issue event in usage ping' do
-        expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_due_date_changed_action).with(author: author)
+        expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_due_date_changed_action)
 
         subject
       end