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

Supprime les caractères non supportés par les flux RSS et ATOM #6329

Merged
merged 3 commits into from
Jul 4, 2022

Conversation

philippemilink
Copy link
Member

@philippemilink philippemilink commented Jun 4, 2022

La principale contribution de cette PR est de supprimer les "caractères de contrôles" qui ne sont pas supportés dans les flux RSS et ATOM, mais qui peuvent être présents dans les contenus qui sont affichés dans les flux. J'en ai profité pour nettoyer un peu le code et ajouter les tests pour les flux des billets (il y avait des tests pour les flux des articles et des tutoriels, mais pas pour les billets).

Contrôle qualité

J'ai ajouté les tests correspondant, donc la CI devrait suffire.

  • Faire une revue de code
  • Créer un article / tutoriel / billet / sujet de forum dont le titre ou la description contiennent des caractères qui posent problème. Ce peut être en copiant le morceau de code inline ]%B5�G�}%B5%D0&@"�s0%DFRncrypted.txt depuis le premier message de ce sujet. Il faut le copier depuis ZdS et non depuis GitHub, car GitHub supprime/convertit les caractères qui posent problème
  • Récupérer les flux RSS et ATOM contenant le contenu que vous venez de créer. La génération des flux doit fonctionner et ne pas déclencher d'erreur.

Je me suis inspiré de cette réponse sur StackOverflow pour implémenter la correction, mais il y a peut-être plus élégant...

Note : vous pouvez visualiser les caractères bizarres et invisibles avec ce site par exemple.

Note à celui qui mergera : ne squashez pas les commits.

@philippemilink philippemilink added S-BUG Corrige un problème C-Back Concerne le back-end Django S-Refactorisation Améliore le code existant sans forcément ajouter de nouvelle fonctionnalité labels Jun 4, 2022
@coveralls
Copy link

coveralls commented Jun 4, 2022

Coverage Status

Coverage increased (+0.03%) to 88.033% when pulling 1d5654a on philippemilink:fix-feeds into dfdf6f4 on zestedesavoir:dev.

@philippemilink philippemilink force-pushed the fix-feeds branch 2 times, most recently from ca27168 to f2ef7fa Compare June 4, 2022 20:52
* Ne scinde pas les chaînes de caractères inutilement
* Meilleure cohérence dans les noms des classes de tests
* Typos dans les commentaires
* Suppression de code inutile dans les tests
…e leur génération

Une erreur 500 UnserializableContentError est levée sinon.

Détecté grâce à Sentry.
Copy link
Contributor

@Arnaud-D Arnaud-D left a comment

Choose a reason for hiding this comment

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

Ça me paraît bon. J'ai testé et les flux RSS ne sont plus cassés par les caractères donnés en exemple.

QA OK ✔️

@Arnaud-D Arnaud-D merged commit b9e68a8 into zestedesavoir:dev Jul 4, 2022
@philippemilink philippemilink deleted the fix-feeds branch July 4, 2022 20:59
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-BUG Corrige un problème S-Refactorisation Améliore le code existant sans forcément ajouter de nouvelle fonctionnalité
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants