Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(registry) feat: add claiming to unit details #77

Merged
merged 4 commits into from
Aug 27, 2024

Conversation

Tanya-atatakai
Copy link
Collaborator

Proposed changes

  1. Restyled rewards section to the table view
  2. Moved some files to typescript
  3. Refactored getPendingIncentives logic (there's no need to get mappers for both agents and components since we pass the unitType)
  4. Added claim button (only visible for the owner and where there is something to claim)
Screenshot 2024-08-16 at 11 28 25 Screenshot 2024-08-16 at 11 28 14

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Copy link

vercel bot commented Aug 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
bond ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 2:09pm
govern ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 2:09pm
launch ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 2:09pm
operate ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 2:09pm
registry ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 2:09pm
tokenomics ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 2:09pm

const owners = await contract.methods.getOwners().call();

if (Number(threshold) > 0 && owners.length > 0) {
// TODO: check and fix error: Property 'getStorageAt' does not exist on type 'JsonRpcProvider | FallbackProvider'.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls take a look at this, ts says that there's getStorageAt on provider, but didn't have much time to investigate if it's safe to switch to provider.getStorage or the impact of this. Maybe we should plan the fix 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember why I wrote this 🙈, we should definitely plan to fix this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this project uses ethers v6 you need: https://docs.ethers.org/v6/api/providers/#Provider-getStorage

Copy link
Collaborator

@mohandast52 mohandast52 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for refactoring—I've added a few minor comments.

Comment on lines +165 to +167
refetch();
fetchPendingIncentives();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe await?

Suggested change
refetch();
fetchPendingIncentives();
await refetch();
await fetchPendingIncentives();

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can run fetchPendingIncentives at the same time when running refetch, no need to wait when it's done, the goal is just to re-request the data. and the result will be recalculated in the rewards useMemo

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I was concerned that the loader might not be in sync because we're not using await.

apps/autonolas-registry/common-util/Details/index.tsx Outdated Show resolved Hide resolved
apps/autonolas-registry/common-util/Details/useDetails.ts Outdated Show resolved Hide resolved
apps/autonolas-registry/common-util/functions/index.ts Outdated Show resolved Hide resolved
apps/autonolas-registry/common-util/functions/requests.ts Outdated Show resolved Hide resolved
const owners = await contract.methods.getOwners().call();

if (Number(threshold) > 0 && owners.length > 0) {
// TODO: check and fix error: Property 'getStorageAt' does not exist on type 'JsonRpcProvider | FallbackProvider'.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember why I wrote this 🙈, we should definitely plan to fix this.

const owners = await contract.methods.getOwners().call();

if (Number(threshold) > 0 && owners.length > 0) {
// TODO: check and fix error: Property 'getStorageAt' does not exist on type 'JsonRpcProvider | FallbackProvider'.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this project uses ethers v6 you need: https://docs.ethers.org/v6/api/providers/#Provider-getStorage

@Tanya-atatakai
Copy link
Collaborator Author

Tanya-atatakai commented Aug 22, 2024

if this project uses ethers v6 you need: https://docs.ethers.org/v6/api/providers/#Provider-getStorage

@truemiller interesting, thanks! will take a look at it

@DavidMinarsch DavidMinarsch merged commit fb02288 into main Aug 27, 2024
11 checks passed
@DavidMinarsch DavidMinarsch deleted the tanya/registry-claim branch August 27, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants