Differential D15483
[Cashtab] Improve alias error handling Authored by emack on Feb 18 2024, 22:23. Tags None Subscribers None
Details
As per feedback in D15466, removing the redundant setIsValidAliasInput(false) calls upon server errors. The remaining instances of setIsValidAliasInput(false) in handleAliasWalletChange, clearInputForms and handleAliasNameInput are untouched as they are genuine alias validation/availability logic.
Diff Detail
Event TimelineComment Actions FYI - using arc diff --nolint for now until I can resolve the arc lint exceptions. My python was already v3.12 and it didn't seem to fix it. Have also tested with versions < 3.12. Comment Actions dunno if it's related to this diff but
  update: I get this in master as well, so seems to be an unrelated issue we should show an 'error retrieiving pricing' error if there is one ... but a pricing error does not imply an error in retrieving existing aliases  in practice this is probably always true now since they come from the same server. but it isn't the way we should do ux. if prices are unavailable, user should see one clear msg that says "Failed to fetch alias price information from server. Alias registration disabled. Refresh page to try again." Something like this. ok to consolidate -- for this diff to be ok, we need the price failure error to appear under the input field. This error trumps all the other possible input validations, since we won't register anything if there is a price failure. The price failure error should not prevent rendering of registered and pending aliases. Those should only not be rendered if there is a server error in fetching them. Comment Actions Replaced setAliasServerError() calls which was the trigger to not render registered and pending alias lists in dropdowns. Now using setAliasValidationError() to hold the error displayed below the input field.
