Produits Entr'ouvert » Authentic 2

Ça m'irait qu'on rapproche l'API du fonctionnement de JSONAPI

# ajouter l'utilisateur 1234
POST /api/roles/xyz/relationships/members/

{
   "data": [
     {"uuid": "1234"}
   ]
}
# écraser la liste des membres avec les utilisateurs pointés
PATCH /api/roles/xyz/relationships/members/

{
   "data": [
     {"uuid": "1234"}
   ]
}

# retirer l'utilisateur 1234
DELETE /api/roles/xyz/relationships/members/

{
   "data": [
     {"uuid": "1234"}
   ]
}

L'idée étant que /api/roles/xyz/members/ fonctionne par contre comme /api/users/ un jour (et faut prendre en compte le cas {"sub": "1234"} pour les services OIDCs.

L'ancienne API serait déprécié progressivement.

Que fait-on lorsqu'un des uuid dans le payload n'existe pas pour a2 ?
J'imagine que le comportement actuel de RoleMembershipsAPI, qui consiste à renvoyer un HTTP 404, n'est plus valable.
Au contraire, on pourrait renvoyer la liste des UUID d'usagers qui ont été affectés par l'appel.

Retourner le nouveau contenu, genre ce que pourrait retourner GET /api/roles/xyz/relationships/members/ qui n'était pas mentionné, ça me semble raisonnable.

Paul Marillonnet a écrit :

Que fait-on lorsqu'un des uuid dans le payload n'existe pas pour a2 ?

Je serai pour renvoyer une erreur 400 par défaut, si c'est gênant on inventer un paramètre pour débrayer ; un truc permissif n'obligera pas les gens à valider le retour.

J'imagine que le comportement actuel de RoleMembershipsAPI, qui consiste à renvoyer un HTTP 404, n'est plus valable.

Non et c'était moche.

Au contraire, on pourrait renvoyer la liste des UUID d'usagers qui ont été affectés par l'appel.

Si tout se passe bien normalement ce serait 204 No content.

Paul Marillonnet a écrit :

Une première version, qui reprend ce qui est fait pour l'API /roles/<role_uuid>/members/<member_uuid>/, en ajoutant du code dans la classe associée.
Elle reprend en particulier l'idée de renvoyer un 20{0,1} quand ça se passe bien, avec une courte phrase de confirmation de l'opération effectuée.

Je préférerai une classe séparée pour pouvoir déprécier l'autre à un moment et pour l'URL /api/roles/<role_uuid>/relationships/members/.

Je ne sais pas ce qu'authentic ou DRF font côté transactions, et du coup je suggérerais des explicites .atomic(). (mais me répondre que c'est bon c'est géré en amont ça me va très bien).

    def patch(self, request, *args, **kwargs):
        self.role.members.all().delete()
        for member in self.members:
            self.role.members.add(member)

et pourquoi pas juste

    def patch(self, request, *args, **kwargs):
        self.role.members.set(members)

?

(tests en django 1.8 qui cassent mais qui cependant n'ont plus lieu d'être, je viens de pousser une branche rebasée sur un master à jour)

C'est PATCH -> add et PUT/POST -> set.

Je ne suis pas d'accord avec le 404 quand on recherche les UUIDs, il faut renvoyer une 400 dans tous les cas, une ValidationError expliquant l'uuid qui foire.

Paul Marillonnet a écrit :

Benjamin Dauvergne a écrit :

Je ne suis pas d'accord avec le 404 quand on recherche les UUIDs, il faut renvoyer une 400 dans tous les cas, une ValidationError expliquant l'uuid qui foire.

Sans aller jusqu'à relire le document JSONAPI (je veux pas en faire une bible, mais on a pas de référence sur comment structurer nos APIs pour l'instant à part renvoyer {'err':...} et donc on fait un peut chaque fois des choses différentes... bref) 404 c'est que la ressource n'existe pas, la ressource existe, par contre la requête est bien erronée, donc 400.

Manque encore un check que request.data est bien un dict (tu te serais moins emmerdé avec un Serializer).