[FEATURE] Add campaign updates / creator posts endpoint (#566) - #727
[FEATURE] Add campaign updates / creator posts endpoint (#566)#727TheDEV111 wants to merge 2 commits into
Conversation
|
@TheDEV111 is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughCampaign updates now support creator-only posting and retrieval through HTTP endpoints. Updates are stored in SQLite, recorded in campaign events, and sent to active contributors through registered handlers or an optional webhook. ChangesCampaign Updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ExpressApp
participant campaignStore
participant SQLite
participant notificationService
Client->>ExpressApp: POST /api/campaigns/:id/updates
ExpressApp->>ExpressApp: Validate campaign ID and payload
ExpressApp->>campaignStore: createCampaignUpdate(campaignId, input)
campaignStore->>SQLite: Insert update and record update_posted event
campaignStore->>notificationService: notifyContributorsOnUpdate(update)
notificationService->>SQLite: Query non-refunded contributors
notificationService-->>campaignStore: NotificationPayload
campaignStore-->>ExpressApp: CampaignUpdateRecord
ExpressApp-->>Client: 201 update response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.5)backend/src/index.tsFile contains syntax errors that prevent linting: Line 514: backend/src/validation/schemas.tsFile contains syntax errors that prevent linting: Line 225: Illegal use of an export declaration not at the top level; Line 246: Illegal use of an export declaration not at the top level; Line 251: Illegal use of an export declaration not at the top level; Line 258: Illegal use of an export declaration not at the top level; Line 264: Illegal use of an export declaration not at the top level; Line 273: Illegal use of an export declaration not at the top level; Line 275: Illegal use of an export declaration not at the top level; Line 280: Illegal use of an export declaration not at the top level; Line 289: Illegal use of an export declaration not at the top level; Line 293: Illegal use of an export declaration not at the top level; Line 312: expected 🔧 ESLint
backend/src/index.tsParsing error: ',' expected. backend/src/validation/schemas.tsParsing error: '}' expected. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/src/services/campaignStore.ts (1)
716-731: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winNo pagination on
getCampaignUpdates.This returns every update for a campaign on each call. For long-running or highly active campaigns this result set only grows; consider adding
limit/offset(or cursor) parameters.🤖 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 716 - 731, Add pagination to getCampaignUpdates by accepting limit and offset (or the project’s established cursor parameters), applying them to the campaign_updates query, and returning only the requested page while preserving the existing campaign validation and ordering.backend/src/services/db.ts (1)
108-121: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider a composite index instead of two single-column indexes.
getCampaignUpdatesfilters bycampaign_idand orders bycreated_at DESC, id DESC. A composite index(campaign_id, created_at DESC, id DESC)lets SQLite satisfy both the filter and the sort in one index scan; the two separate single-column indexes here can only be used for the filter, with the sort still done separately.♻️ Suggested composite index
- CREATE INDEX IF NOT EXISTS idx_campaign_updates_campaign_id ON campaign_updates(campaign_id); - CREATE INDEX IF NOT EXISTS idx_campaign_updates_created_at ON campaign_updates(created_at); + CREATE INDEX IF NOT EXISTS idx_campaign_updates_campaign_created ON campaign_updates(campaign_id, created_at DESC, id DESC);🤖 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/db.ts` around lines 108 - 121, Replace the separate campaign_updates indexes on campaign_id and created_at with one composite index covering (campaign_id, created_at DESC, id DESC), matching getCampaignUpdates filtering and ordering while preserving the existing index naming 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 `@backend/src/services/campaignStore.ts`:
- Around line 659-670: Replace the client-supplied creator address comparison in
the campaign update authorization flow with the app’s existing authenticated
Stellar identity or transaction-verification mechanism, if available. If no such
mechanism exists, require and validate a signed challenge proving control of
campaign.creator before allowing updates. Keep the 403 response for callers who
cannot prove ownership, and do not treat input.creatorAddress, creator, or
creator_address as authorization by itself.
- Around line 697-703: Handle the asynchronous notifyContributorsOnUpdate call
so failures cannot become unhandled promise rejections: either await it within
appropriate error handling or attach a rejection handler that logs the failure
while preserving the successful update response. Keep the update write behavior
unchanged and modify the call site in the update flow.
In `@backend/src/services/notificationService.ts`:
- Around line 72-77: Update the campaign_update_notification_sent log in
notificationService to remove the recipients field and retain only
recipientCount and the other non-sensitive campaign/update metadata.
---
Nitpick comments:
In `@backend/src/services/campaignStore.ts`:
- Around line 716-731: Add pagination to getCampaignUpdates by accepting limit
and offset (or the project’s established cursor parameters), applying them to
the campaign_updates query, and returning only the requested page while
preserving the existing campaign validation and ordering.
In `@backend/src/services/db.ts`:
- Around line 108-121: Replace the separate campaign_updates indexes on
campaign_id and created_at with one composite index covering (campaign_id,
created_at DESC, id DESC), matching getCampaignUpdates filtering and ordering
while preserving the existing index naming convention.
🪄 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: dad17c6a-b1d1-43ba-b8bc-f01a5b48260e
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
backend/src/__tests__/updatesApi.test.tsbackend/src/index.test.tsbackend/src/index.tsbackend/src/services/__tests__/campaignUpdates.test.tsbackend/src/services/campaignStore.tsbackend/src/services/db.tsbackend/src/services/notificationService.tsbackend/src/validation/schemas.ts
|
Hi @TheDEV111, 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 @TheDEV111, 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! |
|
@TheDEV111 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! 🚀 |
|
@ritik4ever conflicts resolved, apologies for the late response kindly review and merge |
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 (6)
backend/src/services/db.ts (1)
139-152: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore the deleted
webhook_dead_letter_queuetable or remove its consumers.
webhookService.tsstill reads from, inserts into, deletes, and retries rows fromwebhook_dead_letter_queue, so removing its schema object will break webhook fallback handling. Keep the table creation in the schema or remove the queue code path before merge.🤖 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/db.ts` around lines 139 - 152, Restore the webhook_dead_letter_queue table definition in the database schema used by the initialization flow, ensuring it matches the columns and constraints consumed by webhookService.ts for reading, inserting, deleting, and retrying queue entries; otherwise remove all corresponding queue consumers, but preserve webhook fallback handling.backend/src/services/campaignStore.ts (1)
522-533: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestore the missing
dispatchWebhookandcreateNotificationimports.
backend/src/services/campaignStore.tsnow imports onlynotifyContributorsOnUpdate, but still callsdispatchWebhookmultiple times andcreateNotificationat lines 1052, 1071, and 1361. This will cause module import/compilation failure. Re-add the required imports or refactor those call sites.🤖 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 522 - 533, Update the imports in campaignStore.ts to include the existing dispatchWebhook and createNotification symbols alongside notifyContributorsOnUpdate, preserving all current call sites and avoiding any unrelated refactoring.backend/src/index.test.ts (1)
270-338: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBind
parseCampaignListQueryin the test file.
backend/src/index.tsimports and callsparseCampaignListQuery, butbackend/src/index.test.tsonly bindsparseCampaignListFilters; therefore every test in this block has no lexicalparseCampaignListQuerybinding and throws aReferenceError.🤖 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/index.test.ts` around lines 270 - 338, Update the imports or setup in the test file so parseCampaignListQuery is lexically bound alongside parseCampaignListFilters before the “Query parameter validation” tests use it. Preserve the existing parser test cases and ensure each invocation resolves to the intended implementation from backend/src/index.ts.backend/src/index.ts (3)
89-89: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSplit the merged import and statement on Line 89.
export const app = express();is appended to the closing line of the./services/campaignCacheimport. This is another merge artifact. Move the statement to its own line.♻️ Proposed fix
-} from './services/campaignCache';export const app = express(); +} from './services/campaignCache'; + +export const app = express();🤖 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/index.ts` at line 89, Separate the `export const app = express();` declaration from the closing `./services/campaignCache` import line, placing the export statement on its own line while preserving the import unchanged.
507-538: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winFix the broken
GET /api/campaigns/:idhandler. The file does not parse.The handler is declared as a synchronous callback with only
(req, res), but the body usesawaitat Line 514 and Line 529 and closes with} catch (error) { next(error); }at Lines 535-537. There is noasynckeyword, nonextparameter, and no openingtry {. Biome reports parse errors on exactly these lines. This looks like an unresolved merge conflict. The backend cannot compile in this state.🐛 Proposed fix
-app.get('/api/campaigns/:id', (req: Request, res: Response) => { - const parsedId = parseCampaignId(req.params.id); - if (!parsedId.ok) { - sendValidationError(parsedId.issues); - } - +app.get('/api/campaigns/:id', async (req: Request, res: Response, next: express.NextFunction) => { + try { + const parsedId = parseCampaignId(req.params.id); + if (!parsedId.ok) { + sendValidationError(parsedId.issues); + } + const cacheKey = `campaigns:detail:${parsedId.value}`;🤖 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/index.ts` around lines 507 - 538, Fix the GET /api/campaigns/:id handler by declaring its callback async, adding the next parameter, and wrapping the existing handler logic in an opening try block to match the closing catch. Preserve the current validation, cache, campaign lookup, response, and error-forwarding behavior using the existing symbols parseCampaignId, getCampaignCacheEntry, setCampaignCacheEntry, and next.Source: Linters/SAST tools
634-638: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winImport
idempotencyMiddlewarebefore using it.
backend/src/index.tscallsidempotencyMiddlewareat line 637 but does not import it from./middleware/idempotencyMiddleware. Add the import sonpm run buildsucceeds.🤖 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/index.ts` around lines 634 - 638, Import idempotencyMiddleware from ./middleware/idempotencyMiddleware in backend/src/index.ts before the app.post route uses it, preserving the existing middleware order for the pledges endpoint.
🧹 Nitpick comments (3)
backend/src/services/db.ts (1)
151-152: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd an index for the update retrieval query.
getCampaignUpdatesfilters bycampaign_idand orders bycreated_at DESC, id DESC. The two single-column indexes cannot efficiently serve both requirements. Add one composite index to avoid sorting or scanning unrelated campaign updates.Proposed fix
- CREATE INDEX IF NOT EXISTS idx_campaign_updates_campaign_id ON campaign_updates(campaign_id); - CREATE INDEX IF NOT EXISTS idx_campaign_updates_created_at ON campaign_updates(created_at); + CREATE INDEX IF NOT EXISTS idx_campaign_updates_campaign_created_id + ON campaign_updates(campaign_id, created_at DESC, id DESC);🤖 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/db.ts` around lines 151 - 152, Update the schema/index definitions near getCampaignUpdates to add a composite index on campaign_updates covering campaign_id, created_at, and id in the query’s filtering and descending ordering sequence. Keep the existing single-column indexes unchanged unless the surrounding schema logic explicitly replaces redundant indexes.backend/src/index.test.ts (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
ValidationModuletype alias.The bootstrap no longer imports the validation module dynamically. ESLint reports this alias as an error, which can fail lint in CI.
♻️ Proposed fix
type DbModule = typeof import('./services/db'); -type ValidationModule = typeof import('./validation/schemas');🤖 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/index.test.ts` at line 13, Remove the unused ValidationModule type alias from the test file, leaving the remaining bootstrap test setup unchanged.Source: Linters/SAST tools
backend/src/services/campaignStore.ts (1)
262-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
checkContributorLimitis unused and duplicates inline logic.ESLint reports this function as unused.
addPledge(Lines 824-832) andreconcileOnChainPledge(Lines 969-977) repeat the same check inline. Either call this helper from both transactions or delete 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 `@backend/src/services/campaignStore.ts` around lines 262 - 277, Remove the unused checkContributorLimit helper, or refactor addPledge and reconcileOnChainPledge to call it instead of duplicating the contributor-limit validation. Ensure both transaction paths retain the existing maximum-per-contributor behavior and error handling without leaving duplicate logic.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/src/validation/schemas.ts`:
- Around line 230-234: Update the content field in createCampaignPayloadSchema
to apply the same script-tag and SQL-comment rejection checks and sanitizeInput
transform used by title and description, while preserving its existing trim and
length validation.
---
Outside diff comments:
In `@backend/src/index.test.ts`:
- Around line 270-338: Update the imports or setup in the test file so
parseCampaignListQuery is lexically bound alongside parseCampaignListFilters
before the “Query parameter validation” tests use it. Preserve the existing
parser test cases and ensure each invocation resolves to the intended
implementation from backend/src/index.ts.
In `@backend/src/index.ts`:
- Line 89: Separate the `export const app = express();` declaration from the
closing `./services/campaignCache` import line, placing the export statement on
its own line while preserving the import unchanged.
- Around line 507-538: Fix the GET /api/campaigns/:id handler by declaring its
callback async, adding the next parameter, and wrapping the existing handler
logic in an opening try block to match the closing catch. Preserve the current
validation, cache, campaign lookup, response, and error-forwarding behavior
using the existing symbols parseCampaignId, getCampaignCacheEntry,
setCampaignCacheEntry, and next.
- Around line 634-638: Import idempotencyMiddleware from
./middleware/idempotencyMiddleware in backend/src/index.ts before the app.post
route uses it, preserving the existing middleware order for the pledges
endpoint.
In `@backend/src/services/campaignStore.ts`:
- Around line 522-533: Update the imports in campaignStore.ts to include the
existing dispatchWebhook and createNotification symbols alongside
notifyContributorsOnUpdate, preserving all current call sites and avoiding any
unrelated refactoring.
In `@backend/src/services/db.ts`:
- Around line 139-152: Restore the webhook_dead_letter_queue table definition in
the database schema used by the initialization flow, ensuring it matches the
columns and constraints consumed by webhookService.ts for reading, inserting,
deleting, and retrying queue entries; otherwise remove all corresponding queue
consumers, but preserve webhook fallback handling.
---
Nitpick comments:
In `@backend/src/index.test.ts`:
- Line 13: Remove the unused ValidationModule type alias from the test file,
leaving the remaining bootstrap test setup unchanged.
In `@backend/src/services/campaignStore.ts`:
- Around line 262-277: Remove the unused checkContributorLimit helper, or
refactor addPledge and reconcileOnChainPledge to call it instead of duplicating
the contributor-limit validation. Ensure both transaction paths retain the
existing maximum-per-contributor behavior and error handling without leaving
duplicate logic.
In `@backend/src/services/db.ts`:
- Around line 151-152: Update the schema/index definitions near
getCampaignUpdates to add a composite index on campaign_updates covering
campaign_id, created_at, and id in the query’s filtering and descending ordering
sequence. Keep the existing single-column indexes unchanged unless the
surrounding schema logic explicitly replaces redundant indexes.
🪄 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: 282123ac-5886-4d7c-9131-57c0634ecab0
📒 Files selected for processing (5)
backend/src/index.test.tsbackend/src/index.tsbackend/src/services/campaignStore.tsbackend/src/services/db.tsbackend/src/validation/schemas.ts
| content: z | ||
| .string() | ||
| .trim() | ||
| .min(1, "Update content cannot be empty.") | ||
| .max(2000, "Update content cannot exceed 2000 characters."), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Apply the same sanitization to content that other user text fields use.
createCampaignPayloadSchema rejects script tags and SQL comment sequences on title and description, then runs sanitizeInput. Update content is free-form text authored by a creator and rendered to backers. Without the same guards, stored XSS payloads pass validation.
🛡️ Proposed fix
content: z
.string()
.trim()
.min(1, "Update content cannot be empty.")
- .max(2000, "Update content cannot exceed 2000 characters."),
+ .max(2000, "Update content cannot exceed 2000 characters.")
+ .refine((val) => !containsScriptTag(val), 'Content cannot contain script tags.')
+ .refine((val) => !containsSqlComment(val), 'Content cannot contain SQL comment sequences.')
+ .transform((val) => sanitizeInput(val)),📝 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.
| content: z | |
| .string() | |
| .trim() | |
| .min(1, "Update content cannot be empty.") | |
| .max(2000, "Update content cannot exceed 2000 characters."), | |
| content: z | |
| .string() | |
| .trim() | |
| .min(1, "Update content cannot be empty.") | |
| .max(2000, "Update content cannot exceed 2000 characters.") | |
| .refine((val) => !containsScriptTag(val), 'Content cannot contain script tags.') | |
| .refine((val) => !containsSqlComment(val), 'Content cannot contain SQL comment sequences.') | |
| .transform((val) => sanitizeInput(val)), |
🤖 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/validation/schemas.ts` around lines 230 - 234, Update the content
field in createCampaignPayloadSchema to apply the same script-tag and
SQL-comment rejection checks and sanitizeInput transform used by title and
description, while preserving its existing trim and length validation.
2 participants
Description
Resolves #566 by adding endpoints for campaign creators to post text updates/announcements to backers and for clients to fetch campaign updates.
Features & Implementation Details
Schema Migration (
backend/src/services/db.ts):campaign_updatestable (id,campaign_id,creator_address,content,created_at) with performance indexes oncampaign_idandcreated_at.Notification Service (
backend/src/services/notificationService.ts):notifyContributorsOnUpdateto gather all unique non-refunded campaign backer addresses.WEBHOOK_URLenvironment variable) and event listeners.Validation & Business Logic (
backend/src/validation/schemas.ts,backend/src/services/campaignStore.ts):creator,creatorAddress, orcreator_address).403 FORBIDDENif non-creator attempts to post).update_postedevent in audit historycampaign_events.REST API Endpoints (
backend/src/index.ts):POST /api/campaigns/:id/updates— Creator posts update text to backers (201 Created).GET /api/campaigns/:id/updates— Retrieves campaign updates in reverse chronological order (200 OK).Acceptance Criteria Verification
403 FORBIDDENfor non-creators).400 VALIDATION_ERRORfor content > 2,000 chars).Testing
backend/src/services/__tests__/campaignUpdates.test.ts.backend/src/__tests__/updatesApi.test.ts.Summary by CodeRabbit
New Features
Tests