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

[IMPORT] Feat context parameter import #3277

Open
wants to merge 13 commits into
base: feat/import-monitorings
Choose a base branch
from

Conversation

andriacap
Copy link
Contributor

@andriacap andriacap commented Dec 6, 2024

TACHES PROJET IMPORT :

feat(wip): get context import by query params url

  • Format params to FieldsMapping interface

  • Send on upload route frontend

  • Set fieldsmapping for t_import on upload route backend

  • Test backend

  • Test frontend : un test e2e a été ajouté pour tester la fonctionnalité

  • Tests frontend existants : certains sont fonctionnels mais ne sont pas lié aux développements de cette fonctionnalité (à résoudre dans une autre PR ?)

    Reviewed-by: andriacap

Tâche(s) dépendante(s)

@andriacap andriacap force-pushed the feat/context-parameter-import branch 2 times, most recently from b4efac7 to ef99612 Compare December 6, 2024 17:19
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.95%. Comparing base (719b85f) to head (2edacce).

Additional details and impacted files
@@                   Coverage Diff                    @@
##           feat/import-monitorings    #3277   +/-   ##
========================================================
  Coverage                    83.94%   83.95%           
========================================================
  Files                          122      122           
  Lines                         9721     9727    +6     
========================================================
+ Hits                          8160     8166    +6     
  Misses                        1561     1561           
Flag Coverage Δ
pytest 83.95% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@jacquesfize jacquesfize changed the base branch from develop to feat/import-monitorings December 9, 2024 12:06
@andriacap andriacap force-pushed the feat/context-parameter-import branch 2 times, most recently from 554881c to aca2da1 Compare December 16, 2024 14:37
@andriacap andriacap force-pushed the feat/context-parameter-import branch from aca2da1 to 164e01e Compare January 2, 2025 15:03
jacquesfize and others added 4 commits January 3, 2025 12:56
* feat(import): can post fieldmapping in new structured format
* fix(import): insert_import_data_in_transient_table
* fix(import): get_import_values
* fix(import): prepare_import
* feat(import): support default values
* feat(import): default value edition support nomenclature widget
 refactor(import): minor refact on getFieldMappingValues()
* feat(import): improve default value json edition
* feat(import): present default values on report
* feat(import): db migration
* feat(import): test default value
* test(import): support fieldmapping format v2
* feat(import): dynamic form on default values
* fix(import): no longer send empty string for date default value
* fix(import): test field jsonschema_definitions
* feat(import): fix occhab revisions
* feat(import): rename obsolete revision
* style(import): code format
* feat(import): fieldmapping validators deal with default values
* feat(import): occhab revision to set bib_fields.type_field
* Fix (migration) move migration to geonature branch

---------

Co-authored-by: Pierre-Narcisi <[email protected]>
…ing model (PnX-SI#3275)

* add id_import in monitoring model
* feat: add id_import columns in monitoring tables
* fix(db): change head of revision to latest in feat/import-monitorings

---------

Co-authored-by: Julien Corny <[email protected]>
Co-authored-by: jacquesfize <[email protected]>
@jacquesfize jacquesfize force-pushed the feat/import-monitorings branch from 2b5f26f to dcc764e Compare January 3, 2025 12:39
jacquesfize and others added 7 commits January 3, 2025 13:43
- Format params to FieldsMapping interface
- Send on upload route frontend
- Set fieldsmapping for t_import on upload route backend

Reviewed-by: andriacap
- Synthese test frontend (ok)
- wip : make occhab condition and refact test

Reviewed-by: andriacap
Use same attribute for QA in all elements in fields mapping step

Reviewed-by: andriacap
- Change fieldmapping for sample data
- Add delete command to each test for fieldmapping context

Reviewed-by: andriacap
Reviewed-by: andriacap
@andriacap andriacap force-pushed the feat/context-parameter-import branch from 3956690 to 85dca11 Compare January 3, 2025 13:29
- Test upload_route with context parameter

Reviewed-by: andriacap
@andriacap andriacap marked this pull request as ready for review January 6, 2025 08:40
@Pierre-Narcisi
Copy link
Contributor

J'ai testé à la main et tout a l'air de fonctionner correctement, juste pour les tests frontend, j'ai l'impression que les erreurs viennent de conflit dans les data-qa, je pense que c'est parce que certain champ sont à la fois dans les habitats et dans les stations, dans ce cas le champ existe deux fois, ce qui fait que deux champs avec le même data-qa existe et ça fait planter les tests.

@@ -91,9 +104,9 @@ export class UploadFileStepComponent implements OnInit {
}
onSaveData(): Observable<Import> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Attention, onSaveData n'est exécuté seulement si le formulaire est modifié ! par conséquent, si je reviens en arrière pour modifier uniquement les valeurs dans les queryParams, cela ne fonctionnera pas ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merci pour ton retour Jacques. Est ce que tu pourrais me détailler le cas d'usage pour être sur de comprendre ? Quel est le comportement souhaité ?

Merci

Copy link
Contributor

Choose a reason for hiding this comment

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

C'est pas très grave. En fait, si je reviens sur l'étape de téléversement pour modifier changer les champs pré-mappés c'est pas possible. Mais je chercher la petite bête :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ca roule alors :)

@andriacap
Copy link
Contributor Author

andriacap commented Jan 20, 2025

J'ai testé à la main et tout a l'air de fonctionner correctement, juste pour les tests frontend, j'ai l'impression que les erreurs viennent de conflit dans les data-qa, je pense que c'est parce que certain champ sont à la fois dans les habitats et dans les stations, dans ce cas le champ existe deux fois, ce qui fait que deux champs avec le même data-qa existe et ça fait planter les tests.

Merci Pierre pour ton retour. Est ce que vous êtes d'accord si on traite les erreurs de tests frontend dans une tâche à part ? Etant donné que les tests frontend ne fonctionnent pas sur la branche de référence feat/import-monitorings

@Pierre-Narcisi Pierre-Narcisi force-pushed the feat/import-monitorings branch from 719b85f to bfcf4a7 Compare January 20, 2025 16:12
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.

5 participants