feat: sdk example app#519
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a ChangesSDK: Cover Metadata Module and Quote Refactor
Next.js wagmi Example App
Sequence Diagram(s)sequenceDiagram
participant User
participant BuyCoverPage
participant NexusSDK
participant NexusAPI
participant CoverBrokerContract
rect rgba(70, 130, 180, 0.5)
note over User,NexusSDK: Quote flow
User->>BuyCoverPage: Enter productId/amount/period/asset → Get Quote
BuyCoverPage->>NexusSDK: getQuoteAndBuyCoverInputs({ coverMetadata? })
NexusSDK->>NexusAPI: GET product + productType
NexusSDK->>NexusAPI: POST /cover-metadata (if coverMetadata present)
NexusAPI-->>NexusSDK: { cid }
NexusSDK->>NexusAPI: GET /quote
NexusAPI-->>NexusSDK: quote + buyCoverInput
NexusSDK-->>BuyCoverPage: displayInfo + buyCoverParams
end
rect rgba(34, 139, 34, 0.5)
note over User,CoverBrokerContract: Buy flow
User->>BuyCoverPage: Confirm Buy Cover
BuyCoverPage->>CoverBrokerContract: buyCover(params) [value=ETH if ETH asset]
CoverBrokerContract-->>BuyCoverPage: transaction receipt
BuyCoverPage-->>User: Etherscan link
end
sequenceDiagram
participant User
participant EditMetadataPage
participant wagmiSignTypedData
participant CoverData
User->>EditMetadataPage: Enter coverMetadataId + ProofOfLossEntry value → Submit
EditMetadataPage->>wagmiSignTypedData: signTypedData(buildCoverMetadataAuthMessage())
wagmiSignTypedData-->>EditMetadataPage: signature string
EditMetadataPage->>CoverData: editCoverMetadata({ coverMetadataId, proofOfLoss, signature })
CoverData->>CoverData: buildAuthHeaders(signature) → x-auth-* headers
CoverData-->>EditMetadataPage: { result } or { error }
EditMetadataPage-->>User: success panel or error message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Deploying sdk with
|
| Latest commit: |
ecd1397
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://32df9258.sdk-9yp.pages.dev |
| Branch Preview URL: | https://feat-sdk-example-app.sdk-9yp.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
src/types/product.ts (2)
33-33: ⚡ Quick winConsider documenting validation behavior.
The
proofOfLossInputTypesfield specifies which proof types are required when proof of loss data is provided. Consider adding a JSDoc comment to clarify that:
- These types are validated only when
coverMetadata.proofOfLossis provided- All specified types must be present (at least one entry per type)
- This is independent of
ProductType.isProofOfLossRequired📝 Suggested documentation
+ /** + * Required proof of loss input types for this product. + * When coverMetadata.proofOfLoss is provided, all types listed here must be present. + * Validated in conjunction with ProductType.isProofOfLossRequired. + */ proofOfLossInputTypes?: ProofOfLossEntry['type'][];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types/product.ts` at line 33, Add a JSDoc comment above the proofOfLossInputTypes field to document its validation behavior. The comment should clarify that these types are validated only when coverMetadata.proofOfLoss is provided, that all specified types must be present with at least one entry per type, and that this validation is independent of ProductType.isProofOfLossRequired. This will help other developers understand the field's purpose and behavior constraints.
15-15: ⚡ Quick winConsider documenting the relationship with
proofOfLossInputTypes.The
isProofOfLossRequiredfield works in conjunction withProduct.proofOfLossInputTypes. Their relationship isn't immediately clear from the type definitions:
isProofOfLossRequired: Whether proof of loss data is mandatory for this product typeproofOfLossInputTypes: Which specific proof types must be present (validated only if proof is provided)Consider adding a JSDoc comment explaining this relationship for future maintainers.
📝 Suggested documentation
+ /** + * Whether proof of loss is required for this product type. + * When true, coverMetadata.proofOfLoss must be provided and non-empty. + * Used in conjunction with Product.proofOfLossInputTypes to validate proof data. + */ isProofOfLossRequired?: boolean;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types/product.ts` at line 15, Add a JSDoc comment to the `isProofOfLossRequired` field that documents its relationship with the `proofOfLossInputTypes` field. The comment should clarify that `isProofOfLossRequired` indicates whether proof of loss data is mandatory for this product type, while `proofOfLossInputTypes` specifies which specific proof types must be present (only validated when proof is actually provided). This will help future maintainers understand how these two fields work together.examples/nextjs-wagmi/app/page.tsx (1)
59-88: ⚡ Quick winAdd error handling for wallet connection failures.
The
ConnectButtoncallsconnect({ connector: injected() })without handling potential errors. If the connection fails (e.g., user rejects, MetaMask not installed), the user receives no feedback.♻️ Proposed fix to add error state
function ConnectButton() { const { address, isConnected } = useAccount(); const { mutate: connect } = useConnect(); const { mutate: disconnect } = useDisconnect(); + const [error, setError] = useState<string>(); if (isConnected && address) { return ( <div className="flex items-center gap-3"> <span className="rounded-full bg-primary-light px-3 py-1.5 text-sm font-mono text-primary"> {address.slice(0, 6)}...{address.slice(-4)} </span> <button onClick={() => disconnect()} className="rounded-lg border border-card-border px-4 py-2 text-sm font-medium text-muted transition-colors hover:border-primary hover:text-foreground" > Disconnect </button> </div> ); } return ( - <button - onClick={() => connect({ connector: injected() })} - className="rounded-lg bg-primary px-5 py-2.5 text-sm font-semibold text-white transition-colors hover:bg-primary-hover" - > - Connect Wallet - </button> + <div> + <button + onClick={() => { + setError(undefined); + connect( + { connector: injected() }, + { + onError: (err) => setError(err.message), + } + ); + }} + className="rounded-lg bg-primary px-5 py-2.5 text-sm font-semibold text-white transition-colors hover:bg-primary-hover" + > + Connect Wallet + </button> + {error && ( + <p className="mt-2 text-sm text-red-600">{error}</p> + )} + </div> ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/app/page.tsx` around lines 59 - 88, The ConnectButton component needs error handling for the wallet connection process. Modify the useConnect hook destructuring to also capture the error state that it returns, then add a local state variable to track connection errors. When the user clicks the connect button and the mutation fails, capture and display the error message to the user in the UI. Consider adding error state management alongside the connection logic in the onClick handler for the Connect Wallet button and displaying the error message conditionally in the component's return statement.examples/nextjs-wagmi/app/view-metadata/page.tsx (1)
17-21: ⚡ Quick winHandle invalid cover ID input explicitly.
Line 20 converts
coverIdto a number without validation. If the user bypasses client-side checks orcoverIdcontains non-numeric characters,Number(coverId)returnsNaN, which would pass an invalid value togetCover().While the UI's
canFetchcheck (line 61) prevents the button from being enabled whencoverIdis invalid, adding explicit validation in the mutation function would make the error case clearer and provide better error messages.♻️ Proposed fix to add explicit validation
const viewMetadata = useMutation({ mutationFn: async (): Promise<ViewCoverMetadataResponse> => { + const coverIdNum = Number(coverId); + if (!Number.isInteger(coverIdNum) || coverIdNum <= 0) { + throw new Error('Cover ID must be a positive integer'); + } + // Step 1: Get cover data (includes coverMetadataId) - const coverResponse = await sdk.cover.getCover(Number(coverId)); + const coverResponse = await sdk.cover.getCover(coverIdNum); if (coverResponse.error) { throw new Error(coverResponse.error.message); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/app/view-metadata/page.tsx` around lines 17 - 21, Add explicit validation for the coverId parameter in the mutationFn function of the viewMetadata useMutation hook before passing it to sdk.cover.getCover(). Instead of directly calling Number(coverId), first validate that coverId is a non-empty string and contains only numeric characters, then parse it. If validation fails, throw an error with a clear message indicating what went wrong (e.g., "Invalid cover ID provided"). This ensures that invalid inputs are caught early in the mutation function with better error messaging rather than relying solely on the UI-level canFetch check.examples/nextjs-wagmi/app/edit-metadata/page.tsx (1)
28-41: ⚖️ Poor tradeoffConsider validating entry values before building proof of loss.
The
buildProofOfLossfunction constructs entries without validating the format ofentryValue. For example, whenentryTypeis'address', the function doesn't verify thatentryValueis a valid Ethereum address. Adding validation would prevent submission of malformed data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/app/edit-metadata/page.tsx` around lines 28 - 41, The buildProofOfLoss function constructs proof of loss entries without validating that entryValue conforms to the expected format for the given entryType. Add validation logic at the beginning of the buildProofOfLoss function that checks entryValue against each case type, such as validating that entryValue is a valid Ethereum address when entryType is 'address', validating API key format when entryType is 'api_key', and so on. This validation should occur before the switch statement and should either throw an error or return an invalid state to prevent malformed data from being submitted.examples/nextjs-wagmi/app/buy-cover/page.tsx (1)
91-95: 💤 Low valueReview the exhaustive-deps disable.
The
useEffectdisables thereact-hooks/exhaustive-depsrule becausequoteMutation.resetis not included in the dependency array. This pattern can lead to stale closure bugs if the mutation object identity changes.Consider whether
quoteMutation.resetshould be added to the deps array or wrap it in auseCallbackto stabilize its identity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/app/buy-cover/page.tsx` around lines 91 - 95, The useEffect hook disables the exhaustive-deps ESLint rule because quoteMutation.reset is used within the effect but not included in the dependency array, which can cause stale closure bugs. Fix this by either adding quoteMutation.reset to the dependency array along with debouncedProductId, or by wrapping quoteMutation.reset in a useCallback hook to stabilize its identity, then remove the eslint-disable comment once the dependency issue is resolved.examples/nextjs-wagmi/config/wagmi.ts (1)
1-11: 💤 Low valueConsider adding a comment about RPC fallback for public accessibility.
The config uses http() with no URL, which falls back to a public RPC, though an authenticated RPC URL is highly recommended to prevent rate-limiting. For an example app, this is acceptable, but a brief comment clarifying this trade-off would improve maintainability.
export const config = createConfig({ chains: [mainnet], connectors: [injected()], transports: { + // Falls back to public RPC; use authenticated URL for production [mainnet.id]: http(), }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/config/wagmi.ts` around lines 1 - 11, The http() function in the transports configuration lacks documentation explaining its behavior. Add a comment above the transports object that clarifies that http() without a URL argument falls back to a public RPC endpoint, which may be rate-limited, and note that while acceptable for example applications, an authenticated RPC URL should be used in production environments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/nextjs-wagmi/app/buy-cover/page.tsx`:
- Line 75: The code on line 75 uses a non-null assertion on response.result
without first verifying that the result property exists. While the error check
on lines 71-73 guards against response.error, it does not guarantee that
response.result is present. Add an explicit check to verify that response.result
is defined and not null before returning it, and handle the case where result is
missing appropriately instead of relying on the non-null assertion operator.
- Line 350: The href attribute in the transaction link is hardcoded to the
mainnet Etherscan URL, which breaks for users on other networks. Instead of
using the hardcoded https://etherscan.io/tx/ string, dynamically construct the
explorer URL by accessing the current chain information from wagmi (typically
available through useNetwork or from the chain object in your wagmi config) and
use the appropriate blockExplorers property from the chain data to build the
correct explorer URL for the current network before appending the txHash.
In `@examples/nextjs-wagmi/app/edit-metadata/page.tsx`:
- Around line 53-69: Remove all debug console.log statements from the
editCoverMetadata function block in the page.tsx file. Specifically, delete the
four console.log calls: the one logging the signature ("enters sig"), the one
logging proofOfLoss, the one logging the response, and the one logging the
error. Keep all the actual business logic intact, including the buildProofOfLoss
call, the sdk.cover.editCoverMetadata API call, and the error handling
conditional check. Only remove the logging statements to prevent sensitive data
like signatures and proof of loss information from being exposed to browser
consoles.
In `@examples/nextjs-wagmi/package.json`:
- Line 26: The eslint-config-next package version set to 16.2.9 does not align
with the actual Next.js version being used in the project. Verify the Next.js
version specified in the dependencies of the package.json file and update the
eslint-config-next version to match the corresponding compatible version for
that Next.js release, ensuring consistency across both packages.
In `@src/cover/Cover.ts`:
- Around line 20-31: The validation check in the getCover method currently does
not prevent Infinity and -Infinity values from passing through since these are
not caught by the `!coverId || coverId <= 0` condition. Replace or enhance the
validation condition to also check that coverId is a finite number by using
Number.isFinite(coverId) or by adding explicit checks to reject Infinity and
-Infinity values alongside the existing checks for zero, negative numbers, and
NaN.
---
Nitpick comments:
In `@examples/nextjs-wagmi/app/buy-cover/page.tsx`:
- Around line 91-95: The useEffect hook disables the exhaustive-deps ESLint rule
because quoteMutation.reset is used within the effect but not included in the
dependency array, which can cause stale closure bugs. Fix this by either adding
quoteMutation.reset to the dependency array along with debouncedProductId, or by
wrapping quoteMutation.reset in a useCallback hook to stabilize its identity,
then remove the eslint-disable comment once the dependency issue is resolved.
In `@examples/nextjs-wagmi/app/edit-metadata/page.tsx`:
- Around line 28-41: The buildProofOfLoss function constructs proof of loss
entries without validating that entryValue conforms to the expected format for
the given entryType. Add validation logic at the beginning of the
buildProofOfLoss function that checks entryValue against each case type, such as
validating that entryValue is a valid Ethereum address when entryType is
'address', validating API key format when entryType is 'api_key', and so on.
This validation should occur before the switch statement and should either throw
an error or return an invalid state to prevent malformed data from being
submitted.
In `@examples/nextjs-wagmi/app/page.tsx`:
- Around line 59-88: The ConnectButton component needs error handling for the
wallet connection process. Modify the useConnect hook destructuring to also
capture the error state that it returns, then add a local state variable to
track connection errors. When the user clicks the connect button and the
mutation fails, capture and display the error message to the user in the UI.
Consider adding error state management alongside the connection logic in the
onClick handler for the Connect Wallet button and displaying the error message
conditionally in the component's return statement.
In `@examples/nextjs-wagmi/app/view-metadata/page.tsx`:
- Around line 17-21: Add explicit validation for the coverId parameter in the
mutationFn function of the viewMetadata useMutation hook before passing it to
sdk.cover.getCover(). Instead of directly calling Number(coverId), first
validate that coverId is a non-empty string and contains only numeric
characters, then parse it. If validation fails, throw an error with a clear
message indicating what went wrong (e.g., "Invalid cover ID provided"). This
ensures that invalid inputs are caught early in the mutation function with
better error messaging rather than relying solely on the UI-level canFetch
check.
In `@examples/nextjs-wagmi/config/wagmi.ts`:
- Around line 1-11: The http() function in the transports configuration lacks
documentation explaining its behavior. Add a comment above the transports object
that clarifies that http() without a URL argument falls back to a public RPC
endpoint, which may be rate-limited, and note that while acceptable for example
applications, an authenticated RPC URL should be used in production
environments.
In `@src/types/product.ts`:
- Line 33: Add a JSDoc comment above the proofOfLossInputTypes field to document
its validation behavior. The comment should clarify that these types are
validated only when coverMetadata.proofOfLoss is provided, that all specified
types must be present with at least one entry per type, and that this validation
is independent of ProductType.isProofOfLossRequired. This will help other
developers understand the field's purpose and behavior constraints.
- Line 15: Add a JSDoc comment to the `isProofOfLossRequired` field that
documents its relationship with the `proofOfLossInputTypes` field. The comment
should clarify that `isProofOfLossRequired` indicates whether proof of loss data
is mandatory for this product type, while `proofOfLossInputTypes` specifies
which specific proof types must be present (only validated when proof is
actually provided). This will help future maintainers understand how these two
fields work together.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33f26520-26d1-4a93-9511-e2a9127874bb
⛔ Files ignored due to path filters (4)
examples/nextjs-wagmi/app/favicon.icois excluded by!**/*.icoexamples/nextjs-wagmi/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlexamples/nextjs-wagmi/public/globe.svgis excluded by!**/*.svgexamples/nextjs-wagmi/public/vercel.svgis excluded by!**/*.svg
📒 Files selected for processing (37)
.npmignoreexamples/nextjs-wagmi/.gitignoreexamples/nextjs-wagmi/README.mdexamples/nextjs-wagmi/app/buy-cover/page.tsxexamples/nextjs-wagmi/app/edit-metadata/page.tsxexamples/nextjs-wagmi/app/globals.cssexamples/nextjs-wagmi/app/layout.tsxexamples/nextjs-wagmi/app/page.tsxexamples/nextjs-wagmi/app/providers.tsxexamples/nextjs-wagmi/app/view-metadata/page.tsxexamples/nextjs-wagmi/config/sdk.tsexamples/nextjs-wagmi/config/wagmi.tsexamples/nextjs-wagmi/eslint.config.mjsexamples/nextjs-wagmi/hooks/useDebounce.tsexamples/nextjs-wagmi/next.config.tsexamples/nextjs-wagmi/package.jsonexamples/nextjs-wagmi/pnpm-workspace.yamlexamples/nextjs-wagmi/postcss.config.mjsexamples/nextjs-wagmi/tsconfig.jsonsrc/auth.test.tssrc/auth.tssrc/cover/Cover.test.tssrc/cover/Cover.tssrc/cover/index.tssrc/index.tssrc/ipfs/Ipfs.tssrc/ipfs/schemas.tssrc/ipfs/uploadIPFSContent.test.tssrc/ipfs/validateIPFSContent.test.tssrc/nexus-sdk.tssrc/quote/Quote.tssrc/quote/getQuoteAndBuyCoverInputs.test.tssrc/types/cover-metadata.tssrc/types/index.tssrc/types/ipfs.tssrc/types/product.tssrc/types/sdk.ts
💤 Files with no reviewable changes (4)
- src/ipfs/validateIPFSContent.test.ts
- src/ipfs/schemas.ts
- src/ipfs/Ipfs.ts
- src/types/ipfs.ts
a56cc4b to
b093af9
Compare
04685f2 to
e2f18bf
Compare
Webapp built with Next.js, wagmi, tailwind and nexusmutual/sdk as a proof of concept for developers who will integrate our sdk
Summary by CodeRabbit
Release Notes
New Features
Updates