Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Affiche les permissions dans les réponses de l'API #3264

Merged
merged 2 commits into from Jan 13, 2016

Conversation

@GerardPaligot
Copy link
Member

commented Jan 2, 2016

Q R
Correction de bugs ? Non
Nouvelle Fonctionnalité ? Oui
Tickets (issues) concernés ZEP-30

Affiche les permissions partout où on affiche un membre ou un MP (et ses messages) à partir de l'API. Voici un exemple d'affichage :

{
    "count": 1,
    "next": null,
    "previous": null,
    "results": [{
        "id": 1,
        "username": "firm1",
        "is_active": true,
        "date_joined": "2016-01-02T00:45:58.025816",
        "avatar_url": "https://secure.gravatar.com/avatar/8684a174694f1724697f66c6f668f440?d=identicon",
        "permissions": {
            "write": true,
            "read": true,
            "update": true,
            "ban": false
        }
    }]
}

Pour la QA : Vérifiez que les routes de l'API des membres et des MPs retourne bien les permissions. Les règles liées aux permissions sont les suivantes :

Membres

  • read : true pour tout le monde.
  • write : true pour l'utilisateur authentifié et pour le staff.
  • update : true uniquement pour l'utilisateur authentifié.
  • ban : true uniquement pour le staff.

MP

  • read : true pour tous les utilisateurs authentifiés et participant à la conversation.
  • write : truepour tous les utilisateurs authentifiés et participant à la conversation.
  • update : true pour tous les utilisateurs authentifiés et auteur de la conversation.

Poste

  • read : true pour tous les utilisateurs authentifiés et participant à la conversation.
  • write : truepour tous les utilisateurs authentifiés et participant à la conversation.
  • update : true pour tous les utilisateurs authentifiés, où le message est le dernier de la conversation et auteur de la conversation.

Enjoy!

@GerardPaligot GerardPaligot force-pushed the GerardPaligot:feat_api_permissions branch 10 times, most recently from cbfbb5c to 0f744fe Jan 2, 2016

@GerardPaligot GerardPaligot force-pushed the GerardPaligot:feat_api_permissions branch from 0f744fe to 8627836 Jan 2, 2016

@@ -55,5 +55,11 @@ def process_request(self, request):
state.notify_member(ban, msg)
return Response(serializer.data)

def get_permissions(self):

This comment has been minimized.

Copy link
@artragis

artragis Jan 2, 2016

Member

peux-tu ajouter un docstring pour qu'on comprenne ce qui est vraiment retourné stp? Car pour quelqu'un qui ne connait pas bien le système c'est pas immédiat. (après c'est pas obligatoire)

This comment has been minimized.

Copy link
@GerardPaligot

GerardPaligot Jan 2, 2016

Author Member

Est-ce nécessaire ? Ce sont des méthodes surchargées avec une documentation au niveau de la méthode parente. Je peux le faire mais je vais me contenter de faire un copier/coller de la documentation officielle de la bibliothèque (chose que je ne trouve pas très pertinent).

This comment has been minimized.

Copy link
@GerardPaligot

GerardPaligot Jan 2, 2016

Author Member

De plus, get_permissions me semble quand même explicite comme nom.

This comment has been minimized.

Copy link
@artragis

artragis Jan 2, 2016

Member

Donc effectivement, ce n'était pas nécessaire.

Le 02/01/2016 18:38, Gérard Paligot a écrit :

In zds/member/api/generics.py
#3264 (comment):

@@ -55,5 +55,11 @@ def process_request(self, request):
state.notify_member(ban, msg)
return Response(serializer.data)

  • def get_permissions(self):

Est-ce nécessaire ? Ce sont des méthodes surchargées avec une
documentation au niveau de la méthode parente. Je peux le faire mais
je vais me contenter de faire un copier/coller de la documentation
officielle de la bibliothèque (chose que je ne trouve pas très pertinent).


Reply to this email directly or view it on GitHub
https://github.com/zestedesavoir/zds-site/pull/3264/files#r48686057.

@GerardPaligot GerardPaligot changed the title Affiche les permissions dans les réponses de l'API des membres Affiche les permissions dans les réponses de l'API Jan 2, 2016

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 2, 2016

Mise à jour de la PR pour couvrir les messages privés. Vous pouvez QA ! :)

@Emeric54

This comment has been minimized.

Copy link
Member

commented Jan 3, 2016

Je viens de tester pour les membres. Quelques trucs me semblent bizarre. Par exemple, si je prend l'utilisateur admin, généré avec les données de test :

<root>
  <id>1</id>
  <username>admin</username>
  [...]
  <permissions>
    <write>False</write>
    <read>True</read>
    <update>False</update>
    <ban>False</ban>
  </permissions>
</root>

Pourquoi ban = False ? Pourquoi write = False, et pourquoi update = False ?

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2016

Tu as fais une requête authentifiée ? Si oui, avec qui ?

