Merge pull request #10340 from tchak/refactor-dossier-champ-by-stable…

…-id-next

refactor(champs): update champs by public_id [remove dead code]

app/components/editable_champ/carte_component.rb

@@ -11,10 +11,6 @@ def initialize(**args)
   end
 
   def update_path
-    if Champ.update_by_stable_id?
-      champs_carte_features_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id)
-    else
-      champs_legacy_carte_features_path(@champ)
-    end
+    champs_carte_features_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id)
   end
 end

app/components/editable_champ/editable_champ_component.rb

@@ -54,17 +54,9 @@ def stimulus_values
 
   def turbo_poll_url_value
     if @champ.private?
-      if Champ.update_by_stable_id?
-        annotation_instructeur_dossier_path(@champ.dossier.procedure, @champ.dossier, @champ.stable_id, row_id: @champ.row_id, with_public_id: true)
-      else
-        annotation_instructeur_dossier_path(@champ.dossier.procedure, @champ.dossier, @champ)
-      end
+      annotation_instructeur_dossier_path(@champ.dossier.procedure, @champ.dossier, @champ.stable_id, row_id: @champ.row_id, with_public_id: true)
     else
-      if Champ.update_by_stable_id?
-        champ_dossier_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id, with_public_id: true)
-      else
-        champ_dossier_path(@champ.dossier, @champ)
-      end
+      champ_dossier_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id, with_public_id: true)
     end
   end
 

app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml

@@ -6,8 +6,3 @@
     = render champ_component
 
     = render Dsfr::InputStatusMessageComponent.new(errors_on_attribute: champ_component.errors_on_attribute?, error_full_messages: champ_component.error_full_messages, describedby_id: @champ.describedby_id, champ: @champ)
-
-    - if Champ.update_by_stable_id?
-      = @form.hidden_field :with_public_id, value: 'true'
-    - else
-      = @form.hidden_field :id, value: @champ.id

app/components/editable_champ/multiple_drop_down_list_component.rb

@@ -10,10 +10,6 @@ def dsfr_champ_container
   end
 
   def update_path(option)
-    if Champ.update_by_stable_id?
-      champs_options_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id, option:)
-    else
-      champs_legacy_options_path(@champ, option:)
-    end
+    champs_options_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id, option:)
   end
 end

app/components/editable_champ/rna_component.rb

@@ -4,10 +4,6 @@ def dsfr_input_classname
   end
 
   def update_path
-    if Champ.update_by_stable_id?
-      champs_rna_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id)
-    else
-      champs_legacy_rna_path(@champ)
-    end
+    champs_rna_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id)
   end
 end

app/components/editable_champ/siret_component.rb

@@ -12,10 +12,6 @@ def hintable?
   end
 
   def update_path
-    if Champ.update_by_stable_id?
-      champs_siret_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id)
-    else
-      champs_legacy_siret_path(@champ)
-    end
+    champs_siret_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id)
   end
 end

app/controllers/champs/champ_controller.rb

@@ -5,15 +5,9 @@ class Champs::ChampController < ApplicationController
   private
 
   def find_champ
-    if params[:champ_id].present?
-      policy_scope(Champ)
-        .includes(:type_de_champ, dossier: :champs)
-        .find(params[:champ_id])
-    else
-      dossier = policy_scope(Dossier).includes(:champs, revision: [:types_de_champ]).find(params[:dossier_id])
-      type_de_champ = dossier.find_type_de_champ_by_stable_id(params[:stable_id])
-      dossier.champ_for_update(type_de_champ, params_row_id)
-    end
+    dossier = policy_scope(Dossier).includes(:champs, revision: [:types_de_champ]).find(params[:dossier_id])
+    type_de_champ = dossier.find_type_de_champ_by_stable_id(params[:stable_id])
+    dossier.champ_for_update(type_de_champ, params_row_id)
   end
 
   def params_row_id

app/controllers/champs/options_controller.rb

@@ -3,9 +3,8 @@ class Champs::OptionsController < Champs::ChampController
 
   def remove
     @champ.remove_option([params[:option]].compact, true)
-    @champ.reload
     @dossier = @champ.private? ? nil : @champ.dossier
