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

Tech - Rendre la side window indépendante #3668

Closed
wants to merge 25 commits into from

Conversation

ivangabriele
Copy link
Member

@ivangabriele ivangabriele commented Sep 19, 2024

Cette PR ajoute une synchronisation du store Redux entre plusieurs onglets navigateur ayant ouvert MonitorFish.

Cela permet d'ouvrir la fenêtre secondaire comme une URL indépendante, qu'elle doit ouvert via windows.open() ou via un nouvel onglet, tout en conservation ses interactions avec la fenêtre principale.

Caution

Il faut garder en tête que le state des slides est synchronisé mais les states Redux RTK, donc les données provenants de l'API, ne le sont pas.

Dans les quelques cas potentiels où c'est une nécessité, il est important de le prendre en compte :

  • soit en s'assurant que les deux onglets/fenêtre aient récupéré les données (avec un léger risque d'asymétrie des données reçues),
  • soit de partager les données d'API dans le state du slice.

Je n'ai pas enquêté sur le sujet de savoir comment synchroniser RTK car j'ai peur d'y perdre du temps. Mais il n'est pas impossible qu'il y ait une solution aisée pour régler ce soucis.

Linked issues


  • Tests E2E (Cypress)

@ivangabriele ivangabriele marked this pull request as draft September 19, 2024 13:54
@ivangabriele ivangabriele changed the title Tech - Make side window independent Tech - Rendre la side window indépendante Sep 19, 2024
@ivangabriele ivangabriele force-pushed the ivan/make-side-window-independent branch from a0b977f to 3b203f2 Compare September 19, 2024 16:25
@louptheron
Copy link
Collaborator

louptheron commented Sep 23, 2024

Soucis actuels :

  • La sérialisation par défaut ne gère pas tous les objets, il faut utiliser un hack : const serializedAction = JSON.parse(JSON.stringify(action))
  • Un dispatch d'action redux qui donne lieu a un appel API déclenchera deux appels si MainWindow et SideWindow écoutent cette action (ex. avec le hook useGetFilteredMissionsQuery())
  • Le cache RTK n'est pas partagé entre les deux fenêtres, il faut la partager manuellement, ex (ReduxStateSync):

handleBroadcastChannelMessage:

case MessageType.DispatchAction:
        if (message.stampedAction) {
          if (message.type.includes('monitorfishApi/executeQuery/fulfilled')) {
            this.#updateCacheManually(this.#dispatch!, message.stampedAction.meta, message.stampedAction.payload)

            return
          }

          this.#dispatch!(message.stampedAction)
        }

        // eslint-disable-next-line no-useless-return
        return
#updateCacheManually(dispatch, args, payload) {
    dispatch(monitorfishApi.util.upsertQueryData(args.endpointName, args.meta.arg.endpointName, (draft) => {
      Object.assign(draft, payload)
    }))
  }

Si une action redux est à l'origine ce fetch RTK, il faut la bloquer dans l'autre navigateur pour éviter de re-fetch la donnée.

@louptheron
Copy link
Collaborator

louptheron commented Sep 23, 2024

Après discussion, il est peut-être plus simple de ne pas gérer la synchro des caches RTKQ.
Conséquence: Il peut y avoir deux appels APIs en simultané dans les deux fenêtres ouvertes.

Répartition des tâches :

  • Loup:
    • Corriger les tests Cypress
      • Fix les states redux non-sérialisables (Measurement, InterestPoint, etc) (erreur object could not be cloned)
    • Vérifier le scope des appels RTKQ
    • Ajouter un cache de 1/2 secondes sur le use-case des missions pour gérer le double appel API
    • Cacher le chat dans la side window
  • Ivan:
    • Supprimer le concept de tabId
    • Faire en sorte de n'avoir qu'une seule side window ouverte et aucune autre main window

@louptheron louptheron force-pushed the ivan/make-side-window-independent branch from 987f887 to 5cab2a8 Compare September 24, 2024 11:36
Copy link

sonarcloud bot commented Sep 25, 2024

@louptheron
Copy link
Collaborator

Fixed by MTES-MCT/monitor-ui#1382

@louptheron louptheron closed this Sep 26, 2024
louptheron added a commit that referenced this pull request Sep 27, 2024
## Linked issues

- cherry-pick depuis la PR
#3668

----

- [ ] Tests E2E (Cypress)
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.

2 participants