(PC-31965)[API]fix: Le patch n'est pas fait sur la 1ère étape du formulaire de mise à jour d'offre si produit #14254

pcharlet-pass · 2024-09-20T10:02:05Z

But de la pull request

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

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

github-actions · 2024-09-20T10:12:56Z

Visit the preview URL for this PR (updated for commit c8cb606):

https://pc-pro-testing--pr14254-pcharlet-pc-31965-no-763dpz8l.web.app

_{(expires Sun, 22 Sep 2024 13:57:52 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1}

AmineLouveau · 2024-09-20T10:24:09Z

pro/src/screens/IndividualOffer/DetailsScreen/utils.ts

@@ -79,9 +79,9 @@ export const buildShowSubTypeOptions = (showType: string): SelectOption[] => {
 export const completeSubcategoryConditionalFields = (


Attention pas mal de soucis d'indentation

Oui, faut faire tourner le lint/prettier et repousser.

AmineLouveau · 2024-09-20T10:24:45Z

pro/src/screens/IndividualOffer/InformationsScreen/InformationsScreen.tsx

@@ -111,6 +113,8 @@ export const InformationsScreen = ({
          isPhysicalEvent
        )

+  console.log('initialValues', initialValues)


AmineLouveau · 2024-09-20T10:27:09Z

pro/src/screens/IndividualOffer/InformationsScreen/InformationsScreen.tsx

@@ -141,6 +145,7 @@ export const InformationsScreen = ({

    // Submit
    try {
+      const shouldNotSendExtraData = isSearchByEanEnabled && !!offer?.productId


Peut être que le test pour savoir si c'est un produit n'est pas complet.
J'ai vu passer ce bout de code dans ./pro/src/screens/IndividualOffer/DetailsScreen/DetailsScreen.tsx

/ (Draft) offers are created via POST request. // On Details screen, the form might be pre-filled with a product, // until the form is submitted, the draft offer is not created yet. const isOfferProductBased = !!offer?.productId const isNotAnOfferYetButProductBased = !offer && !!formik.values.productId const isProductBased = isOfferProductBased || isNotAnOfferYetButProductBased

Ces modifications ne concernent que le patch, donc que lorsque offer est défini : c'est véritablement isOfferProductBased qui nous intéresse.
Par contre ce qui peut être challengé c'est d'avoir une fonction dans utils pour partager ces définitions isOfferProductBased/isNotAnOfferYetButProductBased.

Il me semble que le cas décrit dans ./pro/src/screens/IndividualOffer/DetailsScreen/DetailsScreen.tsx est spécifique à cette étape. Dans les autres, aucun champ front ne permet de spécifier le produit. L'offre ne peut donc avoir l'attribut productId différent de null que si c'est réellement le cas en DB. Je pense que le test est suffisant. @asaez-pass ?

C'est qu'en contexte de création d'offre - étape 1 et avant le POST que la condition isNotAnOfferYetButProductBased a du sens. Arrivé à l'étape 2, on se base sur l'offre (en draft) enregistré en base. Par contre j'ai un doute sur le distingo UsefulInformationScreen (nouveau parcours avec SPLIT) / InformationsScreen (ancien parcours). Il faudrait tester que tout soit ok sans le flag FF_SPLIT_OFFER.

asaez-pass · 2024-09-20T12:32:19Z

pro/src/screens/IndividualOffer/DetailsScreen/DetailsScreen.tsx

+      let response
+      if (!offer) {
+        response = await api.postDraftOffer(serializeDetailsPostData(formValues))
+      } else if (shouldPatchData && !!offer.lastProvider) {


Je pense qu'on peut réécrire :

const shouldNotPatchData = isProviderOffer || (!isSearchByEnabled && !!offer?.productId)
Et ici : (!shouldNotPatchData), pour plus de clarté.

Sinon
const shouldPatchData = !isProviderOffer && (isSearchByEnabled || !offer?.productId)
Et ici : (shouldPatchData)

Si const isProviderOffer = !!offer.lastProvider, et dans la mesure où offer.lastProvider n'est jamais un objet vide.

Also je pense qu'un commentaire type : / / Draft offer PATCH requests are useless for product-based offers and synchronized / provider offers since neither of the inputs displayed in DetailsScreen can be edited at all.

asaez-pass · 2024-09-20T12:35:12Z

pro/src/screens/IndividualOffer/UsefulInformationScreen/UsefulInformationScreen.tsx

@@ -120,13 +121,17 @@ export const UsefulInformationScreen = ({
      }
    }

+    console.log('formValues', formValues)


Log oublié ici.

asaez-pass · 2024-09-20T12:39:52Z

pro/src/screens/IndividualOffer/DetailsScreen/utils.ts

@@ -79,9 +79,9 @@ export const buildShowSubTypeOptions = (showType: string): SelectOption[] => {
 export const completeSubcategoryConditionalFields = (


Oui, faut faire tourner le lint/prettier et repousser.

asaez-pass · 2024-09-20T12:42:18Z

pro/src/screens/IndividualOffer/InformationsScreen/InformationsScreen.tsx

@@ -141,6 +145,7 @@ export const InformationsScreen = ({

    // Submit
    try {
+      const shouldNotSendExtraData = isSearchByEanEnabled && !!offer?.productId


Ces modifications ne concernent que le patch, donc que lorsque offer est défini : c'est véritablement isOfferProductBased qui nous intéresse.
Par contre ce qui peut être challengé c'est d'avoir une fonction dans utils pour partager ces définitions isOfferProductBased/isNotAnOfferYetButProductBased.

…not product base and FF WIP_EAN_CREATION id disabled

ogeber

ok pour le back

pcharlet-pass requested a review from asaez-pass September 20, 2024 10:02

pcharlet-pass force-pushed the pcharlet/pc-31965-no-extra-data-if-offer-with-product branch from f4cbedc to f54d9bb Compare September 20, 2024 10:17

AmineLouveau reviewed Sep 20, 2024

View reviewed changes

AmineLouveau approved these changes Sep 20, 2024

View reviewed changes

ogeber changed the base branch from master to pcharlet/pc-31878-patch-offer-extra-data September 20, 2024 12:29

pcharlet-pass force-pushed the pcharlet/pc-31878-patch-offer-extra-data branch from f998e1e to 3446bdd Compare September 20, 2024 12:49

pcharlet-pass force-pushed the pcharlet/pc-31965-no-extra-data-if-offer-with-product branch from f54d9bb to 069011e Compare September 20, 2024 13:31

asaez-pass reviewed Sep 20, 2024

View reviewed changes

(PC-31965)[API]fix: Update offer extra data allowed only if offer is …

c8cb606

…not product base and FF WIP_EAN_CREATION id disabled

pcharlet-pass force-pushed the pcharlet/pc-31965-no-extra-data-if-offer-with-product branch from 069011e to c8cb606 Compare September 20, 2024 13:45

ogeber approved these changes Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(PC-31965)[API]fix: Le patch n'est pas fait sur la 1ère étape du formulaire de mise à jour d'offre si produit #14254

(PC-31965)[API]fix: Le patch n'est pas fait sur la 1ère étape du formulaire de mise à jour d'offre si produit #14254

pcharlet-pass commented Sep 20, 2024

github-actions bot commented Sep 20, 2024 •

edited

Loading

AmineLouveau Sep 20, 2024

asaez-pass Sep 20, 2024

AmineLouveau Sep 20, 2024

AmineLouveau Sep 20, 2024

asaez-pass Sep 20, 2024

pcharlet-pass Sep 20, 2024

asaez-pass Sep 20, 2024

asaez-pass Sep 20, 2024

asaez-pass Sep 20, 2024

asaez-pass Sep 20, 2024

asaez-pass Sep 20, 2024

asaez-pass Sep 20, 2024

ogeber left a comment

		@@ -79,9 +79,9 @@ export const buildShowSubTypeOptions = (showType: string): SelectOption[] => {
		export const completeSubcategoryConditionalFields = (

(PC-31965)[API]fix: Le patch n'est pas fait sur la 1ère étape du formulaire de mise à jour d'offre si produit #14254

Are you sure you want to change the base?

(PC-31965)[API]fix: Le patch n'est pas fait sur la 1ère étape du formulaire de mise à jour d'offre si produit #14254

Conversation

pcharlet-pass commented Sep 20, 2024

But de la pull request

Vérifications

github-actions bot commented Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogeber left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 20, 2024 •

edited

Loading