Skip to content

Commit

Permalink
Merge pull request #746 from oasisprotocol/csillag/clean-up-error-paths
Browse files Browse the repository at this point in the history
Clean up pagination-related error pages everywhere
  • Loading branch information
csillag authored Jul 18, 2023
2 parents 51b6ed3 + 0cb1c86 commit 68ad4c2
Show file tree
Hide file tree
Showing 21 changed files with 279 additions and 148 deletions.
1 change: 1 addition & 0 deletions .changelog/746.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Un-break lots of pagination-related error pages
2 changes: 1 addition & 1 deletion src/app/components/EmptyState/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const EmptyState: FC<EmptyStateProps> = ({ description, title, light }) =
)
return light ? (
<StyledBoxLight>
{<CancelIcon color="error" fontSize="large" />}
<CancelIcon color="error" fontSize="large" />
{content}
</StyledBoxLight>
) : (
Expand Down
29 changes: 25 additions & 4 deletions src/app/components/ErrorDisplay/index.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { FC } from 'react'
import { useTranslation } from 'react-i18next'
import { FC, ReactNode } from 'react'
import { Trans, useTranslation } from 'react-i18next'
import { isRouteErrorResponse } from 'react-router-dom'
import { EmptyState } from '../EmptyState'
import { AppError, AppErrors, ErrorPayload } from '../../../types/errors'
import { TFunction } from 'i18next'
import { GoToFirstPageLink } from '../Table/GoToFirstPageLink'

type FormattedError = { title: string; message: string }
type FormattedError = { title: string; message: ReactNode }

const errorMap: Record<AppErrors, (t: TFunction, error: ErrorPayload) => FormattedError> = {
[AppErrors.Unknown]: (t, error) => ({ title: t('errors.unknown'), message: error.message }),
Expand All @@ -17,7 +18,27 @@ const errorMap: Record<AppErrors, (t: TFunction, error: ErrorPayload) => Formatt
[AppErrors.InvalidTxHash]: t => ({ title: t('errors.invalidTxHash'), message: t('errors.validateURL') }),
[AppErrors.InvalidPageNumber]: t => ({
title: t('errors.invalidPageNumber'),
message: t('errors.validateURLOrGoToFirstTab'),
message: (
<Trans
t={t}
i18nKey="errors.validateURLOrGoToFirstPage"
components={{
FirstPageLink: <GoToFirstPageLink />,
}}
/>
),
}),
[AppErrors.PageDoesNotExist]: t => ({
title: t('errors.pageDoesNotExist'),
message: (
<Trans
t={t}
i18nKey="errors.validateURLOrGoToFirstPage"
components={{
FirstPageLink: <GoToFirstPageLink />,
}}
/>
),
}),
[AppErrors.NotFoundBlockHeight]: t => ({
title: t('errors.notFoundBlockHeight'),
Expand Down
46 changes: 46 additions & 0 deletions src/app/components/Table/GoToFirstPageLink.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { FC } from 'react'
import { useTranslation } from 'react-i18next'
import { useSearchParams, useHref } from 'react-router-dom'
import Link from '@mui/material/Link'

/**
* Link to attempt to jump to the first page (in pagination)
*
* This is only a best-effort attempts, because we can't really know the name of the 'page' parameter
* used for pagination. The best we can do is to test the trivial option ("page"), but if it's not there,
* then we can't really do anything.
*
* Please note that to get to an invalid / non-existent page, the user probably had to do some manual URL hacking,
* so hopefully it's not terrible that he gets a full reload as the result of that.
*/
export const GoToFirstPageLink: FC = () => {
const { t } = useTranslation()
const [searchParams] = useSearchParams()
const wantedParamName = searchParams.has('page') ? 'page' : undefined
const label = t('errors.loadFirstPage')
const newSearchParams = new URLSearchParams(searchParams)
if (wantedParamName) {
newSearchParams.delete(wantedParamName)
}
const href = useHref({
search: newSearchParams.toString(),
})
// Ideally, we should use a router link here, but even if we change the URL without a reload,
// I can't get the error boundary to recover from the error state.
// when jumping to the first page, the ErrorBoundary component stays in place, only the internals change.
// But to show the first page, the Error Boundary component would need to switch back from the "error" state
// to the "normal" state, which doesn't happen automatically.
// In needs a manual `setState()` for recovering, which is hard to channel through,
// but even if we do it (which I did as an experiment), we find that
// the data around the ErrorBoundary is somewhat weird, and the updated URL is not reflected right away,
// so we just end up running into the same exception again...
// Apparently the solution is to umount and rerender the component outside the error boundary.
// But that's hard to do selectively, so it's easier to just do a full refresh.
return wantedParamName ? (
<Link href={href} sx={{ cursor: 'pointer' }}>
{label}
</Link>
) : (
<span>{label}</span>
)
}
1 change: 1 addition & 0 deletions src/app/components/Table/PaginationEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface PaginatedResults<Item> {
}

export interface ComprehensivePaginationEngine<Item, QueryResult extends List> {
selectedPage: number
offsetForQuery: number
limitForQuery: number
paramsForQuery: { offset: number; limit: number }
Expand Down
1 change: 1 addition & 0 deletions src/app/components/Table/useClientSidePagination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export function useClientSizePagination<Item, QueryResult extends List>({
}

return {
selectedPage: selectedClientPage,
offsetForQuery: offset,
limitForQuery: limit,
paramsForQuery,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export function useComprehensiveSearchParamsPagination<Item, QueryResult extends
}

return {
selectedPage,
offsetForQuery: offset,
limitForQuery: limit,
paramsForQuery,
Expand Down
39 changes: 24 additions & 15 deletions src/app/pages/AccountDetailsPage/AccountTokenTransfersCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,39 @@ export const accountTokenTransfersContainerId = 'transfers'

export const AccountTokenTransfersCard: FC<AccountDetailsContext> = ({ scope, address }) => {
const { t } = useTranslation()

const { isLoading, isFetched, results } = useTokenTransfers(scope, address)

const { account } = useAccount(scope, address)
const transfers = results.data

return (
<Card>
<LinkableDiv id={accountTokenTransfersContainerId}>
<CardHeader disableTypography component="h3" title={t('tokens.transfers')} />
</LinkableDiv>
<CardContent>
<ErrorBoundary light={true}>
{isFetched && !transfers?.length && <CardEmptyState label={t('account.emptyTokenTransferList')} />}
<TokenTransfers
transfers={transfers}
ownAddress={account?.address_eth}
isLoading={isLoading}
limit={NUMBER_OF_ITEMS_ON_SEPARATE_PAGE}
pagination={results.tablePaginationProps}
differentTokens
/>
<AccountTokenTransfers scope={scope} address={address} />
</ErrorBoundary>
</CardContent>
</Card>
)
}

const AccountTokenTransfers: FC<AccountDetailsContext> = ({ scope, address }) => {
const { t } = useTranslation()

const { isLoading, isFetched, results } = useTokenTransfers(scope, address)

const { account } = useAccount(scope, address)
const transfers = results.data

return (
<>
{isFetched && !transfers?.length && <CardEmptyState label={t('account.emptyTokenTransferList')} />}
<TokenTransfers
transfers={transfers}
ownAddress={account?.address_eth}
isLoading={isLoading}
limit={NUMBER_OF_ITEMS_ON_SEPARATE_PAGE}
pagination={results.tablePaginationProps}
differentTokens
/>
</>
)
}
42 changes: 26 additions & 16 deletions src/app/pages/AccountDetailsPage/AccountTransactionsCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,41 @@ export const accountTransactionsContainerId = 'transactions'
export const AccountTransactionsCard: FC<AccountDetailsContext> = ({ scope, address }) => {
const { t } = useTranslation()

const { isLoading, isFetched, transactions, pagination, totalCount, isTotalCountClipped } =
useAccountTransactions(scope, address)
return (
<Card>
<LinkableDiv id={accountTransactionsContainerId}>
<CardHeader disableTypography component="h3" title={t('account.transactionsListTitle')} />
</LinkableDiv>
<CardContent>
<ErrorBoundary light={true}>
{isFetched && !transactions?.length && <CardEmptyState label={t('account.emptyTransactionList')} />}
<Transactions
transactions={transactions}
ownAddress={address}
isLoading={isLoading}
limit={NUMBER_OF_ITEMS_ON_SEPARATE_PAGE}
pagination={{
selectedPage: pagination.selectedPage,
linkToPage: pagination.linkToPage,
totalCount,
isTotalCountClipped,
rowsPerPage: NUMBER_OF_ITEMS_ON_SEPARATE_PAGE,
}}
/>
<AccountTransactions scope={scope} address={address} />
</ErrorBoundary>
</CardContent>
</Card>
)
}

const AccountTransactions: FC<AccountDetailsContext> = ({ scope, address }) => {
const { t } = useTranslation()

const { isLoading, isFetched, transactions, pagination, totalCount, isTotalCountClipped } =
useAccountTransactions(scope, address)
return (
<>
{isFetched && !transactions?.length && <CardEmptyState label={t('account.emptyTransactionList')} />}
<Transactions
transactions={transactions}
ownAddress={address}
isLoading={isLoading}
limit={NUMBER_OF_ITEMS_ON_SEPARATE_PAGE}
pagination={{
selectedPage: pagination.selectedPage,
linkToPage: pagination.linkToPage,
totalCount,
isTotalCountClipped,
rowsPerPage: NUMBER_OF_ITEMS_ON_SEPARATE_PAGE,
}}
/>
</>
)
}
6 changes: 5 additions & 1 deletion src/app/pages/AccountDetailsPage/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,12 @@ export const useAccountTransactions = (scope: SearchScope, address: string) => {
},
)
const { isFetched, isLoading, data } = query

const transactions = data?.data.transactions

if (isFetched && pagination.selectedPage > 1 && !transactions?.length) {
throw AppErrors.PageDoesNotExist
}

const totalCount = data?.data.total_count
const isTotalCountClipped = data?.data.is_total_count_clipped

Expand Down
47 changes: 18 additions & 29 deletions src/app/pages/AccountDetailsPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ import { EvmToken, EvmTokenType, RuntimeAccount } from '../../../oasis-nexus/api
import { accountTokenContainerId } from './AccountTokensCard'
import { useAccount } from './hook'
import { useRequiredScopeParam } from '../../hooks/useScopeParam'
import { showEmptyAccountDetails } from '../../../config'
import { CardEmptyState } from './CardEmptyState'
import { contractCodeContainerId } from './ContractCodeCard'
import { useTokenInfo, useTokenTransfers } from '../TokenDashboardPage/hook'
import { useTokenInfo } from '../TokenDashboardPage/hook'
import { accountTokenTransfersContainerId } from './AccountTokenTransfersCard'
import { getTokenTypePluralName } from '../../../types/tokens'
import { SearchScope } from '../../../types/searchScope'
Expand All @@ -35,23 +34,17 @@ export const AccountDetailsPage: FC = () => {
const { account, isLoading: isAccountLoading, isError } = useAccount(scope, address)
const isContract = !!account?.evm_contract
const { token, isLoading: isTokenLoading } = useTokenInfo(scope, address, isContract)
const { results: tokenResults } = useTokenTransfers(scope, address)
const numberOfTokenTransfers = tokenResults.tablePaginationProps.totalCount

const tokenPriceInfo = useTokenPrice(account?.ticker || Ticker.ROSE)

const showTokenTransfers = showEmptyAccountDetails || !!numberOfTokenTransfers
const tokenTransfersLink = useHref(`token-transfers#${accountTokenTransfersContainerId}`)
const showErc20 = showEmptyAccountDetails || !!account?.tokenBalances[EvmTokenType.ERC20]?.length
const erc20Link = useHref(`tokens/erc-20#${accountTokenContainerId}`)
const showErc721 = showEmptyAccountDetails || !!account?.tokenBalances[EvmTokenType.ERC721]?.length
const erc721Link = useHref(`tokens/erc-721#${accountTokenContainerId}`)
const showTxs = showEmptyAccountDetails || showErc20 || !!account?.stats.num_txns

const txLink = useHref('')
const showCode = isContract
const codeLink = useHref(`code#${contractCodeContainerId}`)

const showDetails = showTxs || showErc20
const isLoading = isAccountLoading || isTokenLoading

const context: AccountDetailsContext = { scope, address }
Expand All @@ -71,26 +64,22 @@ export const AccountDetailsPage: FC = () => {
tokenPriceInfo={tokenPriceInfo}
/>
</SubPageCard>
{showDetails && (
<RouterTabs
tabs={[
{ label: t('common.transactions'), to: txLink, visible: showTxs },
{ label: t('tokens.transfers'), to: tokenTransfersLink, visible: showTokenTransfers },
{
label: getTokenTypePluralName(t, EvmTokenType.ERC20),
to: erc20Link,
visible: showErc20,
},
{
label: getTokenTypePluralName(t, EvmTokenType.ERC721),
to: erc721Link,
visible: showErc721,
},
{ label: t('contract.code'), to: codeLink, visible: showCode },
]}
context={context}
/>
)}
<RouterTabs
tabs={[
{ label: t('common.transactions'), to: txLink },
{ label: t('tokens.transfers'), to: tokenTransfersLink },
{
label: getTokenTypePluralName(t, EvmTokenType.ERC20),
to: erc20Link,
},
{
label: getTokenTypePluralName(t, EvmTokenType.ERC721),
to: erc721Link,
},
{ label: t('contract.code'), to: codeLink, visible: showCode },
]}
context={context}
/>
</PageLayout>
)
}
Expand Down
16 changes: 12 additions & 4 deletions src/app/pages/BlockDetailPage/TransactionsCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,24 @@ const TransactionList: FC<{ scope: SearchScope; blockHeight: number }> = ({ scop
offset: txsOffset,
})

const { isLoading, isFetched, data } = transactionsQuery

const transactions = data?.data.transactions

if (isFetched && txsPagination.selectedPage > 1 && !transactions?.length) {
throw AppErrors.PageDoesNotExist
}

return (
<Transactions
transactions={transactionsQuery.data?.data.transactions}
isLoading={transactionsQuery.isLoading}
transactions={transactions}
isLoading={isLoading}
limit={NUMBER_OF_ITEMS_ON_SEPARATE_PAGE}
pagination={{
selectedPage: txsPagination.selectedPage,
linkToPage: txsPagination.linkToPage,
totalCount: transactionsQuery.data?.data.total_count,
isTotalCountClipped: transactionsQuery.data?.data.is_total_count_clipped,
totalCount: data?.data.total_count,
isTotalCountClipped: data?.data.is_total_count_clipped,
rowsPerPage: NUMBER_OF_ITEMS_ON_SEPARATE_PAGE,
}}
/>
Expand Down
Loading

0 comments on commit 68ad4c2

Please sign in to comment.