feat: add campaign share card generator and OG meta tags (#593) - #746
feat: add campaign share card generator and OG meta tags (#593)#746queenfrostbite wants to merge 3 commits into
Conversation
|
@queenfrostbite is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@queenfrostbite Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe PR updates backend validation, concurrency tests, Redis logging, API documentation, frontend wallet typing, i18n wiring, test infrastructure, and TypeScript build configuration. ChangesBackend runtime and campaign behavior
Frontend integration and quality
API documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
Hi @queenfrostbite, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
1 similar comment
|
Hi @queenfrostbite, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
|
Hi @ritik4ever I've resolved the merge conflict, please review and merge |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@frontend/index.html`:
- Around line 17-19: Add fallback Open Graph and Twitter meta tags alongside the
existing description and title in frontend/index.html, then provide
campaign-specific title, URL, and image metadata in the server or edge response
for /campaigns/:id so previews work before client-side hooks execute. Keep
useOpenGraph and App campaign derivation as client-side enhancements rather than
their only metadata source.
In `@frontend/src/App.tsx`:
- Around line 338-348: Move the ogMeta useMemo and its useOpenGraph(ogMeta) call
below the selectedCampaign useMemo in App, ensuring selectedCampaign is
initialized before either block references it. Preserve the existing Open Graph
values and dependencies.
- Around line 343-346: Update the metadata image handling in the campaign card
metadata flow around metadata.imageUrl so og:image and twitter:image never
receive data: URLs. Validate the persisted image value and omit or replace it
with a hosted HTTPS image before assigning image; ensure the same constraint is
enforced when metadata.imageUrl is saved.
In `@frontend/src/components/CampaignDetailPanel.tsx`:
- Around line 88-89: Update CampaignDetailPanel to use the application-level
addToast callback from App.tsx instead of creating a separate useToast store;
pass the callback into the component and remove its local useToast call,
preserving the existing success and error toast behavior.
- Around line 97-104: Update handleCopyLink to use an async try/catch flow and
guard navigator.clipboard?.writeText before invoking it. When the Clipboard API
is unavailable or fails, reuse the textarea fallback implemented in
ShareButtons.tsx, and preserve the existing success and error toast behavior.
- Around line 3-4: Update CampaignDetailPanel’s imports to use Link from
react-router-dom for the Back to campaigns navigation, and rename the
lucide-react Link icon import to a distinct icon symbol. Update the
corresponding JSX references so the router component receives the to prop while
the icon remains used separately.
In `@frontend/src/components/CampaignShareCard.tsx`:
- Around line 55-68: Update the logo-rendering flow in CampaignShareCard so
drawing occurs only after the Image has loaded and decoded: make drawCard,
generate, and serialization helpers asynchronous, await img.complete &&
img.decode() or the load event before ctx.drawImage, and update all callers of
the shared-card APIs to await them before serialization.
🪄 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: 9e730ee4-95d5-4c3b-a426-37f6df430a6f
📒 Files selected for processing (6)
frontend/index.htmlfrontend/src/App.tsxfrontend/src/components/CampaignDetailPanel.tsxfrontend/src/components/CampaignShareCard.tsxfrontend/src/hooks/useOpenGraph.tsfrontend/src/index.css
| const { downloadPng, toDataUrl } = useCampaignShareCard(); | ||
| const { addToast } = useToast(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the application toast state.
useToast owns state with useState. This call creates a toast store separate from the useToast call in frontend/src/App.tsx. CampaignDetailPanel does not render its local toasts, so the new success and error messages are not visible.
Pass the application addToast callback into this component, or move toast state to a shared provider.
🤖 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/src/components/CampaignDetailPanel.tsx` around lines 88 - 89, Update
CampaignDetailPanel to use the application-level addToast callback from App.tsx
instead of creating a separate useToast store; pass the callback into the
component and remove its local useToast call, preserving the existing success
and error toast behavior.
- Move ogMeta block below selectedCampaign to fix TDZ reference error - Reject data: URLs for og:image so social crawlers can load them - Remove unused toDataUrl from App.tsx - Remove unused Share2 import and toDataUrl from CampaignDetailPanel - Use LinkIcon from lucide and restore Link from react-router-dom - Thread onToast from App into CampaignDetailPanel instead of a local useToast store - Add Clipboard API fallback with textarea execCommand - Decode brand logo image before drawing the share card canvas - Add fallback OG/Twitter meta tags to index.html
|
@ritik4ever the failing tests are pre-existing |
|
@ritik4ever please review and merge |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/CampaignDetailPanel.tsx (1)
329-332: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a loading skeleton for
Time left.The loaded detail grid now contains six statistics, but the loading branch still creates five skeletons. This causes a layout shift when the
Countdownstatistic appears.Proposed fix
- {Array.from({ length: 5 }).map((_, index) => ( + {Array.from({ length: 6 }).map((_, index) => (🤖 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/src/components/CampaignDetailPanel.tsx` around lines 329 - 332, Add a sixth skeleton item to the loading branch of the detail grid in CampaignDetailPanel, matching the Time left statistic rendered by Countdown in the loaded branch. Keep the existing five skeletons unchanged and ensure the loading layout reserves space for all six statistics.
🤖 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.
Outside diff comments:
In `@frontend/src/components/CampaignDetailPanel.tsx`:
- Around line 329-332: Add a sixth skeleton item to the loading branch of the
detail grid in CampaignDetailPanel, matching the Time left statistic rendered by
Countdown in the loaded branch. Keep the existing five skeletons unchanged and
ensure the loading layout reserves space for all six statistics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 49931d14-dc81-493e-9eb4-3f3e21a1177b
📒 Files selected for processing (3)
frontend/index.htmlfrontend/src/App.tsxfrontend/src/components/CampaignDetailPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/index.html
- frontend/src/App.tsx
9e93093 to
7da98ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/openapi.yaml (2)
1411-1424: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDeclare
Idempotency-Keyas a formal header parameter.The description states: "Creates a pledge for a campaign. Use the Idempotency-Key header to make the request idempotent. Cached responses are returned for 24 hours." The
parametersarray for this operation only lists theidpath parameter; there is noin: headerentry forIdempotency-Key.Prose-only documentation of a header is easy for OpenAPI-driven tooling (client codegen, contract tests, mock servers) to miss, since these tools read
parameters, not free-text descriptions. Add a formal header parameter so the contract matches the documented behavior.📝 Proposed fix
parameters: - schema: type: string pattern: ^[1-9]\d*$ example: "1" required: true name: id in: path + - schema: + type: string + required: false + description: Idempotency key. Cached responses are returned for 24 hours. + name: Idempotency-Key + in: 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 `@docs/openapi.yaml` around lines 1411 - 1424, Update the Pledges POST operation’s parameters alongside the existing id path parameter to formally declare the Idempotency-Key header, including its string schema and required status matching the endpoint’s idempotency contract. Keep the existing path parameter and description unchanged.
968-1022: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore the dropped
requiredfields forStatsResponse.data.The new
requiredlist at lines 1013-1020 keeps only the snake_case fields. The camelCase fieldstotalCampaigns,openCampaigns,fundedCampaigns,claimedCampaigns,failedCampaigns,totalPledgeVolume, anduniqueContributorsremain as properties but no longer appear inrequired. Static analysis confirms this: oasdiff reports each of these fields "became optional" forGET /api/statsat status200.The backend continues to emit both naming schemes unconditionally. The stats route returns total_campaigns, open_campaigns, funded_campaigns, failed_campaigns, total_pledged_usdc, total_pledged_xlm, total_contributors, avg_funding_rate_pct, totalCampaigns, openCampaigns, fundedCampaigns, claimedCampaigns, failedCampaigns, totalPledgeVolume, and uniqueContributors together in one response. Since the runtime guarantee did not change, the schema should keep documenting these fields as required. Marking them optional understates the actual API guarantee and can cause consumers or generated clients to add unnecessary null checks or omit handling for fields that are always present.
Add the camelCase fields back to the
requiredarray, alongside the new snake_case fields.📝 Proposed fix
required: - total_campaigns - open_campaigns - funded_campaigns - failed_campaigns - total_pledged_usdc - total_pledged_xlm - total_contributors - avg_funding_rate_pct + - totalCampaigns + - openCampaigns + - fundedCampaigns + - claimedCampaigns + - failedCampaigns + - totalPledgeVolume + - uniqueContributors🤖 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/openapi.yaml` around lines 968 - 1022, Update the StatsResponse.data required list to include all camelCase properties—totalCampaigns, openCampaigns, fundedCampaigns, claimedCampaigns, failedCampaigns, totalPledgeVolume, and uniqueContributors—alongside the existing snake_case fields, preserving the schema’s required-field contract.Source: Linters/SAST tools
🧹 Nitpick comments (1)
frontend/src/components/CampaignUpdates.tsx (1)
48-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the stale required prop from the component contract.
CampaignUpdatesno longer readscampaignId, butCampaignUpdatesPropsstill requires it at Line 8. If this removal is intentional, removecampaignIdfrom the interface and all callers. If compatibility requires the prop, document that contract explicitly.🤖 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/src/components/CampaignUpdates.tsx` around lines 48 - 54, Remove the obsolete campaignId field from CampaignUpdatesProps and update every CampaignUpdates caller to stop passing it, since CampaignUpdates no longer consumes the prop. Keep the remaining component contract unchanged.
🤖 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/src/services/__tests__/webhookService.test.ts`:
- Line 3: Format the webhookService test file with Prettier after removing the
unused initDb import, then rerun the relevant CI checks to confirm the
formatting failure is resolved.
In `@backend/src/services/campaignStore.concurrent.test.ts`:
- Around line 114-127: Replace the microtask-based Promise.allSettled execution
in the concurrent tests with a shared isolated-worker harness that uses separate
SQLite clients and a synchronization barrier. Apply this harness to
backend/src/services/campaignStore.concurrent.test.ts ranges 114-127, 156-169,
and 260-272: preserve the pledge-cap assertions at 114-127, enforce contributor
limits at 156-169, and configure the 260-272 scenario so pledge success depends
on the claim transition rather than the campaign already reaching its target.
In `@backend/src/services/campaignStore.ts`:
- Around line 358-371: Update the creator search construction in the campaign
query to escape the LIKE escape character, `%`, and `_` in rawQuery before
wrapping it with `%`. Add an explicit ESCAPE clause to both
LOWER(campaigns.creator) LIKE expressions, while preserving parameterization and
the existing full-text, ID, and fallback behavior.
In `@docs/openapi.yaml`:
- Around line 1289-1365: Remove the stale 409 response definitions from the
campaign archive delete operation and the /api/campaigns/{id}/restore post
operation in the OpenAPI document. Keep all other response definitions and
descriptions unchanged, including the existing ApiError responses.
In `@frontend/src/components/campaignsTableUtils.test.ts`:
- Around line 282-292: Replace every remaining 'newest' sort key argument in the
campaign sorting tests, including the cases around lines 326, 332, 338, 343, and
360, with 'createdAt' so all calls to sortCampaigns use the supported key.
In `@frontend/src/components/CampaignUpdates.tsx`:
- Around line 176-180: Update the `code` renderer in `CampaignUpdates` to always
render only the `<code>` element, removing the `inline` conditional and any
`<pre>` wrapper; let the Markdown `pre` renderer provide block-code wrapping for
fenced code.
In `@frontend/src/components/KeyboardShortcutsOverlay.test.tsx`:
- Around line 23-33: The dialog in KeyboardShortcutsOverlay.tsx must not remain
inside an aria-hidden="true" ancestor. Update the overlay structure or move
aria-hidden onto only the non-focusable backdrop decoration/control, while
preserving the dialog’s role and keyboard accessibility so getByRole('dialog')
can discover it.
In `@frontend/src/components/WalletWidget.test.tsx`:
- Line 66: Update both render blocks in WalletWidget tests so the
{...defaultProps} spread appears before the explicit onConnect and onDisconnect
callback props, ensuring each test’s spies override the shared noop callbacks
and are the callbacks passed to the component.
In `@frontend/tsconfig.build.json`:
- Line 3: Format the exclude array in tsconfig.build.json with Prettier,
wrapping its entries across multiple lines as needed so the file passes the
repository’s Prettier check.
---
Outside diff comments:
In `@docs/openapi.yaml`:
- Around line 1411-1424: Update the Pledges POST operation’s parameters
alongside the existing id path parameter to formally declare the Idempotency-Key
header, including its string schema and required status matching the endpoint’s
idempotency contract. Keep the existing path parameter and description
unchanged.
- Around line 968-1022: Update the StatsResponse.data required list to include
all camelCase properties—totalCampaigns, openCampaigns, fundedCampaigns,
claimedCampaigns, failedCampaigns, totalPledgeVolume, and
uniqueContributors—alongside the existing snake_case fields, preserving the
schema’s required-field contract.
---
Nitpick comments:
In `@frontend/src/components/CampaignUpdates.tsx`:
- Around line 48-54: Remove the obsolete campaignId field from
CampaignUpdatesProps and update every CampaignUpdates caller to stop passing it,
since CampaignUpdates no longer consumes the prop. Keep the remaining component
contract unchanged.
🪄 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: a0bca428-80e7-4a99-ba0d-f67b93de104c
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (54)
backend/.eslintrc.jsonbackend/src/api.test.tsbackend/src/config.tsbackend/src/historyEndpoint.test.tsbackend/src/index.test.tsbackend/src/index.tsbackend/src/logger.test.tsbackend/src/logger.tsbackend/src/pledgesEndpoint.test.tsbackend/src/rateLimiter.test.tsbackend/src/requestId.test.tsbackend/src/scripts/generateOpenApi.tsbackend/src/security.test.tsbackend/src/services/__tests__/eventMetadata.test.tsbackend/src/services/__tests__/mutation.test.tsbackend/src/services/__tests__/webhookService.test.tsbackend/src/services/cache.tsbackend/src/services/campaignCache.tsbackend/src/services/campaignStore.concurrent.test.tsbackend/src/services/campaignStore.test.tsbackend/src/services/campaignStore.tsbackend/src/services/db.tsbackend/src/validateEnv.tsbackend/src/validation/schemas.test.tsbackend/src/validation/schemas.tsbackend/src/validation/stellarAddress.test.tsbackend/tests/integration.test.tsdocs/openapi.yamlfrontend/.eslintrc.jsonfrontend/package.jsonfrontend/src/App.tsxfrontend/src/components/CampaignDetailPanel.tsxfrontend/src/components/CampaignShareCard.tsxfrontend/src/components/CampaignUpdates.tsxfrontend/src/components/CampaignsTable.a11y.test.tsxfrontend/src/components/CampaignsTable.integration.test.tsxfrontend/src/components/ContributorProfile.tsxfrontend/src/components/ContributorSummary.test.tsxfrontend/src/components/CreateCampaignForm.tsxfrontend/src/components/KeyboardShortcutsOverlay.test.tsxfrontend/src/components/KeyboardShortcutsOverlay.tsxfrontend/src/components/NotificationBell.tsxfrontend/src/components/OfflineBanner.test.tsxfrontend/src/components/SearchInput.stories.tsxfrontend/src/components/SortDropdown.stories.tsxfrontend/src/components/WalletPickerModal.tsxfrontend/src/components/WalletWidget.test.tsxfrontend/src/components/campaignsTableUtils.test.tsfrontend/src/hooks/useDebounce.test.tsfrontend/src/hooks/useOffline.test.tsfrontend/src/lib/wallet.tsfrontend/src/main.tsxfrontend/src/test-setup.tsfrontend/tsconfig.build.json
💤 Files with no reviewable changes (2)
- backend/src/scripts/generateOpenApi.ts
- backend/src/services/db.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/App.tsx
| import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; | ||
| import axios from 'axios'; | ||
| import { initDb, resetDbForTests, getDb } from '../db'; | ||
| import { initDb, resetDbForTests } from '../db'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clear the Prettier failure before merge.
The unused import removal is correct, but CI still reports backend/src/services/__tests__/webhookService.test.ts as unformatted. Run npx prettier --write backend/src/services/__tests__/webhookService.test.ts and rerun CI.
🤖 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/src/services/__tests__/webhookService.test.ts` at line 3, Format the
webhookService test file with Prettier after removing the unused initDb import,
then rerun the relevant CI checks to confirm the formatting failure is resolved.
Source: Pipeline failures
| const results = await Promise.allSettled(pledgePromises.map((fn) => Promise.resolve().then(fn))); | ||
|
|
||
| // All pledges should succeed (no hard cap on total) | ||
| // but campaign should not exceed target in practice | ||
| expect(results).toHaveLength(3); | ||
| // The transaction re-checks the funding cap, so at most one 300 pledge | ||
| // can be accepted; the others are rejected to prevent over-pledging. | ||
| const fulfilled = results.filter((r) => r.status === 'fulfilled'); | ||
| const rejected = results.filter((r) => r.status === 'rejected'); | ||
| expect(fulfilled.length).toBe(1); | ||
| expect(rejected.length).toBe(2); | ||
|
|
||
| const campaign = getCampaign(campaignId); | ||
| expect(campaign).toBeDefined(); | ||
| // Total pledged should be 900 (no hard cap enforced) | ||
| expect(campaign?.pledgedAmount).toBe(900); | ||
| // Total pledged must never exceed the campaign target | ||
| expect(campaign?.pledgedAmount).toBe(300); | ||
| expect(campaign?.pledgedAmount).toBeLessThanOrEqual(500); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use real concurrent database clients for these race tests.
Promise.resolve().then(fn) only queues microtasks. Each synchronous addPledge or claimCampaign call completes before the next callback starts. These tests therefore verify serial behavior, not transaction safety under contention. The claim test also rejects the extra pledge because the campaign already has its 500-unit target, regardless of claim ordering.
backend/src/services/campaignStore.concurrent.test.ts#L114-L127: run pledges through isolated workers or processes with separate SQLite connections and a synchronization barrier.backend/src/services/campaignStore.concurrent.test.ts#L156-L169: use the same concurrent-client harness for contributor-limit enforcement.backend/src/services/campaignStore.concurrent.test.ts#L260-L272: use the harness and a state where the pledge outcome depends on the claim transition.
📍 Affects 1 file
backend/src/services/campaignStore.concurrent.test.ts#L114-L127(this comment)backend/src/services/campaignStore.concurrent.test.ts#L156-L169backend/src/services/campaignStore.concurrent.test.ts#L260-L272
🤖 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/src/services/campaignStore.concurrent.test.ts` around lines 114 -
127, Replace the microtask-based Promise.allSettled execution in the concurrent
tests with a shared isolated-worker harness that uses separate SQLite clients
and a synchronization barrier. Apply this harness to
backend/src/services/campaignStore.concurrent.test.ts ranges 114-127, 156-169,
and 260-272: preserve the pledge-cap assertions at 114-127, enforce contributor
limits at 156-169, and configure the 260-272 scenario so pledge success depends
on the claim transition rather than the campaign already reaching its target.
| const creatorSearchTerm = `%${rawQuery.toLowerCase()}%`; | ||
| const exactTerm = rawQuery; | ||
|
|
||
| if (ftsMatchTerm) { | ||
| whereClauses.push(`( | ||
| campaigns.id IN (SELECT id FROM campaigns_fts WHERE campaigns_fts MATCH ?) | ||
| OR LOWER(campaigns.creator) = LOWER(?) | ||
| OR LOWER(campaigns.creator) LIKE ? | ||
| OR campaigns.id = ? | ||
| )`); | ||
| params.push(ftsMatchTerm, creatorExactTerm, exactTerm); | ||
| params.push(ftsMatchTerm, creatorSearchTerm, exactTerm); | ||
| } else { | ||
| // Fallback if cleaning the query stripped all characters | ||
| whereClauses.push(`(LOWER(campaigns.creator) = LOWER(?) OR campaigns.id = ?)`); | ||
| params.push(creatorExactTerm, exactTerm); | ||
| whereClauses.push(`(LOWER(campaigns.creator) LIKE ? OR campaigns.id = ?)`); | ||
| params.push(creatorSearchTerm, exactTerm); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Escape LIKE metacharacters in creator searches.
rawQuery is parameterized, but % and _ still change LIKE semantics. For example, a search for % uses %%% and matches every creator. Escape %, _, and the escape character before building creatorSearchTerm. Add an explicit ESCAPE clause to both LIKE expressions.
Proposed fix
+ const escapeLike = (value: string) => value.replace(/[\\%_]/g, '\\$&');
- const creatorSearchTerm = `%${rawQuery.toLowerCase()}%`;
+ const creatorSearchTerm = `%${escapeLike(rawQuery.toLowerCase())}%`;
- OR LOWER(campaigns.creator) LIKE ?
+ OR LOWER(campaigns.creator) LIKE ? ESCAPE '\\'
...
- whereClauses.push(`(LOWER(campaigns.creator) LIKE ? OR campaigns.id = ?)`);
+ whereClauses.push(`(LOWER(campaigns.creator) LIKE ? ESCAPE '\\' OR campaigns.id = ?)`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const creatorSearchTerm = `%${rawQuery.toLowerCase()}%`; | |
| const exactTerm = rawQuery; | |
| if (ftsMatchTerm) { | |
| whereClauses.push(`( | |
| campaigns.id IN (SELECT id FROM campaigns_fts WHERE campaigns_fts MATCH ?) | |
| OR LOWER(campaigns.creator) = LOWER(?) | |
| OR LOWER(campaigns.creator) LIKE ? | |
| OR campaigns.id = ? | |
| )`); | |
| params.push(ftsMatchTerm, creatorExactTerm, exactTerm); | |
| params.push(ftsMatchTerm, creatorSearchTerm, exactTerm); | |
| } else { | |
| // Fallback if cleaning the query stripped all characters | |
| whereClauses.push(`(LOWER(campaigns.creator) = LOWER(?) OR campaigns.id = ?)`); | |
| params.push(creatorExactTerm, exactTerm); | |
| whereClauses.push(`(LOWER(campaigns.creator) LIKE ? OR campaigns.id = ?)`); | |
| params.push(creatorSearchTerm, exactTerm); | |
| const escapeLike = (value: string) => value.replace(/[\\%_]/g, '\\$&'); | |
| const creatorSearchTerm = `%${escapeLike(rawQuery.toLowerCase())}%`; | |
| const exactTerm = rawQuery; | |
| if (ftsMatchTerm) { | |
| whereClauses.push(`( | |
| campaigns.id IN (SELECT id FROM campaigns_fts WHERE campaigns_fts MATCH ?) | |
| OR LOWER(campaigns.creator) LIKE ? ESCAPE '\\' | |
| OR campaigns.id = ? | |
| )`); | |
| params.push(ftsMatchTerm, creatorSearchTerm, exactTerm); | |
| } else { | |
| // Fallback if cleaning the query stripped all characters | |
| whereClauses.push(`(LOWER(campaigns.creator) LIKE ? ESCAPE '\\' OR campaigns.id = ?)`); | |
| params.push(creatorSearchTerm, exactTerm); |
🤖 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/src/services/campaignStore.ts` around lines 358 - 371, Update the
creator search construction in the campaign query to escape the LIKE escape
character, `%`, and `_` in rawQuery before wrapping it with `%`. Add an explicit
ESCAPE clause to both LOWER(campaigns.creator) LIKE expressions, while
preserving parameterization and the existing full-text, ID, and fallback
behavior.
| delete: | ||
| tags: | ||
| - Campaigns | ||
| summary: Archive (soft-delete) a campaign | ||
| description: Sets the archivedAt/deletedAt timestamp on a campaign. Archived | ||
| campaigns are excluded from the default campaign list but their pledges | ||
| and history are preserved. Use POST /api/campaigns/{id}/restore to | ||
| un-archive. | ||
| parameters: | ||
| - schema: | ||
| type: string | ||
| pattern: ^[1-9]\d*$ | ||
| example: "1" | ||
| required: true | ||
| name: id | ||
| in: path | ||
| responses: | ||
| "200": | ||
| description: Campaign archived | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/CampaignDetailResponse" | ||
| "400": | ||
| description: Validation error | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/ApiError" | ||
| "404": | ||
| description: Resource not found | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/ApiError" | ||
| "409": | ||
| description: Campaign is already archived | ||
| "429": | ||
| description: Rate limit exceeded | ||
| /api/campaigns/{id}/restore: | ||
| post: | ||
| tags: | ||
| - Campaigns | ||
| summary: Restore an archived campaign | ||
| description: Clears the archivedAt/deletedAt timestamp, making the campaign | ||
| active again. | ||
| parameters: | ||
| - schema: | ||
| type: string | ||
| pattern: ^[1-9]\d*$ | ||
| example: "1" | ||
| required: true | ||
| name: id | ||
| in: path | ||
| responses: | ||
| "200": | ||
| description: Campaign restored | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/CampaignDetailResponse" | ||
| "400": | ||
| description: Validation error | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/ApiError" | ||
| "404": | ||
| description: Resource not found | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/ApiError" | ||
| "409": | ||
| description: Campaign is not archived | ||
| "429": | ||
| description: Rate limit exceeded |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for conflict handling in campaign archive/restore store functions.
set -euo pipefail
fd -t f 'campaignStore.ts$' backend/src | xargs -I{} sh -c 'echo "== {} =="; rg -n -C 5 "already archived|not archived|409|softDeleteCampaign|restoreCampaign" {}'Repository: ritik4ever/stellar-goal-vault
Length of output: 4586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -t f 'apiError|error|campaign' backend/src docs | sed -n '1,120p'
echo
echo "== ApiError and toServiceError definitions/usages =="
rg -n -C 4 "interface ApiError|type ApiError|class.*Error|toServiceError|ServiceError|error handler|ErrorHandler|409" backend/src docs/openapi.yaml | sed -n '1,220p'Repository: ritik4ever/stellar-goal-vault
Length of output: 16299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/src/types/errors.ts =="
cat -n backend/src/types/errors.ts
echo
echo "== campaign archive/restore route implementations and error handling =="
fd -t f 'app\.(ts|js)$|api\.(ts|js)$' backend/src -x sh -c 'echo "== $1 =="; rg -n -C 6 "app\.(delete|post)\('\''/api/campaigns/:id|app\.(delete|post)\('\''/api/campaigns/:id/restore|app\.use|catch|AppError" "$1" || true' sh {}
echo
echo "== docs/openapi.yaml 409 context =="
sed -n '1280,1375p' docs/openapi.yamlRepository: ritik4ever/stellar-goal-vault
Length of output: 4805
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate main router/api files =="
fd -t f '\.(ts|js)$' backend/src | xargs -I{} sh -c 'echo "== {} =="; rg -n "from .*(express|router)|app\.use|app\.listen|express\(\)" "{}" | sed -n "1,80p"'
echo
echo "== campaign archive/restore references =="
fd -t f '\.(ts|js)$' backend/src | xargs -I{} sh -c 'rg -n -C 3 "/api/campaigns|campaigns/:id|campaigns/:id/restore|softDeleteCampaign|restoreCampaign|ALREADY_DELETED|NOT_ARCHIVED" "{}" | sed -n "1,240p"'
echo
echo "== openApi generator relevant lines =="
wc -l backend/src/generateOpenApi.ts backend/src/openapi.ts 2>/dev/null || true
rg -n -C 4 "ApiError|409|DELETE|restore|archived|soft-delete|Campaign already|Campaign is not archived" backend/src docs/openapi.yamlRepository: ritik4ever/stellar-goal-vault
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/src/index.ts error handler and route registration =="
sed -n '530,570p' backend/src/index.ts
sed -n '980,1035p' backend/src/index.ts
echo
echo "== backend/src/index.test.ts archive/restore sections =="
sed -n '1008,1115p' backend/src/index.test.ts
echo
echo "== backend/src/openapi.ts archive/restore response definitions =="
sed -n '768,820p' backend/src/openapi.ts
sed -n '820,870p' backend/src/openapi.tsRepository: ritik4ever/stellar-goal-vault
Length of output: 5930
Update docs/openapi.yaml to remove the stale 409 archive/restore responses.
backend/src/services/campaignStore.ts does not throw 409 for softDeleteCampaign/restoreCampaign; the duplicate-delete test uses a separate ApiError path rather than these store helpers. The 409 entries should be removed from the archive and restore paths, including the generated docs/openapi.yaml.
🤖 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/openapi.yaml` around lines 1289 - 1365, Remove the stale 409 response
definitions from the campaign archive delete operation and the
/api/campaigns/{id}/restore post operation in the OpenAPI document. Keep all
other response definitions and descriptions unchanged, including the existing
ApiError responses.
| describe('Sort by createdAt (newest first)', () => { | ||
| it('should sort campaigns by createdAt descending (newest first)', () => { | ||
| const sorted = sortCampaigns(mockCampaigns, 'newest'); | ||
| const sorted = sortCampaigns(mockCampaigns, 'createdAt'); | ||
| expect(sorted[0].id).toBe('3'); // createdAt: 1710000200 | ||
| expect(sorted[1].id).toBe('2'); // createdAt: 1710000100 | ||
| expect(sorted[2].id).toBe('1'); // createdAt: 1710000000 | ||
| }); | ||
|
|
||
| it('should not mutate the original array', () => { | ||
| const original = [...mockCampaigns]; | ||
| sortCampaigns(mockCampaigns, 'newest'); | ||
| sortCampaigns(mockCampaigns, 'createdAt'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Complete the newest to createdAt migration.
The changed suite uses createdAt, but Lines 326, 332, 338, 343, and 360 still pass 'newest'. The sortCampaigns implementation in frontend/src/components/campaignsTableUtils.ts does not handle that key. Replace every remaining 'newest' argument with 'createdAt'.
🤖 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/src/components/campaignsTableUtils.test.ts` around lines 282 - 292,
Replace every remaining 'newest' sort key argument in the campaign sorting
tests, including the cases around lines 326, 332, 338, 343, and 360, with
'createdAt' so all calls to sortCampaigns use the supported key.
| code: (props: { children?: React.ReactNode; inline?: boolean; className?: string }) => | ||
| props.inline ? ( | ||
| <code className={props.className}>{props.children}</code> | ||
| ) : ( | ||
| <pre><code>{children}</code></pre> | ||
| <pre><code className={props.className}>{props.children}</code></pre> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 3 '"react-markdown"' frontend/package.json
rg -n -C 6 'code:|inline|<pre>' frontend/src/components/CampaignUpdates.tsxRepository: ritik4ever/stellar-goal-vault
Length of output: 1154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Package file:\n'
sed -n '1,60p' frontend/package.json
printf '\nPackage lock entries around react-markdown:\n'
if [ -f frontend/package-lock.json ]; then
rg -n '"react-markdown"|node_modules/react-markdown' frontend/package-lock.json | head -40
rg -n '"node_modules/react-markdown"|name":"react-markdown"|version":' frontend/package-lock.json -C 3 | head -120
elif [ -f package-lock.json ]; then
rg -n '"react-markdown"|node_modules/react-markdown' package-lock.json | head -40
fi
printf '\nReadMarkdown wrapper in CampaignUpdates.tsx:\n'
sed -n '1,230p' frontend/src/components/CampaignUpdates.tsx
printf '\nCheck local/react-markdown API docs in lock/pnpm/yarn if needed\n'
fd -a 'README|react-markdown' . | head -50Repository: ritik4ever/stellar-goal-vault
Length of output: 7125
Use the pre renderer for block code.
react-markdown v10 no longer passes inline to code components. This code renderer receives inline === undefined, so both inline code and fenced code go into the block branch. Fenced Markdown is already wrapped in a pre, and this renderer returns <pre><code>...</code></pre>, which creates nested <pre> markup on fenced code. Only render the <code> element here; let the pre renderer handle block wrapping.
🤖 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/src/components/CampaignUpdates.tsx` around lines 176 - 180, Update
the `code` renderer in `CampaignUpdates` to always render only the `<code>`
element, removing the `inline` conditional and any `<pre>` wrapper; let the
Markdown `pre` renderer provide block-code wrapping for fenced code.
| expect(screen.queryByTestId('keyboard-shortcuts-dialog')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders the dialog when isOpen is true', () => { | ||
| renderOverlay(true); | ||
| expect(screen.getByRole('dialog')).toBeInTheDocument(); | ||
| expect(screen.getByTestId('keyboard-shortcuts-dialog')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('has aria-modal="true" when open', () => { | ||
| renderOverlay(true); | ||
| expect(screen.getByRole('dialog')).toHaveAttribute('aria-modal', 'true'); | ||
| expect(screen.getByTestId('keyboard-shortcuts-dialog')).toHaveAttribute('aria-modal', 'true'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching KeyboardShortcutsOverlay:"
fd -i 'KeyboardShortcutsOverlay' .
echo
echo "Relevant files:"
for f in $(fd -i 'KeyboardShortcutsOverlay' .); do
echo "--- $f ($(wc -l < "$f") lines) ---"
ast-grep outline "$f" || true
done
echo
echo "Search keyboard shortcuts overlay tests and component:"
rg -n "keyboard-shortcuts|KeyboardShortcutsOverlay|aria-modal|aria-hidden|getByRole|queryByRole|testId" .Repository: ritik4ever/stellar-goal-vault
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- KeyboardShortcutsOverlay.tsx lines 1-120 ---"
cat -n frontend/src/components/KeyboardShortcutsOverlay.tsx | sed -n '1,120p'
echo
echo "--- KeyboardShortcutsOverlay.test.tsx lines 1-150 ---"
cat -n frontend/src/components/KeyboardShortcutsOverlay.test.tsx | sed -n '1,150p'
echo
echo "--- KeyboardShortcutsOverlay.a11y.test.tsx ---"
cat -n frontend/src/components/KeyboardShortcutsOverlay.a11y test.tsx | sed -n '1,80p' || true
echo
echo "--- focused aria-hidden and role/testId occurrences in relevant files ---"
rg -n "aria-hidden|getByRole|queryByRole|testId|data-testid|dialog|Keyboard Shortcuts|aria-modal" frontend/src/components/KeyboardShortcutsOverlay.tsx frontend/src/components/KeyboardShortcutsOverlay.test.tsx frontend/src/components/KeyboardShortcutsOverlay.a11y.test.tsxRepository: ritik4ever/stellar-goal-vault
Length of output: 14702
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- KeyboardShortcutsOverlay.tsx lines 1-120 ---"
cat -n frontend/src/components/KeyboardShortcutsOverlay.tsx | sed -n '1,120p'
echo
echo "--- KeyboardShortcutsOverlay.test.tsx lines 1-150 ---"
cat -n frontend/src/components/KeyboardShortcutsOverlay.test.tsx | sed -n '1,150p'
echo
echo "--- KeyboardShortcutsOverlay.a11y.test.tsx ---"
cat -n frontend/src/components/KeyboardShortcutsOverlay.a11y.test.tsx | sed -n '1,80p'
echo
echo "--- focused aria-hidden and role/testId occurrences in relevant files ---"
rg -n "aria-hidden|getByRole|queryByRole|testId|data-testid|dialog|Keyboard Shortcuts|aria-modal" frontend/src/components/KeyboardShortcutsOverlay.tsx frontend/src/components/KeyboardShortcutsOverlay.test.tsx frontend/src/components/KeyboardShortcutsOverlay.a11y.test.tsxRepository: ritik4ever/stellar-goal-vault
Length of output: 15274
Remove aria-hidden from the dialog backdrop.
The tests already use getByRole('dialog'), but KeyboardShortcutsOverlay.tsx lines 72-108 still places the role="dialog" element inside aria-hidden="true". aria-hidden applies to descendants, so the dialog is hidden from the accessibility tree. Apply aria-hidden=true only to non-focusable backdrop decorations/control, or restructure the overlay so the dialog itself is not a descendant.
🤖 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/src/components/KeyboardShortcutsOverlay.test.tsx` around lines 23 -
33, The dialog in KeyboardShortcutsOverlay.tsx must not remain inside an
aria-hidden="true" ancestor. Update the overlay structure or move aria-hidden
onto only the non-focusable backdrop decoration/control, while preserving the
dialog’s role and keyboard accessibility so getByRole('dialog') can discover it.
| network={null} | ||
| onConnect={onConnect} | ||
| onDisconnect={noop} | ||
| {...defaultProps} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Place per-test callback overrides after defaultProps.
The shared spread is the last prop in both render blocks. JSX applies later props last, so it replaces onConnect={onConnect} and onDisconnect={onDisconnect} with the shared noop. The tests then check spies that the component does not receive. Move {...defaultProps} before each explicit callback override.
Also applies to: 126-126
🤖 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/src/components/WalletWidget.test.tsx` at line 66, Update both render
blocks in WalletWidget tests so the {...defaultProps} spread appears before the
explicit onConnect and onDisconnect callback props, ensuring each test’s spies
override the shared noop callbacks and are the callbacks passed to the
component.
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "extends": "./tsconfig.json", | |||
| "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/**/*.a11y.test.tsx", "src/**/*.stories.tsx", "src/test/**"] | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Format this file before merge.
The Prettier check fails for frontend/tsconfig.build.json. Run Prettier or wrap the exclude array.
Proposed formatting
- "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/**/*.a11y.test.tsx", "src/**/*.stories.tsx", "src/test/**"]
+ "exclude": [
+ "src/**/*.test.ts",
+ "src/**/*.test.tsx",
+ "src/**/*.a11y.test.tsx",
+ "src/**/*.stories.tsx",
+ "src/test/**"
+ ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/**/*.a11y.test.tsx", "src/**/*.stories.tsx", "src/test/**"] | |
| "exclude": [ | |
| "src/**/*.test.ts", | |
| "src/**/*.test.tsx", | |
| "src/**/*.a11y.test.tsx", | |
| "src/**/*.stories.tsx", | |
| "src/test/**" | |
| ] |
🤖 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/tsconfig.build.json` at line 3, Format the exclude array in
tsconfig.build.json with Prettier, wrapping its entries across multiple lines as
needed so the file passes the repository’s Prettier check.
Source: Pipeline failures
3 participants
Summary
Adds a canvas-based campaign share card generator and Open Graph meta tags for social link previews.
Closes #593
New files
CampaignShareCard.tsxuseCampaignShareCardhook exposing:downloadPng(),toDataUrl(),toBlob()useOpenGraph.tsog:title,og:description,og:image,og:url,twitter:cardmeta tagsModified files
CampaignDetailPanel.tsxApp.tsxuseOpenGraphhook to set OG meta tags based on selected campaignindex.htmlindex.css.share-actionsflex container stylesAcceptance criteria
campaign_share_{id}.pngSummary by CodeRabbit
New Features
Bug Fixes