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

fix(db): update taxref_version in commons.sql #2339

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

mvergez
Copy link
Contributor

@mvergez mvergez commented Feb 15, 2023

Salut !

Suite à une installation complète de la dernière version (2.11) de GeoNature, taxref est automatiquement mis à jour :

geonature taxref import-v15
else
geonature taxref import-v15 --skip-bdc-statuts
mais gn_commons.t_parameters n'est pas modifié. Le but de cette PR est de mettre à jour cette table

@camillemonchicourt
Copy link
Member

Oui bien vu.
Dans la 2.11, c'est Taxref v15 qui est installé par défaut (et non pas mis à jour automatiquement), mais en effet on a oublié d'incrémenter le paramètre taxref_version.

@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.81 ⚠️

Comparison is base (fc507ac) 69.16% compared to head (7dc21a6) 68.36%.

❗ Current head 7dc21a6 differs from pull request most recent head f5500c1. Consider uploading reports for the commit f5500c1 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2339      +/-   ##
===========================================
- Coverage    69.16%   68.36%   -0.81%     
===========================================
  Files           81       82       +1     
  Lines         7087     7289     +202     
===========================================
+ Hits          4902     4983      +81     
- Misses        2185     2306     +121     
Flag Coverage Δ
pytest 68.36% <ø> (-0.81%) ⬇️

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

see 36 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mvergez
Copy link
Contributor Author

mvergez commented Feb 15, 2023

Merci @camillemonchicourt pour ton retour. En effet bien, vu, j'avais pas suivi ces modifs : 906305b

@bouttier
Copy link
Contributor

Je suis pour supprimer ce paramètre, il n’amène que de la confusion car il n’est jamais mis à jour, ne faisant pas partie du processus d’upgrade de taxref, et ne pouvant en faire partie car appartenant à un schéma GeoNature.

Je suis pour la création d’une commande d’auto-détection de la version de taxref (à partir de cd_nom connu pour avoir été introduit ou supprimé dans telle ou telle version de taxref).

@mvergez
Copy link
Contributor Author

mvergez commented Feb 20, 2023

Je suis d'accord avec toi pour supprimer ce paramètre car il n'a rien avoir avec GeoNature. Mais pour la commande d'auto-detection à voir si ta technique est robuste.
Pourquoi ne pas simplement figer cette version dans la bdd de Taxref ? Qui sera écrite à la fin de apply_changes

@camillemonchicourt
Copy link
Member

Ce paramètre est utilisé dans la table pr_occtax.t_occurrences_occtax.meta_v_taxref et gn_synthese.synthese.meta_v_taxref.

C'est discutable et potentiellement (certainement ?) faux, mais cela est fait pour indiquer avec quelle version de Taxref cette observation a été identifiée. Donc à quelle version de Taxref correspond le champs nom_cite, le champs cd_nom pouvant être modifié lors de mise à jour de Taxref, si un cd_nom est remplacé, supprimé, fusionné, splitté...

Il faut voir si garder ces champs meta_v_taxref a du sens, n'est pas faux actuellement et est utile pour le SINP.

Dans la v2 du standard Occurrence il est indiqué la suppression de l'attribut versionTAXREF qui avait été ajouté dans la version 1.2 du standard, avec cette raison :

versionTAXREF (pas nécessaire quand on ne fournit qu'un cdNom)

@bouttier bouttier added this to the 2.12 milestone Mar 21, 2023
@bouttier
Copy link
Contributor

Bon, vue que :

  • le paramètre est actuellement utilisé par les triggers d’OccTax
  • il faudrait le déplacer vers taxhub afin de gérer sa mise à jour mais on est même pas sûr que son utilité demeure au sein d’occtax
    Je merge cette PR, après l’avoir modifié pour pointer sur TaxRef v16, version cible de la 2.12.

As new GeoNature installations are provided with TaxRef v16.
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.

None yet

3 participants