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

[BO - Signalement] Affichage bouton édition et Modale d'édition de document #2235

Merged
merged 13 commits into from
Feb 26, 2024

Conversation

hmeneuvrier
Copy link
Collaborator

@hmeneuvrier hmeneuvrier commented Feb 9, 2024

Ticket

#2222

Description

Ajout de la possibilité d'éditer le type d'un document (et le desordreSlug le cas échéant).
Ajout d'un feature flipping sur les nouvelles fonctions liées aux documents
SPECS : https://github.com/MTES-MCT/histologe/wiki/Ajout-documents#bo---editer-les-documents-et-photos

Changements apportés

  • Variable d'environnement pour feature flipping
  • Ajout de la route back_signalement_edit_file pour éditer les documents
  • Création d'un EnumTrait et utilisation de ce trait dans l'enum DocumentType (à généraliser)
  • Dans DocumentType, ajout de 2 fonctions pour renvoyer des sous-sélection de l'enum en fonction de document / photo
  • Ajout d'un FILE_EDIT dans le FileVoter
  • Mise à jour du FILE_DELETE
  • Dans SignalementDesordresProcessor, création d'un tableau precisions qui contient toutes les précisions du signalement par slug/label
  • Dans le twig, ajout d'un bouton d'édition sur les photos et documents en fonction de la feature flipping et des droits (FILE_EDIT et FILE_DELETE)
  • Création d'un twig edit-file.html.twig et du js associé pour afficher la modale d'édition de type de documents
  • Création d'un filtre twig pour afficher le nom des documents tronqués
  • Correction de typo sur les json du formulaire

Pré-requis

