Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ETQ dev, je souhaite que les méthodes de manipulation des répétitions soient consolidées et tiennent compte des projections #10715

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions app/controllers/champs/repetition_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@

class Champs::RepetitionController < Champs::ChampController
def add
row = @champ.add_row(@champ.dossier.revision)
@first_champ_id = row.map(&:focusable_input_id).compact.first
@row_id = row.first&.row_id
@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
end

def remove
@champ.remove_row(params[:row_id])
@champ.remove_row(params[:row_id], updated_by: current_user.email)
mfo marked this conversation as resolved.
Show resolved Hide resolved
@to_remove = "safe-row-selector-#{params[:row_id]}"
@to_focus = @champ.focusable_input_id || helpers.dom_id(@champ, :create_repetition)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def resolve(dossier:, annotation_id:, instructeur:)
return { errors: ["L’annotation \"#{annotation_id}\" n’existe pas"] }
end

annotation.add_row(dossier.revision)
annotation.add_row(updated_by: instructeur.email)

{ annotation:, errors: nil }
end
Expand Down
4 changes: 2 additions & 2 deletions app/graphql/types/dossier_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def champs(id: nil)
.for(object, private: false)
.load(ApplicationRecord.id_from_typed_id(id))
else
object.champs_for_revision(scope: :public, root: true).filter(&:visible?)
object.project_champs_public.filter(&:visible?)
end
end

Expand All @@ -180,7 +180,7 @@ def annotations(id: nil)
.for(object, private: true)
.load(ApplicationRecord.id_from_typed_id(id))
else
object.champs_for_revision(scope: :private, root: true).filter(&:visible?)
object.project_champs_private.filter(&:visible?)
end
end

Expand Down
83 changes: 0 additions & 83 deletions app/lib/data_fixer/dossier_champs_missing.rb

This file was deleted.

31 changes: 11 additions & 20 deletions app/models/champs/repetition_champ.rb
Original file line number Diff line number Diff line change
@@ -1,35 +1,26 @@
# frozen_string_literal: true

class Champs::RepetitionChamp < Champ
accepts_nested_attributes_for :champs
delegate :libelle_for_export, to: :type_de_champ

def rows
dossier
.champs_for_revision(scope: type_de_champ)
.group_by(&:row_id)
.sort
.map(&:second)
dossier.project_rows_for(type_de_champ)
end

def row_ids
rows.map { _1.first.row_id }
dossier.repetition_row_ids(type_de_champ)
end

def add_row(revision)
added_champs = []
transaction do
row_id = ULID.generate
revision.children_of(type_de_champ).each do |type_de_champ|
added_champs << type_de_champ.build_champ(row_id:)
end
self.champs << added_champs
end
added_champs
def add_row(updated_by:)
# TODO: clean this up when parent_id is deprecated
row_id, added_champs = dossier.repetition_add_row(type_de_champ, updated_by:)
Copy link
Contributor

@mfo mfo Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spontannément j'aurais inliné la methode dossier.repetition_add_row(type_de_champ, updated_by:) ici (seul appel de cette methode est ici). J'ai l'impression que c'est une indirection de trop, ça me perturbe. T'as voulu collocaliser la gestion des repetitions dans le concern ?

self.champs << added_champs
dossier.champs.reload if dossier.persisted?
row_id
end

def remove_row(row_id)
dossier.champs.where(row_id:).destroy_all
dossier.champs.reload
def remove_row(row_id, updated_by:)
dossier.repetition_remove_row(type_de_champ, row_id, updated_by:)
Copy link
Contributor

@mfo mfo Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spontannément j'aurais inliné la methode dossier.repetition_remove_row(type_de_champ, row_id, updated_by:) ici (seul appel de cette methode est ici). J'ai l'impression que c'est une indirection de trop, ça me perturbe. T'as voulu collocaliser la gestion des repetitions dans le concern ?

end

def focusable_input_id
Expand Down
71 changes: 57 additions & 14 deletions app/models/concerns/dossier_champs_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,10 @@
module DossierChampsConcern
extend ActiveSupport::Concern

def champs_for_revision(scope: nil, root: false)
def champs_for_revision(scope: nil)
tchak marked this conversation as resolved.
Show resolved Hide resolved
champs_index = champs.group_by(&:stable_id)
# Due to some bad data we can have multiple copies of the same champ. Ignore extra copy.
.transform_values { _1.sort_by(&:id).uniq(&:row_id) }

if scope.is_a?(TypeDeChamp)
revision
.children_of(scope)
.flat_map { champs_index[_1.stable_id] || [] }
.filter(&:child?) # TODO: remove once bad data (child champ without a row id) is cleaned
else
revision
.types_de_champ_for(scope:, root:)
.flat_map { champs_index[_1.stable_id] || [] }
end
revision.types_de_champ_for(scope:)
.flat_map { champs_index[_1.stable_id] || [] }
end

# Get all the champs values for the types de champ in the final list.
Expand All @@ -42,6 +31,25 @@ def project_champ(type_de_champ, row_id)
end
end

def project_champs_public
revision.types_de_champ_public.map { project_champ(_1, nil) }
end

def project_champs_private
revision.types_de_champ_private.map { project_champ(_1, nil) }
end

def project_rows_for(type_de_champ)
[] if !type_de_champ.repetition?

children = revision.children_of(type_de_champ)
row_ids = repetition_row_ids(type_de_champ)

row_ids.map do |row_id|
children.map { project_champ(_1, row_id) }
end
end

def find_type_de_champ_by_stable_id(stable_id, scope = nil)
case scope
when :public
Expand Down Expand Up @@ -75,6 +83,41 @@ def update_champs_attributes(attributes, scope, updated_by:)
assign_attributes(champs_attributes:)
end

