Pc 32007 retrieve special event #15606

rpaoloni-pass · 2024-12-19T14:39:53Z

But de la pull request

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

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
J'ai fait la revue fonctionnelle de mon ticket

github-actions · 2024-12-19T14:43:25Z

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

https://pc-pro-testing--pr15606-pc-32007-retrieve-sp-736urpua.web.app

_{(expires Wed, 08 Jan 2025 13:16:47 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1}

…ters

…t retrival

prouzet-pass

Je n'ai pas testé. Je laisse quelques questions, mais comme c'est dense, après rebase et réponses, il serait judicieux de faire une revue fonctionnelle avant d'approuver la PR.

prouzet-pass · 2025-01-13T14:59:25Z

api/src/pcapi/models/feature.py

@@ -50,6 +50,7 @@ class FeatureToggle(enum.Enum):
    ENABLE_CDS_IMPLEMENTATION = "Permet la réservation de place de cinéma avec l'API CDS"
    ENABLE_CODIR_OFFERERS_REPORT = "Active le rapport sur les entités juridiques actives pour le CODIR (tourne la nuit)"
    ENABLE_CRON_TO_UPDATE_OFFERER_STATS = "Active la mise à jour des statistiques des entités juridiques avec un cron"
+    ENABLE_CHRONICLES_RETRIEVAL = "Activer la récupération des chroniques"


Par souci de cohérence, j'utiliserais SYNC plutôt que RETRIEVAL, comme ENABLE_BANK_ACCOUNT_SYNC et ENABLE_DS_SYNC_FOR_USER_ACCOUNT_UPDATE_REQUESTS par exemple.

prouzet-pass · 2025-01-13T15:00:54Z

api/src/pcapi/models/feature.py

@@ -70,9 +71,8 @@ class FeatureToggle(enum.Enum):
    ENABLE_PHONE_VALIDATION = "Active la validation du numéro de téléphone"
    ENABLE_PRO_ACCOUNT_CREATION = "Permettre l'inscription des comptes professionels"
    ENABLE_PRO_BOOKINGS_V2 = "Activer l'affichage de la page booking avec la nouvelle architecture."
-
+    ENABLE_SPECIAL_EVENT_RETRIEVAL = "Activer la récupération des réponses aux opération spéciales"


Suggested change

ENABLE_SPECIAL_EVENT_RETRIEVAL = "Activer la récupération des réponses aux opération spéciales"

ENABLE_SPECIAL_EVENT_RETRIEVAL = "Activer la récupération des réponses aux opérations spéciales"

voire :

Suggested change

ENABLE_SPECIAL_EVENT_RETRIEVAL = "Activer la récupération des réponses aux opération spéciales"

ENABLE_SPECIAL_EVENT_RETRIEVAL = "Activer la synchronisation des réponses aux opérations spéciales"

prouzet-pass · 2025-01-13T15:06:58Z

api/src/pcapi/connectors/typeform.py

+            sort="submitted_at,asc",
+            since=last_date,
+        )
+        yield from forms


Question : Est-ce bon si on n'a aucun item dans forms ? (pas de nouveau, ou le dernier du précédent chunk complet était le tout dernier)
Sinon, un if not forms: break ?

Edit: test_empty_answer semble confirmer que ça marche

prouzet-pass · 2025-01-13T15:10:49Z

api/tests/connectors/typeform/typeform_test.py

