From fd6b0f9b2480b63daae708fbf7c6ee840a97bfc3 Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Tue, 27 Nov 2018 10:21:58 -0500 Subject: [PATCH 1/3] Add vue component for confirmation popover Have the confirmation popover in a separate Vue component fixes a bug in IE that was causing the `form` element in the popover to be ignored. Since `form`s cannot be nested, the `form` element in the popover was being discarded by IE and the revoke/resend invitation buttons did nothing. Breaking the functionality into a Vue component moves the `form` into a separate template. When the popover is displayed, the component is added to the DOM at the end, so the `form` is properly not-nested. --- .../confirmation_popover.test.js.snap | 3 ++ .../__tests__/confirmation_popover.test.js | 22 +++++++++++++ js/components/confirmation_popover.js | 32 +++++++++++++++++++ js/index.js | 2 ++ templates/components/confirmation_button.html | 27 ++++++---------- templates/workspaces/members/edit.html | 2 -- 6 files changed, 68 insertions(+), 20 deletions(-) create mode 100644 js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap create mode 100644 js/components/__tests__/confirmation_popover.test.js create mode 100644 js/components/confirmation_popover.js diff --git a/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap b/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap new file mode 100644 index 00000000..232dfb2f --- /dev/null +++ b/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ConfirmationPopover matches snapshot 1`] = ` `; diff --git a/js/components/__tests__/confirmation_popover.test.js b/js/components/__tests__/confirmation_popover.test.js new file mode 100644 index 00000000..d82564e5 --- /dev/null +++ b/js/components/__tests__/confirmation_popover.test.js @@ -0,0 +1,22 @@ +import { shallowMount } from '@vue/test-utils' + +import ConfirmationPopover from '../confirmation_popover' + + +describe('ConfirmationPopover', () => { + it('matches snapshot', () => { + const wrapper = shallowMount(ConfirmationPopover, { + propsData: { + action: '/some-url', + btn_text: 'Do something dangerous', + cancel_btn_text: 'Cancel', + confirm_btn_text: 'Confirm', + confirm_msg: 'Are you sure you want to do that?', + csrf_token: '42' + } + }) + + expect(wrapper).toMatchSnapshot() + }) +}) + diff --git a/js/components/confirmation_popover.js b/js/components/confirmation_popover.js new file mode 100644 index 00000000..5bccc719 --- /dev/null +++ b/js/components/confirmation_popover.js @@ -0,0 +1,32 @@ +export default { + name: 'confirmation-popover', + + props: { + action: String, + btn_text: String, + cancel_btn_text: String, + confirm_btn_text: String, + confirm_msg: String, + csrf_token: String + }, + + template: ` + + + + + ` +} diff --git a/js/index.js b/js/index.js index 4edd4c08..2d5b9ff2 100644 --- a/js/index.js +++ b/js/index.js @@ -23,6 +23,7 @@ import CcpoApproval from './components/forms/ccpo_approval' import MembersList from './components/forms/members_list' import LocalDatetime from './components/local_datetime' import RequestsList from './components/forms/requests_list' +import ConfirmationPopover from './components/confirmation_popover' Vue.config.productionTip = false @@ -50,6 +51,7 @@ const app = new Vue({ EditEnvironmentRole, EditProjectRoles, RequestsList, + ConfirmationPopover, }, mounted: function() { diff --git a/templates/components/confirmation_button.html b/templates/components/confirmation_button.html index 71b15a3b..31db3355 100644 --- a/templates/components/confirmation_button.html +++ b/templates/components/confirmation_button.html @@ -1,19 +1,10 @@ -{% macro ConfirmationButton(btn_text, action, csrf_token, confirm_msg="Are you sure?", confirm_btn="Confirm", cancel_btn="Cancel") -%} - - - - +{% macro ConfirmationButton(btn_text, action, confirm_msg="Are you sure?", confirm_btn="Confirm", cancel_btn="Cancel") -%} + + {%- endmacro %} diff --git a/templates/workspaces/members/edit.html b/templates/workspaces/members/edit.html index 0c1e8f7a..0b609e5b 100644 --- a/templates/workspaces/members/edit.html +++ b/templates/workspaces/members/edit.html @@ -44,14 +44,12 @@ {{ ConfirmationButton( "Revoke Invitation", url_for("workspaces.revoke_invitation", workspace_id=workspace.id, token=member.latest_invitation.token), - form.csrf_token ) }} {% endif %} {% if member.can_resend_invitation %} {{ ConfirmationButton ( "Resend Invitation", url_for("workspaces.resend_invitation", workspace_id=workspace.id, token=member.latest_invitation.token), - form.csrf_token, confirm_msg="Are you sure? This will send an email to invite the user to join this workspace." )}} {% endif %} From 178892c62e66b6127f49b0698f33df99a085a7ec Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Tue, 27 Nov 2018 11:00:51 -0500 Subject: [PATCH 2/3] Use localVue to squash missing custom element error --- .../__snapshots__/confirmation_popover.test.js.snap | 2 +- js/components/__tests__/confirmation_popover.test.js | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap b/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap index 232dfb2f..2146b1aa 100644 --- a/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap +++ b/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ConfirmationPopover matches snapshot 1`] = ` `; +exports[`ConfirmationPopover matches snapshot 1`] = ` `; diff --git a/js/components/__tests__/confirmation_popover.test.js b/js/components/__tests__/confirmation_popover.test.js index d82564e5..72369e00 100644 --- a/js/components/__tests__/confirmation_popover.test.js +++ b/js/components/__tests__/confirmation_popover.test.js @@ -1,11 +1,15 @@ -import { shallowMount } from '@vue/test-utils' +import { createLocalVue, shallowMount } from '@vue/test-utils' +import VTooltip from 'v-tooltip' import ConfirmationPopover from '../confirmation_popover' +const localVue = createLocalVue() +localVue.use(VTooltip) describe('ConfirmationPopover', () => { it('matches snapshot', () => { const wrapper = shallowMount(ConfirmationPopover, { + localVue, propsData: { action: '/some-url', btn_text: 'Do something dangerous', From 1feaa726d2beb119bf29cfe0cdc0d0bdab0fe43f Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Tue, 27 Nov 2018 11:13:23 -0500 Subject: [PATCH 3/3] Add another test to ensure csrf token added correctly --- .../__tests__/confirmation_popover.test.js | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/js/components/__tests__/confirmation_popover.test.js b/js/components/__tests__/confirmation_popover.test.js index 72369e00..cf31b253 100644 --- a/js/components/__tests__/confirmation_popover.test.js +++ b/js/components/__tests__/confirmation_popover.test.js @@ -7,20 +7,26 @@ const localVue = createLocalVue() localVue.use(VTooltip) describe('ConfirmationPopover', () => { - it('matches snapshot', () => { - const wrapper = shallowMount(ConfirmationPopover, { - localVue, - propsData: { - action: '/some-url', - btn_text: 'Do something dangerous', - cancel_btn_text: 'Cancel', - confirm_btn_text: 'Confirm', - confirm_msg: 'Are you sure you want to do that?', - csrf_token: '42' - } - }) + const wrapper = shallowMount(ConfirmationPopover, { + localVue, + propsData: { + action: '/some-url', + btn_text: 'Do something dangerous', + cancel_btn_text: 'Cancel', + confirm_btn_text: 'Confirm', + confirm_msg: 'Are you sure you want to do that?', + csrf_token: '42' + } + }) + it('matches snapshot', () => { expect(wrapper).toMatchSnapshot() }) + + it('renders form with hidden csrf input', () => { + const input = wrapper.find('input[type=hidden]') + expect(input.exists()).toBe(true) + expect(input.attributes('value')).toBe('42') + }) })