Do not rely on javascript for form submits without direct upload

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

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);

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';

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');
-      }
-    });
-  }
-});

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;
+}

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

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')