Build campaign detail and dashboard MVP - #40
Conversation
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThis PR adds production API infrastructure, signed admin sessions, campaign and donation administration, idempotent Midtrans checkout, a session-based dashboard, public campaign flows, SEO edge functions, deployment automation, CI, migrations, and operational documentation. ChangesBackend platform and fundraising operations
Dashboard and public landing experiences
Deployment and operations
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
kebaikanku-landing | 98c1a30 | Aug 01 2026, 08:32 AM |
There was a problem hiding this comment.
Code Review
This pull request introduces campaign updating capabilities, expands the campaign domain model with additional metadata fields (such as subcategory, banner URL, location, and beneficiary notes), and adds support for anonymous donations. On the frontend, the dashboard has been refactored into a tabbed layout with clean routing, and the landing page now features a dedicated campaign detail page with a multi-step donation flow. Feedback on these changes focuses on improving robustness: specifically, checking RowsAffected on GORM updates and handling specific database errors (like not found or duplicate slug) in the API; validating campaign end dates and donation limits in the frontend to prevent crashes or invalid submissions; implementing a more robust CSV parser to handle quoted commas; and properly formatting zero or invalid dates.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fb086d254
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
frontend/dashboard/src/routes/+page.svelte (1)
2-4: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winPrefer a route
loadredirect overonMountnavigation.This redirect is client-only and runs after hydration. A load-level redirect avoids flash and works in non-JS scenarios.
♻️ Suggested change
-<script> - import { goto } from '$app/navigation'; - import { onMount } from 'svelte'; - onMount(() => goto('/campaigns', { replaceState: true })); -</script> +<!-- redirect handled in +page.js -->// frontend/dashboard/src/routes/+page.js import { redirect } from '`@sveltejs/kit`'; export const load = () => { throw redirect(307, '/campaigns'); };🤖 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 `@frontend/dashboard/src/routes/`+page.svelte around lines 2 - 4, Remove the onMount hook and goto import from the current +page.svelte file, then create a new +page.js file in the same route directory. In the +page.js file, import redirect from `@sveltejs/kit` and export a load function that throws a redirect with status code 307 to the '/campaigns' path. This server-level redirect avoids the client-side flash and works in non-JavaScript scenarios.
🤖 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 `@backend/cmd/api/main.go`:
- Around line 260-263: The UpdateCampaign call in this handler only checks for
database errors but does not verify that the update actually affected any rows.
When a campaign with the given ID does not exist, GORM returns nil error but
RowsAffected is 0, causing the handler to return HTTP 200 with success:true.
Modify the appStore.UpdateCampaign method (or its caller) to check the
RowsAffected value and return an appropriate 404 Not Found error when no rows
are affected, indicating the campaign ID does not exist.
In `@backend/internal/repository/store.go`:
- Around line 53-67: The UpdateCampaign method in the Store struct returns all
database errors generically without distinguishing duplicate slug constraint
violations. Modify the UpdateCampaign method to check if the returned error from
the database operation is a duplicate key error (similar to how
handleCreateCampaign does with isDuplicateDBError), and wrap the error
appropriately so that the handler can detect and return a 409 DUPLICATE_CAMPAIGN
response instead of a generic 500 error. This ensures consistent error handling
between campaign creation and update operations.
In `@frontend/dashboard/src/lib/Dashboard.svelte`:
- Around line 29-35: The saveToken function clears the admin token when it is
removed but does not clear the cached donations data, leaving donor information
visible in the UI after logout. In the else branch of the saveToken function
where sessionStorage.removeItem is called for the token, also clear the
donations data structure by resetting it to an empty state or null to ensure no
donor data remains accessible after the token is removed.
- Line 144: In the Dashboard.svelte file, locate the preview link in the actions
div that opens to an external URL with
href="{landingBase}/campaigns/{item.slug}" and target="_blank". This external
link is missing the rel attribute, which creates a security vulnerability. Add
rel="noopener noreferrer" to this anchor tag to prevent reverse-tabnabbing
attacks and ensure the opened tab cannot access the window.opener property.
- Around line 82-84: The fillCampaign function currently converts the end_date
to UTC ISO format using toISOString().slice(0, 16), but datetime-local inputs
require local time values adjusted by the client's timezone offset. Create a new
helper function called toDatetimeLocal() that takes a UTC date string and
adjusts it by the client's timezone offset before formatting to the
datetime-local format (YYYY-MM-DDTHH:mm), then replace the current
toISOString().slice(0, 16) call in the fillCampaign function with a call to this
new toDatetimeLocal() function to ensure the displayed deadline is correct
regardless of the user's timezone.
In `@frontend/landing/src/routes/campaigns/`[slug]/+page.svelte:
- Line 21: The submitDonation function hardcodes the payment_method value as
'midtrans_snap' in the request body instead of using the user-selected value
from the form state. Replace the hardcoded 'midtrans_snap' string with the
form.payment_method variable (which the form tracks and allows users to select
via dropdown) so that the user's payment method selection is actually sent to
the backend.
- Around line 94-97: The nextStep() function only validates the minimum donation
amount (Rp 2.000) but does not validate the maximum limit of Rp 100.000.000 that
is referenced elsewhere in the code. Add an additional validation condition in
the step === 1 check to verify that form.amount does not exceed 100000000, and
set the donationError to an appropriate message if the maximum limit is
exceeded, similar to how the minimum validation is currently implemented.
---
Nitpick comments:
In `@frontend/dashboard/src/routes/`+page.svelte:
- Around line 2-4: Remove the onMount hook and goto import from the current
+page.svelte file, then create a new +page.js file in the same route directory.
In the +page.js file, import redirect from `@sveltejs/kit` and export a load
function that throws a redirect with status code 307 to the '/campaigns' path.
This server-level redirect avoids the client-side flash and works in
non-JavaScript scenarios.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eae32de2-1a24-4acf-84f6-280d22df3671
📒 Files selected for processing (14)
backend/cmd/api/main.gobackend/internal/domain/campaign.gobackend/internal/repository/store.gofrontend/dashboard/src/lib/Dashboard.sveltefrontend/dashboard/src/routes/+page.sveltefrontend/dashboard/src/routes/campaign/form/+page.sveltefrontend/dashboard/src/routes/campaigns/+page.sveltefrontend/dashboard/src/routes/donations/+page.sveltefrontend/dashboard/src/routes/settings/+page.sveltefrontend/dashboard/vite.config.jsfrontend/landing/src/routes/campaigns/+page.sveltefrontend/landing/src/routes/campaigns/[slug]/+page.jsfrontend/landing/src/routes/campaigns/[slug]/+page.sveltefrontend/landing/vite.config.js
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/api.md (1)
111-122: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument
must_change_passwordin the authentication responses.Both login and session responses include this field in the implementation, but the documented response omits it. Include the field so the dashboard can handle the generated-password path correctly.
🤖 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 `@docs/api.md` around lines 111 - 122, Update the documented authentication response examples for login and GET /api/v1/admin/session to include the implementation’s must_change_password field, preserving the existing success and authenticated fields so the dashboard can handle generated-password sessions.frontend/landing/src/routes/+page.svelte (1)
122-134: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
stats.traditional_*was repurposed from "competitor problem" copy to neutral public-donation copy, but the card that renders it still uses warning styling. The comparison section now reads as "Public Donations / Active Campaigns" in a red alert card next to the emerald self-hosted card, which misleads readers.
frontend/landing/src/routes/+page.svelte#L122-L134: restyle this card to informational (emerald/teal accent, non-warning icon), or point it at appropriately named keys.frontend/landing/src/lib/locales/en.json#L24-L26: renametraditional_title/traditional_fee/traditional_descto reflect the new meaning (e.g.public_title/public_highlight/public_desc).frontend/landing/src/lib/locales/id.json#L24-L25: apply the matching rename so both locales stay in sync.🤖 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 `@frontend/landing/src/routes/`+page.svelte around lines 122 - 134, Rename the traditional_title/traditional_fee/traditional_desc translation keys to public_title/public_highlight/public_desc in both frontend/landing/src/lib/locales/en.json (24-26) and frontend/landing/src/lib/locales/id.json (24-25), then update the comparison card in frontend/landing/src/routes/+page.svelte (122-134) to use the renamed keys and informational emerald/teal styling with a non-warning icon.backend/cmd/api/main.go (1)
608-636: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winCSV export is vulnerable to formula injection.
campaignTitle,donorName, andprovider_statusare attacker-influenced free text; a value beginning with=,+,-, or@executes as a formula when admins opendonations.csvin Excel/Sheets. Prefix such fields with'(or a leading tab) before writing.🤖 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 `@backend/cmd/api/main.go` around lines 608 - 636, Sanitize attacker-controlled CSV text fields before writing them in the donations export: apply formula-injection protection to campaignTitle, donorName, and donation.ProviderStatus when constructing the row, prefixing values beginning with =, +, -, or @ with a single quote. Keep IDs, numeric values, statuses, and timestamps unchanged.
🟡 Minor comments (12)
frontend/dashboard/src/lib/Dashboard.svelte-461-461 (1)
461-461: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCard view omits “Selesaikan” for paused campaigns.
The table view (Line 459) offers both
AktifkanandSelesaikanforpaused, but the card view only offersAktifkan, so mobile/card users can't complete a paused campaign.🐛 Suggested fix
-{:else if item.status === 'paused'}<button class="text-link action-link" onclick={() => updateCampaignStatus(item, 'active')}>Aktifkan</button>{/if} +{:else if item.status === 'paused'}<button class="text-link action-link" onclick={() => updateCampaignStatus(item, 'active')}>Aktifkan</button><button class="text-link action-link" onclick={() => updateCampaignStatus(item, 'completed')}>Selesaikan</button>{/if}🤖 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 `@frontend/dashboard/src/lib/Dashboard.svelte` at line 461, Update the card-view action conditions in the campaign markup around visibleCampaigns so paused campaigns render both Aktifkan and Selesaikan actions, matching the table view. Keep the existing active-campaign actions and status-update calls unchanged.frontend/dashboard/src/lib/Dashboard.svelte-248-270 (1)
248-270: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUpload has no timeout or abort path.
The XHR has no
timeout/ontimeouthandler, so a stalled upload leavesuploading = trueand the progress bar forever, with no way to retry. Also worth resetting the file input on failure so the same file can be re-selected.🛠️ Suggested change
const request = new XMLHttpRequest(); request.open('POST', `${apiBase}/api/v1/admin/uploads`); request.withCredentials = true; + request.timeout = 60000; + request.ontimeout = () => { uploading = false; uploadError = 'Unggahan gambar melebihi batas waktu. Coba lagi.'; };🤖 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 `@frontend/dashboard/src/lib/Dashboard.svelte` around lines 248 - 270, Update uploadBanner to configure an XMLHttpRequest timeout and handle ontimeout by stopping the upload, resetting uploading and uploadProgress, and showing a retryable uploadError. Ensure failure paths, including timeout and network errors, reset the associated file input so the same file can be selected again..env.production.example-37-37 (1)
37-37: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPoint waitlist emails to the waitlist page.
The waitlist function uses
WAITLIST_EMAIL_URLdirectly, while its fallback is/coming-soon. This example sets the URL to the site root, so confirmation emails may send users to the wrong page. Set it to the intended/coming-soonroute or confirm the root URL is deliberate.🤖 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 @.env.production.example at line 37, Update the WAITLIST_EMAIL_URL example value to the intended /coming-soon route so waitlist confirmation emails point to the waitlist page.docs/database.md-156-166 (1)
156-166: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd
000004_admin_payment_settingsto the tracked baseline.The production migration set includes the payment-settings migration, but this list stops at
000003. Keep the documented schema baseline aligned withbackend/migrationsso deployment and rollback checks do not miss it.🤖 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 `@docs/database.md` around lines 156 - 166, Update the “Current tracked schema version” list in the database documentation to include 000004_admin_payment_settings after 000003_donation_checkout_idempotency, preserving the existing migration order and production baseline guidance.README.md-25-29 (1)
25-29: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSynchronize release and backend documentation with the implemented platform.
The current status and API inventory still describe parts of the old bearer-token/MVP state.
README.md#L25-L29: remove Midtrans integration and CI from the pending checklist, or rewrite them as completed.backend/README.md#L11-L13: document the signed single-admin session flow and retain only multi-institution auth as future work.backend/README.md#L114-L124: add the current admin donations, upload, and payment-settings endpoints or link to a complete canonical inventory.🤖 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 `@README.md` around lines 25 - 29, Synchronize the release documentation: in README.md lines 25-29, remove or mark Midtrans integration and CI checks as completed; in backend/README.md lines 11-13, document the signed single-admin session flow and leave only multi-institution authentication as future work; in backend/README.md lines 114-124, add the current admin donations, upload, and payment-settings endpoints or link to a complete canonical API inventory.frontend/landing/src/lib/locales/id.json-26-26 (1)
26-26: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMixed terminology: "campaign" vs "kampanye" in the Indonesian locale. The rest of
id.jsonconsistently uses "kampanye" (lines 116-117, 137-138). Use "kampanye" here for consistency.✏️ Suggested wording
- "traditional_desc": "Pilih campaign yang tersedia dan lanjutkan pembayaran melalui halaman aman Midtrans.", + "traditional_desc": "Pilih kampanye yang tersedia dan lanjutkan pembayaran melalui halaman aman Midtrans.", ... - "subtitle": "Fitur inti untuk campaign dan donasi publik tersedia. Otomatisasi lanjutan ditandai sebagai roadmap.", + "subtitle": "Fitur inti untuk kampanye dan donasi publik tersedia. Otomatisasi lanjutan ditandai sebagai roadmap.",Also applies to: 37-37
🤖 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 `@frontend/landing/src/lib/locales/id.json` at line 26, Update the Indonesian locale entries traditional_desc at both referenced locations to use “kampanye” instead of “campaign”, preserving the rest of each translation unchanged.frontend/landing/src/routes/+page.svelte-343-343 (1)
343-343: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign FAQ accordion labels with their control index
faq.q3/faq.a3exist, but the second accordion still usestoggleFaq(2)/openFaq === 2, leavingfaq.q2/faq.a2unused and making the markup easier to miswire. Either update the control/indexes to 3 and remove unused keys, or keep the current indexes and restore the missingq2/a2strings.🤖 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 `@frontend/landing/src/routes/`+page.svelte at line 343, Align the second FAQ accordion’s label, answer, and control state by updating its toggleFaq and openFaq references to index 3 so they use faq.q3/faq.a3 consistently, or alternatively restore the missing faq.q2/a2 entries while retaining index 2. Ensure no FAQ keys remain unused or mismatched.frontend/landing/src/routes/+page.svelte-405-412 (1)
405-412: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake the reduced-motion reset global. Svelte scopes component
<style>selectors by default, so this*, *::before, *::afterrule only covers this page component’s elements and won’t suppress animations defined outside it. Use:global(*), :global(*::before), :global(*::after)here, or move it tofrontend/landing/src/routes/layout.css.🤖 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 `@frontend/landing/src/routes/`+page.svelte around lines 405 - 412, Update the prefers-reduced-motion rule in the page component’s style block to use global selectors for the element and pseudo-elements, ensuring the animation, transition, and scroll-behavior reset applies across the entire application. Keep the existing reset declarations unchanged.frontend/landing/src/routes/+layout.svelte-324-341 (1)
324-341: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winBottom nav labels bypass i18n. "Beranda / Kampanye / Biaya / FAQ" and
aria-label="Navigasi utama"are hardcoded Indonesian while the site ships a locale switcher; keys already exist (nav.campaigns,nav.pricing,nav.faq).♻️ Use the existing translation keys
- <nav aria-label="Navigasi utama" class="fixed inset-x-3 ..."> + <nav aria-label={$t('footer.links_title')} class="fixed inset-x-3 ..."> ... - Beranda + {$t('nav.home')} ... - Kampanye + {$t('nav.campaigns')} ... - Biaya + {$t('nav.pricing')}(
nav.homeneeds adding to both locale files.)🤖 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 `@frontend/landing/src/routes/`+layout.svelte around lines 324 - 341, Replace the hardcoded bottom-navigation labels and nav aria-label in the navigation markup with the existing i18n keys for campaigns, pricing, and FAQ, and add the missing nav.home key to both locale files. Reuse the project’s established translation helper and preserve the current navigation structure and active-state behavior.frontend/landing/functions/api/v1/waitlist.js-12-15 (1)
12-15: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSize limit is bypassable via a spoofed/omitted
Content-Lengthheader.
bodySizetrusts the client-supplied header; a request with nocontent-length(or an understated one) skips this guard entirely, andrequest.json()will still buffer the full body regardless. Measure the actual payload instead of trusting the header.🛡️ Enforce the limit on the actual body
- const bodySize = Number(request.headers.get('content-length') || 0); - if (bodySize > MAX_BODY_BYTES) { - return jsonError(413, 'PAYLOAD_TOO_LARGE', 'Payload is too large.'); - } - - let payload; - try { - payload = await request.json(); - } catch { - return jsonError(400, 'INVALID_JSON', 'Payload must be valid JSON.'); - } + let rawBody; + try { + rawBody = await request.text(); + } catch { + return jsonError(400, 'INVALID_JSON', 'Payload must be valid JSON.'); + } + if (new TextEncoder().encode(rawBody).length > MAX_BODY_BYTES) { + return jsonError(413, 'PAYLOAD_TOO_LARGE', 'Payload is too large.'); + } + + let payload; + try { + payload = JSON.parse(rawBody); + } catch { + return jsonError(400, 'INVALID_JSON', 'Payload must be valid JSON.'); + }🤖 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 `@frontend/landing/functions/api/v1/waitlist.js` around lines 12 - 15, Update the request handling around the body-size check in the waitlist endpoint to enforce MAX_BODY_BYTES using the actual request body rather than the client-supplied content-length header. Read and measure the body before JSON parsing, reject oversized payloads with the existing jsonError(413, 'PAYLOAD_TOO_LARGE', ...) response, and preserve normal parsing for requests within the limit.backend/cmd/api/main.go-1330-1330 (1)
1330-1330: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
X-CSRIDlooks like a typo and no CSRF token is enforced.The allowed header is presumably meant to be
X-CSRF-Token; as written it permits a header nothing sends or validates. SinceAllowCredentialsis enabled with cookie auth, either wire a real CSRF token check or drop the header.🤖 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 `@backend/cmd/api/main.go` at line 1330, Update the CORS configuration containing AllowedHeaders to use the actual CSRF header name X-CSRF-Token only if the API validates that token; otherwise remove the incorrect X-CSRID entry. Ensure the setting matches the authentication and CSRF enforcement behavior while preserving the other allowed headers.backend/cmd/api/main.go-747-777 (1)
747-777: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReset path silently discards the submitted
mode.When
server_keyis empty the handler validatesreq.Modeagainst the environment keys and then deletes the override row entirely, so the effective mode falls back toappConfig.MidtransEnv. APUT {"mode":"production"}with sandbox env keys is either rejected (Line 764) or accepted while the effective mode remains whateverMIDTRANS_ENVsays — the caller's intent is dropped. Either persist a keyless override row carryingMode, or reject mode changes that cannot be honored.🤖 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 `@backend/cmd/api/main.go` around lines 747 - 777, Update handlePutPaymentSettings so a reset request with an empty ServerKey does not silently discard the submitted Mode: either persist a keyless payment-settings override carrying req.Mode, or reject the request when that mode cannot be honored; ensure validation against environment keys and the response’s effective payment mode remain consistent with the chosen behavior.
🧹 Nitpick comments (7)
frontend/dashboard/src/lib/Dashboard.svelte (1)
330-346: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unused
parseCSV.CSV export now uses a direct blob download, and
parseCSVhas no remaining call site in the repository.🤖 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 `@frontend/dashboard/src/lib/Dashboard.svelte` around lines 330 - 346, Remove the unused parseCSV function from Dashboard.svelte, leaving the direct blob-download CSV export implementation and surrounding code unchanged.docs/release-alpha.md (1)
90-96: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPoll for webhook completion before asserting export success.
The Midtrans notification can arrive after the payment redirect, so one immediate export request may legitimately show
pending. Add bounded retries against the donation status or export endpoint before failing the E2E check.🤖 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 `@docs/release-alpha.md` around lines 90 - 96, Update the donation export verification step after payment completion to poll the donation status or export endpoint with bounded retries before asserting success. Continue polling when the exported row is still pending, and fail after the retry limit unless status is success with provider_status settlement or capture.frontend/landing/src/routes/campaigns/+page.svelte (1)
81-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded Indonesian
aria-labelon a localized page. Every visible label here goes through$t(...); the group label stays Indonesian for English users. Also considerrole="group"so the label is actually exposed on thediv.♻️ Suggested tweak
- <div class="flex flex-wrap gap-2" aria-label="Filter kategori kampanye"> + <div class="flex flex-wrap gap-2" role="group" aria-label={$t('campaigns.filter_all')}>(or add a dedicated
campaigns.filter_group_labelkey to both locale files)🤖 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 `@frontend/landing/src/routes/campaigns/`+page.svelte at line 81, Update the filter container div’s aria-label to use the existing localization mechanism, or add and use a campaigns.filter_group_label translation key in both locale files, so it is localized for English users. Add role="group" to the same div so the localized label is exposed semantically.frontend/landing/src/lib/locales/en.json (1)
30-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
cloud_*keys are now dead. The Cloud comparison card and Managed Cloud pricing card were removed fromfrontend/landing/src/routes/+page.svelte; empty-string placeholders left behind will drift. Consider deletingcloud_title/cloud_fee/cloud_descfrom both locale files (also checkpricing.cloud_*and the unusedcampaigns.filter_*/select_first/pay_with_midtranskeys while you're in here).🤖 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 `@frontend/landing/src/lib/locales/en.json` around lines 30 - 32, Remove the unused cloud_title, cloud_fee, and cloud_desc locale keys from both locale files, including their pricing.cloud_* counterparts if present. Also remove the unused campaigns.filter_*, select_first, and pay_with_midtrans keys identified in the comment, without changing active translations.frontend/landing/src/routes/campaigns/[slug]/+page.svelte (1)
133-136: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
checkoutKeyisn't persisted, so an interrupted checkout loses idempotency protection.persistCheckout()stores onlyformandcheckoutURL. If the POST is in flight (or the tab reloads before the response lands), the next attempt mints a fresh UUID and the backend creates a second pending donation + Snap transaction for the same intent, defeating theIdempotency-Keycontract.♻️ Persist and restore the key
function persistCheckout() { if (typeof sessionStorage === 'undefined') return; - sessionStorage.setItem(`donation:${page.params.slug}`, JSON.stringify({ form, checkoutURL })); + sessionStorage.setItem(`donation:${page.params.slug}`, JSON.stringify({ form, checkoutURL, checkoutKey })); }and in
onMount:form = { ...form, ...state.form }; checkoutURL = state.checkoutURL || ''; + checkoutKey = state.checkoutKey || '';🤖 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 `@frontend/landing/src/routes/campaigns/`[slug]/+page.svelte around lines 133 - 136, Update persistCheckout() to include checkoutKey alongside form and checkoutURL in the sessionStorage payload, and update the onMount restore logic to read the persisted key back into checkoutKey before retrying checkout. Preserve the existing storage key and behavior for sessions without a saved checkoutKey.frontend/landing/functions/campaigns/[slug].js (1)
11-11: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueFallback responses share the same long CDN cache TTL as successful metadata injections.
Both the "no
apiBase"/API-error fallback and the successful campaign-metadata response gets-maxage=300. A transient backend outage could leave the generic shell cached at the edge for up to 5 minutes after the API recovers, delaying correct SEO tags reaching crawlers.Also applies to: 19-19, 23-25, 79-84
🤖 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 `@frontend/landing/functions/campaigns/`[slug].js at line 11, Update the fallback response paths in the campaign handler, including the no-apiBase branch, API-error handling, and related fallback returns, to use a short or non-cacheable CDN TTL instead of s-maxage=300. Preserve the 300-second cache duration for successful campaign-metadata responses.backend/migrations/000001_initial_schema.up.sql (1)
67-88: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider indexing
donations.statusfor the admin listing query.
ListDonationsPagefilters onstatus(and optionallycampaign_id) for the admin dashboard; without an index this becomes a full scan as donation volume grows.♻️ Suggested addition
CREATE INDEX IF NOT EXISTS idx_donations_provider_transaction_id ON donations (provider_transaction_id); +CREATE INDEX IF NOT EXISTS idx_donations_status_campaign_id ON donations (status, campaign_id);🤖 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 `@backend/migrations/000001_initial_schema.up.sql` around lines 67 - 88, Add an index for the donations.status column alongside the existing donations indexes in the initial schema migration, using the established index naming and IF NOT EXISTS convention.
🤖 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 @.env.production.example:
- Around line 7-8: Update the DB_DSN and MIGRATION_DATABASE_URL production
examples to require PostgreSQL TLS with certificate verification instead of
sslmode=disable. Use the repository’s established certificate configuration if
available, or document and enforce the trusted isolated-network model explicitly
when certificate verification cannot be configured.
In @.github/workflows/ci.yml:
- Line 18: Update every actions/checkout@v4 step in the workflow, including the
four checkout occurrences, to set persist-credentials to false. Keep the
existing checkout configuration unchanged otherwise.
In `@backend/cmd/api/main.go`:
- Around line 984-1011: Add dedicated brute-force protection to handleAdminLogin
instead of relying on the shared publicRateLimit: apply a substantially lower
per-IP limiter to this route and track failed password attempts to impose
increasing delays or temporary lockout after repeated failures. Ensure failed
authentication updates the protection state and successful login clears or
resets it, while preserving the existing response behavior.
- Around line 869-887: Refactor effectivePaymentClient and the payment request
paths to cache and reuse the resolved Midtrans client, avoiding repeated
GetPaymentSetting, decryption, and transport creation per request. Resolve the
payment setting/configuration only once per call, and invalidate or refresh the
cache when PUT /admin/settings/payment updates the setting. Preserve fallback
behavior when no payment setting exists and propagate configuration errors
unchanged.
- Around line 1272-1300: Update publicRateLimit to avoid sweeping
publicRequestWindow on every request: track the last expiry sweep and perform
the existing stale-entry cleanup at most once per publicRateLimitWindow while
holding publicRequestGuard. Keep rate-limit entry lookup, counting, and
enforcement unchanged, ensuring lastSweep is updated safely under the same
mutex.
In `@backend/cmd/bootstrap/main.go`:
- Around line 16-25: Update the bootstrap organization initialization to avoid
the guessable `PasswordHash: "bootstrap-managed"` value used by
`adminSessionSecret`; generate and persist a bcrypt hash of a cryptographically
random secret using the existing `database.SeedDefaults` approach, or fail
bootstrap when the required session secret configuration is absent.
In `@backend/Dockerfile`:
- Around line 13-18: Update the Docker runtime setup around the app user and API
startup so /data/uploads is created and writable by the non-root app user before
the API runs. Add a startup initialization step that creates the directory and
fixes its ownership or permissions, then launches the existing API entrypoint
while preserving the current USER app security boundary.
In `@backend/internal/database/database.go`:
- Around line 43-46: Align the production check in the database initialization
flow with the isProduction logic in main.go so both recognize “prod” and
case-insensitive “production” consistently. Prefer reusing a shared
production-detection helper, or normalize cfg.Env in config.Load, then use that
normalized/shared result before allowing GORM AutoMigrate.
In `@backend/migrations/000003_donation_checkout_idempotency.up.sql`:
- Around line 2-3: Ensure checkout_token and checkout_redirect_url are
compatible with non-pointer string fields in Domain.Donation: update
backend/migrations/000003_donation_checkout_idempotency.up.sql lines 2-3 to
backfill existing rows and enforce non-null values with suitable defaults, and
update backend/internal/domain/donation.go lines 17-18 only if choosing the
nullable-field approach instead. Keep all loading and export paths safe from
NULL scan failures.
In `@docker-compose.production.yml`:
- Around line 38-40: Update the ADMIN_PASSWORD configuration in the production
compose environment to require a non-empty value, matching the fail-closed
behavior of ADMIN_SESSION_SECRET and ADMIN_SETTINGS_ENCRYPTION_KEY. Replace the
permissive default in the ADMIN_PASSWORD entry with the required-variable form
and an appropriate setup message.
- Line 22: Update the migration service command configuration to stop
interpolating MIGRATION_DATABASE_URL, including its password, directly into
command-line arguments. Use the project’s supported secret or mounted credential
mechanism, such as a password file or wrapper that constructs the connection
securely at runtime, while preserving the existing migration path and up
operation.
In `@docs/deployment.md`:
- Around line 54-57: Complete the production environment checklist in the
deployment documentation by explicitly including PUBLIC_LANDING_URL,
MIGRATION_DATABASE_URL, and CORS_ALLOWED_ORIGINS, or document how the Compose
configuration derives each value. Ensure the guidance covers their required
production values and avoids localhost defaults or missing migration
configuration.
In `@frontend/landing/src/routes/`+layout.svelte:
- Around line 206-215: Restore mobile access to the language and theme toggles
and the donate CTA by surfacing them in a slot visible below the md breakpoint,
or by adding them to the bottom navigation alongside the existing section links.
Update the relevant actions block and bottom-nav markup in the layout while
preserving the current desktop controls and navigation behavior.
- Around line 86-100: Update the generic SEO metadata guard in the layout to use
the route-level metadata flag consistently, ensuring routes that provide their
own metadata—including /campaigns, /coming-soon, and static legal pages—skip the
layout’s generic tags and do not emit duplicate title or description metadata.
Adjust the logic that computes or consumes hasRouteMetadata rather than changing
the individual meta elements.
In `@frontend/landing/src/routes/campaigns/`[slug]/+page.svelte:
- Around line 103-122: Update the submission error handling around the response
parsing and catch block to preserve and display payload.error.message for
backend failures, especially validation errors such as VALIDATION_FAILED,
instead of throwing a message-less Error. Keep the existing
IDEMPOTENCY_PREVIOUSLY_FAILED recovery behavior unchanged and retain the generic
fallback when no usable backend message is available.
In `@frontend/landing/src/routes/payments/`[id]/+page.svelte:
- Around line 33-48: Add bounded exponential backoff to the refresh polling flow
in refresh, tracking attempts and stopping automatic retries after a finite cap
while preserving the existing “Periksa ulang” button as the manual escape hatch.
Reset the attempt counter in the manual retry click handler so a user-triggered
retry starts polling from the initial delay; keep pending payments polling only
while the cap has not been reached.
In `@scripts/backup-postgres.sh`:
- Around line 13-17: Protect generated database backups in the backup script by
setting a restrictive umask before creating the backup directory and ensuring
the directory is private (mode 700). Apply this around the existing mkdir and
pg_dump flow, preserving the current backup filename and dump behavior.
- Around line 14-17: Update the backup flow around BACKUP_FILE and pg_dump to
write into a temporary file in BACKUP_DIR, register cleanup for that temporary
path on exit, and rename it to BACKUP_FILE only after pg_dump succeeds. Preserve
the existing filename and ensure failed or interrupted dumps never leave a
partial final .dump file.
In `@scripts/restore-postgres.sh`:
- Around line 8-17: Update the restore procedure around the docker compose
invocation to enforce that the api service is stopped before running pg_restore.
Detect a running api container and abort with an error, or explicitly stop the
api service as part of the procedure, while preserving the existing confirmation
and restore validation checks.
---
Outside diff comments:
In `@backend/cmd/api/main.go`:
- Around line 608-636: Sanitize attacker-controlled CSV text fields before
writing them in the donations export: apply formula-injection protection to
campaignTitle, donorName, and donation.ProviderStatus when constructing the row,
prefixing values beginning with =, +, -, or @ with a single quote. Keep IDs,
numeric values, statuses, and timestamps unchanged.
In `@docs/api.md`:
- Around line 111-122: Update the documented authentication response examples
for login and GET /api/v1/admin/session to include the implementation’s
must_change_password field, preserving the existing success and authenticated
fields so the dashboard can handle generated-password sessions.
In `@frontend/landing/src/routes/`+page.svelte:
- Around line 122-134: Rename the
traditional_title/traditional_fee/traditional_desc translation keys to
public_title/public_highlight/public_desc in both
frontend/landing/src/lib/locales/en.json (24-26) and
frontend/landing/src/lib/locales/id.json (24-25), then update the comparison
card in frontend/landing/src/routes/+page.svelte (122-134) to use the renamed
keys and informational emerald/teal styling with a non-warning icon.
---
Minor comments:
In @.env.production.example:
- Line 37: Update the WAITLIST_EMAIL_URL example value to the intended
/coming-soon route so waitlist confirmation emails point to the waitlist page.
In `@backend/cmd/api/main.go`:
- Line 1330: Update the CORS configuration containing AllowedHeaders to use the
actual CSRF header name X-CSRF-Token only if the API validates that token;
otherwise remove the incorrect X-CSRID entry. Ensure the setting matches the
authentication and CSRF enforcement behavior while preserving the other allowed
headers.
- Around line 747-777: Update handlePutPaymentSettings so a reset request with
an empty ServerKey does not silently discard the submitted Mode: either persist
a keyless payment-settings override carrying req.Mode, or reject the request
when that mode cannot be honored; ensure validation against environment keys and
the response’s effective payment mode remain consistent with the chosen
behavior.
In `@docs/database.md`:
- Around line 156-166: Update the “Current tracked schema version” list in the
database documentation to include 000004_admin_payment_settings after
000003_donation_checkout_idempotency, preserving the existing migration order
and production baseline guidance.
In `@frontend/dashboard/src/lib/Dashboard.svelte`:
- Line 461: Update the card-view action conditions in the campaign markup around
visibleCampaigns so paused campaigns render both Aktifkan and Selesaikan
actions, matching the table view. Keep the existing active-campaign actions and
status-update calls unchanged.
- Around line 248-270: Update uploadBanner to configure an XMLHttpRequest
timeout and handle ontimeout by stopping the upload, resetting uploading and
uploadProgress, and showing a retryable uploadError. Ensure failure paths,
including timeout and network errors, reset the associated file input so the
same file can be selected again.
In `@frontend/landing/functions/api/v1/waitlist.js`:
- Around line 12-15: Update the request handling around the body-size check in
the waitlist endpoint to enforce MAX_BODY_BYTES using the actual request body
rather than the client-supplied content-length header. Read and measure the body
before JSON parsing, reject oversized payloads with the existing jsonError(413,
'PAYLOAD_TOO_LARGE', ...) response, and preserve normal parsing for requests
within the limit.
In `@frontend/landing/src/lib/locales/id.json`:
- Line 26: Update the Indonesian locale entries traditional_desc at both
referenced locations to use “kampanye” instead of “campaign”, preserving the
rest of each translation unchanged.
In `@frontend/landing/src/routes/`+layout.svelte:
- Around line 324-341: Replace the hardcoded bottom-navigation labels and nav
aria-label in the navigation markup with the existing i18n keys for campaigns,
pricing, and FAQ, and add the missing nav.home key to both locale files. Reuse
the project’s established translation helper and preserve the current navigation
structure and active-state behavior.
In `@frontend/landing/src/routes/`+page.svelte:
- Line 343: Align the second FAQ accordion’s label, answer, and control state by
updating its toggleFaq and openFaq references to index 3 so they use
faq.q3/faq.a3 consistently, or alternatively restore the missing faq.q2/a2
entries while retaining index 2. Ensure no FAQ keys remain unused or mismatched.
- Around line 405-412: Update the prefers-reduced-motion rule in the page
component’s style block to use global selectors for the element and
pseudo-elements, ensuring the animation, transition, and scroll-behavior reset
applies across the entire application. Keep the existing reset declarations
unchanged.
In `@README.md`:
- Around line 25-29: Synchronize the release documentation: in README.md lines
25-29, remove or mark Midtrans integration and CI checks as completed; in
backend/README.md lines 11-13, document the signed single-admin session flow and
leave only multi-institution authentication as future work; in backend/README.md
lines 114-124, add the current admin donations, upload, and payment-settings
endpoints or link to a complete canonical API inventory.
---
Nitpick comments:
In `@backend/migrations/000001_initial_schema.up.sql`:
- Around line 67-88: Add an index for the donations.status column alongside the
existing donations indexes in the initial schema migration, using the
established index naming and IF NOT EXISTS convention.
In `@docs/release-alpha.md`:
- Around line 90-96: Update the donation export verification step after payment
completion to poll the donation status or export endpoint with bounded retries
before asserting success. Continue polling when the exported row is still
pending, and fail after the retry limit unless status is success with
provider_status settlement or capture.
In `@frontend/dashboard/src/lib/Dashboard.svelte`:
- Around line 330-346: Remove the unused parseCSV function from
Dashboard.svelte, leaving the direct blob-download CSV export implementation and
surrounding code unchanged.
In `@frontend/landing/functions/campaigns/`[slug].js:
- Line 11: Update the fallback response paths in the campaign handler, including
the no-apiBase branch, API-error handling, and related fallback returns, to use
a short or non-cacheable CDN TTL instead of s-maxage=300. Preserve the
300-second cache duration for successful campaign-metadata responses.
In `@frontend/landing/src/lib/locales/en.json`:
- Around line 30-32: Remove the unused cloud_title, cloud_fee, and cloud_desc
locale keys from both locale files, including their pricing.cloud_* counterparts
if present. Also remove the unused campaigns.filter_*, select_first, and
pay_with_midtrans keys identified in the comment, without changing active
translations.
In `@frontend/landing/src/routes/campaigns/`[slug]/+page.svelte:
- Around line 133-136: Update persistCheckout() to include checkoutKey alongside
form and checkoutURL in the sessionStorage payload, and update the onMount
restore logic to read the persisted key back into checkoutKey before retrying
checkout. Preserve the existing storage key and behavior for sessions without a
saved checkoutKey.
In `@frontend/landing/src/routes/campaigns/`+page.svelte:
- Line 81: Update the filter container div’s aria-label to use the existing
localization mechanism, or add and use a campaigns.filter_group_label
translation key in both locale files, so it is localized for English users. Add
role="group" to the same div so the localized label is exposed semantically.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e627b54a-9ad7-4986-a3f3-e1a2b41aad3d
⛔ Files ignored due to path filters (1)
frontend/dashboard/static/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (57)
.env.production.example.github/workflows/ci.yml.gitignoreREADME.mdbackend/.dockerignorebackend/Dockerfilebackend/README.mdbackend/cmd/api/main.gobackend/cmd/api/main_test.gobackend/cmd/bootstrap/main.gobackend/internal/config/config.gobackend/internal/database/database.gobackend/internal/database/database_test.gobackend/internal/domain/donation.gobackend/internal/domain/payment_setting.gobackend/internal/payment/midtrans.gobackend/internal/payment/midtrans_test.gobackend/internal/payment/settings.gobackend/internal/payment/settings_test.gobackend/internal/repository/store.gobackend/internal/repository/store_test.gobackend/migrations/000001_initial_schema.up.sqlbackend/migrations/000002_campaign_metadata.up.sqlbackend/migrations/000003_donation_checkout_idempotency.up.sqlbackend/migrations/000004_admin_payment_settings.up.sqldocker-compose.production.ymldocs/api.mddocs/architecture.mddocs/database.mddocs/deployment.mddocs/deployment_checklist.mddocs/onboarding.mddocs/release-alpha.mddocs/security.mdfrontend/dashboard/README.mdfrontend/dashboard/src/lib/Dashboard.sveltefrontend/dashboard/src/routes/+page.sveltefrontend/landing/.env.examplefrontend/landing/README.mdfrontend/landing/functions/api/v1/waitlist.jsfrontend/landing/functions/campaigns/[slug].jsfrontend/landing/functions/campaigns/campaign-metadata.test.jsfrontend/landing/migrations/0001_waitlists.sqlfrontend/landing/src/lib/locales/en.jsonfrontend/landing/src/lib/locales/id.jsonfrontend/landing/src/routes/+layout.sveltefrontend/landing/src/routes/+page.sveltefrontend/landing/src/routes/campaigns/+page.sveltefrontend/landing/src/routes/campaigns/[slug]/+page.sveltefrontend/landing/src/routes/coming-soon/+page.sveltefrontend/landing/src/routes/payments/[id]/+page.jsfrontend/landing/src/routes/payments/[id]/+page.sveltefrontend/landing/src/routes/privacy/+page.sveltefrontend/landing/src/routes/terms/+page.sveltefrontend/landing/wrangler.jsoncscripts/backup-postgres.shscripts/restore-postgres.sh
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
backend/cmd/bootstrap/main.go (1)
28-31: 🔒 Security & Privacy | 🟠 MajorRotate legacy bootstrap credentials before reporting success.
FirstOrCreatedoes not update an existing organization. A database created by the previous bootstrap code can therefore retainPasswordHash == "bootstrap-managed"after this command generates and discards a new hash. The API fallback inbackend/cmd/api/main.gocan still use that guessable value to sign admin sessions.Add a one-time migration that replaces only the exact legacy value, or fail bootstrap until the credential is rotated.
🔎 Verification
#!/usr/bin/env bash set -euo pipefail rg -n -C 5 'FirstOrCreate|bootstrap-managed|GetDefaultAdminPasswordHash|ADMIN_SESSION_SECRET' backend🤖 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 `@backend/cmd/bootstrap/main.go` around lines 28 - 31, Update the bootstrap organization flow around FirstOrCreate to rotate any existing organization credential whose PasswordHash is exactly "bootstrap-managed", persisting the newly generated hash before logging success. Preserve existing credentials that do not match the legacy value, and ensure bootstrap fails if the migration cannot be saved.
🧹 Nitpick comments (3)
backend/cmd/api/main.go (1)
89-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude the underlying error in the panic message.
The other startup panics report the cause. This one discards
err, so an operator cannot tell whether the failure came from the key decode, the database read, or decryption.♻️ Proposed change
- if err := refreshPaymentClient(); err != nil { - panic("Invalid encrypted payment settings configuration") - } + if err := refreshPaymentClient(); err != nil { + panic(fmt.Sprintf("Invalid encrypted payment settings configuration: %v", err)) + }🤖 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 `@backend/cmd/api/main.go` around lines 89 - 91, Update the refreshPaymentClient error path in the startup flow to include the captured err details in the panic message, while preserving the existing configuration context and panic behavior.scripts/backup-postgres.sh (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
INTandTERMto the trap for reliable cleanup.The
EXITtrap covers a normal failure. Add the signal names so an interrupted run also removes the temporary file across shells.♻️ Proposed change
-trap 'rm -f "$TEMP_FILE"' EXIT +trap 'rm -f "$TEMP_FILE"' EXIT INT TERMKeep
trap - EXITafter themv, and extend it totrap - EXIT INT TERM.🤖 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 `@scripts/backup-postgres.sh` at line 18, Update the cleanup trap in the backup script to handle EXIT, INT, and TERM so interruptions also remove TEMP_FILE; after the successful mv, disable all three traps with trap - EXIT INT TERM..env.production.example (1)
8-8: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueQuote
DB_DSNto ensure shell consumers parse the example correctly.The production scripts and Compose example pass this file to
docker compose --env-file, where the value is preserved. A shell that sources.env.productionwould split the unquoted value afterhost=postgresand fail. UseDB_DSN="host=postgres user=kebaikanku password=replace-with-a-long-random-password dbname=kebaikanku port=5432 sslmode=disable TimeZone=Asia/Jakarta".🤖 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 @.env.production.example at line 8, Quote the DB_DSN value in the production environment example so shell consumers preserve the full connection string, including its spaces. Update the DB_DSN assignment in .env.production.example without changing any connection parameters.Source: Linters/SAST tools
🤖 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 `@backend/cmd/bootstrap/main.go`:
- Line 17: Update the ADMIN_PASSWORD input in the bootstrap password-hashing
flow around bcrypt.GenerateFromPassword to read the secret without trimming
leading or trailing whitespace, preserving its exact value for authentication.
Keep the existing required helper for identifier inputs in the same section and
apply the non-trimming reader to all ADMIN_PASSWORD usages, including the
related lines 34–47 flow.
In `@frontend/landing/src/routes/`+layout.svelte:
- Around line 213-217: Adjust the mobile header layout around the controls in
the root layout so it fits within 320px without wrapping or overflowing. Reduce
spacing/control widths or allow the logo area to shrink and truncate, while
preserving the three controls’ usability and existing behavior; verify the
result at 320px and 375px viewport widths.
- Around line 213-216: Localize the mobile and related navigation controls
around toggleLanguage and toggleTheme by replacing fixed Indonesian aria-labels,
navigation labels, and tab labels with the existing $t(...) translation
mechanism. Add matching translation keys and English/Indonesian values to both
locale definitions, preserving the current labels and behavior for each
language.
In `@scripts/restore-postgres.sh`:
- Around line 11-12: Update the guard around API_CONTAINER to inspect every
container ID returned by docker compose ps -q api, rather than passing a
newline-separated list as one docker inspect argument. Block restoration if any
API replica is running, while preserving the current behavior when no API
containers are found.
---
Duplicate comments:
In `@backend/cmd/bootstrap/main.go`:
- Around line 28-31: Update the bootstrap organization flow around FirstOrCreate
to rotate any existing organization credential whose PasswordHash is exactly
"bootstrap-managed", persisting the newly generated hash before logging success.
Preserve existing credentials that do not match the legacy value, and ensure
bootstrap fails if the migration cannot be saved.
---
Nitpick comments:
In @.env.production.example:
- Line 8: Quote the DB_DSN value in the production environment example so shell
consumers preserve the full connection string, including its spaces. Update the
DB_DSN assignment in .env.production.example without changing any connection
parameters.
In `@backend/cmd/api/main.go`:
- Around line 89-91: Update the refreshPaymentClient error path in the startup
flow to include the captured err details in the panic message, while preserving
the existing configuration context and panic behavior.
In `@scripts/backup-postgres.sh`:
- Line 18: Update the cleanup trap in the backup script to handle EXIT, INT, and
TERM so interruptions also remove TEMP_FILE; after the successful mv, disable
all three traps with trap - EXIT INT TERM.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3daad8c9-5c3e-4eb5-8fa7-1bcdd2ff541d
📒 Files selected for processing (15)
.env.production.example.github/workflows/ci.ymlbackend/Dockerfilebackend/cmd/api/main.gobackend/cmd/api/main_test.gobackend/cmd/bootstrap/main.gobackend/internal/database/database.gobackend/migrations/000003_donation_checkout_idempotency.up.sqldocker-compose.production.ymldocs/deployment.mdfrontend/landing/src/routes/+layout.sveltefrontend/landing/src/routes/campaigns/[slug]/+page.sveltefrontend/landing/src/routes/payments/[id]/+page.sveltescripts/backup-postgres.shscripts/restore-postgres.sh
🚧 Files skipped from review as they are similar to previous changes (8)
- backend/migrations/000003_donation_checkout_idempotency.up.sql
- backend/Dockerfile
- backend/internal/database/database.go
- frontend/landing/src/routes/campaigns/[slug]/+page.svelte
- .github/workflows/ci.yml
- docs/deployment.md
- docker-compose.production.yml
- backend/cmd/api/main_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/deployment.md (1)
75-97: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd the required bootstrap variables to the production environment block.
docker-compose.production.ymlrequiresPILOT_ORGANIZATION_NAMEandPILOT_ORGANIZATION_EMAIL. This block omits both variables. An operator following this block can cause thebootstrapservice to fail before the API starts. Include both variables and document thatPILOT_ORGANIZATION_IDdefaults topilot-org.🤖 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 `@docs/deployment.md` around lines 75 - 97, Update the required production environment variables block in the deployment documentation to include PILOT_ORGANIZATION_NAME and PILOT_ORGANIZATION_EMAIL, and document that PILOT_ORGANIZATION_ID defaults to pilot-org. Keep the existing bootstrap and dashboard organization guidance consistent with these variables.
🧹 Nitpick comments (1)
docs/deployment.md (1)
129-153: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftDocument backup and restore for the upload volume.
The documented scripts restore PostgreSQL only. The production stack stores campaign uploads in
uploads_data, so a database restore can leave campaign records pointing to missing banners. Define the volume backup and restore mechanism, and include it in the tested restore procedure.🤖 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 `@docs/deployment.md` around lines 129 - 153, Extend the production backup and restore documentation around the PostgreSQL scripts to define how the persistent uploads_data volume, including /data/uploads campaign banners, is backed up and restored. Add the upload-volume restore step to the tested restore procedure and require verifying both database records and restored banner files after recovery.
🤖 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 `@docs/roadmap.md`:
- Around line 35-43: Update the roadmap’s Phase 2 exit state to reflect the
bootstrapped single-organization pilot, removing the claim that institution
accounts can register while retaining login, campaign creation, and public
campaign access as applicable. In Phase 5, mark migrations, CI, and
backup/restore as complete or remove them from remaining work so status labels
consistently match the deployment guide and stated scope.
---
Outside diff comments:
In `@docs/deployment.md`:
- Around line 75-97: Update the required production environment variables block
in the deployment documentation to include PILOT_ORGANIZATION_NAME and
PILOT_ORGANIZATION_EMAIL, and document that PILOT_ORGANIZATION_ID defaults to
pilot-org. Keep the existing bootstrap and dashboard organization guidance
consistent with these variables.
---
Nitpick comments:
In `@docs/deployment.md`:
- Around line 129-153: Extend the production backup and restore documentation
around the PostgreSQL scripts to define how the persistent uploads_data volume,
including /data/uploads campaign banners, is backed up and restored. Add the
upload-volume restore step to the tested restore procedure and require verifying
both database records and restored banner files after recovery.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f07b31c0-8272-4a2d-91d6-e2356a7e536a
📒 Files selected for processing (10)
README.mdbackend/cmd/bootstrap/main.godocs/deployment.mddocs/release-alpha.mddocs/roadmap.mddocs/security.mdfrontend/landing/src/lib/locales/en.jsonfrontend/landing/src/lib/locales/id.jsonfrontend/landing/src/routes/+layout.sveltescripts/restore-postgres.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- scripts/restore-postgres.sh
- frontend/landing/src/lib/locales/id.json
- frontend/landing/src/lib/locales/en.json
- docs/security.md
- frontend/landing/src/routes/+layout.svelte
Summary
Checks
Notes
Summary by CodeRabbit