Skip to content

Comments

fix(import): make config default_value_mapping multi-dest#3801

Open
VincentCauchois wants to merge 1 commit intodevelopfrom
fix/allow-destination-in-config-import-default-value-mapping
Open

fix(import): make config default_value_mapping multi-dest#3801
VincentCauchois wants to merge 1 commit intodevelopfrom
fix/allow-destination-in-config-import-default-value-mapping

Conversation

@VincentCauchois
Copy link
Member

Replace GN [IMPORT] config param DEFAULT_VALUE_MAPPING_ID - single ID used for whatever destination - with DEFAULT_VALUE_MAPPINGS. This allows to specify one default mapping for each destination, which is necessary as a mapping should be destination-wise and is actually associated to one destination at most.

@VincentCauchois VincentCauchois self-assigned this Nov 24, 2025
@VincentCauchois VincentCauchois force-pushed the fix/allow-destination-in-config-import-default-value-mapping branch 2 times, most recently from afb91ac to 2e21623 Compare November 24, 2025 15:32
@VincentCauchois VincentCauchois changed the title fix(import): make config default_value_mapping multi-dest fix(import): make config default_value_mapping multi-dest Nov 24, 2025
@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.20%. Comparing base (f3acce7) to head (f9b4852).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3801      +/-   ##
===========================================
- Coverage    86.21%   86.20%   -0.02%     
===========================================
  Files          141      141              
  Lines        10933    10935       +2     
===========================================
  Hits          9426     9426              
- Misses        1507     1509       +2     
Flag Coverage Δ
pytest 86.20% <100.00%> (-0.02%) ⬇️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@camillemonchicourt
Copy link
Member

camillemonchicourt commented Dec 31, 2025

OK pour moi avec les ID par défaut. C'était déjà comme ça, y a pas d'autre solution simple et c'est une fonctionnalité peu (pas ?) utilisée.

Et c'est plutôt sur ce point que je m'interroge.
Toute la gestion des valeurs par défaut est bien complexe dans le module Import, et on a plusieurs évoqué le fait que pouvoir masquer l'étape de mapping des nomenclatures avait peu d'intérêt et privait les utilisateurs de bien faire correspondre leurs données aux valeurs attendues.
Ne pas proposer cette étape encourage les utilisateurs à ne pas mapper ces colonnes et donc à fournir moins d'infos que ceux dont ils disposent, en attribuant NON RENSEIGNE à des champs pour lesquelles ils avaient pourtant des données...

Je ne pense pas que la possibilité de masquer l'étape de mapping des nomenclatures soit vraiment utilisé et je le déconseille.
Je ne sais pas si dans DEPOBIO cette étape est toujours masquée, car j'avais compris qu'on voulait permettre aux utilisateurs de mapper leurs nomenclatures pour améliorer le complétude et la qualité des données fournies et leur faciliter la vie si ils ont des nomenclatures un peu différentes dans leurs propres outils.
Si cela est encore utilisé dans DEPOBIO, je ne pense pas que cela soit le cas ailleurs et j'irai dans le sens de supprimer toute cette partie de fonctionnalité qui complique le module IMPORT et sa partie "valeurs par défaut" déjà bien complexe à comprendre, tester et maintenir.

Une partie de récapitulatif sur le fonctionnement de cette partie est disponible sur PnX-SI/gn_module_import#68 (comment).

Replace GN [IMPORT] config param `DEFAULT_VALUE_MAPPING_ID` - single ID used for whatever destination - with `DEFAULT_VALUE_MAPPINGS`.
This allows to specify one default mapping for each destination, which is necessary as a mapping should be destination-wise and is actually associated to one destination at most.
@jacquesfize jacquesfize force-pushed the fix/allow-destination-in-config-import-default-value-mapping branch from 2e21623 to f9b4852 Compare February 2, 2026 14:30
@jacquesfize jacquesfize added this to the 2.17.0 milestone Feb 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants