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

refactor(champs): update champs by public_id [remove dead code]
This commit is contained in:
Paul Chavard 2024-05-27 12:38:37 +00:00 committed by GitHub
commit dbfd5eed8b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
27 changed files with 50 additions and 245 deletions

View file

@ -11,10 +11,6 @@ class EditableChamp::CarteComponent < EditableChamp::EditableChampBaseComponent
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
end
end

View file

@ -54,17 +54,9 @@ class EditableChamp::EditableChampComponent < ApplicationComponent
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
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
end
end

View file

@ -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

View file

@ -10,10 +10,6 @@ class EditableChamp::MultipleDropDownListComponent < EditableChamp::EditableCham
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
end
end

View file

@ -4,10 +4,6 @@ class EditableChamp::RNAComponent < EditableChamp::EditableChampBaseComponent
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
end
end

View file

@ -12,10 +12,6 @@ class EditableChamp::SiretComponent < EditableChamp::EditableChampBaseComponent
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
end
end

View file

@ -5,16 +5,10 @@ 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
end
def params_row_id
params[:row_id]

View file

@ -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

View file

@ -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?)

View file

@ -275,7 +275,7 @@ module Instructeurs
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 @@ module Instructeurs
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 @@ module Instructeurs
:accreditation_number,
:accreditation_birthdate,
:feature,
:with_public_id,
value: []
]
# Strong attributes do not support records (indexed hash); they only support hashes with

View file

@ -319,13 +319,8 @@ module Users
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 @@ module Users
: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 @@ module Users
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

View file

@ -9,11 +9,7 @@ module ChampHelper
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
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?

View file

@ -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 @@ class Champ < ApplicationRecord
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")

View file

@ -65,13 +65,6 @@ module DossierChampsConcern
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

View file

@ -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 @@ class Dossier < ApplicationRecord
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

View file

@ -93,8 +93,6 @@ class DossierPreloader
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 @@ class DossierPreloader
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]] }

View file

@ -31,8 +31,7 @@ features = [
:groupe_instructeur_api_hack,
:hide_instructeur_email,
:sva,
:switch_domain,
:champ_update_by_stable_id
:switch_domain
]
def database_exists?

View file

@ -208,20 +208,6 @@ Rails.application.routes.draw do
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]

View file

@ -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 @@ describe Champs::CarteController, type: :controller do
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 Champs::CarteController, type: :controller do
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 Champs::CarteController, type: :controller do
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 @@ describe Champs::CarteController, type: :controller do
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 @@ describe Champs::CarteController, type: :controller do
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

View file

@ -18,13 +18,5 @@ describe Champs::OptionsController, type: :controller do
expect { subject }.to change { champ.reload.selected_options.size }.from(2).to(1)
end
end
context 'with champ_id' do
subject { delete :remove, params: { champ_id: champ.id, option: 'tata' }, format: :turbo_stream }
it 'remove option' do
expect { subject }.to change { champ.reload.selected_options.size }.from(2).to(1)
end
end
end
end

View file

@ -121,7 +121,7 @@ describe Champs::RNAController, type: :controller do
end
context 'when user is not signed in' do
subject! { get :show, params: { champ_id: champ.id }, format: :turbo_stream }
subject! { get :show, params: { dossier_id: champ.dossier_id, stable_id: champ.stable_id }, format: :turbo_stream }
it { expect(response.code).to redirect_to(new_user_session_path) }
end

View file

@ -151,7 +151,7 @@ describe Champs::SiretController, type: :controller do
end
context 'when user is not signed in' do
subject! { get :show, params: { champ_id: champ.id }, format: :turbo_stream }
subject! { get :show, params: { dossier_id: champ.dossier_id, stable_id: champ.stable_id }, format: :turbo_stream }
it { expect(response).to redirect_to(new_user_session_path) }
end

View file

