(PC-32268)[API] feat: Return individual or collective statistics only if offerer is concerned

[pcharlet-pass]

But de la pull request

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

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]

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

https://pc-pro-testing--pr14968-pcharlet-pc-32268-st-ywfc3dvu.web.app

_{(expires Fri, 15 Nov 2024 11:28:04 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1}

[damien-ramelet]

Ça n'est pas directement en lien avec ta PR, mais d'après git tu l'as implémenté. J'ai l'impression qu'il y a un soucis d'implémentation, cette fonction renvoit True à partir du moment où au moins un id de venue_ids appartient à l'utilisateur. Je pense (en tout cas le naming et la logique métier laissent à penser) qu'elle devrait plutôt renvoyer True si et seulement si tous les id de venue_ids appartiennent à l'utilisateur.

Le soucis se situe au niveau du exists, où on vérifie qu'il y a au moins un user_offerer correspondant au user passé en paramètre et à la liste de venue_id, on devrait plutôt vérifier que tous existent non ?

(testable facilement en local avec les données de sandbox)

[damien-ramelet]

(je ne voulais pas approve, j'ai misclick : )

[damien-ramelet]

On peut facilement écrire un test de non regression, comme ceci:

     @patch("pcapi.connectors.clickhouse.testing_backend.TestingBackend.run_query")
     def test_get_statistics_from_multiple_venues(self, run_query, client):
+        venue_not_belonging_to_user = offerers_factories.VenueFactory()
         user = users_factories.UserFactory()
         offerer = offerers_factories.OffererFactory()
         offerers_factories.UserOffererFactory(user=user, offerer=offerer)
         venue2 = offerers_factories.VenueFactory(managingOfferer=offerer)
         venue_id = venue.id
         venue2_id = venue2.id
+        foreign_venue = venue_not_belonging_to_user.id
         test_client = client.with_session_auth(email=user.email)
         num_queries = testing.AUTHENTICATION_QUERIES
         num_queries += 1  # select Offerer
         with testing.assert_num_queries(num_queries):
             run_query.return_value = fixtures.YEARLY_AGGREGATED_VENUE_REVENUE
-            response = test_client.get(f"/get-statistics/?venue_ids={venue_id}&venue_ids={venue2_id}")
+            response = test_client.get(f"/get-statistics/?venue_ids={venue_id}&venue_ids={venue2_id}&venue_ids={foreign_venue}")
             assert response.status_code == 200

On ne devrait pas avoir une 200 dans ces conditions

[pcharlet-pass]

En effet ! Merci d'avoir trouvé ça ! (corrigé sur les 2 PR)

[damien-ramelet]

Merci à toi !

[pcharlet-pass]

La PR version 2 est plus propre #14992