This has now been adjusted to only prevent rendering when aliasValidationError is false and aliasServerError exists, i.e. if and only if server error. Note: when you're re-testing during review, since the price error is simulated by malforming the alias-server url, it will result in no registered/pending aliases being rendered. Comment Actions
This is the kind of thing that should not be too difficult to add an integration test for. Since we have none at all for the Alias component, please add this in this diff using the standardized wrappers. Use jest when to mock an error from the price endpoint and a good return from the registered aliases endpoint(s). Configure.test.js is a good model to follow for integration tests using the standardized wrapper on a specific route + opening collapses, clicking buttons. Comment Actions Added integration tests that validates behavior between aliasValidationError and aliasServerError states.
Comment Actions
This refers to the state var that render errors underneath the alias input field. The earlier feedback was for price errors to be displayed in the same place, as well as to consolidate these vars that disable the register button, hence I decided to use this same var instead of another priceError var. Maybe I can rename it to 'aliasParsingError' to be more generic? Comment Actions It's okay to have two vars if they mean two different things. A validation error is one thing, a bad price response from the server is another thing. If we want both of these cases to show an error in the same place, we handle the logic at that place. For example, the rendering logic of an error in the validation error field can be priceError || validationError. Minimizing state fields is good but there is also a reason to have more than zero, more than one, more than two, etc. We don't want to just make a price error set validationError to true. Maybe this works fine at first, but would be very difficult to debug later if a dev keeps seeing a validation error when there is no validation error and has no idea that 300 diffs back someone made price errors always trigger the validation flag. Comment Actions anyway i don't like how it's currently managed, but this diff still improves things and adds tests. no blocker Comment Actions Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.
Failed tests logs: ====== CashTab Unit Tests: <Home /> Renders Sideshift button if user loads with a new wallet ======
Error: Unable to find role="button" and name `/Exchange to XEC via SideShift/`
Ignored nodes: comments, script, style
<body>
<div>
<div
class="ant-spin-nested-loading css-dev-only-do-not-override-1rqnfsa"
>
<div>
<div
aria-busy="true"
aria-live="polite"
class="ant-spin ant-spin-spinning css-dev-only-do-not-override-1rqnfsa"
>
<span
aria-label="loading"
class="anticon anticon-loading anticon-spin cashLoadingIcon ant-spin-dot"
role="img"
>
<svg
aria-hidden="true"
data-icon="loading"
fill="currentColor"
focusable="false"
height="1em"
viewBox="0 0 1024 1024"
width="1em"
>
<path
d="M988 548c-19.9 0-36-16.1-36-36 0-59.4-11.6-117-34.6-171.3a440.45 440.45 0 00-94.3-139.9 437.71 437.71 0 00-139.9-94.3C629 83.6 571.4 72 512 72c-19.9 0-36-16.1-36-36s16.1-36 36-36c69.1 0 136.2 13.5 199.3 40.3C772.3 66 827 103 874 150c47 47 83.9 101.8 109.7 162.7 26.7 63.1 40.2 130.2 40.2 199.3.1 19.9-16 36-35.9 36z"
/>
</svg>
</span>
</div>
</div>
<div
class="ant-spin-container ant-spin-blur"
>
<div
class="sc-LKuAh jSXNAo"
>
<div
class="sc-kZmsYB eHkWzo"
>
<div
class="sc-RcBXQ gMCskf"
>
<div
class="sc-hzNEM iiFjIO"
>
<div
class="sc-iSDuPN ccpgHi"
>
<img
alt="cashtab"
class="sc-fZwumE bcurWd"
src="test-file-stub"
/>
</div>
<div
class="sc-elJkPf geNavj"
data-testid="wallet-info-ctn"
>
<div
class="sc-dEoRIm iVOOxs"
>
<div
class="sc-jtggT dNidya"
>
Transaction Fixtures
</div>
<a
href="/configure"
>
<svg
class="sc-jTzLTM bTdWCF"
/>
</a>
<div>
<button
aria-checked="true"
class="ant-switch ant-switch-small css-dev-only-do-not-override-1rqnfsa ant-switch-checked"
role="switch"
type="button"
>
<div
class="ant-switch-handle"
/>
<span
class="ant-switch-inner"
>
<span
class="ant-switch-inner-checked"
>
<svg
class="sc-cSHVUG bgwEHu"
/>
</span>
<span
class="ant-switch-inner-unchecked"
>
<svg
class="sc-kAzzGY fUTkYj"
/>
</span>
</span>
</button>
</div>
</div>
<div
class="sc-ebFjAB kxaSkj"
data-testid="balance-xec"
>
0.00
XEC
</div>
<div
class="sc-jKVCRD bKzIGa"
data-testid="balance-fiat"
>
$
0.00
USD
</div>
<p
class="sc-kaNhvL kDOvrh"
data-testid="ecash-price"
>
1
XEC
=
0.00003000
USD
</p>
</div>
</div>
<div
class="sc-jwKygS hNrbtP"
data-testid="loading-ctn"
/>
</div>
<div
class="sc-iBEsjs jmPiFQ"
>
<button
class="sc-gmeYpB dpikyJ"
>
<svg />
</button>
<button
class="sc-gmeYpB eQwTbZ"
data-testid="nav-btn-send"
>
<svg
class="sc-kEYyzF dWIuoY"
style="margin-top: -9px;"
/>
</button>
<button
class="sc-gmeYpB eQwTbZ"
data-testid="nav-btn-etokens"
>
<span
aria-label="appstore-add"
class="anticon anticon-appstore-add"
role="img"
style="font-size: 24px;"
>
<svg
aria-hidden="true"
data-icon="appstore-add"
fill="currentColor"
focusable="false"
height="1em"
viewBox="64 64 896 896"
width="1em"
>
<defs />
<path
d="M464 144H160c-8.8 0-16 7.2-16 16v304c0 8.8 7.2 16 16 16h304c8.8 0 16-7.2 16-16V160c0-8.8-7.2-16-16-16zm-52 268H212V212h200v200zm452-268H560c-8.8 0-16 7.2-16 16v304c0 8.8 7.2 16 16 16h304c8.8 0 16-7.2 16-16V160c0-8.8-7.2-16-16-16zm-52 268H612V212h200v200zm52 132H560c-8.8 0-16 7.2-16 16v304c0 8.8 7.2 16 16 16h304c8.8 0 16-7.2 16-16V560c0-8.8-7.2-16-16-16zm-52 268H612V612h200v200zM424 712H296V584c0-4.4-3.6-8-8-8h-48c-4.4 0-8 3.6-8 8v128H104c-4.4 0-8 3.6-8 8v48c0 4.4 3.6 8 8 8h128v128c0 4.4 3.6 8 8 8h48c4.4 0 8-3.6 8-8V776h128c4.4 0 8-3.6 8-8v-48c0-4.4-3.6-8-8-8z"
/>
</svg>
</span>
</button>
<button
class="sc-gmeYpB eQwTbZ"
data-testid="nav-btn-receive"
>
<svg />
</button>
<div
class="sc-chbbiW ktwTIv"
data-testid="hamburger"
>
<span
class="sc-kxynE bbUulF"
/>
<div
class="sc-cooIXK gkGTON"
data-testid="hamburger-menu"
>
<button
class="sc-fcdeBU RXlWo"
data-testid="nav-btn-airdrop"
>
<p>
Airdrop
</p>
<svg
height="33px"
width="30px"
/>
</button>
<button
class="sc-fcdeBU RXlWo"
data-testid="nav-btn-swap"
>
<p>
Swap
</p>
<span
aria-label="swap"
class="anticon anticon-swap"
role="img"
style="font-size: 24px;"
>
<svg
aria-hidden="true"
data-icon="swap"
fill="currentColor"
focusable="false"
height="1em"
viewBox="64 64 896 896"
width="1em"
>
<path
d="M847.9 592H152c-4.4 0-8 3.6-8 8v60c0 4.4 3.6 8 8 8h605.2L612.9 851c-4.1 5.2-.4 13 6.3 13h72.5c4.9 0 9.5-2.2 12.6-6.1l168.8-214.1c16.5-21 1.6-51.8-25.2-51.8zM872 356H266.8l144.3-183c4.1-5.2.4-13-6.3-13h-72.5c-4.9 0-9.5 2.2-12.6 6.1L150.9 380.2c-16.5 21-1.6 51.8 25.1 51.8h696c4.4 0 8-3.6 8-8v-60c0-4.4-3.6-8-8-8z"
/>
</svg>
</span>
</button>
<button
class="sc-fcdeBU RXlWo"
data-testid="nav-btn-signverifymsg"
>
<p>
Sign & Verify
</p>
<svg
class="sc-chPdSV kiHcnD"
/>
</button>
<button
class="sc-fcdeBU RXlWo"
data-testid="nav-btn-configure"
>
<p>
Settings
</p>
<svg
height="33px"
width="30px"
/>
</button>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</body>
at waitForWrapper (/work/cashtab/node_modules/@testing-library/dom/dist/wait-for.js:163:27)
at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:86:33
at Object.findByRole (/work/cashtab/src/components/Home/__tests__/Home.test.js:128:26)Each failure log is accessible here: | |||||||||||||||||
