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

refactor: remove champ factories #10569

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Conversation

tchak
Copy link
Member

@tchak tchak commented Jul 1, 2024

some failing specs

rspec ./spec/models/etablissement_spec.rb:121 # Etablissement update search terms schedule update search terms

Copy link

what-the-diff bot commented Jul 1, 2024

PR Summary

  • Added types_de_champ_public
    The feature types_de_champ_public has been added to the files api/v2/graphql_controller_spec.rb, dossier.rb, and dossier_spec.rb, and procedure_export_service_spec.rb. This indicates enhanced functionalities related to the public fields of forms or documents.

  • Removed association :champ from geo_area.rb file
    An association champ has been stripped off from the geo_area.rb file suggesting a possible restructuring or improvement in the management of geographical data.

  • Added procedure, types_de_champ_public, and types_de_champ_private
    These attributes were added to the dossier_spec.rb file, suggesting further enhancements to existing capabilities for managing documents or forms.

  • Added declarative_with_state and procedure
    The implementation of these features in the dossier_spec.rb file may improve how forms or documents status and handling are carried out.

  • Removed etablissement and champ
    These were removed from the procedure_export_service_spec.rb file, indicating changes in how establishment data and form fields are exported.

  • Added procedure and dossier
    The addition of these features to the serializer_service_spec.rb and views/shared/champs/multiple_drop_down_list/_show.html.haml_spec.rb files indicate improved functionalities in the serialization and user interface relating to forms or document handling.

@tchak tchak force-pushed the refactor-remove-champ-factories branch from 78f6ce8 to 6e75a4e Compare July 1, 2024 14:46
Copy link

codecov bot commented Jul 1, 2024

Codecov Report

Attention: Patch coverage is 90.90909% with 3 lines in your changes missing coverage. Please review.

Project coverage is 80.23%. Comparing base (860e062) to head (229483d).

Files Patch % Lines
...jobs/migrations/backfill_dossier_repetition_job.rb 0.00% 2 Missing ⚠️
app/models/dossier_preloader.rb 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10569      +/-   ##
==========================================
- Coverage   80.45%   80.23%   -0.23%     
==========================================
  Files        1235     1233       -2     
  Lines       26309    26263      -46     
  Branches     4725     4716       -9     
==========================================
- Hits        21166    21071      -95     
- Misses       5143     5192      +49     

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

@tchak tchak force-pushed the refactor-remove-champ-factories branch 3 times, most recently from 7076a17 to 0cfab44 Compare July 2, 2024 08:10
@mfo mfo force-pushed the refactor-remove-champ-factories branch from bc039c4 to a865082 Compare July 2, 2024 14:49
@tchak tchak marked this pull request as ready for review July 2, 2024 16:35
@tchak tchak force-pushed the refactor-remove-champ-factories branch 4 times, most recently from 2018a2a to 9f945dd Compare July 8, 2024 16:01
@mfo mfo force-pushed the refactor-remove-champ-factories branch from 84b731e to 6a10a89 Compare July 9, 2024 13:14
context 'when creating the model directly' do
let(:champ_text_row_1) { create(:champ_text, type_de_champ: tdc_text, row_id: ULID.generate, parent: champ, dossier: nil) }

it 'associates nested champs to the parent dossier' do
Copy link
Contributor

Choose a reason for hiding this comment

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

l'impression que cette spec ne servait a rien si ce n'est de tester les factories

let(:data) { "data" }
subject { champ.update_with_external_data!(data: data) }

it { expect { subject }.to change { champ.reload.data }.to(data) }
Copy link
Contributor

Choose a reason for hiding this comment

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

same, on testait un update!(data:)

let(:champ_pj) { create (:champ_piece_justificative) }
it { is_expected.to be_falsy }
end
end
Copy link
Contributor

@mfo mfo Jul 9, 2024

Choose a reason for hiding this comment

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

deadspec – il y avait un attribute skip_validation sur les champs, en 2020...

@tchak tchak force-pushed the refactor-remove-champ-factories branch 2 times, most recently from e9e1066 to 9d19ba6 Compare July 9, 2024 15:53
# reload: it can be out of sync in test if some tdcs are added wihtout using add_tdc
types_de_champ_public.reload.map(&:build_champ)
types_de_champ_public.reload.map { _1.build_champ(dossier:) }
Copy link
Contributor

Choose a reason for hiding this comment

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

le reload ne serait-t'il pas de trop ? ou p-e le conditionné à l'env de test car il n'y a pas d'interet en prod

# reload: it can be out of sync in test if some tdcs are added wihtout using add_tdc
types_de_champ_public.reload.map(&:build_champ)
types_de_champ_public.reload.map { _1.build_champ(dossier:) }
Copy link
Contributor

@mfo mfo Jul 10, 2024

Choose a reason for hiding this comment

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

Suggested change
types_de_champ_public.reload.map { _1.build_champ(dossier:) }
types_de_champ_public.reload if Rails.env.test?
types_de_champ_public.map { _1.build_champ(dossier:) }

Copy link
Member Author

Choose a reason for hiding this comment

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

j'ai un refactoring sur ce code qui arrive dans la prochaine PR. Je n'ai pas trop envie de creuser ça maintenant.

# reload: it can be out of sync in test if some tdcs are added wihtout using add_tdc
types_de_champ_private.reload.map(&:build_champ)
types_de_champ_private.reload.map { _1.build_champ(dossier:) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
types_de_champ_private.reload.map { _1.build_champ(dossier:) }
types_de_champ_private.reload if Rails.env.test?
types_de_champ_private.map { _1.build_champ(dossier:) }

@tchak tchak force-pushed the refactor-remove-champ-factories branch 3 times, most recently from c2c4c2b to f9026b7 Compare July 15, 2024 07:21
@tchak tchak force-pushed the refactor-remove-champ-factories branch 2 times, most recently from 59c74b7 to 2b46f67 Compare July 22, 2024 10:47
@tchak tchak force-pushed the refactor-remove-champ-factories branch from 2b46f67 to 229483d Compare July 22, 2024 11:54
@tchak tchak added this pull request to the merge queue Jul 22, 2024
Merged via the queue into main with commit bf1a057 Jul 22, 2024
18 checks passed
@tchak tchak deleted the refactor-remove-champ-factories branch July 22, 2024 14:04
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