vegaprotocol / vegawallet-browser

A wallet for the Vega protocol implemented as a browser extension for Firefox and Chromium-based browsers.
https://vegaprotocol.github.io/vegawallet-browser/
MIT License
4 stars 1 forks source link

Feat/update assets based on feedback #605

Closed dexturr closed 1 year ago

dexturr commented 1 year ago

Closes #562 Closes #580 Closes https://github.com/vegaprotocol/vegawallet-browser/issues/555 Closes https://github.com/vegaprotocol/vegawallet-browser/issues/425

And address error cases for when there is an error fetching data from a data node.

github-actions[bot] commented 1 year ago

Here is your approbation coverage BEFORE this PR:

*Criteria*: 206  *Covered*: 200  *by/FeatTest*: -  *by/SysTest*: -  *Uncovered*: 6  *Coverage*: 97.1%

Here is your approbation coverage AFTER this PR:

*Criteria*: 206  *Covered*: 200  *by/FeatTest*: -  *by/SysTest*: -  *Uncovered*: 6  *Coverage*: 97.1%

AC Diff Report:

There are no changes to AC coverage %, please note any new coverage is ignored by this report IF it retains 100% coverage for the given file.

dalebennett1992 commented 1 year ago

so to my eyes, I do not see how this AC is addressed As a user I want to see a reasonable empty state So that I am informed about how to deposit assets to vega and which markets I can trade in

Here is my view with no balance image

here is the view when expanded image

if we are aiming to meet the above AC I am not sure any of the changes do this, I am specifically referring to the part of the AC referencing being informed on how to deposit assets to vega. Of course I could be missing something

dalebennett1992 commented 1 year ago

Regarding the error handling and getting this view image

The above works if these details have been retrieved once. So if I load successfully, then kill the mock server and open key details for a second time I will see the above. However, this still bombs out if the assets endpoint does not respond in the first instance. image

I managed to get that by commenting out my /assets response so the info could not be retrieved on the initial load. Should we address this?

dalebennett1992 commented 1 year ago

When I send a transaction with a node completely down (no mock running) I get this error image

Here is the error message "Error: Asset with id fc7fd956078fb1fc9db5c19b88f0874c4299b2a7639ad05a47a28c0aef291b55 not found\n at getAssetById (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:112044:20)\n at useFormatAssetAmount (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:112441:24)\n at EnrichedTransferView (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:112455:42)\n at renderWithHooks (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:21100:20)\n at mountIndeterminateComponent (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:24864:15)\n at beginWork (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:26377:18)\n at beginWork$1 (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:32201:16)\n at performUnitOfWork (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:31335:14)\n at workLoopSync (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:31241:7)\n at renderRootSync (chrome-extension://jfaancmgehieoohdnmcdfdlkblfcehph/popup.js:31209:9)"

asset id should be irrelevant given the node is completely dead, but for what it's worth here is the transfer obj

export const dummyTransaction = {
  fromAccountType: 4,
  toAccountType: 4,
  amount: '1',
  asset: 'fc7fd956078fb1fc9db5c19b88f0874c4299b2a7639ad05a47a28c0aef291b55',
  to: 'fc7fd956078fb1fc9db5c19b88f0874c4299b2a7639ad05a47a28c0aef291b55',
  kind: {
    oneOff: {}
  }
}

Here is the test code (it fails at the point of the static wait), I think this is a bug.


    closeServer = false
    await navigateToExtensionLandingPage(driver)
    await viewWallet.openKeyDetails('Key 1')
    const errorText = await keyDetails.getNotificationText()
    expect(errorText).toBe('An error occurred when loading account information: Failed to fetch data')
    await vegaAPI.connectWalletAndCheckSuccess()
    await connectWallet.checkOnConnectWallet()
    await connectWallet.approveConnectionAndCheckSuccess()
    const keys = await vegaAPI.listKeys()
    await vegaAPI.sendTransaction(keys[0].publicKey, { transfer: dummyTransaction })
    await staticWait(30000)
    await transaction.checkOnTransactionPage()
  })```
dalebennett1992 commented 1 year ago

all tests that should pass now pass, the failing tests is due to a legit bug. If we get that final test passing then this can go down. Over to you @dexturr ;)

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

100.0% 100.0% Coverage
0.5% 0.5% Duplication