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.

Sign up for GitHub

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

Jump to bottom

[DDW-579] Undelegated wallet shown as delegated on "Delegation center" screen #2404

Merged
merged 5 commits into from
Feb 27, 2021
Merged

[DDW-579] Undelegated wallet shown as delegated on "Delegation center" screen #2404

merged 5 commits into from
Feb 27, 2021

Conversation

topseniors
Copy link
Contributor

@topseniors topseniors commented Feb 25, 2021 •
edited by miorsufianiohk
Loading

This PR fixed delegated stake pool status on Delegation Center screen
Jira Ticket

Todos

  • Fix delegated stake pool status on Delegation Center screen

Testing Checklist

Test Cases

Scenario 1: Wallet undelegated on Yoroi shown as delegated on Daedalus
Given I have undelegated a wallet on Yoroi
When I go to Daedalus Delegation Center
Then this wallet will be shown as undelegated

Scenario: Wallet undelegated

App

  • Go to Staking => Delegation Center and make sure delegated stake pool statuses of wallets are properly shown up

Screenshots

English

image

Japanese

image

Review Checklist

Basics

  • PR has been assigned and has appropriate labels (feature/bug/chore, release-x.x.x)
  • PR is updated to the most recent version of the target branch (and there are no conflicts)
  • PR has a good description that summarizes all changes and shows some screenshots or animated GIFs of important UI changes
  • CHANGELOG entry has been added to the top of the appropriate section (Features, Fixes, Chores) and is linked to the correct PR on GitHub
  • Automated tests: All acceptance and unit tests are passing (yarn test)
  • Manual tests (minimum tests should cover newly added feature/fix): App works correctly in development build (yarn dev)
  • Manual tests (minimum tests should cover newly added feature/fix): App works correctly in production build (yarn package / CI builds)
  • There are no flow errors or warnings (yarn flow:test)
  • There are no lint errors or warnings (yarn lint)
  • There are no prettier errors or warnings (yarn prettier:check)
  • There are no missing translations (running yarn manage:translations produces no changes)
  • Text changes are proofread and approved (Jane Wild / Amy Reeve)
  • Japanese text changes are proofread and approved (Junko Oda)
  • UI changes look good in all themes (Alexander Rukin)
  • Storybook works and no stories are broken (yarn storybook)
  • In case of dependency changes yarn.lock file is updated

Code Quality

  • Important parts of the code are properly commented and documented
  • Code is properly typed with flow
  • React components are split-up enough to avoid unnecessary re-renderings
  • Any code that only works in main process is neatly separated from components

Testing

  • New feature/change is covered by acceptance tests
  • New feature/change is manually tested and approved by QA team
  • All existing acceptance tests are still up-to-date
  • New feature/change is covered by Daedalus Testing scenario
  • All existing Daedalus Testing scenarios are still up-to-date

After Review

  • Merge the PR
  • Delete the source branch
  • Move the ticket to done column on the YouTrack board
  • Update Slack QA thread by marking it with a green checkmark

@topseniors topseniors self-assigned this Feb 25, 2021
@nikolaglumac
Copy link
Contributor

@yakovkaravelov please merge latest develop.

tomislavhoracek
tomislavhoracek suggested changes Feb 26, 2021
View reviewed changes
Copy link
Contributor

@tomislavhoracek tomislavhoracek left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@yakovkaravelov Nice job.
I left just one comment.
P.S. Why this is changed if you didn't touch translations? Can you check this please
source/renderer/app/i18n/locales/defaultMessages.json
And also merge latest develop branch and fix conflicts

source/renderer/app/components/staking/delegation-center/WalletRow.js Outdated Show resolved Hide resolved
@topseniors
Copy link
Contributor Author

@yakovkaravelov Nice job.
I left just one comment.
P.S. Why this is changed if you didn't touch translations? Can you check this please
source/renderer/app/i18n/locales/defaultMessages.json
And also merge latest develop branch and fix conflicts

@tomislavhoracek defaultMessage.json can be changed when there is line number change regarding translation definitions even if there is no change in translation resources.
It happens after running yarn manage:translations

@topseniors topseniors removed the WIP label Feb 26, 2021
@miorsufianiohk
Copy link

@yakovkaravelov Unable to test as not able to undelegate on Yoroi due to transaction issue on testnet. See screenshot

tomislavhoracek
tomislavhoracek approved these changes Feb 26, 2021
View reviewed changes
Copy link
Contributor

@tomislavhoracek tomislavhoracek left a comment

Choose a reason for hiding this comment

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

@yakovkaravelov code looks good now 👍

@darko-mijic
Copy link
Contributor

Reviewing this now with Trezor HW wallet on the mainnet...

@darko-mijic
Copy link
Contributor

image
Works great. IOHK Trezor 2 is now undelegated.

@darko-mijic darko-mijic self-requested a review February 26, 2021 14:55
gabriela-ponce
gabriela-ponce approved these changes Feb 26, 2021
View reviewed changes
Copy link

@gabriela-ponce gabriela-ponce left a comment

Choose a reason for hiding this comment

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

I undelegated in Yoroi and the changes are visible in Daedalus for Ledger S and software wallets.

miorsufianiohk
miorsufianiohk approved these changes Feb 26, 2021
View reviewed changes
Copy link

@miorsufianiohk miorsufianiohk left a comment

Choose a reason for hiding this comment

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

Tested with software wallet on Yoroi testnet and Daedalus testnet. Good work @yakovkaravelov 👍

@nikolaglumac nikolaglumac merged commit 64fc98e into develop Feb 27, 2021
@iohk-bors iohk-bors bot deleted the chore/ddw-579-undelegated-wallet-shown-as-delegated-on-delegation-center-screen branch February 27, 2021 13:25
@nikolaglumac nikolaglumac added release-4.0.0-RC1 Daedalus Testnet release-4.0.0-FC1 Daedalus Flight release-4.0.0 Daedalus Mainnet and removed ⏳release-vNext labels Feb 27, 2021
@nikolaglumac nikolaglumac mentioned this pull request Mar 30, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miorsufianiohk miorsufianiohk miorsufianiohk approved these changes

@tomislavhoracek tomislavhoracek tomislavhoracek approved these changes

@gabriela-ponce gabriela-ponce gabriela-ponce approved these changes

@darko-mijic darko-mijic darko-mijic approved these changes

@nikolaglumac nikolaglumac nikolaglumac approved these changes

@thedanheller thedanheller Awaiting requested review from thedanheller

@ManusMcCole ManusMcCole Awaiting requested review from ManusMcCole

Assignees

@topseniors topseniors

Labels
chore release-4.0.0-FC1 Daedalus Flight release-4.0.0-RC1 Daedalus Testnet release-4.0.0 Daedalus Mainnet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@topseniors @nikolaglumac @miorsufianiohk @darko-mijic @gabriela-ponce @tomislavhoracek