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

(PC-32841) script: fix the offerVenue field for collective offers whe… #14935

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jcicurel-pass
Copy link
Contributor

…re otherAddress is null

But de la pull request

Ticket Jira (ou description si BSR) : https://passculture.atlassian.net/browse/PC-32841

Pour les offres avec offerVenue["otherAddress"] à null, on passe la valeur à "" à la place comme attendu par les schémas

Vérifications

  • J'ai écrit les tests nécessaires
  • J'ai mis à jour le fichier des plans de tests du portail pro si nécessaire
  • J'ai mis à jour la liste des routes et des titres de pages du portail pro si j'en ai rajouté/modifié ou supprimé une.
  • J'ai relu attentivement les migrations, en particulier pour éviter les locks, et je préviens les équipes Shérif et Data
  • J'ai ajouté des screenshots pour d'éventuels changements graphiques

@jcicurel-pass jcicurel-pass marked this pull request as ready for review November 6, 2024 10:55
@rprasquier-pass
Copy link
Contributor

rprasquier-pass commented Nov 6, 2024

En faire un script comme ceci offre un niveau de confiance assez faible.

Il s'agit ici de fixer des données fallacieuses, j'aurai préféré que cela soit dans une migration afin de s'assurer que cela soit executé sur tous les env (je pense aux env de perf par exemple).
Par ailleurs, je doute que cela se fasse ici, mais mettre la logique dans une fonction et la tester me semble une bonne maniere de se rassurer.

@jcicurel-pass
Copy link
Contributor Author

En faire un script comme ceci offre un niveau de confiance assez faible.

Il s'agit ici de fixer des données fallacieuses, j'aurai préféré que cela soit dans une migration afin de s'assurer que cela soit executé sur tous les env (je pense aux env de perf par exemple). Par ailleurs, je doute que cela se fasse ici, mais mettre la logique dans une fonction et la tester me semble une bonne maniere de se rassurer.

Pour moi ça ne doit pas être une migration car ce n'est pas lié à un changement de code ou quoi
J'ai testé en testing et c'était ok, je testerai ensuite en staging avant de le faire en prod comme d'hab pour se rassurer
ça concerne une faible quantité de lignes en prod en l'occurrence
en fait je vois pas la différence avec les script de rattrapage qu'on a pu faire dernièrement (et pour lesquelles on a pas écrit de test)

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.

3 participants