fix: observations metadata #1829
Conversation
arnaudambro
had a problem deploying
to
review-auto
GitHub Actions
Error
| @@ -29,7 +29,6 @@ export default function useDataMigrator() { | |||
| const setLoadingText = useSetRecoilState(loadingTextState); | |||
| const user = useRecoilValue(userState); | |||
| const setOrganisation = useSetRecoilState(organisationState); | |||
| const customFieldsObs = useRecoilValue(customFieldsObsSelector); | |||
There was a problem hiding this comment.
ici, customFieldsObs n'est pas encore initialisé
arnaudambro
force-pushed
the
fix/observations-metadata
branch
from
d532b56
to
415e467
Compare
arnaudambro
temporarily deployed
to
review-auto
GitHub Actions
Inactive
|
| @@ -22,13 +22,12 @@ export const customFieldsObsSelector = selector({ | |||
| key: 'customFieldsObsSelector', | |||
| get: ({ get }) => { | |||
| const organisation = get(organisationState); | |||
| if (!organisation) return defaultCustomFields; | |||
There was a problem hiding this comment.
c'était une mauvaise idée: j'avais mis ça parce qu'on utilisait ce selecteur dans DataMigrator mais ça hurlait qu'organisation était null donc organisation.customFieldsObs de la ligne en dessous faisait planter le système
| @@ -108,19 +109,19 @@ const SignIn = () => { | |||
| if (organisation._id !== window.localStorage.getItem('mano-organisationId')) { | |||
| await resetCache(); | |||
| } | |||
| setOrganisation(organisation); | |||
There was a problem hiding this comment.
placer ces deux lignes avant la demande de clé fait du bien: on peut se servir du state recoil organisation et user dans la suite
|
🎉 Deployment for commit 415e467 : IngressesDocker images
|
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
ça fonctionne chez moi, je n'ai pas eu de bugs.
J'ai lu le code, et comme je n'ai pas tout le contexte (malgré les détails dans le ticket, merci beaucoup d'ailleurs !), je n'ai pas tout compris, seulement un peu MAIS c'est OK pour moi : en effet, tu as double check cette fois ci et tu as fait ton correctif en connaissance de cause + je ne vois rien de choquant !
|
Go ! |
|
🎉 This PR is included in version 1.302.11 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
PR "intéressante"parce qu'elle fait suite à un gros bug semaine dernière.
Contexte: j'avais fait une migration sur les observations de territoire parce que j'avais remarqué que certaines n'avaient pas de
observedAt, d'autrews avaient un timestamp au lieu d'une date ISO, etc. donc pour harmoniser le tout, j'avais fait une migration... qui a foutu un sacré bazar: toutes les données custos avaient disparues !j'ai compris pourquoi: c'est à cause de ce fichu
batch statede react (https://react.dev/learn/queueing-a-series-of-state-updates#react-batches-state-updates)qui faisait qu'au moment d'appeler
const customFieldsObs = useRecoilValue(customFieldsObsSelector);, l'organisation n'était pas encore dans le state recoil (PS: aucun rapport mais avec zustand on n'aurait pas ce problème je pense)cette PR fixe le problème et relance la migration, avec laquelle j'espère sans trop savoir pourquoi résoudre le problème de stats https://trello.com/c/8zAAkkVg/1308-bug-ema-emos-statistiques-dobservations-qui-matchent-pas
n'empêche que ça fait mal au cerveau