-    champs_attributes = { @champ.public_id => params[:champ_id].present? ? { id: @champ.id } : { with_public_id: true } }
+    champs_attributes = { @champ.public_id => {} }
     @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_attributes, @champ.dossier.champs)
   end
 end

app/controllers/concerns/turbo_champs_concern.rb

@@ -4,13 +4,8 @@ module TurboChampsConcern
   private
 
   def champs_to_turbo_update(params, champs)
-    to_update = if params.values.filter { _1.key?(:with_public_id) }.empty?
-                  champ_ids = params.values.map { _1[:id] }.compact.map(&:to_i)
-                  champs.filter { _1.id.in?(champ_ids) }
-                else
-                  champ_public_ids = params.keys
-                  champs.filter { _1.public_id.in?(champ_public_ids) }
-                end.filter { _1.refresh_after_update? || _1.forked_with_changes? }
+    to_update = champs.filter { _1.public_id.in?(params.keys) }
+      .filter { _1.refresh_after_update? || _1.forked_with_changes? }
 
     to_show, to_hide = champs.filter(&:conditional?)
       .partition(&:visible?)

app/controllers/instructeurs/dossiers_controller.rb

@@ -275,7 +275,7 @@ def create_avis
 
     def update_annotations
       dossier_with_champs.update_champs_attributes(champs_private_attributes_params, :private)
-      if dossier.champs.any?(&:changed_for_autosave?) || dossier.champs_private_all.any?(&:changed_for_autosave?) # TODO remove second condition after one deploy
+      if dossier.champs.any?(&:changed_for_autosave?)
         dossier.last_champ_private_updated_at = Time.zone.now
       end
 
@@ -296,13 +296,8 @@ def print
 
     def annotation
       @dossier = dossier_with_champs(pj_template: false)
-      annotation_id_or_stable_id = params[:stable_id]
-      annotation = if params[:with_public_id].present?
-        type_de_champ = @dossier.find_type_de_champ_by_stable_id(annotation_id_or_stable_id, :private)
-        @dossier.project_champ(type_de_champ, params[:row_id])
-      else
-        @dossier.champs_private_all.find(annotation_id_or_stable_id)
-      end
+      type_de_champ = @dossier.find_type_de_champ_by_stable_id(params[:stable_id], :private)
+      annotation = @dossier.project_champ(type_de_champ, params[:row_id])
 
       respond_to do |format|
         format.turbo_stream do
@@ -418,7 +413,6 @@ def champs_private_params
         :accreditation_number,
         :accreditation_birthdate,
         :feature,
-        :with_public_id,
         value: []
       ]
       # Strong attributes do not support records (indexed hash); they only support hashes with

app/controllers/users/dossiers_controller.rb

@@ -319,13 +319,8 @@ def merci
 
     def champ
       @dossier = dossier_with_champs(pj_template: false)
-      champ_id_or_stable_id = params[:stable_id]
-      champ = if params[:with_public_id].present?
-        type_de_champ = @dossier.find_type_de_champ_by_stable_id(champ_id_or_stable_id, :public)
-        @dossier.project_champ(type_de_champ, params[:row_id])
-      else
-        @dossier.champs_public_all.find(champ_id_or_stable_id)
-      end
+      type_de_champ = @dossier.find_type_de_champ_by_stable_id(params[:stable_id], :public)
+      champ = @dossier.project_champ(type_de_champ, params[:row_id])
 
       respond_to do |format|
         format.turbo_stream do
@@ -511,7 +506,6 @@ def champs_public_params
         :accreditation_number,
         :accreditation_birthdate,
         :feature,
-        :with_public_id,
         value: []
       ]
       # Strong attributes do not support records (indexed hash); they only support hashes with
@@ -556,7 +550,7 @@ def set_dossier_as_editing_fork
 
     def update_dossier_and_compute_errors
       @dossier.update_champs_attributes(champs_public_attributes_params, :public)
-      if @dossier.champs.any?(&:changed_for_autosave?) || @dossier.champs_public_all.any?(&:changed_for_autosave?) # TODO remove second condition after one deploy
+      if @dossier.champs.any?(&:changed_for_autosave?)
         @dossier.last_champ_updated_at = Time.zone.now
       end
 

app/helpers/champ_helper.rb

@@ -9,11 +9,7 @@ def format_text_value(text)
 
   def auto_attach_url(object, params = {})
     if object.is_a?(Champ)
