convert some handlers to getAPIRouterNoError #4067
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,13 @@ export const notifyUser = (text: string) => { | |
| return apiPost('notify-user', { text }); | ||
| }; | ||
|
|
||
| type TOpenResponse = { | ||
| success: true; | ||
| } | { | ||
| success: false; | ||
| errorMessage?: string; | ||
|
Collaborator
There was a problem hiding this comment. Many types use SuccessResponse and FailResponse, but this has slightly different properties. Instead of "errorMessage" it has just "message". Therer is a comment that this uses export type SuccessResponse = {
success: true;
};
// if the backend uses maybeBB02Err
export type FailResponse = {
code?: number;
message?: string;
success: false;
};https://github.com/BitBoxSwiss/bitbox-wallet-app/blob/master/frontends/web/src/api/response.ts I have no strong opinion on which one we should prefer, but it would be nice if we could just use those everywhere. type TOpenResponse = SuccessResponse | FailResponse;Would it make sense to refactor maybeBB02Err to use errorMessage etc? (also some endpoints return "errMsg")
Contributor
Author
There was a problem hiding this comment. Almost all endpoints outside use errorMessage I think. Makes sense to me to make BitBox02 handlers use the same, but that's for another PR, as it is unrelated to this one.
Collaborator
There was a problem hiding this comment. Ok then we should change FailResponse to use errorMessage in the future and use and extend from SuccessResponse FailResponse everywhere. Should we rename SuccessResponse to SuccessResponseLegacy and already start with the new one? But I'd love if in the end all use the same and we dont end up having a mix again. |
||
| }; | ||
|
|
||
| export const open = (href: string): Promise<TOpenResponse> => { | ||
| return apiPost('open', href); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { ReactNode, SyntheticEvent } from 'react'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import { open } from '@/api/system'; | ||
| import { alertUser } from '@/components/alert/Alert'; | ||
| import { runningInIOS } from '@/utils/env'; | ||
| import style from './anchor.module.css'; | ||
|
|
||
|
|
@@ -32,6 +34,8 @@ export const A = ({ | |
| children, | ||
| ...props | ||
| }: TProps) => { | ||
| const { t } = useTranslation(); | ||
|
|
||
| return ( | ||
| <span | ||
| className={` | ||
|
|
@@ -41,7 +45,15 @@ export const A = ({ | |
| title={props.title || href} | ||
| onClick={(e: SyntheticEvent) => { | ||
| e.preventDefault(); | ||
| open(href) | ||
| .then(response => { | ||
| if (!response.success) { | ||
| alertUser(response.errorMessage | ||
|
Collaborator
There was a problem hiding this comment. At some point we should stop using
Contributor
Author
There was a problem hiding this comment. Can there be a solution that is as easy to use as alertUser? Making ad-hoc ways to display errors everywhere is difficult (for me). Maybe you could make a PR sometime that removes all uses of alertUser and replaces it with a streamlined way of showing such messages?
Collaborator
There was a problem hiding this comment. This is pretty easy have a look: https://github.com/BitBoxSwiss/bitbox-wallet-app/pull/4082/changes. But I agree it would be nice to have a good reusable way. In the future we may want to have a global notification center / error component. 🤷
Contributor
Author
There was a problem hiding this comment. The issue with that one is that it's a lot more effort - need to think where to place the error message, test if it looks good, etc. alertUser "just works" - would be nice to have something that is as easy to use.
Collaborator
There was a problem hiding this comment. it's harder yes but much nicer UI than popup. we also have popup over popup, somewhere in one case 3 popups over each other each with different dimension and visible. I'll find a screenshot once. also from our friend gpt:
Collaborator
There was a problem hiding this comment. Keep as is for this PR, but if you don't mind maybe well change it for smth better and remove alertUser ok? |
||
| ? t('unknownError', { errorMessage: response.errorMessage }) | ||
| : t('genericError')); | ||
| } | ||
| }) | ||
| .catch(console.error); | ||
|
benma marked this conversation as resolved.
|
||
| }} | ||
| tabIndex={0} | ||
| {...props}> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import '../../../__mocks__/i18n'; | ||
| import type { ReactNode } from 'react'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| vi.mock('@/components/layout', () => ({ | ||
| Header: ({ title }: { title: ReactNode }) => <div>{title}</div>, | ||
| })); | ||
| vi.mock('@/components/spinner/Spinner', () => ({ | ||
| Spinner: ({ text }: { text?: string }) => <div>{text}</div>, | ||
| })); | ||
| vi.mock('@/hooks/backbutton', () => ({ | ||
| UseDisableBackButton: () => null, | ||
| })); | ||
| vi.mock('@/hooks/darkmode', () => ({ | ||
| useDarkmode: () => ({ isDarkMode: false, toggleDarkmode: vi.fn() }), | ||
| })); | ||
| vi.mock('@/hooks/vendor-iframe', () => ({ | ||
| useVendorIframeResizeHeight: () => ({ | ||
| containerRef: { current: null }, | ||
| height: 480, | ||
| iframeLoaded: false, | ||
| onIframeLoad: vi.fn(), | ||
| }), | ||
| useVendorTerms: () => ({ | ||
| agreedTerms: true, | ||
| setAgreedTerms: vi.fn(), | ||
| }), | ||
| })); | ||
| vi.mock('./guide', () => ({ | ||
| MarketGuide: () => null, | ||
| })); | ||
| vi.mock('@/api/market', async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import('@/api/market')>(); | ||
| return { | ||
| ...actual, | ||
| getMoonpayBuyInfo: vi.fn(), | ||
| }; | ||
| }); | ||
| vi.mock('@/utils/config', async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import('@/utils/config')>(); | ||
| return { | ||
| ...actual, | ||
| getConfig: vi.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| import { render, screen } from '@testing-library/react'; | ||
| import type { TAccount } from '@/api/account'; | ||
| import * as marketApi from '@/api/market'; | ||
| import * as config from '@/utils/config'; | ||
| import { Moonpay } from './moonpay'; | ||
|
|
||
| const account: TAccount = { | ||
| keystore: { | ||
| connected: true, | ||
| lastConnected: '', | ||
| name: 'BitBox02', | ||
| rootFingerprint: 'f23ab988', | ||
| watchonly: false, | ||
| }, | ||
| active: true, | ||
| blockExplorerTxPrefix: '', | ||
| code: 'btc-account', | ||
| coinCode: 'btc', | ||
| coinName: 'Bitcoin', | ||
| coinUnit: 'BTC', | ||
| isToken: false, | ||
| name: 'Bitcoin Account', | ||
| }; | ||
|
|
||
| describe('routes/market/moonpay', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.mocked(config.getConfig).mockResolvedValue({ | ||
| frontend: { | ||
| skipMoonpayDisclaimer: true, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it('renders the MoonPay iframe on success', async () => { | ||
| vi.mocked(marketApi.getMoonpayBuyInfo).mockReturnValue(() => Promise.resolve({ | ||
| success: true, | ||
| url: 'https://buy.moonpay.com?walletAddress=bc1qexample', | ||
| address: 'bc1qexample', | ||
| })); | ||
|
|
||
| render(<Moonpay accounts={[account]} code={account.code} />); | ||
|
|
||
| const iframe = await screen.findByTitle('Moonpay'); | ||
| expect(iframe).toHaveAttribute( | ||
| 'src', | ||
| 'https://buy.moonpay.com?walletAddress=bc1qexample&colorCode=%235E94BF&theme=light', | ||
| ); | ||
| }); | ||
|
|
||
| it('renders an error message on failure', async () => { | ||
| vi.mocked(marketApi.getMoonpayBuyInfo).mockReturnValue(() => Promise.resolve({ | ||
| success: false, | ||
| errorMessage: 'Account is not valid.', | ||
| })); | ||
|
|
||
| render(<Moonpay accounts={[account]} code={account.code} />); | ||
|
|
||
| expect(await screen.findByText('Account is not valid.')).toBeInTheDocument(); | ||
| expect(screen.queryByTitle('Moonpay')).not.toBeInTheDocument(); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.