diff --git a/app/components/procedure/revision_changes_component.rb b/app/components/procedure/revision_changes_component.rb index 981a0e157..f011e3b0b 100644 --- a/app/components/procedure/revision_changes_component.rb +++ b/app/components/procedure/revision_changes_component.rb @@ -4,6 +4,7 @@ class Procedure::RevisionChangesComponent < ApplicationComponent def initialize(new_revision:, previous_revision:) @previous_revision = previous_revision @new_revision = new_revision + @procedure = new_revision.procedure @tdc_changes = previous_revision.compare_types_de_champ(new_revision) @public_move_changes, @private_move_changes = @tdc_changes.filter { _1.op == :move }.partition { !_1.private? } @@ -14,6 +15,10 @@ class Procedure::RevisionChangesComponent < ApplicationComponent private + def used_by_routing_rules?(type_de_champ) + @procedure.used_by_routing_rules?(type_de_champ) + end + def total_dossiers @total_dossiers ||= @previous_revision.dossiers .visible_by_administration diff --git a/app/components/procedure/revision_changes_component/revision_changes_component.html.haml b/app/components/procedure/revision_changes_component/revision_changes_component.html.haml index ed7f550c8..2cfed321b 100644 --- a/app/components/procedure/revision_changes_component/revision_changes_component.html.haml +++ b/app/components/procedure/revision_changes_component/revision_changes_component.html.haml @@ -77,7 +77,7 @@ - if !total_dossiers.zero? && !change.can_rebase? .fr-alert.fr-alert--warning.fr-mt-1v %p= t('.breaking_change', count: total_dossiers) - - if (removed.present? || added.present? ) && change.type_de_champ.used_by_routing_rules? + - if (removed.present? || added.present? ) && used_by_routing_rules?(change.type_de_champ) .fr-alert.fr-alert--warning.fr-mt-1v = t(".#{prefix}.update_drop_down_options_alert", label: change.label) - when :drop_down_other diff --git a/app/components/types_de_champ_editor/champ_component.rb b/app/components/types_de_champ_editor/champ_component.rb index 1e1860ab9..a0fa6a516 100644 --- a/app/components/types_de_champ_editor/champ_component.rb +++ b/app/components/types_de_champ_editor/champ_component.rb @@ -78,7 +78,7 @@ class TypesDeChampEditor::ChampComponent < ApplicationComponent def piece_justificative_template_options { attached_file: type_de_champ.piece_justificative_template, - auto_attach_url: helpers.auto_attach_url(type_de_champ), + auto_attach_url: helpers.auto_attach_url(type_de_champ, procedure_id: procedure.id), view_as: :download } end @@ -86,7 +86,7 @@ class TypesDeChampEditor::ChampComponent < ApplicationComponent def notice_explicative_options { attached_file: type_de_champ.notice_explicative, - auto_attach_url: helpers.auto_attach_url(type_de_champ), + auto_attach_url: helpers.auto_attach_url(type_de_champ, procedure_id: procedure.id), view_as: :download } end diff --git a/app/controllers/administrateurs/types_de_champ_controller.rb b/app/controllers/administrateurs/types_de_champ_controller.rb index 0fee638ec..051191af7 100644 --- a/app/controllers/administrateurs/types_de_champ_controller.rb +++ b/app/controllers/administrateurs/types_de_champ_controller.rb @@ -20,15 +20,14 @@ module Administrateurs def update type_de_champ = draft.find_and_ensure_exclusive_use(params[:stable_id]) + @coordinate = draft.coordinate_for(type_de_champ) - if type_de_champ.revision_type_de_champ.used_by_routing_rules? && changing_of_type?(type_de_champ) - coordinate = draft.coordinate_for(type_de_champ) + if @coordinate.used_by_routing_rules? && changing_of_type?(type_de_champ) errors = "« #{type_de_champ.libelle} » est utilisé pour le routage, vous ne pouvez pas modifier son type." - @morphed = [champ_component_from(coordinate, focused: false, errors:)] + @morphed = [champ_component_from(@coordinate, focused: false, errors:)] flash.alert = errors elsif type_de_champ.update(type_de_champ_update_params) reload_procedure_with_includes - @coordinate = draft.coordinate_for(type_de_champ) @morphed = champ_components_starting_at(@coordinate) else flash.alert = type_de_champ.errors.full_messages diff --git a/app/helpers/champ_helper.rb b/app/helpers/champ_helper.rb index cec15a4c5..449f9b799 100644 --- a/app/helpers/champ_helper.rb +++ b/app/helpers/champ_helper.rb @@ -9,13 +9,13 @@ module ChampHelper simple_format(auto_linked_text, {}, sanitize: false) end - def auto_attach_url(object, params = {}) + def auto_attach_url(object, procedure_id: nil) if object.is_a?(Champ) - champs_piece_justificative_url(object.dossier, object.stable_id, params.merge(row_id: object.row_id)) + champs_piece_justificative_url(object.dossier, object.stable_id, row_id: object.row_id) elsif object.is_a?(TypeDeChamp) && object.piece_justificative? - piece_justificative_template_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id: object.procedure.id, **params) + piece_justificative_template_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id:) elsif object.is_a?(TypeDeChamp) && object.explication? - notice_explicative_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id: object.procedure.id, **params) + notice_explicative_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id:) end end end diff --git a/app/models/champ.rb b/app/models/champ.rb index fb7bf56e9..8efa30748 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -74,7 +74,6 @@ class Champ < ApplicationRecord delegate :to_typed_id, :to_typed_id_for_query, to: :type_de_champ, prefix: true delegate :revision, to: :dossier, prefix: true - delegate :used_by_routing_rules?, to: :type_de_champ scope :updated_since?, -> (date) { where('champs.updated_at > ?', date) } scope :prefilled, -> { where(prefilled: true) } @@ -106,6 +105,10 @@ class Champ < ApplicationRecord type_de_champ.champ_blank?(self) end + def used_by_routing_rules? + procedure.used_by_routing_rules?(type_de_champ) + end + def search_terms [to_s] end diff --git a/app/models/dubious_procedure.rb b/app/models/dubious_procedure.rb index 6960aad6f..792b1526d 100644 --- a/app/models/dubious_procedure.rb +++ b/app/models/dubious_procedure.rb @@ -19,11 +19,11 @@ class DubiousProcedure end def self.all - procedures_with_forbidden_tdcs_sql = TypeDeChamp - .joins(:procedure) + procedures_with_forbidden_tdcs_sql = ProcedureRevisionTypeDeChamp + .joins(:procedure, :type_de_champ) .select("string_agg(types_de_champ.libelle, ' - ') as dubious_champs, procedures.id as procedure_id, procedures.libelle as procedure_libelle, procedures.aasm_state as procedure_aasm_state, procedures.hidden_at_as_template as procedure_hidden_at_as_template") .where("unaccent(types_de_champ.libelle) ~* unaccent(?)", forbidden_regexp) - .where(type_champ: [TypeDeChamp.type_champs.fetch(:text), TypeDeChamp.type_champs.fetch(:textarea)]) + .where(types_de_champ: { type_champ: [TypeDeChamp.type_champs.fetch(:text), TypeDeChamp.type_champs.fetch(:textarea)] }) .where(procedures: { closed_at: nil, whitelisted_at: nil }) .group("procedures.id") .order("procedures.id asc") diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 376f9e88b..05c3e3346 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -687,7 +687,7 @@ class Procedure < ApplicationRecord end def routing_champs - active_revision.types_de_champ_public.filter(&:used_by_routing_rules?).map(&:libelle) + active_revision.revision_types_de_champ_public.filter(&:used_by_routing_rules?).map(&:libelle) end def can_be_deleted_by_administrateur? @@ -829,8 +829,8 @@ class Procedure < ApplicationRecord end end - def stable_ids_used_by_routing_rules - @stable_ids_used_by_routing_rules ||= groupe_instructeurs.flat_map { _1.routing_rule&.sources }.compact + def used_by_routing_rules?(type_de_champ) + type_de_champ.stable_id.in?(stable_ids_used_by_routing_rules) end # We need this to unfuck administrate + aasm @@ -871,6 +871,10 @@ class Procedure < ApplicationRecord private + def stable_ids_used_by_routing_rules + @stable_ids_used_by_routing_rules ||= groupe_instructeurs.flat_map { _1.routing_rule&.sources }.compact.uniq + end + def published_revisions_types_de_champ(parent = nil) # all published revisions revision_ids = revisions.ids - [draft_revision_id] diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index 6a0631eb9..3b6127d3e 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -230,7 +230,7 @@ class ProcedureRevision < ApplicationRecord end def coordinate_for(tdc) - revision_types_de_champ.find_by!(type_de_champ: tdc) + revision_types_de_champ.find { _1.stable_id == tdc.stable_id } end def carte? diff --git a/app/models/procedure_revision_type_de_champ.rb b/app/models/procedure_revision_type_de_champ.rb index 1a0d27c01..3963ae630 100644 --- a/app/models/procedure_revision_type_de_champ.rb +++ b/app/models/procedure_revision_type_de_champ.rb @@ -75,7 +75,7 @@ class ProcedureRevisionTypeDeChamp < ApplicationRecord end def used_by_routing_rules? - stable_id.in?(procedure.stable_ids_used_by_routing_rules) + procedure.used_by_routing_rules?(type_de_champ) end def used_by_ineligibilite_rules? diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index abb5df34e..50e170baa 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -142,13 +142,9 @@ class TypeDeChamp < ApplicationRecord :header_section_level has_many :revision_types_de_champ, -> { revision_ordered }, class_name: 'ProcedureRevisionTypeDeChamp', dependent: :destroy, inverse_of: :type_de_champ - has_one :revision_type_de_champ, -> { revision_ordered }, class_name: 'ProcedureRevisionTypeDeChamp', inverse_of: false has_many :revisions, -> { ordered }, through: :revision_types_de_champ - has_one :revision, through: :revision_type_de_champ - has_one :procedure, through: :revision delegate :estimated_fill_duration, :estimated_read_duration, :tags_for_template, :libelles_for_export, :libelle_for_export, :primary_options, :secondary_options, :columns, to: :dynamic_type - delegate :used_by_routing_rules?, to: :revision_type_de_champ class WithIndifferentAccess def self.load(options) @@ -213,7 +209,6 @@ class TypeDeChamp < ApplicationRecord before_save :remove_attachment, if: -> { type_champ_changed? } before_validation :set_drop_down_list_options, if: -> { type_champ_changed? } - before_save :remove_block, if: -> { type_champ_changed? } def valid?(context = nil) super @@ -818,14 +813,6 @@ class TypeDeChamp < ApplicationRecord end end - def remove_block - if !block? && procedure.present? - procedure - .draft_revision # action occurs only on draft - .remove_children_of(self) - end - end - def normalize_libelle self.libelle&.strip! end diff --git a/app/models/types_de_champ/prefill_type_de_champ.rb b/app/models/types_de_champ/prefill_type_de_champ.rb index 1b112f094..1917f7b55 100644 --- a/app/models/types_de_champ/prefill_type_de_champ.rb +++ b/app/models/types_de_champ/prefill_type_de_champ.rb @@ -72,7 +72,7 @@ class TypesDeChamp::PrefillTypeDeChamp < SimpleDelegator link_to( I18n.t("views.prefill_descriptions.edit.possible_values.link.text"), - Rails.application.routes.url_helpers.prefill_type_de_champ_path(revision.procedure_path, self), + Rails.application.routes.url_helpers.prefill_type_de_champ_path(@revision.procedure_path, self), title: new_tab_suffix(I18n.t("views.prefill_descriptions.edit.possible_values.link.title")), **external_link_attributes ) diff --git a/lib/tasks/deployment/20230602165134_migrate_remaining_data_for_routing_with_dropdown_list.rake b/lib/tasks/deployment/20230602165134_migrate_remaining_data_for_routing_with_dropdown_list.rake index d9a03d1e9..adc865a4c 100644 --- a/lib/tasks/deployment/20230602165134_migrate_remaining_data_for_routing_with_dropdown_list.rake +++ b/lib/tasks/deployment/20230602165134_migrate_remaining_data_for_routing_with_dropdown_list.rake @@ -29,7 +29,7 @@ namespace :after_party do procedure_ids = Procedure.with_discarded .where(routing_enabled: true) .where(migrated_champ_routage: [nil, false]) - .filter { |p| p.active_revision.types_de_champ.none?(&:used_by_routing_rules?) } + .filter { |p| p.active_revision.revision_types_de_champ_public.none?(&:used_by_routing_rules?) } .filter { |p| p.groupe_instructeurs.active.count > 1 } .pluck(:id) diff --git a/spec/components/types_de_champ_editor/champ_component_spec.rb b/spec/components/types_de_champ_editor/champ_component_spec.rb index 2efc1f7cc..da52aa878 100644 --- a/spec/components/types_de_champ_editor/champ_component_spec.rb +++ b/spec/components/types_de_champ_editor/champ_component_spec.rb @@ -16,7 +16,7 @@ describe TypesDeChampEditor::ChampComponent, type: :component do describe 'tdc dropdown' do let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :drop_down_list, libelle: 'Votre ville', options: ['Paris', 'Lyon', 'Marseille'] }]) } let(:tdc) { procedure.draft_revision.types_de_champ.first } - let(:coordinate) { tdc.revision_type_de_champ } + let(:coordinate) { procedure.draft_revision.coordinate_for(tdc) } context 'drop down tdc not used for routing' do it do diff --git a/spec/controllers/administrateurs/types_de_champ_controller_spec.rb b/spec/controllers/administrateurs/types_de_champ_controller_spec.rb index aaadbcbb3..a4c3a6696 100644 --- a/spec/controllers/administrateurs/types_de_champ_controller_spec.rb +++ b/spec/controllers/administrateurs/types_de_champ_controller_spec.rb @@ -98,7 +98,7 @@ describe Administrateurs::TypesDeChampController, type: :controller do it do is_expected.to have_http_status(:ok) - expect(assigns(:coordinate)).to be_nil + expect(assigns(:coordinate)).to eq(second_coordinate) expect(flash.alert).to eq(["Le champ « Libelle » doit être rempli"]) end end diff --git a/spec/factories/champ.rb b/spec/factories/champ.rb index e3f173c0a..bab863c84 100644 --- a/spec/factories/champ.rb +++ b/spec/factories/champ.rb @@ -190,7 +190,7 @@ FactoryBot.define do end after(:build) do |champ_repetition, evaluator| - revision = champ_repetition.type_de_champ.procedure.active_revision + revision = champ_repetition.procedure.active_revision parent = revision.revision_types_de_champ.find { _1.type_de_champ == champ_repetition.type_de_champ } types_de_champ = revision.revision_types_de_champ.filter { _1.parent == parent }.map(&:type_de_champ) diff --git a/spec/models/procedure_revision_spec.rb b/spec/models/procedure_revision_spec.rb index 97e53841f..1d6856b1d 100644 --- a/spec/models/procedure_revision_spec.rb +++ b/spec/models/procedure_revision_spec.rb @@ -63,7 +63,7 @@ describe ProcedureRevision do it do expect { subject }.to change { draft.reload.types_de_champ.count }.from(4).to(5) expect(draft.children_of(type_de_champ_repetition).last).to eq(subject) - expect(draft.children_of(type_de_champ_repetition).map(&:revision_type_de_champ).map(&:position)).to eq([0, 1]) + expect(draft.children_of(type_de_champ_repetition).map { draft.coordinate_for(_1).position }).to eq([0, 1]) expect(last_coordinate.position).to eq(1) diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index ba22d25b1..25784929d 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -717,26 +717,26 @@ describe Procedure do procedure.draft_revision.types_de_champ_public.zip(subject.draft_revision.types_de_champ_public).each do |ptc, stc| expect(stc).to have_same_attributes_as(ptc) - expect(stc.revision).to eq(subject.draft_revision) + expect(stc.revisions).to include(subject.draft_revision) end public_repetition = type_de_champ_repetition cloned_public_repetition = subject.draft_revision.types_de_champ_public.repetition.first procedure.draft_revision.children_of(public_repetition).zip(subject.draft_revision.children_of(cloned_public_repetition)).each do |ptc, stc| expect(stc).to have_same_attributes_as(ptc) - expect(stc.revision).to eq(subject.draft_revision) + expect(stc.revisions).to include(subject.draft_revision) end procedure.draft_revision.types_de_champ_private.zip(subject.draft_revision.types_de_champ_private).each do |ptc, stc| expect(stc).to have_same_attributes_as(ptc) - expect(stc.revision).to eq(subject.draft_revision) + expect(stc.revisions).to include(subject.draft_revision) end private_repetition = type_de_champ_private_repetition cloned_private_repetition = subject.draft_revision.types_de_champ_private.repetition.first procedure.draft_revision.children_of(private_repetition).zip(subject.draft_revision.children_of(cloned_private_repetition)).each do |ptc, stc| expect(stc).to have_same_attributes_as(ptc) - expect(stc.revision).to eq(subject.draft_revision) + expect(stc.revisions).to include(subject.draft_revision) end expect(subject.attestation_template.title).to eq(procedure.attestation_template.title) diff --git a/spec/tasks/maintenance/delete_draft_revision_type_de_champs_task_spec.rb b/spec/tasks/maintenance/delete_draft_revision_type_de_champs_task_spec.rb index 05886ae4e..25b93ebcf 100644 --- a/spec/tasks/maintenance/delete_draft_revision_type_de_champs_task_spec.rb +++ b/spec/tasks/maintenance/delete_draft_revision_type_de_champs_task_spec.rb @@ -51,9 +51,9 @@ module Maintenance tdc = find_by_stable_id(11) expect(tdc).to be_nil - tdc = find_by_stable_id(131) + tdc, coord = find_with_coordinate_by_stable_id(131) expect(tdc).not_to be_nil - expect(tdc.revision_type_de_champ.position).to eq(0) # reindexed + expect(coord.position).to eq(0) # reindexed tdc = find_by_stable_id(132) expect(tdc).to be_nil @@ -63,5 +63,10 @@ module Maintenance def find_by_stable_id(stable_id) procedure.draft_revision.types_de_champ.find { _1.stable_id == stable_id } end + + def find_with_coordinate_by_stable_id(stable_id) + tdc = find_by_stable_id(stable_id) + [tdc, procedure.draft_revision.coordinate_for(tdc)] + end end end diff --git a/spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb b/spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb index 57e01ef8d..f27c199d6 100644 --- a/spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb +++ b/spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb @@ -40,31 +40,31 @@ module Maintenance it "updates the type de champ" do process - tdc = find_by_stable_id(12) - expect(tdc.revision_type_de_champ.position).to eq(0) + tdc, coord = find_with_coordinate_by_stable_id(12) + expect(coord.position).to eq(0) expect(tdc.libelle).to eq("[NEW] Number") expect(tdc.description).to eq("[NEW] Number desc") expect(tdc.mandatory).to eq(true) - tdc = find_by_stable_id(13) - expect(tdc.revision_type_de_champ.position).to eq(1) + tdc, coord = find_with_coordinate_by_stable_id(13) + expect(coord.position).to eq(1) expect(tdc.libelle).to eq("Bloc") expect(tdc.description).to eq("[NEW] bloc desc") expect(tdc.mandatory).to eq(false) - tdc = find_by_stable_id(132) - expect(tdc.revision_type_de_champ.position).to eq(0) + tdc, coord = find_with_coordinate_by_stable_id(132) + expect(coord.position).to eq(0) expect(tdc.libelle).to eq("[NEW] RepNum") expect(tdc.mandatory).to eq(true) - tdc = find_by_stable_id(131) - expect(tdc.revision_type_de_champ.position).to eq(1) + tdc, coord = find_with_coordinate_by_stable_id(131) + expect(coord.position).to eq(1) expect(tdc.libelle).to eq("[NEW] RepText") expect(tdc.description).to eq("") expect(tdc.mandatory).to eq(false) - tdc = find_by_stable_id(11) - expect(tdc.revision_type_de_champ.position).to eq(2) + tdc, coord = find_with_coordinate_by_stable_id(11) + expect(coord.position).to eq(2) expect(tdc.libelle).to eq("[supp] Text") expect(tdc.mandatory).to eq(false) end @@ -73,5 +73,10 @@ module Maintenance def find_by_stable_id(stable_id) procedure.draft_revision.types_de_champ.find { _1.stable_id == stable_id } end + + def find_with_coordinate_by_stable_id(stable_id) + tdc = find_by_stable_id(stable_id) + [tdc, procedure.draft_revision.coordinate_for(tdc)] + end end end