Merge branch '60540-merge-request-popover-is-not-working-on-the-to-do-page' into 'master'

Resolve "Merge Request Popover is not working on the To Do page"

Closes #60540

See merge request gitlab-org/gitlab-ce!27382

(cherry picked from commit 6015b521)

5e38e42b Make sure to allow project information data tags on GFM
18885d50 Do not show MR Popover if all necessary data is not present

app/assets/javascripts/mr_popover/index.js

@@ -22,13 +22,10 @@ const handleUserPopoverMouseOut = ({ target }) => {
  * Adds a MergeRequestPopover component to the body, hands over as much data as the target element has in data attributes.
  * loads based on data-project-path and data-iid more data about an MR from the API and sets it on the popover
  */
-const handleMRPopoverMount = apolloProvider => ({ target }) => {
+const handleMRPopoverMount = ({ apolloProvider, projectPath, mrTitle, iid }) => ({ target }) => {
   // Add listener to actually remove it again
   target.addEventListener('mouseleave', handleUserPopoverMouseOut);
 
-  const { projectPath, mrTitle, iid } = target.dataset;
-  const mergeRequest = {};
-
   renderFn = setTimeout(() => {
     const MRPopoverComponent = Vue.extend(MRPopover);
     renderedPopover = new MRPopoverComponent({
@@ -36,7 +33,6 @@ const handleMRPopoverMount = apolloProvider => ({ target }) => {
         target,
         projectPath,
         mergeRequestIID: iid,
-        mergeRequest,
         mergeRequestTitle: mrTitle,
       },
       apolloProvider,
@@ -57,8 +53,13 @@ export default elements => {
     const listenerAddedAttr = 'data-mr-listener-added';
 
     mrLinks.forEach(el => {
-      if (!el.getAttribute(listenerAddedAttr)) {
-        el.addEventListener('mouseenter', handleMRPopoverMount(apolloProvider));
+      const { projectPath, mrTitle, iid } = el.dataset;
+
+      if (!el.getAttribute(listenerAddedAttr) && projectPath && mrTitle && iid) {
+        el.addEventListener(
+          'mouseenter',
+          handleMRPopoverMount({ apolloProvider, projectPath, mrTitle, iid }),
+        );
         el.setAttribute(listenerAddedAttr, true);
       }
     });

app/helpers/markup_helper.rb

@@ -83,7 +83,8 @@ def first_line_in_markdown(object, attribute, max_chars = nil, options = {})
     text = sanitize(
       text,
       tags: tags,
-      attributes: Rails::Html::WhiteListSanitizer.allowed_attributes + ['style', 'data-src', 'data-name', 'data-unicode-version']
+      attributes: Rails::Html::WhiteListSanitizer.allowed_attributes +
+          %w(style data-src data-name data-unicode-version data-iid data-project-path data-mr-title)
     )
 
     # since <img> tags are stripped, this can leave empty <a> tags hanging around

changelogs/unreleased/60540-merge-request-popover-is-not-working-on-the-to-do-page.yml

+---
+title: Fix MR popover on ToDos page
+merge_request: 27382
+author:
+type: fixed

spec/features/dashboard/todos/todos_spec.rb

@@ -17,6 +17,26 @@
     end
   end
 
+  context 'when the todo references a merge request' do
+    let(:referenced_mr) { create(:merge_request, source_project: project) }
+    let(:note) { create(:note, project: project, note: "Check out #{referenced_mr.to_reference}") }
+    let!(:todo) { create(:todo, :mentioned, user: user, project: project, author: author, note: note) }
+
+    before do
+      sign_in(user)
+      visit dashboard_todos_path
+    end
+
+    it 'renders the mr link with the extra attributes' do
+      link = page.find_link(referenced_mr.to_reference)
+
+      expect(link).not_to be_nil
+      expect(link['data-iid']).to eq(referenced_mr.iid.to_s)
+      expect(link['data-project-path']).to eq(referenced_mr.project.full_path)
+      expect(link['data-mr-title']).to eq(referenced_mr.title)
+    end
+  end
+
   context 'User has a todo', :js do
     before do
       create(:todo, :mentioned, user: user, project: project, target: issue, author: author)

spec/frontend/mr_popover/index_spec.js

@@ -7,18 +7,28 @@ createDefaultClient.default = jest.fn();
 describe('initMRPopovers', () => {
   let mr1;
   let mr2;
+  let mr3;
 
   beforeEach(() => {
     setHTMLFixture(`
-      <div id="one" class="gfm-merge_request">MR1</div>
-      <div id="two" class="gfm-merge_request">MR2</div>
+      <div id="one" class="gfm-merge_request" data-mr-title="title" data-iid="1" data-project-path="group/project">
+        MR1
+      </div>
+      <div id="two" class="gfm-merge_request" data-mr-title="title" data-iid="1" data-project-path="group/project">
+        MR2
+      </div>
+      <div id="three" class="gfm-merge_request">
+        MR3
+      </div>
     `);
 
     mr1 = document.querySelector('#one');
     mr2 = document.querySelector('#two');
+    mr3 = document.querySelector('#three');
 
     mr1.addEventListener = jest.fn();
     mr2.addEventListener = jest.fn();
+    mr3.addEventListener = jest.fn();
   });
 
   it('does not add the same event listener twice', () => {
@@ -27,4 +37,10 @@ describe('initMRPopovers', () => {
     expect(mr1.addEventListener).toHaveBeenCalledTimes(1);
     expect(mr2.addEventListener).toHaveBeenCalledTimes(1);
   });
+
+  it('does not add listener if it does not have the necessary data attributes', () => {
+    initMRPopovers([mr1, mr2, mr3]);
+
+    expect(mr3.addEventListener).not.toHaveBeenCalled();
+  });
 });