Jennifer/scan updates #1661
Jennifer/scan updates #1661
Conversation
Jennievon
requested review from
otherview,
0xjba,
CaisManai and
BedrockSquirrel
WalkthroughThe updates across the ObscuroScan v3 frontend involve enhancing error handling with custom components for 404 and 500 errors, improving SEO with a new Changes
TipsChat with CodeRabbit Bot (
|
…en-protocol/go-ten into jennifer/scan-updates
…en-protocol/go-ten into jennifer/scan-updates
…nnifer/scan-updates
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- tools/obscuroscan_v3/frontend/public/docs/terms.json
Files selected for processing (13)
- tools/obscuroscan_v3/frontend/pages/404.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/pages/500.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/pages/_app.tsx (2 hunks)
- tools/obscuroscan_v3/frontend/pages/_error.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/pages/docs/[id].tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/components/head-seo.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/components/layouts/footer.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/components/modules/dashboard/index.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/components/ui/spinner.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/lib/constants.ts (1 hunks)
- tools/obscuroscan_v3/frontend/src/lib/siteMetadata.ts (1 hunks)
- tools/obscuroscan_v3/frontend/src/types/interfaces/index.ts (1 hunks)
- tools/obscuroscan_v3/frontend/styles/globals.css (1 hunks)
Files skipped from review due to trivial changes (3)
- tools/obscuroscan_v3/frontend/src/components/modules/dashboard/index.tsx
- tools/obscuroscan_v3/frontend/src/lib/constants.ts
- tools/obscuroscan_v3/frontend/styles/globals.css
Additional comments: 13
tools/obscuroscan_v3/frontend/pages/404.tsx (1)
- 4-39:
The implementation ofCustom404Errorcorrectly uses theErrorcomponent and provides sensible defaults and customization options for a 404 error page. The use of template literals and conditional rendering is appropriate for dynamic content.tools/obscuroscan_v3/frontend/src/components/ui/spinner.tsx (1)
- 1-11:
The implementation of theSpinnercomponent looks correct and follows best practices for functional components in React. It is properly styled using Tailwind CSS classes.tools/obscuroscan_v3/frontend/src/lib/siteMetadata.ts (2)
1-15: - The
siteLogoandsiteLogoSquareproperties in thesiteMetadataobject are assigned empty template literals, which could lead to issues if the values are expected to be URLs or paths to image assets. Verify that these properties are intentionally left empty and if not, provide the correct values.1-15: - The
socialLinksimport is used to populate thegithubproperties in thesiteMetadataobject. Ensure that theconstantsfile exists and contains thesocialLinksexport with the appropriate structure and values.tools/obscuroscan_v3/frontend/pages/500.tsx (1)
- 1-32:
The implementation of theCustom500Errorcomponent aligns with the change summary, correctly handling the rendering of a 500 error page with customizable options.tools/obscuroscan_v3/frontend/src/components/layouts/footer.tsx (2)
34-36: The
hrefattribute for the "Privacy" link has been correctly updated to point to the new dynamic documentation route. Ensure that the dynamic route/docs/privacyis properly configured in the routing system to serve the Privacy Policy content.40-42: The
hrefattribute for the "Terms" link has been correctly updated to point to the new dynamic documentation route. Ensure that the dynamic route/docs/termsis properly configured in the routing system to serve the Terms of Service content.tools/obscuroscan_v3/frontend/src/types/interfaces/index.ts (2)
3-11:
TheSeoPropsinterface is well-defined and aligns with the pull request's goal to improve SEO. Ensure that all properties are being used where theHeadSeocomponent is implemented.13-15:
No changes have been made to theErrorTypeinterface in this hunk, which is consistent with the summary stating there are no alterations to existing interfaces.tools/obscuroscan_v3/frontend/pages/_app.tsx (2)
12-18:
The new imports are correctly placed and relevant to the functionality being added to the_app.tsxfile.69-80:
TheHeadSeocomponent is correctly implemented with appropriate properties fromsiteMetadata.tools/obscuroscan_v3/frontend/pages/docs/[id].tsx (2)
80-82:
Ensure that the content passed todangerouslySetInnerHTMLis sanitized to prevent XSS attacks.56-60:
Verify that theidused in theuseEffecthook is validated to ensure it's a proper document identifier before fetching the document.
| <title>{title}</title> | ||
| <meta charSet="utf-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
| <meta name={siteMetadata.metaTitle} content={siteMetadata.companyName} /> |
There was a problem hiding this comment.
The meta tag for the site title is incorrect.
- <meta name={siteMetadata.metaTitle} content={siteMetadata.companyName} />
+ <meta name="title" content={title} />Commitable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| <meta name={siteMetadata.metaTitle} content={siteMetadata.companyName} /> | |
| <meta name="title" content={title} /> |
| <> | ||
| <Script | ||
| strategy="lazyOnload" | ||
| src={`https://www.googletagmanager.com/gtag/js?id='${GOOGLE_ANALYTICS_ID}'`} |
There was a problem hiding this comment.
The script URL contains extra quotes that may prevent the correct loading of the Google Analytics script.
- src={`https://www.googletagmanager.com/gtag/js?id='${GOOGLE_ANALYTICS_ID}'`}
+ src={`https://www.googletagmanager.com/gtag/js?id=${GOOGLE_ANALYTICS_ID}`}Commitable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| src={`https://www.googletagmanager.com/gtag/js?id='${GOOGLE_ANALYTICS_ID}'`} | |
| src={`https://www.googletagmanager.com/gtag/js?id=${GOOGLE_ANALYTICS_ID}`} |
| const { query } = useRouter(); | ||
| const { id } = query; | ||
|
|
||
| const [document, setDocument] = React.useState<Document>({} as Document); |
There was a problem hiding this comment.
Consider initializing the document state with null or a more appropriate default value that reflects the absence of data until it's fetched.
- const [document, setDocument] = React.useState<Document>({} as Document);
+ const [document, setDocument] = React.useState<Document | null>(null);Commitable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const [document, setDocument] = React.useState<Document>({} as Document); | |
| const [document, setDocument] = React.useState<Document | null>(null); |
| content: data.content.map((item: any) => { | ||
| return { | ||
| heading: item.heading, | ||
| content: item.content.map((paragraph: any) => { | ||
| return paragraph.replace( | ||
| /siteMetadata.email/g, | ||
| siteMetadata.email | ||
| ); | ||
| }), |
There was a problem hiding this comment.
The placeholder replacement logic is incorrect. Use a regular expression to replace 'siteMetadata.email' with the actual email from siteMetadata.
- return paragraph.replace(/siteMetadata.email/g, siteMetadata.email);
+ return paragraph.replace(/\[email\]/g, siteMetadata.email);Commitable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| content: data.content.map((item: any) => { | |
| return { | |
| heading: item.heading, | |
| content: item.content.map((paragraph: any) => { | |
| return paragraph.replace( | |
| /siteMetadata.email/g, | |
| siteMetadata.email | |
| ); | |
| }), | |
| content: data.content.map((item: any) => { | |
| return { | |
| heading: item.heading, | |
| content: item.content.map((paragraph: any) => { | |
| return paragraph.replace( | |
| /\[email\]/g, | |
| siteMetadata.email | |
| ); | |
| }), |
| function ErrorMessage({ | ||
| statusText, | ||
| message, | ||
| showMessage, | ||
| showStatusText, | ||
| }: any) { | ||
| return ( | ||
| <div className="error-message"> | ||
| {showStatusText && <h3>{statusText}</h3>} | ||
| {message && showMessage && ( | ||
| <p className="text-muted-foreground">{message}</p> | ||
| )} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The ErrorMessage component should have a defined prop type for better type safety and to prevent potential bugs.
- function ErrorMessage({
- statusText,
- message,
- showMessage,
- showStatusText,
- }: any) {
+ interface ErrorMessageProps {
+ statusText: string;
+ message: string;
+ showMessage: boolean;
+ showStatusText: boolean;
+ }
+ function ErrorMessage({
+ statusText,
+ message,
+ showMessage,
+ showStatusText,
+ }: ErrorMessageProps) {Commitable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| function ErrorMessage({ | |
| statusText, | |
| message, | |
| showMessage, | |
| showStatusText, | |
| }: any) { | |
| return ( | |
| <div className="error-message"> | |
| {showStatusText && <h3>{statusText}</h3>} | |
| {message && showMessage && ( | |
| <p className="text-muted-foreground">{message}</p> | |
| )} | |
| </div> | |
| ); | |
| } | |
| interface ErrorMessageProps { | |
| statusText: string; | |
| message: string; | |
| showMessage: boolean; | |
| showStatusText: boolean; | |
| } | |
| function ErrorMessage({ | |
| statusText, | |
| message, | |
| showMessage, | |
| showStatusText, | |
| }: ErrorMessageProps) { | |
| return ( | |
| <div className="error-message"> | |
| {showStatusText && <h3>{statusText}</h3>} | |
| {message && showMessage && ( | |
| <p className="text-muted-foreground">{message}</p> | |
| )} | |
| </div> | |
| ); | |
| } |
| CustomError.getInitialProps = async ({ res, err }: any) => { | ||
| const statusCode = res ? res.statusCode : err?.statusCode || 404; | ||
| const errorInitialProps = await NextErrorComponent.getInitialProps({ | ||
| res, | ||
| err, | ||
| } as any); | ||
| errorInitialProps.statusCode = statusCode; | ||
|
|
||
| return statusCode < 500 | ||
| ? errorInitialProps | ||
| : { ...errorInitialProps, statusCode }; |
There was a problem hiding this comment.
The getInitialProps method should use proper types for its parameters instead of any to ensure type safety.
- CustomError.getInitialProps = async ({ res, err }: any) => {
+ CustomError.getInitialProps = async ({ res, err }: { res?: NextApiResponse, err?: Error }) => {Commitable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| CustomError.getInitialProps = async ({ res, err }: any) => { | |
| const statusCode = res ? res.statusCode : err?.statusCode || 404; | |
| const errorInitialProps = await NextErrorComponent.getInitialProps({ | |
| res, | |
| err, | |
| } as any); | |
| errorInitialProps.statusCode = statusCode; | |
| return statusCode < 500 | |
| ? errorInitialProps | |
| : { ...errorInitialProps, statusCode }; | |
| CustomError.getInitialProps = async ({ res, err }: { res?: NextApiResponse, err?: Error }) => { | |
| const statusCode = res ? res.statusCode : err?.statusCode || 404; | |
| const errorInitialProps = await NextErrorComponent.getInitialProps({ | |
| res, | |
| err, | |
| } as any); | |
| errorInitialProps.statusCode = statusCode; | |
| return statusCode < 500 | |
| ? errorInitialProps | |
| : { ...errorInitialProps, statusCode }; |
- use polling interval only on home page
…nnifer/scan-updates
There was a problem hiding this comment.
Review Status
Actionable comments generated: 16
Configuration used: CodeRabbit UI
Files selected for processing (13)
- tools/obscuroscan_v3/frontend/pages/batches/index.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/pages/blocks/index.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/pages/docs/[id].tsx (1 hunks)
- tools/obscuroscan_v3/frontend/pages/transactions/index.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/components/modules/batches/batch-details.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/components/modules/common/data-table/data-table-pagination.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/components/modules/common/data-table/data-table-toolbar.tsx (4 hunks)
- tools/obscuroscan_v3/frontend/src/components/modules/common/data-table/data-table.tsx (5 hunks)
- tools/obscuroscan_v3/frontend/src/lib/constants.ts (2 hunks)
- tools/obscuroscan_v3/frontend/src/lib/siteMetadata.ts (1 hunks)
- tools/obscuroscan_v3/frontend/src/services/useBatchesService.ts (1 hunks)
- tools/obscuroscan_v3/frontend/src/services/useBlocksService.ts (1 hunks)
- tools/obscuroscan_v3/frontend/src/services/useTransactionsService.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- tools/obscuroscan_v3/frontend/src/components/modules/batches/batch-details.tsx
Additional comments: 14
tools/obscuroscan_v3/frontend/pages/batches/index.tsx (2)
20-23: The
useEffecthook correctly setsNoPollingto true, but ensure that this behavior is intended and that there are no side effects or scenarios where polling should continue.36-41: The
DataTablecomponent is correctly using theBatchesDataandTotalvariables, and therefetchandtotalprops are added. Ensure that theDataTablecomponent is designed to handle these props correctly.tools/obscuroscan_v3/frontend/src/components/modules/common/data-table/data-table-pagination.tsx (1)
- 41-47: The update to the page size options in the
DataTablePaginationcomponent from a range of 10 to 50 to a range of 5 to 100 is a positive change for flexibility. Ensure that thetable.setPageSizefunction and any related pagination logic can handle the new range of values without issues.tools/obscuroscan_v3/frontend/src/lib/constants.ts (2)
2-4: Ensure that the updated social links are correctly reflected in all parts of the application where they are used.
17-17: Verify that the
GOOGLE_ANALYTICS_IDis being utilized correctly in the application for Google Analytics integration.tools/obscuroscan_v3/frontend/src/lib/siteMetadata.ts (1)
- 10-11: The
siteLogoandsiteLogoSquarefields are empty. If this is not intentional, they should be populated with appropriate URLs or paths to the logo images.tools/obscuroscan_v3/frontend/src/services/useBatchesService.ts (3)
4-5: The addition of
useStateanduseRouterimports aligns with the new state management and routing requirements.10-10: The introduction of the
noPollingstate variable is a good approach to control the polling behavior dynamically.38-39: Exposing
setNoPollingandrefetchBatchesin the return object allows for external control over polling and data refetching, which is a good enhancement.tools/obscuroscan_v3/frontend/src/services/useTransactionsService.ts (5)
9-12: The addition of
useRouteris appropriate for accessing query parameters for pagination and other options.28-28: The introduction of
noPollingstate variable and its setter functionsetNoPollingis a good practice for controlling polling behavior.38-46: The modification of the
useQueryhook to include dynamicoptionsand arefetchfunction is a good enhancement for flexibility and control over data fetching.43-44: The update to
fetchTransactionsto acceptoptionsas a parameter aligns with the dynamic nature of the query parameters.87-91: Exposing
refetchTransactionsandsetNoPollingin the return object allows for external control over data fetching and polling, which is a good practice for component reusability.
| React.useEffect(() => { | ||
| setNoPolling(true); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []); |
There was a problem hiding this comment.
Consider adding a cleanup function in the useEffect hook to reset the noPolling state when the component unmounts to prevent potential side effects in other components or instances that might rely on the same state.
React.useEffect(() => {
setNoPolling(true);
+ return () => setNoPolling(false);
}, []);Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| React.useEffect(() => { | |
| setNoPolling(true); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, []); | |
| React.useEffect(() => { | |
| setNoPolling(true); | |
| return () => setNoPolling(false); | |
| }, []); |
| {TransactionsData ? ( | ||
| <DataTable | ||
| columns={columns} | ||
| data={transactions?.result?.TransactionsData} | ||
| data={TransactionsData} | ||
| refetch={refetchTransactions} | ||
| total={+Total} | ||
| /> | ||
| ) : ( | ||
| <p>Loading...</p> |
There was a problem hiding this comment.
The conditional rendering logic to display the loading message is incorrect. Since TransactionsData is initialized to an empty array, the condition will always evaluate to true, and the loading message will never be shown. Consider checking if the array is empty instead.
- {TransactionsData ? (
+ {TransactionsData.length > 0 ? (Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| {TransactionsData ? ( | |
| <DataTable | |
| columns={columns} | |
| data={transactions?.result?.TransactionsData} | |
| data={TransactionsData} | |
| refetch={refetchTransactions} | |
| total={+Total} | |
| /> | |
| ) : ( | |
| <p>Loading...</p> | |
| {TransactionsData.length > 0 ? ( | |
| <DataTable | |
| columns={columns} | |
| data={TransactionsData} | |
| refetch={refetchTransactions} | |
| total={+Total} | |
| /> | |
| ) : ( | |
| <p>Loading...</p> |
tools/obscuroscan_v3/frontend/src/components/modules/common/data-table/data-table.tsx
Show resolved
Hide resolved
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [id]); |
There was a problem hiding this comment.
Consider explaining why the react-hooks/exhaustive-deps rule is disabled or include all necessary dependencies in the dependency array of useEffect.
| } catch (error) { | ||
| toast({ | ||
| variant: "destructive", | ||
| description: "Error fetching document", | ||
| }); |
There was a problem hiding this comment.
Consider adding more robust error handling, such as logging the error or providing a more detailed error message to the user.
| ) : ( | ||
| <p>Loading...</p> |
There was a problem hiding this comment.
Consider using a more robust loading indicator, such as a spinner or progress bar, to improve user experience.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- tools/obscuroscan_v3/frontend/pages/batches/[hash].tsx (2 hunks)
- tools/obscuroscan_v3/frontend/src/components/layouts/footer.tsx (2 hunks)
- tools/obscuroscan_v3/frontend/src/components/modules/batches/batch-details.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/src/components/modules/personal/index.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- tools/obscuroscan_v3/frontend/pages/batches/[hash].tsx
- tools/obscuroscan_v3/frontend/src/components/modules/personal/index.tsx
Additional comments: 3
tools/obscuroscan_v3/frontend/src/components/layouts/footer.tsx (2)
47-47: The
versionvariable is used here but it's not clear where it's defined. Ensure thatversionis imported or defined within the scope to avoid reference errors.34-45: The use of
Linkcomponents from "next/link" is appropriate for client-side navigation and is consistent with the summary provided.tools/obscuroscan_v3/frontend/src/components/modules/batches/batch-details.tsx (1)
- 8-12: Ensure that all references to the
BatchDetailsfunction have been updated toBatchDetailsComponentthroughout the codebase to prevent any import errors.
add privacy policy
Jennievon
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- tools/obscuroscan_v3/frontend/public/docs/privacy.json
Files selected for processing (4)
- tools/obscuroscan_v3/frontend/pages/404.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/pages/_error.tsx (1 hunks)
- tools/obscuroscan_v3/frontend/pages/docs/[id].tsx (1 hunks)
- tools/obscuroscan_v3/frontend/styles/globals.css (1 hunks)
Files skipped from review due to trivial changes (1)
- tools/obscuroscan_v3/frontend/styles/globals.css
Additional comments: 2
tools/obscuroscan_v3/frontend/pages/404.tsx (1)
- 4-38: The implementation of
Custom404Errorfunction is consistent with the summary and follows good practices for optional parameters and default values. The use of theErrorcomponent for rendering the error message is appropriate and the function is correctly exported as the default export of the file.tools/obscuroscan_v3/frontend/pages/_error.tsx (1)
- 23-91: The summary states that the type of parameter
propswas changed to accept any type, but the hunk does not reflect this change. Please verify if this change is implemented elsewhere or if the summary is incorrect.
Jennievon
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
Jennievon
had a problem deploying
to
dev-testnet
GitHub Actions
Failure
Jennievon
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
Done! |
Why this change is needed
Please provide a description and a link to the underlying ticket
What changes were made as part of this PR
Please provide a high level list of the changes made
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks