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

Conversation

tchak
Copy link
Member

@tchak tchak commented Aug 26, 2024

No description provided.

Copy link

what-the-diff bot commented Aug 26, 2024

PR Summary

  • Controller and Preloader updates

    • The dossiers_controller.rb file introduces a line to preload a DossierPreloader.
    • Updates to champ_controller.rb and repetition_controller.rb improve row addition and removal actions by tracking which user made these updates.
  • Revised Method Usage

    • Several files, including dossier_type.rb, attestation_template.rb, dossier_champs_concern.rb, and dossier_searchable_concern.rb have been revised to replace champs_for_revision with project_champs_public, project_champs_private and related methods. This alteration aims to improve alignment and consistency across methods.
  • Enhancements in Champ Processing

    • Updates in several files, including champ.rb, repetition_champ.rb, and champs_validate_concern.rb, refine how champs are validated, interpreted, and processed, particularly considering if the champ is a repetition and if it is part of the dossier revision.
  • Dossier and Procedure Revamps

    • The dossier.rb and procedure.rb files have been adjusted to improve handling of dossiers using newer methods for public and private champs as well as implementing user tracking in updates.
  • Improved Serialization and Tests

    • Changes in the champ_serializer.rb file and numerous testing files reflect the wider method updates, ensuring tests remain relevant and effective as code evolves.
  • Instruction Button Motivation Update

    • The _instruction_button_motivation.html.haml file has been updated to replace unspecified_annotations_privees and unspecified_champs with unspecified_attestation_champs.partition(&:private?).
  • New 'in_dossier_revision?' Checks

    • Several _champ_spec.rb files have had in_dossier_revision? checks added to support expanded champ processing and validation.

This PR endeavors to enhance how the application handles champs and their various contexts, states, and values. Improvements in user tracking also provide greater insight and traceability into champ updates.

@tchak tchak force-pushed the refactor-repetition branch from f2cfcbd to 7ec8b25 Compare August 27, 2024 08:31
Copy link

codecov bot commented Aug 27, 2024

Codecov Report

Attention: Patch coverage is 98.63014% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.69%. Comparing base (9167e9a) to head (a8e605f).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
app/models/procedure_revision.rb 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10715   +/-   ##
=======================================
  Coverage   84.68%   84.69%           
=======================================
  Files        1132     1130    -2     
  Lines       25098    25064   -34     
  Branches     4703     4701    -2     
=======================================
- Hits        21255    21228   -27     
+ Misses       3843     3836    -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tchak tchak force-pushed the refactor-repetition branch from 7ec8b25 to a167bd4 Compare August 27, 2024 09:15
@tchak tchak force-pushed the refactor-repetition branch 3 times, most recently from 7900fd4 to bb5b82a Compare September 24, 2024 09:26
@tchak tchak changed the title Refactor repetition refactor(repetition): consolidate repetition manipulation methods Sep 24, 2024
@tchak tchak marked this pull request as ready for review September 24, 2024 09:34
@tchak tchak changed the title refactor(repetition): consolidate repetition manipulation methods ETQ dev, je souhaite que les méthodes de manipulation des répétitions soient consolidées et tiennent compte des projections Sep 24, 2024
@tchak tchak force-pushed the refactor-repetition branch 3 times, most recently from 2a5a9ca to 84590b9 Compare September 26, 2024 13:37
Copy link
Contributor

@mfo mfo left a comment

Choose a reason for hiding this comment

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

1ere passe, c'est canon !

Voici un petit patch pour virer du code mort qui utilisait le DataFixer::DossierChampsMissing
0001-clean-deadcode-remove-deadcode.patch

app/models/concerns/dossier_champs_concern.rb Show resolved Hide resolved
app/models/procedure_revision.rb Show resolved Hide resolved
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 ?

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 ?

@tchak tchak force-pushed the refactor-repetition branch from 84590b9 to a8e605f Compare September 27, 2024 09:35
@tchak tchak enabled auto-merge September 27, 2024 09:35
@tchak tchak added this pull request to the merge queue Sep 27, 2024
Merged via the queue into demarches-simplifiees:main with commit 2a80091 Sep 27, 2024
18 checks passed
@tchak tchak deleted the refactor-repetition branch September 27, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants