diff --git a/app/assets/javascripts/mr_popover/index.js b/app/assets/javascripts/mr_popover/index.js
index 9a97e98f9db9019658280e4a39292e614a80323c..18c0e2013001815da85e76677065c7adbf520c86 100644
--- a/app/assets/javascripts/mr_popover/index.js
+++ b/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);
}
});
diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb
index d83c69603a91b13c0e27902ce64cb294734ba3e5..e0aca2b49eb6042ce5c3ce965411b416acf38e65 100644
--- a/app/helpers/markup_helper.rb
+++ b/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
tags are stripped, this can leave empty tags hanging around
diff --git a/changelogs/unreleased/60540-merge-request-popover-is-not-working-on-the-to-do-page.yml b/changelogs/unreleased/60540-merge-request-popover-is-not-working-on-the-to-do-page.yml
new file mode 100644
index 0000000000000000000000000000000000000000..7b5fae016bb98f73216d016367d00f512640d15e
--- /dev/null
+++ b/changelogs/unreleased/60540-merge-request-popover-is-not-working-on-the-to-do-page.yml
@@ -0,0 +1,5 @@
+---
+title: Fix MR popover on ToDos page
+merge_request: 27382
+author:
+type: fixed
diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb
index fd8677feab553f5a226d881b18f29405715025b0..d58e3b2841ec30bd7f89c8c589e26d3ae5825c62 100644
--- a/spec/features/dashboard/todos/todos_spec.rb
+++ b/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)
diff --git a/spec/frontend/mr_popover/index_spec.js b/spec/frontend/mr_popover/index_spec.js
index 8c33e52a04bdc079bff89cefabc76e0ed755e1b3..b9db2342687358540a82c5809d905a1170d844b4 100644
--- a/spec/frontend/mr_popover/index_spec.js
+++ b/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(`
- MR1
- MR2
+
+ MR1
+
+
+ MR2
+
+
+ MR3
+
`);
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();
+ });
});