Feat : Rates in BuyNow Modal #21
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BuyNowModal
participant useGetRate
participant QueryClient
participant ApolloProvider
User->>BuyNowModal: Open Modal
BuyNowModal->>useGetRate: Fetch Rate
useGetRate->>QueryClient: Query for Rate
QueryClient-->>useGetRate: Return Rate
useGetRate-->>BuyNowModal: Provide Rate
BuyNowModal->>User: Display Pricing Info
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (4)
src/utils/string.ts (1)
1-9: Consider adding JSDoc documentation.Since this is a utility function that might be used across the application, adding comprehensive documentation would be beneficial.
Consider adding this documentation:
+/** + * Shortens a string to a specified length while maintaining readability. + * If the string is longer than the specified length, it will be truncated + * with an ellipsis in the middle. + * + * @param str - The string to shorten + * @param length - The desired maximum length + * @returns The shortened string or empty string if input is invalid + * @example + * shortenString("Hello World!", 8) // "Hel...ld!" + * shortenString(undefined, 8) // "" + */ export function shortenString(str: string | undefined, length: number): string {src/lib/react-query/client.ts (1)
1-36: Consider implementing a comprehensive error handling strategySince this QueryClient configuration will be used across the application, consider:
- Implementing different error handling strategies based on error types (network, validation, etc.)
- Adding retry logic for transient failures, especially important for rate fetching
- Implementing proper error boundaries in React components to catch and handle render errors
- Adding proper logging for errors to help with debugging and monitoring
Also, consider implementing a caching strategy specific to rate data:
- Implement optimistic updates for better UX
- Consider using websockets for real-time rate updates if rates change frequently
🧰 Tools
🪛 eslint (1.23.1)
[error] 15-15: React Hook "useToast" is called in function "onError" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use".
(react-hooks/rules-of-hooks)
src/lib/andrjs/hooks/ado/useGetRate.ts (1)
20-35: Consider implementing optimistic UI updates and proper error statesSince this hook is used in the BuyNow flow, consider these architectural improvements:
- Implement optimistic UI updates to show estimated rates while fetching
- Add proper loading states in the UI to prevent layout shifts
- Consider implementing a background refresh strategy for rates
- Add retry logic for failed requests with exponential backoff
Would you like me to provide implementation examples for any of these suggestions?
src/modules/modals/components/BuyNowModal.tsx (1)
66-66: Removeconsole.logstatements before production.The
console.logstatements at lines 66 and 80 are useful for debugging but should be removed in production code to clean up console output.Apply the following diff to remove the logs:
- console.log(totalAmount) ... - console.log("DENOM:", DENOM);Also applies to: 80-80
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
app/providers.tsx(2 hunks)package.json(2 hunks)src/lib/andrjs/hooks/ado/useGetRate.ts(1 hunks)src/lib/react-query/client.ts(1 hunks)src/modules/modals/components/BuyNowModal.tsx(4 hunks)src/utils/string.ts(1 hunks)
🧰 Additional context used
🪛 eslint (1.23.1)
src/lib/react-query/client.ts
[error] 15-15: React Hook "useToast" is called in function "onError" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use".
(react-hooks/rules-of-hooks)
🪛 Biome (1.9.4)
src/modules/modals/components/BuyNowModal.tsx
[error] 152-152: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (6)
src/utils/string.ts (2)
1-4: LGTM! Good type safety and null checks.
The function properly handles undefined values and non-string inputs, returning an empty string in these cases.
1-9: Verify the impact on error message readability.
Since this function is used for shortening error messages in toast notifications, we should verify that it maintains message clarity across different error scenarios.
✅ Verification successful
Let me analyze the results and generate additional verification scripts to get a complete picture.
Based on the gathered information, I can now provide the final response:
Error message shortening implementation is safe and properly constrained
The shortenString function is only used in one place - the error toast notifications with a fixed length of 100 characters. The implementation preserves both the start and end portions of the message, which maintains readability. The error modal uses the full message text in a textarea, ensuring no information is lost for detailed error viewing.
- The only usage is in
src/lib/react-query/client.tsfor toast notifications - Error modal in
src/modules/modals/components/ModalError.tsxshows full messages - No test cases found that need updating
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find and analyze error messages that might be shortened
# Look for error-related string constants and toast usage
# Search for error messages in the codebase
echo "Searching for potential error messages..."
rg -g '!*.{json,md}' -i "error.*message|toast.*error" -B 2 -A 2
# Look for toast notification usage
echo "Analyzing toast notification patterns..."
rg -g '!*.{json,md}' "toast\(" -B 2 -A 2
Length of output: 2549
Script:
#!/bin/bash
# Let's verify the actual usage of shortenString function and its impact
# First, find all direct usages of shortenString
echo "Finding direct usages of shortenString..."
rg "shortenString\(" -B 2 -A 2
# Then, let's look at the truncate function mentioned in ModalError.tsx
# as it seems to be an alternative string shortening implementation
echo "Finding truncate function implementation..."
rg "function truncate" -B 2 -A 2
# Also check for any test cases related to these functions
echo "Looking for related test cases..."
fd -e test.ts -e spec.ts -e test.tsx -e spec.tsx --exec rg -l "shortenString|truncate"
Length of output: 1250
src/lib/react-query/client.ts (1)
1-3: LGTM!
The imports are appropriate for the required functionality.
src/lib/andrjs/hooks/ado/useGetRate.ts (1)
1-2: LGTM! Imports are clean and necessary.
app/providers.tsx (1)
21-31: LGTM! Provider hierarchy is well structured
The integration of QueryClientProvider as the root provider is correct, and the existing provider hierarchy is properly maintained. This setup ensures that React Query's functionality will be available throughout the application while preserving the functionality of other providers.
package.json (1)
26-26: Verify React Query v5 stability
The project is using a recent version of React Query (v5.60.6). Let's verify its stability and any potential breaking changes.
✅ Verification successful
React Query v5 implementation is stable and properly configured
The codebase shows a well-structured React Query setup with:
- Proper configuration in
client.tswith sensible defaults (5-minute stale time) - Correct provider setup in the application root
- Standard usage patterns across multiple components
- Compatible peer dependencies (React 18/19)
- Only one minor version behind latest (5.60.6 vs 5.61.0)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check React Query's latest stable version and recent releases
# Get latest version and release date
curl -s https://registry.npmjs.org/@tanstack/react-query | jq -r '.["dist-tags"].latest, .time.modified'
# Check for any deprecation notices or breaking changes
curl -s https://raw.githubusercontent.com/TanStack/query/main/CHANGELOG.md | head -n 50
Length of output: 243
Script:
#!/bin/bash
# Let's check the package-lock.json for any peer dependencies and version constraints
fd package-lock.json --exec cat {} | jq -r '.packages["node_modules/@tanstack/react-query"]'
# Check if there are any direct usages of React Query v5 specific features
rg -l "useQuery|useMutation|useInfiniteQuery|useQueries|QueryClient" --type ts --type tsx
# Look for any React Query configuration or setup files
fd -e ts -e tsx -e js -e jsx --exec grep -l "QueryClient\|QueryClientProvider" {}
Length of output: 799
Script:
#!/bin/bash
# Let's examine the React Query setup files
cat ./src/lib/react-query/client.ts
cat ./app/providers.tsx
# Let's try searching for React Query usage with a different command
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "useQuery\|useMutation\|useInfiniteQuery\|useQueries" {} \;
Length of output: 2702
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (10)
src/lib/react-query/client.ts (3)
4-5: Remove extra blank lineThere's an unnecessary extra blank line that can be removed to maintain consistent spacing.
24-28: Remove extra blank lines in the error handlerThere are unnecessary blank lines after the toast configuration object and before the closing parenthesis. Consider removing them to maintain consistent code formatting.
duration: 3000, isClosable: true } - ) -
13-18: Consider enhancing error handling robustnessWhile the current error handling is functional, consider these improvements:
- Define a specific error type instead of using the generic
Error- Add more specific error handling for different error types
- Consider logging errors for debugging purposes
- onError: (err) => { + onError: (err: unknown) => { if ("error" in err) { - err = err.error as Error + err = err.error as Error; + // Consider logging for debugging + console.error('Mutation error:', err); } - let message: string = err?.message ?? "No Description" + let message: string; + if (err instanceof Error) { + message = err.message; + } else if (typeof err === 'string') { + message = err; + } else { + message = "An unexpected error occurred"; + }src/modules/modals/components/BuyNowModal.tsx (6)
43-56: Consider improving type safety and simplifying rate calculationsThe
calcAmountfunction could benefit from better type safety and simplified logic.Consider this refactored version:
- const calcAmount = (): Coin | undefined => { + type RateValue = { + flat?: { amount: string; denom: string }; + percent?: { percent: string }; + }; + + const calcAmount = (): Coin | undefined => { + if (!rateValue) return undefined; + if (flatRate && flatRateDenom) { return { amount: flatRate.amount, denom: flatRateDenom } - } else if (percentRate) { + } + + if (percentRate && marketplaceAmount) { const percentAmount = parseFloat(percentRate.percent) * floatMarketplaceAmount return { amount: percentAmount.toFixed(0), denom: marketplaceState!.latestSaleState.coin_denom } } + + return undefined; }
66-66: Remove console.log statementDebug statements should not be committed to production code.
- console.log(totalAmount)
65-65: Add validation for totalAmount calculationThe totalAmount calculation should handle edge cases where rateCoin is undefined but rateType is "additive".
- const totalAmount = (rateCoin && rateType === "additive") ? sumCoins([rateCoin, marketplaceCoin]) : [marketplaceCoin]; + const totalAmount = (rateCoin && rateType === "additive") + ? sumCoins([rateCoin, marketplaceCoin]) + : marketplaceCoin ? [marketplaceCoin] : [];
78-79: Remove debug console.log statementsDebug statements should not be committed to production code.
- console.log(JSON.stringify(msg)); - console.log("DENOM:", DENOM);
94-159: Consider extracting reusable components and consolidating stylesThe price display section contains repeated UI patterns that could be extracted into reusable components.
Consider creating these components:
type PriceRowProps = { label: string; amount: string; denom: string; prefix?: string; withBorder?: boolean; }; const PriceRow: FC<PriceRowProps> = ({ label, amount, denom, prefix = "", withBorder = false }) => ( <> <Box w="100%" h="8" padding={1} textAlign="start" fontSize="3-xl" marginBottom={1} borderBottom={withBorder ? "1px" : undefined} borderBottomColor={withBorder ? "blackAlpha.300" : undefined} > <Text textStyle="light"> <b>{label}</b> </Text> </Box> <Box w="100%" h="8" padding={1} textAlign="end" marginBottom={2} borderBottom={withBorder ? "1px" : undefined} borderBottomColor={withBorder ? "blackAlpha.300" : undefined} > <Text textStyle="light"> {prefix}{amount} {denom} </Text> </Box> </> );This would simplify the template and make it more maintainable.
174-185: Add input validation and improve type safety in sumCoins utilityThe utility function could benefit from input validation and more specific return types.
- export const sumCoins = (coins: Coin[]) => { + export const sumCoins = (coins: Coin[]): Coin[] | undefined => { + if (!Array.isArray(coins)) { + throw new Error('Input must be an array of Coins'); + } + if (coins.length === 0) return undefined; const mapped_coins: Record<string, Coin> = {}; coins.forEach(coin => { + if (!coin.denom || !coin.amount) { + throw new Error('Invalid coin format'); + } if (!mapped_coins[coin.denom]) { mapped_coins[coin.denom] = coin; } else { mapped_coins[coin.denom] = addCoins(mapped_coins[coin.denom], coin); } }) return Object.values(mapped_coins); };src/lib/andrjs/hooks/ado/useGetRate.ts (1)
28-40: SetstaleTimeinuseQueryto optimize data fetchingSince rates retrieved during active NFT sales are fixed and do not change frequently, setting a 5-minute
staleTimefor the rate data is acceptable and can improve performance by reducing unnecessary network requests.Apply this diff to set the
staleTimeoption:return useQuery({ queryKey: ["ado", "rate", address, action, client?.isConnected], enabled: !!client?.isConnected, queryFn: async () => { // existing code }, + staleTime: 5 * 60 * 1000, // 5 minutes })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/lib/andrjs/hooks/ado/useGetRate.ts(1 hunks)src/lib/react-query/client.ts(1 hunks)src/modules/modals/components/BuyNowModal.tsx(4 hunks)src/utils/string.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/string.ts
🧰 Additional context used
📓 Learnings (2)
src/lib/andrjs/hooks/ado/useGetRate.ts (2)
Learnt from: leifasorensen
PR: andromedaprotocol/embeddable-marketplace-demo#21
File: src/lib/react-query/client.ts:6-10
Timestamp: 2024-11-27T13:44:13.134Z
Learning: In `src/lib/andrjs/hooks/ado/useGetRate.ts`, rates retrieved during active NFT sales are fixed and do not change; therefore, a 5-minute stale time for rate data is acceptable.
Learnt from: leifasorensen
PR: andromedaprotocol/embeddable-marketplace-demo#21
File: src/lib/andrjs/hooks/ado/useGetRate.ts:4-18
Timestamp: 2024-11-27T13:55:46.048Z
Learning: In `src/lib/andrjs/hooks/ado/useGetRate.ts`, the `Rate` interface's `rate_type` value is enforced by the contract as required and cannot be empty. Undefined values for both `flat` and `percent` in `value` would only occur due to data retrieval or reference failures; therefore, handling dual `undefined` values is unnecessary.
src/lib/react-query/client.ts (1)
Learnt from: leifasorensen
PR: andromedaprotocol/embeddable-marketplace-demo#21
File: src/lib/react-query/client.ts:6-10
Timestamp: 2024-11-27T13:44:13.134Z
Learning: In `src/lib/andrjs/hooks/ado/useGetRate.ts`, rates retrieved during active NFT sales are fixed and do not change; therefore, a 5-minute stale time for rate data is acceptable.
🔇 Additional comments (3)
src/lib/react-query/client.ts (1)
33-33: LGTM!
The export statement is correctly implemented.
src/lib/andrjs/hooks/ado/useGetRate.ts (2)
7-19:
Correct the TypeScript syntax for the union type in the value property
The current syntax for the union type in the value property may not be valid TypeScript. To properly define a union within an object property, you should wrap the union types in parentheses directly after the property key. Here's how you can correct it:
Apply this diff to fix the syntax:
export interface Rate {
"local": {
"rate_type": "additive" | "deductive",
- "value":
- {
- "flat": {
- "denom": string,
- "amount": string
- }
- }
- |
- {
- "percent": {
- "percent": string
- }
- },
+ "value": (
+ {
+ "flat": {
+ "denom": string,
+ "amount": string
+ }
+ }
+ |
+ {
+ "percent": {
+ "percent": string
+ }
+ }
+ ),
"description": string
}
}⛔ Skipped due to learnings
Learnt from: leifasorensen
PR: andromedaprotocol/embeddable-marketplace-demo#21
File: src/lib/andrjs/hooks/ado/useGetRate.ts:4-18
Timestamp: 2024-11-27T13:55:46.048Z
Learning: In `src/lib/andrjs/hooks/ado/useGetRate.ts`, the `Rate` interface's `rate_type` value is enforced by the contract as required and cannot be empty. Undefined values for both `flat` and `percent` in `value` would only occur due to data retrieval or reference failures; therefore, handling dual `undefined` values is unnecessary.
24-43:
Add error handling for undefined client to prevent runtime errors
Currently, the function uses non-null assertions (!), assuming that client, chainClient, and queryClient are always defined. To prevent potential runtime errors, add checks for these properties and handle cases where they might be undefined.
Apply this diff to enhance error handling:
export function useGetRate(address: string, action: string) {
const client = useAndromedaClient();
return useQuery({
queryKey: ["ado", "rate", address, action, client?.isConnected],
enabled: !!client?.isConnected,
queryFn: async () => {
+ if (!client?.chainClient?.queryClient) {
+ throw new Error("Client is not initialized");
+ }
const rate = await client.chainClient.queryClient.queryContractSmart(address, {
"rates": {
"action": action
}
})
return rate as Rate;
}
})
}Likely invalid or redundant comment.
Motivation
Rates added in the marketplace did not show up during purchase of NFT.
Implementation
Created a hook "useGetRate" to get the data of the rate (like rateType, value,description) and used it in the BuyNowModal. Imported sumCoins function to add the coins.
Created function named shortenString to shorten the length of error.
Show rates in BuyNowModal.
Summary by CodeRabbit
Release Notes
New Features
useGetRate, for fetching rate information.Bug Fixes
Documentation
Chores