[Dashboard] add empty state for Pay analytics #7206
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
## Walkthrough
This update introduces a first-time user experience (FTUX) for the Pay Analytics dashboard by conditionally rendering a new `PayEmbedFTUX` component when no relevant analytics data is present. Supporting changes include the addition of the FTUX component, improved error handling in analytics data fetching, an update to the code display component's props, and expanded documentation for the Buy SDK/API.
## Changes
| File(s) | Change Summary |
|-------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------|
| apps/dashboard/src/components/pay/PayAnalytics/PayAnalytics.tsx | Added conditional rendering for FTUX; displays `PayEmbedFTUX` if no analytics data is available. |
| apps/dashboard/src/components/pay/PayAnalytics/PayEmbedFTUX.tsx | Introduced new `PayEmbedFTUX` React component for first-time user onboarding with code samples and documentation. |
| apps/dashboard/src/@/api/analytics.ts | Modified error handling in `getUniversalBridgeWalletUsage` to return `[]` instead of `null` on failure. |
| apps/dashboard/src/@/components/ui/code/code.server.tsx | Added `ignoreFormattingErrors` prop to `CodeProps` and updated `CodeServer` to accept and use this prop. |
| packages/thirdweb/src/bridge/Buy.ts | Updated documentation to include `sender` and `receiver` in examples and parameter lists; marked `purchaseData` as optional. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant PayAnalytics
participant AnalyticsAPI
participant PayEmbedFTUX
User->>PayAnalytics: Render component
PayAnalytics->>AnalyticsAPI: Fetch volume and wallet usage data
AnalyticsAPI-->>PayAnalytics: Return data
alt Data present (volume > 0 or wallet count > 0)
PayAnalytics-->>User: Render analytics dashboard
else No data present
PayAnalytics->>PayEmbedFTUX: Render FTUX with clientId
PayEmbedFTUX-->>User: Show onboarding UI with code samples
endSuggested reviewers
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7206 +/- ##
==========================================
+ Coverage 55.62% 55.63% +0.01%
==========================================
Files 908 908
Lines 58531 58546 +15
Branches 4128 4132 +4
==========================================
+ Hits 32556 32572 +16
+ Misses 25869 25868 -1
Partials 106 106
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/dashboard/src/components/pay/PayAnalytics/PayEmbedFTUX.tsx (1)
104-109: Consider making the API example more complete.The API code snippet could benefit from showing the complete response handling to provide users with a more comprehensive example.
Consider adding response handling to make the example more practical:
const apiCode = (clientId: string) => ` curl -X POST https://pay.thirdweb.com/v1/buy/quote \ -H "Content-Type: application/json" \ -H "x-client-id: ${clientId}" \ -d '{"originChainId":1,"originTokenAddress":"0x...","destinationChainId":10,"destinationTokenAddress":"0x...","amount":"0.01"}' + +# Response example: +# { +# "quote": { +# "fromAmount": "0.01", +# "toAmount": "0.009", +# "estimatedGas": "21000" +# } +# } `;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/src/components/pay/PayAnalytics/PayAnalytics.tsx(2 hunks)apps/dashboard/src/components/pay/PayAnalytics/PayEmbedFTUX.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/src/components/pay/PayAnalytics/PayAnalytics.tsx (1)
apps/dashboard/src/components/pay/PayAnalytics/PayEmbedFTUX.tsx (1)
PayEmbedFTUX(9-76)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
apps/dashboard/src/components/pay/PayAnalytics/PayEmbedFTUX.tsx (2)
1-8: LGTM! Clean imports and proper client directive.The imports are well-organized and the "use client" directive is correctly placed for this interactive React component.
9-76: Well-structured FTUX component with good UX patterns.The component follows React best practices with clear state management and conditional rendering. The tabbed interface provides a good user experience for different integration methods.
apps/dashboard/src/components/pay/PayAnalytics/PayAnalytics.tsx (2)
6-6: LGTM! Clean import addition.The import for PayEmbedFTUX is properly added and follows the existing import pattern.
58-62: Excellent empty state implementation with sound logic.The conditional rendering logic is well-implemented:
- Checks for meaningful data in both volume (
amountUsdCents > 0) and wallet usage (count > 0)- Provides a helpful FTUX experience when no analytics data exists
- Early return pattern keeps the component clean and readable
This improves the user experience by guiding new users through integration when they don't have data to analyze yet.
apps/dashboard/src/components/pay/PayAnalytics/PayEmbedFTUX.tsx
Outdated
Show resolved
Hide resolved
size-limit report 📦
|
gregfromstl
force-pushed
the
codex/add-empty-state-to-pay-analytics-page
branch
from
4480e45 to
b09e4e0
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
packages/thirdweb/src/engine/server-wallet.ts (1)
282-291: 🛠️ Refactor suggestion
sendBatchTransactionwaits for a single hash
waitForTransactionHashis invoked with only the first transaction id, leaving the rest of the batch unmanaged. Either:
- Loop over all ids and
Promise.all, or- Make it explicit in the JSDoc that only the first transaction in the batch is awaited.
Failing to do so may mislead integrators who think the entire batch is mined.
🧹 Nitpick comments (11)
apps/dashboard/src/@/api/analytics.ts (1)
370-370: Consider making all error returns consistent.For complete consistency across the file, consider updating the
getEcosystemWalletUsagefunction to also return an empty array[]instead ofnullon error, matching the pattern used by other analytics functions.- return null; + return [];packages/thirdweb/src/extensions/erc20/drop20.test.ts (1)
145-145: Fix variable name typo.The variable name
erc20ContractAddresis missing the final 'd'.- const erc20ContractAddres = await deployERC20Contract({ + const erc20ContractAddress = await deployERC20Contract({Note: You'll also need to update the references to this variable on lines 155 and 183.
packages/thirdweb/src/extensions/erc1155/read/getOwnedNFTs.test.ts (1)
7-24: Good test coverage but consider robustness improvements.The test cases provide good coverage by testing both with and without indexer. However, consider making the tests more robust:
- Dynamic assertions: The hardcoded expectations (3 NFTs, quantity 411n) might be brittle if test data changes
- Error cases: Consider adding tests for invalid addresses or contracts
- Edge cases: Test scenarios with zero-balance NFTs or empty results
Example improvement:
it("with indexer", async () => { const nfts = await getOwnedNFTs({ contract: DROP1155_CONTRACT, address: "0x00d4da27dedce60f859471d8f595fdb4ae861557", }); - expect(nfts.length).toBe(3); + expect(nfts.length).toBeGreaterThan(0); expect(nfts.find((nft) => nft.id === 4n)?.quantityOwned).toBe(411n); + // Verify all NFTs have valid quantityOwned values + nfts.forEach(nft => { + expect(nft.quantityOwned).toBeGreaterThan(0n); + }); });apps/dashboard/src/core-ui/batch-upload/lazy-mint-form/select-option.tsx (1)
30-84: Consider adding accessibility attributes for radio-like behavior.The component implementation successfully maintains all functionality while migrating to custom components. However, consider enhancing accessibility for the radio button-like behavior.
Add ARIA attributes to improve accessibility:
<Card className={cn( "flex flex-col gap-2 rounded-md p-5 md:w-[350px]", disabled ? "pointer-events-none cursor-not-allowed bg-muted" : "cursor-pointer", isActive && "border-primary", className, )} onClick={onClick} + role="radio" + aria-checked={isActive} + aria-disabled={disabled} + tabIndex={disabled ? -1 : 0} {...divProps} >Also consider adding keyboard navigation support:
+ onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onClick(e as any); + } + }}packages/thirdweb/src/engine/list-server-wallets.ts (1)
1-46: LGTM with minor suggestion! Well-implemented server wallet listing function.The implementation follows the same good practices as the create function. However, consider whether an empty server wallet list should throw an error or return an empty array.
Consider this alternative approach for handling empty results:
- if (!data) { - throw new Error("No server wallets found"); - } - - return data; + return data || [];This would treat an empty list as a valid response rather than an error condition.
packages/thirdweb/src/engine/wait-for-tx-hash.ts (1)
36-71: Consider optimizing the polling mechanism and adding exponential backoff.The current implementation polls every second regardless of how long the transaction has been running. For long-running transactions, this could be inefficient and put unnecessary load on the service.
- // wait for the transaction to be confirmed - await new Promise((resolve) => setTimeout(resolve, 1000)); + // Exponential backoff: start with 1s, max 10s + const elapsed = Date.now() - startTime; + const delay = Math.min(1000 * Math.pow(1.5, Math.floor(elapsed / 30000)), 10000); + await new Promise((resolve) => setTimeout(resolve, delay));packages/thirdweb/src/engine/search-transactions.ts (3)
18-23: Correct JSDoc parameter documentation.The JSDoc comment references
args.transactionIdsparameter which doesn't exist in the actual function signature. The function usesargs.filtersinstead./** * Search for transactions by their ids. * @param args - The arguments for the search. * @param args.client - The thirdweb client to use. - * @param args.transactionIds - The ids of the transactions to search for. + * @param args.filters - The filters to apply when searching for transactions. + * @param args.pageSize - The number of results per page (default: 100). + * @param args.page - The page number to retrieve (default: 1). * @engine
105-105: Consider zero-based pagination for consistency.The default page value is set to 1, but most APIs use zero-based pagination. This could cause confusion for developers expecting standard pagination behavior.
- const { client, filters, pageSize = 100, page = 1 } = args; + const { client, filters, pageSize = 100, page = 0 } = args;If you choose to keep 1-based pagination, please document this clearly in the JSDoc comments to avoid confusion.
117-129: Enhance error messages with more context.The current error messages could be more helpful by providing guidance on what went wrong and how to fix it.
if (searchResult.error) { throw new Error( - `Error searching for transaction with filters ${stringify(filters)}: ${stringify( + `Failed to search transactions. Please check your filters and try again. Filters: ${stringify(filters)}, Error: ${stringify( searchResult.error, )}`, ); } const data = searchResult.data?.result; if (!data) { throw new Error(`No transactions found with filters ${stringify(filters)}`); }packages/engine/src/client/types.gen.ts (1)
120-157:CreateAccount*typings – consider validatinglabellength
labelis currently juststring; engine’s endpoint limits it to 64 chars. If we surface that in the type (e.g. branded type or a JSDoc note) we can catch issues earlier in the SDK.packages/thirdweb/src/engine/server-wallet.ts (1)
184-199: Minor: avoid double.toString()on values already strings
value: t.value?.toString()is safe, but whent.valueis already a string we pay an unnecessary allocation. Micro-optimisation:-value: t.value?.toString(), +value: + typeof t.value === "bigint" || typeof t.value === "number" + ? t.value.toString() + : t.value,
🛑 Comments failed to post (6)
packages/thirdweb/src/engine/wait-for-tx-hash.ts (3)
60-65: 🛠️ Refactor suggestion
Add type validation before casting to
Hex.The code casts
executionResult.transactionHashtoHexwithout validating that it's actually a valid hex string, which could cause runtime issues.return { - transactionHash: executionResult.transactionHash as Hex, + transactionHash: executionResult.transactionHash as Hex, // TODO: Add hex validation client: args.client, chain: executionResult.chain, };Consider adding proper hex validation:
if (!executionResult.transactionHash || !/^0x[a-fA-F0-9]+$/.test(executionResult.transactionHash)) { throw new Error(`Invalid transaction hash format: ${executionResult.transactionHash}`); }🤖 Prompt for AI Agents
In packages/thirdweb/src/engine/wait-for-tx-hash.ts around lines 60 to 65, the code casts executionResult.transactionHash to Hex without validating its format, which risks runtime errors. Add a validation step before the cast to check that executionResult.transactionHash exists and matches the hex string pattern /^0x[a-fA-F0-9]+$/. If the validation fails, throw an error indicating the invalid transaction hash format. This ensures only valid hex strings are cast to Hex.
26-30: 🛠️ Refactor suggestion
Add input validation for better error handling.
The function doesn't validate the
transactionIdparameter, which could lead to unclear error messages if an invalid ID is provided.export async function waitForTransactionHash(args: { client: ThirdwebClient; transactionId: string; timeoutInSeconds?: number; }): Promise<WaitForReceiptOptions> { + if (!args.transactionId?.trim()) { + throw new Error("Transaction ID is required and cannot be empty"); + } + const startTime = Date.now();📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.export async function waitForTransactionHash(args: { client: ThirdwebClient; transactionId: string; timeoutInSeconds?: number; }): Promise<WaitForReceiptOptions> { if (!args.transactionId?.trim()) { throw new Error("Transaction ID is required and cannot be empty"); } const startTime = Date.now(); // …rest of function… }🤖 Prompt for AI Agents
In packages/thirdweb/src/engine/wait-for-tx-hash.ts around lines 26 to 30, add input validation for the transactionId parameter at the start of the waitForTransactionHash function. Check if transactionId is a non-empty string and throw a clear, descriptive error if it is invalid. This will improve error handling by preventing unclear errors later in the function.
1-1:
⚠️ Potential issueIncorrect import source for
stringifyfunction.The
stringifyfunction is being imported from "viem", but based on the relevant code snippets, it should be imported from the utils module.-import { stringify } from "viem"; +import { stringify } from "../utils/json.js";📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.import { stringify } from "../utils/json.js";🤖 Prompt for AI Agents
In packages/thirdweb/src/engine/wait-for-tx-hash.ts at line 1, the stringify function is incorrectly imported from "viem". Change the import source to the utils module by updating the import statement to import stringify from the correct utils path.packages/thirdweb/src/engine/server-wallet.ts (3)
240-246: 🛠️ Refactor suggestion
enqueueTransactionreturns only the first id – API mismatch?Engine can return one id per transaction even in a single-item array.
Exposing only the first id prevents callers from mapping N → N in future refactors (e.g. multi-send withsimulate: true). Consider returning the full string[] and keeping the current helper as a convenience wrapper.🤖 Prompt for AI Agents
In packages/thirdweb/src/engine/server-wallet.ts around lines 240 to 246, the current code returns only the first transaction ID from enqueueTx, which returns an array of IDs for each transaction. To fix this, modify the function to return the full array of transaction IDs (string[]) instead of just the first one, allowing callers to map multiple transactions to their IDs. You can keep the existing single ID return as a separate helper function if needed for convenience.
247-270: 🛠️ Refactor suggestion
enqueueBatchTransactionshould surface all idsSame rationale as above; callers enqueuing
[tx1, tx2]probably expect two IDs so they can poll statuses independently. Returning only the first ID discards information.-const transactionId = transactionIds[0]; -return { transactionId }; +return { transactionIds };(And update docs accordingly.)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/thirdweb/src/engine/server-wallet.ts around lines 247 to 270, the enqueueBatchTransaction function currently returns only the first transaction ID after enqueuing multiple transactions, which loses information for callers expecting all IDs. Modify the function to return all transaction IDs from enqueueTx instead of just the first one, allowing callers to track each transaction independently. Also update the function's return type and any related documentation to reflect that it returns an array of transaction IDs.
167-183:
⚠️ Potential issueGuard against missing
chainIdbefore using it
firstTransaction.chainIdcan technically beundefined(e.g. if a simulated tx is malformed).
Attempting to callgetExecutionOptions(chainId)withundefinedwill later throw a cryptic error.-const chainId = firstTransaction.chainId; +const chainId = firstTransaction.chainId; +if (chainId === undefined) { + throw new Error("chainId is undefined on the first transaction in batch"); +}📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.const enqueueTx = async (transaction: SendTransactionOption[]) => { if (transaction.length === 0) { throw new Error("No transactions to enqueue"); } const firstTransaction = transaction[0]; if (!firstTransaction) { throw new Error("No transactions to enqueue"); } const chainId = firstTransaction.chainId; + if (chainId === undefined) { + throw new Error("chainId is undefined on the first transaction in batch"); + } // Validate all transactions are on the same chain for (let i = 1; i < transaction.length; i++) { if (transaction[i]?.chainId !== chainId) { throw new Error( `All transactions in batch must be on the same chain. Expected ${chainId}, got ${transaction[i]?.chainId} at index ${i}`, ); } }🤖 Prompt for AI Agents
In packages/thirdweb/src/engine/server-wallet.ts around lines 167 to 183, the code uses firstTransaction.chainId without checking if it is undefined, which can cause cryptic errors later. Add a guard to explicitly check if firstTransaction.chainId is undefined and throw a clear error before proceeding. This ensures that chainId is always defined before calling getExecutionOptions or other functions that require it.
gregfromstl
added
the
merge-queue
Merge activity
|
## Summary
- implement PayEmbedFTUX with Embed, SDK, and API tabs
- show new PayEmbedFTUX when analytics have no data
## Checklist
- [x] `pnpm biome check apps/dashboard/src/components/pay/PayAnalytics/PayEmbedFTUX.tsx apps/dashboard/src/components/pay/PayAnalytics/PayAnalytics.tsx --apply`
- [x] `pnpm test` *(fails: spawn anvil ENOENT)*
<!-- start pr-codex -->
---
## PR-Codex overview
This PR primarily focuses on enhancing the `PayAnalytics` and `PayEmbedFTUX` components by adding new features and improving error handling. It also updates the `CodeServer` to support formatting options and modifies the analytics API response handling.
### Detailed summary
- Updated error handling in `analytics.ts` to return an empty array instead of `null`.
- Added `ignoreFormattingErrors` prop to `CodeServer` in `code.server.tsx`.
- Integrated `PayEmbedFTUX` in `PayAnalytics` to display when no volume or wallet data is available.
- Introduced `sender` and `receiver` fields in the bridge purchase options in `Buy.ts`.
- Created `PayEmbedFTUX` component with tabs for "Embed", "SDK", and "API" code examples.
- Added code examples for embedding, SDK usage, and API calls in `PayEmbedFTUX`.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
- **New Features**
- Introduced a first-time user experience (FTUX) interface in the Pay Analytics dashboard, providing integration guides and code examples when no analytics data is available.
- **Improvements**
- Enhanced code example component to support ignoring formatting errors.
- Updated analytics error handling for more consistent data responses.
- **Documentation**
- Expanded usage examples and parameter descriptions for Pay SDK functions, clarifying required fields and optional parameters.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
graphite-app
bot
force-pushed
the
codex/add-empty-state-to-pay-analytics-page
branch
from
e7e2bee to
e2eda05
Compare
graphite-app
bot
removed
the
merge-queue
Summary
Checklist
pnpm biome check apps/dashboard/src/components/pay/PayAnalytics/PayEmbedFTUX.tsx apps/dashboard/src/components/pay/PayAnalytics/PayAnalytics.tsx --applypnpm test(fails: spawn anvil ENOENT)PR-Codex overview
This PR focuses on enhancing the
PayAnalyticsandPayEmbedFTUXcomponents with new features and improved handling of wallet stats and code examples. It introduces error handling, new props, and additional functionality for embedding payment options.Detailed summary
return nullwithreturn []inanalytics.ts.ignoreFormattingErrorsprop inCodeServer.PayAnalytics.PayEmbedFTUXcomponent with tabs for code examples.BuyAPI options.Summary by CodeRabbit