From 6e6b7f446a28f007aced48e5338ab86eeddc2726 Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Thu, 28 Mar 2024 13:52:07 -0600
Subject: [PATCH 1/5] Simplify HAML notes in commits/snippets/wiki

---
 app/helpers/application_helper.rb           |  4 +-
 app/views/discussions/_discussion.html.haml | 45 +----------------
 app/views/discussions/_headline.html.haml   | 16 ------
 app/views/shared/notes/_note.html.haml      | 55 +++++++++++----------
 4 files changed, 31 insertions(+), 89 deletions(-)
 delete mode 100644 app/views/discussions/_headline.html.haml

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index e8567f6d5c397e..25824c364c4e85 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -207,11 +207,11 @@ def time_ago_with_tooltip(time, placement: 'top', html_class: '', short_format:
   def edited_time_ago_with_tooltip(editable_object, placement: 'top', html_class: 'time_ago', exclude_author: false)
     return unless editable_object.edited?
 
-    content_tag :small, class: 'edited-text' do
+    content_tag :div, class: 'gl-mt-4 gl-text-gray-500 gl-font-sm' do
       timeago = time_ago_with_tooltip(editable_object.last_edited_at, placement: placement, html_class: html_class)
 
       if !exclude_author && editable_object.last_edited_by
-        author_link = link_to_member(editable_object.project, editable_object.last_edited_by, avatar: false, extra_class: 'gl-hover-text-decoration-underline', author_class: nil)
+        author_link = link_to_member(editable_object.project, editable_object.last_edited_by, avatar: false, extra_class: 'gl-hover-text-decoration-underline gl-text-gray-700', author_class: nil)
         output = safe_format(_("Edited %{timeago} by %{author}"), timeago: timeago, author: author_link)
       else
         output = safe_format(_("Edited %{timeago}"), timeago: timeago)
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index 224930e28df6ab..0a2091ace6845f 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -3,52 +3,9 @@
   .timeline-entry-inner
     .timeline-content
       .discussion.js-toggle-container{ data: { discussion_id: discussion.id, is_expanded: expanded.to_s } }
-        .discussion-header.gl-display-flex.gl--flex-center
-          .timeline-icon.gl-flex-shrink-0
-            = link_to user_path(discussion.author) do
-              = render Pajamas::AvatarComponent.new(discussion.author, size: 32, class: 'gl-mr-3')
-          = link_to_member(@project, discussion.author, avatar: false)
-
-          .inline.discussion-headline-light.gl-mx-3
-            = discussion.author.to_reference
-            started a thread
-
-            - url = discussion_path(discussion)
-            - if discussion.for_commit? && @noteable != discussion.noteable
-              on
-              - commit = discussion.noteable
-              - if commit
-                commit
-                = link_to commit.short_id, url, class: 'commit-sha'
-              - else
-                a deleted commit
-            - elsif discussion.diff_discussion?
-              on
-              = conditional_link_to url.present?, url do
-                - if discussion.on_merge_request_commit?
-                  - unless discussion.active?
-                    an outdated change in
-                  commit
-
-                  %span.commit-sha= truncate_sha(discussion.commit_id)
-                - else
-                  - unless discussion.active?
-                    an old version of
-                  the diff
-
-            = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
-          = render "discussions/headline", discussion: discussion
-
-          .discussion-actions.gl-ml-auto
-            %button.note-action-button.discussion-toggle-button.js-toggle-button{ type: "button", class: ("js-toggle-lazy-diff" unless expanded) }
-              = sprite_icon('chevron-up', css_class: "js-sidebar-collapse #{'hidden' unless expanded}")
-              = sprite_icon('chevron-down', css_class: "js-sidebar-expand #{'hidden' if expanded}")
-              %span.js-sidebar-collapse{ class: "#{'hidden' unless expanded}" }= _('Hide thread')
-              %span.js-sidebar-expand{ class: "#{'hidden' if expanded}" }= _('Show thread')
-
         .discussion-body.js-toggle-content{ class: ("hide" unless expanded) }
           - if discussion.diff_discussion? && discussion.diff_file
             = render "discussions/diff_with_notes", discussion: discussion
           - else
-            .card
+            .card.discussion-wrapper
               = render partial: "discussions/notes", locals: { discussion: discussion, disable_collapse_class: true }
diff --git a/app/views/discussions/_headline.html.haml b/app/views/discussions/_headline.html.haml
deleted file mode 100644
index b865eb815f0ec7..00000000000000
--- a/app/views/discussions/_headline.html.haml
+++ /dev/null
@@ -1,16 +0,0 @@
-- if discussion.resolved?
-  .discussion-headline-light.js-discussion-headline
-    = discussion_resolved_intro(discussion)
-    - if discussion.resolved_by
-      by
-      = link_to_member(@project, discussion.resolved_by, avatar: false)
-    - if discussion.resolved_by_push?
-      with a push
-    = time_ago_with_tooltip(discussion.resolved_at, placement: "bottom")
-- elsif discussion.updated?
-  .discussion-headline-light.js-discussion-headline
-    Last updated
-    - if discussion.last_updated_by
-      by
-      = link_to_member(@project, discussion.last_updated_by, avatar: false)
-    = time_ago_with_tooltip(discussion.last_updated_at, placement: "bottom")
diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml
index 3e72a66561bdae..c9dbdc0b387732 100644
--- a/app/views/shared/notes/_note.html.haml
+++ b/app/views/shared/notes/_note.html.haml
@@ -51,30 +51,31 @@
               = render 'snippets/notes/actions', note: note, note_editable: note_editable
             - else
               = render 'projects/notes/actions', note: note, note_editable: note_editable
-      .note-body{ class: note_editable ? 'js-task-list-container' : '' }
-        .note-text.md{ data: { testid: 'note-content' } }
-          = markdown_field(note, :note)
-        = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago')
-        .original-note-content.hidden{ data: { post_url: note_url(note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } }
-          #{note.note}
-        - if note_editable
-          = render 'shared/notes/edit', note: note
-        .note-awards
-          = render 'award_emoji/awards_block', awardable: note, inline: false
-        - if note.system
-          .system-note-commit-list-toggler.hide
-            = _("Toggle commit list")
-            = sprite_icon('chevron-down', css_class: 'js-chevron-down gl-ml-1 gl-vertical-align-text-bottom')
-            = sprite_icon('chevron-up', css_class: 'js-chevron-up gl-ml-1 gl-vertical-align-text-bottom gl-display-none')
-      - if note.attachment.url
-        .note-attachment
-          - if note.attachment.image?
-            = link_to note.attachment.url, target: '_blank' do
-              = image_tag note.attachment.url, class: 'note-image-attach col-lg-4'
-          .attachment
-            = link_to note.attachment.url, target: '_blank' do
-              = sprite_icon('paperclip')
-              = note.attachment_identifier
-              = link_to delete_attachment_project_note_path(note.project, note),
-                title: _('Delete this attachment'), method: :delete, remote: true, data: { confirm: _('Are you sure you want to remove the attachment?') }, class: 'danger js-note-attachment-delete' do
-                = sprite_icon('remove', css_class: 'cred')
+      .timeline-discussion-body
+        .note-body{ class: note_editable ? 'js-task-list-container' : '' }
+          .note-text.md{ data: { testid: 'note-content' } }
+            = markdown_field(note, :note)
+          = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago')
+          .original-note-content.hidden{ data: { post_url: note_url(note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } }
+            #{note.note}
+          - if note_editable
+            = render 'shared/notes/edit', note: note
+          .note-awards
+            = render 'award_emoji/awards_block', awardable: note, inline: false
+          - if note.system
+            .system-note-commit-list-toggler.hide
+              = _("Toggle commit list")
+              = sprite_icon('chevron-down', css_class: 'js-chevron-down gl-ml-1 gl-vertical-align-text-bottom')
+              = sprite_icon('chevron-up', css_class: 'js-chevron-up gl-ml-1 gl-vertical-align-text-bottom gl-display-none')
+        - if note.attachment.url
+          .note-attachment
+            - if note.attachment.image?
+              = link_to note.attachment.url, target: '_blank' do
+                = image_tag note.attachment.url, class: 'note-image-attach col-lg-4'
+            .attachment
+              = link_to note.attachment.url, target: '_blank' do
+                = sprite_icon('paperclip')
+                = note.attachment_identifier
+                = link_to delete_attachment_project_note_path(note.project, note),
+                  title: _('Delete this attachment'), method: :delete, remote: true, data: { confirm: _('Are you sure you want to remove the attachment?') }, class: 'danger js-note-attachment-delete' do
+                  = sprite_icon('remove', css_class: 'cred')
-- 
GitLab


From 41d20d0b2a1e3e296f69528c3c15402160a81c1b Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Thu, 28 Mar 2024 15:50:00 -0600
Subject: [PATCH 2/5] Remove unnecessary classes and css; fix badges

---
 app/assets/stylesheets/pages/notes.scss       | 11 ----------
 app/views/projects/notes/_actions.html.haml   | 20 +++++++++----------
 .../notes/_more_actions_dropdown.html.haml    |  2 +-
 app/views/shared/notes/_note.html.haml        |  4 ++--
 app/views/snippets/notes/_actions.html.haml   | 11 +++++-----
 5 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index b4c868861e7ad8..be4c2ed46b5445 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -898,17 +898,6 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio;
   min-width: 180px;
 }
 
-.note-actions-item {
-  margin-left: 12px;
-  display: flex;
-  align-items: center;
-
-  &.more-actions {
-    // compensate for narrow icon
-    margin-left: 10px;
-  }
-}
-
 .discussion-toggle-button {
   padding: 0 $gl-padding-8 0 0;
   background-color: transparent;
diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml
index e4ab1bafdf2dbc..55574ba4763ea7 100644
--- a/app/views/projects/notes/_actions.html.haml
+++ b/app/views/projects/notes/_actions.html.haml
@@ -1,24 +1,24 @@
 - access = note_human_max_access(note)
 - if note.noteable_author?(@noteable)
-  = render Pajamas::BadgeComponent.new(_("Author"), variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! note-role user-access-role has-tooltip gl-display-none gl-md-display-inline-block', title: _("This user is the author of this %{noteable}.") % { noteable: @noteable.human_class_name })
+  = render Pajamas::BadgeComponent.new(_("Author"), variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user is the author of this %{noteable}.") % { noteable: @noteable.human_class_name })
 - if access
-  = render Pajamas::BadgeComponent.new(access, variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! note-role user-access-role has-tooltip', title: _("This user has the %{access} role in the %{name} project.") % { access: access.downcase, name: note.project_name })
+  = render Pajamas::BadgeComponent.new(access, variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user has the %{access} role in the %{name} project.") % { access: access.downcase, name: note.project_name })
 - elsif note.contributor?
-  = render Pajamas::BadgeComponent.new(_("Contributor"), variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! note-role user-access-role has-tooltip', title: _("This user has previously committed to the %{name} project.") % { name: note.project_name })
+  = render Pajamas::BadgeComponent.new(_("Contributor"), variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user has previously committed to the %{name} project.") % { name: note.project_name })
 
 - if can?(current_user, :award_emoji, note)
   - if note.emoji_awardable?
-    .note-actions-item
-      = render Pajamas::ButtonComponent.new(category: :tertiary,
-        button_options: { title: _('Add reaction'), class: 'btn-icon add-reaction-button note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip', data: { position: 'right', container: 'body' }, 'aria-label': _('Add reaction') }) do
-        = sprite_icon('slight-smile', css_class: 'reaction-control-icon-neutral award-control-icon-neutral gl-button-icon gl-icon')
-        = sprite_icon('smiley', css_class: 'reaction-control-icon-positive award-control-icon-positive gl-button-icon gl-icon !gl-left-3')
-        = sprite_icon('smile', css_class: 'reaction-control-icon-super-positive award-control-icon-super-positive gl-button-icon gl-icon !gl-left-3 ')
+    = render Pajamas::ButtonComponent.new(category: :tertiary,
+      button_options: { title: _('Add reaction'), class: 'btn-icon add-reaction-button note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip', data: { position: 'right', container: 'body' }, 'aria-label': _('Add reaction') }) do
+      = sprite_icon('slight-smile', css_class: 'reaction-control-icon-neutral award-control-icon-neutral gl-button-icon gl-icon')
+      = sprite_icon('smiley', css_class: 'reaction-control-icon-positive award-control-icon-positive gl-button-icon gl-icon !gl-left-3')
+      = sprite_icon('smile', css_class: 'reaction-control-icon-super-positive award-control-icon-super-positive gl-button-icon gl-icon !gl-left-3 ')
 
   - if note_editable
-    .note-actions-item.gl-ml-0
+    .gl-ml-0
       = render Pajamas::ButtonComponent.new(category: :tertiary,
         icon: 'pencil',
         button_options: { class: 'note-action-button js-note-edit has-tooltip', data: { container: 'body', testid: 'edit-comment-button' }, title: _('Edit comment'), 'aria-label': _('Edit comment') })
 
   = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable
+
diff --git a/app/views/projects/notes/_more_actions_dropdown.html.haml b/app/views/projects/notes/_more_actions_dropdown.html.haml
index 6502907ad8d3b2..376517d907a883 100644
--- a/app/views/projects/notes/_more_actions_dropdown.html.haml
+++ b/app/views/projects/notes/_more_actions_dropdown.html.haml
@@ -1,7 +1,7 @@
 - is_current_user = current_user == note.author
 
 - if note_editable || !is_current_user
-  %div{ class: "dropdown more-actions note-actions-item gl-ml-0!" }
+  %div{ class: "dropdown more-actions gl-ml-0!" }
     = render Pajamas::ButtonComponent.new(icon: 'ellipsis_v', category: :tertiary, button_options: { class: 'note-action-button more-actions-toggle has-tooltip', data: { title: 'More actions', toggle: 'dropdown', container: 'body', testid: 'more-actions-dropdown' }})
     %ul.dropdown-menu.more-actions-dropdown.dropdown-open-left
       %li
diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml
index c9dbdc0b387732..8caf1c4a58f6ed 100644
--- a/app/views/shared/notes/_note.html.haml
+++ b/app/views/shared/notes/_note.html.haml
@@ -35,9 +35,9 @@
             %span.note-header-author-name.bold
               = note.author.name
             = user_status(note.author)
-            %spannote-headline-light{ data: { testid: 'note-author-content' } }
+            %span.note-headline-light{ data: { testid: 'note-author-content' } }
               = note.author.to_reference
-          %span.note-headline-ligh.note-headline-meta
+          %span.note-headline-light.note-headline-meta
             - if note.system
               %span.system-note-message
                 = markdown_field(note, :note)
diff --git a/app/views/snippets/notes/_actions.html.haml b/app/views/snippets/notes/_actions.html.haml
index 7a317c7af12721..284d0cac89758d 100644
--- a/app/views/snippets/notes/_actions.html.haml
+++ b/app/views/snippets/notes/_actions.html.haml
@@ -1,13 +1,12 @@
 - if current_user
   - if note.emoji_awardable?
-    .note-actions-item
-      = render Pajamas::ButtonComponent.new(category: :tertiary, button_options: { title: _('Add reaction'), class: 'btn-icon add-reaction-button note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip' }) do
-        = sprite_icon('slight-smile', css_class: 'reaction-control-icon-neutral award-control-icon-neutral gl-button-icon gl-icon')
-        = sprite_icon('smiley', css_class: 'reaction-control-icon-positive award-control-icon-positive gl-button-icon gl-icon !gl-left-3')
-        = sprite_icon('smile', css_class: 'reaction-control-icon-super-positive award-control-icon-super-positive gl-button-icon gl-icon !gl-left-3 ')
+    = render Pajamas::ButtonComponent.new(category: :tertiary, button_options: { title: _('Add reaction'), class: 'btn-icon add-reaction-button note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip' }) do
+      = sprite_icon('slight-smile', css_class: 'reaction-control-icon-neutral award-control-icon-neutral gl-button-icon gl-icon')
+      = sprite_icon('smiley', css_class: 'reaction-control-icon-positive award-control-icon-positive gl-button-icon gl-icon !gl-left-3')
+      = sprite_icon('smile', css_class: 'reaction-control-icon-super-positive award-control-icon-super-positive gl-button-icon gl-icon !gl-left-3 ')
 
   - if note_editable
-    .note-actions-item.gl-ml-0
+    .gl-ml-0
       = render Pajamas::ButtonComponent.new(category: :tertiary, icon: 'pencil', button_options: { title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip', data: { container: 'body', testid: 'edit-comment-button' } })
 
   = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable
-- 
GitLab


From 3576cfd8d6a896ab6c1629e293630c80dc544fad Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Thu, 28 Mar 2024 16:21:24 -0600
Subject: [PATCH 3/5] Fix mobile styles

---
 app/views/projects/notes/_actions.html.haml               | 2 +-
 app/views/projects/notes/_more_actions_dropdown.html.haml | 5 ++++-
 app/views/shared/notes/_note.html.haml                    | 6 +++---
 app/views/snippets/notes/_actions.html.haml               | 2 +-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml
index 55574ba4763ea7..735d067a85fb3a 100644
--- a/app/views/projects/notes/_actions.html.haml
+++ b/app/views/projects/notes/_actions.html.haml
@@ -18,7 +18,7 @@
     .gl-ml-0
       = render Pajamas::ButtonComponent.new(category: :tertiary,
         icon: 'pencil',
-        button_options: { class: 'note-action-button js-note-edit has-tooltip', data: { container: 'body', testid: 'edit-comment-button' }, title: _('Edit comment'), 'aria-label': _('Edit comment') })
+        button_options: { class: 'gl-display-none gl-sm-display-inline-block note-action-button js-note-edit has-tooltip', data: { container: 'body', testid: 'edit-comment-button' }, title: _('Edit comment'), 'aria-label': _('Edit comment') })
 
   = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable
 
diff --git a/app/views/projects/notes/_more_actions_dropdown.html.haml b/app/views/projects/notes/_more_actions_dropdown.html.haml
index 376517d907a883..fa0eb9455cf95e 100644
--- a/app/views/projects/notes/_more_actions_dropdown.html.haml
+++ b/app/views/projects/notes/_more_actions_dropdown.html.haml
@@ -2,8 +2,11 @@
 
 - if note_editable || !is_current_user
   %div{ class: "dropdown more-actions gl-ml-0!" }
-    = render Pajamas::ButtonComponent.new(icon: 'ellipsis_v', category: :tertiary, button_options: { class: 'note-action-button more-actions-toggle has-tooltip', data: { title: 'More actions', toggle: 'dropdown', container: 'body', testid: 'more-actions-dropdown' }})
+    = render Pajamas::ButtonComponent.new(icon: 'ellipsis_v', category: :tertiary, button_options: { class: 'note-action-button more-actions-toggle', data: { title: 'More actions', toggle: 'dropdown', container: 'body', testid: 'more-actions-dropdown' }})
     %ul.dropdown-menu.more-actions-dropdown.dropdown-open-left
+      %li{ class: "gl-sm-display-none!" }
+        = render Pajamas::ButtonComponent.new(category: :tertiary, button_options: { class: 'menu-item note-action-button js-note-edit', data: { container: 'body', testid: 'edit-comment-button' } }) do
+          = _("Edit comment")
       %li
         = clipboard_button(text: noteable_note_url(note), title: _('Copy reference'), button_text: _('Copy link'), class: 'gl-rounded-0!', size: :medium, hide_tooltip: true, hide_button_icon: true)
       - unless is_current_user
diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml
index 8caf1c4a58f6ed..6fbe6ced0ae71f 100644
--- a/app/views/shared/notes/_note.html.haml
+++ b/app/views/shared/notes/_note.html.haml
@@ -35,14 +35,14 @@
             %span.note-header-author-name.bold
               = note.author.name
             = user_status(note.author)
-            %span.note-headline-light{ data: { testid: 'note-author-content' } }
-              = note.author.to_reference
+          %span.note-headline-light{ data: { testid: 'note-author-content' } }
+            = note.author.to_reference
           %span.note-headline-light.note-headline-meta
             - if note.system
               %span.system-note-message
                 = markdown_field(note, :note)
             - if note.created_at
-              %span.system-note-separator
+              %span.system-note-separator.d-none.d-sm-inline
                 &middot;
               %a.system-note-separator{ href: "##{dom_id(note)}" }= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
         - unless note.system?
diff --git a/app/views/snippets/notes/_actions.html.haml b/app/views/snippets/notes/_actions.html.haml
index 284d0cac89758d..29501219487c22 100644
--- a/app/views/snippets/notes/_actions.html.haml
+++ b/app/views/snippets/notes/_actions.html.haml
@@ -7,6 +7,6 @@
 
   - if note_editable
     .gl-ml-0
-      = render Pajamas::ButtonComponent.new(category: :tertiary, icon: 'pencil', button_options: { title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip', data: { container: 'body', testid: 'edit-comment-button' } })
+      = render Pajamas::ButtonComponent.new(category: :tertiary, icon: 'pencil', button_options: { title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip gl-display-none gl-sm-display-block', data: { container: 'body', testid: 'edit-comment-button' } })
 
   = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable
-- 
GitLab


From 763b621756e9603467f65848ec203c345fd7293f Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Fri, 29 Mar 2024 10:50:23 -0600
Subject: [PATCH 4/5] Fix specs; remove potentially unneeded stuff

---
 .../projects/discussions_controller.rb        |  18 +--
 app/controllers/projects/notes_controller.rb  |  33 -----
 app/helpers/application_helper.rb             |   2 +-
 app/views/projects/notes/_actions.html.haml   |   8 +-
 .../notes/_more_actions_dropdown.html.haml    |   9 +-
 app/views/shared/notes/_note.html.haml        |   4 +-
 .../projects/notes_controller_spec.rb         | 119 ------------------
 .../_more_actions_dropdown.html.haml_spec.rb  |  16 +++
 8 files changed, 32 insertions(+), 177 deletions(-)

diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb
index 34b283b87f5df6..97022a944b40b5 100644
--- a/app/controllers/projects/discussions_controller.rb
+++ b/app/controllers/projects/discussions_controller.rb
@@ -33,12 +33,10 @@ def show
   private
 
   def render_discussion
-    if serialize_notes?
-      prepare_notes_for_rendering(discussion.notes)
-      render_json_with_discussions_serializer
-    else
-      render_json_with_html
-    end
+    return unless serialize_notes?
+
+    prepare_notes_for_rendering(discussion.notes)
+    render_json_with_discussions_serializer
   end
 
   def render_json_with_discussions_serializer
@@ -47,14 +45,6 @@ def render_json_with_discussions_serializer
       .represent(discussion, context: self, render_truncated_diff_lines: true)
   end
 
-  # Legacy method used to render discussions notes when not using Vue on views.
-  def render_json_with_html
-    render json: {
-      resolved_by: discussion.resolved_by.try(:name),
-      discussion_headline_html: view_to_html_string('discussions/_headline', discussion: discussion)
-    }
-  end
-
   # rubocop: disable CodeReuse/ActiveRecord
   def noteable
     @noteable ||= noteable_finder_class.new(current_user, project_id: @project.id).find_by!(iid: params[:noteable_id])
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 3d8a787afcb446..d4dca3649f2fbf 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -44,39 +44,6 @@ def delete_attachment
     end
   end
 
-  def resolve
-    return render_404 unless note.resolvable?
-
-    Notes::ResolveService.new(project, current_user).execute(note)
-
-    discussion = note.discussion
-
-    if serialize_notes?
-      render_json_with_notes_serializer
-    else
-      render json: {
-        resolved_by: note.resolved_by.try(:name),
-        discussion_headline_html: (view_to_html_string('discussions/_headline', discussion: discussion) if discussion)
-      }
-    end
-  end
-
-  def unresolve
-    return render_404 unless note.resolvable?
-
-    note.unresolve!
-
-    discussion = note.discussion
-
-    if serialize_notes?
-      render_json_with_notes_serializer
-    else
-      render json: {
-        discussion_headline_html: (view_to_html_string('discussions/_headline', discussion: discussion) if discussion)
-      }
-    end
-  end
-
   def outdated_line_change
     diff_lines = Rails.cache.fetch(['note', note.id, 'oudated_line_change'], expires_in: 7.days) do
       Gitlab::Json.dump(::MergeRequests::OutdatedDiscussionDiffLinesService.new(project: note.noteable.source_project, note: note).execute)
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 25824c364c4e85..061acb42fb7f29 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -207,7 +207,7 @@ def time_ago_with_tooltip(time, placement: 'top', html_class: '', short_format:
   def edited_time_ago_with_tooltip(editable_object, placement: 'top', html_class: 'time_ago', exclude_author: false)
     return unless editable_object.edited?
 
-    content_tag :div, class: 'gl-mt-4 gl-text-gray-500 gl-font-sm' do
+    content_tag :div, class: 'edited-text gl-mt-4 gl-text-gray-500 gl-font-sm' do
       timeago = time_ago_with_tooltip(editable_object.last_edited_at, placement: placement, html_class: html_class)
 
       if !exclude_author && editable_object.last_edited_by
diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml
index 735d067a85fb3a..e3ab9b6139ab90 100644
--- a/app/views/projects/notes/_actions.html.haml
+++ b/app/views/projects/notes/_actions.html.haml
@@ -1,10 +1,10 @@
 - access = note_human_max_access(note)
 - if note.noteable_author?(@noteable)
-  = render Pajamas::BadgeComponent.new(_("Author"), variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user is the author of this %{noteable}.") % { noteable: @noteable.human_class_name })
+  = render Pajamas::BadgeComponent.new(_("Author"), variant: 'neutral', class: 'has-tooltip gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user is the author of this %{noteable}.") % { noteable: @noteable.human_class_name })
 - if access
-  = render Pajamas::BadgeComponent.new(access, variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user has the %{access} role in the %{name} project.") % { access: access.downcase, name: note.project_name })
+  = render Pajamas::BadgeComponent.new(access, variant: 'neutral', class: 'has-tooltip gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user has the %{access} role in the %{name} project.") % { access: access.downcase, name: note.project_name })
 - elsif note.contributor?
-  = render Pajamas::BadgeComponent.new(_("Contributor"), variant: 'neutral', class: 'gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user has previously committed to the %{name} project.") % { name: note.project_name })
+  = render Pajamas::BadgeComponent.new(_("Contributor"), variant: 'neutral', class: 'has-tooltip gl-bg-transparent! gl-inset-border-1-gray-100! gl-display-none gl-md-display-inline-block gl-mr-3', title: _("This user has previously committed to the %{name} project.") % { name: note.project_name })
 
 - if can?(current_user, :award_emoji, note)
   - if note.emoji_awardable?
@@ -18,7 +18,7 @@
     .gl-ml-0
       = render Pajamas::ButtonComponent.new(category: :tertiary,
         icon: 'pencil',
-        button_options: { class: 'gl-display-none gl-sm-display-inline-block note-action-button js-note-edit has-tooltip', data: { container: 'body', testid: 'edit-comment-button' }, title: _('Edit comment'), 'aria-label': _('Edit comment') })
+        button_options: { class: 'gl-display-none gl-sm-display-inline-block note-action-button js-note-edit has-tooltip', data: { testid: 'edit-comment-button' }, title: _('Edit comment'), 'aria-label': _('Edit comment') })
 
   = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable
 
diff --git a/app/views/projects/notes/_more_actions_dropdown.html.haml b/app/views/projects/notes/_more_actions_dropdown.html.haml
index fa0eb9455cf95e..6ab384d6d07856 100644
--- a/app/views/projects/notes/_more_actions_dropdown.html.haml
+++ b/app/views/projects/notes/_more_actions_dropdown.html.haml
@@ -1,12 +1,13 @@
 - is_current_user = current_user == note.author
 
 - if note_editable || !is_current_user
-  %div{ class: "dropdown more-actions gl-ml-0!" }
+  .dropdown{ class: "more-actions gl-ml-0!" }
     = render Pajamas::ButtonComponent.new(icon: 'ellipsis_v', category: :tertiary, button_options: { class: 'note-action-button more-actions-toggle', data: { title: 'More actions', toggle: 'dropdown', container: 'body', testid: 'more-actions-dropdown' }})
     %ul.dropdown-menu.more-actions-dropdown.dropdown-open-left
-      %li{ class: "gl-sm-display-none!" }
-        = render Pajamas::ButtonComponent.new(category: :tertiary, button_options: { class: 'menu-item note-action-button js-note-edit', data: { container: 'body', testid: 'edit-comment-button' } }) do
-          = _("Edit comment")
+      - if note_editable
+        %li{ class: "gl-sm-display-none!" }
+          = render Pajamas::ButtonComponent.new(category: :tertiary, button_options: { class: 'menu-item note-action-button js-note-edit', data: { container: 'body', testid: 'edit-comment-button' } }) do
+            = _("Edit comment")
       %li
         = clipboard_button(text: noteable_note_url(note), title: _('Copy reference'), button_text: _('Copy link'), class: 'gl-rounded-0!', size: :medium, hide_tooltip: true, hide_button_icon: true)
       - unless is_current_user
diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml
index 6fbe6ced0ae71f..ec304c58fcea44 100644
--- a/app/views/shared/notes/_note.html.haml
+++ b/app/views/shared/notes/_note.html.haml
@@ -32,7 +32,7 @@
       .note-header
         .note-header-info
           %a{ href: user_path(note.author) }
-            %span.note-header-author-name.bold
+            %span.note-header-author-name.gl-font-weight-bold
               = note.author.name
             = user_status(note.author)
           %span.note-headline-light{ data: { testid: 'note-author-content' } }
@@ -42,7 +42,7 @@
               %span.system-note-message
                 = markdown_field(note, :note)
             - if note.created_at
-              %span.system-note-separator.d-none.d-sm-inline
+              %span.system-note-separator.gl-display-none.gl-sm-display-inline
                 &middot;
               %a.system-note-separator{ href: "##{dom_id(note)}" }= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
         - unless note.system?
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 6b440b90f3771d..f9efea652daa7a 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -1040,125 +1040,6 @@ def request
     end
   end
 
-  describe "resolving and unresolving" do
-    let(:project) { create(:project, :repository) }
-    let(:merge_request) { create(:merge_request, source_project: project) }
-    let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) }
-
-    describe 'POST resolve' do
-      before do
-        sign_in user
-      end
-
-      specify { expect(post(:resolve, params: request_params)).to have_request_urgency(:low) }
-
-      context "when the user is not authorized to resolve the note" do
-        it "returns status 404" do
-          post :resolve, params: request_params
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-
-      context "when the user is authorized to resolve the note" do
-        before do
-          project.add_developer(user)
-        end
-
-        context "when the note is not resolvable" do
-          before do
-            note.update!(system: true)
-          end
-
-          it "returns status 404" do
-            post :resolve, params: request_params
-
-            expect(response).to have_gitlab_http_status(:not_found)
-          end
-        end
-
-        context "when the note is resolvable" do
-          it "resolves the note" do
-            post :resolve, params: request_params
-
-            expect(note.reload.resolved?).to be true
-            expect(note.reload.resolved_by).to eq(user)
-          end
-
-          it "sends notifications if all discussions are resolved" do
-            expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance|
-              expect(instance).to receive(:execute).with(merge_request)
-            end
-
-            post :resolve, params: request_params
-          end
-
-          it "returns the name of the resolving user" do
-            post :resolve, params: request_params.merge(html: true)
-
-            expect(json_response["resolved_by"]).to eq(user.name)
-          end
-
-          it "returns status 200" do
-            post :resolve, params: request_params
-
-            expect(response).to have_gitlab_http_status(:ok)
-          end
-        end
-      end
-    end
-
-    describe 'DELETE unresolve' do
-      before do
-        sign_in user
-
-        note.resolve!(user)
-      end
-
-      specify { expect(delete(:unresolve, params: request_params)).to have_request_urgency(:low) }
-
-      context "when the user is not authorized to resolve the note" do
-        it "returns status 404" do
-          delete :unresolve, params: request_params
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-
-      context "when the user is authorized to resolve the note" do
-        before do
-          project.add_developer(user)
-        end
-
-        context "when the note is not resolvable" do
-          before do
-            note.update!(system: true)
-          end
-
-          it "returns status 404" do
-            delete :unresolve, params: request_params
-
-            expect(response).to have_gitlab_http_status(:not_found)
-          end
-        end
-
-        context "when the note is resolvable" do
-          it "unresolves the note" do
-            delete :unresolve, params: request_params
-
-            expect(note.reload.resolved?).to be false
-          end
-
-          it "returns status 200" do
-            delete :unresolve, params: request_params
-
-            expect(response).to have_gitlab_http_status(:ok)
-          end
-        end
-      end
-    end
-  end
-
   describe 'GET outdated_line_change' do
     let(:request_params) do
       {
diff --git a/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb b/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb
index d604eb2c8d6fe8..3202f99e14697e 100644
--- a/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb
+++ b/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb
@@ -39,4 +39,20 @@
 
     expect(rendered).to have_link('Delete comment')
   end
+
+  it 'shows Edit button if editable and current users comment' do
+    render 'projects/notes/more_actions_dropdown', current_user: author_user, note_editable: true, note: note
+
+    expect(rendered).to have_selector('.js-note-edit')
+
+    expect(rendered).to have_button('Edit comment')
+  end
+
+  it 'does not show Edit button if not editable and not current users comment' do
+    render 'projects/notes/more_actions_dropdown', current_user: not_author_user, note_editable: false, note: note
+
+    expect(rendered).not_to have_selector('.js-note-edit')
+
+    expect(rendered).not_to have_button('Edit comment')
+  end
 end
-- 
GitLab


From 2d6bb916ae77d1f2458046ab1489af6e42200664 Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Thu, 11 Apr 2024 15:44:59 -0600
Subject: [PATCH 5/5] Remove more unused code

---
 .../projects/discussions_controller.rb        |  2 --
 app/controllers/projects/notes_controller.rb  | 33 +++++++++++++++++++
 app/helpers/notes_helper.rb                   |  4 ---
 spec/helpers/notes_helper_spec.rb             | 18 ----------
 4 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb
index 97022a944b40b5..671d19084b39e2 100644
--- a/app/controllers/projects/discussions_controller.rb
+++ b/app/controllers/projects/discussions_controller.rb
@@ -33,8 +33,6 @@ def show
   private
 
   def render_discussion
-    return unless serialize_notes?
-
     prepare_notes_for_rendering(discussion.notes)
     render_json_with_discussions_serializer
   end
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index d4dca3649f2fbf..3d8a787afcb446 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -44,6 +44,39 @@ def delete_attachment
     end
   end
 
+  def resolve
+    return render_404 unless note.resolvable?
+
+    Notes::ResolveService.new(project, current_user).execute(note)
+
+    discussion = note.discussion
+
+    if serialize_notes?
+      render_json_with_notes_serializer
+    else
+      render json: {
+        resolved_by: note.resolved_by.try(:name),
+        discussion_headline_html: (view_to_html_string('discussions/_headline', discussion: discussion) if discussion)
+      }
+    end
+  end
+
+  def unresolve
+    return render_404 unless note.resolvable?
+
+    note.unresolve!
+
+    discussion = note.discussion
+
+    if serialize_notes?
+      render_json_with_notes_serializer
+    else
+      render json: {
+        discussion_headline_html: (view_to_html_string('discussions/_headline', discussion: discussion) if discussion)
+      }
+    end
+  end
+
   def outdated_line_change
     diff_lines = Rails.cache.fetch(['note', note.id, 'oudated_line_change'], expires_in: 7.days) do
       Gitlab::Json.dump(::MergeRequests::OutdatedDiscussionDiffLinesService.new(project: note.noteable.source_project, note: note).execute)
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 1a8e729de3863f..9ac3d4efe17a42 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -197,10 +197,6 @@ def notes_data(issuable)
     data
   end
 
-  def discussion_resolved_intro(discussion)
-    discussion.resolved_by_push? ? 'Automatically resolved' : 'Resolved'
-  end
-
   def rendered_for_merge_request?
     params[:from_merge_request].present?
   end
diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb
index 58d39caa90ccd5..18e4dfc3cf2f21 100644
--- a/spec/helpers/notes_helper_spec.rb
+++ b/spec/helpers/notes_helper_spec.rb
@@ -295,24 +295,6 @@
     end
   end
 
-  describe '#discussion_resolved_intro' do
-    context 'when the discussion was resolved by a push' do
-      let(:discussion) { double(:discussion, resolved_by_push?: true) }
-
-      it 'returns "Automatically resolved"' do
-        expect(discussion_resolved_intro(discussion)).to eq('Automatically resolved')
-      end
-    end
-
-    context 'when the discussion was not resolved by a push' do
-      let(:discussion) { double(:discussion, resolved_by_push?: false) }
-
-      it 'returns "Resolved"' do
-        expect(discussion_resolved_intro(discussion)).to eq('Resolved')
-      end
-    end
-  end
-
   describe '#notes_data' do
     let_it_be(:issue) { create(:issue, project: project) }
 
-- 
GitLab