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

Refactorise le module de MP #6288

Conversation

Arnaud-D
Copy link
Contributor

@Arnaud-D Arnaud-D commented Apr 10, 2022

Un des items de #6246.

  • Refacto des URL :

    • utilise path au lieu de re_path et les nouveaux motifs en conséquence
    • ajoute un namespace mp pour ce module
    • change les noms de route pour des noms plus courts, plus simples et visant à être plus clairs
    • change la route pour éditer un message qui requérait l'id et le slug du topic pour agir seulement sur un message (ils ont leur propre id)
    • change la route pour marquer non lu qui avait été faite de manière un peu tordue (par moi, en l'occurrence :D)
  • Autres refacto au passage :

    • bouge du code (perform_unread) pour marquer un message comme non lu à un meilleur endroit (c'était surprenant)
    • refactoriser perform_unread parce que c'est illisible

Note importante : je change les URL "applicatives" (les machins sur lesquels ont clique mais vers lesquels on ne lie jamais normalement), mais aussi l'URL pour voir un MP. J'ai mis une redirection pour le lien vers un MP uniquement.

Contrôle qualité

Vérifier que les MP fonctionnent bien en exerçant toutes les URL.

Vérifier que marquer les MP non-lu fonctionne bien (en gras + notification).

@Arnaud-D Arnaud-D added C-Back Concerne le back-end Django S-Refactorisation Améliore le code existant sans forcément ajouter de nouvelle fonctionnalité labels Apr 10, 2022
@Arnaud-D Arnaud-D force-pushed the passez_votre_chemin_refacto_urls_en_cours_2 branch 3 times, most recently from cb1c64c to fb6a067 Compare April 10, 2022 17:08
@coveralls
Copy link

coveralls commented Apr 10, 2022

Coverage Status

Coverage decreased (-0.01%) to 88.019% when pulling ad585b6 on Arnaud-D:passez_votre_chemin_refacto_urls_en_cours_2 into 1bf72c7 on zestedesavoir:dev.

@Arnaud-D Arnaud-D force-pushed the passez_votre_chemin_refacto_urls_en_cours_2 branch from fb6a067 to 0943d48 Compare April 10, 2022 20:55
@Arnaud-D Arnaud-D force-pushed the passez_votre_chemin_refacto_urls_en_cours_2 branch from 0943d48 to 990eacb Compare May 6, 2022 22:25
@Arnaud-D Arnaud-D marked this pull request as ready for review May 6, 2022 22:33
@Arnaud-D Arnaud-D force-pushed the passez_votre_chemin_refacto_urls_en_cours_2 branch from 990eacb to 9d02f5b Compare June 2, 2022 21:48
@Arnaud-D Arnaud-D force-pushed the passez_votre_chemin_refacto_urls_en_cours_2 branch from 9d02f5b to 7a1f3c9 Compare June 26, 2022 16:53
Copy link
Member

@philippemilink philippemilink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai mis une redirection pour le lien vers un MP uniquement.

Est-ce qu'on en a vraiment besoin ? Cette URL est utilisée par les extensions navigateurs ou autre ?

templates/mp/topic/index.html Outdated Show resolved Hide resolved
zds/mp/urls.py Show resolved Hide resolved
@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented Jul 3, 2022

Je ne suis pas sûr qu'on en ait besoin, mais c'est le genre de lien qu'on pourrait avoir dans ses favoris pour une raison ou une autre. Si on ne redirige pas, le lien est cassé. Comme on peut l'empêcher à pas trop cher, je me suis dit que ça serait mieux de le faire.

Je suis totalement ouvert au fait de l'enlever si tu penses que c'est superflu. Je l'ai mis justement parce que je ne voulais pas qu'on me dise en QA que ça serait mieux de le rajouter. :D

@philippemilink
Copy link
Member

Je ne suis pas sûr qu'on en ait besoin, mais c'est le genre de lien qu'on pourrait avoir dans ses favoris pour une raison ou une autre. Si on ne redirige pas, le lien est cassé. Comme on peut l'empêcher à pas trop cher, je me suis dit que ça serait mieux de le faire.

Ok pour garder cette URL, mais alors ajoute un petit test pour tester que la route fonctionne bien :)

@Arnaud-D Arnaud-D force-pushed the passez_votre_chemin_refacto_urls_en_cours_2 branch from 88262b8 to 7edc1d0 Compare July 4, 2022 07:34
@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented Jul 4, 2022

Voilà qui est fait.

Copy link
Member

@philippemilink philippemilink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA OK ✔️

@philippemilink philippemilink merged commit 7c61904 into zestedesavoir:dev Jul 14, 2022
@Arnaud-D Arnaud-D deleted the passez_votre_chemin_refacto_urls_en_cours_2 branch July 15, 2022 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Back Concerne le back-end Django S-Refactorisation Améliore le code existant sans forcément ajouter de nouvelle fonctionnalité
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants