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

1460 EPCI benefit simulation statistiques #129

Merged
merged 3 commits into from
Oct 12, 2023

Conversation

baptou12
Copy link
Contributor

@baptou12 baptou12 commented Oct 6, 2023

Ticket: https://trello.com/c/TvwI345k/1460-en-tant-que-charg%C3%A9e-de-d%C3%A9ploiement-je-veux-savoir-dans-quels-epci-jai-des-aides-et-dans-lesquels-je-nen-ai-pas-encore

Ajoute une page pour voir les statistiques sur les institutions :
image

Pour le moment seulement les EPCI.
Montre la population, le nombre de simulation sur les 6 derniers mois et le nombre d'aides.
Possible de :

  • filtrer par nom, code ou type d'EPCI (filtres cumulables)
  • ordonner par population, nombre de simulation ou nombre d'aide (ordres exclusifs)

Tech : Ajoute un service pour filtrer et ordonner les tableau (collection d'objets similaire), on pourra utiliser ça dans d'autres pages pour séparer la logique de fetch de la logique de filtre/ordre.

@baptou12 baptou12 marked this pull request as draft October 6, 2023 15:15
@baptou12 baptou12 force-pushed the 1460-epci-benefit-simulation-statistiques branch 2 times, most recently from 4fffdf7 to f756b0b Compare October 9, 2023 15:35
@baptou12 baptou12 marked this pull request as ready for review October 9, 2023 15:53
@baptou12 baptou12 requested a review from Cugniere October 9, 2023 15:53
@baptou12 baptou12 force-pushed the 1460-epci-benefit-simulation-statistiques branch from 633669d to 1d49abc Compare October 9, 2023 15:56
@baptou12 baptou12 force-pushed the 1460-epci-benefit-simulation-statistiques branch from 1d49abc to c96bd49 Compare October 10, 2023 07:55
pages/institutions.js Outdated Show resolved Hide resolved
Comment on lines +16 to +24
sortByKey(key, order = "asc") {
const newLocal = order === "asc"
this.filteredData = [
...this.filteredData.sort((a, b) =>
newLocal ? (a[key] > b[key] ? 1 : -1) : a[key] < b[key] ? 1 : -1,
),
]
return this
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Le problème de faire du tri uniquement sur les colonnes numériques c'est qu'on perd du coup la capacité de faire un tri alphabétique sur les noms d'epci, voir de retourner à l'état de la page d'origine (ordonné par code d'epci croissant).

C'est pas forcément important sur cette PR vu qu'on utilise le filtrage, mais ça reste assez facile de trier en fonction d'un paramètre du type de contenu de la colonne

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ouais, il faudra que je le rajoute en effet, j'avais en tête mais je voyais pas trop comment le justifier dans cette PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Après, non pas dans l'implémentation ici mais dans son utilisation historique, on peut aussi se demander si le tri alphabétique est vraiment pertinent niveau UX, vis à vis d'un filter par exemple.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ça se débat, mais si on a un tri par défaut ça a du sens de pouvoir le reproduire côté utilisateur après avoir trié autrement le tableau une première fois

pages/institutions.js Outdated Show resolved Hide resolved
@baptou12 baptou12 force-pushed the 1460-epci-benefit-simulation-statistiques branch from c96bd49 to 2166449 Compare October 11, 2023 10:09
Copy link
Contributor

@Cugniere Cugniere 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, ça sera peut être nécessaire d'itérer sur le tri du tableaux dans une autre PR si on estime que c'est pertinent

@baptou12 baptou12 merged commit 3f412b9 into main Oct 12, 2023
3 checks passed
@guillett guillett added this to the BC 1406243900 milestone Oct 17, 2023
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.

3 participants