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

Allow claim action for every logged in person #2650

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

alicjakujawa
Copy link
Contributor

Description

This PR updates logic for claim button.

  • Every logged in person can claim tokens

Resolves #2647

@alicjakujawa alicjakujawa requested a review from a team August 17, 2021 14:49
@alicjakujawa alicjakujawa self-assigned this Aug 17, 2021
Copy link
Contributor

@ArmandoGraterol ArmandoGraterol left a comment

Choose a reason for hiding this comment

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

It's working as expected! Good work

Copy link
Member

@rdig rdig left a comment

Choose a reason for hiding this comment

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

Other than change I pointed out, this is good to go

@@ -136,7 +126,7 @@ const UnclaimedTransfersItem = ({
error={ActionTypes.COLONY_CLAIM_TOKEN_ERROR}
success={ActionTypes.COLONY_CLAIM_TOKEN_SUCCESS}
transform={transform}
disabled={!userHasPermission || !isNetworkAllowed}
disabled={!isNetworkAllowed || ethereal}
Copy link
Member

Choose a reason for hiding this comment

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

So this needs to check if the user has a registered profile as well, not just that it's not an ethereal wallet.

Something like this:

  const { walletAddress, username, ethereal } = useLoggedInUser();

  const hasRegisteredProfile = !!username && !ethereal;

This is not necessarily something in the contracts, but it something we don't allow in the dapp, if you don't have your profile registered first.

Copy link
Contributor

@chinins chinins left a comment

Choose a reason for hiding this comment

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

Good job!

@alicjakujawa alicjakujawa force-pushed the bug/issue-2647-everyone-can-claim branch from 82a017d to 3609e79 Compare August 18, 2021 12:54
@alicjakujawa alicjakujawa force-pushed the bug/issue-2647-everyone-can-claim branch from 3609e79 to a7d3cd2 Compare August 18, 2021 12:55
@alicjakujawa alicjakujawa merged commit dea9815 into master Aug 18, 2021
@alicjakujawa alicjakujawa deleted the bug/issue-2647-everyone-can-claim branch August 18, 2021 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Example: Unable to log in with MetaMask wallet
4 participants