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

[FO/BO - Signalement] Limiter les types de documents acceptés (notamment pas de vidéos) #2553

Merged
merged 7 commits into from
May 24, 2024

Conversation

hmeneuvrier
Copy link
Collaborator

@hmeneuvrier hmeneuvrier commented May 13, 2024

Ticket

#1971

Description

Pour empêcher l'ajout de vidéos dans la plateforme, nous avons décidés de limiter les types de documents possibles en upload.
Après création d'un tableau sur metabase
https://histologe-metabase.osc-fr1.scalingo.io/question/1165-extension-des-fichiers-presents-en-prod
nous avons choisi d'accepter les fichiers suivants (sachant qu'on se base sur les types mimes et pas les extensions):

extension type mime occurences en prod
jpg image/jpeg 174570
pdf application/pdf 67523
png image/png 8392
docx application/vnd.openxmlformats-officedocument.wordprocessingml.document 2782
eml 963
odt application/vnd.oasis.opendocument.text 1535
doc application/msword 885
jpeg image/jpeg 755
txt text/plain 148
msg 265
xls application/vnd.ms-excel 19
xlsx application/vnd.openxmlformats-officedocument.spreadsheetml.sheet 9

Par contre, nous n'avons pas accepté les types de fichiers suivants :

extension type mime occurences en prod pourquoi
eml application/octet-stream 963 type mime pas fiable
mp4 609 vidéo
msg application/octet-stream 265 type mime pas fiable
mov 194 vidéo
heic 191 image commpliquée à afficher (déjà exclue)
pas d'extension 187 bahhh ...
zip 184 dangereux
bin 119 dangereux
webp 59 vidéo
... et les autres sous 100 occurrences

Changements apportés

  • Création d'un tableau des types mimes acceptés pour les documents (et des extensions) dans UploadHandlerService
  • Création de fonction twig pour passer les tableaux des types mimes acceptés aux templates (et des extensions conseillées)
  • Modification des templates et des js pour vérifier les types acceptés et afficher un message d'erreur
  • Modification des routes pour faire de la vérification back

Pré-requis

npm run watch

Tests

  • Faire un signalement depuis le nouveau formulaire et vérifier les fichiers acceptés pour l'ajout de photos aux désordres et l'ajout de document (DPE, bail, état des lieux, diagnostique plomb)
  • Sur la fiche de suivi usager, vérifier l'ajout de photos et l'ajout de document (extensions acceptées lors du clic sur le bouton, et en dropdown)
  • Sur la fiche BO du signalement, vérifier l'ajout de photos de situation, l'ajout de documents de situations, l'ajout de documents de procédure (extensions acceptées lors du clic sur le bouton, et en dropdown)
  • Créer une visite, et vérifier l'ajout de photos de visites (extensions acceptées lors du clic sur le bouton, et en dropdown), et l'ajout de rapport de visite (essayer aussi de glisser un type interdit sur le bouton d'ajout de rapport de visite)
  • Si possible, lors de la création de signalement depuis le nouveau formulaire, essayer de chinter la limitation front pour tester les vérifications back

@hmeneuvrier hmeneuvrier force-pushed the bugfix/1971-fobo-signalement-video branch 2 times, most recently from ebca624 to 0bc48cb Compare May 14, 2024 12:47
@hmeneuvrier hmeneuvrier changed the base branch from develop to feature/2558-qa-supprimer-le-feature-flipping-de-lespace-documentaire May 14, 2024 12:47
@hmeneuvrier hmeneuvrier changed the title [WIP]accept only few type of file #1971 [WIP][FO/BO - Signalement] Limiter les types de documents acceptés (notamment pas de vidéos) May 14, 2024
@hmeneuvrier hmeneuvrier changed the title [WIP][FO/BO - Signalement] Limiter les types de documents acceptés (notamment pas de vidéos) [FO/BO - Signalement] Limiter les types de documents acceptés (notamment pas de vidéos) May 14, 2024
@hmeneuvrier hmeneuvrier marked this pull request as ready for review May 14, 2024 15:43
@hmeneuvrier hmeneuvrier force-pushed the bugfix/1971-fobo-signalement-video branch from 33ee1fb to c9b9872 Compare May 15, 2024 08:49
@hmeneuvrier hmeneuvrier force-pushed the feature/2558-qa-supprimer-le-feature-flipping-de-lespace-documentaire branch from 55db74f to 704f089 Compare May 15, 2024 08:49
@hmeneuvrier hmeneuvrier marked this pull request as draft May 15, 2024 12:35
@hmeneuvrier hmeneuvrier changed the base branch from feature/2558-qa-supprimer-le-feature-flipping-de-lespace-documentaire to develop May 16, 2024 07:48
@hmeneuvrier hmeneuvrier force-pushed the bugfix/1971-fobo-signalement-video branch from 6bad54a to bec0d51 Compare May 16, 2024 07:52
@hmeneuvrier hmeneuvrier marked this pull request as ready for review May 16, 2024 07:53
Copy link
Collaborator

@emilschn emilschn left a comment

Choose a reason for hiding this comment

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

Un retour non-bloquant d'après moi :
1/ Si on prend un fichier non-compatible (ex pour le bail), on peut quand même passer à la suite.
Si jamais on n'a pas vu l'erreur, on peut passer à la suite sans se poser la question.
Et en même temps, je ne saurais pas comment "annuler" l'erreur si la personne n'a pas d'autre fichier à mettre.

Autres retours :
2/ Dans le formulaire, l'erreur pour les photos indique qu'on ne peut mettre que des JPG et PNG, alors que les GIF sont acceptés.
Dans la fiche usager, le message d'erreur correspond à la réalité :
Impossible d'ajouter le fichier sample1.heic car le format n'est pas pris en charge. Veuillez sélectionner un fichier au format JPEG, JPG, PNG ou GIF.
Voir pour améliorer le message d'erreur du formulaire ?

3/ Quand on crée une visite dans le passé et qu'on ajoute un rapport, la visite se crée quand même, et on a une erreur sur le type de fichier. Ce n'est pas forcément hyper limpide.
On peut garder ce fonctionnement, même si il est moins idéal que le retour immédiat des modales d'upload.
Ce qu'on pourrait améliorer :

  • le message d'erreur avec la liste d'extension séparée par des virgules et sans espace
  • préciser les types de fichiers acceptés au moment du bouton Parcourir

@hmeneuvrier
Copy link
Collaborator Author

1/ Si on prend un fichier non-compatible (ex pour le bail), on peut quand même passer à la suite. Si jamais on n'a pas vu l'erreur, on peut passer à la suite sans se poser la question. Et en même temps, je ne saurais pas comment "annuler" l'erreur si la personne n'a pas d'autre fichier à mettre.

mouais je ne sais pas trop comment faire

Autres retours : 2/ Dans le formulaire, l'erreur pour les photos indique qu'on ne peut mettre que des JPG et PNG, alors que les GIF sont acceptés. Dans la fiche usager, le message d'erreur correspond à la réalité : Impossible d'ajouter le fichier sample1.heic car le format n'est pas pris en charge. Veuillez sélectionner un fichier au format JPEG, JPG, PNG ou GIF. Voir pour améliorer le message d'erreur du formulaire ?

ok c'est fait

3/ Quand on crée une visite dans le passé et qu'on ajoute un rapport, la visite se crée quand même, et on a une erreur sur le type de fichier. Ce n'est pas forcément hyper limpide. On peut garder ce fonctionnement, même si il est moins idéal que le retour immédiat des modales d'upload. Ce qu'on pourrait améliorer :

* le message d'erreur avec la liste d'extension séparée par des virgules et sans espace

* préciser les types de fichiers acceptés au moment du bouton Parcourir

ok c'est fait

Copy link
Collaborator

@numew numew left a comment

Choose a reason for hiding this comment

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

Ok pour moi.

Il y'aurait une amélioration possible sur la gestion d'erreur coté nouveau formulaire, afin d'afficher l'erreur retourné plutot qu'un message standard quand le cas se présente :
error-file
pour le produire j'ai envoyé un fichier .mp4 renommé en .pdf (ainsi il passe le controle front mais pas back)

@hmeneuvrier
Copy link
Collaborator Author

Il y'aurait une amélioration possible sur la gestion d'erreur coté nouveau formulaire, afin d'afficher l'erreur retourné plutot qu'un message standard quand le cas se présente :

Super, je gère ça en début de semaine prochaine !

@hmeneuvrier hmeneuvrier force-pushed the bugfix/1971-fobo-signalement-video branch from 4f4631e to 36dbd9f Compare May 17, 2024 14:55
@hmeneuvrier
Copy link
Collaborator Author

Il y'aurait une amélioration possible sur la gestion d'erreur coté nouveau formulaire, afin d'afficher l'erreur retourné plutot qu'un message standard quand le cas se présente :

corrigé

Copy link

sonarcloud bot commented May 17, 2024

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Collaborator

@emilschn emilschn 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 re-testé mes retours, c'est ok.
@sfinx13 tu veux faire un tour pour vérifier tes retours ?

@numew numew merged commit 1fad486 into develop May 24, 2024
3 checks passed
@hmeneuvrier hmeneuvrier deleted the bugfix/1971-fobo-signalement-video branch June 11, 2024 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants