Retour arrière depuis une fiche métier après une recherche ne fonctionne pas

skelz0r commented 2 years ago

Les étapes pour reproduire:

Aller sur https://entreprise.api.gouv.fr/catalogue
Taper "liasses fiscales" dans la recherche
Cliquer sur la fiche des liasses fiscales
On arrive sur https://entreprise.api.gouv.fr/catalogue/dgfip/liasses_fiscales
Faire retour arrière

De là on devrait voir le catalogue, la page ne change pas mais l'url change.

L'erreur js:

application-69390e0216692ccb31e3bcc64724aec6077285b83ff080dd613d3151defafbd1.js:821 

Uncaught (in promise) TypeError: Cannot read properties of null (reading 'querySelectorAll')
    at extended._handleHit (application-69390e0216692ccb31e3bcc64724aec6077285b83ff080dd613d3151defafbd1.js:821:12)
    at application-69390e0216692ccb31e3bcc64724aec6077285b83ff080dd613d3151defafbd1.js:804:40
    at Array.forEach (<anonymous>)
    at extended._updateResults (application-69390e0216692ccb31e3bcc64724aec6077285b83ff080dd613d3151defafbd1.js:803:31)
    at application-69390e0216692ccb31e3bcc64724aec6077285b83ff080dd613d3151defafbd1.js:785:26

Le code:

      _handleHit(controller, hit, entriesInResult) {
        var entry = document.querySelector(
          "[data-algolia-search-catalogue-hit='" + hit.objectID + "']"
        );
        entriesInResult.push(entry);

        entry
          .querySelectorAll(
            "[data-algolia-search-catalogue-hit-attribute]"
          )

c'est dans app/assets/javascripts/controllers/algolia_search_catalogue_controller.js

Samuelfaure commented 2 years ago

J'investigue un peu ici:

Je pense pas que l'erreur en console soit directement responsable du bug, plutôt le contraire en fait

Probablement un soucis avec turbo, je fais quelques recherches

skelz0r commented 2 years ago

Probablement un soucis avec turbo

Pour le coup je suis quasiment sûr que c'est un souci avec notre controller js qui ne s'init pas bien.

skelz0r commented 1 year ago

@Un3x ici

skelz0r commented 1 year ago

je précise: algolia est dans les choux là y'a moyen que ce bug ne soit pas solvable aujourd'hui.

skelz0r commented 1 year ago

Sur Brave Version 1.46.144 Chromium: 108.0.5359.128 (Official Build) (arm64) j'ai encore le problème avec exactement mes étapes.

Un3x commented 1 year ago

Alors moi j'ai des comportements assez chelou dans l'ensemble mais pas ceux que tu décris.

Je viens de tester et j'ai ni erreur js en console, et le comportement de l'enchainement de ces pages me semble ok. Par contre moi j'ai ca :

Aller sur https://entreprise.api.gouv.fr/catalogue
Taper "liasses fiscales" dans la recherche
Faire retour en arrière
[x] Les données filtrées correspondent bien à la recherche précédente
[x] L'url correspond bien à la recherche précédente
[ ] Le champs input n'est pas à jour vis à vis des params de l'url
[ ] Appuyer sur le bouton rechercher ne déclenche pas la recherche avec le contenu du champ

Trop bizarre, j'ai l'impression que la page filtre sur les paramètre get et la modification du champ input change les paramètres mais n'est pas initialisé avec les params (et le bouton recherché marche pas).

Comportement identique sous firefox, chrome et chromium

Un3x commented 1 year ago

Ok effectivement sous chrome j'ai le soucis. Sous firefox c'est ok

Un3x commented 1 year ago

Quelques informations complémentaire sur le comportement observé.

Suivre le lien d'un catalogue sans faire de recherche puis revenir en arrière ne produit pas le bug.

Enfin, le même problème est aussi présent dans la recherche algolia sur la recherche des documentations

Un3x commented 1 year ago

Okay la source du bug a été identifiée.

Il semblerait que le bug soit issue de Stimulus, plus particulièrement quand l'on revient en arrière d'une page sans Stimulus vers une page avec Stimulus. Pour être encore plus précis, il n'est pas improbable que Turbo intervienne dedans vu que cela ne concerne pas tous les enchainements de pages. Cela se produit uniquement quand la page d'où on initie le go back est une sous url (hormis les paramètres) de la page précédentes.

Cas fonctionnel :

Aller sur /catalogue
Faire une recherches sur liasses fiscales
Cliquer sur la home qui renvoie vers / (ou sur le swagger, tant que la furture url ne contient pas /catalogue)
Faire un retour arrière

Cas non fonctionnel :

Aller sur /catalogue
Faire une recherches sur liasses fiscales
Cliquer sur la documentation (on arrive sur /catalogue/infogreffe/rcs/extrait
Faire un retour arrière

Une solution a été décrite dans une issue github

On all non-stimulus pages that you want to enable to go Back into a stimulus-oriented page, add this script. It causes a reload only when going back into a Turbo-based page:

<script>
  if (window.history.state && window.history.state.turbo) {
    window.addEventListener("popstate", function () { location.reload(true); });
  }
</script>

Ca fonctionne, dans le cas où le bug est présent. Cependant, dans le cas où le bug n'est pas présent ca trigger un 2nd reload de la page qui est vraiment pas agréable lors de la navigation

skelz0r commented 1 year ago

GG pour l'investigation en tout cas. J'étais persuadé que le souci venait de la première ligne https://github.com/etalab/admin_api_entreprise/blob/acc707708888dffe3a159fab041afcc7c96f445e/app/assets/javascripts/controllers/algolia_search_catalogue_controller.js#L1 où turbo était load avant le document ready 🤷

skelz0r commented 1 year ago

imo en l'état on attend de voir ce qui se dit sur l'issue voir si un patch va sortir à un moment

Un3x commented 1 year ago

Okay, j'ai rajouté dans l'issue github chez eux un peu de contexte.

À mon avis, ils ne corrigeront pas si on leur fournis pas un bout de code reproductible là dessus mais bon.

etalab / admin_api_entreprise

Retour arrière depuis une fiche métier après une recherche ne fonctionne pas #675