Merged
Conversation
Moves S3_CONFIG injection token and S3ModuleOptions interface from s3.module.ts into s3.tokens.ts to break circular import dependencies when consumers need the token without importing the module.
Adds attraction_type, attraction_area, best_season, best_time_of_day, and difficulty enums. Creates attractions table with PostGIS point geometry, slug, pricing in piasters, ratings, and image gallery. Includes search_path fix to include public schema for PostGIS.
…tions CRUD Adds JWT auth strategy, attractions module with public and admin controllers, service with filtering/pagination/nearby search, Zod DTOs, presigned S3 upload URLs, and full app module wiring including throttling, global guards, and validation pipe.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the foundational 'Attractions' feature to the guide booking service, enabling comprehensive management and discovery of tourist attractions. It establishes a robust API layer, distinguishing between public access for browsing and administrative control for content management. The changes involve substantial updates to the application's core infrastructure, including database schema modifications to support geospatial data and refined S3 integration for media handling, setting the stage for rich location-based functionalities. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive CRUD functionality for attractions within the guide-booking service. The implementation is well-structured, featuring separate public and admin-facing endpoints, robust validation using Zod, and effective use of Drizzle with PostGIS for geospatial queries. I've identified a couple of areas for improvement to enhance configuration security and fix a potential bug in the slug generation logic. Overall, this is a solid feature addition.
Owner
Author
|
@coderabbitai review? |
|
✅ Actions performedReview triggered.
|
WalkthroughMoves S3 configuration tokens into a new 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/guide-booking/drizzle/20260316212720_robust_madame_masque.sql`:
- Line 10: The migration defines "slug" text NOT NULL but allows reuse via a
partial unique index, causing nearby_slugs to resolve to a different attraction
after soft-delete; update the schema so references are stable by either making
slug globally unique (remove the partial unique index and create a full unique
constraint on "slug") or, preferably, change the nearby_slugs relations to
reference the immutable attraction primary key (e.g. attraction_id) instead of
"slug" and update any affected indices and FK constraints; locate the "slug"
column, the partial unique index on slug, and the nearby_slugs table in this
migration and adjust them accordingly.
- Around line 30-35: The review_count column is currently nullable despite
having DEFAULT 0 and CHECK ("guide_booking"."attractions"."review_count" >= 0);
modify the column definition for "review_count" in the
"guide_booking"."attractions" table to add NOT NULL so it cannot be NULL (keep
the DEFAULT 0 and existing CHECK constraint
"chk_attractions_review_count_non_neg" intact); update the migration SQL to
alter the column definition (or replace the column line) to include NOT NULL and
ensure any existing data is backfilled to 0 before making it NOT NULL if needed.
In `@services/guide-booking/src/attractions/attractions.service.spec.ts`:
- Around line 19-45: The Biome lint error is caused by assigning a `.then`
property directly to the plain mock object in createMockDb; replace that pattern
with an explicit thenable wrapper to avoid the noThenProperty warning. Refactor
createMockDb so it returns either a Proxy or an instance of a small
ThenableChain class that delegates the chain methods (select, from, where, etc.
- the functions defined on chain) and implements a proper then(onFulfilled,
onRejected) method that calls Promise.resolve([]).then(onFulfilled, onRejected);
ensure the test code continues to receive the same behavior (each await still
invokes the then implementation) by returning that wrapper instead of mutating
the plain chain object.
In `@services/guide-booking/src/attractions/attractions.service.ts`:
- Around line 92-109: The current lexicographic order on attractions.slug (the
block using this.db.select(...).orderBy(desc(attractions.slug)).limit(1) and the
subsequent latest/ suffix parsing) can pick "9" over "10"; instead, query all
matching slugs for the given base (select attractions.slug where slug = base OR
slug LIKE base + '-%'), map each slug to a numeric suffix (treat missing suffix
as 1), filter out non-numeric suffixes, compute the maximum numeric suffix, and
return `${base}-${max + 1}` (or `${base}-2` when only base exists) — replace the
single .orderBy(...).limit(1) logic and the latest/suffix parsing with this
numeric-max calculation using the attractions.slug values returned from the DB.
- Around line 224-229: The softDelete path in softDelete currently only sets
deletedAt so updatedAt stays stale; modify the update in softDelete (the call
that updates the attractions table) to also set updatedAt (or updated_at
depending on the column name used elsewhere) to the current timestamp (new
Date()) so every write advances the updated timestamp alongside deletedAt;
locate softDelete and the update(attractions).set(...) call and add updatedAt:
new Date() to the set object to match application-managed updatedAt semantics.
In `@services/guide-booking/src/attractions/dto/attraction-filters.dto.ts`:
- Line 10: The `featured` filter currently uses z.coerce.boolean(), which
incorrectly parses string "false" as true; replace it with a preprocessing
boolean that explicitly converts string values "true" and "false" to booleans
before validation. Update the schema entry for featured (symbol: featured in
attraction-filters.dto.ts) to use z.preprocess (or an equivalent preprocess
step) that returns val === 'true' for string inputs, leaves actual booleans
untouched, and then validate with z.boolean().optional() so query strings like
"false" parse correctly.
In `@services/guide-booking/src/attractions/dto/update-attraction.dto.ts`:
- Line 5: The issue is that updateAttractionSchema =
createAttractionSchema.partial() causes Zod defaults for isActive/isFeatured to
be applied on empty PATCH bodies, so pickDefined() sees those values and
overwrites DB fields; fix by ensuring the update schema does NOT apply defaults
to those booleans—create the update schema from createAttractionSchema.partial()
but explicitly override isActive and isFeatured to be optional (not defaulted)
e.g., extend the partial with z.boolean().optional() for isActive and isFeatured
so omitted fields remain undefined for pickDefined() to filter out.
In `@services/guide-booking/src/auth/jwt.strategy.ts`:
- Around line 22-24: The validate(payload: JwtPayload): JwtPayload method
currently returns the payload without runtime checks; add explicit runtime
validation of required claims on the JwtPayload (verify existence and types for
sub, email, role) inside validate and throw an appropriate NestJS error (e.g.,
UnauthorizedException) when any claim is missing or malformed so only
well-formed tokens are accepted; keep returning the validated JwtPayload on
success.
In `@services/guide-booking/src/db/schema/attractions.ts`:
- Around line 52-68: The reviewCount column currently allows explicit NULL
despite default(0), so update the column definition integer('review_count') in
the attractions schema to include .notNull() (i.e., reviewCount:
integer('review_count').default(0).notNull()) so NULLs cannot be persisted; keep
or leave the existing check chk_attractions_review_count_non_neg
(sql`${t.reviewCount} >= 0`) as-is since adding .notNull() makes that check
effective.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94ba6057-9408-4a61-b064-6607ddd3bedd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
docs/roadmap/BOARD.mdpackages/nest-common/src/modules/s3/index.tspackages/nest-common/src/modules/s3/s3.module.tspackages/nest-common/src/modules/s3/s3.service.tspackages/nest-common/src/modules/s3/s3.tokens.tsservices/guide-booking/drizzle/20260316212720_robust_madame_masque.sqlservices/guide-booking/drizzle/meta/20260316212720_snapshot.jsonservices/guide-booking/drizzle/meta/_journal.jsonservices/guide-booking/package.jsonservices/guide-booking/src/app.module.tsservices/guide-booking/src/attractions/admin-attractions.controller.tsservices/guide-booking/src/attractions/attractions.controller.tsservices/guide-booking/src/attractions/attractions.module.tsservices/guide-booking/src/attractions/attractions.service.spec.tsservices/guide-booking/src/attractions/attractions.service.tsservices/guide-booking/src/attractions/dto/attraction-filters.dto.tsservices/guide-booking/src/attractions/dto/create-attraction.dto.tsservices/guide-booking/src/attractions/dto/index.tsservices/guide-booking/src/attractions/dto/update-attraction.dto.tsservices/guide-booking/src/attractions/dto/upload-url.dto.tsservices/guide-booking/src/auth/jwt.strategy.tsservices/guide-booking/src/db/enums.tsservices/guide-booking/src/db/migrate.tsservices/guide-booking/src/db/schema/attractions.tsservices/guide-booking/src/db/schema/index.ts
Fetch all similar slugs into a Set and iterate to find the next available suffix, fixing both the lexicographic sort bug after -9 and the NaN/parseInt edge case on non-numeric suffixes. Change slug unique index from partial (active only) to global so soft-deleted slugs cannot be reused and silently retarget nearby_slugs references. Also mark review_count as NOT NULL.
…write Replace z.coerce.boolean() with union+transform so query string "false" is no longer coerced to true. Override isActive/isFeatured in the update schema to strip .default() values that would silently overwrite stored booleans on empty PATCH requests.
Stamp both deletedAt and updatedAt to the same instant so soft-deleted rows do not retain a stale updatedAt timestamp.
Validate that sub, email, and role are present in the JWT payload at runtime, rejecting malformed tokens with UnauthorizedException.
Replace ?? '' fallbacks with requireEnv() that throws in production and logs a warning in development, preventing silent S3 failures.
Owner
Author
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
|
Hello @0-sayed, I've completed my review of your pull request, "feat: guide booking attractions crud". This is a substantial and well-structured feature implementation for managing attractions, demonstrating good use of NestJS, Drizzle ORM, and Zod for validation. The refactoring of the S3 module and the comprehensive database schema with geospatial capabilities are particularly well done. Here are a few points for your consideration: Review Summary
Detailed Feedback
Overall, this is a well-executed feature. Great work! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/guide-booking/src/app.module.ts`:
- Around line 41-44: Replace the non-null assertion on process.env.DATABASE_URL
in the DrizzleModule.forRoot call with the existing requireEnv pattern to
validate and surface a clear error; specifically, change the connectionString
value passed into DrizzleModule.forRoot to use requireEnv('DATABASE_URL') (or
the project's equivalent helper) instead of process.env.DATABASE_URL!, ensuring
requireEnv is imported/available in the module where DrizzleModule.forRoot is
invoked.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 368c6da2-72a9-4ee5-9c3e-7a006a8e616a
📒 Files selected for processing (10)
services/guide-booking/drizzle/20260317140321_white_diamondback.sqlservices/guide-booking/drizzle/meta/20260317140321_snapshot.jsonservices/guide-booking/drizzle/meta/_journal.jsonservices/guide-booking/src/app.module.tsservices/guide-booking/src/attractions/attractions.service.spec.tsservices/guide-booking/src/attractions/attractions.service.tsservices/guide-booking/src/attractions/dto/attraction-filters.dto.tsservices/guide-booking/src/attractions/dto/update-attraction.dto.tsservices/guide-booking/src/auth/jwt.strategy.tsservices/guide-booking/src/db/schema/attractions.ts
✅ Files skipped from review due to trivial changes (1)
- services/guide-booking/drizzle/meta/20260317140321_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (4)
- services/guide-booking/src/db/schema/attractions.ts
- services/guide-booking/src/attractions/dto/attraction-filters.dto.ts
- services/guide-booking/src/attractions/attractions.service.ts
- services/guide-booking/drizzle/meta/_journal.json
Replace non-null assertion with requireEnv() for a clearer error message if DATABASE_URL is unset, consistent with AWS credential validation in the same file.
Enable noUncheckedIndexedAccess, noImplicitOverride, noImplicitReturns, noFallthroughCasesInSwitch, and isolatedModules for stronger type safety.
Promote no-explicit-any to error, add no-unsafe-type-assertion as warning, configure NestJS-friendly overrides for no-extraneous-class and restrict-template-expressions, and relax test files appropriately.
Add @total-typescript/ts-reset for stricter stdlib types with reset.d.ts references in each service. Add pnpm check script (typecheck + lint). Remove --max-warnings 0 from lint (warnings are review signals). Remove unused root deps (supertest), nest-common deps (throttler, passport-jwt, argon2 types). Add dotenv to identity devDeps.
Add migrate.ts as entry points, ignore planned-but-unused deps per workspace, exclude enumMembers (forward-looking public API), and ignore pino-pretty (runtime transport).
Add override keyword to JwtAuthGuard methods. Replace unsafe casts with proper type narrowing in RolesGuard. Use nullish coalescing over logical OR. Type serializers and header access in LoggerModule. Fix error handling with instanceof in RedisStreamsService. Guard indexed access in decodeStreamFields.
Add explicit return types to auth controller methods. Inline Argon2id constant for isolatedModules compatibility. Use Zod pipe() for email/url validation. Type mock objects in tests. Add null guard to users insert. Remove unnecessary nullish fallbacks from ms() calls. Add eslint-disable comments for validated non-null assertions.
Promote no-explicit-any to error-level, add rules against eslint-disable for warnings, double-casts, JSDoc suppression hacks, and noPropertyAccessFromIndexSignature.
ms() returns NaN for unparseable duration strings, which silently corrupts token generation. Fail fast at startup if either JWT_REFRESH_EXPIRES_IN or JWT_ACCESS_EXPIRES_IN parses to an invalid value.
Transitive dependency via @aws-sdk/client-s3 pinned fast-xml-parser at 5.4.1 which has an incomplete fix for numeric entity expansion. Also updates @aws-sdk/client-s3 and @aws-sdk/s3-request-presigner to latest.
After merging chore/tsconfig-eslint-strictness, guide-booking had errors from noUncheckedIndexedAccess, no-misused-spread, and no-unnecessary-condition. - Add null guard after insert .returning() (noUncheckedIndexedAccess) - Use createAttractionSchema.parse(dto) to avoid class spread (no-misused-spread) - Add eslint-disable for validated non-null assertion (matches identity pattern) - Remove unnecessary optional chains on required JwtPayload fields - Type MockChain explicitly so mockDb.then is non-optional - Remove unused AttractionFiltersDto import, simplify baseFilters type - Update knip: remove stale @hena-wadeena/types ignore, add drizzle-zod
Owner
Author
|
@coderabbitai review |
|
✅ Actions performedFull review triggered. |
|
Thanks for the detailed review, @gemini-code-assist! I appreciate the thoroughness and the specific suggestions. I'll address these points:
I'll commit these changes soon. Thanks again for your help! |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/nest-common/src/modules/redis/redis-streams.service.ts (2)
95-124:⚠️ Potential issue | 🟠 MajorKey retry state by stream and group, not just message ID.
Line 103 uses
msgIdas the sole retry key. That can collide across streams/groups and corrupt retry bookkeeping, so one message can accidentally clear or consume another message’s retries.Suggested fix
await Promise.all( messages.map(async ([msgId, fields]) => { + const retryKey = `${streamName}:${group}:${msgId}`; const data = this.decodeStreamFields(fields); try { await handler({ stream: streamName, id: msgId, data }); await this.redis.xack(streamName, group, msgId); - this.retryCount.delete(msgId); + this.retryCount.delete(retryKey); } catch (err) { - const retries = (this.retryCount.get(msgId) ?? 0) + 1; - this.retryCount.set(msgId, retries); + const retries = (this.retryCount.get(retryKey) ?? 0) + 1; + this.retryCount.set(retryKey, retries); this.logger.error( `Handler error for ${streamName}:${msgId} (attempt ${retries}/${this.maxRetries})`, err, ); if (retries >= this.maxRetries) { @@ ); await this.redis.xack(streamName, group, msgId); - this.retryCount.delete(msgId); + this.retryCount.delete(retryKey); this.logger.error( `Message ${msgId} moved to DLQ ${dlqStream} after ${this.maxRetries} failed attempts`, ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nest-common/src/modules/redis/redis-streams.service.ts` around lines 95 - 124, The retry bookkeeping uses msgId alone (retryCount.get/set/delete) which can collide across streams/groups; change the retry key to a composite string (e.g. `${streamName}:${group}:${msgId}`) and update all usages in redis-streams.service.ts (where retryCount.get, retryCount.set, retryCount.delete and the retry counting logic occur) so the composite key is used when incrementing retries, checking against maxRetries, and deleting after success or moving to DLQ; keep all other logic (decodeStreamFields, handler call, xack, xadd, encodeStreamFields) unchanged.
28-33:⚠️ Potential issue | 🟡 MinorReject empty payloads before calling
XADD.If
payloadis{}, Line 30 sends no field/value pairs and Redis fails with an argument error. Guard this upfront so callers get a deterministic application error instead of a low-level Redis failure.Suggested fix
async publish(eventName: EventName, payload: Record<string, string>): Promise<string> { const fields = this.encodeStreamFields(payload); + if (fields.length === 0) { + throw new Error(`Cannot publish empty payload to stream: ${eventName}`); + } const id = await this.redis.xadd(eventName as string, '*', ...fields); if (!id) throw new Error(`Failed to publish to stream: ${eventName}`); this.logger.debug(`Published ${eventName}: ${id}`); return id; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nest-common/src/modules/redis/redis-streams.service.ts` around lines 28 - 33, The publish method calls redis.xadd with no field/value pairs when payload is empty, causing Redis argument errors; update publish (and its use of encodeStreamFields) to validate payload/fields before calling this.redis.xadd: if encodeStreamFields(payload) yields an empty array (or payload is an empty object) throw a clear application error like "Empty payload for stream publish: <eventName>" so callers receive a deterministic error instead of a Redis failure; ensure logger.debug remains after a successful xadd.
♻️ Duplicate comments (1)
services/guide-booking/src/auth/jwt.strategy.ts (1)
22-25:⚠️ Potential issue | 🟠 MajorHarden claim validation to enforce runtime types, not just truthiness.
At Line 23, malformed truthy values can pass (e.g., object claims). In auth code, validate
sub,roleas non-empty strings.🔐 Proposed fix
validate(payload: JwtPayload): JwtPayload { - if (!payload.sub || !payload.email || !payload.role) { + const isNonEmptyString = (value: unknown): value is string => + typeof value === 'string' && value.trim().length > 0; + + if ( + !isNonEmptyString(payload?.sub) || + !isNonEmptyString(payload?.email) || + !isNonEmptyString(payload?.role) + ) { throw new UnauthorizedException('Invalid token payload'); } return payload; }#!/bin/bash set -euo pipefail echo "Locate JwtPayload definition:" rg -nP --type=ts '\b(interface|type)\s+JwtPayload\b' packages services -C3 echo echo "Inspect strategy validation:" cat -n services/guide-booking/src/auth/jwt.strategy.ts | sed -n '1,200p'Expected result: verify whether shared
JwtPayloadis strictly string-typed; even then, runtime guards should still enforce type/shape for untrusted token input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/guide-booking/src/auth/jwt.strategy.ts` around lines 22 - 25, The validate method in JwtStrategy (validate(payload: JwtPayload)) currently only checks truthiness which allows non-string objects to pass; update validate to enforce runtime types by verifying payload.sub, payload.email, and payload.role are non-empty strings (e.g., typeof === 'string' && payload.x.trim().length > 0) and throw UnauthorizedException('Invalid token payload') if any check fails; reference the validate method and JwtPayload type to add these explicit guards so untrusted token claims cannot be objects or other non-string values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nest-common/src/modules/logger/logger.module.ts`:
- Around line 19-21: The current correlation-id extraction returns an empty
string as valid because headerId is accepted if it's a string; update the logic
that reads req.headers['x-request-id'] (headerId) so it rejects empty strings or
whitespace-only values before falling back to generateId(): check that headerId
is a non-empty trimmed string and only then return it, otherwise return
generateId(); adjust the function/method that performs this extraction (the
block referencing headerId, req.headers['x-request-id'], and generateId())
accordingly.
In `@services/guide-booking/package.json`:
- Around line 29-36: The package.json currently lists "drizzle-zod": "^0.8.3"
and "zod": "^4.3.6" which are incompatible; update the dependency pair so both
use Zod v4 or both use Zod v3. Specifically, either upgrade "drizzle-zod" to a
release that declares peerDependency on Zod v4 (replace the "drizzle-zod"
version string with a Zod-4-compatible version) or change "zod" to a ^3.x
version; update the corresponding entry for "drizzle-zod" or "zod" in
package.json and run install to confirm no peerDependency conflicts.
In `@services/guide-booking/src/app.module.ts`:
- Around line 25-35: The warning in requireEnv is misleading because it
hardcodes "S3 uploads will fail" even when the missing variable (e.g.,
DATABASE_URL) is unrelated; update the requireEnv function to emit a generic,
contextual warning that includes the missing variable name (e.g., `⚠️ ${name} is
not set — this may cause runtime failures`) or customize the message based on
known env keys (like DATABASE_URL vs S3 keys) so the log accurately reflects the
likely impact; keep the existing production throw behavior and only change the
warning text in requireEnv.
---
Outside diff comments:
In `@packages/nest-common/src/modules/redis/redis-streams.service.ts`:
- Around line 95-124: The retry bookkeeping uses msgId alone
(retryCount.get/set/delete) which can collide across streams/groups; change the
retry key to a composite string (e.g. `${streamName}:${group}:${msgId}`) and
update all usages in redis-streams.service.ts (where retryCount.get,
retryCount.set, retryCount.delete and the retry counting logic occur) so the
composite key is used when incrementing retries, checking against maxRetries,
and deleting after success or moving to DLQ; keep all other logic
(decodeStreamFields, handler call, xack, xadd, encodeStreamFields) unchanged.
- Around line 28-33: The publish method calls redis.xadd with no field/value
pairs when payload is empty, causing Redis argument errors; update publish (and
its use of encodeStreamFields) to validate payload/fields before calling
this.redis.xadd: if encodeStreamFields(payload) yields an empty array (or
payload is an empty object) throw a clear application error like "Empty payload
for stream publish: <eventName>" so callers receive a deterministic error
instead of a Redis failure; ensure logger.debug remains after a successful xadd.
---
Duplicate comments:
In `@services/guide-booking/src/auth/jwt.strategy.ts`:
- Around line 22-25: The validate method in JwtStrategy (validate(payload:
JwtPayload)) currently only checks truthiness which allows non-string objects to
pass; update validate to enforce runtime types by verifying payload.sub,
payload.email, and payload.role are non-empty strings (e.g., typeof === 'string'
&& payload.x.trim().length > 0) and throw UnauthorizedException('Invalid token
payload') if any check fails; reference the validate method and JwtPayload type
to add these explicit guards so untrusted token claims cannot be objects or
other non-string values.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 780f7222-7552-4d1e-b757-71aaa5400791
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (57)
CLAUDE.mddocs/roadmap/BOARD.mdeslint.config.mjsknip.tspackage.jsonpackages/nest-common/package.jsonpackages/nest-common/src/guards/jwt-auth.guard.tspackages/nest-common/src/guards/roles.guard.tspackages/nest-common/src/modules/drizzle/drizzle.module.tspackages/nest-common/src/modules/logger/logger.module.tspackages/nest-common/src/modules/redis/redis-streams.service.tspackages/nest-common/src/modules/redis/redis.module.tspackages/nest-common/src/modules/s3/index.tspackages/nest-common/src/modules/s3/s3.module.tspackages/nest-common/src/modules/s3/s3.service.tspackages/nest-common/src/modules/s3/s3.tokens.tsservices/guide-booking/drizzle/20260316212720_robust_madame_masque.sqlservices/guide-booking/drizzle/20260317140321_white_diamondback.sqlservices/guide-booking/drizzle/meta/20260316212720_snapshot.jsonservices/guide-booking/drizzle/meta/20260317140321_snapshot.jsonservices/guide-booking/drizzle/meta/_journal.jsonservices/guide-booking/package.jsonservices/guide-booking/src/app.module.tsservices/guide-booking/src/attractions/admin-attractions.controller.tsservices/guide-booking/src/attractions/attractions.controller.tsservices/guide-booking/src/attractions/attractions.module.tsservices/guide-booking/src/attractions/attractions.service.spec.tsservices/guide-booking/src/attractions/attractions.service.tsservices/guide-booking/src/attractions/dto/attraction-filters.dto.tsservices/guide-booking/src/attractions/dto/create-attraction.dto.tsservices/guide-booking/src/attractions/dto/index.tsservices/guide-booking/src/attractions/dto/update-attraction.dto.tsservices/guide-booking/src/attractions/dto/upload-url.dto.tsservices/guide-booking/src/auth/jwt.strategy.tsservices/guide-booking/src/db/enums.tsservices/guide-booking/src/db/migrate.tsservices/guide-booking/src/db/schema/attractions.tsservices/guide-booking/src/db/schema/index.tsservices/guide-booking/src/reset.d.tsservices/identity/package.jsonservices/identity/src/app.module.tsservices/identity/src/auth/auth.controller.tsservices/identity/src/auth/auth.module.tsservices/identity/src/auth/auth.service.spec.tsservices/identity/src/auth/auth.service.tsservices/identity/src/auth/dto/shared.tsservices/identity/src/auth/hashing.service.tsservices/identity/src/db/migrate.tsservices/identity/src/reset.d.tsservices/identity/src/test-utils/create-mock-db.tsservices/identity/src/users/dto/update-profile.dto.tsservices/identity/src/users/users.service.tsservices/map/src/db/migrate.tsservices/map/src/reset.d.tsservices/market/src/db/migrate.tsservices/market/src/reset.d.tstsconfig.json
💤 Files with no reviewable changes (1)
- packages/nest-common/package.json
Trim and validate the x-request-id header so empty strings don't slip through as correlation IDs — generateId() kicks in instead.
…tractions-crud # Conflicts: # docs/roadmap/BOARD.md # knip.ts # packages/nest-common/src/modules/logger/logger.module.ts # packages/nest-common/src/modules/redis/redis.module.ts # packages/nest-common/src/modules/s3/s3.module.ts # packages/nest-common/src/modules/s3/s3.service.ts
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
2 participants
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.
No description provided.