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

feat: add graceful support for max Celo votes #856

Merged

Conversation

andyhass
Copy link
Contributor

@andyhass andyhass commented Aug 5, 2022

📝 Description

There are a maximum number of Celo validator groups that can be voted for. Previously, this error was not handled that well. The user would be able to select the option to vote, but would then see this error message when choosing an amount to vote:

Screen Shot 2022-08-04 at 12 28 33 PM

With this change, we indicate that the user cannot vote in staking management dashboard and prevent them from getting to the voting flow:

Screen Shot 2022-08-05 at 10 33 50 AM

❓ Context

  • Impacted projects: LLD, LLC
  • Linked resource(s): ``

✅ Checklist

  • Test coverage
  • Atomic delivery
  • No breaking changes

📸 Demo

See above

🚀 Expectations to reach

Please make sure you follow these Important Steps.

Pull Requests must pass the CI and be internally validated in order to be merged.

@changeset-bot
Copy link

changeset-bot bot commented Aug 5, 2022

🦋 Changeset detected

Latest commit: 99d64e2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
ledger-live-desktop Minor
@ledgerhq/live-common Minor
@ledgerhq/live-cli Patch
live-mobile Patch
live-common-tools Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Aug 5, 2022

@andyhass is attempting to deploy a commit to the LedgerHQ Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Aug 5, 2022

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

Name Status Preview Updated
live-common-tools ✅ Ready (Inspect) Visit Preview Aug 10, 2022 at 8:13PM (UTC)
2 Ignored Deployments
Name Status Preview Updated
native-ui-storybook ⬜️ Ignored (Inspect) Aug 10, 2022 at 8:13PM (UTC)
react-ui-storybook ⬜️ Ignored (Inspect) Aug 10, 2022 at 8:13PM (UTC)

@github-actions github-actions bot added common Has changes in live-common desktop Has changes in LLD translations Translation files have been touched fork Pull request base branch comes from a fork. labels Aug 5, 2022
@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #856 (99d64e2) into feat/celo (88f47e0) will decrease coverage by 0.21%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##           feat/celo     #856      +/-   ##
=============================================
- Coverage      47.32%   47.11%   -0.22%     
=============================================
  Files            618      626       +8     
  Lines          27617    28263     +646     
  Branches        6949     7323     +374     
=============================================
+ Hits           13071    13315     +244     
+ Misses         14486    13799     -687     
- Partials          60     1149    +1089     
Flag Coverage Δ
test 47.11% <ø> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
libs/ledger-live-common/src/transaction/common.ts 46.93% <0.00%> (-29.54%) ⬇️
libs/ledger-live-common/src/platform/converters.ts 35.71% <0.00%> (-25.83%) ⬇️
libs/ledger-live-common/src/transaction/index.ts 40.00% <0.00%> (-17.15%) ⬇️
...er-live-common/src/families/bitcoin/transaction.ts 32.85% <0.00%> (-13.30%) ⬇️
libs/ledger-live-common/src/reconciliation.ts 32.50% <0.00%> (-9.69%) ⬇️
...-common/src/families/stellar/js-synchronization.ts 26.66% <0.00%> (-8.63%) ⬇️
...bs/ledger-live-common/src/account/serialization.ts 54.18% <0.00%> (-6.09%) ⬇️
...ve-common/src/families/stellar/js-signOperation.ts 22.91% <0.00%> (-4.87%) ⬇️
...ger-live-common/src/families/celo/serialization.ts 23.07% <0.00%> (-4.20%) ⬇️
libs/ledger-live-common/src/families/celo/logic.ts 34.48% <0.00%> (-3.76%) ⬇️
... and 353 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ae-ledger
Copy link
Member

Hey @andyhass,
Thank you for your contribution. Some things to add:

  • Could you add changeset (pnpm changeset)?
  • Have a look on the conflicting files
    Best regards.

@andyhass
Copy link
Contributor Author

Hey @andyhass, Thank you for your contribution. Some things to add:

  • Could you add changeset (pnpm changeset)?
  • Have a look on the conflicting files
    Best regards.

Hey @aelmilligy-ledger - thanks for reviewing. I've added the changelog and fixed up the merge conflicts as requested 😄

@@ -45,9 +46,10 @@ const ManageModal = ({ name, account, ...rest }: Props) => {
},
[dispatch, account],
);

const groupsVotedFor = [...new Set(celoResources.votes.map(v => v.validatorGroup))];
Copy link

Choose a reason for hiding this comment

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

Maybe useMemo would be useful here?...

@ghost ghost merged commit 99988f3 into LedgerHQ:feat/celo Aug 19, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Has changes in live-common desktop Has changes in LLD fork Pull request base branch comes from a fork. translations Translation files have been touched
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants