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 - Liste signalement] [Front] Affichage de la liste #2528

Merged
merged 7 commits into from
May 13, 2024

Conversation

sfinx13
Copy link
Collaborator

@sfinx13 sfinx13 commented May 3, 2024

Ticket

#2122

Description

Note

Ne pas faire de revue sur les filtres ! (Ce n'est pas ce ticket)

Maquette

Spec

Changements apportés

  • Implémentation des différents composants :
    - TheHistoAppSignalementList
    - TheHistoSignalementListCards
    - TheHistoSignalementListHeader
    - TheHistoSignalementListPagination
  • Utilisation de WidgetsSettings pour avoir les infos de l'utilisateur
  • Ajout du profilDeclarant dans le DTO
  • Suppression de csrf_token sur l'export (pas utile)
  • Tester si les paramètres d'URL se mette à jour à la pagination et au tri
    image

Test review app pour la pagination avec un peu plus d'enregistrement

Pré-requis

make npm-build
FEATURE_LIST_FILTER_ENABLE=1

Tests

  • Afficher la liste en tant que Super Admin et Admin territoire, le bouton supprimer et les badges d'affectation s'affiche
  • Vérifier que les cartes s'affichent dynamiquement
  • Afficher la liste en tant que agent partenaire, le bouton supprimer et les badges d'affectation ne s'affiche pas
  • Trier la liste
  • Paginer sur la liste
  • Supprimer un signalement et vérifier qu'il n'est plus présent (vérifier message de confirmation)
  • Faire un export

@sfinx13 sfinx13 force-pushed the feature/2122-new-list-signalement branch 6 times, most recently from fbefd03 to ea30ee8 Compare May 6, 2024 08:49
Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

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

Les tests sont tous OK, mais j'ai quelques remarques :

  • l'export csv n'est pas trié suivant le tri choisi, est-ce normal ?
  • je trouve qu'il y a un peu trop d'espace entre la ligne "Dernier suivi" et la ligne des boutons (mais j'aurais du le dire à la PR précédente)
  • le tri par référence est un tri alphabétiquee (2022-10 arrive avant 2022-8) et je pense que ça risque de faire râler les partenaires... je crois qu'on a une fonction twig pour ça quelque-part, il faudrait peut-être l'adapter.
  • la selct-box de tri n'est pas tout à fait comme ça dans la maquette, mais je ne sais pas si ça fait partie des choses à tester dans cette PR (Normalement le Trier par : est hors de la select, et les intitulés sont simplement : Ordre décroissant, Ordre croissant , Ordre alphabétique (A -> Z), Ordre alphabétique inversé (Z -> A) , Le plus récent , Le plus ancien

$response->setCallback(function () use ($signalementExportLoader, $filters, $user) {
$signalementExportLoader->load($user, $filters);
});
$filters = $request->getSession()->get('filters');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bon, pour le tri dans l'export, je vois que ça n'est pas le cas non plus dans l'ancienne liste, ça pourrait faire l'objet d'un ticket à part si @mathildepoulpux le juge utile

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A voir à son retour, je ne sais si c'est utile

Copy link
Collaborator

@mathildepoulpux mathildepoulpux May 27, 2024

Choose a reason for hiding this comment

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

C'est effectivement un ticket à créer, ça fait l'objet de plusieurs retours de territoire !
Je créer le ticket dans "à spécifier"

src/Controller/Back/SignalementController.php Outdated Show resolved Hide resolved
@emilschn
Copy link
Collaborator

emilschn commented May 6, 2024

Retours de test :

  • quand on change de page, le scroll n'est pas tout en haut ; il faudrait mettre le scrollTop à 0
  • en tant qu'usager, j'ai quand même la liste des partenaires affectés (mais pas le badges), c'est peut-être ça que tu voulais tester ?
  • quand il n'y a pas de déclarant, il est quand même inscrit Déclarant : ; mais c'est peut-être un souci de fixtures [EDIT : déjà remonté par Hélène]

Page suivante </a></li>
<li><a class="fr-pagination__link fr-pagination__link--last" href="#"> Dernière page </a></li>
<li>
<a :href="pagination.current_page > 1 ? '#': null"
Copy link
Collaborator

@emilschn emilschn May 6, 2024

Choose a reason for hiding this comment

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

c'est censé être plutôt des boutons, si on ne change pas de page :) (ça évite la gestion du href)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

C'est vrai que comme ça c'est plutôt des boutons mais je vais plutôt remplacer les # par des paramètres d'url ?page=[numero_page]
C'est comme si c'était un changement de page et ça permettra aussi d’accéder au page directement va la barre d'adresse

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oui, si on peut, c'est top :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

C'est bon c'est géré je vous laisse tester j"édite la PR

Copy link
Collaborator

Choose a reason for hiding this comment

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

c'est super cool

@sfinx13 sfinx13 force-pushed the feature/2122-new-list-signalement branch from 14e4081 to 2c300d8 Compare May 7, 2024 10:18
@hmeneuvrier
Copy link
Collaborator

  • quand on change de page, le scroll n'est pas tout en haut ; il faudrait mettre le scrollTop à 0

seul test encore KO pour moi, tout le reste est TOP

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.

Trop bien. Relecture et Retest ok ! :)

@sfinx13 sfinx13 force-pushed the feature/2122-new-list-signalement branch from f2cc6f9 to af1f3d9 Compare May 13, 2024 08:27
Copy link

sonarcloud bot commented May 13, 2024

Quality Gate Passed Quality Gate passed

Issues
0 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

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

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

Ok parfait

@hmeneuvrier hmeneuvrier merged commit 8c36a0d into develop May 13, 2024
4 checks passed
@sfinx13 sfinx13 deleted the feature/2122-new-list-signalement branch May 13, 2024 15:37
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.

4 participants