Fix Campaign Issues #341
Fix Campaign Issues #341
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update enhances the campaign management system with improved type safety and new functionalities for donation processing. Changes include using a unified Changes
Sequence Diagram(s)sequenceDiagram
participant CS as CampaignSettings UI
participant Hook as DonationProcessingHook
participant Client as Campaign Client
participant API as Contract API
CS->>Hook: Initialize with campaignId
Hook->>Client: Invoke view functions (hasEscrowedDonations / canProcessRefunds)
Client->>API: Execute contract API view call
API-->>Client: Return status data
Client-->>Hook: Relay processing data
Hook-->>CS: Update UI with donation/refund status
sequenceDiagram
participant CS as CampaignSettings UI
participant Hook as useCampaignForm
participant Client as Campaign Client
participant API as Contract API
CS->>Hook: Trigger donation/refund processing
Hook->>Client: Call process_escrowed_donations_batch / process_refunds_batch with campaignId
Client->>API: Submit contract call with specific gas limits
API-->>Client: Return transaction response
Client-->>Hook: Provide operation result
Hook-->>CS: Update UI with processing outcome
Poem
✨ Finishing Touches
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: 2
🧹 Nitpick comments (7)
src/entities/campaign/components/CampaignCard.tsx (1)
23-29: Add descriptive alt text for campaign image.The current empty alt text (
alt="") should be replaced with a meaningful description of the campaign cover image to improve accessibility.- alt="" + alt={`Cover image for ${data.name} campaign`}src/entities/campaign/components/CampaignBanner.tsx (2)
30-32: Enhance error handling with more context and consistent styling.The current error handling could be more informative and maintain visual consistency with the loading state.
Consider this improvement:
- if (campaignLoadingError) { - return <h1>Error Loading Campaign</h1>; - } + if (campaignLoadingError) { + return ( + <div className="flex h-40 items-center justify-center"> + <div className="text-center"> + <h1 className="text-lg font-semibold text-red-600">Error Loading Campaign</h1> + <p className="text-sm text-gray-600">Please try again later</p> + </div> + </div> + ); + }
54-54: Clean up redundant text color classes.The text color classes could be simplified as
text-foregroundis redundant whentext-whiteis applied.Consider this cleanup:
- <h1 className="text-[24px] font-bold text-white">{campaign?.name}</h1> + <h1 className="text-2xl font-bold text-white">{campaign?.name}</h1> <div className={cn( - "text-foreground flex flex-col items-start gap-2 p-0 text-[12px] text-white", + "flex flex-col items-start gap-2 p-0 text-xs text-white", "md:flex-row md:items-center md:text-[15px]", )}Also applies to: 58-59
src/common/contracts/core/campaigns/client.ts (1)
36-45: Consider extracting hardcoded gas limit to a constant.The gas limit "300000000000000" is repeated across multiple functions. Consider extracting it to a named constant for better maintainability and to ensure consistency.
+const DEFAULT_GAS_LIMIT = "300000000000000"; + export const process_escrowed_donations_batch = ({ args, }: { args: { campaign_id: CampaignId }; }) => { return contractApi.call("process_escrowed_donations_batch", { args, - gas: "300000000000000", + gas: DEFAULT_GAS_LIMIT, }); };Also applies to: 51-56
src/entities/campaign/components/CampaignSettings.tsx (2)
63-78: Consider extracting complex conditions into helper functions.While the logic is correct, the enable conditions could be more readable if extracted:
+const canProcessEscrowedDonations = (campaign?: Campaign) => + !!campaign?.min_amount && + Number(campaign?.total_raised_amount) >= Number(campaign?.min_amount); + +const canProcessRefunds = (campaign?: Campaign) => + !!campaign?.end_ms && + campaign?.end_ms < Temporal.Now.instant().epochMilliseconds && + Number(campaign?.total_raised_amount) < Number(campaign?.min_amount); + const { data: hasEscrowedDonations } = campaignsContractHooks.useHasEscrowedDonationsToProcess({ campaignId, - enabled: - !!campaign?.min_amount && - Number(campaign?.total_raised_amount) >= Number(campaign?.min_amount), + enabled: canProcessEscrowedDonations(campaign), }); const { data: isDonationRefundsProcessed } = campaignsContractHooks.useIsDonationRefundsProcessed({ campaignId, - enabled: - !!campaign?.end_ms && - campaign?.end_ms < Temporal.Now.instant().epochMilliseconds && - Number(campaign?.total_raised_amount) < Number(campaign?.min_amount), + enabled: canProcessRefunds(campaign), });
124-134: Consider adding loading states to the donation processing buttons.The implementation is good, but could be enhanced with loading states during processing:
-<Button onClick={handleProcessEscrowedDonations}>Process Donation</Button> +<Button + onClick={handleProcessEscrowedDonations} + loading={isProcessingDonations} + disabled={isProcessingDonations} +> + {isProcessingDonations ? 'Processing...' : 'Process Donation'} +</Button>src/entities/campaign/components/CampaignForm.tsx (1)
36-63: Consider adding error handling for amount conversions.While the initialization logic is correct, it could benefit from error handling:
+const safeYoctoNearToFloat = (amount?: string) => { + try { + return amount ? yoctoNearToFloat(amount) : undefined; + } catch (error) { + console.error('Invalid amount format:', error); + return undefined; + } +}; useEffect(() => { if (isUpdate && existingData) { setCoverImage(existingData?.cover_image_url); form.setValue("cover_image_url", existingData?.cover_image_url); form.setValue("recipient", existingData?.recipient); form.setValue("name", existingData?.name); form.setValue("description", existingData.description); - form.setValue("target_amount", yoctoNearToFloat(existingData?.target_amount)); + form.setValue("target_amount", safeYoctoNearToFloat(existingData?.target_amount)); if (existingData.min_amount != undefined) { - form.setValue("min_amount", yoctoNearToFloat(existingData.min_amount)); + form.setValue("min_amount", safeYoctoNearToFloat(existingData.min_amount)); } if (existingData.max_amount != undefined) { - form.setValue("max_amount", yoctoNearToFloat(existingData.max_amount)); + form.setValue("max_amount", safeYoctoNearToFloat(existingData.max_amount)); } } }, [isUpdate, existingData, form]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/common/contracts/core/campaigns/client.ts(4 hunks)src/common/contracts/core/campaigns/hooks.ts(1 hunks)src/entities/campaign/components/CampaignBanner.tsx(4 hunks)src/entities/campaign/components/CampaignCard.tsx(1 hunks)src/entities/campaign/components/CampaignForm.tsx(2 hunks)src/entities/campaign/components/CampaignSettings.tsx(5 hunks)src/entities/campaign/components/CampaignsList.tsx(1 hunks)src/entities/campaign/hooks/forms.ts(4 hunks)src/features/donation/components/DonationDirectAllocation.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/entities/campaign/components/CampaignsList.tsx
🔇 Additional comments (9)
src/entities/campaign/components/CampaignCard.tsx (1)
20-20: LGTM! Width behavior change improves layout flexibility.The change from
md:w-104tomd:min-w-104allows the card to grow beyond 104 units while maintaining a minimum width, which can better accommodate varying content lengths and responsive layouts.src/entities/campaign/components/CampaignBanner.tsx (2)
7-7: LGTM! Clean import addition.The Spinner component is correctly imported alongside SocialsShare from the common UI components.
40-44: LGTM! Well-implemented loading state.The loading state is well-structured with proper centering and consistent sizing. The Spinner provides a better user experience compared to a text-only loading indicator.
src/common/contracts/core/campaigns/hooks.ts (1)
46-60: LGTM! Well-structured hooks for campaign donation processing.The new hooks follow consistent patterns, properly handle client-side checks, and maintain type safety.
Also applies to: 62-76
src/entities/campaign/hooks/forms.ts (1)
79-81: Consider timezone handling in time conversion.The current implementation might be affected by timezone differences. Consider using UTC or explicitly handling timezones.
const timeToMilliseconds = (time: string) => { - return new Date(time).getTime(); + return new Date(time + 'Z').getTime(); // Ensure UTC timezone };src/features/donation/components/DonationDirectAllocation.tsx (1)
70-71: LGTM! Improved null safety in USD value calculation.The updated implementation properly handles undefined values for both amount and token price.
src/entities/campaign/components/CampaignSettings.tsx (2)
4-4: LGTM! Well-structured imports and hooks setup.The additions enhance the component's functionality with:
- Reliable timestamp handling using Temporal
- Loading state management with Spinner
- Donation processing hooks
Also applies to: 9-9, 15-15, 53-53
80-91: LGTM! Proper error and loading state handling.Good implementation of:
- Clear error message for non-existent campaigns
- Visual loading indicator with Spinner
src/entities/campaign/components/CampaignForm.tsx (1)
20-35: LGTM! Well-structured component props and initialization.Good improvements:
- Clear prop types with optional campaignId
- Simplified update mode check
- Proper hook initialization
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/entities/campaign/hooks/forms.ts (2)
146-148: Consider consolidating duplicate toast notifications.The success message is shown twice - once via toast and once via Redux dispatch.
Consider removing the Redux dispatch since we're moving towards using toasts for notifications:
- dispatch.campaignEditor.updateCampaignModalState({ - header: `You've successfully updated this Campaign`, - description: - "If you are not a member of the project, the campaign will be considered unofficial until it has been approved by the project.", - type: CampaignEnumType.UPDATE_CAMPAIGN, - });Also applies to: 163-180
189-203: Fix error message in create campaign error handler.The console error message incorrectly states "Failed to update Campaign" when it should be "Failed to create Campaign".
- console.error("Failed to update Campaign:", error); + console.error("Failed to create Campaign:", error);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/entities/campaign/hooks/forms.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/entities/campaign/hooks/forms.ts (2)
Learnt from: carina-akaia
PR: PotLock/potlock-nextjs-app#341
File: src/entities/campaign/hooks/forms.ts:94-116
Timestamp: 2025-02-14T13:55:21.902Z
Learning: The toast component from shadcn/ui is imported from "@/hooks/use-toast". For success messages, use a simple toast with description. For error states, use the "destructive" variant.
Learnt from: carina-akaia
PR: PotLock/potlock-nextjs-app#341
File: src/entities/campaign/hooks/forms.ts:94-116
Timestamp: 2025-02-14T13:55:21.902Z
Learning: The toast component from shadcn/ui is used via the useToast() hook which returns a toast() function. The toast() function accepts an object with title, description, and variant properties, rather than having separate success/error methods.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Fleek - broad-haze-5260
🔇 Additional comments (5)
src/entities/campaign/hooks/forms.ts (5)
8-10: LGTM! Clean import organization.The imports are well-organized, with clear separation between utility functions, types, and UI components.
17-17: LGTM! Type-safe parameter handling.The hook now accepts an optional
campaignIdparameter with proper TypeScript typing usingCampaignId, improving type safety.
80-82: Consider edge cases in time conversion.The current implementation might not handle invalid date strings gracefully.
Consider adding validation:
const timeToMilliseconds = (time: string) => { + const date = new Date(time); + if (isNaN(date.getTime())) { + throw new Error('Invalid date string provided'); + } return date.getTime(); };
95-115: LGTM! Well-implemented donation processing.The function follows the established patterns:
- Uses promise chains instead of try/catch
- Implements proper error handling with toast notifications
- Includes console logging for debugging
117-137: LGTM! Consistent refund processing implementation.The refund processing follows the same pattern as donation processing, maintaining consistency in the codebase.
* Reimplement wallet bindings & Introduce performance optimizations (#332) Co-authored-by: Jiku Godwill Nsanwi <jikugodwill@yahoo.com> Co-authored-by: Ebube111 <ebubeagwaze@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Account for natural precision loss in matching pool distribution (#339) * fix: Unlock donation submit with default value (#340) * Implement payout justification publishing (#342) * Fix Campaign Issues (#341) * campaign changes done * implemented suggested changes --------- Co-authored-by: Carina.Akaia.io <cvo.akaia@gmail.com> Co-authored-by: Jiku Godwill Nsanwi <jikugodwill@yahoo.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Style