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

Update state of wallets after changing network #316

Merged
merged 1 commit into from
Oct 18, 2021
Merged

Conversation

lukaw3d
Copy link
Member

@lukaw3d lukaw3d commented Oct 14, 2021

Fixes #270
Fixes #305

@github-actions
Copy link

github-actions bot commented Oct 14, 2021

Mega-Linter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ EDITORCONFIG editorconfig-checker 3 0 1.32s
✅ GIT git_diff yes no 0.01s
✅ TSX eslint 1 0 0 6.06s
✅ TYPESCRIPT eslint 2 0 0 6.14s

See errors details in artifact Mega-Linter reports on GitHub Action page
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

Comment on lines +102 to +107
// Reload wallet balances if network changes
useEffect(() => {
for (const wallet of Object.values(wallets)) {
dispatch(walletActions.fetchWallet(wallet))
}
}, [dispatch, wallets, selectedNetwork])
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I'm not sure what is the best practice for combining different sagas (network => wallet), so this just follows accountActions.fetchAccount implementation

@tjanez
Copy link
Member

tjanez commented Oct 15, 2021

Hmm... I tried to test this locally and got:

Http response at 400 or 500 level

The above error occurred in task lt
    created by takeEvery(network/selectNetwork, lt)
    created by dt
Tasks cancelled due to error:
takeEvery(network/selectNetwork, lt)

Copy link
Member

@tjanez tjanez left a comment

Choose a reason for hiding this comment

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

The commit message should probably be:
Update wallet's state after changing network

@lukaw3d lukaw3d changed the title Update wallets state after changing network Update state of wallets after changing network Oct 15, 2021
@codecov
Copy link

codecov bot commented Oct 15, 2021

Codecov Report

Merging #316 (b079e99) into develop (db0b2ac) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #316      +/-   ##
===========================================
+ Coverage    90.82%   90.87%   +0.05%     
===========================================
  Files           96       96              
  Lines         1548     1557       +9     
  Branches       269      269              
===========================================
+ Hits          1406     1415       +9     
  Misses         142      142              
Flag Coverage Δ
cypress 64.65% <100.00%> (+0.21%) ⬆️
jest 74.22% <54.54%> (-0.18%) ⬇️

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

Impacted Files Coverage Δ
src/app/pages/AccountPage/index.tsx 100.00% <100.00%> (ø)
src/app/state/wallet/index.ts 76.92% <100.00%> (+0.92%) ⬆️
src/app/state/wallet/saga.ts 98.64% <100.00%> (+0.07%) ⬆️

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 db0b2ac...b079e99. Read the comment docs.

@lukaw3d
Copy link
Member Author

lukaw3d commented Oct 15, 2021

@tjanez Hmm... I tried to test this locally and got: 400 or 500 error

Did you docker-compose up --build?

I just used REACT_APP_BYPASS_LOCAL=1 yarn run start without local network

@tjanez
Copy link
Member

tjanez commented Oct 15, 2021

@tjanez Hmm... I tried to test this locally and got: 400 or 500 error

Did you docker-compose up --build?

No.

I just used REACT_APP_BYPASS_LOCAL=1 yarn run start without local network

I used:

yarn build
yarn add serve
./node_modules/.bin/serve -s build/

@lukaw3d
Copy link
Member Author

lukaw3d commented Oct 15, 2021

ah, so equivalent of yarn start:prod. I'll try

@lukaw3d
Copy link
Member Author

lukaw3d commented Oct 15, 2021

https://testnet.grpc.oasis.dev/oasis-core.Consensus/GetGenesisDocument is returning 503 with headers

grpc-message: upstream connect error or disconnect/reset before headers. reset reason: connection failure
grpc-status: 14

@tjanez
Copy link
Member

tjanez commented Oct 18, 2021

https://testnet.grpc.oasis.dev/oasis-core.Consensus/GetGenesisDocument is returning 503 with headers

grpc-message: upstream connect error or disconnect/reset before headers. reset reason: connection failure
grpc-status: 14

Yeah, that was the issue. It works now.

@lukaw3d lukaw3d merged commit e4ca3da into develop Oct 18, 2021
@lukaw3d lukaw3d deleted the lw/fix-balance branch October 18, 2021 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants