etalab / transport-site

Rendre disponible, valoriser et améliorer les données transports
https://transport.data.gouv.fr
198 stars 30 forks source link

Nettoyage du contrôleur de resources : actions producteurs bougées #4169

Closed vdegove closed 2 months ago

vdegove commented 2 months ago

Closes #4069

À lire plutôt dans l’ordre des commits vu qu’il y a du code déplacé.

Cette PR finit le travail de nettoyage de l’espace producteur. Il s’agit principalement d’une PR technique, qui sépare les actions liées à l’espace producteur des actions publiques du contrôleur TransportWeb.ResourceController.

L’expérience utilisateur est cependant améliorée en termes d’autorisation  : on ne peut plus afficher les formulaires d’édition pour un dataset absent de transport.data.gouv.fr ou dont on n’est pas le producteur (mais on peut bien éditer des resources qui n’ont pas encore été importées). Même si les données sont publiques et que l’API de Datagouv contrôlait la bonne autorisation lors du POST de modification ou le DELETE, cela pouvait paraître étrange de voir un formulaire pour une resource d’une autre organisation, même pas présente sur le PAN.

AntoineAugusti commented 2 months ago

Je regarde ceci demain, merci pour la PR !

vdegove commented 2 months ago

Normalement ceci ne survient pas dans un fonctionnement classique. Il n'y a pas d'autre changement en terme d'UI/UX ?

Une dégradation que je n’ai pas mentionné : en cas d’erreur dans l’input du formulaire qui génèrerait une erreur côté datagouv ou lors du réimport du dataset, au lieu de rendre le formulaire, on rend la page d’accueil de l’espace producteur. Voir cette ligne sur ce commit.

La raison : je n’ai pas modifié pour les actions de POST les routes (qui ont les identifiants datagouv) et la logique, du coup on n’a pas forcément le dataset / resource_id, et ça reste une fonction de contrôleur unique pour la création et l’édition – or j’ai splitté les actions pour l’affichage du formulaire, et il faudrait donc faire de la logique pour savoir dans quel cas on se trouve.

C’est pas foufou mais je voulais pas aller plus loin dans le refactor.

Aussi, juste une petite broutille : le lien depuis la page publique d’un dataset vers l’espace producteur renvoie directement à la page du dataset dans l’espace producteur, voir le commit dédié

Sinon, c’est un peu plus blindé pour éviter que les utilisateurs n’accèdent à une URL d’un dataset + resource qui n’existe pas : potentiellement, ça nous prémunit d’erreurs 500 (même si j’avais déjà fait une partie du taf avant).

Dernière broutille : pour les actions du formulaire qui vont taper sur l’API de datagouv, je nettoie la payload d’abord pour n’autoriser que certains paramètres (avant on faisait grosso modo du forward de la payload du POST en disant à datagouv «démerde-toi avec ça, c’est ce que l’utilisateur nous a filé»).

Je n'ai pas navigué en local. Tu pourras refaire un tour en EN puis FR pour vérifier que certaines traductions ne sont pas manquantes ?

C’est fait, tout a l’air OK.

Concernant les tests, est-ce qu'il y a des changements ?

Oui, dans le sens où il y a un appel en BDD + un appel à l’API de datagouv www.data.gouv.fr/api/1/me en plus sur chaque action liée au resources.

AntoineAugusti commented 2 months ago

@vdegove Merge quand ça te convient, tout est bon pour moi