diff --git a/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml b/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml index e87b3787a4c..4ead12827c4 100644 --- a/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml +++ b/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml @@ -3,7 +3,7 @@ %span.autosave-explanation-text - if annotation? = t('.annotations.explanation') - - elsif dossier.editing_fork? + - elsif dossier.en_construction_for_editor? = t('.en_construction.explanation') - if dossier.can_passer_en_construction? = t('.en_construction.submit_them') @@ -16,7 +16,7 @@ %span.fr-icon-success-fill.fr-mr-1v{ 'aria-hidden': 'true' } - if annotation? = t('.annotations.confirmation_html') - - elsif dossier.editing_fork? + - elsif dossier.en_construction_for_editor? = t('.en_construction.confirmation_html') - else = t('.brouillon.confirmation_html') @@ -27,7 +27,7 @@ %span.fr-icon-warning-fill.fr-mr-1v{ 'aria-hidden': 'true' } - if annotation? = t('.annotations.error_html') - - elsif dossier.editing_fork? + - elsif dossier.en_construction_for_editor? = t('.en_construction.error_html') - else = t('.brouillon.error_html') diff --git a/app/components/dossiers/edit_footer_component.rb b/app/components/dossiers/edit_footer_component.rb index 4ac97ef7939..01cafeddd2a 100644 --- a/app/components/dossiers/edit_footer_component.rb +++ b/app/components/dossiers/edit_footer_component.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Dossiers::EditFooterComponent < ApplicationComponent - delegate :can_passer_en_construction?, :can_transition_to_en_construction?, :forked_with_changes?, to: :@dossier + delegate :can_passer_en_construction?, :can_transition_to_en_construction?, :user_buffer_changes?, to: :@dossier def initialize(dossier:, annotation:) @dossier = dossier @@ -27,7 +27,7 @@ def can_submit_draft? end def can_submit_en_construction? - forked_with_changes? && owner? + owner? && user_buffer_changes? end def submit_button_label @@ -41,8 +41,10 @@ def submit_button_label def submit_button_path if can_submit_draft? brouillon_dossier_path(@dossier) - else + elsif @dossier.editing_fork? modifier_dossier_path(@dossier.editing_fork_origin) + else + modifier_dossier_path(@dossier) end end diff --git a/app/components/dossiers/en_construction_not_submitted_component.rb b/app/components/dossiers/en_construction_not_submitted_component.rb index 5d40b7c9a2d..8ee0d9e3b82 100644 --- a/app/components/dossiers/en_construction_not_submitted_component.rb +++ b/app/components/dossiers/en_construction_not_submitted_component.rb @@ -7,11 +7,9 @@ class Dossiers::EnConstructionNotSubmittedComponent < ApplicationComponent def initialize(dossier:, user:) @dossier = dossier @user = user - - @fork = @dossier.find_editing_fork(user, rebase: false) end def render? - @fork&.forked_with_changes? + @dossier.user_buffer_changes? end end diff --git a/app/components/editable_champ/champ_label_content_component/champ_label_content_component.html.haml b/app/components/editable_champ/champ_label_content_component/champ_label_content_component.html.haml index b4dbc315aa4..d0385350707 100644 --- a/app/components/editable_champ/champ_label_content_component/champ_label_content_component.html.haml +++ b/app/components/editable_champ/champ_label_content_component/champ_label_content_component.html.haml @@ -3,7 +3,7 @@ - if @champ.mandatory? = render EditableChamp::AsteriskMandatoryComponent.new -- if @champ.forked_with_changes? +- if @champ.user_buffer_changes? %span.updated-at.highlighted = t('.changes_to_save') - elsif @champ.updated_at.present? && @seen_at.present? diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index c66e3cbf98d..f5b27ab8e75 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -23,7 +23,10 @@ def destroy if champ? @attachment = champ.piece_justificative_file.find { _1.blob.id == @blob.id } - @attachment&.purge_later + if @attachment.present? + @attachment.purge_later + champ.update_timestamps + end champ.piece_justificative_file.reload else @attachment.purge_later @@ -50,6 +53,7 @@ def champ? def champ @champ ||= if champ? + record.dossier.with_update_stream(current_user) 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/carte_controller.rb b/app/controllers/champs/carte_controller.rb index 33a4aeb6ff6..ddcd8ae131f 100644 --- a/app/controllers/champs/carte_controller.rb +++ b/app/controllers/champs/carte_controller.rb @@ -14,6 +14,7 @@ def create geo_area = @champ.geo_areas.build(source: params_source, properties: {}) if save_feature(geo_area, create_params_feature) + @champ.update_timestamps FetchCadastreRealGeometryJob.perform_later(geo_area) if geo_area.cadastre? render json: { feature: geo_area.to_feature }, status: :created else @@ -28,6 +29,7 @@ def update geo_area = @champ.geo_areas.find(params[:id]) if save_feature(geo_area, update_params_feature) + @champ.update_timestamps FetchCadastreRealGeometryJob.perform_later(geo_area) if geo_area.cadastre? head :no_content else @@ -37,7 +39,7 @@ def update def destroy @champ.geo_areas.find(params[:id]).destroy! - propagate_touch_champs_changed + @champ.update_timestamps head :no_content end @@ -81,9 +83,6 @@ def save_feature(geo_area, feature) if feature[:properties] geo_area.properties.merge!(feature[:properties]) end - if geo_area.save - propagate_touch_champs_changed - true - end + geo_area.save end end diff --git a/app/controllers/champs/champ_controller.rb b/app/controllers/champs/champ_controller.rb index c0d20ceab24..ded73040c1a 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_update_stream(current_user) 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) @@ -23,9 +24,4 @@ def params_row_id def set_champ @champ = find_champ end - - def propagate_touch_champs_changed - @champ.touch - @champ.dossier.touch_champs_changed([:last_champ_updated_at]) - end end diff --git a/app/controllers/champs/piece_justificative_controller.rb b/app/controllers/champs/piece_justificative_controller.rb index ed6eeda5117..ec2b34490cf 100644 --- a/app/controllers/champs/piece_justificative_controller.rb +++ b/app/controllers/champs/piece_justificative_controller.rb @@ -32,7 +32,7 @@ def attach_piece_justificative end if save_succeed && dossier.brouillon? - dossier.touch_champs_changed([:last_champ_updated_at, :last_champ_piece_jointe_updated_at]) + @champ.update_timestamps end save_succeed diff --git a/app/controllers/champs/repetition_controller.rb b/app/controllers/champs/repetition_controller.rb index f2d3e32ccd2..1b17dacd982 100644 --- a/app/controllers/champs/repetition_controller.rb +++ b/app/controllers/champs/repetition_controller.rb @@ -5,14 +5,14 @@ def add @row_id = @champ.add_row(updated_by: current_user.email) @first_champ_id = @champ.focusable_input_id @row_number = @row_id.nil? ? 0 : @champ.row_ids.find_index(@row_id) + 1 - @champ.dossier.touch_champs_changed([:last_champ_updated_at]) + @champ.update_timestamps end def remove @champ.remove_row(params[:row_id], updated_by: current_user.email) @to_remove = "safe-row-selector-#{params[:row_id]}" @to_focus = @champ.focusable_input_id || helpers.dom_id(@champ, :create_repetition) - @champ.dossier.touch_champs_changed([:last_champ_updated_at]) + @champ.update_timestamps end private diff --git a/app/controllers/champs/rna_controller.rb b/app/controllers/champs/rna_controller.rb index 78431d67b21..261b910908f 100644 --- a/app/controllers/champs/rna_controller.rb +++ b/app/controllers/champs/rna_controller.rb @@ -8,6 +8,6 @@ def show unless @champ.fetch_association!(rna) @error = @champ.association_fetch_error_key end - @champ.dossier.touch_champs_changed([:last_champ_updated_at]) + @champ.update_timestamps end end diff --git a/app/controllers/champs/siret_controller.rb b/app/controllers/champs/siret_controller.rb index 4657c1a09d2..dcac7f1e9ab 100644 --- a/app/controllers/champs/siret_controller.rb +++ b/app/controllers/champs/siret_controller.rb @@ -11,6 +11,6 @@ def show # Anyway it would be clear when updating the value without validation @champ.validate(params[:validate].to_sym) if params[:validate] - @champ.dossier.touch_champs_changed([:last_champ_updated_at]) + @champ.update_timestamps end end diff --git a/app/controllers/concerns/turbo_champs_concern.rb b/app/controllers/concerns/turbo_champs_concern.rb index c132de46e5a..23140983568 100644 --- a/app/controllers/concerns/turbo_champs_concern.rb +++ b/app/controllers/concerns/turbo_champs_concern.rb @@ -7,7 +7,7 @@ module TurboChampsConcern def champs_to_turbo_update(params, champs) to_update = champs.filter { _1.public_id.in?(params.keys) } - .filter { _1.refresh_after_update? || _1.forked_with_changes? } + .filter { _1.refresh_after_update? || _1.user_buffer_changes? } to_show, to_hide = champs.filter(&:conditional?) .partition(&:visible?) diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index 5364edbdebb..adc0640e349 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -299,13 +299,17 @@ def create_avis end def update_annotations - dossier_with_champs.update_champs_attributes(champs_private_attributes_params, :private, updated_by: current_user.email) - if dossier.champs.any?(&:changed_for_autosave?) - dossier.touch(:last_champ_private_updated_at) + public_id, annotation_attributes = champs_private_attributes_params.to_h.first + annotation = dossier.private_champ_for_update(public_id, updated_by: current_user.email) + annotation.assign_attributes(annotation_attributes) + annotation_changed = annotation.changed_for_autosave? + + if annotation.save(context: :champs_private_value) && annotation_changed + annotation.update_timestamps + dossier.index_search_terms_later end - dossier.save(context: :champs_private_value) - dossier.index_search_terms_later + dossier.validate(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 fa3c0af5740..a20c10a7629 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 :ensure_editing_brouillon, only: [: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,34 +254,51 @@ 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 + DossierPreloader.load_one(@dossier_for_editing) + 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) - # merge_fork do a `reload`, the preloader is used to reload the whole tree - editing_fork_origin = DossierPreloader.load_one(editing_fork_origin) - 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) + # merge_fork do a `reload`, the preloader is used to reload the whole tree + DossierPreloader.load_one(editing_fork_origin) + else + dossier.merge_user_buffer_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 = update_with_fork? ? dossier.find_editing_fork(dossier.user) : dossier @dossier = dossier_with_champs(pj_template: false) update_dossier_and_compute_errors @@ -538,23 +556,37 @@ def set_dossier_as_editing_fork redirect_to dossier_path(dossier) end + def set_dossier_stream + dossier.with_update_stream(current_user) + end + + def update_with_stream? + dossier.update_with_stream? + end + + def update_with_fork? + dossier.update_with_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?) + public_id, champ_attributes = champs_public_attributes_params.to_h.first + champ = dossier.public_champ_for_update(public_id, updated_by: current_user.email) + champ.assign_attributes(champ_attributes) + champ_changed = champ.changed_for_autosave? # 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 - if dossier.brouillon? && updated_champs.present? - dossier.touch_champs_changed([:last_champ_updated_at]) - if updated_champs.any?(&:used_by_routing_rules?) + if Dossier.no_touching { champ.save } + if dossier.brouillon? && champ_changed + champ.update_timestamps + if champ.used_by_routing_rules? @update_contact_information = true RoutingEngine.compute(dossier) end end if params[:validate].present? - dossier.valid?(:champs_public_value) + dossier.validate(:champs_public_value) end end end @@ -563,6 +595,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/champ.rb b/app/models/champ.rb index 6871796f2bf..b53c4f63571 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -242,16 +242,12 @@ def focusable_input_id input_id end - def forked_with_changes? - public? && dossier.champ_forked_with_changes?(self) + def user_buffer_changes? + public? && dossier.user_buffer_changes_on_champ?(self) end def public_id - if row_id.blank? - stable_id.to_s - else - "#{stable_id}-#{row_id}" - end + TypeDeChamp.public_id(stable_id, row_id) end def html_id @@ -278,6 +274,61 @@ def normalize write_attribute(:value, value.delete("\u0000")) end + MAIN_STREAM = 'main' + USER_BUFFER_STREAM = 'user:buffer' + HISTORY_STREAM = 'history:' + + def main_stream? + stream == MAIN_STREAM + end + + def user_buffer_stream? + stream == USER_BUFFER_STREAM + end + + def history_stream? + stream.start_with?(HISTORY_STREAM) + end + + def clone_value_from(champ) + self.value = champ.value + self.external_id = champ.external_id + self.value_json = champ.value_json + self.data = champ.data + + self.geo_areas = champ.geo_areas.dup + + ClonePiecesJustificativesService.clone_attachments(champ, self) + + if champ.etablissement.present? + self.etablissement = champ.etablissement.dup + ClonePiecesJustificativesService.clone_attachments(champ.etablissement, self.etablissement) + end + + save! + end + + def update_timestamps + return if public? && dossier.en_construction? + + updated_at = Time.zone.now + attributes = { updated_at: } + update_columns(attributes) if persisted? + + if piece_justificative_or_titre_identite? + attributes[:last_champ_piece_jointe_updated_at] = updated_at + end + + if private? + attributes[:last_champ_private_updated_at] = updated_at + else + attributes[:last_champ_updated_at] = updated_at + attributes[:brouillon_close_to_expiration_notice_sent_at] = nil + end + + dossier.update_columns(attributes) + end + class NotImplemented < ::StandardError def initialize(method) super(":#{method} not implemented") diff --git a/app/models/concerns/champ_validate_concern.rb b/app/models/concerns/champ_validate_concern.rb index ab8472a775b..e58312e566e 100644 --- a/app/models/concerns/champ_validate_concern.rb +++ b/app/models/concerns/champ_validate_concern.rb @@ -27,7 +27,11 @@ def validate_champ_value? end def can_validate? - in_dossier_revision? && is_same_type_as_revision? && !row? && !in_discarded_row? + in_dossier_stream? && in_dossier_revision? && is_same_type_as_revision? && !row? && !in_discarded_row? + end + + def in_dossier_stream? + dossier.stream == stream end end end diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb index 5b1817d5af1..bcea8083789 100644 --- a/app/models/concerns/dossier_champs_concern.rb +++ b/app/models/concerns/dossier_champs_concern.rb @@ -10,7 +10,7 @@ def project_champ(type_de_champ, row_id: nil) value = type_de_champ.champ_blank?(champ) ? nil : champ.value updated_at = champ&.updated_at || depose_at || created_at rebased_at = champ&.rebased_at - type_de_champ.build_champ(dossier: self, row_id:, updated_at:, rebased_at:, value:) + type_de_champ.build_champ(dossier: self, row_id:, updated_at:, rebased_at:, value:, stream:) else champ end @@ -119,12 +119,16 @@ def champ_for_update(type_de_champ, row_id: nil, updated_by:) champ end - def update_champs_attributes(attributes, scope, updated_by:) - champs_attributes = attributes.to_h.map do |public_id, attributes| - champ_attributes_by_public_id(public_id, attributes, scope, updated_by:) - end + def public_champ_for_update(public_id, updated_by:) + stable_id, row_id = public_id.split('-') + type_de_champ = find_type_de_champ_by_stable_id(stable_id, :public) + champ_for_update(type_de_champ, row_id:, updated_by:) + end - assign_attributes(champs_attributes:) + def private_champ_for_update(public_id, updated_by:) + stable_id, row_id = public_id.split('-') + type_de_champ = find_type_de_champ_by_stable_id(stable_id, :private) + champ_for_update(type_de_champ, row_id:, updated_by:) end def repetition_rows_for_export(type_de_champ) @@ -136,7 +140,7 @@ def repetition_rows_for_export(type_de_champ) def repetition_row_ids(type_de_champ) return [] if !type_de_champ.repetition? @repetition_row_ids ||= {} - @repetition_row_ids[type_de_champ.stable_id] ||= champs_in_revision + @repetition_row_ids[type_de_champ.stable_id] ||= champs_on_stream .filter { _1.row? && _1.stable_id == type_de_champ.stable_id && !_1.discarded? } .map(&:row_id) .sort @@ -165,10 +169,124 @@ def reload super.tap { reset_champs_cache } end + def merge_user_buffer_stream! + buffer_champ_ids_h = champs.where(stream: Champ::USER_BUFFER_STREAM, stable_id: revision_stable_ids) + .pluck(:id, :stable_id, :row_id) + .index_by { |(_, stable_id, row_id)| TypeDeChamp.public_id(stable_id, row_id) } + .transform_values(&:first) + + return if buffer_champ_ids_h.empty? + + changed_main_champ_ids_h = champs.where(stream: Champ::MAIN_STREAM, stable_id: revision_stable_ids) + .pluck(:id, :stable_id, :row_id) + .index_by { |(_, stable_id, row_id)| TypeDeChamp.public_id(stable_id, row_id) } + .transform_values(&:first) + + buffer_champ_ids = buffer_champ_ids_h.values + changed_main_champ_ids = changed_main_champ_ids_h.filter_map { |public_id, id| id if buffer_champ_ids_h.key?(public_id) } + + now = Time.zone.now + history_stream = "#{Champ::HISTORY_STREAM}#{now}" + changed_champs = champs.filter { _1.id.in?(buffer_champ_ids) } + + transaction do + champs.where(id: changed_main_champ_ids, stream: Champ::MAIN_STREAM).update_all(stream: history_stream) + champs.where(id: buffer_champ_ids, stream: Champ::USER_BUFFER_STREAM).update_all(stream: Champ::MAIN_STREAM, updated_at: now) + update_champs_timestamps(changed_champs) + end + + # update loaded champ instances + champs.each do |champ| + if champ.id.in?(changed_main_champ_ids) + champ.stream = history_stream + elsif champ.id.in?(buffer_champ_ids) + champ.stream = Champ::MAIN_STREAM + end + end + + reset_champs_cache + end + + def reset_user_buffer_stream! + champs.where(stream: Champ::USER_BUFFER_STREAM).delete_all + + # update loaded champ instances + association(:champs).target = champs.filter { _1.stream != Champ::USER_BUFFER_STREAM } + + reset_champs_cache + end + + def user_buffer_changes? + # TODO remove when all forks are gone + return true if forked_with_changes? + + champs_on_user_buffer_stream.present? + end + + def user_buffer_changes_on_champ?(champ) + # TODO remove when all forks are gone + return true if champ_forked_with_changes?(champ) + + champs_on_user_buffer_stream.any? { _1.public_id == champ.public_id } + end + + def with_update_stream(user, &block) + if update_with_stream? && user.owns_or_invite?(self) + with_stream(Champ::USER_BUFFER_STREAM, &block) + else + with_stream(Champ::MAIN_STREAM, &block) + end + end + + def with_main_stream(&block) + with_stream(Champ::MAIN_STREAM, &block) + end + + def stream + @stream || Champ::MAIN_STREAM + end + + def history + champs_in_revision.filter(&:history_stream?) + end + + def update_with_stream? + en_construction? && procedure.feature_enabled?(:user_buffer_stream) && !with_editing_fork? + end + + def update_with_fork? + en_construction? && !procedure.feature_enabled?(:user_buffer_stream) + end + private + def with_stream(stream) + if block_given? + previous_stream = @stream + @stream = stream + reset_champs_cache + result = yield + @stream = previous_stream + reset_champs_cache + result + else + @stream = stream + reset_champs_cache + self + end + end + def champs_by_public_id - @champs_by_public_id ||= champs_in_revision.index_by(&:public_id) + @champs_by_public_id ||= champs_on_stream.index_by(&:public_id) + end + + def champs_on_stream + @champs_on_stream ||= case stream + when Champ::USER_BUFFER_STREAM + (champs_on_user_buffer_stream + champs_on_main_stream).uniq(&:public_id) + else + champs_on_main_stream + end end def revision_stable_ids @@ -179,6 +297,14 @@ def champs_in_revision champs.filter { stable_id_in_revision?(_1.stable_id) } end + def champs_on_main_stream + champs_in_revision.filter(&:main_stream?) + end + + def champs_on_user_buffer_stream + champs_in_revision.filter(&:user_buffer_stream?) + end + def filled_champ(type_de_champ, row_id: nil) champ = champs_by_public_id[type_de_champ.public_id(row_id)] if type_de_champ.champ_blank?(champ) || !champ.visible? @@ -188,26 +314,24 @@ def filled_champ(type_de_champ, row_id: nil) end end - def champ_attributes_by_public_id(public_id, attributes, scope, updated_by:) - stable_id, row_id = public_id.split('-') - type_de_champ = find_type_de_champ_by_stable_id(stable_id, scope) - champ = champ_upsert_by!(type_de_champ, row_id) - attributes.merge(id: champ.id, updated_by:) - end - def champ_upsert_by!(type_de_champ, row_id) check_valid_row_id_on_write?(type_de_champ, row_id) + row_id_or_null = row_id || Champ::NULL_ROW_ID + champ = Dossier.no_touching do champs .create_with(**type_de_champ.params_for_champ) - .create_or_find_by!(stable_id: type_de_champ.stable_id, row_id: row_id || Champ::NULL_ROW_ID, stream: 'main') + .create_or_find_by!(stable_id: type_de_champ.stable_id, row_id: row_id_or_null, stream:) end # Needed when a revision change the champ type in this case, we reset the champ data if !champ.is_a?(type_de_champ.champ_class) champ = champ.becomes!(type_de_champ.champ_class) champ.assign_attributes(value: nil, value_json: nil, external_id: nil, data: nil) + elsif stream != Champ::MAIN_STREAM && champ.previously_new_record? + main_stream_champ = champs.find_by(stable_id: type_de_champ.stable_id, row_id: row_id_or_null, stream: Champ::MAIN_STREAM) + champ.clone_value_from(main_stream_champ) if main_stream_champ.present? end # If the champ returned from `create_or_find_by` is not the same as the one already loaded in `dossier.champs`, we need to update the association cache @@ -255,5 +379,6 @@ def reset_champs_cache @project_champs_private = nil @repetition_row_ids = nil @revision_stable_ids = nil + @champs_on_stream = nil end end diff --git a/app/models/concerns/dossier_clone_concern.rb b/app/models/concerns/dossier_clone_concern.rb index b538eae2637..c4572b0ebc2 100644 --- a/app/models/concerns/dossier_clone_concern.rb +++ b/app/models/concerns/dossier_clone_concern.rb @@ -37,16 +37,13 @@ def editing_fork? editing_fork_origin_id.present? end - def forked_with_changes? - return false if forked_diff.blank? - - forked_diff.values.any?(&:present?) || forked_groupe_instructeur_changed? + def with_editing_fork? + editing_forks.present? end - def champ_forked_with_changes?(champ) - return false if forked_diff.blank? - - forked_diff.values.any? { |champs| champs.any? { _1.public_id == champ.public_id } } + # TODO remove when all forks are gone + def en_construction_for_editor? + en_construction? || editing_fork? end def make_diff(editing_fork) @@ -77,10 +74,8 @@ def merge_fork(editing_fork) diff = make_diff(editing_fork) apply_diff(diff) - attributes_to_touch = [:last_champ_updated_at] - attributes_to_touch << :last_champ_piece_jointe_updated_at if diff[:updated].any? { |c| c.class.in?([Champs::PieceJustificativeChamp, Champs::TitreIdentiteChamp]) } - - touch_champs_changed(attributes_to_touch) + changed_champs = diff.values.flatten + update_champs_timestamps(changed_champs) end reload index_search_terms_later @@ -92,10 +87,10 @@ def clone(user: nil, fork: false) dossier_attributes += [:groupe_instructeur_id] if fork relationships = [:individual, :etablissement] - discarded_row_ids = champs_in_revision + discarded_row_ids = champs_on_main_stream .filter { _1.row? && _1.discarded? } .to_set(&:row_id) - cloned_champs = champs_in_revision + cloned_champs = champs_on_main_stream .reject { discarded_row_ids.member?(_1.row_id) } .index_by(&:id) .transform_values { [_1, _1.clone(fork)] } @@ -139,6 +134,24 @@ def clone(user: nil, fork: false) cloned_dossier.reload end + protected + + def forked_with_changes? + if with_editing_fork? + find_editing_fork(user, rebase: false)&.forked_with_changes? + elsif forked_diff.present? + forked_diff.values.any?(&:present?) || forked_groupe_instructeur_changed? + end + end + + def champ_forked_with_changes?(champ) + if with_editing_fork? + find_editing_fork(user, rebase: false)&.champ_forked_with_changes?(champ) + elsif forked_diff.present? + forked_diff.values.any? { |champs| champs.any? { _1.public_id == champ.public_id } } + end + end + private def forked_diff diff --git a/app/models/concerns/dossier_state_concern.rb b/app/models/concerns/dossier_state_concern.rb index 8401d0bb93f..5f70d4e74c2 100644 --- a/app/models/concerns/dossier_state_concern.rb +++ b/app/models/concerns/dossier_state_concern.rb @@ -54,6 +54,7 @@ def after_passer_en_instruction(h) .processed_at save! + reset_user_buffer_stream! MailTemplatePresenterService.create_commentaire_for_state(self, Dossier.states.fetch(:en_instruction)) resolve_pending_correction! @@ -68,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 @@ -333,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/app/models/dossier.rb b/app/models/dossier.rb index 303938d973d..8344ef86c08 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -1037,11 +1037,14 @@ def termine_and_accuse_lecture? procedure.accuse_lecture? && termine? end - def touch_champs_changed(attributes) - now = Time.zone.now - update_columns(attributes.each_with_object({ brouillon_close_to_expiration_notice_sent_at: nil, updated_at: now }) do |attribute, hash| - hash[attribute] = now - end) + def update_champs_timestamps(changed_champs) + return if changed_champs.empty? + updated_at = Time.zone.now + attributes = { updated_at:, last_champ_updated_at: updated_at } + if changed_champs.any?(&:piece_justificative_or_titre_identite?) + attributes[:last_champ_piece_jointe_updated_at] = updated_at + end + update_columns(attributes) end private diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index 6586fdbc035..97ee9bd72a8 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -255,7 +255,7 @@ def params_for_champ private: private?, type: champ_class.name, stable_id:, - stream: 'main' + stream: Champ::MAIN_STREAM } end @@ -590,11 +590,7 @@ def invalid_regexp? end def public_id(row_id) - if row_id.blank? - stable_id.to_s - else - "#{stable_id}-#{row_id}" - end + self.class.public_id(stable_id, row_id) end def libelle_as_filename @@ -680,6 +676,14 @@ def html_id(row_id = nil) end class << self + def public_id(stable_id, row_id) + if row_id.blank? || row_id == Champ::NULL_ROW_ID + stable_id.to_s + else + "#{stable_id}-#{row_id}" + end + end + def type_champ_to_champ_class_name(type_champ) "Champs::#{type_champ.classify}Champ" end diff --git a/app/views/champs/carte/index.turbo_stream.haml b/app/views/champs/carte/index.turbo_stream.haml index 55694596aeb..ca4a3c35107 100644 --- a/app/views/champs/carte/index.turbo_stream.haml +++ b/app/views/champs/carte/index.turbo_stream.haml @@ -4,6 +4,6 @@ - if @focus = turbo_stream.dispatch 'map:feature:focus', bbox: @champ.bounding_box -- if @champ.dossier.editing_fork? +- if @champ.dossier.en_construction_for_editor? = turbo_stream.replace_all '.dossier-edit-sticky-footer' do = render Dossiers::EditFooterComponent.new(dossier: @champ.dossier, annotation: @champ.private?) diff --git a/app/views/champs/piece_justificative/show.turbo_stream.haml b/app/views/champs/piece_justificative/show.turbo_stream.haml index b07549e9cd4..a451076b5d5 100644 --- a/app/views/champs/piece_justificative/show.turbo_stream.haml +++ b/app/views/champs/piece_justificative/show.turbo_stream.haml @@ -6,6 +6,6 @@ - if last_attached_file = turbo_stream.focus_all "#persisted_row_attachment_#{last_attached_file.id} .attachment-filename a" -- if @champ.dossier.editing_fork? +- if @champ.dossier.en_construction_for_editor? = turbo_stream.replace_all '.dossier-edit-sticky-footer' do = render Dossiers::EditFooterComponent.new(dossier: @champ.dossier, annotation: @champ.private?) diff --git a/spec/components/dossiers/edit_footer_component_spec.rb b/spec/components/dossiers/edit_footer_component_spec.rb index 5eb01c1648d..fb97b70dc21 100644 --- a/spec/components/dossiers/edit_footer_component_spec.rb +++ b/spec/components/dossiers/edit_footer_component_spec.rb @@ -30,7 +30,7 @@ context 'when en construction' do let(:fork_origin) { create(:dossier, :en_construction) } let(:dossier) { fork_origin.clone(fork: true) } - before { allow(dossier).to receive(:forked_with_changes?).and_return(true) } + before { allow(dossier).to receive(:user_buffer_changes?).and_return(true) } context 'when dossier can be submitted' do before { allow(component).to receive(:can_passer_en_construction?).and_return(true) } diff --git a/spec/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb index e5449451a37..cdd39fb9fb8 100644 --- a/spec/controllers/instructeurs/dossiers_controller_spec.rb +++ b/spec/controllers/instructeurs/dossiers_controller_spec.rb @@ -1089,46 +1089,99 @@ after do Timecop.return end + let(:champs_private_attributes) do + { + champ_multiple_drop_down_list.public_id => { + value: ['', 'val1', 'val2'] + } + } + end let(:params) do { procedure_id: procedure.id, dossier_id: dossier.id, - dossier: { - champs_private_attributes: { - champ_multiple_drop_down_list.public_id => { - value: ['', 'val1', 'val2'] - }, - champ_datetime.public_id => { - value: '2019-12-21T13:17' - }, - champ_linked_drop_down_list.public_id => { - primary_value: 'primary', - secondary_value: 'secondary' - }, - champ_repetition.rows.first.first.public_id => { - value: 'text' - }, - champ_drop_down_list.public_id => { - value: '__other__', - value_other: 'other value' - } - } - } + dossier: { champs_private_attributes: } } end it { expect(champ_multiple_drop_down_list.value).to eq('["val1","val2"]') - expect(champ_linked_drop_down_list.primary_value).to eq('primary') - expect(champ_linked_drop_down_list.secondary_value).to eq('secondary') - expect(champ_datetime.value).to eq(Time.zone.parse('2019-12-21T13:17:00').iso8601) - expect(champ_text.value).to eq('text') - expect(champ_drop_down_list.value).to eq('other value') expect(dossier.last_champ_private_updated_at).to eq(now) expect(response).to have_http_status(200) assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob) } + context 'datetime' do + let(:champs_private_attributes) do + { + champ_datetime.public_id => { + value: '2019-12-21T13:17' + } + } + end + + it { + expect(champ_datetime.value).to eq(Time.zone.parse('2019-12-21T13:17:00').iso8601) + expect(dossier.last_champ_private_updated_at).to eq(now) + expect(response).to have_http_status(200) + assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob) + } + end + + context 'linked_drop_down' do + let(:champs_private_attributes) do + { + champ_linked_drop_down_list.public_id => { + primary_value: 'primary', + secondary_value: 'secondary' + } + } + end + + it { + expect(champ_linked_drop_down_list.primary_value).to eq('primary') + expect(champ_linked_drop_down_list.secondary_value).to eq('secondary') + expect(dossier.last_champ_private_updated_at).to eq(now) + expect(response).to have_http_status(200) + assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob) + } + end + + context 'repetition' do + let(:champs_private_attributes) do + { + champ_repetition.rows.first.first.public_id => { + value: 'text' + } + } + end + + it { + expect(champ_text.value).to eq('text') + expect(dossier.last_champ_private_updated_at).to eq(now) + expect(response).to have_http_status(200) + assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob) + } + end + + context 'drop_down_list' do + let(:champs_private_attributes) do + { + champ_drop_down_list.public_id => { + value: '__other__', + value_other: 'other value' + } + } + end + + it { + expect(champ_drop_down_list.value).to eq('other value') + expect(dossier.last_champ_private_updated_at).to eq(now) + expect(response).to have_http_status(200) + assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob) + } + end + it 'updates the annotations' do Timecop.travel(now + 1.hour) expect(instructeur.followed_dossiers.with_notifications).to eq([]) diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 0a5f9f8086b..33c8b702610 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -594,10 +594,7 @@ end context 'when dossier was already submitted' do - before do - expect_any_instance_of(Dossier).to receive(:remove_not_visible_or_empty_champs!) - post :submit_en_construction, params: payload - end + before { post :submit_en_construction, params: payload } it 'redirects to the dossier' do subject @@ -684,17 +681,12 @@ 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 - } - } - } + dossier: { champs_public_attributes: } + } + end + let(:champs_public_attributes) do + { + first_champ.public_id => { value: value } } end let(:payload) { submit_payload } @@ -750,11 +742,56 @@ 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)) expect(dossier.reload.brouillon_close_to_expiration_notice_sent_at).to be_nil + expect(first_champ.reload.value).to eq('beautiful value') + end + + context 'updates the pj' do + let(:champs_public_attributes) do + { + piece_justificative_champ.public_id => { piece_justificative_file: file } + } + end + + it do + subject + expect(piece_justificative_champ.reload.piece_justificative_file).to be_attached + end + end + + it 'updates the dossier timestamps' do + subject + dossier.reload + expect(dossier.updated_at).to eq(now) + expect(dossier.last_champ_updated_at).to eq(now) + 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(:champs_public_attributes) do + { + 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 eq(now) + end end end @@ -774,7 +811,7 @@ { id: dossier.id, dossier: { - champs_public_attributes: { first_champ.public_id => { value: value } } + champs_public_attributes: { first_champ.public_id => { value: } } } } end @@ -811,16 +848,8 @@ id: dossier.id, validate:, 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: - } + number_champ.public_id => { value: } } } } @@ -888,17 +917,12 @@ 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 - } - } - } + dossier: { champs_public_attributes: } + } + end + let(:champs_public_attributes) do + { + first_champ.public_id => { value: value } } end let(:payload) { submit_payload } @@ -923,7 +947,19 @@ it 'updates the champs' do subject expect(first_champ.reload.value).to eq('beautiful value') - expect(piece_justificative_champ.reload.piece_justificative_file).to be_attached + end + + context 'updates the pj' do + let(:champs_public_attributes) do + { + piece_justificative_champ.public_id => { piece_justificative_file: file } + } + end + + it do + subject + expect(piece_justificative_champ.reload.piece_justificative_file).to be_attached + end end it 'updates the dossier timestamps' do @@ -1013,8 +1049,201 @@ it { expect(response).to have_http_status(:ok) } 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 + expect(first_champ.reload.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 + expect(first_champ.reload.value).to eq('45187272') + 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_buffer_stream, _1) } } + let!(:dossier) { create(:dossier, :en_construction, user:, procedure:) } + let(:first_champ) { dossier.project_champs_public.first } + let(:first_champ_user_buffer) { dossier.with_update_stream(dossier.user) { dossier.project_champs_public.first } } + let(:piece_justificative_champ) { dossier.project_champs_public.last } + let(:piece_justificative_champ_user_buffer) { dossier.with_update_stream(dossier.user) { 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: { champs_public_attributes: } + } + end + let(:champs_public_attributes) do + { + first_champ.public_id => { value: value } + } + 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_buffer_changes?).to be_truthy + expect(first_champ_user_buffer.stream).to eq(Champ::USER_BUFFER_STREAM) + expect(first_champ_user_buffer.value).to eq('beautiful value') + expect(first_champ_user_buffer.updated_at).to eq(now) + end + + context 'updates the pj' do + let(:champs_public_attributes) do + { + piece_justificative_champ.public_id => { piece_justificative_file: file } + } + end + + it do + subject + dossier.reload + expect(dossier.user_buffer_changes?).to be_truthy + expect(piece_justificative_champ_user_buffer.stream).to eq(Champ::USER_BUFFER_STREAM) + expect(piece_justificative_champ_user_buffer.piece_justificative_file).to be_attached + end + end + + it 'does not update the dossier timestamps' do + subject + dossier.reload + expect(dossier.updated_at).not_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 'does not update the dossier timestamps' do + subject + dossier.reload + expect(dossier.updated_at).not_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_user_buffer)] + ) + 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 '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 + 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_user_buffer.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(:dossier) { create(:dossier, procedure:) } let(:instructeur) { create(:instructeur) } let!(:invite) { create(:invite, dossier:, user:) } @@ -1025,7 +1254,7 @@ 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([dossier.reload]) + expect(instructeur.reload.followed_dossiers.with_notifications).to eq([]) end end @@ -1050,7 +1279,8 @@ let(:value) { '0612345678' } it 'updates the value' do subject - expect(first_champ.reload.value).to eq('0612345678') + dossier.reload + expect(first_champ_user_buffer.value).to eq('0612345678') end end @@ -1058,7 +1288,8 @@ let(:value) { '45187272'.to_i } it 'updates the value' do subject - expect(first_champ.reload.value).to eq('45187272') + dossier.reload + expect(first_champ_user_buffer.value).to eq('45187272') end end end diff --git a/spec/models/champs/siret_champ_spec.rb b/spec/models/champs/siret_champ_spec.rb index 4f61989b797..9e1e88560ec 100644 --- a/spec/models/champs/siret_champ_spec.rb +++ b/spec/models/champs/siret_champ_spec.rb @@ -1,46 +1,42 @@ # frozen_string_literal: true describe Champs::SiretChamp do - let(:champ) { Champs::SiretChamp.new(value: "", dossier: build(:dossier)) } - before do - allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_siret)) - allow(champ).to receive(:in_dossier_revision?).and_return(true) - end - - def with_value(value) - champ.tap { _1.value = value } - end + let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :siret }]) } + let(:dossier) { create(:dossier, procedure:) } + let(:champ) { dossier.champs.first.tap { _1.update(value:, etablissement:) } } + let(:value) { "" } + let(:etablissement) { nil } describe '#validate' do subject { champ.tap { _1.validate(:champs_public_value) } } context 'when empty' do - it { expect(with_value(nil)).to be_valid } + let(:value) { nil } + + it { is_expected.to be_valid } end context 'with invalid format' do - before { with_value('12345') } + let(:value) { "12345" } it { subject.errors[:value].should include('doit comporter exactement 14 chiffres') } end context 'with invalid checksum' do - before { with_value('12345678901234') } + let(:value) { "12345678901234" } it { subject.errors[:value].should include("n‘est pas valide") } end context 'with valid format but no etablissement' do - before { with_value('12345678901245') } + let(:value) { "12345678901245" } it { subject.errors[:value].should include("aucun établissement n’est rattaché à ce numéro") } end context 'with valid SIRET and etablissement' do - before do - with_value('12345678901245') - champ.etablissement = build(:etablissement, siret: champ.value) - end + let(:value) { "12345678901245" } + let(:etablissement) { build(:etablissement, siret: value) } it { is_expected.to be_valid } end diff --git a/spec/models/concerns/champ_validate_concern_spec.rb b/spec/models/concerns/champ_validate_concern_spec.rb index 1eaffa94c5c..d013479e884 100644 --- a/spec/models/concerns/champ_validate_concern_spec.rb +++ b/spec/models/concerns/champ_validate_concern_spec.rb @@ -8,10 +8,7 @@ let(:types_de_champ_public) { [{ type: :email }] } def update_champ(value) - dossier.update_champs_attributes({ - public_id => { value: } - }, :public, updated_by: 'test') - dossier.save + dossier.public_champ_for_update(public_id, updated_by: 'test').update(value:) end context 'when in revision' do diff --git a/spec/models/concerns/dossier_champs_concern_spec.rb b/spec/models/concerns/dossier_champs_concern_spec.rb index e60f9f3d123..97e58e3293a 100644 --- a/spec/models/concerns/dossier_champs_concern_spec.rb +++ b/spec/models/concerns/dossier_champs_concern_spec.rb @@ -102,6 +102,43 @@ } end end + + context 'draft user stream' do + let(:row_id) { nil } + subject { dossier.with_update_stream(dossier.user).project_champ(type_de_champ_public, row_id:) } + + it { expect(subject.persisted?).to be_truthy } + + context "in repetition" do + let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) } + let(:row_id) { dossier.project_champ(type_de_champ_repetition).row_ids.first } + + it { + expect(subject.new_record?).to be_truthy + expect(subject.row_id).to eq(row_id) + } + end + + context "missing champ" do + before { dossier.champs.where(type: 'Champs::TextChamp').destroy_all; dossier.reload } + + it { + expect(subject.new_record?).to be_truthy + expect(subject.is_a?(Champs::TextChamp)).to be_truthy + } + + context "in repetition" do + let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) } + let(:row_id) { ULID.generate } + + it { + expect(subject.new_record?).to be_truthy + expect(subject.is_a?(Champs::TextChamp)).to be_truthy + expect(subject.row_id).to eq(row_id) + } + end + end + end end describe '#project_champs_public' do @@ -294,7 +331,7 @@ end end - describe "#update_champs_attributes(public)" do + describe "#public_champ_for_update" do let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) } let(:row_id) { ULID.generate } @@ -310,7 +347,14 @@ let(:champ_991) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(991)) } let(:champ_994) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(994), row_id:) } - subject { dossier.update_champs_attributes(attributes, :public, updated_by: dossier.user.email) } + def assign_champs_attributes(attributes) + attributes.each do |public_id, attributes| + champ = dossier.public_champ_for_update(public_id, updated_by: dossier.user.email) + champ.assign_attributes(attributes) + end + end + + subject { assign_champs_attributes(attributes) } it { subject @@ -360,7 +404,7 @@ end end - describe "#update_champs_attributes(private)" do + describe "#private_champ_for_update" do let(:attributes) do { "995" => { value: "Hello" } @@ -369,7 +413,14 @@ let(:annotation_995) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(995)) } - subject { dossier.update_champs_attributes(attributes, :private, updated_by: dossier.user.email) } + def assign_champs_attributes(attributes) + attributes.each do |public_id, attributes| + champ = dossier.private_champ_for_update(public_id, updated_by: dossier.user.email) + champ.assign_attributes(attributes) + end + end + + subject { assign_champs_attributes(attributes) } it { subject @@ -389,4 +440,143 @@ } end end + + context 'en_construction(user)' do + let(:dossier) { create(:dossier, :en_construction, procedure:) } + + describe "#public_champ_for_update" do + before { Flipper.enable(:user_buffer_stream, procedure) } + + let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) } + let(:row_ids) { dossier.project_champ(type_de_champ_repetition).row_ids } + let(:row_id) { row_ids.first } + + let(:attributes) do + { + "99" => { value: "Hello" }, + "991" => { value: "World" }, + "994-#{row_id}" => { value: "Greer" } + } + end + + let(:new_attributes) do + { + "99" => { value: "Hello!!!" }, + "994-#{row_id}" => { value: "Greer is the best" } + } + end + + let(:bad_attributes) do + { + "99" => { value: "bad" }, + "994-#{row_id}" => { value: "bad" } + } + end + + def main_champ(stable_id, row_id = nil) + dossier.with_main_stream do + dossier.project_champ(dossier.find_type_de_champ_by_stable_id(stable_id), row_id:) + end + end + + def draft_champ(stable_id, row_id = nil) + dossier.with_update_stream(dossier.user) do + dossier.project_champ(dossier.find_type_de_champ_by_stable_id(stable_id), row_id:) + end + end + + def main_champ_99 = main_champ(99) + def main_champ_991 = main_champ(991) + def main_champ_994 = main_champ(994, row_id) + def draft_champ_99 = draft_champ(99) + def draft_champ_991 = draft_champ(991) + def draft_champ_994 = draft_champ(994, row_id) + + def assign_champs_attributes(attributes) + attributes.each do |public_id, attributes| + champ = dossier.public_champ_for_update(public_id, updated_by: dossier.user.email) + champ.assign_attributes(attributes) + end + end + + subject do + dossier.with_update_stream(dossier.user) { assign_champs_attributes(attributes) } + end + + it { + subject + dossier.save! + + expect(dossier.user_buffer_changes?).to be_truthy + + expect(main_champ_99.stream).to eq(Champ::MAIN_STREAM) + expect(main_champ_991.stream).to eq(Champ::MAIN_STREAM) + expect(main_champ_994.stream).to eq(Champ::MAIN_STREAM) + + expect(main_champ_99.value).to be_nil + expect(main_champ_991.value).to be_nil + expect(main_champ_994.value).to be_nil + + expect(draft_champ_99.stream).to eq(Champ::USER_BUFFER_STREAM) + expect(draft_champ_991.stream).to eq(Champ::USER_BUFFER_STREAM) + expect(draft_champ_994.stream).to eq(Champ::USER_BUFFER_STREAM) + + expect(draft_champ_99.value).to eq("Hello") + expect(draft_champ_991.value).to eq("World") + expect(draft_champ_994.value).to eq("Greer") + expect(dossier.history.size).to eq(0) + + dossier.merge_user_buffer_stream! + + expect(main_champ_99.value).to eq("Hello") + expect(main_champ_991.value).to eq("World") + expect(main_champ_994.value).to eq("Greer") + expect(dossier.history.size).to eq(2) + + Timecop.freeze(1.hour.from_now) do + dossier.with_update_stream(dossier.user) { assign_champs_attributes(new_attributes) } + dossier.save! + dossier.merge_user_buffer_stream! + end + + expect(main_champ_99.value).to eq("Hello!!!") + expect(main_champ_994.value).to eq("Greer is the best") + expect(dossier.history.size).to eq(4) + + Timecop.freeze(2.hours.from_now) do + dossier.with_update_stream(dossier.user) { assign_champs_attributes(bad_attributes) } + dossier.save! + end + + expect(draft_champ_99.value).to eq("bad") + expect(draft_champ_991.value).to eq("World") + expect(draft_champ_994.value).to eq("bad") + dossier.reset_user_buffer_stream! + expect(draft_champ_99.value).to eq("Hello!!!") + expect(draft_champ_991.value).to eq("World") + expect(draft_champ_994.value).to eq("Greer is the best") + } + + context "missing champs" do + before { dossier; Champs::TextChamp.destroy_all; dossier.champs.reload } + + it { + subject + dossier.save! + + expect(draft_champ_99.stream).to eq(Champ::USER_BUFFER_STREAM) + expect(draft_champ_991.stream).to eq(Champ::USER_BUFFER_STREAM) + expect(draft_champ_994.stream).to eq(Champ::USER_BUFFER_STREAM) + + expect(draft_champ_99.value).to eq("Hello") + expect(draft_champ_991.value).to eq("World") + expect(draft_champ_994.value).to eq("Greer") + + expect(dossier.history.size).to eq(0) + dossier.merge_user_buffer_stream! + expect(dossier.history.size).to eq(0) + } + end + end + end end diff --git a/spec/models/concerns/dossier_clone_concern_spec.rb b/spec/models/concerns/dossier_clone_concern_spec.rb index f7bab0f4794..d452e55358e 100644 --- a/spec/models/concerns/dossier_clone_concern_spec.rb +++ b/spec/models/concerns/dossier_clone_concern_spec.rb @@ -290,7 +290,7 @@ it do expect(subject).to eq(added: [], updated: [], removed: []) - expect(forked_dossier.forked_with_changes?).to be_truthy + expect(forked_dossier.user_buffer_changes?).to be_truthy end end @@ -299,11 +299,11 @@ before { updated_champ.update(value: 'new value') } - it 'forked_with_changes? should reflect dossier state' do + it 'user_buffer_changes? should reflect dossier state' do expect(subject).to eq(added: [], updated: [updated_champ], removed: []) - expect(dossier.forked_with_changes?).to be_falsey - expect(forked_dossier.forked_with_changes?).to be_truthy - expect(updated_champ.forked_with_changes?).to be_truthy + expect(dossier.user_buffer_changes?).to be_truthy + expect(forked_dossier.user_buffer_changes?).to be_truthy + expect(updated_champ.user_buffer_changes?).to be_truthy end end diff --git a/spec/models/concerns/dossier_searchable_concern_spec.rb b/spec/models/concerns/dossier_searchable_concern_spec.rb index 62f198a03f0..dde4e4e2857 100644 --- a/spec/models/concerns/dossier_searchable_concern_spec.rb +++ b/spec/models/concerns/dossier_searchable_concern_spec.rb @@ -38,10 +38,11 @@ end it "update columns en construction" do - dossier.update_champs_attributes({ champ_public.public_id => { value: 'nouvelle valeur publique' } }, :public, updated_by: 'test') - dossier.update_champs_attributes({ champ_private.public_id => { value: 'nouvelle valeur privee' } }, :private, updated_by: 'test') + dossier.public_champ_for_update(champ_public.public_id, updated_by: 'test').tap { _1.assign_attributes(value: 'nouvelle valeur publique') } + dossier.private_champ_for_update(champ_private.public_id, updated_by: 'test').tap { _1.assign_attributes(value: 'nouvelle valeur privee') } assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob) do + dossier.save! dossier.passer_en_construction end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index bafe0412746..ff6bf7b55bb 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -2426,22 +2426,30 @@ it { expect(dossier.sva_svr_decision_in_days).to eq 10 } end - describe '#touch_champs_changed' do - let(:dossier) { create(:dossier, brouillon_close_to_expiration_notice_sent_at: 10.days.ago) } + describe '#update_champs_timestamps' do + let(:procedure) { create(:procedure, types_de_champ_public: [{}, { type: :piece_justificative }, { type: :titre_identite }]) } + let(:dossier) { create(:dossier, procedure:, brouillon_close_to_expiration_notice_sent_at: 10.days.ago) } + let(:changed_champs) { dossier.champs.filter(&:text?) } - subject { -> { dossier.touch_champs_changed(attributes) } } - - let(:attributes) { [:last_champ_updated_at] } + subject { -> { dossier.update_champs_timestamps(changed_champs) } } it { is_expected.to change(dossier, :last_champ_updated_at) } + it { is_expected.to change(dossier, :updated_at) } + + context 'when there is piece justificative' do + let(:changed_champs) { dossier.champs.filter(&:piece_justificative?) } - it { is_expected.to change(dossier, :brouillon_close_to_expiration_notice_sent_at).to(nil) } + it { is_expected.to change(dossier, :last_champ_updated_at) } + it { is_expected.to change(dossier, :last_champ_piece_jointe_updated_at) } + it { is_expected.to change(dossier, :updated_at) } + end - context 'when there is two attributes' do - let(:attributes) { [:last_champ_updated_at, :last_champ_piece_jointe_updated_at] } + context 'when there is titre identite' do + let(:changed_champs) { dossier.champs.filter(&:titre_identite?) } it { is_expected.to change(dossier, :last_champ_updated_at) } it { is_expected.to change(dossier, :last_champ_piece_jointe_updated_at) } + it { is_expected.to change(dossier, :updated_at) } end end diff --git a/spec/views/shared/dossiers/_edit.html.haml_spec.rb b/spec/views/shared/dossiers/_edit.html.haml_spec.rb index 047d05c6a03..df1e7ffd340 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_update_stream(dossier.user) } + + 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}']")