Redirection vers la version publique (si elle existe) d’un contenu inaccessible (#5858)

[SpaceFox]

Fait en sorte que si un utilisateur n’a pas accès à un contenu en brouillon/bêta (URL en /contenu) on le renvoie vers la version publique (URL en /article, /tutoriel ou /billet) si elle existe. Dans le cas contraire, on conserve le comportement actuel avec une erreur de type « accès interdit ».

Cette PR est en POC parce que la solution que j’ai mise en place m’a l’air étrange et va tripoter un fichier mixins.py, donc je préfère avoir une solution à peu près propre conceptuellement avant de m’attaquer aux tests. D’ailleurs je ne sais pas où sont faits lesdits tests, je ne vois pas de tests_mixins.py.

Numéro du ticket concerné : #5858

(Tests en attente, cf supra)

(Pas d’action particulière de MEP)

Contrôle qualité

• [ ] Vérifier que l’URL privée d’un tuto, d’un article et d’un billet publiés renvoie biens vers l’URL publique quand on est pas connecté.
• [ ] Idem quand on est connecté avec un compte utilisateur normal.
• [ ] Vérifier qu’on est pas redirigé dans ces mêmes cas avec un compte auteur des contenus.
• [ ] Idem avec un compte admin.
• [ ] Vérifier qu’on a bien une erreur d’accès sur l’URL privée d’un tuto, d’un article et d’un billet non publiés quand on est pas connectés.
• [ ] Idem avec un compte utilisateur normal.
• [ ] Vérifier qu’on est pas redirigé dans ces mêmes cas avec un compte auteur des contenus.
• [ ] Idem avec un compte admin.
• [ ] Et tout pareil avec une URL qui contient un SHA explicite (URLs de bêta).

Je me rends compte en l’écrivant qu’elle va être pénible à tester, cette MR…

[Situphen]

Merci pour la PR ! Personnellement, cette solution me paraît très correcte et je ne trouve pas dérangeant que la redirection s'effectue dans la mixin étant donné que la gestion des droits y est déjà. Cependant libre à toi de chercher si une meilleure solution existe :)

En ce qui concerne les tests, je ne crois pas qu'il y ait de tests dédiés aux mixins. Cependant les vues sont testées, donc elles sont testées indirectement. Il y a d'ailleurs plusieurs tests qui échouent et qu'il te faudra modifier (c'est normal, on passe d'un accès refusé à une redirection). C'est dans ces fichier là, dans zds/tutorialv2/tests/tests_views/, je pense qu'il te faudra regarder s'il est nécessaire d'ajouter de nouveaux tests pour couvrir ce comportement ou pas. Vu qu'il y a 10 tests qui échouent, on peut penser que les tests actuels couvrent déjà ce comportement, mais à vérifier.

[SpaceFox]

J’espère que la CI ne trouvera pas de plantages dans des tests que j’aurais oubliés :D

[SpaceFox]

OK, j’espère que cette fois ça passe. Les tests en local me trouvent 3 erreurs mais rien à voir avec ce que me signalait la CI avant ni avec ce que j’ai modifié…

[coveralls]

Coverage increased (+0.003%) to 87.022% when pulling 2c5676bfbc553a32e3ab92005abf83895a97252b on SpaceFox:fix-5858_redirection_contenu_vers_public into 0cf61a969ac0518651b7dd9921f59a9a35a14788 on zestedesavoir:dev.

[Arnaud-D]

QA NOK :heavy_multiplication_x:

:heavy_multiplication_x: quand je suis déconnecté, je suis redirigé vers la page de connexion et pas la page publique du contenu pour un contenu publié (je pense que c'est parce que la vue DisplayContent (ou une autre, je n'ai pas vérifié la vue exacte) dérive d'abord de LoginRequiredMixin avant SingleContentDetailViewMixin et donc on vérifie la connexion avant de tenter de rediriger, donc on n'a pas le comportement indiqué dans les instructions de QA) :heavy_check_mark: redirection en tant que simple membre vers la version publique depuis l'URL privée des tutos, articles et billets :heavy_check_mark: admin pas redirigé (tutos, articles, billets) :heavy_check_mark: auteur pas redirigé (tutos, articles, billets) :heavy_check_mark: sur un contenu non-public, on est redirigé vers la page de connexion (c'est le comportement attendu et pas une erreur comme mentionné dans les instructions de QA) :heavy_check_mark: sur un contenu non-public, 403 si on est pas auteur ou staff, c'est bon :heavy_check_mark: sur un contenu non-public, accès correct pour les auteurs ou staff

J'ai pas testé en détail les URLs avec SHA.

[SpaceFox]

Ha merde :(

J’ai regardé un peu ce WE, je suis perdu dans l’enchainement des mixins… si quelqu’un a une idée de comment arriver à une version fonctionnelle, je prends.

[Arnaud-D]

J'essaierai de regarder à l'occasion. J'ai mouillé dans ce genre de problèmes, et la solution que j'avais retenue était de ne pas utiliser certains mixins, qui apportent de la tortuosité plus que de la lisibilité. Ça se passe là si tu es curieux de jeter un œil : https://github.com/zestedesavoir/zds-site/pull/6188/files#r724487370

[Arnaud-D]

@SpaceFox J'ai regardé, et le problème est bien là où je pensais.

LoginRequiredMixin est fait pour être en première position (c'est ce qui est conseillé dans la doc de Django qui propose aussi une implémentation de ce mixin). C'est ce qui est appelé en premier, tu ne peux pas changer ce comportement depuis un mixin comme SingleContentDetailViewMixin (ou un de ses parents) qui vient après dans l'imbrication d'appels à dispatch.

Une possibilité pour faire l'action avant, c'est d'écrire un petit bout de dispatch dans la vue et continuer ensuite avec les dispatch parents pour avoir le comportement avec vérif du login, etc qui vient après. Ça donne un truc du genre :

class DisplayContent(LoginRequiredMixin, SingleContentDetailViewMixin):
    [...]

    def dispatch(self, request, *args, **kwargs):
        if not request.user.is_authenticated:
            return HttpResponsePermanentRedirect(self.get_public_object().get_absolute_url_online())
        return super().dispatch(request, *args, **kwargs)

À noter que ce code ne marche as en l'état parce que self.get_public_object().get_absolute_url_online() provoque une erreur, parce que self.object n'est pas encore initialisé à ce stade et est utilisé quelque part dans les appels. Cette initialisation est faite dans le get d'un des mixins, mais qui n'est appelé que après dispatch...

C'est clairement pas l'endroit le plus facile du code, il a des aspects spaghetti. :-/

[Arnaud-D]

@SpaceFox, tu as le courage de retravailler cette PR ou tu préfères jeter l'éponge ?

[SpaceFox]

Je jette l'éponge, je n'ai clairement pas envie de m'attaquer aux subtilités d'une technologie que je n'utilise plus par ailleurs.

Le jeu. 8 sept. 2022, 10:39, Arnaud-D @.***> a écrit :

@SpaceFox https://github.com/SpaceFox, tu as le courage de retravailler cette PR ou tu préfères jeter l'éponge ?

— Reply to this email directly, view it on GitHub https://github.com/zestedesavoir/zds-site/pull/6226#issuecomment-1240408934, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMEVHFHOC4GHB2ZVM6SDPLV5GQ43ANCNFSM5MQ4DO6Q . You are receiving this because you were mentioned.Message ID: @.***>