opendatateam / udata-front-kit

Verticales thématiques adossées à data.gouv.fr
MIT License
4 stars 3 forks source link

Feat: archived dataset in bouquet shows unavailable tag #510

Closed narduin closed 1 month ago

narduin commented 2 months ago

J'ai ajouté le tag "non disponible" lorsqu'un dataset est archivé. Il me semble que @martyKN voulait ajouter un texte supplémentaire.

J'ai réutilisé le code de BouquetDatasetCard pour pouvoir accéder à dataset.archived

Fix https://github.com/ecolabdata/ecospheres/issues/276

netlify[bot] commented 2 months ago

Deploy Preview for meteo-france ready!

Name Link
Latest commit a960c0343ebfc6d63b9048a0e46650b5c62fe63f
Latest deploy log https://app.netlify.com/sites/meteo-france/deploys/66e9a604f7d7d800087c66b8
Deploy Preview https://deploy-preview-510--meteo-france.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 2 months ago

Deploy Preview for ecospheres ready!

Name Link
Latest commit a960c0343ebfc6d63b9048a0e46650b5c62fe63f
Latest deploy log https://app.netlify.com/sites/ecospheres/deploys/66e9a60400d69d00086ce5d7
Deploy Preview https://deploy-preview-510--ecospheres.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

narduin commented 1 month ago

Il faudrait hydrater les datasetsProperties plus "haut" dans la hiérarchie des composants avec ces infos qui nous intéressent.

Peut-être qu'on peut intégrer ça dans le composable useExtras ? Et ajouter un attribut local archived dans le modèle DatasetProperties à l'image de remoteDeleted.

J'ai ajouté une propriété archived aux datasetProperties dans useExtras. Pas sûr qu'elle soit locale mais on ne fait plus qu'un seul appel API.

narduin commented 1 month ago

Effectivement il n'y a plus qu'un appel, en revanche "non disponible" ne s'affiche plus sur le titre pour moi :-/

En effet, au chargement/reload le tag n'apparaissait pas mais après sauvegarde de l'éditeur on le voyait bien… J'ai changé get() par load() et ça semble fonctionner.

abulte commented 1 month ago

En effet ! Mais on retombe sur le double appel API :-)

Je pense qu'il manque la migration de la partie remoteDeleted dans le composable.

narduin commented 1 month ago

En effet ! Mais on retombe sur le double appel API :-)

J'ai du mal à y croire mais j'ai uniquement laissé la propriété archived dans topic.ts et ça semble fonctionner. Je me suis compliqué la vie je crois…

narduin commented 1 month ago

Ca marche parce que l'attribut archived a été persisté dans l'API. On veut éviter ça parce que c'est une propriété dont on a besoin qu'en "local" et on ne veut pas gérer la synchronisation sur le serveur entre dataset.archived et datasetProperties[].archived.

Je pense que la meilleure solution reste de récupérer remoteDeleted et archived dans le composable useExtras. Il faut aussi retrouver comment remoteDeleted se retrouve exclu de la persistence via l'API et appliquer le même traitement à archived.

D'accord je n'avais pas cette notion de synchro. J'ai creusé un peu remoteDeleted mais je n'avais pas réussi à reproduire le comportement. Je vais chercher !

A court terme il faudrait supprimer ton bouquet de test qui comporte des extras non conformes (et éventuellement d'autres sur lesquels tu aurais testé ?).

Il n'y a que celui-ci.

narduin commented 1 month ago

Au final c'était assez tordu de mettre l'appel aux datasets dans le useExtras. Du coup je l'ai remonté dans la liste, ce qui permet d'envoyer des props aux différents composants de l'accordéon directement. Je veux bien ton avis là dessus @abulte.

Je me suis également heurté à un petit soucis aléatoire des temps de réponse de l'API. J'ai mis un FIXME pour y penser et voir si ma solution convient (un splice au lieu d'un push).

narduin commented 1 month ago

Il reste un problème sur le remoteDeleted qui n'a plus l'air de se propager correctement. Cf https://demo.ecologie.data.gouv.fr/bouquets/bouquet-alb vs http://localhost:5173/bouquets/bouquet-alb (attendu ci-dessous).

C'est le check sur le Map dans le v-if qui faisait sauter la card. Je l'ai déplacé dans la props

narduin commented 1 month ago

Aussi, quand j'ajoute un jeu de données archivé à un bouquet, je n'ai pas immédiatement la carte du jeu de données dans le bouquet. Au reload c'est bon.

Les nouveaux datasets n'étaient pas concernés par le load qui était effectué uniquement onMounted J'ai refactor tout ça pour prendre en compte les ajouts sans dupliquer les appels API.

narduin commented 1 month ago

oh non le rebase 😱

abulte commented 1 month ago

Ah oui pas terrible le rebase :-/ Il y a des commits dans cette PR qui sont déjà sur main. Peut-être revenir au commit avant ton rebase et tenter plutôt un merge ?