From 8b79c32a55765b44374b66c1959af7abfc08fdbb Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 2 Aug 2018 15:49:14 +0200 Subject: [PATCH] Do not rely on javascript for form submits without direct upload --- .../new_user/dossiers_controller.rb | 2 +- app/javascript/packs/application-old.js | 3 +- app/javascript/packs/application.js | 4 +- .../progress.js} | 27 ----- app/javascript/shared/activestorage/ujs.js | 105 ++++++++++++++++++ app/views/shared/dossiers/_edit.html.haml | 12 +- .../new_user/dossiers_controller_spec.rb | 16 +-- 7 files changed, 122 insertions(+), 47 deletions(-) rename app/javascript/shared/{direct-uploads.js => activestorage/progress.js} (59%) create mode 100644 app/javascript/shared/activestorage/ujs.js diff --git a/app/controllers/new_user/dossiers_controller.rb b/app/controllers/new_user/dossiers_controller.rb index b94747c7e..595bc2ac9 100644 --- a/app/controllers/new_user/dossiers_controller.rb +++ b/app/controllers/new_user/dossiers_controller.rb @@ -209,7 +209,7 @@ module NewUser end def draft? - params[:submit_action] == 'draft' + params[:save_draft] end end end diff --git a/app/javascript/packs/application-old.js b/app/javascript/packs/application-old.js index 38a2ae10f..89c9a9706 100644 --- a/app/javascript/packs/application-old.js +++ b/app/javascript/packs/application-old.js @@ -1,6 +1,6 @@ import Turbolinks from 'turbolinks'; import Rails from 'rails-ujs'; -import * as ActiveStorage from 'activestorage'; +import ActiveStorage from '../shared/activestorage/ujs'; import Chartkick from 'chartkick'; import Highcharts from 'highcharts'; import Bloodhound from 'bloodhound-js'; @@ -14,7 +14,6 @@ import 'babel-polyfill'; import 'typeahead.js'; import '../shared/rails-ujs-fix'; -import '../shared/direct-uploads'; // Start Rails helpers Chartkick.addAdapter(Highcharts); diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index 73f5862e8..dce4ef0c5 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -1,7 +1,6 @@ import Turbolinks from 'turbolinks'; import Rails from 'rails-ujs'; -import * as ActiveStorage from 'activestorage'; - +import ActiveStorage from '../shared/activestorage/ujs'; import Chartkick from 'chartkick'; import Highcharts from 'highcharts'; import jQuery from 'jquery'; @@ -15,7 +14,6 @@ import 'select2'; import 'typeahead.js'; import '../shared/rails-ujs-fix'; -import '../shared/direct-uploads'; import '../new_design/buttons'; import '../new_design/form-validation'; diff --git a/app/javascript/shared/direct-uploads.js b/app/javascript/shared/activestorage/progress.js similarity index 59% rename from app/javascript/shared/direct-uploads.js rename to app/javascript/shared/activestorage/progress.js index eca7daa45..efaf9349c 100644 --- a/app/javascript/shared/direct-uploads.js +++ b/app/javascript/shared/activestorage/progress.js @@ -47,30 +47,3 @@ addEventListener('direct-upload:end', event => { element.classList.add('direct-upload--complete'); }); - -addEventListener('turbolinks:load', () => { - const submitButtons = document.querySelectorAll( - 'form button[type=submit][data-action]' - ); - const hiddenInput = document.querySelector( - 'form input[type=hidden][name=submit_action]' - ); - - for (let button of submitButtons) { - button.addEventListener('click', () => { - // Active Storage will intercept the form.submit event to upload - // the attached files, and then fire the submit action again – but forgetting - // which button was clicked. So we manually set the type of action that trigerred - // the form submission. - const action = button.getAttribute('data-action'); - hiddenInput.value = action; - // Some form fields are marked as mandatory, but when saving a draft we don't want them - // to be enforced by the browser. - if (action === 'submit') { - button.form.removeAttribute('novalidate'); - } else { - button.form.setAttribute('novalidate', 'novalidate'); - } - }); - } -}); diff --git a/app/javascript/shared/activestorage/ujs.js b/app/javascript/shared/activestorage/ujs.js new file mode 100644 index 000000000..1963fcb72 --- /dev/null +++ b/app/javascript/shared/activestorage/ujs.js @@ -0,0 +1,105 @@ +import { DirectUploadsController } from 'activestorage/src/direct_uploads_controller'; +import { findElement } from 'activestorage/src/helpers'; +import './progress'; + +// This is a patched copy of https://github.com/rails/rails/blob/master/activestorage/app/javascript/activestorage/ujs.js +// It fixes support for multiple input/button elements on direct upload forms + +const processingAttribute = 'data-direct-uploads-processing'; +let started = false; + +export function start() { + if (!started) { + started = true; + document.addEventListener('submit', didSubmitForm); + document.addEventListener('click', didSubmitFormElement); + document.addEventListener('ajax:before', didSubmitRemoteElement); + } +} + +export default { start }; + +function didSubmitForm(event) { + handleFormSubmissionEvent(event); +} + +function didSubmitFormElement(event) { + const { target } = event; + if (isSubmitElement(target)) { + handleFormSubmissionEvent(formSubmitEvent(event), target); + } +} + +function didSubmitRemoteElement(event) { + if (event.target.tagName == 'FORM') { + handleFormSubmissionEvent(event); + } +} + +function formSubmitEvent(event) { + return { + target: event.target.form, + preventDefault() { + event.preventDefault(); + } + }; +} + +function isSubmitElement({ tagName, type, form }) { + if (form && (tagName === 'BUTTON' || tagName === 'INPUT')) { + return type === 'submit'; + } + return false; +} + +function handleFormSubmissionEvent(event, button) { + const form = event.target; + + if (form.hasAttribute(processingAttribute)) { + event.preventDefault(); + return; + } + + const controller = new DirectUploadsController(form); + const { inputs } = controller; + + if (inputs.length) { + event.preventDefault(); + form.setAttribute(processingAttribute, ''); + inputs.forEach(disable); + controller.start(error => { + form.removeAttribute(processingAttribute); + if (error) { + inputs.forEach(enable); + } else { + submitForm(form, button); + } + }); + } +} + +function submitForm(form, button) { + button = button || findElement(form, 'input[type=submit]'); + if (button) { + const { disabled } = button; + button.disabled = false; + button.focus(); + button.click(); + button.disabled = disabled; + } else { + button = document.createElement('input'); + button.type = 'submit'; + button.style.display = 'none'; + form.appendChild(button); + button.click(); + form.removeChild(button); + } +} + +function disable(input) { + input.disabled = true; +} + +function enable(input) { + input.disabled = false; +} diff --git a/app/views/shared/dossiers/_edit.html.haml b/app/views/shared/dossiers/_edit.html.haml index 2c83362b3..b7138253f 100644 --- a/app/views/shared/dossiers/_edit.html.haml +++ b/app/views/shared/dossiers/_edit.html.haml @@ -4,7 +4,7 @@ - if apercu - form_options = { url: '', method: :get, html: { class: 'form', multipart: true } } - else - - form_options = { url: modifier_dossier_url(dossier), method: :patch, html: { class: 'form', multipart: true, novalidate: dossier.brouillon? } } + - form_options = { url: modifier_dossier_url(dossier), method: :patch, html: { class: 'form', multipart: true } } = form_for dossier, form_options do |f| @@ -59,8 +59,6 @@ - if !apercu .send-wrapper - = hidden_field_tag 'submit_action', 'draft' - - if dossier.brouillon? - if current_user.owns?(dossier) = link_to ask_deletion_dossier_path(dossier), @@ -71,19 +69,21 @@ = f.button 'Enregistrer le brouillon', formnovalidate: true, + name: :save_draft, + value: true, class: 'button send secondary', - data: { action: 'draft', disable_with: 'Envoi...' } + data: { disable_with: 'Envoi...' } - if dossier.can_transition_to_en_construction? = f.button 'Soumettre le dossier', class: 'button send primary', disabled: !current_user.owns?(dossier), - data: { action: 'submit', disable_with: 'Envoi...' } + data: { disable_with: 'Envoi...' } - else = f.button 'Enregistrer les modifications du dossier', class: 'button send primary', - data: { action: 'submit', disable_with: 'Envoi...' } + data: { disable_with: 'Envoi...' } - if dossier.brouillon? && !current_user.owns?(dossier) .send-notice.invite-cannot-submit diff --git a/spec/controllers/new_user/dossiers_controller_spec.rb b/spec/controllers/new_user/dossiers_controller_spec.rb index 15f75cae1..a3605c7d2 100644 --- a/spec/controllers/new_user/dossiers_controller_spec.rb +++ b/spec/controllers/new_user/dossiers_controller_spec.rb @@ -100,38 +100,38 @@ describe NewUser::DossiersController, type: :controller do let(:user) { create(:user) } let(:asked_dossier) { create(:dossier) } let(:ensure_authorized) { :forbid_invite_submission! } - let(:submit_action) { 'submit' } + let(:draft) { false } before do - @controller.params = @controller.params.merge(dossier_id: asked_dossier.id, submit_action: submit_action) + @controller.params = @controller.params.merge(dossier_id: asked_dossier.id, save_draft: draft) allow(@controller).to receive(:current_user).and_return(user) allow(@controller).to receive(:redirect_to) end context 'when a user save their own draft' do let(:asked_dossier) { create(:dossier, user: user) } - let(:submit_action) { 'draft' } + let(:draft) { true } it_behaves_like 'does not redirect nor flash' end context 'when a user submit their own dossier' do let(:asked_dossier) { create(:dossier, user: user) } - let(:submit_action) { 'submit' } + let(:draft) { false } it_behaves_like 'does not redirect nor flash' end context 'when an invite save the draft for a dossier where they where invited' do before { create(:invite, dossier: asked_dossier, user: user, type: 'InviteUser') } - let(:submit_action) { 'draft' } + let(:draft) { true } it_behaves_like 'does not redirect nor flash' end context 'when an invite submit a dossier where they where invited' do before { create(:invite, dossier: asked_dossier, user: user, type: 'InviteUser') } - let(:submit_action) { 'submit' } + let(:draft) { false } it_behaves_like 'redirects and flashes' end @@ -353,7 +353,7 @@ describe NewUser::DossiersController, type: :controller do it { expect(flash.alert).to eq(['Le champ l doit être rempli.', 'pj']) } context 'and the user saves a draft' do - let(:payload) { submit_payload.merge(submit_action: 'draft') } + let(:payload) { submit_payload.merge(save_draft: true) } it { expect(response).to render_template(:modifier) } it { expect(flash.notice).to eq('Votre brouillon a bien été sauvegardé.') } @@ -376,7 +376,7 @@ describe NewUser::DossiersController, type: :controller do let!(:invite) { create(:invite, dossier: dossier, user: user, type: 'InviteUser') } context 'and the invite saves a draft' do - let(:payload) { submit_payload.merge(submit_action: 'draft') } + let(:payload) { submit_payload.merge(save_draft: true) } before do first_champ.type_de_champ.update(mandatory: true, libelle: 'l')