-      if Champ.update_by_stable_id?
-        champs_piece_justificative_url(object.dossier, object.stable_id, params.merge(row_id: object.row_id))
-      else
-        champs_legacy_piece_justificative_url(object.id, params)
-      end
+      champs_piece_justificative_url(object.dossier, object.stable_id, params.merge(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)
     elsif object.is_a?(TypeDeChamp) && object.explication?

app/models/champ.rb

@@ -2,9 +2,6 @@ class Champ < ApplicationRecord
   include ChampConditionalConcern
   include ChampsValidateConcern
 
-  # TODO: remove after one deploy
-  attr_writer :with_public_id
-
   belongs_to :dossier, inverse_of: false, touch: true, optional: false
   belongs_to :type_de_champ, inverse_of: :champ, optional: false
   belongs_to :parent, class_name: 'Champ', optional: true
@@ -295,10 +292,6 @@ def normalize
     self.value = value.delete("\u0000")
   end
 
-  def self.update_by_stable_id?
-    Flipper.enabled?(:champ_update_by_stable_id, Current.user)
-  end
-
   class NotImplemented < ::StandardError
     def initialize(method)
       super(":#{method} not implemented")

app/models/concerns/dossier_champs_concern.rb

@@ -65,13 +65,6 @@ def champ_for_update(type_de_champ, row_id)
   end
 
   def update_champs_attributes(attributes, scope)
-    # TODO: remove after one deploy
-    if attributes.present? && attributes.values.filter { _1.key?(:with_public_id) }.empty?
-      assign_attributes("champs_#{scope}_all_attributes".to_sym => attributes)
-      @champs_by_public_id = nil
-      return
-    end
-
     champs_attributes = attributes.to_h.map do |public_id, attributes|
       champ_attributes_by_public_id(public_id, attributes, scope)
     end

app/models/dossier.rb

@@ -49,8 +49,6 @@ class Dossier < ApplicationRecord
   has_many :champs_to_destroy, -> { order(:parent_id) }, class_name: 'Champ', inverse_of: false, dependent: :destroy
   has_many :champs_public, -> { root.public_only }, class_name: 'Champ', inverse_of: false
   has_many :champs_private, -> { root.private_only }, class_name: 'Champ', inverse_of: false
-  has_many :champs_public_all, -> { public_only }, class_name: 'Champ', inverse_of: false
-  has_many :champs_private_all, -> { private_only }, class_name: 'Champ', inverse_of: false
   has_many :prefilled_champs_public, -> { root.public_only.prefilled }, class_name: 'Champ', inverse_of: false
 
   has_many :commentaires, inverse_of: :dossier, dependent: :destroy
@@ -145,8 +143,6 @@ def classer_sans_suite(motivation: nil, instructeur: nil, processed_at: Time.zon
   accepts_nested_attributes_for :champs
   accepts_nested_attributes_for :champs_public
   accepts_nested_attributes_for :champs_private
-  accepts_nested_attributes_for :champs_public_all
-  accepts_nested_attributes_for :champs_private_all
   accepts_nested_attributes_for :individual
 
   include AASM

app/models/dossier_preloader.rb

@@ -93,8 +93,6 @@ def load_dossier(dossier, champs, children_by_parent = {})
     champs_public, champs_private = champs.partition(&:public?)
 
     dossier.association(:champs).target = []
-    dossier.association(:champs_public_all).target = []
-    dossier.association(:champs_private_all).target = []
     load_champs(dossier, :champs_public, champs_public, dossier, children_by_parent)
     load_champs(dossier, :champs_private, champs_private, dossier, children_by_parent)
 
@@ -122,12 +120,6 @@ def load_champs(parent, name, champs, dossier, children_by_parent)
 
     dossier.association(:champs).target += champs
 
-    if champs.first.public?
-      dossier.association(:champs_public_all).target += champs
-    else
-      dossier.association(:champs_private_all).target += champs
-    end
-
     parent.association(name).target = champs
       .filter { positions[dossier.revision_id][_1.type_de_champ_id].present? }
       .sort_by { [_1.row_id, positions[dossier.revision_id][_1.type_de_champ_id]] }

config/initializers/flipper.rb

@@ -31,8 +31,7 @@ def setup_features(features)
   :groupe_instructeur_api_hack,
   :hide_instructeur_email,
   :sva,
-  :switch_domain,
-  :champ_update_by_stable_id
+  :switch_domain
 ]
 
 def database_exists?

config/routes.rb

@@ -208,20 +208,6 @@
     get ':dossier_id/:stable_id/piece_justificative', to: 'piece_justificative#show', as: :piece_justificative
     put ':dossier_id/:stable_id/piece_justificative', to: 'piece_justificative#update'
     get ':dossier_id/:stable_id/piece_justificative/template', to: 'piece_justificative#template', as: :piece_justificative_template
-
-    # TODO: remove after migration is ower
-    get ':champ_id/siret', to: 'siret#show', as: :legacy_siret
-    get ':champ_id/rna', to: 'rna#show', as: :legacy_rna
-    delete ':champ_id/options', to: 'options#remove', as: :legacy_options
-
-    get ':champ_id/carte/features', to: 'carte#index', as: :legacy_carte_features
-    post ':champ_id/carte/features', to: 'carte#create'
-    patch ':champ_id/carte/features/:id', to: 'carte#update'
-    delete ':champ_id/carte/features/:id', to: 'carte#destroy'
-
-    get ':champ_id/piece_justificative', to: 'piece_justificative#show', as: :legacy_piece_justificative
-    put ':champ_id/piece_justificative', to: 'piece_justificative#update'
-    get ':champ_id/piece_justificative/template', to: 'piece_justificative#template'
   end
 
   resources :attachments, only: [:show, :destroy]

spec/controllers/champs/carte_controller_spec.rb

@@ -1,30 +1,28 @@
 describe Champs::CarteController, type: :controller do
   let(:user) { create(:user) }
-  let(:procedure) { create(:procedure, :published) }
+  let(:procedure) { create(:procedure, :published, types_de_champ_public: [{ type: :carte, options: { cadastres: true } }]) }
   let(:dossier) { create(:dossier, user: user, procedure: procedure) }
   let(:params) do
     {
       dossier: {
         champs_public_attributes: {
-          '1' => { value: value }
+          champ.public_id => { value: value }
         }
       },
       position: '1',
-      champ_id: champ.id
+      dossier_id: champ.dossier_id,
+      stable_id: champ.stable_id
     }
   end
-  let(:champ) do
-    create(:type_de_champ_carte, options: {
-      cadastres: true
-    }).champ.create(dossier: dossier)
-  end
+  let(:champ) { dossier.champs.first }
 
   describe 'features' do
     let(:feature) { attributes_for(:geo_area, :polygon) }
     let(:geo_area) { create(:geo_area, :selection_utilisateur, :polygon, champ: champ) }
     let(:params) do
       {
-        champ_id: champ.id,
+        dossier_id: champ.dossier_id,
+        stable_id: champ.stable_id,
         feature: feature,
         source: GeoArea.sources.fetch(:selection_utilisateur)
       }
@@ -49,7 +47,8 @@
         let(:feature) { attributes_for(:geo_area, :invalid_point) }
         let(:params) do
           {
-            champ_id: champ.id,
+            dossier_id: champ.dossier_id,
+            stable_id: champ.stable_id,
             feature: feature,
             source: GeoArea.sources.fetch(:selection_utilisateur)
           }
@@ -62,7 +61,8 @@
     describe 'PATCH #update' do
       let(:params) do
         {
-          champ_id: champ.id,
+          dossier_id: champ.dossier_id,
+          stable_id: champ.stable_id,
           id: geo_area.id,
           feature: feature
         }
@@ -101,7 +101,8 @@
     describe 'DELETE #destroy' do
       let(:params) do
         {
-          champ_id: champ.id,
+          dossier_id: champ.dossier_id,
+          stable_id: champ.stable_id,
           id: geo_area.id
         }
       end
@@ -122,7 +123,10 @@
 
       context 'without focus' do
         let(:params) do
-          { champ_id: champ.id }
+          {
+            dossier_id: champ.dossier_id,
+            stable_id: champ.stable_id
+          }
         end
 
         it 'updates the list' do
@@ -134,7 +138,8 @@
       context "update list and focus" do
         let(:params) do
           {
-            champ_id: champ.id,
+            dossier_id: champ.dossier_id,
+            stable_id: champ.stable_id,
             focus: true
           }
         end