@ -1006,24 +1006,19 @@ describe Instructeurs::DossiersController, type: :controller do
dossier: {
champs_private_attributes: {
champ_multiple_drop_down_list.public_id => {
with_public_id: true,
value: ['', 'val1', 'val2']
},
champ_datetime.public_id => {
with_public_id: true,
value: '2019-12-21T13:17'
},
champ_linked_drop_down_list.public_id => {
with_public_id: true,
primary_value: 'primary',
secondary_value: 'secondary'
},
champ_repetition.champs.first.public_id => {
with_public_id: true,
value: 'text'
},
champ_drop_down_list.public_id => {
with_public_id: true,
value: '__other__',
value_other: 'other value'
}
@ -1079,29 +1074,6 @@ describe Instructeurs::DossiersController, type: :controller do
Timecop.return
end
context "with new values for champs_private (legacy)" do
let(:params) do
{
procedure_id: procedure.id,
dossier_id: dossier.id,
dossier: {
champs_private_attributes: {
'0': {
id: champ_datetime.id,
value: '2024-03-30T07:03'
}
}
}
}
end
it 'update champs_private' do
patch :update_annotations, params: params, format: :turbo_stream
champ_datetime.reload
expect(champ_datetime.value).to eq(Time.zone.parse('2024-03-30T07:03:00').iso8601)
end
end
context "without new values for champs_private" do
let(:params) do
{
@ -1111,7 +1083,6 @@ describe Instructeurs::DossiersController, type: :controller do
champs_private_attributes: {},
champs_public_attributes: {
champ_multiple_drop_down_list.public_id => {
with_public_id: true,
value: ['', 'val1', 'val2']
}
}
@ -1141,7 +1112,6 @@ describe Instructeurs::DossiersController, type: :controller do
dossier: {
champs_private_attributes: {
champ_datetime.public_id => {
with_public_id: true,
value: '2024-03-30T07:03'
}
}

View file

@ -675,11 +675,9 @@ describe Users::DossiersController, type: :controller do
groupe_instructeur_id: dossier.groupe_instructeur_id,
champs_public_attributes: {
first_champ.public_id => {
with_public_id: true,
value: value
},
piece_justificative_champ.public_id => {
with_public_id: true,
piece_justificative_file: file
}
}
@ -713,22 +711,6 @@ describe Users::DossiersController, type: :controller do
expect(dossier.reload.updated_at.year).to eq(2100)
expect(dossier.reload.state).to eq(Dossier.states.fetch(:brouillon))
end
context 'without new values for champs' do
let(:submit_payload) do
{
id: dossier.id,
dossier: {
champs_public_attributes: { first_champ.public_id => { with_public_id: true } }
}
}
end
it "doesn't set last_champ_updated_at" do
subject
expect(dossier.reload.last_champ_updated_at).to eq(nil)
end
end
end
context 'when the user has an invitation but is not the owner' do
@ -747,7 +729,7 @@ describe Users::DossiersController, type: :controller do
{
id: dossier.id,
dossier: {
champs_public_attributes: { first_champ.public_id => { with_public_id: true, value: value } }
champs_public_attributes: { first_champ.public_id => { value: value } }
}
}
end
@ -801,11 +783,9 @@ describe Users::DossiersController, type: :controller do
groupe_instructeur_id: dossier.groupe_instructeur_id,
champs_public_attributes: {
first_champ.public_id => {
with_public_id: true,
value: value
},
piece_justificative_champ.public_id => {
with_public_id: true,
piece_justificative_file: file
}
}
@ -866,7 +846,6 @@ describe Users::DossiersController, type: :controller do
dossier: {
champs_public_attributes: {
piece_justificative_champ.public_id => {
with_public_id: true,
piece_justificative_file: file
}
}
@ -962,7 +941,6 @@ describe Users::DossiersController, type: :controller do
dossier: {
champs_public_attributes: {
first_champ.public_id => {
with_public_id: true,
value: value
}
}

View file

@ -181,9 +181,9 @@ RSpec.describe DossierChampsConcern do
let(:attributes) do
{
"99" => { value: "Hello", with_public_id: true },
"991" => { value: "World", with_public_id: true },
"994-#{row_id}" => { value: "Greer", with_public_id: true }
"99" => { value: "Hello" },
"991" => { value: "World" },
"994-#{row_id}" => { value: "Greer" }
}
end
@ -218,36 +218,12 @@ RSpec.describe DossierChampsConcern do
expect(champ_994.value).to eq("Greer")
}
end
context 'legacy attributes' do
let(:attributes) do
{
champ_99.id => { value: "Hello", id: champ_99.id },
champ_991.id => { value: "World", id: champ_991.id },
champ_994.id => { value: "Greer", id: champ_994.id }
}
end
let(:champ_99_updated) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(99), nil) }
let(:champ_991_updated) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(991), nil) }
let(:champ_994_updated) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(994), row_id) }
it {
subject
expect(dossier.champs_public_all.any?(&:changed_for_autosave?)).to be_truthy
dossier.save
dossier.reload
expect(champ_99_updated.value).to eq("Hello")
expect(champ_991_updated.value).to eq("World")
expect(champ_994_updated.value).to eq("Greer")
}
end
end
describe "#update_champs_attributes(private)" do
let(:attributes) do
{
"995" => { value: "Hello", with_public_id: true }
"995" => { value: "Hello" }
}
end
@ -272,23 +248,5 @@ RSpec.describe DossierChampsConcern do
expect(annotation_995.value).to eq("Hello")
}
end
context 'legacy attributes' do
let(:attributes) do
{
annotation_995.id => { value: "Hello", id: annotation_995.id }
}
end
let(:annotation_995_updated) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(995), nil) }
it {
subject
expect(dossier.champs_private_all.any?(&:changed_for_autosave?)).to be_truthy
dossier.save
dossier.reload
expect(annotation_995_updated.value).to eq("Hello")
}
end
end
end