+        with mock.patch("pcapi.connectors.typeform.get_responses", side_effect=list_expected) as get_responses:
+            for r, e in zip(typeform.get_responses_generator(time_function, form_id), chain(*list_expected)):
+                if cycles % 5 == 0:
+                    get_responses.assert_called_with(


Je ne suis pas certain que ça réinitialise à chaque appel pour vérifier que le suivant a les bons paramètres.

prouzet-pass · 2025-01-13T15:20:55Z

api/src/pcapi/core/operations/api.py

+            logger.error(
+                "An error happened while retrieving special event",
+                extra={
+                    "event_id": event.id,


J'ajouterais bien event.externalId dans les logs, pour s'y retrouver sans la bdd.

prouzet-pass · 2025-01-13T15:28:22Z

api/src/pcapi/core/operations/api.py

+            models.SpecialEventAnswer(
+                responseId=response.id,
+                questionId=questions[answer.field_id].id,
+                text=answer.text or "",


On a if not answer.text: au-dessus, donc selon ma compréhension, on peut se limiter à :

Suggested change

text=answer.text or "",

text=answer.text,

prouzet-pass · 2025-01-13T15:31:16Z

api/src/pcapi/core/operations/api.py

+            update_form_title_from_typeform(event=event, form=form)
+            questions = update_form_questions_from_typeform(event=event, form=form)


Suggestion/débat : je pense qu'on pourrait dans la plupart des appels se passer de ces deux lignes, si "last_updated_at" (dans le json reçu de Typeform, et que l'on pourrait reporter dans TypeformForm) est antérieur à la date d'une dernière mise à jour que l'on pourrait stocker. Cela économisera pas mal d'appels à la base de données.

prouzet-pass · 2025-01-13T15:39:22Z

api/tests/core/operations/test_api.py

+            response_id="qwerty123",
+            date_submitted=datetime.datetime(2020, 1, 1),
+            phone_number="0123456789",
+            email="[email protected]",


Détail : plutôt valid email ?

prouzet-pass · 2025-01-13T15:41:42Z

api/src/pcapi/core/operations/api.py

+        )
+        db.session.add(response)
+        db.session.flush()
+    except sa.exc.IntegrityError:


Ce cas est-il censé se produire en prod avec des données réelles ?
Si non, peut-être vaut-il mieux ne pas le passer sous silence, et ajouter un log d'erreur ?

rpaoloni-pass force-pushed the pc-32007-retrieve-special-event branch from 4c55562 to 408f54a Compare December 20, 2024 09:26

rpaoloni-pass added 2 commits January 6, 2025 14:13

(BSR)[API] feat: factorize some code to be used in all typeform impor…

0644418

…ters

(PC-32007)[API] feat: cleanup database schema for special events

13d4515

rpaoloni-pass force-pushed the pc-32007-retrieve-special-event branch 2 times, most recently from 1469934 to a606f4d Compare January 6, 2025 14:27

rpaoloni-pass added 2 commits January 6, 2025 15:30

(PC-32007)[API] feat: add special event importer

1df8b1b

(PC-32007)[API] feat: add FF to deactivate chronicle and special even…

2e7b18d

…t retrival

rpaoloni-pass force-pushed the pc-32007-retrieve-special-event branch from a606f4d to 2e7b18d Compare January 6, 2025 14:31

rpaoloni-pass marked this pull request as ready for review January 6, 2025 14:34

prouzet-pass reviewed Jan 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pc 32007 retrieve special event #15606

Pc 32007 retrieve special event #15606

rpaoloni-pass commented Dec 19, 2024

github-actions bot commented Dec 19, 2024 •

edited

Loading

prouzet-pass left a comment

prouzet-pass Jan 13, 2025

prouzet-pass Jan 13, 2025

prouzet-pass Jan 13, 2025

prouzet-pass Jan 13, 2025

prouzet-pass Jan 13, 2025

prouzet-pass Jan 13, 2025

prouzet-pass Jan 13, 2025

prouzet-pass Jan 13, 2025

prouzet-pass Jan 13, 2025

	ENABLE_SPECIAL_EVENT_RETRIEVAL = "Activer la récupération des réponses aux opération spéciales"
	ENABLE_SPECIAL_EVENT_RETRIEVAL = "Activer la récupération des réponses aux opérations spéciales"

		update_form_title_from_typeform(event=event, form=form)
		questions = update_form_questions_from_typeform(event=event, form=form)

Pc 32007 retrieve special event #15606

Are you sure you want to change the base?

Pc 32007 retrieve special event #15606

Conversation

rpaoloni-pass commented Dec 19, 2024

But de la pull request

Vérifications

github-actions bot commented Dec 19, 2024 • edited Loading

prouzet-pass left a 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

github-actions bot commented Dec 19, 2024 •

edited

Loading