diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index cfba84e7e91..fbdad91e9f7 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -48,6 +48,7 @@ def champ? def champ @champ ||= if champ? + record.dossier.with_stream(record.dossier.stream_for_update) record.dossier.champ_for_update(record.type_de_champ, row_id: record.row_id, updated_by: current_user.email) end end diff --git a/app/controllers/champs/champ_controller.rb b/app/controllers/champs/champ_controller.rb index b551009f049..f12f00baec2 100644 --- a/app/controllers/champs/champ_controller.rb +++ b/app/controllers/champs/champ_controller.rb @@ -8,6 +8,7 @@ class Champs::ChampController < ApplicationController def find_champ dossier = policy_scope(Dossier).includes(:champs, revision: [:types_de_champ]).find(params[:dossier_id]) + dossier.with_stream(dossier.stream_for_update) type_de_champ = dossier.find_type_de_champ_by_stable_id(params[:stable_id]) if type_de_champ.repetition? dossier.project_champ(type_de_champ) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 00160b98458..3661a8562de 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -18,7 +18,8 @@ class DossiersController < UserController before_action :ensure_dossier_can_be_viewed, only: [:show] before_action :forbid_invite_submission!, only: [:submit_brouillon] before_action :forbid_closed_submission!, only: [:submit_brouillon] - before_action :set_dossier_as_editing_fork, only: [:submit_en_construction] + before_action :set_dossier_as_editing_fork, only: [:submit_en_construction], if: :update_with_fork? + before_action :set_dossier_stream, only: [:modifier, :update, :submit_en_construction, :champ], if: :update_with_stream? before_action :show_demarche_en_test_banner before_action :store_user_location!, only: :new @@ -253,32 +254,47 @@ def extend_conservation_and_restore def modifier @dossier = dossier_with_champs - @dossier_for_editing = dossier.owner_editing_fork + + if update_with_stream? + @dossier_for_editing = dossier + else + # TODO remove when all forks are gone + @dossier_for_editing = dossier.owner_editing_fork + end end def submit_en_construction @dossier = dossier_with_champs(pj_template: false) - editing_fork_origin = @dossier.editing_fork_origin + editing_fork_origin = dossier.editing_fork_origin + dossier_en_construction = editing_fork_origin || dossier if cast_bool(params.dig(:dossier, :pending_correction)) - editing_fork_origin.resolve_pending_correction + dossier_en_construction.resolve_pending_correction end submit_dossier_and_compute_errors if dossier.errors.blank? && dossier.can_passer_en_construction? - editing_fork_origin.merge_fork(dossier) - editing_fork_origin.submit_en_construction! - redirect_to dossier_path(editing_fork_origin) + if editing_fork_origin.present? + # TODO remove when all forks are gone + editing_fork_origin.merge_fork(dossier) + else + dossier.merge_stream(Champ::USER_DRAFT_STREAM) + end + + dossier_en_construction.submit_en_construction! + redirect_to dossier_path(dossier_en_construction) else @dossier_for_editing = dossier - @dossier = editing_fork_origin + if editing_fork_origin.present? + @dossier = editing_fork_origin + end + render :modifier end end def update - @dossier = dossier.en_construction? ? dossier.find_editing_fork(dossier.user) : dossier @dossier = dossier_with_champs(pj_template: false) update_dossier_and_compute_errors @@ -530,6 +546,18 @@ def set_dossier_as_editing_fork redirect_to dossier_path(dossier) end + def set_dossier_stream + dossier.with_stream(Champ::USER_DRAFT_STREAM) + end + + def update_with_stream? + dossier.update_with_stream? + end + + def update_with_fork? + dossier.with_editing_fork? + end + def update_dossier_and_compute_errors dossier.update_champs_attributes(champs_public_attributes_params, :public, updated_by: current_user.email) updated_champs = dossier.champs.filter(&:changed_for_autosave?) @@ -555,6 +583,7 @@ def submit_dossier_and_compute_errors dossier.validate(:champs_public_value) dossier.check_mandatory_and_visible_champs + # TODO remove when all forks are gone if dossier.editing_fork_origin&.pending_correction? dossier.editing_fork_origin.validate(:champs_public_value) dossier.editing_fork_origin.errors.where(:pending_correction).each do |error| diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb index ed169f8dbd4..9f95826e80f 100644 --- a/app/models/concerns/dossier_champs_concern.rb +++ b/app/models/concerns/dossier_champs_concern.rb @@ -237,6 +237,18 @@ def history champs_in_revision.filter(&:history_stream?) end + def update_with_stream? + en_construction? && procedure.feature_enabled?(:user_draft_stream) && !legacy_fork? + end + + def stream_for_update + if update_with_stream? + Champ::USER_DRAFT_STREAM + else + Champ::MAIN_STREAM + end + end + private def merge_user_draft_stream diff --git a/app/models/concerns/dossier_state_concern.rb b/app/models/concerns/dossier_state_concern.rb index f00b1f718fb..f2bff4fda0c 100644 --- a/app/models/concerns/dossier_state_concern.rb +++ b/app/models/concerns/dossier_state_concern.rb @@ -69,6 +69,7 @@ def after_commit_passer_en_instruction(h) NotificationMailer.send_notification_for_tiers(self).deliver_later if self.for_tiers? end + # TODO remove when all forks are gone editing_forks.each(&:destroy_editing_fork!) end @@ -334,6 +335,7 @@ def clean_champs_after_submit! remove_discarded_rows! remove_not_visible_rows! remove_not_visible_or_empty_champs! + # TODO remove when all forks are gone editing_forks.each(&:destroy_editing_fork!) end diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 62970f9f55f..e9d1f173de7 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -1034,6 +1034,368 @@ end end + describe '#update brouillon' do + before { sign_in(user) } + + let(:procedure) { create(:procedure, :published, types_de_champ_public:) } + let(:types_de_champ_public) { [{}, { type: :piece_justificative, mandatory: false }] } + let(:dossier) { create(:dossier, user:, procedure:) } + let(:first_champ) { dossier.project_champs_public.first } + let(:piece_justificative_champ) { dossier.project_champs_public.last } + let(:value) { 'beautiful value' } + let(:file) { fixture_file_upload('spec/fixtures/files/piece_justificative_0.pdf', 'application/pdf') } + let(:now) { Time.zone.parse('01/01/2100') } + + let(:submit_payload) do + { + id: dossier.id, + dossier: { + groupe_instructeur_id: dossier.groupe_instructeur_id, + champs_public_attributes: { + first_champ.public_id => { + value: value + }, + piece_justificative_champ.public_id => { + piece_justificative_file: file + } + } + } + } + end + let(:payload) { submit_payload } + + subject do + Timecop.freeze(now) do + patch :update, params: payload, format: :turbo_stream + end + end + + context 'when the dossier cannot be updated by the user' do + let(:dossier) { create(:dossier, :en_instruction, user:, procedure:) } + + it 'redirects to the dossiers list' do + subject + + expect(response).to redirect_to(dossier_path(dossier)) + expect(flash.alert).to eq('Votre dossier ne peut plus être modifié') + end + end + + context 'when dossier can be updated by the owner' do + it 'updates the champs' do + subject + + expect(response).to have_http_status(:ok) + expect(dossier.reload.updated_at.year).to eq(2100) + expect(dossier.reload.state).to eq(Dossier.states.fetch(:brouillon)) + end + end + + context 'when the user has an invitation but is not the owner' do + let(:dossier) { create(:dossier, procedure: procedure) } + let!(:invite) { create(:invite, dossier: dossier, user: user) } + + before { subject } + + it { expect(first_champ.reload.value).to eq('beautiful value') } + it { expect(response).to have_http_status(:ok) } + end + + context 'decimal number champ separator' do + let (:procedure) { create(:procedure, :published, types_de_champ_public: [{ type: :decimal_number }]) } + let (:submit_payload) do + { + id: dossier.id, + dossier: { + champs_public_attributes: { first_champ.public_id => { value: value } } + } + } + end + + context 'when spearator is dot' do + let(:value) { '3.14' } + + it "saves the value" do + subject + expect(first_champ.reload.value).to eq('3.14') + end + end + + context 'when spearator is comma' do + let(:value) { '3,14' } + + it "saves the value" do + subject + expect(first_champ.reload.value).to eq('3.14') + end + end + end + + context 'having ineligibilite_rules setup' do + include Logic + render_views + + let(:types_de_champ_public) { [{ type: :text }, { type: :integer_number }] } + let(:text_champ) { dossier.project_champs_public.first } + let(:number_champ) { dossier.project_champs_public.last } + let(:submit_payload) do + { + id: dossier.id, + dossier: { + groupe_instructeur_id: dossier.groupe_instructeur_id, + champs_public_attributes: { + text_champ.public_id => { + with_public_id: true, + value: "hello world" + }, + number_champ.public_id => { + with_public_id: true, + value: + } + } + } + } + end + let(:must_be_greater_than) { 10 } + + before do + procedure.published_revision.update( + ineligibilite_enabled: true, + ineligibilite_message: 'lol', + ineligibilite_rules: greater_than(champ_value(number_champ.stable_id), constant(must_be_greater_than)) + ) + procedure.published_revision.save! + end + render_views + + context 'when it switches from true to false' do + let(:value) { must_be_greater_than + 1 } + + it 'raises popup' do + subject + dossier.reload + expect(dossier.can_passer_en_construction?).to be_falsey + expect(assigns(:can_passer_en_construction_was)).to eq(true) + expect(assigns(:can_passer_en_construction_is)).to eq(false) + expect(response.body).to match(ActionView::RecordIdentifier.dom_id(dossier, :ineligibilite_rules_broken)) + end + end + + context 'when it stays true' do + let(:value) { must_be_greater_than - 1 } + it 'does nothing' do + subject + dossier.reload + expect(dossier.can_passer_en_construction?).to be_truthy + expect(assigns(:can_passer_en_construction_was)).to eq(true) + expect(assigns(:can_passer_en_construction_is)).to eq(true) + expect(response.body).not_to have_selector("##{ActionView::RecordIdentifier.dom_id(dossier, :ineligibilite_rules_broken)}") + end + end + end + end + + describe '#update en_construction (stream)' do + before { sign_in(user) } + + let(:types_de_champ_public) { [{}, { type: :piece_justificative }] } + let(:procedure) { create(:procedure, :published, types_de_champ_public:).tap { Flipper.enable(:user_draft_stream, _1) } } + let!(:dossier) { create(:dossier, :en_construction, user:, procedure:) } + let(:first_champ) { dossier.project_champs_public.first } + let(:first_champ_draft) { dossier.with_stream(Champ::USER_DRAFT_STREAM) { dossier.project_champs_public.first } } + let(:piece_justificative_champ) { dossier.project_champs_public.last } + let(:piece_justificative_champ_draft) { dossier.with_stream(Champ::USER_DRAFT_STREAM) { dossier.project_champs_public.last } } + let(:anchor_to_first_champ) { controller.helpers.link_to I18n.t('views.users.dossiers.fix_champ'), brouillon_dossier_path(anchor: first_champ.labelledby_id), class: 'error-anchor' } + let(:value) { 'beautiful value' } + let(:file) { fixture_file_upload('spec/fixtures/files/piece_justificative_0.pdf', 'application/pdf') } + let(:now) { Time.zone.parse('01/01/2100') } + + let(:submit_payload) do + { + id: dossier.id, + dossier: { + groupe_instructeur_id: dossier.groupe_instructeur_id, + champs_public_attributes: { + first_champ.public_id => { + value: value + }, + piece_justificative_champ.public_id => { + piece_justificative_file: file + } + } + } + } + end + let(:payload) { submit_payload } + + subject do + Timecop.freeze(now) do + patch :update, params: payload, format: :turbo_stream + end + end + + context 'when the dossier cannot be updated by the user' do + let!(:dossier) { create(:dossier, :en_instruction, user:, procedure:) } + + it 'redirects to the dossiers list' do + subject + expect(response).to redirect_to(dossier_path(dossier)) + expect(flash.alert).to eq('Votre dossier ne peut plus être modifié') + end + end + + context 'when dossier can be updated by the owner' do + it 'updates the champs' do + subject + dossier.reload + expect(dossier.user_draft_changes?).to be_truthy + expect(first_champ_draft.stream).to eq(Champ::USER_DRAFT_STREAM) + expect(first_champ_draft.value).to eq('beautiful value') + expect(piece_justificative_champ_draft.stream).to eq(Champ::USER_DRAFT_STREAM) + expect(piece_justificative_champ_draft.piece_justificative_file).to be_attached + end + + it 'updates the dossier timestamps' do + subject + dossier.reload + expect(dossier.updated_at).to eq(now) + expect(dossier.last_champ_updated_at).to be_nil + end + + it { is_expected.to have_http_status(:ok) } + + context 'when only a single file champ are modified' do + # A bug in ActiveRecord causes records changed through grand-parent <-> parent <-> child + # relationships do not touch the grand-parent record on change. + # This situation is hit when updating just the attachment of a champ (and not the + # champ itself). + # + # This test ensures that, whatever workaround we wrote for this, it still works properly. + # + # See https://github.com/rails/rails/issues/26726 + let(:submit_payload) do + { + id: dossier.id, + dossier: { + champs_public_attributes: { + piece_justificative_champ.public_id => { + piece_justificative_file: file + } + } + } + } + end + + it 'updates the dossier timestamps' do + subject + dossier.reload + expect(dossier.updated_at).to eq(now) + expect(dossier.last_champ_updated_at).to be_nil + end + end + end + + context 'when the update fails' do + render_views + + context 'classic error' do + before do + allow_any_instance_of(Dossier).to receive(:save).and_return(false) + allow_any_instance_of(Dossier).to receive(:errors).and_return( + [message: 'nop', inner_error: double(base: first_champ_draft)] + ) + subject + end + + it { expect(response).to render_template(:update) } + + it 'does not update the dossier timestamps' do + dossier.reload + expect(dossier.updated_at).not_to eq(now) + expect(dossier.last_champ_updated_at).to be_nil + end + end + + context 'iban error' do + let(:types_de_champ_public) { [{ type: :iban }] } + let(:value) { 'abc' } + + before { subject } + + it do + dossier.reload + expect(dossier.updated_at).to eq(now) + expect(dossier.last_champ_updated_at).to be_nil + expect(response).to have_http_status(:success) + end + end + end + + context 'when the user has an invitation but is not the owner' do + let(:dossier) { create(:dossier, :en_construction, procedure:) } + let!(:invite) { create(:invite, dossier:, user:) } + + before { subject } + + it do + dossier.reload + expect(first_champ_draft.value).to eq('beautiful value') + expect(response).to have_http_status(:ok) + end + end + + context 'when the dossier is followed by an instructeur' do + let(:instructeur) { create(:instructeur) } + let!(:invite) { create(:invite, dossier:, user:) } + + before do + instructeur.follow(dossier) + end + + it 'the follower has a notification' do + expect(instructeur.reload.followed_dossiers.with_notifications).to eq([]) + subject + expect(instructeur.reload.followed_dossiers.with_notifications).to eq([]) + end + end + + context 'when the champ is a phone number' do + let(:types_de_champ_public) { [{ type: :phone }] } + let(:now) { Time.zone.parse('01/01/2100') } + + let(:submit_payload) do + { + id: dossier.id, + dossier: { + champs_public_attributes: { + first_champ.public_id => { + value: value + } + } + } + } + end + + context 'with a valid value sent as string' do + let(:value) { '0612345678' } + it 'updates the value' do + subject + dossier.reload + expect(first_champ_draft.value).to eq('0612345678') + end + end + + context 'with a valid value sent as number' do + let(:value) { '45187272'.to_i } + it 'updates the value' do + subject + dossier.reload + expect(first_champ_draft.value).to eq('45187272') + end + end + end + end + describe '#index' do before { sign_in(user) } diff --git a/spec/views/shared/dossiers/_edit.html.haml_spec.rb b/spec/views/shared/dossiers/_edit.html.haml_spec.rb index 047d05c6a03..531a5817e75 100644 --- a/spec/views/shared/dossiers/_edit.html.haml_spec.rb +++ b/spec/views/shared/dossiers/_edit.html.haml_spec.rb @@ -124,6 +124,15 @@ end end + context 'when dossier is en construction (stream)' do + let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure:) } + let(:dossier_for_editing) { dossier.with_stream(Champ::USER_DRAFT_STREAM) } + + it 'can delete a piece justificative' do + expect(subject).to have_selector("[title='Supprimer le fichier #{champ.piece_justificative_file.attachments[0].filename}']") + end + end + context 'when dossier is brouillon' do it 'can delete a piece justificative' do expect(subject).to have_selector("[title='Supprimer le fichier #{champ.piece_justificative_file.attachments[0].filename}']")