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

Fix: Updated logic for funds links #3617

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

Julianahlk
Copy link
Contributor

@Julianahlk Julianahlk commented Jul 12, 2022

Description

This PR fixes the "Available Funds" link that should take the user to the funds page and be enabled regardless of permissions but it is currently inactive.

Additionally, once you go inside the funds page, the Move funds, Mint Tokens, Manage Tokens links should all be active depending on the following:

  1. If Governance Extension is enabled:
  • Move Funds - visible
  • Mint Tokens - visible
  • Manage Tokens - visible
  1. If Governance Extension is NOT enabled:
  • Move Funds - needs the Funding role
  • Mint Tokens - needs the Root role
  • Manage Tokens - needs the Root role

Changes 🏗

  • Removed permissions requirement for available funds link.

Screen Shot 2022-07-12 at 11 42 40 PM

  • Made the Move funds, Mint Tokens, Manage Tokens links enabled when the governance extension is active.

Screen Shot 2022-07-12 at 11 39 26 PM

TODO

  • Activate the "Available Funds" link.
  • Make the Move funds, Mint Tokens, Manage Tokens links enabled when the governance extension is active.

Resolves #3550

@Julianahlk Julianahlk marked this pull request as ready for review July 12, 2022 20:49
@Julianahlk Julianahlk requested a review from a team July 12, 2022 20:49
Copy link
Collaborator

@willm30 willm30 left a comment

Choose a reason for hiding this comment

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

Part 2 looks good. Part 1, I the 'available funds' link is still disabled if you're not a member of the colony. I have commented where I believe the relevant logic is.

const allUserRoles = useTransformer(getAllUserRoles, [colony, walletAddress]);

const canMoveFunds = !!username && !ethereal && canFund(allUserRoles);
const canMoveFunds = !!username && !ethereal;

const {
colonyAddress,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line 60 below canMoveFunds ? ... is the logic that needs to be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! Thanks Will.

rdig
rdig previously requested changes Jul 13, 2022
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.

Agree with Will, the link disabling / enabling works fine, however the link to the Manage Funds page does not.

Here's with a logged out user (it should work for that case as well)
Screenshot from 2022-07-13 21-44-39

Note that the incoming funds section works correctly, and even point to the same location. So you can just copy/paste it's logic

While after being logged in, it works correctly

Screenshot from 2022-07-13 21-45-00

@Julianahlk
Copy link
Contributor Author

Julianahlk commented Jul 13, 2022

Agree with Will, the link disabling / enabling works fine, however the like to the Manage Funds page does not.

It should be working now. Thank you!

Copy link
Collaborator

@willm30 willm30 left a comment

Choose a reason for hiding this comment

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

Looking good!

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 to go!

@rdig rdig dismissed their stale review July 14, 2022 18:33

Stale

@Julianahlk Julianahlk force-pushed the fix/3550-inactive-links-on-funds-page branch from d84f995 to e57ac6e Compare July 14, 2022 18:45
@Julianahlk Julianahlk merged commit a240417 into master Jul 14, 2022
@Julianahlk Julianahlk deleted the fix/3550-inactive-links-on-funds-page branch July 14, 2022 19:54
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.

Bug: Links are Inactive on Funds Page
4 participants