View file

@ -347,7 +347,7 @@ describe DossierRebaseConcern do
it "updates the brouillon champs with the latest revision changes" do
expect(dossier.revision).to eq(procedure.published_revision)
expect(dossier.champs_public.size).to eq(5)
expect(dossier.champs_public_all.size).to eq(7)
expect(dossier.champs.count(&:public?)).to eq(7)
expect(repetition_champ.rows.size).to eq(2)
expect(repetition_champ.rows[0].size).to eq(1)
expect(repetition_champ.rows[1].size).to eq(1)
@ -360,7 +360,7 @@ describe DossierRebaseConcern do
expect(procedure.revisions.size).to eq(3)
expect(dossier.revision).to eq(procedure.published_revision)
expect(dossier.champs_public.size).to eq(6)
expect(dossier.champs_public_all.size).to eq(12)
expect(dossier.champs.count(&:public?)).to eq(12)
expect(rebased_text_champ.value).to eq(text_champ.value)
expect(rebased_text_champ.type_de_champ_id).not_to eq(text_champ.type_de_champ_id)
expect(rebased_datetime_champ.type_champ).to eq(TypeDeChamp.type_champs.fetch(:date))

View file

@ -29,13 +29,13 @@ describe DossierPreloader do
expect(first_child.type).to eq('Champs::TextChamp')
expect(repetition.id).not_to eq(first_child.id)
expect(subject.champs.first.dossier).to eq(subject)
expect(subject.champs_public_all.first.dossier).to eq(subject)
expect(subject.champs.find(&:public?).dossier).to eq(subject)
expect(subject.champs_public.first.dossier).to eq(subject)
expect(subject.champs_public.first.type_de_champ.piece_justificative_template.attached?).to eq(false)
expect(subject.champs.first.conditional?).to eq(false)
expect(subject.champs_public_all.first.conditional?).to eq(false)
expect(subject.champs.find(&:public?).conditional?).to eq(false)
expect(subject.champs_public.first.conditional?).to eq(false)
expect(first_child.parent).to eq(repetition)