2084 Bug | Roles Data doesn't refresh

[DarksightKellar]

This PR fixes an elusive bug: switching safes was not reloading the new safe's roles. IOW, the previous safe's roles would be visible in the new safe's roles page.

Ideally I should've branched off develop and used that as this PR's base, but, alas, I branched off roles-0.2.0/streams instead. Means this fix will make it to develop only when streams does. Not the most terrible thing to have happened in the world, since this was never an urgent bug in the first place.

Also snuck in a tiny update I made to the "today" indicator on the DecentDatePicker component

[netlify]

✅ Deploy Preview for decent-interface-dev ready!

Name	Link
🔨 Latest commit	3b89526
🔍 Latest deploy log	https://app.netlify.com/sites/decent-interface-dev/deploys/66b4d9ff3ffc710008d33fe2
😎 Deploy Preview	https://deploy-preview-2188.app.dev.decentdao.org
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[adamgall]

I'd like to see an explicit reset function, rather than just calling setHatsTreeId(null)

[adamgall]

It's possible to switch Safes without invoking the HomePage component at all (via the search bar). Does this work in that case?

There is another place in the app where resetSafeState() is called. Should this Zustand reset go there, too?

[DarksightKellar]

It's possible to switch Safes without invoking the HomePage component at all (via the search bar). Does this work in that case?

Tested this, the roles don't persist into the next safe

There is another place in the app where resetSafeState() is called. Should this Zustand reset go there, too?

I don't think so. Regardless, all paths to switch safes that I tested didn't persist a previous safe's roles. Also this bug was hard to reproduce in the first place, so while I did test alternate paths, most testing was focused on the reliable edge case where roles were not getting cleared out
@adamgall

[adamgall]

@DarksightKellar what is the mechanism that allows the Zustand stores to reset themselves when a Safe is changed through the search bar, if we're not explicitly doing that?

[DarksightKellar]

@adamgall I think this useEffect in useKeyValuePairs is the answer:

it's got a daoAddress dependency (and chain id)

getHatsTreeId itself may return null or undefined, so if the context safe has no such events, the "reset" happens, here (which is something I believe you anticipated when building this part out, hence this code):

[adamgall]

I don't like relying on this side effect to "clear out" the state. Would prefer that happens more explicitly.

[DarksightKellar]

I don't like relying on this side effect to "clear out" the state. Would prefer that happens more explicitly.

@adamgall Hmm I think you've gotten too hung up on the "clear out state" semantic. Hear me out:

useKeyValuePairs is the way to load up a safe's hat tree, regardless. We don't know before checking it if a safe has a tree or not, so getHatsTreeId(safeEvents... could return null or undefined if the context safe doesn't in fact have a (declared) tree. In that case setHatsTreeId will (and should) be called with null or undefined. This is a feature and not a happy accident -- It's not a "clearing out state" side effect, it's a "setting the tree for this safe we have just switched to" intentional effect.

I don't see any need to, somewhere else, do:

• listen for safe changes
• try to get the new safe's tree
• if safe has got no tree, specifically reset the tree store
• OR specifically set the tree store if the safe happens to have one.

Actually a better argument would be to rename useKeyValuePairs to something along the lines of useLoadSafeHatsTree, because that is all it is doing. It just so happens to be using the KeyValuePairs contract internally.

[tomstuart123]

@DarksightKellar - to replicate is it as simple as jumping between safe's role pages quickly?

[adamgall]

Approving, but one suggestion

[adamgall]

This returns boolean | undefined. Can it be cleaned up to only return a boolean?

[DarksightKellar]

Ah yeah good call

[tomstuart123]

@adamgall / @DarksightKellar - roles jumping between safes issue still appears for me
Video of issue risen here - https://decentdao.slack.com/archives/C06QZTUTKRC/p1722952950606279?thread_ts=1722949970.035939&cid=C06QZTUTKRC