[PRD] | [Issue #2529] Organization Hierarchy Cache

[Da-Colon]

Closes #2529

GIF

[netlify]

✅ Deploy Preview for decent-interface-dev ready!

Name	Link
🔨 Latest commit	67d1c1e
🔍 Latest deploy log	https://app.netlify.com/sites/decent-interface-dev/deploys/674e0ae6d1af1800081459a2
😎 Deploy Preview	https://deploy-preview-2566.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]

This implementation uses a cache expiry of 1 week. So at least every 7 days this cache will expire and be re-fetched fresh again.

I'm not sure if that experience is really what we're after here.

But the problem with making the cache expiration longer, is that DAO names can change! So, if a DAO name changes, we'd be displaying the old name until the cache expires.

I dealt with this recently in my "cache favorites" PR. My solution was to:

• set the cache to never expire
• load the data from the cache immediately
• in the background do a fetch and if there have been changes, update the cache

Would something like that be appropriate for this PR?

---

As an aside, we are storing a lot of data for each "Node"; the whole DaoInfo object. All we really need are the address and name, right? Wonder if this PR is the place to refactor our Organization page's data requirements.

[Da-Colon]

This implementation uses a cache expiry of 1 week. So at least every 7 days this cache will expire and be re-fetched fresh again.

I'm not sure if that experience is really what we're after here.

But the problem with making the cache expiration longer, is that DAO names can change! So, if a DAO name changes, we'd be displaying the old name until the cache expires.

I dealt with this recently in my "cache favorites" PR. My solution was to:

• set the cache to never expire
• load the data from the cache immediately
• in the background do a fetch and if there have been changes, update the cache

Would something like that be appropriate for this PR?

As an aside, we are storing a lot of data for each "Node"; the whole DaoInfo object. All we really need are the address and name, right? Wonder if this PR is the place to refactor our Organization page's data requirements.

We can definitely discuss, but this is that change. The view layer component has been updated with only what it needs.

About DAO Info that is the bare min. it does two calls per safe right now

SafeAPI -> We will need to modules, I could take this out but we will need it back in later anyway
Subgraph -> All the other stuff, DAOName and Heirarchy.

And thats currently it that is what makes up DAOInfo

[Da-Colon]

I could break out the Safe Type from DAOInfo, right now I only need address and the in the future we will be using the modules to determine type (so still don't really need to save the all whole safe thing right now)

[Da-Colon]

I don't know why I argue you are right, second I took at look at the type I mean duh. one second

[Da-Colon]

@adamgall Still need to make the same calls. But created its own type

[Da-Colon]

Before this is merged. I am pausing with the thought. So we don't have to migrate. might need to go ahead and add in the work for the type of governance? so that can get cached as well?

[adamgall]

Can bump the CACHE_VERSION for this new local storage store if we ever need to invalidate its cache.

edit: speaking of which, should probably set a CACHE_VERSION for this so we can invalidate it in the future if/when we need to add more stuff to it, like you're hypothesizing in your previous comment @Da-Colon !

edit2: just tested, if we don't have a CACHE_VERSION for this new CacheKey, it's all good it just starts at version undefined. In the future we can add a CACHE_VERSION and set it to 1 and that will cause old local storage keys to be invalidated.

edit3: but i mean, it might be good just to explicitly set the version to 1 now in this PR, just for clarity.

[Da-Colon]

Can bump the CACHE_VERSION for this new local storage store if we ever need to invalidate its cache.

edit: speaking of which, should probably set a CACHE_VERSION for this so we can invalidate it in the future if/when we need to add more stuff to it, like you're hypothesizing in your previous comment @Da-Colon !

edit2: just tested, if we don't have a CACHE_VERSION for this new CacheKey, it's all good it just starts at version undefined. In the future we can add a CACHE_VERSION and set it to 1 and that will cause old local storage keys to be invalidated.

edit3: but i mean, it might be good just to explicitly set the version to 1 now in this PR, just for clarity.

100% agreed to set at 1

[Da-Colon]

So as you said the expiry is defaults to ONE_WEEK. I'm not really sure what to set it.

This got me thinking. There are places were caching got us in trouble but saves alot. We could think about a sync IconButton that will allow a user to manually refresh cached data? (not for this PR lol). For when things look a little off. got refresh a section of data or something.

In this case. we are caching the childDAOs. if that changes then this quickly will no longer be displaying correctly..