Tests

  • Créer un signalement avec le nouveau formulaire en ajoutant photo et document (bail, dpe et diagnostique plomx dans Logement/Sécurité)
  • Dans la page signalement, ajouter des photos et documents avec plusieurs utilisateurs
  • Vérifier qu'on peut éditer les documents avec les bons utilisateurs
  • Vérifier qu'on peut supprimer les documents avec les bons utilisateurs
  • Vérifier que si on édite une photo on a seulement les types possibles pour photo qui apparaissent, idem pour document
  • Vérifier qu'en validant le type est bien changé (en base et dans l'affichage)
  • Vérifier que le diagnostique plomb apparait bien tagué
  • Pour les photos de désordres, (ou une photo qu'on passe en type désordre) vérifier que la liste des désordres possibles s'affiche et que le changement de désordre est pris en compte à la validation (en base, mais aussi dans la liste des désordres)
  • Vérifier que ça fonctionne aussi avec les photos et documents sur les "vieux" signalements

enum DocumentType: String
{
use EnumTrait;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je n'ai pas généralisé l'utilisation de l'EnumTrait, car ça mme semble risqué de mesurer les effets de bords.
Peut-être à penser à chaque fois qu'on touche un Enum

@hmeneuvrier hmeneuvrier changed the base branch from feature/2155-update-types-documents to develop February 12, 2024 09:40
@hmeneuvrier hmeneuvrier force-pushed the feature/2222-bo-signalement-documents-edition branch from 42e79f2 to 882ef45 Compare February 12, 2024 09:42
@hmeneuvrier hmeneuvrier changed the title [WIP] [BO - Signalement] Affichage bouton édition et Modale d'édition de document Feb 12, 2024
@hmeneuvrier hmeneuvrier force-pushed the feature/2222-bo-signalement-documents-edition branch from 70b99f0 to 03788eb Compare February 12, 2024 13:03
@hmeneuvrier hmeneuvrier marked this pull request as ready for review February 12, 2024 13:03
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.

Quelques remarques dans le code + une en image (certains libellé de précisions contiennent du HTML ce qui est pas top dans le select :
Capture d'écran 2024-02-12 174603

@hmeneuvrier
Copy link
Collaborator Author

hmeneuvrier commented Feb 13, 2024

Quelques remarques dans le code + une en image (certains libellé de précisions contiennent du HTML ce qui est pas top dans le select

@numew effectivement, certains labels ont du html, j'avais oublié ça... 🤔 ... tu as une idée pour le dégager ?
Sinon, l'idée que j'ai, c'est de ne pas lister les précisions de désordres, mais les critères de désordres... il n'y a pas de html dans les labels, et comme visuellement les photos sont rattachées au critère qu'elles soient liées à un critère ou à une précision, je ne pense pas que ce serait très gênant... Vous en pensez quoi ?

@numew
Copy link
Collaborator

numew commented Feb 13, 2024

Quelques remarques dans le code + une en image (certains libellé de précisions contiennent du HTML ce qui est pas top dans le select

@numew effectivement, certains labels ont du html, j'avais oublié ça... 🤔 ... tu as une idée pour le dégager ? Sinon, l'idée que j'ai, c'est de ne pas lister les précisions de désordres, mais les critères de désordres... il n'y a pas de html dans les labels, et comme visuellement les photos sont rattachées au critère qu'elles soient liées à un critère ou à une précision, je ne pense pas que ce serait très gênant... Vous en pensez quoi ?

Je n'avais pas d'idée particulière en faisant la remarque, je pense que couper la chaîne à partir du premier caractère "<" ferait l'affaire, l'utilisation du critère me va bien aussi

@hmeneuvrier
Copy link
Collaborator Author

Si on coupe à partir du premier < dans ton exemple on se retrouverait avec 3 fois le même désordre dans la liste, ce serait bizarre.

@hmeneuvrier hmeneuvrier force-pushed the feature/2222-bo-signalement-documents-edition branch from 41b70a9 to 9704fdf Compare February 14, 2024 09:19
src/Factory/FileFactory.php Show resolved Hide resolved
@hmeneuvrier hmeneuvrier force-pushed the feature/2222-bo-signalement-documents-edition branch from 8235089 to b0bf8b2 Compare February 16, 2024 10:29
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.

Je n'ai à présent plus les bouton de suppression sur les photo et document en tant que super admin


private function isPartnerFileDeletableByAdmin(File $file, User $user): bool
{
return $file->isPartnerFile() && $this->isAdminOrRTonHisTerritory($file, $user);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Je n'ai plus la possibilité de supprimer les image et documents (même en SUPER ADMIN), à première vu ca viendrait d'ici (il faut une condition OU au lieu de ET si je dis pas de bêtise)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

D'après les specs, même les super admins ne peuvent pas supprimer les photos et documents uploadés par les utilisateurs : https://github.com/MTES-MCT/histologe/wiki/Ajout-documents#bo---supprimer-les-documents-et-photos

Copy link
Collaborator

Choose a reason for hiding this comment

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

Je suis pas convaincu par cette spec mais d'accord

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 de code en 2 fois
J'attends que tu valides avec Mathilde pour tester

templates/back/signalement/view/photos-documents.html.twig Outdated Show resolved Hide resolved
@@ -77,6 +77,7 @@
content: (reference) => '<strong style="white-space: nowrap">' + reference.getAttribute('data-user') + '</strong>' + '<hr class="fr-pb-1v"><span style="white-space: nowrap">' + reference.getAttribute('data-mail') + '</span>',
allowHTML: true,
interactive: true,
hideOnClick: true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

j'ai essayé ça pour faire disparaitre le tooltip quand on affiche la modale. Je n'ai pas l'impression que ça marche.
J'ai aussi essayé document.querySelector('.part-infos-hover')._tippy.hide(); sans succès, si vous avez des idées

Copy link
Collaborator

Choose a reason for hiding this comment

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

je pense que la fonction hide() n'existe qu'avec jQuery
peut-être essayent document.querySelector('.part-infos-hover').style.display = 'none';
ou un truc du genre ?
c'est dommage que l'option hideOnClick ne fonctionne pas

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Non, ça ne marche pas non plus...

@hmeneuvrier hmeneuvrier force-pushed the feature/2222-bo-signalement-documents-edition branch from d7c312e to 66475e4 Compare February 20, 2024 15:09
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.

Mes retours de test (désolé si c'est précisé dans les specs que je n'ai pas lues intégralement...)

1/ Marges verticales à ajouter (déjà dit dans le code)

2/ si je suis dans un territoire, je peux sélectionner un type de doc ou photo (OK).
Mais en tant que SA, la liste est vide.
Normal ?

3/ la liaison aux désordres semble être différente de celle faite jusque là.
C'est à dire : je crée un signalement avec des photos liées à des désordres. Dans la modale Album, le nom du désordre s'affiche bien.
Par contre : je modifie le désordre dans la modale d'édition. Dans la modale Album, le nom du désordre est au format slug.

Editer <span class="fr-modal-file-edit-type"></span>
</h1>

<div class="fr-text--center fr-grid-row fr-grid-row--center">
Copy link
Collaborator

Choose a reason for hiding this comment

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

ajouter un peu de marge verticale

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

En fait j'avoue que je ne vois pas trop pour la marge... c'est comme sur les autres modales.
On regarde jeudi ?

@hmeneuvrier
Copy link
Collaborator Author

hmeneuvrier commented Feb 20, 2024

1/ Marges verticales à ajouter (déjà dit dans le code)

ok

2/ si je suis dans un territoire, je peux sélectionner un type de doc ou photo (OK). Mais en tant que SA, la liste est vide. Normal ?

non pas du tout... et je n'ai pas ce comportement (j'ai fait beaucoup de tests en SA)... c'est systématique ?

3/ la liaison aux désordres semble être différente de celle faite jusque là. C'est à dire : je crée un signalement avec des photos liées à des désordres. Dans la modale Album, le nom du désordre s'affiche bien. Par contre : je modifie le désordre dans la modale d'édition. Dans la modale Album, le nom du désordre est au format slug.

alors, il y a effectivement un truc bizarre, je ne pense pas que ça vienne de la PR, c'est dans SignalementDesordresProcessor.php, on a

                $this->addPhotoBySlug($photos, $signalement, $desordreCritereSlug, $desordrePrecisionSlug);
                $this->addPhotoBySlug($photos, $signalement, $desordreCritereSlug, $desordreCritereSlug);
                $this->addPhotoBySlug($photos, $signalement, $labelCategorieBO, $desordreCategorieSlug);

ça veut dire qu'on a le label seulement si la photo est rattachée à une catégorie de désordre, et pas si elle est rattachée au critère ou à la précision... cf ici : https://histologe-staging.osc-fr1.scalingo.io/bo/signalements/ef321266-8703-4cab-9a93-25e31879d126
En fait c'est à peu prrès normal que ça fonctionne comme ça car on n'a pas de slug pour les catégories... c'est peut-être qu'on n'aurait pas du utiliser ce tableau pour l'album photo... je vais voir comment on peut corriger ce souci
`

@hmeneuvrier
Copy link
Collaborator Author

@emilschn je me suis occupée du point 3, pour le 1 et 2, je veux bien voir avec toi jeudi

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.

Relecture et tests OK

@emilschn
Copy link
Collaborator

Un retour de test !

Je suis agent
Je vais sur un vieux signalement (fixture http://localhost:8080/bo/signalements/00000000-0000-0000-2023-000000000006)
J'ajoute une photo, puis je clique pour l'éditer
Je peux sélectionner que c'est une photo de désordre, mais je ne peux pas sélectionner le désordre correspondant (la deuxième liste reste vide).
(pas sûr que ce soit nécessaire, mais je le signale)

J'ai réussi à reproduire la liste de type vide, mais c'était un problème de cache js. Ca fonctionne pour le reste :)

@hmeneuvrier hmeneuvrier force-pushed the feature/2222-bo-signalement-documents-edition branch from c00cd96 to f6860d9 Compare February 26, 2024 08:18
@hmeneuvrier
Copy link
Collaborator Author

Un retour de test !

Je suis agent Je vais sur un vieux signalement (fixture http://localhost:8080/bo/signalements/00000000-0000-0000-2023-000000000006) J'ajoute une photo, puis je clique pour l'éditer Je peux sélectionner que c'est une photo de désordre, mais je ne peux pas sélectionner le désordre correspondant (la deuxième liste reste vide). (pas sûr que ce soit nécessaire, mais je le signale)

J'ai réussi à reproduire la liste de type vide, mais c'était un problème de cache js. Ca fonctionne pour le reste :)

corrigé

Copy link

sonarcloud bot commented Feb 26, 2024

Quality Gate Passed Quality Gate passed

Issues
3 New 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.

Je valide :) Bravo !

@emilschn emilschn merged commit 8e22be9 into develop Feb 26, 2024
3 checks passed
@hmeneuvrier hmeneuvrier deleted the feature/2222-bo-signalement-documents-edition branch March 5, 2024 14:45
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