diff options
author | Phil Hughes <me@iamphill.com> | 2019-04-24 14:09:37 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-04-29 18:25:21 +0000 |
commit | 9a5567ffee0b92e104a51aa04d14a3d3644511d5 (patch) | |
tree | ac7113d17ee1824003e332d50bbbde70bbf7c91b | |
parent | f3e84e78b62450d3fc6beb9191596a22bf271a38 (diff) | |
download | gitlab-ce-9a5567ffee0b92e104a51aa04d14a3d3644511d5.tar.gz |
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 6015b521751ba1f272fbc7160cd138fe4411c8ee)
5e38e42b Make sure to allow project information data tags on GFM
18885d50 Do not show MR Popover if all necessary data is not present
-rw-r--r-- | app/assets/javascripts/mr_popover/index.js | 15 | ||||
-rw-r--r-- | app/helpers/markup_helper.rb | 3 | ||||
-rw-r--r-- | changelogs/unreleased/60540-merge-request-popover-is-not-working-on-the-to-do-page.yml | 5 | ||||
-rw-r--r-- | spec/features/dashboard/todos/todos_spec.rb | 20 | ||||
-rw-r--r-- | spec/frontend/mr_popover/index_spec.js | 20 |
5 files changed, 53 insertions, 10 deletions
diff --git a/app/assets/javascripts/mr_popover/index.js b/app/assets/javascripts/mr_popover/index.js index 9a97e98f9db..18c0e201300 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 d83c69603a9..e0aca2b49eb 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 <img> tags are stripped, this can leave empty <a> 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 00000000000..7b5fae016bb --- /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 fd8677feab5..d58e3b2841e 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 8c33e52a04b..b9db2342687 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(` - <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(); + }); }); |