@Emeric54

This comment has been minimized.

Copy link
Member

commented Jan 3, 2016

Et si je prends un simple utilisateur, voici ses permissions :

<permissions>
  <write>True</write>
  <read>True</read>
  <update>False</update>
  <ban>True</ban>
</permissions>
@Emeric54

This comment has been minimized.

Copy link
Member

commented Jan 3, 2016

Donc soit il y a un problème, soit c'est moi qui dijoncte...

Tu as fais une requête authentifié ? Si oui, avec qui ?

Une requête non authentifié depuis le navigateur.

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2016

Pour les requêtes non authentifiées :

  • ban = False parce qu'un utilisateur anonyme ne peut pas bannir (uniquement staff).
  • write = False parce qu'un utilisateur anonyme ne peut pas modifier un profil (uniquement l'utilisateur concerné).
  • update = False pareil que pour le write.
@Emeric54

This comment has been minimized.

Copy link
Member

commented Jan 3, 2016

Ah oui d'accord, j'avais pris ça par le mauvais bout.

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2016

Attention à ce que tu fais aussi, l'API est "intelligente". Si tu es connecté avec un compte sur ton instance locale et que tu fais une requête dans ton navigateur, le serveur sera assez intelligent pour faire une requête authentifiée grâce à ta connexion active.

Pour être certain de ce que tu fais, je ne peux que te conseiller de faire tes tests sur une console REST.

@Emeric54

This comment has been minimized.

Copy link
Member

commented Jan 3, 2016

Attention à ce que tu fais aussi, l'API est "intelligente". Si tu es connecté avec un compte sur ton instance locale et que tu fais une requête dans ton navigateur, le serveur sera assez intelligent pour faire une requête authentifiée grâce à ta connexion active.

Yep, je faisais attention à ça.

Pour être certain de ce que tu fais, je ne peux que te conseiller de faire tes tests sur une console REST.

Je vais essayer ça, merci.

@gustavi gustavi added the Evolution label Jan 9, 2016

@SpaceFox

This comment has been minimized.

Copy link
Member

commented Jan 12, 2016

Toujours QA KO ici ?

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 12, 2016

A ma connaissance, il n'y a ni KO, ni OK. J'attends simplement une personne pour QA.

@Emeric54

This comment has been minimized.

Copy link
Member

commented Jan 12, 2016

QA OK pour l'API des membres.

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 12, 2016

Merci @Emeric54, tu sais faire pareil pour les MPs ? ^^

@Emeric54

This comment has been minimized.

Copy link
Member

commented Jan 12, 2016

QA OK pour le reste !

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 13, 2016

Super, merci @Emeric54 ! Avant de merge, j'aimerais simplement mettre à jour la lib de 0.1.5 à 0.1.6 qui embarque ma PR qui confirme la compatibilité Django 1.9. Le reste, c'est de la doc donc il n'y aura pas besoin de refaire une QA (limite, les tests suffiront).

@artragis

This comment has been minimized.

Copy link
Member

commented Jan 13, 2016

tu parles de django 1.9 rassure moi.

Le 13 janvier 2016 à 09:35, Gérard Paligot notifications@github.com a
écrit :

Super, merci @Emeric54 https://github.com/Emeric54 ! Avant de merge,
j'aimerais simplement mettre à jour la lib de 0.1.5 à 0.1.6 qui embarque ma
PR dbkaplan/dry-rest-permissions#19 qui
confirme la compatibilité Django 1.6. Le reste, c'est de la doc donc il n'y
aura pas besoin de refaire une QA (limite, les tests suffiront).


Reply to this email directly or view it on GitHub
#3264 (comment)
.

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 13, 2016

Bien sûr, corrigé.

@@ -30,6 +30,7 @@ django-oauth-toolkit==0.9.0
drf-extensions==0.2.7
django-rest-swagger==0.2.9
django-cors-headers==1.0.0
dry-rest-permissions==0.1.5

This comment has been minimized.

Copy link
@gustavi

gustavi Jan 13, 2016

Member

Je vois que la 0.1.6 est sortie :)

@GerardPaligot GerardPaligot force-pushed the GerardPaligot:feat_api_permissions branch from b3c4720 to b40ab3f Jan 13, 2016

@GerardPaligot

This comment has been minimized.

Copy link
Member Author

commented Jan 13, 2016

PR mise à jour, elle peut être mergée puisque QA ok par @Emeric54. :)

gustavi added a commit that referenced this pull request Jan 13, 2016
Merge pull request #3264 from GerardPaligot/feat_api_permissions
Affiche les permissions dans les réponses de l'API

@gustavi gustavi merged commit 624a6b4 into zestedesavoir:dev Jan 13, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gustavi gustavi added this to the Version de développement milestone Jan 13, 2016

@GerardPaligot GerardPaligot deleted the GerardPaligot:feat_api_permissions branch Jan 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.