From 3c1576dacf0186b4694dd7e916f9e3c2b58bc7a1 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 4 Sep 2019 10:24:31 +0100 Subject: [PATCH 1/4] Add helper for mocking/stubbing form.submit JSDOM doesn't implement the submit method on form elements. It does have a submit method but this just fires a 'not implemented' error. We need to spy on form submissions fired by clicking on the submit button but can't because this event calls the submit method internally so spying on `form.submit` doesn't work. This adds a helper which spies on the internal method that is actually called. When JSDOM implements the submit method properly this should be removed. --- tests/javascripts/support/helpers.js | 2 ++ tests/javascripts/support/helpers/forms.js | 33 ++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/javascripts/support/helpers/forms.js diff --git a/tests/javascripts/support/helpers.js b/tests/javascripts/support/helpers.js index 620bba003..65c7f6a0d 100644 --- a/tests/javascripts/support/helpers.js +++ b/tests/javascripts/support/helpers.js @@ -3,6 +3,7 @@ const domInterfaces = require('./helpers/dom_interfaces.js'); const html = require('./helpers/html.js'); const elements = require('./helpers/elements.js'); const rendering = require('./helpers/rendering.js'); +const forms = require('./helpers/forms.js'); exports.triggerEvent = events.triggerEvent; exports.clickElementWithMouse = events.clickElementWithMouse; @@ -15,3 +16,4 @@ exports.getRadios = html.getRadios; exports.element = elements.element; exports.WindowMock = rendering.WindowMock; exports.ScreenMock = rendering.ScreenMock; +exports.spyOnFormSubmit = forms.spyOnFormSubmit; diff --git a/tests/javascripts/support/helpers/forms.js b/tests/javascripts/support/helpers/forms.js new file mode 100644 index 000000000..415ac0dbe --- /dev/null +++ b/tests/javascripts/support/helpers/forms.js @@ -0,0 +1,33 @@ + +// helper for spying on the submit method on a form element +// JSDOM's implementation of submit just wraps a 'not implemented' error so we need to mock that to track calls to it +// +// * Remove when JSDOM implements submit on its form elements * +// +// elements in JSDOM have a public API and a private implementation API, only used by internal code +// For form elements, these are instances of the following classes: +// - HTMLFormElement +// - HTMLFormElementImpl +// +// form elements link to their implementation instance via a symbol property +// this spies on the submit method of the implementation instance for a form element and mocks it to prevent 'not implemented' errors +function spyOnFormSubmit (jest, form) { + + const formImplementationSymbols = Object.getOwnPropertySymbols(form).filter( + symbol => form[symbol].constructor.name === 'HTMLFormElementImpl' + ); + + if (!formImplementationSymbols.length) { + throw Error("Error mocking form.submit: symbol reference to HTMLFormElementImpl instance not found on form element"); + } + + const HTMLFormElementImpl = form[formImplementationSymbols[0]]; + + const submitSpy = jest.spyOn(HTMLFormElementImpl, 'submit') + + submitSpy.mockImplementation(); + return submitSpy; + +}; + +exports.spyOnFormSubmit = spyOnFormSubmit; From 9967f29ca6da1484b1a212dacb7b8fa2a5d05271 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 4 Sep 2019 10:28:25 +0100 Subject: [PATCH 2/4] Add tests for preventing duplicate form submit --- .../preventDuplicateFormSubmissions.test.js | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tests/javascripts/preventDuplicateFormSubmissions.test.js diff --git a/tests/javascripts/preventDuplicateFormSubmissions.test.js b/tests/javascripts/preventDuplicateFormSubmissions.test.js new file mode 100644 index 000000000..43ebfbce0 --- /dev/null +++ b/tests/javascripts/preventDuplicateFormSubmissions.test.js @@ -0,0 +1,68 @@ +const helpers = require('./support/helpers.js'); + +beforeAll(() => { + jest.useFakeTimers(); +}); + +afterAll(() => { + require('./support/teardown.js'); +}); + +describe('Prevent duplicate form submissions', () => { + + let form; + let button; + let formSubmitSpy; + + beforeEach(() => { + + // set up DOM + document.body.innerHTML = ` +
+ +
`; + + form = document.querySelector('form'); + button = document.querySelector('button'); + + // requires a helper due to JSDOM not implementing the submit method + formSubmitSpy = helpers.spyOnFormSubmit(jest, form); + + require('../../app/assets/javascripts/preventDuplicateFormSubmissions.js'); + + }); + + afterEach(() => { + + document.body.innerHTML = ''; + + // we run the previewPane.js script every test + // the module cache needs resetting each time for the script to execute + jest.resetModules(); + + formSubmitSpy.mockClear(); + + }); + + test("It should prevent any clicks of the 'submit' button after the first one submitting the form", () => { + + helpers.triggerEvent(button, 'click'); + helpers.triggerEvent(button, 'click'); + + expect(formSubmitSpy.mock.calls.length).toEqual(1); + + }); + + test("It should allow clicks again after 1.5 minutes", () => { + + helpers.triggerEvent(button, 'click'); + + jest.advanceTimersByTime(1500); + + helpers.triggerEvent(button, 'click'); + + expect(formSubmitSpy.mock.calls.length).toEqual(2); + + }); + +}); From 9d8fcf34fcde0a62ddc246e4f22359116d9b2405 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 4 Sep 2019 11:46:18 +0100 Subject: [PATCH 3/4] Fix variable assignment Without the `var` prefix this was making `$submitButton` a global variable, which is not what the code intended. --- app/assets/javascripts/preventDuplicateFormSubmissions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/preventDuplicateFormSubmissions.js b/app/assets/javascripts/preventDuplicateFormSubmissions.js index 0eca28039..61e2e6890 100644 --- a/app/assets/javascripts/preventDuplicateFormSubmissions.js +++ b/app/assets/javascripts/preventDuplicateFormSubmissions.js @@ -4,7 +4,7 @@ let disableSubmitButtons = function(event) { - $submitButton = $(this).find(':submit'); + var $submitButton = $(this).find(':submit'); if ($submitButton.data('clicked') == 'true') { From 45e61d6ba57fccb1aefd85e104cdef82e9ce14a7 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 5 Sep 2019 10:51:15 +0100 Subject: [PATCH 4/4] Fix description of time until re-enable clicks --- tests/javascripts/preventDuplicateFormSubmissions.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/javascripts/preventDuplicateFormSubmissions.test.js b/tests/javascripts/preventDuplicateFormSubmissions.test.js index 43ebfbce0..181b78853 100644 --- a/tests/javascripts/preventDuplicateFormSubmissions.test.js +++ b/tests/javascripts/preventDuplicateFormSubmissions.test.js @@ -53,7 +53,7 @@ describe('Prevent duplicate form submissions', () => { }); - test("It should allow clicks again after 1.5 minutes", () => { + test("It should allow clicks again after 1.5 seconds", () => { helpers.triggerEvent(button, 'click');