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

dapp: fix moved use raiden account setting #2722

Conversation

weilbith
Copy link
Contributor

Follow up/fix of #2708

Short description
Our type checks for this part of the code base are horrible. We can't use refactoring tools and also linting does not find these issues.

Definition of Done

  • Steps to manually test the change have been documented
  • Acceptance criteria are met
  • Change has been manually tested by the reviewer (dApp)

Steps to manually test the change (dApp)

  1. Go to the account page
  2. Make sure there is no error in the console from the Vuex store
  3. Do the same for the UDC screen

@weilbith weilbith requested a review from taleldayekh May 18, 2021 12:52
@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #2722 (88c9c09) into master (9a40721) will decrease coverage by 0.01%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2722      +/-   ##
==========================================
- Coverage   93.12%   93.11%   -0.02%     
==========================================
  Files         184      184              
  Lines        7288     7291       +3     
  Branches     1216     1216              
==========================================
+ Hits         6787     6789       +2     
- Misses        415      416       +1     
  Partials       86       86              
Flag Coverage Δ
dapp 87.63% <85.71%> (+0.01%) ⬆️
dapp.unit 87.63% <85.71%> (+0.01%) ⬆️
sdk 95.71% <ø> (-0.03%) ⬇️
sdk.e2e 75.93% <ø> (-0.15%) ⬇️
sdk.integration 80.40% <ø> (-0.03%) ⬇️
sdk.unit 47.28% <ø> (ø)

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

Impacted Files Coverage Δ
raiden-dapp/src/views/account/UDC.vue 96.15% <75.00%> (+0.32%) ⬆️
...den-dapp/src/components/account/AccountContent.vue 78.57% <100.00%> (+0.79%) ⬆️
raiden-ts/src/transfers/epics/withdraw.ts 92.45% <0.00%> (-0.95%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a40721...88c9c09. Read the comment docs.

@weilbith weilbith self-assigned this May 18, 2021
@weilbith weilbith force-pushed the bugfix/missing-adaptions-for-user-settings-store-module branch from 200c604 to 1da3a2b Compare May 18, 2021 13:22
Copy link
Contributor

@taleldayekh taleldayekh left a comment

Choose a reason for hiding this comment

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

Looks good and not getting any errors in the console. 👍🏽

The root state of the store formerly included the property if the user wants
to use a Raiden account or not. This was moved into the new stores
submodule for the user settings. Unfortunately it was forgotten to
adapt this usage in some components. This commit is fixing that.

This is another indicator that our linting/type safety for pure string
in regards of the Vuex store does not work. We need to find a solution
for that.
@weilbith weilbith force-pushed the bugfix/missing-adaptions-for-user-settings-store-module branch from 1da3a2b to 88c9c09 Compare May 18, 2021 14:39
@weilbith
Copy link
Contributor Author

No CHANGELOG entry necessary for this.

@weilbith weilbith merged commit 4f6361d into raiden-network:master May 18, 2021
@weilbith weilbith deleted the bugfix/missing-adaptions-for-user-settings-store-module branch May 18, 2021 16:12
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.

2 participants