fix(ui): fix losing session data when navigating different versions

[n1k0]

🔧 Problem

This patch:

• reapplies fix(ui): fix losing session data when navigating different versions #1694 to fix Perte des bookmarks lorsqu'on passe de la v7 à la dernière version #1684
• improves it to preserve existing bookmarks around in every case
• it also removes all version pills from the bookmark lists in the sidebar and the comparator
• removes the version info from the footer, only refers to commit hash

🚨 Points to watch/comments

Contrary to the initial impl. in #1694, this one is non-destructive, meaning there won't be a session reset nor emptying the list of existing bookmarks for end users when deployed to production.

The patch introduces a new separate localStorage key for the ongoing version (ecobalyse) and leaves untouched the previous one (store); while it's a bit sad to have those two stores, the UX is better and safer for end users and I think that's what matters most.

🧐 How to test

• Switch from current to regulatory version and vice-versa multiple times: your version-compatible bookmarks should always be retrieved everytime, no data loss, ever
• Tweak your serialized JSON data in localStorage to create an invalid bookmark (pick one from the tests added in the diff) and refresh the page: your authentication status should be preserved, only the forged invalid bookmarks should have been dropped from the list.
• You could also get your JSON localStorage data from production and paste it in the localStorage of the PR to see how much non-destructive it is 😅

[n1k0]

Hmm hold on reviewing, I spotted a last mistake to fix

[n1k0]

@vjousse this is back to ready for review… not super proud of the solution to have both app versions living on the same domain by using two distinct store keys, but backporting all this elm code to stable/textile was too complicated and cumbersome so I don't think it's worth doing. Tell me what you think :)

[vjousse]

Tested locally, with valid and invalid JSON: works as expected, god job! Are we sure we want to keep invalid bookmarks in the local storage when we detect one? Shouldn’t we delete the invalid entry from the storage?

[n1k0]

Are we sure we want to keep invalid bookmarks in the local storage when we detect one? Shouldn’t we delete the invalid entry from the storage?

We actually have to keep them because they may be valid in another version (a valid v7 one is invalid in ongoing version and the other way around)

Edit: Oh, you mean now we have distinct stores, it doesn't matter anymore and we should garbage collect them? That makes sense, thanks for the feedback 🙏

[vjousse]

Yes indeed, that’s what I was talking about!

[n1k0]

@vjousse implemented the change in 21eeffc, would you mind checking it again? 🙏

[vjousse]

It looks like I still have the 3 entries in the local storage even if one is invalid

[n1k0]

It looks like I still have the 3 entries in the local storage even if one is invalid

@vjousse that's because you're testing the v7 regulatory version, which I can't patch as previously stated… the filtering of invalid bookmarks is only implemented for master. Or do you observe this behavior on the ongoing version as well?

[n1k0]

@vjousse Oh I see the ecobalyse key name in your capture… I think you need to perform at list a first bookmark update — eg. renaming one — in order for the reencoding, filtering and saving to happen. Could you please try? 🙏

Edit: tested this myself, it's indeed because you have to perform an update to the session for the filtering to be performed and persisted.

[vjousse]

Indeed, with an update of the bookmarks, it works as expected 🎉

[vjousse]

LGTM!