From c93141d8bb632a1420ad31dd2c6b592543b567bb Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 26 Oct 2023 16:01:15 +0200 Subject: [PATCH 1/2] chore(coldwired): update utils --- .../controllers/autosave_submit_controller.ts | 2 +- .../controllers/autosubmit_controller.ts | 56 ++++++------ .../type_de_champ_editor_controller.ts | 47 +++++----- app/javascript/shared/utils.test.ts | 85 +------------------ app/javascript/shared/utils.ts | 44 ---------- package.json | 2 +- yarn.lock | 5 ++ 7 files changed, 53 insertions(+), 188 deletions(-) diff --git a/app/javascript/controllers/autosave_submit_controller.ts b/app/javascript/controllers/autosave_submit_controller.ts index 94e9e78d4..ba88e2697 100644 --- a/app/javascript/controllers/autosave_submit_controller.ts +++ b/app/javascript/controllers/autosave_submit_controller.ts @@ -1,4 +1,4 @@ -import { isButtonElement } from '@utils'; +import { isButtonElement } from '@coldwired/utils'; import { ApplicationController } from './application_controller'; diff --git a/app/javascript/controllers/autosubmit_controller.ts b/app/javascript/controllers/autosubmit_controller.ts index fe381e521..09e299238 100644 --- a/app/javascript/controllers/autosubmit_controller.ts +++ b/app/javascript/controllers/autosubmit_controller.ts @@ -1,10 +1,4 @@ -import { - isSelectElement, - isCheckboxOrRadioInputElement, - isTextInputElement, - isDateInputElement -} from '@utils'; -import { isFormInputElement } from '@coldwired/utils'; +import { isFormInputElement, matchInputElement } from '@coldwired/utils'; import { ApplicationController } from './application_controller'; @@ -28,14 +22,9 @@ export class AutosubmitController extends ApplicationController { private onChange(event: Event) { const target = this.findTargetElement(event); - if (!target) return; - if ( - isSelectElement(target) || - isCheckboxOrRadioInputElement(target) || - isTextInputElement(target) - ) { - if (isDateInputElement(target)) { + matchInputElement(target, { + date: (target) => { if (target.value.trim() == '' || !isNaN(Date.parse(target.value))) { this.#dateTimeChangedInputs.add(target); this.debounce(this.submit, AUTOSUBMIT_DATE_DEBOUNCE_DELAY); @@ -43,34 +32,34 @@ export class AutosubmitController extends ApplicationController { this.#dateTimeChangedInputs.delete(target); this.cancelDebounce(this.submit); } - } else { - this.cancelDebounce(this.submit); - this.submit(); - } - } + }, + text: () => this.submitNow(), + changeable: () => this.submitNow() + }); } private onInput(event: Event) { const target = this.findTargetElement(event); - if (!target) return; - if (!isDateInputElement(target) && isTextInputElement(target)) { - this.debounce(this.submit, AUTOSUBMIT_DEBOUNCE_DELAY); - } + matchInputElement(target, { + date: () => {}, + inputable: () => this.debounce(this.submit, AUTOSUBMIT_DEBOUNCE_DELAY) + }); } private onBlur(event: Event) { const target = this.findTargetElement(event); if (!target) return; - if (isDateInputElement(target)) { - Promise.resolve().then(() => { - if (this.#dateTimeChangedInputs.has(target)) { - this.cancelDebounce(this.submit); - this.submit(); - } - }); - } + matchInputElement(target, { + date: () => { + Promise.resolve().then(() => { + if (this.#dateTimeChangedInputs.has(target)) { + this.submitNow(); + } + }); + } + }); } private findTargetElement(event: Event) { @@ -111,6 +100,11 @@ export class AutosubmitController extends ApplicationController { return eventTypes.length == 0 ? true : eventTypes; } + private submitNow() { + this.cancelDebounce(this.submit); + this.submit(); + } + private submit() { const submitter = this.hasSubmitterTarget ? this.submitterTarget : null; const form = diff --git a/app/javascript/controllers/type_de_champ_editor_controller.ts b/app/javascript/controllers/type_de_champ_editor_controller.ts index 6f6af508d..a813b1de0 100644 --- a/app/javascript/controllers/type_de_champ_editor_controller.ts +++ b/app/javascript/controllers/type_de_champ_editor_controller.ts @@ -1,12 +1,8 @@ /* eslint-disable react-hooks/rules-of-hooks */ import { ActionEvent } from '@hotwired/stimulus'; -import { - httpRequest, - isSelectElement, - isCheckboxOrRadioInputElement, - isTextInputElement, - getConfig -} from '@utils'; +import { httpRequest, getConfig } from '@utils'; +import { matchInputElement } from '@coldwired/utils'; + import { AutoUpload } from '../shared/activestorage/auto-upload'; import { ApplicationController } from './application_controller'; @@ -59,29 +55,26 @@ export class TypeDeChampEditorController extends ApplicationController { } private onChange(event: Event) { - const target = event.target as HTMLElement & { form?: HTMLFormElement }; - - if ( - target.form && - (isSelectElement(target) || isCheckboxOrRadioInputElement(target)) - ) { - this.save(target.form); - } + matchInputElement(event.target, { + file: (target) => { + if (target.files?.length) { + const autoupload = new AutoUpload(target, target.files[0]); + autoupload.start(); + } + }, + changeable: (target) => this.save(target.form) + }); } private onInput(event: Event) { - const target = event.target as HTMLInputElement; - - // mark input as touched so we know to not overwrite it's value with next re-render - target.setAttribute('data-touched', 'true'); - - if (target.form && isTextInputElement(target)) { - this.#dirtyForms.add(target.form); - this.debounce(this.save, AUTOSAVE_DEBOUNCE_DELAY); - } else if (target.form && target.type == 'file' && target.files?.length) { - const autoupload = new AutoUpload(target, target.files[0]); - autoupload.start(); - } + matchInputElement(event.target, { + inputable: (target) => { + if (target.form) { + this.#dirtyForms.add(target.form); + this.debounce(this.save, AUTOSAVE_DEBOUNCE_DELAY); + } + } + }); } private onSortableEnd(event: CustomEvent<{ position: number }>) { diff --git a/app/javascript/shared/utils.test.ts b/app/javascript/shared/utils.test.ts index 8597cb61e..f6ee12073 100644 --- a/app/javascript/shared/utils.test.ts +++ b/app/javascript/shared/utils.test.ts @@ -1,14 +1,6 @@ import { suite, test, expect } from 'vitest'; -import { - show, - hide, - toggle, - toggleExpandIcon, - isSelectElement, - isTextInputElement, - isCheckboxOrRadioInputElement -} from './utils'; +import { show, hide, toggle, toggleExpandIcon } from './utils'; suite('@utils', () => { test('show', () => { @@ -46,79 +38,4 @@ suite('@utils', () => { expect(icon.classList.contains('fr-icon-arrow-down-s-line')).toBeTruthy(); expect(icon.classList.contains('fr-icon-arrow-up-s-line')).toBeFalsy(); }); - - test('isSelectElement', () => { - const select = document.createElement('select'); - const input = document.createElement('input'); - const textarea = document.createElement('textarea'); - - expect(isSelectElement(select)).toBeTruthy(); - expect(isSelectElement(input)).toBeFalsy(); - expect(isSelectElement(textarea)).toBeFalsy(); - - input.type = 'text'; - expect(isSelectElement(input)).toBeFalsy(); - - input.type = 'email'; - expect(isSelectElement(input)).toBeFalsy(); - - input.type = 'checkbox'; - expect(isSelectElement(input)).toBeFalsy(); - - input.type = 'radio'; - expect(isSelectElement(input)).toBeFalsy(); - - input.type = 'file'; - expect(isSelectElement(input)).toBeFalsy(); - }); - - test('isTextInputElement', () => { - const select = document.createElement('select'); - const input = document.createElement('input'); - const textarea = document.createElement('textarea'); - - expect(isTextInputElement(select)).toBeFalsy(); - expect(isTextInputElement(input)).toBeTruthy(); - expect(isTextInputElement(textarea)).toBeTruthy(); - - input.type = 'text'; - expect(isTextInputElement(input)).toBeTruthy(); - - input.type = 'email'; - expect(isTextInputElement(input)).toBeTruthy(); - - input.type = 'checkbox'; - expect(isTextInputElement(input)).toBeFalsy(); - - input.type = 'radio'; - expect(isTextInputElement(input)).toBeFalsy(); - - input.type = 'file'; - expect(isTextInputElement(input)).toBeFalsy(); - }); - - test('isCheckboxOrRadioInputElement', () => { - const select = document.createElement('select'); - const input = document.createElement('input'); - const textarea = document.createElement('textarea'); - - expect(isCheckboxOrRadioInputElement(select)).toBeFalsy(); - expect(isCheckboxOrRadioInputElement(input)).toBeFalsy(); - expect(isCheckboxOrRadioInputElement(textarea)).toBeFalsy(); - - input.type = 'text'; - expect(isCheckboxOrRadioInputElement(input)).toBeFalsy(); - - input.type = 'email'; - expect(isCheckboxOrRadioInputElement(input)).toBeFalsy(); - - input.type = 'checkbox'; - expect(isCheckboxOrRadioInputElement(input)).toBeTruthy(); - - input.type = 'radio'; - expect(isCheckboxOrRadioInputElement(input)).toBeTruthy(); - - input.type = 'file'; - expect(isCheckboxOrRadioInputElement(input)).toBeFalsy(); - }); }); diff --git a/app/javascript/shared/utils.ts b/app/javascript/shared/utils.ts index 3d3d8d44e..5a756a850 100644 --- a/app/javascript/shared/utils.ts +++ b/app/javascript/shared/utils.ts @@ -269,50 +269,6 @@ export function isNumeric(s: string) { return !isNaN(n) && isFinite(n); } -export function isButtonElement( - element: Element -): element is HTMLButtonElement { - return ( - element.tagName == 'BUTTON' || - (element.tagName == 'INPUT' && - (element as HTMLInputElement).type == 'submit') - ); -} - -export function isSelectElement( - element: HTMLElement -): element is HTMLSelectElement { - return element.tagName == 'SELECT'; -} - -export function isCheckboxOrRadioInputElement( - element: HTMLElement & { type?: string } -): element is HTMLInputElement { - return ( - element.tagName == 'INPUT' && - (element.type == 'checkbox' || element.type == 'radio') - ); -} - -export function isDateInputElement( - element: HTMLElement & { type?: string } -): element is HTMLInputElement { - return ( - element.tagName == 'INPUT' && - (element.type == 'date' || element.type == 'datetime-local') - ); -} - -export function isTextInputElement( - element: HTMLElement & { type?: string } -): element is HTMLInputElement { - return ( - ['INPUT', 'TEXTAREA'].includes(element.tagName) && - typeof element.type == 'string' && - !['checkbox', 'radio', 'file'].includes(element.type) - ); -} - export function fire(obj: EventTarget, name: string, data?: T) { const event = new CustomEvent(name, { bubbles: true, diff --git a/package.json b/package.json index 8102e2987..616963604 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "dependencies": { "@coldwired/actions": "^0.11.2", "@coldwired/turbo-stream": "^0.11.1", - "@coldwired/utils": "^0.11.1", + "@coldwired/utils": "^0.11.4", "@gouvfr/dsfr": "^1.10.1", "@graphiql/plugin-explorer": "^0.3.4", "@graphiql/toolkit": "^0.9.1", diff --git a/yarn.lock b/yarn.lock index d730b8d32..2a00dc5dd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1218,6 +1218,11 @@ resolved "https://registry.yarnpkg.com/@coldwired/utils/-/utils-0.11.1.tgz#d126246ab66591467e9e4c765769f133f6236ddb" integrity sha512-vuW1hVhD5U4NX/0dl+fT4RL92T5fEITwc9l/DnaBoP5SAuCAVRnHCWcdyROGz55E4WvSXFqAHD6qkxRwOKG2og== +"@coldwired/utils@^0.11.4": + version "0.11.4" + resolved "https://registry.yarnpkg.com/@coldwired/utils/-/utils-0.11.4.tgz#4889103dc73c1d86eff7d21574a0bde37b1f3c03" + integrity sha512-JvYosEc++zcuZmyfHAKG1cZ/jEIBM8ciJsD2ZcgFdzjSla1wtG+p6GOYiIVXZpCKO2oWGR4EmiYFYpBNIy3WDw== + "@emotion/is-prop-valid@^0.8.2": version "0.8.8" resolved "https://registry.yarnpkg.com/@emotion/is-prop-valid/-/is-prop-valid-0.8.8.tgz#db28b1c4368a259b60a97311d6a952d4fd01ac1a" From 869d83dda997c49bbe5008f80a889f749dd4fc76 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 26 Oct 2023 16:02:23 +0200 Subject: [PATCH 2/2] feat(dossier): validate on change and revalidate on input --- .../administrateurs/procedures_controller.rb | 6 + .../instructeurs/dossiers_controller.rb | 5 +- app/controllers/users/dossiers_controller.rb | 30 ++--- .../controllers/autosave_controller.ts | 118 +++++++++++------- .../controllers/autosave_submit_controller.ts | 15 ++- app/models/champ.rb | 17 +++ app/models/champs/cnaf_champ.rb | 20 +-- app/models/champs/decimal_number_champ.rb | 2 +- app/models/champs/dgfip_champ.rb | 20 +-- .../champs/expression_reguliere_champ.rb | 2 +- app/models/champs/iban_champ.rb | 2 +- app/models/champs/integer_number_champ.rb | 2 +- app/models/champs/mesri_champ.rb | 2 +- app/models/champs/pole_emploi_champ.rb | 2 +- app/models/champs/rna_champ.rb | 2 +- ...rna_champ_association_fetchable_concern.rb | 4 +- app/models/dossier.rb | 2 +- .../users/dossiers_controller_spec.rb | 4 +- spec/models/champs/cnaf_champ_spec.rb | 2 +- spec/models/champs/dgfip_champ_spec.rb | 2 +- spec/models/champs/iban_champ_spec.rb | 14 +-- spec/models/champs/rna_champ_spec.rb | 10 +- spec/models/dossier_spec.rb | 2 +- .../api_particulier/api_particulier_spec.rb | 35 +++--- spec/system/users/brouillon_spec.rb | 19 ++- spec/system/users/dossier_shared_examples.rb | 1 + 26 files changed, 194 insertions(+), 146 deletions(-) diff --git a/app/controllers/administrateurs/procedures_controller.rb b/app/controllers/administrateurs/procedures_controller.rb index f7e8c44ac..019d21972 100644 --- a/app/controllers/administrateurs/procedures_controller.rb +++ b/app/controllers/administrateurs/procedures_controller.rb @@ -61,7 +61,13 @@ module Administrateurs def apercu @dossier = procedure_without_control.draft_revision.dossier_for_preview(current_user) + DossierPreloader.load_one(@dossier) @tab = apercu_tab + if @tab == 'dossier' + @dossier.validate(:champs_public_value) + else + @dossier.validate(:champs_private_value) + end end def new diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index 339b814f2..8ba1829ee 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -292,9 +292,8 @@ module Instructeurs if dossier.champs_private_all.any?(&:changed?) dossier.last_champ_private_updated_at = Time.zone.now end - if !dossier.save(context: :annotations) - flash.now.alert = dossier.errors.full_messages - end + + dossier.save(context: :champs_private_value) respond_to do |format| format.turbo_stream do diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 5a4c047d1..88e50718b 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -204,7 +204,7 @@ module Users session.delete(:prefill_token) session.delete(:prefill_params) @dossier = dossier_with_champs - @dossier.valid?(context: :prefilling) + @dossier.validate(:champs_public_value) end def submit_brouillon @@ -519,27 +519,28 @@ module Users end def update_dossier_and_compute_errors - errors = [] - @dossier.assign_attributes(champs_public_params) if @dossier.champs_public_all.any?(&:changed_for_autosave?) @dossier.last_champ_updated_at = Time.zone.now end - if !@dossier.save(**validation_options) - errors = @dossier.errors + # We save the dossier without validating fields, and if it is successful and the client + # requests it, we ask for field validation errors. + if @dossier.save && params[:validate].present? + @dossier.valid?(:champs_public_value) end - errors + @dossier.errors end def submit_dossier_and_compute_errors - @dossier.valid?(**submit_validation_options) + @dossier.validate(:champs_public_value) errors = @dossier.errors @dossier.check_mandatory_and_visible_champs.map do |error_on_champ| errors.import(error_on_champ) end + errors end @@ -588,20 +589,5 @@ module Users def commentaire_params params.require(:commentaire).permit(:body, :piece_jointe) end - - def submit_validation_options - # rubocop:disable Lint/BooleanSymbol - # Force ActiveRecord to re-validate associated records. - { context: :false } - # rubocop:enable Lint/BooleanSymbol - end - - def validation_options - if dossier.brouillon? - { context: :brouillon } - else - submit_validation_options - end - end end end diff --git a/app/javascript/controllers/autosave_controller.ts b/app/javascript/controllers/autosave_controller.ts index 025010f8f..863794195 100644 --- a/app/javascript/controllers/autosave_controller.ts +++ b/app/javascript/controllers/autosave_controller.ts @@ -1,11 +1,5 @@ -import { - httpRequest, - ResponseError, - isSelectElement, - isCheckboxOrRadioInputElement, - isTextInputElement, - getConfig -} from '@utils'; +import { httpRequest, ResponseError, getConfig } from '@utils'; +import { matchInputElement, isButtonElement } from '@coldwired/utils'; import { ApplicationController } from './application_controller'; import { AutoUpload } from '../shared/activestorage/auto-upload'; @@ -54,66 +48,84 @@ export class AutosaveController extends ApplicationController { } onClickRetryButton(event: Event) { - const target = event.target as HTMLButtonElement; - const inputTargetSelector = target.dataset.inputTarget; - if (inputTargetSelector) { - const target = - this.element.querySelector(inputTargetSelector); - if ( - target && - target.type == 'file' && - target.dataset.autoAttachUrl && - target.files?.length - ) { - this.enqueueAutouploadRequest(target, target.files[0]); + const target = event.target; + if (isButtonElement(target)) { + const inputTargetSelector = target.dataset.inputTarget; + if (inputTargetSelector) { + const target = + this.element.querySelector(inputTargetSelector); + if ( + target && + target.type == 'file' && + target.dataset.autoAttachUrl && + target.files?.length + ) { + this.enqueueAutouploadRequest(target, target.files[0]); + } } } } private onChange(event: Event) { - const target = event.target as HTMLInputElement; - if (!target.disabled) { - if (target.type == 'file') { + matchInputElement(event.target, { + file: (target) => { if (target.dataset.autoAttachUrl && target.files?.length) { this.globalDispatch('autosave:input'); this.enqueueAutouploadRequest(target, target.files[0]); } - } else if (target.type == 'hidden') { - this.globalDispatch('autosave:input'); - // In React comboboxes we dispatch a "change" event on hidden inputs to trigger autosave. - // We want to debounce them. - this.debounce(this.enqueueAutosaveRequest, AUTOSAVE_DEBOUNCE_DELAY); - } else if ( - isSelectElement(target) || - isCheckboxOrRadioInputElement(target) - ) { + }, + changeable: (target) => { this.globalDispatch('autosave:input'); + // Wait next tick so champs having JS can interact // with form elements before extracting form data. setTimeout(() => { this.enqueueAutosaveRequest(); this.showConditionnalSpinner(target); }, 0); + }, + inputable: (target) => this.enqueueOnInput(target, true), + hidden: (target) => { + // In comboboxes we dispatch a "change" event on hidden inputs to trigger autosave. + // We want to debounce them. + this.enqueueOnInput(target, true); } - } + }); } private onInput(event: Event) { - const target = event.target as HTMLInputElement; - if ( - !target.disabled && - // Ignore input from React comboboxes. We trigger "change" events on them when selection is changed. - target.getAttribute('role') != 'combobox' && - isTextInputElement(target) - ) { - this.globalDispatch('autosave:input'); - this.debounce(this.enqueueAutosaveRequest, AUTOSAVE_DEBOUNCE_DELAY); - - this.showConditionnalSpinner(target); - } + matchInputElement(event.target, { + inputable: (target) => { + // Ignore input from React comboboxes. We trigger "change" events on them when selection is changed. + if (target.getAttribute('role') != 'combobox') { + const validate = this.needsValidation(target); + this.enqueueOnInput(target, validate); + } + } + }); } - private showConditionnalSpinner(target: HTMLInputElement) { + private enqueueOnInput( + target: HTMLInputElement | HTMLTextAreaElement, + validate: boolean + ) { + this.globalDispatch('autosave:input'); + + const callback = validate + ? this.enqueueAutosaveWithValidationRequest + : this.enqueueAutosaveRequest; + this.debounce(callback, AUTOSAVE_DEBOUNCE_DELAY); + + this.showConditionnalSpinner(target); + } + + private needsValidation(target: HTMLElement) { + return target.getAttribute('aria-invalid') == 'true'; + } + + private showConditionnalSpinner( + target: HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement + ) { const champWrapperElement = target.closest( '.editable-champ[data-dependent-conditions]' ); @@ -198,9 +210,18 @@ export class AutosaveController extends ApplicationController { this.didEnqueue(); } + private enqueueAutosaveWithValidationRequest() { + this.#latestPromise = this.#latestPromise.finally(() => + this.sendAutosaveRequest(true) + .then(() => this.didSucceed()) + .catch((error) => this.didFail(error)) + ); + this.didEnqueue(); + } + // Create a fetch request that saves the form. // Returns a promise fulfilled when the request completes. - private sendAutosaveRequest(): Promise { + private sendAutosaveRequest(validate = false): Promise { this.#abortController = new AbortController(); const { form, inputs } = this; @@ -222,6 +243,9 @@ export class AutosaveController extends ApplicationController { formData.append(input.name, input.value); } } + if (validate) { + formData.append('validate', 'true'); + } this.#pendingPromiseCount++; diff --git a/app/javascript/controllers/autosave_submit_controller.ts b/app/javascript/controllers/autosave_submit_controller.ts index ba88e2697..deeaa51c3 100644 --- a/app/javascript/controllers/autosave_submit_controller.ts +++ b/app/javascript/controllers/autosave_submit_controller.ts @@ -4,7 +4,7 @@ import { ApplicationController } from './application_controller'; export class AutosaveSubmitController extends ApplicationController { #isSaving = false; - #shouldSubmit = true; + #shouldSubmit = false; #buttonText?: string; connect(): void { @@ -46,15 +46,20 @@ export class AutosaveSubmitController extends ApplicationController { private disableButton() { if (isButtonElement(this.element)) { - this.#buttonText = this.element.value; - this.element.value = this.element.dataset.disableWith ?? ''; + const disableWith = this.element.dataset.disableWith ?? ''; + if (disableWith) { + this.#buttonText = this.element.textContent ?? undefined; + this.element.textContent = disableWith; + } this.element.disabled = true; } } private enableButton() { - if (isButtonElement(this.element) && this.#buttonText) { - this.element.value = this.#buttonText; + if (isButtonElement(this.element)) { + if (this.#buttonText) { + this.element.textContent = this.#buttonText; + } this.element.disabled = false; } } diff --git a/app/models/champ.rb b/app/models/champ.rb index c73a6da61..ac95133db 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -250,8 +250,25 @@ class Champ < ApplicationRecord public? && dossier.champ_forked_with_changes?(self) end + protected + + def valid_champ_value? + valid?(public? ? :champs_public_value : :champs_private_value) + end + private + def validate_champ_value? + case validation_context + when :champs_public_value + public? + when :champs_private_value + private? + else + false + end + end + def html_id "champ-#{stable_id}-#{id}" end diff --git a/app/models/champs/cnaf_champ.rb b/app/models/champs/cnaf_champ.rb index 2ddb6586f..bf8013266 100644 --- a/app/models/champs/cnaf_champ.rb +++ b/app/models/champs/cnaf_champ.rb @@ -1,7 +1,7 @@ class Champs::CnafChamp < Champs::TextChamp # see https://github.com/betagouv/api-particulier/blob/master/src/presentation/middlewares/cnaf-input-validation.middleware.ts - validates :numero_allocataire, format: { with: /\A\d{1,7}\z/ }, if: -> { code_postal.present? && validation_context != :brouillon } - validates :code_postal, format: { with: /\A\w{5}\z/ }, if: -> { numero_allocataire.present? && validation_context != :brouillon } + validates :numero_allocataire, format: { with: /\A\d{1,7}\z/ }, if: -> { code_postal.present? && validate_champ_value? } + validates :code_postal, format: { with: /\A\w{5}\z/ }, if: -> { numero_allocataire.present? && validate_champ_value? } store_accessor :value_json, :numero_allocataire, :code_postal @@ -14,14 +14,14 @@ class Champs::CnafChamp < Champs::TextChamp end def fetch_external_data - if valid? - APIParticulier::CnafAdapter.new( - procedure.api_particulier_token, - numero_allocataire, - code_postal, - procedure.api_particulier_sources - ).to_params - end + return unless valid_champ_value? + + APIParticulier::CnafAdapter.new( + procedure.api_particulier_token, + numero_allocataire, + code_postal, + procedure.api_particulier_sources + ).to_params end def external_id diff --git a/app/models/champs/decimal_number_champ.rb b/app/models/champs/decimal_number_champ.rb index 267ce6bca..a47e28862 100644 --- a/app/models/champs/decimal_number_champ.rb +++ b/app/models/champs/decimal_number_champ.rb @@ -25,7 +25,7 @@ class Champs::DecimalNumberChamp < Champ end def processed_value - return if invalid? + return unless valid_champ_value? value&.to_f end diff --git a/app/models/champs/dgfip_champ.rb b/app/models/champs/dgfip_champ.rb index 832897518..e702f84aa 100644 --- a/app/models/champs/dgfip_champ.rb +++ b/app/models/champs/dgfip_champ.rb @@ -1,7 +1,7 @@ class Champs::DgfipChamp < Champs::TextChamp # see https://github.com/betagouv/api-particulier/blob/master/src/presentation/middlewares/dgfip-input-validation.middleware.ts - validates :numero_fiscal, format: { with: /\A\w{13,14}\z/ }, if: -> { reference_avis.present? && validation_context != :brouillon } - validates :reference_avis, format: { with: /\A\w{13,14}\z/ }, if: -> { numero_fiscal.present? && validation_context != :brouillon } + validates :numero_fiscal, format: { with: /\A\w{13,14}\z/ }, if: -> { reference_avis.present? && validate_champ_value? } + validates :reference_avis, format: { with: /\A\w{13,14}\z/ }, if: -> { numero_fiscal.present? && validate_champ_value? } store_accessor :value_json, :numero_fiscal, :reference_avis @@ -14,14 +14,14 @@ class Champs::DgfipChamp < Champs::TextChamp end def fetch_external_data - if valid? - APIParticulier::DgfipAdapter.new( - procedure.api_particulier_token, - numero_fiscal, - reference_avis, - procedure.api_particulier_sources - ).to_params - end + return unless valid_champ_value? + + APIParticulier::DgfipAdapter.new( + procedure.api_particulier_token, + numero_fiscal, + reference_avis, + procedure.api_particulier_sources + ).to_params end def external_id diff --git a/app/models/champs/expression_reguliere_champ.rb b/app/models/champs/expression_reguliere_champ.rb index c8f1f3a74..d85a4f13f 100644 --- a/app/models/champs/expression_reguliere_champ.rb +++ b/app/models/champs/expression_reguliere_champ.rb @@ -1,3 +1,3 @@ class Champs::ExpressionReguliereChamp < Champ - validates_with ExpressionReguliereValidator, if: -> { validation_context != :brouillon } + validates_with ExpressionReguliereValidator, if: :validate_champ_value? end diff --git a/app/models/champs/iban_champ.rb b/app/models/champs/iban_champ.rb index 577dbc09c..578820a75 100644 --- a/app/models/champs/iban_champ.rb +++ b/app/models/champs/iban_champ.rb @@ -1,5 +1,5 @@ class Champs::IbanChamp < Champ - validates_with IbanValidator, if: -> { validation_context != :brouillon } + validates_with IbanValidator, if: :validate_champ_value? after_validation :format_iban def for_api diff --git a/app/models/champs/integer_number_champ.rb b/app/models/champs/integer_number_champ.rb index 8fe6747a6..deaf3b5f9 100644 --- a/app/models/champs/integer_number_champ.rb +++ b/app/models/champs/integer_number_champ.rb @@ -20,7 +20,7 @@ class Champs::IntegerNumberChamp < Champ private def processed_value - return if invalid? + return unless valid_champ_value? value&.to_i end diff --git a/app/models/champs/mesri_champ.rb b/app/models/champs/mesri_champ.rb index d12bc3c59..145f251a3 100644 --- a/app/models/champs/mesri_champ.rb +++ b/app/models/champs/mesri_champ.rb @@ -11,7 +11,7 @@ class Champs::MesriChamp < Champs::TextChamp end def fetch_external_data - return if !valid? + return unless valid_champ_value? APIParticulier::MesriAdapter.new( procedure.api_particulier_token, diff --git a/app/models/champs/pole_emploi_champ.rb b/app/models/champs/pole_emploi_champ.rb index e7cfde785..eec697860 100644 --- a/app/models/champs/pole_emploi_champ.rb +++ b/app/models/champs/pole_emploi_champ.rb @@ -11,7 +11,7 @@ class Champs::PoleEmploiChamp < Champs::TextChamp end def fetch_external_data - return if !valid? + return unless valid_champ_value? APIParticulier::PoleEmploiAdapter.new( procedure.api_particulier_token, diff --git a/app/models/champs/rna_champ.rb b/app/models/champs/rna_champ.rb index 3e521dd88..fd1187b1d 100644 --- a/app/models/champs/rna_champ.rb +++ b/app/models/champs/rna_champ.rb @@ -3,7 +3,7 @@ class Champs::RNAChamp < Champ validates :value, allow_blank: true, format: { with: /\AW[0-9]{9}\z/, message: I18n.t(:not_a_rna, scope: 'activerecord.errors.messages') - }, if: -> { validation_context != :brouillon } + }, if: :validate_champ_value? delegate :id, to: :procedure, prefix: true diff --git a/app/models/concerns/rna_champ_association_fetchable_concern.rb b/app/models/concerns/rna_champ_association_fetchable_concern.rb index 87e74ddbd..1d2483891 100644 --- a/app/models/concerns/rna_champ_association_fetchable_concern.rb +++ b/app/models/concerns/rna_champ_association_fetchable_concern.rb @@ -7,7 +7,7 @@ module RNAChampAssociationFetchableConcern self.value = rna return clear_association!(:empty) if rna.empty? - return clear_association!(:invalid) unless valid? + return clear_association!(:invalid) unless valid?(:champs_public_value) return clear_association!(:not_found) if (data = APIEntreprise::RNAAdapter.new(rna, procedure_id).to_params).blank? update!(data: data) @@ -21,7 +21,7 @@ module RNAChampAssociationFetchableConcern def clear_association!(error) @association_fetch_error_key = error self.data = nil - save!(context: :brouillon) + save! false end end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 7b022faf4..f01a109a4 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -440,7 +440,7 @@ class Dossier < ApplicationRecord validates :user, presence: true, if: -> { deleted_user_email_never_send.nil? }, unless: -> { prefilled } validates :individual, presence: true, if: -> { revision.procedure.for_individual? } - validates_associated :prefilled_champs_public, on: :prefilling + validates_associated :prefilled_champs_public, on: :champs_public_value def types_de_champ_public types_de_champ diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 754941ecc..e030fb3c9 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -411,7 +411,7 @@ describe Users::DossiersController, type: :controller do render_views let(:error_message) { 'nop' } before do - expect_any_instance_of(Dossier).to receive(:valid?).and_return(false) + expect_any_instance_of(Dossier).to receive(:validate).and_return(false) expect_any_instance_of(Dossier).to receive(:errors).and_return( [double(inner_error: double(base: first_champ), message: 'nop')] ) @@ -518,7 +518,7 @@ describe Users::DossiersController, type: :controller do render_views before do - expect_any_instance_of(Dossier).to receive(:valid?).and_return(false) + expect_any_instance_of(Dossier).to receive(:validate).and_return(false) expect_any_instance_of(Dossier).to receive(:errors).and_return( [double(inner_error: double(base: first_champ), message: 'nop')] ) diff --git a/spec/models/champs/cnaf_champ_spec.rb b/spec/models/champs/cnaf_champ_spec.rb index a57d67f39..3182ee1f8 100644 --- a/spec/models/champs/cnaf_champ_spec.rb +++ b/spec/models/champs/cnaf_champ_spec.rb @@ -38,7 +38,7 @@ describe Champs::CnafChamp, type: :model do let(:numero_allocataire) { '1234567' } let(:code_postal) { '12345' } let(:champ) { described_class.new(dossier: create(:dossier), type_de_champ: create(:type_de_champ_cnaf)) } - let(:validation_context) { :create } + let(:validation_context) { :champs_public_value } subject { champ.valid?(validation_context) } diff --git a/spec/models/champs/dgfip_champ_spec.rb b/spec/models/champs/dgfip_champ_spec.rb index 6a54c2ad4..5563f9d62 100644 --- a/spec/models/champs/dgfip_champ_spec.rb +++ b/spec/models/champs/dgfip_champ_spec.rb @@ -38,7 +38,7 @@ describe Champs::DgfipChamp, type: :model do let(:numero_fiscal) { '1122299999092' } let(:reference_avis) { 'FC22299999092' } let(:champ) { described_class.new(dossier: create(:dossier), type_de_champ: create(:type_de_champ_dgfip)) } - let(:validation_context) { :create } + let(:validation_context) { :champs_public_value } subject { champ.valid?(validation_context) } diff --git a/spec/models/champs/iban_champ_spec.rb b/spec/models/champs/iban_champ_spec.rb index 971c27526..09c8b2bfc 100644 --- a/spec/models/champs/iban_champ_spec.rb +++ b/spec/models/champs/iban_champ_spec.rb @@ -1,17 +1,17 @@ describe Champs::IbanChamp do describe '#valid?' do it do - expect(build(:champ_iban, value: nil)).to be_valid - expect(build(:champ_iban, value: "FR35 KDSQFDJQSMFDQMFDQ")).to_not be_valid - expect(build(:champ_iban, value: "FR7630006000011234567890189")).to be_valid - expect(build(:champ_iban, value: "FR76 3000 6000 0112 3456 7890 189")).to be_valid - expect(build(:champ_iban, value: "FR76 3000 6000 0112 3456 7890 189DSF")).to_not be_valid - expect(build(:champ_iban, value: "FR76 3000 6000 0112 3456 7890 189")).to be_valid + expect(build(:champ_iban, value: nil).valid?(:champs_public_value)).to be_truthy + expect(build(:champ_iban, value: "FR35 KDSQFDJQSMFDQMFDQ").valid?(:champs_public_value)).to be_falsey + expect(build(:champ_iban, value: "FR7630006000011234567890189").valid?(:champs_public_value)).to be_truthy + expect(build(:champ_iban, value: "FR76 3000 6000 0112 3456 7890 189").valid?(:champs_public_value)).to be_truthy + expect(build(:champ_iban, value: "FR76 3000 6000 0112 3456 7890 189DSF").valid?(:champs_public_value)).to be_falsey + expect(build(:champ_iban, value: "FR76 3000 6000 0112 3456 7890 189").valid?(:champs_public_value)).to be_truthy end it 'format value after validation' do champ = build(:champ_iban, value: "FR76 3000 6000 0112 3456 7890 189") - champ.valid? + champ.valid?(:champs_public_value) expect(champ.value).to eq("FR76 3000 6000 0112 3456 7890 189") end end diff --git a/spec/models/champs/rna_champ_spec.rb b/spec/models/champs/rna_champ_spec.rb index 08981e457..fcecd6606 100644 --- a/spec/models/champs/rna_champ_spec.rb +++ b/spec/models/champs/rna_champ_spec.rb @@ -2,11 +2,11 @@ describe Champs::RNAChamp do let(:champ) { create(:champ_rna, value: "W182736273") } describe '#valid?' do - it { expect(build(:champ_rna, value: nil)).to be_valid } - it { expect(build(:champ_rna, value: "2736251627")).to_not be_valid } - it { expect(build(:champ_rna, value: "A172736283")).to_not be_valid } - it { expect(build(:champ_rna, value: "W1827362718")).to_not be_valid } - it { expect(build(:champ_rna, value: "W182736273")).to be_valid } + it { expect(build(:champ_rna, value: nil).valid?(:champs_public_value)).to be_truthy } + it { expect(build(:champ_rna, value: "2736251627").valid?(:champs_public_value)).to be_falsey } + it { expect(build(:champ_rna, value: "A172736283").valid?(:champs_public_value)).to be_falsey } + it { expect(build(:champ_rna, value: "W1827362718").valid?(:champs_public_value)).to be_falsey } + it { expect(build(:champ_rna, value: "W182736273").valid?(:champs_public_value)).to be_truthy } end describe "#export" do diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 86b461907..9836bcf10 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1589,7 +1589,7 @@ describe Dossier, type: :model do before do champ = dossier.champs_public.first champ.value = expression_reguliere_exemple_text - dossier.save + dossier.save(context: :champs_public_value) end it 'should have errors' do diff --git a/spec/system/api_particulier/api_particulier_spec.rb b/spec/system/api_particulier/api_particulier_spec.rb index 4fe12f340..2a8aee5da 100644 --- a/spec/system/api_particulier/api_particulier_spec.rb +++ b/spec/system/api_particulier/api_particulier_spec.rb @@ -262,7 +262,7 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do before { login_as user, scope: :user } context 'CNAF' do - scenario 'it can fill an cnaf champ' do + scenario 'it can fill an cnaf champ', vcr: { cassette_name: 'api_particulier/success/composition_familiale' } do visit commencer_path(path: procedure.path) click_on 'Commencer la démarche' @@ -274,7 +274,6 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do fill_in 'Le numéro d’allocataire CAF', with: numero_allocataire fill_in 'Le code postal', with: 'wrong_code' - wait_for_autosave dossier = Dossier.last cnaf_champ = dossier.champs_public.find(&:cnaf?) @@ -284,12 +283,12 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do click_on 'Déposer le dossier' expect(page).to have_content("cnaf doit posséder 5 caractères") - VCR.use_cassette('api_particulier/success/composition_familiale') do - fill_in 'Le code postal', with: code_postal - wait_for_autosave - click_on 'Déposer le dossier' - perform_enqueued_jobs - end + fill_in 'Le code postal', with: code_postal + wait_until { cnaf_champ.reload.external_id.present? } + + click_on 'Déposer le dossier' + perform_enqueued_jobs + expect(page).to have_current_path(merci_dossier_path(Dossier.last)) perform_enqueued_jobs @@ -321,7 +320,7 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do context 'Pôle emploi' do let(:api_particulier_token) { '06fd8675601267d2988cbbdef56ecb0de1d45223' } - scenario 'it can fill a Pôle emploi field' do + scenario 'it can fill a Pôle emploi field', vcr: { cassette_name: 'api_particulier/success/situation_pole_emploi' } do visit commencer_path(path: procedure.path) click_on 'Commencer la démarche' @@ -332,7 +331,6 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do click_button('Continuer') fill_in "Identifiant", with: 'wrong code' - wait_for_autosave dossier = Dossier.last pole_emploi_champ = dossier.champs_public.find(&:pole_emploi?) @@ -342,12 +340,12 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do clear_enqueued_jobs pole_emploi_champ.update(external_id: nil, identifiant: nil) - VCR.use_cassette('api_particulier/success/situation_pole_emploi') do - fill_in "Identifiant", with: identifiant - wait_until { pole_emploi_champ.reload.external_id.present? } - click_on 'Déposer le dossier' - perform_enqueued_jobs - end + fill_in "Identifiant", with: identifiant + wait_until { pole_emploi_champ.reload.external_id.present? } + + click_on 'Déposer le dossier' + perform_enqueued_jobs + expect(page).to have_current_path(merci_dossier_path(Dossier.last)) perform_enqueued_jobs @@ -406,7 +404,6 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do click_button('Continuer') fill_in "INE", with: 'wrong code' - wait_for_autosave dossier = Dossier.last mesri_champ = dossier.champs_public.find(&:mesri?) @@ -471,7 +468,6 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do fill_in 'Le numéro fiscal', with: numero_fiscal fill_in "La référence d’avis d’imposition", with: 'wrong_code' - wait_for_autosave dossier = Dossier.last dgfip_champ = dossier.champs_public.find(&:dgfip?) @@ -482,7 +478,8 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do expect(page).to have_content(/dgfip doit posséder 13 ou 14 caractères/) fill_in "La référence d’avis d’imposition", with: reference_avis - wait_for_autosave + wait_until { dgfip_champ.reload.external_id.present? } + click_on 'Déposer le dossier' perform_enqueued_jobs diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb index 84b1c488e..b8ebdb22f 100644 --- a/spec/system/users/brouillon_spec.rb +++ b/spec/system/users/brouillon_spec.rb @@ -148,7 +148,9 @@ describe 'The user' do create(:procedure, :published, :for_individual, types_de_champ_public: [ { mandatory: true, libelle: 'texte obligatoire' }, { mandatory: false, libelle: 'texte optionnel' }, { mandatory: false, libelle: "nombre entier", type: :integer_number }, - { mandatory: false, libelle: "nombre décimal", type: :decimal_number } + { mandatory: false, libelle: "nombre décimal", type: :decimal_number }, + { mandatory: false, libelle: 'address', type: :address }, + { mandatory: false, libelle: 'IBAN', type: :iban } ]) } @@ -162,6 +164,17 @@ describe 'The user' do expect(page).to have_current_path(brouillon_dossier_path(user_dossier)) + fill_in('IBAN', with: 'FR') + wait_until { champ_value_for('IBAN') == 'FR' } + + expect(page).not_to have_content 'n’est pas au format IBAN' + blur + expect(page).to have_content 'n’est pas au format IBAN' + + fill_in('IBAN', with: 'FR7630006000011234567890189') + wait_until { champ_value_for('IBAN') == 'FR76 3000 6000 0112 3456 7890 189' } + expect(page).not_to have_content 'n’est pas au format IBAN' + # Check an incomplete dossier cannot be submitted when mandatory fields are missing click_on 'Déposer le dossier' expect(user_dossier.reload.brouillon?).to be(true) @@ -169,15 +182,15 @@ describe 'The user' do # Check a dossier can be submitted when all mandatory fields are filled fill_in('texte obligatoire', with: 'super texte') + wait_until { champ_value_for('texte obligatoire') == 'super texte' } click_on 'Déposer le dossier' wait_until { user_dossier.reload.en_construction? } - expect(champ_value_for('texte obligatoire')).to eq('super texte') expect(page).to have_current_path(merci_dossier_path(user_dossier)) end scenario 'fill address not in BAN', js: true, retry: 3 do - log_in(user, procedure) + log_in(user, simple_procedure) fill_individual fill_in('address', with: '2 rue de la paix, 92094 Belgique') diff --git a/spec/system/users/dossier_shared_examples.rb b/spec/system/users/dossier_shared_examples.rb index b4a3e0ee8..3b83d68a3 100644 --- a/spec/system/users/dossier_shared_examples.rb +++ b/spec/system/users/dossier_shared_examples.rb @@ -12,6 +12,7 @@ RSpec.shared_examples 'the user can edit the submitted demande' do fill_in('Texte obligatoire', with: 'Nouveau texte') click_on 'Déposer les modifications' + expect(page).to have_current_path(dossier_path(dossier)) click_on 'Demande' expect(page).to have_current_path(demande_dossier_path(dossier))