Add local image upload into flat pages content (ref #3552)

[marcantoinedupre]

Cette PR ajoute la possibilité d'uploader des images locales pour les insérer dans le contenu des pages statiques.

Description

La fonctionnalité se base sur l'option image_upload_url du plugin image de TinyMCE et l'ajout de la View correspondante côté Django.

Reprend l'implémentation de @babastienne pour django-mapentity. Dans l'attente d'une solution pour gérer le cycle de vie des images (suivi du lien entre image uploadée et page statique, suppression quand plus utilisée) la feature est ajoutée spécifiquement pour les pages statiques où le volume d'image uploadée sera peu important.

Liens :

PR django-mapentity

Related Issue

Attach files on flatpages #3552

Checklist

• I have followed the guidelines in our Contributing document
• My code respects the Definition of done available in the Development section of the documentation
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests pass locally with my changes
• I added an entry in the changelog file
• My commits are all using prefix convention (emoji + tag name) and references associated issues
• I added a label to the PR corresponding to the perimeter of my contribution
• The title of my PR mentionned the issue associated

[cypress]

Passing run #8590 ↗︎

0

24

0

0

0

Details:

Merge a4b03d4 into a66550d...
Project: Geotrek-admin	Commit: d23ba60e90 ℹ️
Status: Passed	Duration: 02:11 💡
Started: Apr 2, 2024 4:00 PM	Ended: Apr 2, 2024 4:02 PM

Review all test suite changes for PR #4024 ↗︎

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.41%. Comparing base (a66550d) to head (a4b03d4).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4024   +/-   ##
=======================================
  Coverage   98.41%   98.41%           
=======================================
  Files         282      284    +2     
  Lines       20757    20777   +20     
=======================================
+ Hits        20427    20447   +20     
  Misses        330      330           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[submarcos]

Pour la partie technique,

çà serait mieux d'utiliser une CBV.

Pour la partie fonctionnelle, çà serait mieux d'utiliser les storage django plutot que de tout faire à la main.
De plus, le contenu de /media/ doit être lié à une entrée BDD, sinon la commande clean_attachments suppriemra le fichier. Il faut donc utiliser un Attachment sur les flatpage

Du genre: j'upload un fichier, çà créé automatiquement un attachment de type photo exploitable.
La difficulté, c'est qu'il est impossible d'enregistrer un attacmet avant d'avoir enregistré une premiere fois la page statique

[marcantoinedupre]

Fait pour la remarque sur utiliser l'interface du storage django plutôt que les fichiers directement.

[camillemonchicourt]

Comme indiqué dans la PR côté Mapentity (makinacorpus/django-mapentity#277), je trouve dommage de bloquer cette évolution globale du WYSIWYG côté Mapentity pour un mécanisme de suppression des images orphelines très secondaire.
En effet, il n'y aura que quelques dizaines de photos ajoutées par le WYSIWYG et peu ou pas d'images orphelins.
L'enjeu est bien plus fort au niveau des attachments sur les différents objets des modules où là on a beaucoup beaucoup d'images et pas mal d'orphelins, notamment quand on importe des données depuis des plateformes tiers, et là une commande de nettoyage existe.
Et même dans les CMS où on a des centaines d'articles et des centaines de photos, il n'y a pas forcément de mécanisme de suppression automatique des médias orphelins.
C'est donc dommage que cette PR globale côté Mapentity se perde, pour un sujet secondaire selon moi.

---

A noter aussi : il me semble dommage de mettre /tinymce/ dans l'URL des médias uploadées car le chemin pourra être visible par les utilisateurs dans les catalogues d'images des moteurs de recherche et autres.

---

A noter aussi : On pourrait avoir besoin de ce WYSIWYG dans d'autres objets de l'AdminSite (recommandation des étiquettes, description des lieux de renseignement...) donc ce développement ne doit pas forcément être spécifiques aux pages statiques.

[submarcos]

Comme indiqué dans la PR côté Mapentity (makinacorpus/django-mapentity#277), je trouve dommage de bloquer cette évolution globale du WYSIWYG côté Mapentity pour un mécanisme de suppression des images orphelines très secondaire. En effet, il n'y aura que quelques dizaines de photos ajoutées par le WYSIWYG et peu ou pas d'images orphelins. L'enjeu est bien plus fort au niveau des attachments sur les différents objets des modules où là on a beaucoup beaucoup d'images et pas mal d'orphelins, notamment quand on importe des données depuis des plateformes tiers, et là une commande de nettoyage existe. Et même dans les CMS où on a des centaines d'articles et des centaines de photos, il n'y a pas forcément de mécanisme de suppression automatique des médias orphelins. C'est donc dommage que cette PR globale côté Mapentity se perde, pour un sujet secondaire selon moi.

A noter aussi : il me semble dommage de mettre /tinymce/ dans l'URL des médias uploadées car le chemin pourra être visible par les utilisateurs dans les catalogues d'images des moteurs de recherche et autres.

A noter aussi : On pourrait avoir besoin de ce WYSIWYG dans d'autres objets de l'AdminSite (recommandation des étiquettes, description des lieux de renseignement...) donc ce développement ne doit pas forcément être spécifiques aux pages statiques.

Côté django, les conventions de dev exigent que tout ce qui est dans /media/ soit reférencé en BDD. C'est pas vraiment difficile et peut être totalement transparent pour l'utilisateur, quitte à utiliser un autre modèle qu'attachment.

De plus, utiliser un modele ety un ImageField permetrait d'exploiter la possibilité de création des thumbnails, là on ouvre la porte aux utilisateurs uploadant des fichiers gigantesques, ou utiliser le mecanisme de nettoyage de fichier vérolé mis en place avec les signalements

Voyant ce que donne des instances apres 10ans d'utilisations, je suis pour que dans tous les cas on puisse nettoyer les fichiers inutilisés. Ca prend de la place dans les backups etc.

Outre ce point à discuter, l'utilisation des storage django est primordiale, ne serait-ce que pour pouvoir utiliser d'autres backends, ou pour s'assurer que le nom de fichier n'est pas déjà utilisé https://docs.djangoproject.com/en/5.0/topics/files/#storage-objects

[camillemonchicourt]

Oui c'est intéressant si c'est géré comme des attachments comme les autres médias.
Cela permet de regrouper toutes les images au même endroit, de ne pas avoir à modifier les stratégies de sauvegardes, de pouvoir gérer les médias orphelins, d'envisager des vignettes, mais aussi potentiellement du redimensionnement automatique comme on peut déjà le faire au niveau des attachments existants sur les objets.

[marcantoinedupre]

Je suis d'accord avec vos remarques concernant l'utilisation des Attachments pour les images uploadées pour bénéficier des checks de sécurité, du redimensionnement automatique, du suivi et du nettoyage des fichiers uploadés.

Comme il s'agit d'un développement non-trivial (dû au fait que l'Attachement ne peut pas être créé directement dans le endpoint appelé par TinyMCE images_upload car il manque l'ID de la page ou de l'objet) la team @GeotrekCE/makina-admin propose de merger cette PR apportant la fonctionnalité pour les pages statiques et de revenir à la PR mapentity -- pour ajouter l'upload de manière carrée pour toutes les entités mapentity -- dans un second temps. On parle d'une échéance courte, de quelques mois.

La mise à jour pour les utilisateurs ayant déjà commencé à uploader des images sera invisible, les fichiers déjà uploadés seront toujours disponibles. Il manquera juste les Attachments correspondants.