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

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 @@ module MarkupHelper
     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 @@ describe 'Dashboard Todos' do
     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();
+  });
 });