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

[Django 3.x] Mise à jour des dépendances qui gèrent les slugs #5951

Merged
merged 6 commits into from Dec 17, 2020

Conversation

Situphen
Copy link
Member

@Situphen Situphen commented Oct 6, 2020

Depuis la ZEP 12, les dépendances qui gèrent les slugs (django-uuslug et python-slugify) n'ont pas été mises à jour. Cela va nous poser soucis lorsqu'on passera à Django 3.x car les versions utilisées actuellement ne sont pas compatible avec cette version.

Actuellement lors de la génération des slugs, les apostrophes sont supprimées. Si on met à jour ces dépendances telles quelles, les apostrophes seront remplacées par un tiret. Actuellement, « C'est la vie » devient « cest-la-vie » mais après la mise à jour ce sera « c-est-la-vie ».

La solution retenue que j'ai retenu est de ne pas utiliser directement les fonctions slugify et uuslug proposées par django-uuslug mais de créer deux fonctions wrapper qui suppriment les apostrophes avant d'envoyer le texte à slugify et uuslug. J'ai découvert qu'en dehors des contenus une autre fonction nommée slugify et définie dans zds/utils est utilisée pour les slugs des tags, galeries, catégories de forum, etc. Pour éviter toute confusion j'ai décidé de la renommer old_slugify (choix arbitraire, n'hésitez pas à proposer autre chose).

Quitte à faire de la QA sur tout ce qui touche aux slugs, j'en ai profité pour vérifier que tous les appels à content.save() dans le code spécifient bien la valeur de force_slug_update. Ensuite, j'ai inversé la logique : appeler content.save() ne met pas à jour le slug sauf lorsque le slug est vide (c'est-à-dire à la création du contenu) ou lorsque force_slug_update=True est spécifié.

Closes #6000
Closes #3553 (qui a mon avis est identique à #6000)

QA :

  • Github Actions
  • source zdsenv/bin/activate && make update && make zmd-start && make run-back
  • Vérifier que le site web tourne correctement, notamment tout ce qui touche aux slugs

Merci de penser à utiliser le bouton « Rebase and merge » pour garder les commits séparés (et non pas « Squash and merge ») au cas où il faudrait revenir en arrière sur un commit en particulier (étant donné la criticité de ce qui touche aux slugs)

@coveralls
Copy link

coveralls commented Oct 6, 2020

Coverage Status

Coverage increased (+0.006%) to 86.861% when pulling c817587 on Situphen:slug into 9324f6e on zestedesavoir:dev.

@Situphen Situphen changed the title Mise à jour des dépendances qui gèrent les slugs [Django 3] Mise à jour des dépendances qui gèrent les slugs Oct 6, 2020
@Situphen Situphen changed the title [Django 3] Mise à jour des dépendances qui gèrent les slugs [Django 3.x] Mise à jour des dépendances qui gèrent les slugs Oct 7, 2020
@AmauryCarrade AmauryCarrade added the C-Back Concerne le back-end Django label Oct 7, 2020
@artragis
Copy link
Member

artragis commented Oct 8, 2020

il va falloir passer sur tous les moments où on fait un publishable_content.save() et vérifier qu'onpasse le force_update_slug=False sauf dans les moments où on change le titre du contenu

Suivi des PR automation moved this from En attente de QA to Fusionnable après rebase Dec 17, 2020
Copy link
Member

@AmauryCarrade AmauryCarrade 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

Je me suis baladé dans a peu près tout le site, ait publié beaucoup trop d'articles, ait tenté de reproduire #6000 (c'est bon, ça marche normalement), et autres. Je n'ai pas rencontré de problème.

@AmauryCarrade AmauryCarrade merged commit b351f2e into zestedesavoir:dev Dec 17, 2020
Suivi des PR automation moved this from Fusionnable après rebase to Fusionnée Dec 17, 2020
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
Projects
Suivi des PR
  
Fusionnée
4 participants