Add recipient registry info to store #358
Conversation
| } from '@/api/contributions' | ||
| import { recipientRegistryType } from '@/api/core' | ||
| import { loginUser, logoutUser } from '@/api/gun' | ||
| import { getRegistryInfo, RegistryInfo } from '@/api/recipient-registry-optimistic' |
There was a problem hiding this comment.
Other types of registries (such as Kleros adapter) can have registry info too, but of different type.
There should be a wrapper function in api/projects.ts
There was a problem hiding this comment.
I checked out api/projects.ts but I'm only seeing wrapper functions around getting/registering projects. I'm not seeing anything around registry info in api/recipient-registry-kleros but please let me know if I'm missing something you'd like me to add here (or if there's a wrapper function you'd like me to create).
|
Thanks for the review @xuhcc - I made some updates after reading your comments & thinking through this a bit more. Reasoning on this approach:
Again, please let me know what you think here. We need this functionality in our fork so that we can conditionally display calls to action for projects to apply (if the round is using an optimistic recipient registry). I figure this is logic this core project could benefit from as well. |
| async mounted() { | ||
| const roundAddress = await getCurrentRound() | ||
| await this.$store.dispatch(SELECT_ROUND, roundAddress) | ||
| this.$store.dispatch(LOAD_RECIPIENT_REGISTRY_INFO) |
There was a problem hiding this comment.
This is not correct because the round address can also come from route parameter in ProjectList component.
See my previous comment on this section of code
There was a problem hiding this comment.
Construction of the app's state begins with a round address, because almost everything else depends on it.
There are two ways in which this variable can be set:
- If user lands on the project list page, the
ProjectListcomponent gets round address from route parameters. - If route parameters does not contain round address, or if user lands on some other page, the app gets round address from the
FundingRoundFactorycontract.
(1) is the reason why the initialization does not currently take place in the App.created(). Doing initialization in App should be possible, but it would require more work.
Your proposed change simply creates a race for dispatching SELECT_ROUND between App and ProjectList components.
There was a problem hiding this comment.
Thanks for the explanation.
- If route parameters does not contain round address, or if user lands on some other page, the app gets round address from the FundingRoundFactory contract.
Where does this logic take place? Does it happen on page load / app initialization? I see we dispatch LOAD_ROUND_INFO every 60 seconds in App.vue but for our purposes we need to fetch round info on any initial page load. I'm open to refactoring this to accomplish that without creating a potential clash vs. the route parameters of ProjectList.
|
Closing this out. Realizing this change is more specific to the use case of https://github.com/ethereum/clrfund where we intend to have the frontend app tailored to a single funding round (vs. having the ability to view multiple rounds). Thanks again for the feedback! |
@xuhcc looking to get your take on this...
Currently we store recipientRegistryAddress in the store.
In our Eth2 CLR fund work I found myself querying registry info in multiple different components. I think the ideal approach here is to fetch that data once & add it to the Vuex store.
Please let me know what you think! Appreciate any feedback.