def repetition_row_ids(type_de_champ)
[] if !type_de_champ.repetition?

stable_ids = revision.children_of(type_de_champ).map(&:stable_id)
champs.filter { _1.stable_id.in?(stable_ids) && _1.row_id.present? }
.map(&:row_id)
.uniq
.sort
end

def repetition_add_row(type_de_champ, updated_by:)
raise "Can't add row to non-repetition type de champ" if !type_de_champ.repetition?

row_id = ULID.generate
types_de_champ = revision.children_of(type_de_champ)
# TODO: clean this up when parent_id is deprecated
added_champs = types_de_champ.map { _1.build_champ(row_id:, updated_by:) }
@champs_by_public_id = nil
[row_id, added_champs]
end

def repetition_remove_row(type_de_champ, row_id, updated_by:)
raise "Can't remove row from non-repetition type de champ" if !type_de_champ.repetition?

champs.where(row_id:).destroy_all
champs.reload if persisted?
@champs_by_public_id = nil
end

def reload
super.tap do
@champs_by_public_id = nil
end
end
tchak marked this conversation as resolved.
Show resolved Hide resolved

private

def champs_by_public_id
Expand Down
4 changes: 2 additions & 2 deletions app/models/concerns/dossier_searchable_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ def index_search_terms

search_terms = [
user&.email,
*champs_public.flat_map(&:search_terms),
*project_champs_public.flat_map(&:search_terms),
*etablissement&.search_terms,
individual&.nom,
individual&.prenom,
mandataire_first_name,
mandataire_last_name
].compact_blank.join(' ')

private_search_terms = champs_private.flat_map(&:search_terms).compact_blank.join(' ')
private_search_terms = project_champs_private.flat_map(&:search_terms).compact_blank.join(' ')

sql = "UPDATE dossiers SET search_terms = :search_terms, private_search_terms = :private_search_terms WHERE id = :id"
sanitized_sql = self.class.sanitize_sql_array([sql, search_terms:, private_search_terms:, id:])
Expand Down
25 changes: 16 additions & 9 deletions app/models/dossier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,10 @@ def build_default_champs_for_new_dossier
champs_private << champ
end
champs_public.filter { _1.repetition? && _1.mandatory? }.each do |champ|
champ.add_row(revision)
champ.add_row(updated_by: nil)
end
champs_private.filter(&:repetition?).each do |champ|
champ.add_row(revision)
champ.add_row(updated_by: nil)
end
end

Expand Down Expand Up @@ -942,14 +942,21 @@ def remove_piece_justificative_file_not_visible!
end

def check_mandatory_and_visible_champs
champs_for_revision(scope: :public)
.filter { _1.child? ? _1.parent.visible? : true }
.filter(&:visible?)
.filter(&:mandatory_blank?)
.map do |champ|
champ.errors.add(:value, :missing)
project_champs_public.filter(&:visible?).each do |champ|
if champ.mandatory_blank?
error = champ.errors.add(:value, :missing)
errors.import(error)
end
.each { errors.import(_1) }
if champ.repetition?
champ.rows.each do |champs|
champs.filter(&:visible?).filter(&:mandatory_blank?).each do |champ|
error = champ.errors.add(:value, :missing)
errors.import(error)
end
end
end
end
errors
end

def demander_un_avis!(avis)
Expand Down
4 changes: 2 additions & 2 deletions app/models/dossier_preloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ def load_dossier(dossier, champs, pj_template: false)
dossier.association(:revision).target = revision
end
dossier.association(:champs).target = champs
dossier.association(:champs_public).target = dossier.champs_for_revision(scope: :public, root: true)
dossier.association(:champs_private).target = dossier.champs_for_revision(scope: :private, root: true)
dossier.association(:champs_public).target = dossier.project_champs_public
dossier.association(:champs_private).target = dossier.project_champs_private

# remove once parent_id is deprecated
champs_by_parent_id = champs.group_by(&:parent_id)
Expand Down
13 changes: 4 additions & 9 deletions app/models/procedure_revision.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,19 +179,14 @@ def dossier_for_preview(user)
dossier
end

def types_de_champ_for(scope: nil, root: false)
# We return an unordered collection
return types_de_champ if !root && scope.nil?
return types_de_champ.filter { scope == :public ? _1.public? : _1.private? } if !root

# We return an ordered collection
def types_de_champ_for(scope: nil)
tchak marked this conversation as resolved.
Show resolved Hide resolved
case scope
when :public
types_de_champ_public
types_de_champ.filter(&:public?)
when :private
types_de_champ_private
types_de_champ.filter(&:private?)
else
types_de_champ_public + types_de_champ_private
types_de_champ
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ def initialize(champ, repetition, index, revision)
def to_assignable_attributes
return unless repetition.is_a?(Hash)

row = champ.rows[index] || champ.add_row(revision)
row_id = row.first.row_id
row_id = champ.row_ids[index] || champ.add_row(updated_by: nil)

repetition.map do |key, value|
next unless key.is_a?(String) && key.starts_with?("champ_")
Expand Down
6 changes: 1 addition & 5 deletions app/serializers/champ_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,7 @@
end

def rows
object.dossier
.champs_for_revision(scope: object.type_de_champ)
.group_by(&:row_id)
.values
.map.with_index(1) { |champs, index| Row.new(index:, champs:) }
object.rows.map.with_index(1) { |champs, index| Row.new(index:, champs:) }

Check warning on line 51 in app/serializers/champ_serializer.rb

View check run for this annotation

Codecov / codecov/patch

app/serializers/champ_serializer.rb#L51

Added line #L51 was not covered by tests
end

def include_etablissement?
Expand Down
Loading
Loading