post route and service - #16
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds shared post validation schemas, status-aware API error handling, user-scoped post rate limiting, post read/write service methods, and authenticated post routes. It also switches the shared package to ESM and replaces db wildcard exports with explicit named exports. ChangesPost API and shared schema updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant postsRouter
participant Clerk
participant postService
participant Database
Client->>postsRouter: GET /user/:userId
postsRouter->>postService: getUserPosts(userId)
postService->>Database: select non-deleted posts
Database-->>postService: rows
postService-->>postsRouter: data
postsRouter-->>Client: { success, data }
Client->>postsRouter: POST /
postsRouter->>Clerk: getAuth(req)
Clerk-->>postsRouter: userId
postsRouter->>postService: createPost(authorId, input)
postService->>Database: insert post
Database-->>postService: inserted row
postService-->>postsRouter: data
postsRouter-->>Client: 201 { success, data }
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 5
🤖 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 `@apps/api/src/routes/posts.ts`:
- Around line 11-14: The posts route is validating query params locally and
leaving path params unvalidated, so it bypasses the shared Zod contract and
unknown-field rejection. Update the handlers in posts.ts to use schemas from
`@journal/shared` (or add the missing feed/query/param schemas there), apply
.strict() where appropriate, and parse req.query plus req.params.id /
req.params.userId before calling postService.
- Around line 21-79: The posts routes currently have no visible rate limiting,
and the `postsRouter.post("/")` create-post endpoint in particular needs a
tighter per-user Redis-backed limit. Add or wire in rate-limiting middleware on
the relevant handlers in `postsRouter`, using per-user limits for authenticated
routes and per-IP limits for unauthenticated ones, and make sure `createPost`
gets the stricter policy. If this is intentionally enforced elsewhere, confirm
that `postsRouter` and `postService.createPost` are covered by that shared
middleware.
- Around line 16-19: The post routes are using a local auth helper instead of
the shared middleware, which makes protection easy to bypass on future handlers.
Replace the per-handler `requireAuthed` check in `posts.ts` with the existing
`requireAuth` middleware from `middleware/requireAuth`, and attach it in the
router chain for the protected post mutations so `create`, `update`, and similar
handlers are guarded consistently.
In `@apps/api/src/services/postService.ts`:
- Around line 58-67: The `updatePost` flow can reach `db.update(posts).set()`
with an empty object because `updatePostSchema` is partial, which leads to
invalid SQL. Add a no-op guard in `postService.updatePost` before building the
`.set()` payload, or tighten `updatePostSchema`, so an empty PATCH body returns
a clear 400 instead of calling the database. Use the `updatePost` method and the
`updatePostSchema` validation path as the main places to fix this.
- Around line 52-70: The updatePost flow currently checks ownership from
getPostById() but the UPDATE itself does not enforce the soft-delete boundary,
so a row deleted after the read can still be mutated. Update the updatePost
query to include the same soft-delete guard in its where clause by combining
eq(posts.id, id) with isNull(posts.deletedAt), keeping the read/write behavior
consistent.
🪄 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: 1eccf4e7-c70d-4e75-b754-06b7e4925193
📒 Files selected for processing (5)
apps/api/src/lib/httpError.tsapps/api/src/middleware/errorHandler.tsapps/api/src/routes/posts.tsapps/api/src/services/postService.tspackages/db/src/index.ts
Comment on lines
+16
to
+19
| function requireAuthed(req: Request) { | ||
| const { isAuthenticated } = getAuth(req); | ||
| if (!isAuthenticated) throw new HttpError(401, "Unauthorized"); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect requireAuth.ts middleware exports
ast-grep outline apps/api/src/middleware/requireAuth.ts --view expanded
rg -n 'export (function|const)' apps/api/src/middleware/requireAuth.tsRepository: OumB2021/Journal
Length of output: 464
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' apps/api/src/routes/posts.ts
printf '\n---\n'
ast-grep outline apps/api/src/middleware/requireAuth.ts --view expanded
printf '\n---\n'
rg -n 'requireAuth\\(|requireAuthed\\(' apps/api/src/routes apps/api/src/middlewareRepository: OumB2021/Journal
Length of output: 2677
Use the shared requireAuth middleware here
apps/api/src/middleware/requireAuth.ts already exports requireAuth(req, res, next). Attach it to the protected post routes instead of calling requireAuthed() inside each handler so auth is enforced by the router chain and can’t be skipped on new mutations.
🤖 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 `@apps/api/src/routes/posts.ts` around lines 16 - 19, The post routes are using
a local auth helper instead of the shared middleware, which makes protection
easy to bypass on future handlers. Replace the per-handler `requireAuthed` check
in `posts.ts` with the existing `requireAuth` middleware from
`middleware/requireAuth`, and attach it in the router chain for the protected
post mutations so `create`, `update`, and similar handlers are guarded
consistently.
Source: Coding guidelines
Comment on lines
+58
to
+67
| const [updated] = await db | ||
| .update(posts) | ||
| .set({ | ||
| ...(input.title !== undefined && { title: input.title }), | ||
| ...(input.body !== undefined && { body: input.body }), | ||
| ...(input.image_url !== undefined && { imageUrl: input.image_url }), | ||
| ...(input.category !== undefined && { category: input.category }), | ||
| }) | ||
| .where(eq(posts.id, id)) | ||
| .returning(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does drizzle-orm's PostgreSQL update().set({}) with no keys throw an error or generate invalid SQL?
💡 Result:
In Drizzle ORM, executing a PostgreSQL update().set({}) with an empty object will generate invalid SQL and will not throw an error at the Drizzle level [1]. When you provide an empty object to the set() method, Drizzle generates an SQL statement resembling UPDATE "table_name" SET WHERE ... [1]. Because the SET clause is empty, this is invalid PostgreSQL syntax, and the database driver will return a syntax error [1]. This behavior is well-documented in community issue reports as a known issue [1]. Drizzle does not currently perform a validation check to prevent the generation of such invalid SQL queries when the update set is empty [1]. Developers are advised to handle this case in their own application logic (e.g., by checking if the object contains keys before initiating the update) to avoid generating invalid queries [1].
Citations:
🏁 Script executed:
git ls-files 'apps/api/src/services/postService.ts' 'package.json' 'apps/api/package.json' '**/package.json' | sed -n '1,200p'Repository: OumB2021/Journal
Length of output: 334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== apps/api/package.json ==\n'
cat -n apps/api/package.json
printf '\n== packages/db/package.json ==\n'
cat -n packages/db/package.json
printf '\n== apps/api/src/services/postService.ts (1-220) ==\n'
sed -n '1,220p' apps/api/src/services/postService.ts | cat -n
printf '\n== search for update schema / post update route ==\n'
rg -n "update.*post|updatePost|patch.*post|No fields provided|empty.*body|zod.*post|partial\\(" apps/api/src packages/shared/src packages/db/src -SRepository: OumB2021/Journal
Length of output: 6003
Handle empty post updates before calling .set()
updatePostSchema allows {} through partial(), so an empty PATCH body reaches this path and db.update(...).set({}) will emit invalid SET SQL. Reject no-op updates here or in the schema to return a clear 400 instead of a driver error.
🤖 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 `@apps/api/src/services/postService.ts` around lines 58 - 67, The `updatePost`
flow can reach `db.update(posts).set()` with an empty object because
`updatePostSchema` is partial, which leads to invalid SQL. Add a no-op guard in
`postService.updatePost` before building the `.set()` payload, or tighten
`updatePostSchema`, so an empty PATCH body returns a clear 400 instead of
calling the database. Use the `updatePost` method and the `updatePostSchema`
validation path as the main places to fix this.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/src/middleware/rateLimit.ts (1)
39-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate rate-limit boilerplate.
createPostRateLimitrepeats the exact consume/try-catch/429 pattern fromrateLimit(Lines 13-28). Extracting a small factory would prevent the two implementations from drifting as more per-route limiters are added.♻️ Proposed refactor to share the consume/error-handling logic
+function makeRateLimitMiddleware( + limiter: RateLimiterRedis, + keyFn: (req: Request) => string, +) { + return async function (req: Request, res: Response, next: NextFunction) { + try { + await limiter.consume(keyFn(req)); + next(); + } catch (err) { + if (!(err instanceof Error)) { + res.status(429).json({ error: "Too many requests" }); + } else { + next(err); + } + } + }; +} + -export async function createPostRateLimit( - req: Request, - res: Response, - next: NextFunction, -) { - const { userId } = getAuth(req); - - try { - await createPostLimiter.consume(userId ?? req.ip ?? "unknown"); - next(); - } catch (err) { - if (!(err instanceof Error)) { - res.status(429).json({ error: "Too many requests" }); - } else { - next(err); - } - } -} +export const createPostRateLimit = makeRateLimitMiddleware( + createPostLimiter, + (req) => getAuth(req).userId ?? req.ip ?? "unknown", +);🤖 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 `@apps/api/src/middleware/rateLimit.ts` around lines 39 - 56, createPostRateLimit duplicates the same consume/try-catch/429 handling already implemented in rateLimit, so extract that shared logic into a small reusable factory/helper and have both middleware functions use it. Keep the route-specific limiter injection (for createPostLimiter and the existing limiter) but centralize the request key selection, consume call, and error handling so future per-route limiters don’t drift.
🤖 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 `@packages/shared/src/index.ts`:
- Around line 13-17: The updatePostSchema in index.ts currently uses
.partial().refine(), which wraps the schema and allows unknown fields to slip
through in PATCH bodies. Update createPostSchema’s derived schema so it applies
.strict() before the refine check, ensuring extra keys are rejected while still
requiring at least one provided field. Keep the fix anchored around
updatePostSchema and its parse path used by the PATCH route.
---
Nitpick comments:
In `@apps/api/src/middleware/rateLimit.ts`:
- Around line 39-56: createPostRateLimit duplicates the same
consume/try-catch/429 handling already implemented in rateLimit, so extract that
shared logic into a small reusable factory/helper and have both middleware
functions use it. Keep the route-specific limiter injection (for
createPostLimiter and the existing limiter) but centralize the request key
selection, consume call, and error handling so future per-route limiters don’t
drift.
🪄 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: 78eaacea-3f58-4131-8b97-b318632d75c1
📒 Files selected for processing (5)
apps/api/src/middleware/rateLimit.tsapps/api/src/routes/posts.tsapps/api/src/services/postService.tspackages/shared/package.jsonpackages/shared/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/routes/posts.ts
- apps/api/src/services/postService.ts
This was referenced Jul 5, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit