From 5d7584ecb5895b04d5555ccc1beb15cd5e39ff17 Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Thu, 5 Oct 2023 12:15:32 -0600
Subject: [PATCH] Move notifications and todos on issues

Changelog: changed
---
 .../issues/show/components/header_actions.vue |  2 +-
 .../components/sticky_header.vue              |  2 +-
 .../notes/components/discussion_counter.vue   |  2 +-
 .../sidebar_subscriptions_widget.vue          | 58 ++++++++++---------
 .../todo_toggle/sidebar_todo_widget.vue       | 28 +++++----
 .../components/todo_toggle/todo_button.vue    |  2 +-
 .../components/mr_more_dropdown.vue           |  2 +-
 app/assets/stylesheets/framework/sidebar.scss |  4 ++
 .../projects/incidents_controller.rb          |  1 +
 app/controllers/projects/issues_controller.rb |  1 +
 app/views/shared/issuable/_sidebar.html.haml  |  9 ++-
 spec/features/issues/todo_spec.rb             |  1 +
 .../issues/user_toggles_subscription_spec.rb  | 23 ++++++++
 13 files changed, 85 insertions(+), 50 deletions(-)

diff --git a/app/assets/javascripts/issues/show/components/header_actions.vue b/app/assets/javascripts/issues/show/components/header_actions.vue
index 681d2c79ba14ae..13bcb4706c9510 100644
--- a/app/assets/javascripts/issues/show/components/header_actions.vue
+++ b/app/assets/javascripts/issues/show/components/header_actions.vue
@@ -406,7 +406,7 @@ export default {
       right
       @shown="dismissPopover"
     >
-      <template v-if="showMovedSidebarOptions">
+      <template v-if="showMovedSidebarOptions && !glFeatures.notificationsTodosButtons">
         <sidebar-subscriptions-widget
           :iid="String(iid)"
           :full-path="fullPath"
diff --git a/app/assets/javascripts/merge_requests/components/sticky_header.vue b/app/assets/javascripts/merge_requests/components/sticky_header.vue
index acb7b8353c0765..e8bdb854334014 100644
--- a/app/assets/javascripts/merge_requests/components/sticky_header.vue
+++ b/app/assets/javascripts/merge_requests/components/sticky_header.vue
@@ -87,7 +87,7 @@ export default {
       return isLoggedIn();
     },
     isNotificationsTodosButtons() {
-      return this.glFeatures.notificationsTodosButtons;
+      return this.glFeatures.notificationsTodosButtons && this.glFeatures.movedMrSidebar;
     },
   },
   watch: {
diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index 9404fd04bc914d..b392ad55fa28d7 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -72,7 +72,7 @@ export default {
       return options;
     },
     isNotificationsTodosButtons() {
-      return this.glFeatures.notificationsTodosButtons;
+      return this.glFeatures.notificationsTodosButtons && this.glFeatures.movedMrSidebar;
     },
   },
   methods: {
diff --git a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue
index 54b9ad90b62419..866db2a43b8f79 100644
--- a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue
+++ b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue
@@ -2,7 +2,6 @@
 import {
   GlButton,
   GlDisclosureDropdownItem,
-  GlDropdownForm,
   GlIcon,
   GlLoadingIcon,
   GlToggle,
@@ -33,7 +32,6 @@ export default {
   components: {
     GlButton,
     GlDisclosureDropdownItem,
-    GlDropdownForm,
     GlIcon,
     GlLoadingIcon,
     GlToggle,
@@ -133,7 +131,10 @@ export default {
       return this.emailsDisabled || !this.isLoggedIn;
     },
     isNotificationsTodosButtons() {
-      return this.glFeatures.notificationsTodosButtons;
+      return this.glFeatures.notificationsTodosButtons && this.glFeatures.movedMrSidebar;
+    },
+    isMergeRequest() {
+      return this.issuableType === 'merge_request';
     },
   },
   methods: {
@@ -199,20 +200,8 @@ export default {
 </script>
 
 <template>
-  <gl-dropdown-form v-if="isMovedMrSidebar && isIssuable" class="gl-dropdown-item">
-    <div class="gl-px-5 gl-pb-2 gl-pt-1">
-      <gl-toggle
-        :value="subscribed"
-        :label="$options.i18n.notifications"
-        class="merge-request-notification-toggle"
-        label-position="left"
-        data-testid="notification-toggle"
-        @change="toggleSubscribed"
-      />
-    </div>
-  </gl-dropdown-form>
   <gl-disclosure-dropdown-item
-    v-else-if="isMovedMrSidebar && !isNotificationsTodosButtons"
+    v-if="isMovedMrSidebar && !isNotificationsTodosButtons"
     data-testid="notification-toggle"
     @action="toggleSubscribed"
   >
@@ -225,17 +214,32 @@ export default {
       />
     </template>
   </gl-disclosure-dropdown-item>
-  <gl-button
-    v-else-if="isNotificationsTodosButtons"
-    ref="tooltip"
-    v-gl-tooltip.hover.top
-    category="secondary"
-    data-testid="subscribe-button"
-    :title="notificationTooltip"
-    @click="toggleSubscribed"
-  >
-    <gl-icon :name="notificationIcon" :size="16" :class="{ 'gl-fill-blue-500': subscribed }" />
-  </gl-button>
+  <div v-else-if="isNotificationsTodosButtons" :class="{ 'inline-block': !isMergeRequest }">
+    <gl-button
+      ref="tooltip"
+      v-gl-tooltip.hover.top
+      category="secondary"
+      data-testid="subscribe-button"
+      class="hide-collapsed"
+      :title="notificationTooltip"
+      :class="{ 'gl-ml-2': isIssuable, 'btn-icon': isNotificationsTodosButtons }"
+      @click="toggleSubscribed"
+    >
+      <gl-icon :name="notificationIcon" :size="16" :class="{ 'gl-fill-blue-500': subscribed }" />
+    </gl-button>
+    <gl-button
+      v-if="!isMergeRequest"
+      ref="tooltip"
+      v-gl-tooltip.left.viewport
+      category="secondary"
+      data-testid="subscribe-button"
+      :title="notificationTooltip"
+      class="sidebar-collapsed-icon sidebar-collapsed-container gl-rounded-0! gl-shadow-none!"
+      @click="toggleSubscribed"
+    >
+      <gl-icon :name="notificationIcon" :size="16" :class="{ 'gl-fill-blue-500': subscribed }" />
+    </gl-button>
+  </div>
   <sidebar-editable-item
     v-else
     ref="editable"
diff --git a/app/assets/javascripts/sidebar/components/todo_toggle/sidebar_todo_widget.vue b/app/assets/javascripts/sidebar/components/todo_toggle/sidebar_todo_widget.vue
index 31133139bd4f9b..e034607f79ce32 100644
--- a/app/assets/javascripts/sidebar/components/todo_toggle/sidebar_todo_widget.vue
+++ b/app/assets/javascripts/sidebar/components/todo_toggle/sidebar_todo_widget.vue
@@ -115,7 +115,7 @@ export default {
       return todoLabel(this.hasTodo);
     },
     isNotificationsTodosButtons() {
-      return this.glFeatures.notificationsTodosButtons;
+      return this.glFeatures.notificationsTodosButtons && this.glFeatures.movedMrSidebar;
     },
   },
   methods: {
@@ -186,27 +186,25 @@ export default {
 </script>
 
 <template>
-  <gl-button
-    v-if="isNotificationsTodosButtons && isMergeRequest"
-    v-gl-tooltip.hover.top
-    data-testid="sidebar-todo"
-    :title="tootltipTitle"
-    category="secondary"
-    type="reset"
-    @click.stop.prevent="toggleTodo"
-  >
-    <gl-icon :class="{ 'todo-undone gl-fill-blue-500': hasTodo }" :name="collapsedButtonIcon" />
-  </gl-button>
-  <div v-else data-testid="sidebar-todo">
+  <div data-testid="sidebar-todo" :class="{ 'inline-block': !isMergeRequest }">
     <todo-button
+      v-gl-tooltip.hover.top
+      :title="isNotificationsTodosButtons ? tootltipTitle : undefined"
       :issuable-type="issuableType"
       :issuable-id="issuableId"
       :is-todo="hasTodo"
       :loading="isLoading"
-      :size="isMergeRequest ? 'medium' : 'small'"
+      :size="isMergeRequest || isNotificationsTodosButtons ? 'medium' : 'small'"
+      :class="{ 'btn-icon': isNotificationsTodosButtons }"
       class="hide-collapsed"
       @click.stop.prevent="toggleTodo"
-    />
+    >
+      <gl-icon
+        v-if="isNotificationsTodosButtons"
+        :class="{ 'todo-undone gl-fill-blue-500': hasTodo }"
+        :name="collapsedButtonIcon"
+      />
+    </todo-button>
     <gl-button
       v-if="isClassicSidebar && !isMergeRequest"
       v-gl-tooltip.left.viewport
diff --git a/app/assets/javascripts/sidebar/components/todo_toggle/todo_button.vue b/app/assets/javascripts/sidebar/components/todo_toggle/todo_button.vue
index b49b8fc389b7e5..2aa79b450939e8 100644
--- a/app/assets/javascripts/sidebar/components/todo_toggle/todo_button.vue
+++ b/app/assets/javascripts/sidebar/components/todo_toggle/todo_button.vue
@@ -39,6 +39,6 @@ export default {
 
 <template>
   <gl-button v-bind="$attrs" :aria-label="buttonLabel" @click="onToggle($event)">
-    {{ buttonLabel }}
+    <slot>{{ buttonLabel }}</slot>
   </gl-button>
 </template>
diff --git a/app/assets/javascripts/vue_shared/components/mr_more_dropdown.vue b/app/assets/javascripts/vue_shared/components/mr_more_dropdown.vue
index e2227904daf52f..8d228216d844e0 100644
--- a/app/assets/javascripts/vue_shared/components/mr_more_dropdown.vue
+++ b/app/assets/javascripts/vue_shared/components/mr_more_dropdown.vue
@@ -144,7 +144,7 @@ export default {
       return this.glFeatures.movedMrSidebar;
     },
     isNotificationsTodosButtons() {
-      return this.glFeatures.notificationsTodosButtons;
+      return this.glFeatures.notificationsTodosButtons && this.glFeatures.movedMrSidebar;
     },
     draftLabel() {
       return this.draft ? this.$options.i18n.markAsReady : this.$options.i18n.markAsDraft;
diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss
index 9c31559cc0ea08..0619d5f166e999 100644
--- a/app/assets/stylesheets/framework/sidebar.scss
+++ b/app/assets/stylesheets/framework/sidebar.scss
@@ -55,6 +55,10 @@
   padding-right: 0;
   z-index: $zindex-dropdown-menu;
 
+  .inline-block {
+    @include gl-display-inline-block;
+  }
+
   &.right-sidebar-merge-requests {
     width: $right-sidebar-width;
 
diff --git a/app/controllers/projects/incidents_controller.rb b/app/controllers/projects/incidents_controller.rb
index e78c9171a53590..bacf3192ee67e0 100644
--- a/app/controllers/projects/incidents_controller.rb
+++ b/app/controllers/projects/incidents_controller.rb
@@ -12,6 +12,7 @@ class Projects::IncidentsController < Projects::ApplicationController
     push_force_frontend_feature_flag(:work_items_mvc_2, @project&.work_items_mvc_2_feature_flag_enabled?)
     push_frontend_feature_flag(:moved_mr_sidebar, project)
     push_force_frontend_feature_flag(:linked_work_items, @project&.linked_work_items_feature_flag_enabled?)
+    push_frontend_feature_flag(:notifications_todos_buttons, project)
   end
 
   feature_category :incident_management
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 80783534ca3160..4849cccac52a11 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -70,6 +70,7 @@ class Projects::IssuesController < Projects::ApplicationController
     push_frontend_feature_flag(:epic_widget_edit_confirmation, project)
     push_frontend_feature_flag(:moved_mr_sidebar, project)
     push_force_frontend_feature_flag(:linked_work_items, project.linked_work_items_feature_flag_enabled?)
+    push_frontend_feature_flag(:notifications_todos_buttons, project)
   end
 
   around_action :allow_gitaly_ref_name_caching, only: [:discussions]
diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml
index 3afde51041e18c..f863fbd296f35c 100644
--- a/app/views/shared/issuable/_sidebar.html.haml
+++ b/app/views/shared/issuable/_sidebar.html.haml
@@ -15,11 +15,14 @@
 
 %aside.right-sidebar.js-right-sidebar.js-issuable-sidebar{ data: { always_show_toggle: true, signed: { in: signed_in }, issuable_type: issuable_type }, class: "#{sidebar_gutter_collapsed_class(is_merge_request_with_flag)} #{'right-sidebar-merge-requests' if is_merge_request_with_flag}", 'aria-live' => 'polite', 'aria-label': issuable_type }
   .issuable-sidebar{ class: "#{'is-merge-request' if is_merge_request_with_flag}" }
-    .issuable-sidebar-header{ class: "#{'gl-pb-2! gl-md-display-flex gl-justify-content-end gl-lg-display-none!' if is_merge_request_with_flag}" }
+    .issuable-sidebar-header{ class: "gl-pb-4! #{'gl-pb-2! gl-md-display-flex gl-justify-content-end gl-lg-display-none!' if is_merge_request_with_flag}" }
       = render Pajamas::ButtonComponent.new(button_options: { class: "gutter-toggle float-right js-sidebar-toggle has-tooltip gl-shadow-none! #{'gl-display-block' if moved_sidebar_enabled}", type: 'button', 'aria-label' => _('Toggle sidebar'), title: sidebar_gutter_tooltip_text, data: { container: 'body', placement: 'left', boundary: 'viewport' } }) do
         = sidebar_gutter_toggle_icon
-      - if signed_in && !is_merge_request_with_flag
-        .js-sidebar-todo-widget-root{ data: { project_path: issuable_sidebar[:project_full_path], iid: issuable_sidebar[:iid], id: issuable_sidebar[:id] } }
+      - if signed_in
+        - if !is_merge_request_with_flag
+          .js-sidebar-todo-widget-root{ data: { project_path: issuable_sidebar[:project_full_path], iid: issuable_sidebar[:iid], id: issuable_sidebar[:id] } }
+        - if notifications_todos_buttons_enabled?
+          .js-sidebar-subscriptions-widget-root
 
     = form_for issuable_type, url: issuable_sidebar[:issuable_json_path], remote: true, html: { class: 'issuable-context-form inline-update js-issuable-update' } do |f|
       .block.assignee{ class: "#{'gl-mt-3' if !signed_in && moved_sidebar_enabled}", data: { testid: 'assignee-block-container' } }
diff --git a/spec/features/issues/todo_spec.rb b/spec/features/issues/todo_spec.rb
index c503c18be8d885..2095453ac29cd8 100644
--- a/spec/features/issues/todo_spec.rb
+++ b/spec/features/issues/todo_spec.rb
@@ -8,6 +8,7 @@
   let!(:user)    { create(:user, :no_super_sidebar) }
 
   before do
+    stub_feature_flags(notifications_todos_buttons: false)
     project.add_maintainer(user)
     sign_in(user)
     visit project_issue_path(project, issue)
diff --git a/spec/features/issues/user_toggles_subscription_spec.rb b/spec/features/issues/user_toggles_subscription_spec.rb
index af8a31afd5f007..713a169f061ee3 100644
--- a/spec/features/issues/user_toggles_subscription_spec.rb
+++ b/spec/features/issues/user_toggles_subscription_spec.rb
@@ -11,6 +11,7 @@
   context 'user is not logged in' do
     before do
       stub_feature_flags(moved_mr_sidebar: false)
+      stub_feature_flags(notifications_todos_buttons: false)
       visit(project_issue_path(project, issue))
     end
 
@@ -22,6 +23,7 @@
   context 'user is logged in' do
     before do
       stub_feature_flags(moved_mr_sidebar: false)
+      stub_feature_flags(notifications_todos_buttons: false)
       project.add_developer(user)
       sign_in(user)
       visit(project_issue_path(project, issue))
@@ -54,6 +56,7 @@
   context 'user is logged in without edit permission' do
     before do
       stub_feature_flags(moved_mr_sidebar: false)
+      stub_feature_flags(notifications_todos_buttons: false)
       sign_in(user2)
 
       visit(project_issue_path(project, issue))
@@ -73,4 +76,24 @@
       expect(subscription_button).to have_css("button.is-checked")
     end
   end
+
+  context 'with notifications_todos_buttons feature flag enabled' do
+    before do
+      stub_feature_flags(moved_mr_sidebar: true)
+      stub_feature_flags(notifications_todos_buttons: true)
+      sign_in(user2)
+
+      visit(project_issue_path(project, issue))
+    end
+
+    it 'toggles subscription' do
+      subscription_button = find('[data-testid="subscribe-button"]')
+
+      expect(page).to have_selector("button[title='Notifications off']")
+      subscription_button.click
+      wait_for_requests
+
+      expect(page).to have_selector("button[title='Notifications on']")
+    end
+  end
 end
-- 
GitLab