refactor: separate intent controller from ecosystem controller#1569
Merged
pranalidhanavade merged 8 commits intomainfrom Feb 18, 2026
Merged
refactor: separate intent controller from ecosystem controller#1569pranalidhanavade merged 8 commits intomainfrom
pranalidhanavade merged 8 commits intomainfrom
Conversation
pranalidhanavade
requested review from
GHkrishna,
RinkalBhojani,
shitrerohit and
sujitaw
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExtracted intent/template endpoints into a new IntentController, updated multiple EcosystemController routes and parameter decorators (moving orgId params to query), registered IntentController in EcosystemModule, added intent-existence checks in repository/service, removed some gateway DTOs and narrowed the ecosystem public API surface. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIGW as API Gateway\n(EcosystemController / IntentController)
participant EcosystemSvc as EcosystemService
participant DB as Database
Client->>APIGW: POST /ecosystem/intent (createIntent)
APIGW->>EcosystemSvc: validate & forward createIntent request
EcosystemSvc->>DB: SELECT intents WHERE name ILIKE ? AND ecosystemId = ?
DB-->>EcosystemSvc: returns matching intent? (yes/no)
alt exists
EcosystemSvc-->>APIGW: throw ConflictException (intentAlreadyExists)
APIGW-->>Client: 409 Conflict
else not exists
EcosystemSvc->>DB: INSERT INTO intents (...)
DB-->>EcosystemSvc: created intent
EcosystemSvc-->>APIGW: return created intent
APIGW-->>Client: 201 Created
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api-gateway/src/ecosystem/ecosystem.controller.ts (2)
285-300:⚠️ Potential issue | 🟡 MinorHTTP status mismatch: DELETE handler returns
201 CREATED.On success the response body carries
statusCode: HttpStatus.OK(200) but the actual HTTP status sent at Line 292 isHttpStatus.CREATED(201). For a DELETE operation,200 OK(or204 No Content) is more appropriate.Proposed fix
- return res.status(HttpStatus.CREATED).json(finalResponse); + return res.status(HttpStatus.OK).json(finalResponse);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts` around lines 285 - 300, The deleteEcosystemUsers handler returns inconsistent HTTP codes: it builds finalResponse.statusCode = HttpStatus.OK but sends res.status(HttpStatus.CREATED) on success and returns BAD_REQUEST on not-found; update deleteEcosystemUsers to send the correct HTTP statuses aligned with the response body and semantics — when ecosystemService.deleteEcosystemOrgs returns result.count > 0 send res.status(HttpStatus.OK) (or HttpStatus.NO_CONTENT if you prefer no body) with the finalResponse, and when no records are deleted send res.status(HttpStatus.NOT_FOUND) with the error finalResponse; ensure you update the two res.status(...) calls accordingly and keep the finalResponse.message and statusCode values consistent.
319-343:⚠️ Potential issue | 🟡 MinorHTTP status mismatch: PUT handler returns
201 CREATED.Same issue as above —
updateEcosystemOrgStatusreturns201 CREATED(Line 335) on success for a PUT/update operation while the body carriesstatusCode: HttpStatus.OK. Should be200 OK.Proposed fix
- return res.status(HttpStatus.CREATED).json(finalResponse); + return res.status(HttpStatus.OK).json(finalResponse);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts` around lines 319 - 343, The handler updateEcosystemOrgStatus returns HttpStatus.CREATED on success but the response body uses HttpStatus.OK; change the success response to use res.status(HttpStatus.OK).json(finalResponse) so the HTTP status code matches the body (keep the existing finalResponse.statusCode: HttpStatus.OK and message), and ensure no other branches for success use HttpStatus.CREATED in this method.
🧹 Nitpick comments (3)
apps/api-gateway/src/ecosystem/ecosystem.controller.ts (1)
150-150:/:orgIdroute param lacks UUID validation.Other endpoints in this controller validate UUID params with
ParseUUIDPipe, butcreateNewEcosystemat Line 165 accepts@Param('orgId') orgId: stringwithout any validation. A non-UUIDorgId(e.g., a random string) would pass through to the service layer.Proposed fix
async createNewEcosystem( `@Body`() createEcosystemDto: CreateEcosystemDto, - `@Param`('orgId') orgId: string, + `@Param`('orgId', new ParseUUIDPipe({ + exceptionFactory: (): Error => { + throw new BadRequestException(ResponseMessages.ecosystem.error.invalidOrgId); + } + })) orgId: string, `@User`() user: user, `@Res`() res: Response ): Promise<Response> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts` at line 150, The route decorator for createNewEcosystem accepts `@Param`('orgId') orgId: string without UUID validation; update the controller method signature for createNewEcosystem to validate the orgId using Nest's ParseUUIDPipe (e.g., replace the plain `@Param`('orgId') with `@Param`('orgId', new ParseUUIDPipe()) or equivalent) so the orgId is guaranteed to be a valid UUID before calling the service layer.apps/api-gateway/src/ecosystem/intent/intent.controller.ts (2)
170-221:updateIntentusesuser?.idwhiledeleteIntentusesuser.id— inconsistent null-safety.Both endpoints are behind
AuthGuard('jwt'), sousershould never be null. Either useuser.idconsistently (since the guard guarantees it) oruser?.ideverywhere as a defensive measure.Also applies to: 227-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` around lines 170 - 221, The controller mixes null-safe and non-null-safe access to the authenticated user (user?.id in updateIntent vs user.id in deleteIntent), so make them consistent; since both methods are protected by AuthGuard('jwt') ensure you use user.id (non-null) throughout—update the assignment in updateIntent (updateIntentDto.userId = user.id) and any similar places in deleteIntent and other methods (e.g., where you set userId or read user) to use user.id to match the guarantee provided by the guard.
278-316: Method namegetTemplateByIntentIdis misleading — it fetches verification templates byorgId.The method name suggests it retrieves data by intent ID, but the route is
/org/:orgId/verification-templatesand the implementation callsgetVerificationTemplates(orgId, ...). Consider renaming to something likegetVerificationTemplatesByOrgIdfor clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` around lines 278 - 316, The controller method name getTemplateByIntentId is misleading because the route '/org/:orgId/verification-templates' and service call this.ecosystemService.getVerificationTemplates(orgId, pageDto) clearly operate on orgId; rename the method to getVerificationTemplatesByOrgId (update the method declaration and any references/tests) and ensure decorators (`@Get` path, Params, Query, Response usage) remain unchanged so behavior is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts`:
- Around line 571-585: The deleteIntentTemplate endpoint currently takes
`@Param`('id') as a raw string; add Nest's ParseUUIDPipe to the parameter to
validate the id before calling ecosystemService.deleteIntentTemplate. Update the
method signature for deleteIntentTemplate to use `@Param`('id', new
ParseUUIDPipe()) id: string (or the equivalent import/use of ParseUUIDPipe) so
invalid UUIDs are rejected with a 400 and never reach
ecosystemService.deleteIntentTemplate.
- Around line 387-411: The route GET('/:intentName/org/:orgId/templates')
declares path params but the handler getIntentTemplateByIntentAndOrg reads a
`@Query`() GetIntentTemplateByIntentAndOrgDto, so intentName/orgId are ignored and
this route can shadow GET('/:intentId/templates'); fix by switching the handler
to read path params: replace the `@Query`() parameter with `@Param`() (or add
`@Param`() alongside) and extract intentName and orgId (or verifierOrgId) to pass
into ecosystemService.getIntentTemplateByIntentAndOrg(intentName, orgId), update
the method signature (remove or repurpose GetIntentTemplateByIntentAndOrgDto)
and adjust response logic accordingly so the path params are actually used;
alternatively, if you prefer query params, change the route to a
non-parameterized path like GET('/templates/by-intent-and-org') and keep the
`@Query`() DTO.
In `@apps/ecosystem/repositories/ecosystem.repository.ts`:
- Around line 374-378: The call to updateEcosystemInvitationStatusByEmail is
passing arguments in the old order (userEmail, orgId, ecosystemId, status)
causing orgId and email to be swapped; update the call in ecosystem.service.ts
(where reopen logic occurs) to pass (orgId, userEmail, ecosystemId,
Invitation.PENDING) so the repository method
updateEcosystemInvitationStatusByEmail(orgId: string, email: string,
ecosystemId: string, status: Invitation) receives parameters in the correct
order.
---
Outside diff comments:
In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts`:
- Around line 285-300: The deleteEcosystemUsers handler returns inconsistent
HTTP codes: it builds finalResponse.statusCode = HttpStatus.OK but sends
res.status(HttpStatus.CREATED) on success and returns BAD_REQUEST on not-found;
update deleteEcosystemUsers to send the correct HTTP statuses aligned with the
response body and semantics — when ecosystemService.deleteEcosystemOrgs returns
result.count > 0 send res.status(HttpStatus.OK) (or HttpStatus.NO_CONTENT if you
prefer no body) with the finalResponse, and when no records are deleted send
res.status(HttpStatus.NOT_FOUND) with the error finalResponse; ensure you update
the two res.status(...) calls accordingly and keep the finalResponse.message and
statusCode values consistent.
- Around line 319-343: The handler updateEcosystemOrgStatus returns
HttpStatus.CREATED on success but the response body uses HttpStatus.OK; change
the success response to use res.status(HttpStatus.OK).json(finalResponse) so the
HTTP status code matches the body (keep the existing finalResponse.statusCode:
HttpStatus.OK and message), and ensure no other branches for success use
HttpStatus.CREATED in this method.
---
Nitpick comments:
In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts`:
- Line 150: The route decorator for createNewEcosystem accepts `@Param`('orgId')
orgId: string without UUID validation; update the controller method signature
for createNewEcosystem to validate the orgId using Nest's ParseUUIDPipe (e.g.,
replace the plain `@Param`('orgId') with `@Param`('orgId', new ParseUUIDPipe()) or
equivalent) so the orgId is guaranteed to be a valid UUID before calling the
service layer.
In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts`:
- Around line 170-221: The controller mixes null-safe and non-null-safe access
to the authenticated user (user?.id in updateIntent vs user.id in deleteIntent),
so make them consistent; since both methods are protected by AuthGuard('jwt')
ensure you use user.id (non-null) throughout—update the assignment in
updateIntent (updateIntentDto.userId = user.id) and any similar places in
deleteIntent and other methods (e.g., where you set userId or read user) to use
user.id to match the guarantee provided by the guard.
- Around line 278-316: The controller method name getTemplateByIntentId is
misleading because the route '/org/:orgId/verification-templates' and service
call this.ecosystemService.getVerificationTemplates(orgId, pageDto) clearly
operate on orgId; rename the method to getVerificationTemplatesByOrgId (update
the method declaration and any references/tests) and ensure decorators (`@Get`
path, Params, Query, Response usage) remain unchanged so behavior is preserved.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/api-gateway/src/ecosystem/intent/intent.controller.ts (1)
295-295: Method namegetTemplateByIntentIdis misleading — it fetches verification templates byorgId.The method retrieves verification templates scoped to an organization, not by intent ID. Consider renaming to
getVerificationTemplatesByOrgIdfor clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` at line 295, Rename the misleading method getTemplateByIntentId to getVerificationTemplatesByOrgId and update its identifier everywhere (exports, controller class method, any route handlers, service calls, tests and references) so the name accurately reflects that it fetches verification templates by orgId; ensure the method signature and behavior remain unchanged, update any JSDoc/comments and OpenAPI/route metadata to match the new name, and run/type-check tests to catch remaining references to getTemplateByIntentId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts`:
- Line 165: The orgId query parameter in the ecosystem controller endpoint is
not validated; update the parameter decorator to use ParseUUIDPipe (replace
`@Query`('orgId') orgId: string with `@Query`('orgId', new ParseUUIDPipe()) orgId:
string) and ensure ParseUUIDPipe is imported from `@nestjs/common` at the top of
the file (matching how getEcosystemDashboard/getEcosystems validate orgId). This
will enforce UUID format before the value reaches the service layer.
In `@apps/ecosystem/repositories/ecosystem.repository.ts`:
- Around line 211-223: The checkIntentExist method currently only filters by
name causing cross-ecosystem matches; update the method signature for
checkIntentExist(name?: string, ecosystemId: string, excludeIntentId?: string)
and modify the Prisma query in ecosystem.repository
(this.prisma.intents.findFirst) to include ecosystemId in the where clause and,
if excludeIntentId is provided, add a NOT/id not-equals filter to avoid matching
the same record during updates (e.g., where: { name, ecosystemId,
...(excludeIntentId ? { id: { not: excludeIntentId } } : {}) }). Ensure callers
are updated to pass ecosystemId (and excludeIntentId where appropriate).
In `@apps/ecosystem/src/ecosystem.service.ts`:
- Around line 736-741: The duplicate-name check is currently global; update the
repository method checkIntentExist to accept ecosystemId and an optional
excludeIntentId (e.g., checkIntentExist(name: string, ecosystemId: string,
excludeIntentId?: string)) and change its query to filter by ecosystem_id and,
when excludeIntentId is provided, exclude that intent id; then update service
calls in createIntent to pass the extracted ecosystemId along with the name, and
in updateIntent to extract ecosystemId from UpdateIntentDto and pass ecosystemId
plus the current intentId as excludeIntentId so renaming to the same name
doesn't false-positive.
---
Duplicate comments:
In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts`:
- Around line 387-411: The route decorator on getIntentTemplateByIntentAndOrg
defines path params (:intentName and :orgId) but the handler reads from
GetIntentTemplateByIntentAndOrgDto via `@Query`(), causing path params to be
ignored; fix by either (A) changing the method signature to accept `@Param`()
values (e.g., add `@Param`('intentName') intentName: string and `@Param`('orgId')
orgId: string or a DTO via `@Param`() and pass those to
ecosystemService.getIntentTemplateByIntentAndOrg), or (B) keep the current
DTO/@Query() approach and change the route to a query-based path (e.g.,
/templates/by-intent-and-org) so path params are removed; update the call to
ecosystemService.getIntentTemplateByIntentAndOrg to use the extracted param
variables (intentName, orgId/verifierOrgId) accordingly.
---
Nitpick comments:
In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts`:
- Line 295: Rename the misleading method getTemplateByIntentId to
getVerificationTemplatesByOrgId and update its identifier everywhere (exports,
controller class method, any route handlers, service calls, tests and
references) so the name accurately reflects that it fetches verification
templates by orgId; ensure the method signature and behavior remain unchanged,
update any JSDoc/comments and OpenAPI/route metadata to match the new name, and
run/type-check tests to catch remaining references to getTemplateByIntentId.
There was a problem hiding this comment.
🧹 Nitpick comments (5)
apps/api-gateway/src/ecosystem/intent/intent.controller.ts (5)
84-94:createIntentis missingTrimStringParamPipeonecosystemId.Every other endpoint that accepts
ecosystemIdappliesTrimStringParamPipebeforeParseUUIDPipe(seegetIntentsline 134,updateIntentline 185,deleteIntentline 242), butcreateIntentskips it.Proposed fix
`@Param`( 'ecosystemId', + TrimStringParamPipe, new ParseUUIDPipe({ exceptionFactory: (): Error => { throw new BadRequestException(ResponseMessages.ecosystem.error.invalidFormatOfEcosystemId); } }) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` around lines 84 - 94, The createIntent method's ecosystemId param is missing the TrimStringParamPipe before ParseUUIDPipe; update the ecosystemId parameter decorator on createIntent to apply TrimStringParamPipe first (the same pipe used in getIntents/updateIntent/deleteIntent) and then ParseUUIDPipe (keeping the existing exceptionFactory that throws BadRequestException with ResponseMessages.ecosystem.error.invalidFormatOfEcosystemId) so incoming whitespace is trimmed before UUID validation.
461-461: Trailing slash in route definition is inconsistent with every other route in this file.- `@Get`('/org/:orgId/templates/') + `@Get`('/org/:orgId/templates')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` at line 461, The route decorator `@Get`('/org/:orgId/templates/') has an inconsistent trailing slash compared to other routes; remove the trailing slash so it reads `@Get`('/org/:orgId/templates') to match the project's routing style and avoid duplicate route variants—update the decorator on the controller method handling org templates (the method annotated with `@Get`('/org/:orgId/templates/')) and run tests to ensure no routing regressions.
294-308: Method namegetTemplateByIntentIdis misleading — it actually queries byorgId.The route is
/org/:orgId/verification-templates, the param isorgId, and the service call isgetVerificationTemplates(orgId, …). The name implies lookup by intent ID.Suggested rename
- async getTemplateByIntentId( + async getVerificationTemplatesByOrgId(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` around lines 294 - 308, The controller method getTemplateByIntentId is misnamed because it accepts orgId and calls ecosystemService.getVerificationTemplates(orgId, …); rename the method to getVerificationTemplates (or getVerificationTemplatesByOrg) and update all references/usages (including decorators/tests and any route handler references) to use the new name so it correctly reflects that it queries by orgId; ensure the method signature (params: orgId, res: Response, pageDto: PaginationDto) and behavior remain unchanged and run tests to verify no broken imports.
334-602: Template endpoints use hard-coded string literals; intent endpoints useResponseMessages— inconsistency throughout.All intent CRUD responses (lines 105, 158, 215, 268, 312) reference
ResponseMessages.*constants. The template portion of the controller inlines strings directly:
Line Hard-coded string 342 'Intent template created successfully'374 'Intent templates retrieved successfully'412 'Intent template retrieved successfully'/'No intent template found'439 'Invalid intent ID format'(error factory)449 'Intent templates retrieved successfully'481 'Invalid orgId format'(error factory)490 'Intent templates retrieved successfully'527 'Intent template retrieved successfully'564 'Intent template updated successfully'599 'Intent template deleted successfully'All of these should be extracted to
ResponseMessagesto stay consistent and to keep client-facing strings in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` around lines 334 - 602, The template endpoints in createIntentTemplate, getAllIntentTemplates, getIntentTemplateByIntentAndOrg, getIntentTemplatesByIntentId, getIntentTemplatesByOrgId, getIntentTemplateById, updateIntentTemplate and deleteIntentTemplate use hard-coded response and error strings—replace each literal message and the ParseUUIDPipe exceptionFactory messages with appropriate constants from ResponseMessages (e.g., ResponseMessages.oid4vpIntentToTemplate.success.created, .retrieved, .notFound, .error.invalidIntentId/invalidOrgId, .updated, .deleted or create new descriptive keys in ResponseMessages if missing), update the finalResponse.message assignments and the thrown BadRequestException calls to reference those constants, and ensure any new keys follow the existing ResponseMessages structure.
396-416:intentNameparam lacksTrimStringParamPipeand an@ApiParamSwagger annotation.
@Param('intentName') intentName: stringhas no trimming (whitespace will reach the service) and no@ApiParamdecorator, so the path segment is invisible in the generated Swagger UI. TheorgIdparam is also missingTrimStringParamPipe.Proposed fix
+ `@ApiParam`({ name: 'intentName', required: true, description: 'Intent name' }) + `@ApiParam`({ name: 'orgId', required: true, description: 'Organization ID' }) `@ApiResponse`({ status: HttpStatus.OK, description: 'Intent template retrieved successfully', type: ApiResponseDto }) async getIntentTemplateByIntentAndOrg( - `@Param`('intentName') intentName: string, + `@Param`('intentName', TrimStringParamPipe) intentName: string, `@Param`( 'orgId', + TrimStringParamPipe, new ParseUUIDPipe({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` around lines 396 - 416, In getIntentTemplateByIntentAndOrg, apply TrimStringParamPipe to the intentName and orgId `@Param` decorators (replace `@Param`('intentName') intentName: string and the orgId `@Param`(...) orgId: string with versions that use TrimStringParamPipe in the pipe list alongside ParseUUIDPipe for orgId) and add an `@ApiParam`(...) Swagger decorator for intentName so the path parameter appears in OpenAPI; keep the existing ParseUUIDPipe/exceptionFactory for orgId and ensure references to intentName/orgId remain unchanged when calling ecosystemService.getIntentTemplateByIntentAndOrg.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts`:
- Around line 84-94: The createIntent method's ecosystemId param is missing the
TrimStringParamPipe before ParseUUIDPipe; update the ecosystemId parameter
decorator on createIntent to apply TrimStringParamPipe first (the same pipe used
in getIntents/updateIntent/deleteIntent) and then ParseUUIDPipe (keeping the
existing exceptionFactory that throws BadRequestException with
ResponseMessages.ecosystem.error.invalidFormatOfEcosystemId) so incoming
whitespace is trimmed before UUID validation.
- Line 461: The route decorator `@Get`('/org/:orgId/templates/') has an
inconsistent trailing slash compared to other routes; remove the trailing slash
so it reads `@Get`('/org/:orgId/templates') to match the project's routing style
and avoid duplicate route variants—update the decorator on the controller method
handling org templates (the method annotated with
`@Get`('/org/:orgId/templates/')) and run tests to ensure no routing regressions.
- Around line 294-308: The controller method getTemplateByIntentId is misnamed
because it accepts orgId and calls
ecosystemService.getVerificationTemplates(orgId, …); rename the method to
getVerificationTemplates (or getVerificationTemplatesByOrg) and update all
references/usages (including decorators/tests and any route handler references)
to use the new name so it correctly reflects that it queries by orgId; ensure
the method signature (params: orgId, res: Response, pageDto: PaginationDto) and
behavior remain unchanged and run tests to verify no broken imports.
- Around line 334-602: The template endpoints in createIntentTemplate,
getAllIntentTemplates, getIntentTemplateByIntentAndOrg,
getIntentTemplatesByIntentId, getIntentTemplatesByOrgId, getIntentTemplateById,
updateIntentTemplate and deleteIntentTemplate use hard-coded response and error
strings—replace each literal message and the ParseUUIDPipe exceptionFactory
messages with appropriate constants from ResponseMessages (e.g.,
ResponseMessages.oid4vpIntentToTemplate.success.created, .retrieved, .notFound,
.error.invalidIntentId/invalidOrgId, .updated, .deleted or create new
descriptive keys in ResponseMessages if missing), update the
finalResponse.message assignments and the thrown BadRequestException calls to
reference those constants, and ensure any new keys follow the existing
ResponseMessages structure.
- Around line 396-416: In getIntentTemplateByIntentAndOrg, apply
TrimStringParamPipe to the intentName and orgId `@Param` decorators (replace
`@Param`('intentName') intentName: string and the orgId `@Param`(...) orgId: string
with versions that use TrimStringParamPipe in the pipe list alongside
ParseUUIDPipe for orgId) and add an `@ApiParam`(...) Swagger decorator for
intentName so the path parameter appears in OpenAPI; keep the existing
ParseUUIDPipe/exceptionFactory for orgId and ensure references to
intentName/orgId remain unchanged when calling
ecosystemService.getIntentTemplateByIntentAndOrg.
027247e to
6077608
Compare
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)
apps/ecosystem/src/ecosystem.service.ts (1)
78-83:⚠️ Potential issue | 🟠 MajorWeakened guard allows duplicate ECOSYSTEM-type invitations to be created.
The original check blocked any re-invite if a pending invitation existed. The new condition
existingInvitation?.type === InviteType.MEMBERmeans that if the user already has a pending ECOSYSTEM-type invitation (i.e., was already invited to create an ecosystem), the check silently passes and a second identical ECOSYSTEM invitation is created for the same email.The intent appears to be: allow a user who already has a MEMBER invitation to still receive an ECOSYSTEM invitation. The check should additionally block when the existing pending invitation is already of the ECOSYSTEM type:
🐛 Proposed fix
- if (existingInvitation?.type === InviteType.MEMBER) { + if (existingInvitation) { + if (existingInvitation.type === InviteType.MEMBER) { throw new RpcException({ statusCode: HttpStatus.CONFLICT, message: ResponseMessages.ecosystem.error.invitationAlreadySent }); + } + // Block duplicate ECOSYSTEM invitations + throw new RpcException({ + statusCode: HttpStatus.CONFLICT, + message: ResponseMessages.ecosystem.error.invitationAlreadySent + }); }Or more concisely:
- if (existingInvitation?.type === InviteType.MEMBER) { + if (existingInvitation) { throw new RpcException({ statusCode: HttpStatus.CONFLICT, message: ResponseMessages.ecosystem.error.invitationAlreadySent }); }If the specific intent is to allow users with an existing ECOSYSTEM invitation (but not MEMBER) to be re-invited, the guard should be:
- if (existingInvitation?.type === InviteType.MEMBER) { + if (existingInvitation?.type === InviteType.ECOSYSTEM) { throw new RpcException({ statusCode: HttpStatus.CONFLICT, message: ResponseMessages.ecosystem.error.invitationAlreadySent }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ecosystem/src/ecosystem.service.ts` around lines 78 - 83, The guard currently only blocks when existingInvitation?.type === InviteType.MEMBER which lets duplicate ECOSYSTEM invitations be created; update the check in ecosystem.service.ts (the block referencing existingInvitation and InviteType) to throw when an existing pending invitation is either InviteType.MEMBER or InviteType.ECOSYSTEM (e.g., existingInvitation?.type === InviteType.MEMBER || existingInvitation?.type === InviteType.ECOSYSTEM) so duplicate ECOSYSTEM invites are prevented while still allowing MEMBER→ECOSYSTEM transitions if that is intended.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/ecosystem/src/ecosystem.service.ts`:
- Around line 78-83: The guard currently only blocks when
existingInvitation?.type === InviteType.MEMBER which lets duplicate ECOSYSTEM
invitations be created; update the check in ecosystem.service.ts (the block
referencing existingInvitation and InviteType) to throw when an existing pending
invitation is either InviteType.MEMBER or InviteType.ECOSYSTEM (e.g.,
existingInvitation?.type === InviteType.MEMBER || existingInvitation?.type ===
InviteType.ECOSYSTEM) so duplicate ECOSYSTEM invites are prevented while still
allowing MEMBER→ECOSYSTEM transitions if that is intended.
---
Duplicate comments:
In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts`:
- Around line 412-420: Apply ParseUUIDPipe to the orgId query parameter in
getEcosystemMemberInvitations so the orgId is validated; specifically ensure the
`@Query`('orgId', new ParseUUIDPipe({ exceptionFactory: (): Error => { throw new
BadRequestException(ResponseMessages.organisation.error.invalidOrgId); } }))
decorator is present on the orgId parameter and that ParseUUIDPipe,
BadRequestException and ResponseMessages are correctly imported/available so
invalid UUIDs produce the descriptive BadRequestException.
In `@apps/ecosystem/src/ecosystem.service.ts`:
- Around line 737-741: The uniqueness check for intent names should be scoped to
the target ecosystem: update the call and implementation so
checkIntentExist(name, ecosystemId) on ecosystemRepository (used where
createIntentDto.name and ecosystemId are available) verifies duplicates only
within that ecosystem; ensure the repository method signature and query use the
ecosystemId filter and keep throwing
ConflictException(ResponseMessages.ecosystem.error.intentAlreadyExists) when a
match is found.
shitrerohit
approved these changes
Feb 18, 2026
GHkrishna
approved these changes
Feb 18, 2026
pranalidhanavade
force-pushed
the
refactor/separate-intent-controller
branch
from
6077608 to
1a56edc
Compare
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 (3)
apps/api-gateway/src/ecosystem/ecosystem.controller.ts (2)
393-437:⚠️ Potential issue | 🟡 MinorMissing
@ApiQueryfor the requiredorgIdquery parameter.
orgIdis required (nooptional: true) and validated withParseUUIDPipe, but no@ApiQuerydecorator is present for it. Swagger will not document it, making the endpoint undiscoverable.@ApiQuery({ name: 'ecosystemId', required: false })is present for the other query param, so the same treatment should apply toorgId.📄 Suggested addition
+ `@ApiQuery`({ name: 'orgId', required: true, type: String, description: 'UUID of the organization for authorization' }) `@Get`('/invitations')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts` around lines 393 - 437, Add an `@ApiQuery` decorator for the orgId query parameter in the getEcosystemMemberInvitations controller so Swagger documents it: annotate the orgId parameter (validated by ParseUUIDPipe in getEcosystemMemberInvitations) with `@ApiQuery`({ name: 'orgId', required: true, type: 'string', description: 'Organisation ID (UUID)' }) (matching the existing pattern used for ecosystemId) so the orgId query is visible and required in the OpenAPI spec.
150-189:⚠️ Potential issue | 🟡 MinorMissing
@ApiQueryfor requiredorgIdin Swagger.
orgIdis now a required query parameter enforced byParseUUIDPipe, but there is no@ApiQuerydecorator documenting it. Swagger UI will not show this parameter, so API consumers have no way to discover it from the spec. Compare withgetEcosystems(lines 201–205) which correctly adds@ApiQueryfor its optionalorgId.📄 Suggested addition above the handler
+ `@ApiQuery`({ name: 'orgId', required: true, type: String, description: 'UUID of the organization' }) `@Post`('/') `@ApiOperation`({...})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts` around lines 150 - 189, Add an `@ApiQuery` decorator to document the required orgId query param for the createNewEcosystem handler: annotate the createNewEcosystem method with `@ApiQuery`({ name: 'orgId', required: true, type: String, description: 'Organization ID (UUID)', format: 'uuid' }) (place the decorator above the `@Post`('/')/@ApiOperation decorators) so Swagger shows the required UUID query parameter that is validated by the ParseUUIDPipe on the orgId parameter.apps/ecosystem/src/ecosystem.service.ts (1)
79-84:⚠️ Potential issue | 🟠 MajorDuplicate-check bypass: platform admin invitations lose all deduplication protection.
getPendingInvitationByEmailexclusively returns records withecosystemId: null. Platform-admin invitations (created byinviteUserToCreateEcosystem) are always of typeInviteType.ECOSYSTEM— member invitations always have a non-nullecosystemId. Therefore the new conditionexistingInvitation?.type === InviteType.MEMBERis unreachable in practice, and all duplicate protection for ECOSYSTEM-type invitations is silently removed.PostgreSQL's unique constraint treats
NULL ≠ NULL, so rows with(email, ecosystemId=NULL, invitedOrg=NULL)are not protected by the DB-level unique index — the learning on the file confirms that application-level validation is required for exactly this scenario. Repeated calls will now insert duplicate rows.If the intent is to allow a resend, the correct pattern is an upsert — fetch the existing record and update its timestamp/status rather than inserting a new one.
🛡️ Suggested fix
- if (existingInvitation?.type === InviteType.MEMBER) { + if (existingInvitation) { throw new RpcException({ statusCode: HttpStatus.CONFLICT, message: ResponseMessages.ecosystem.error.invitationAlreadySent }); }Or, if resend semantics are genuinely desired, resend the email and return without creating a new record when
existingInvitation?.type === InviteType.ECOSYSTEM.Based on learnings: "Application-level validation prevents duplicate records with the same email when both ecosystemId and orgId are null, addressing the PostgreSQL NULL uniqueness behavior."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ecosystem/src/ecosystem.service.ts` around lines 79 - 84, The duplicate-check bypass occurs because getPendingInvitationByEmail only returns records with ecosystemId === null so the existing condition checking existingInvitation?.type === InviteType.MEMBER never catches platform admin invites (InviteType.ECOSYSTEM); update the logic in inviteUserToCreateEcosystem (or the caller that uses getPendingInvitationByEmail) to detect existingInvitation?.type === InviteType.ECOSYSTEM as well and handle it: either throw the same RpcException to block duplicate creation, or implement resend semantics by updating the existing invitation record (an upsert/update of timestamp/status) and resending the email instead of inserting a new row; ensure you reference and adjust getPendingInvitationByEmail, InviteType.ECOSYSTEM, inviteUserToCreateEcosystem, and existingInvitation handling accordingly.
🧹 Nitpick comments (1)
apps/api-gateway/src/ecosystem/ecosystem.controller.ts (1)
366-372: Hardcoded error string — use aResponseMessagesconstant for consistency.Every other
ParseUUIDPipeexception factory in this controller (lines 168–170, 215–217, 254–256, 264–266) uses aResponseMessagesconstant. This one uses a raw string.♻️ Proposed fix
- throw new BadRequestException('Invalid Uuid'); + throw new BadRequestException(ResponseMessages.ecosystem.error.invalidFormatOfEcosystemId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts` around lines 366 - 372, Replace the hardcoded 'Invalid Uuid' string inside the ParseUUIDPipe exceptionFactory used on the `@Query`('ecosystemId') parameter with the shared ResponseMessages constant used elsewhere in the controller; update the BadRequestException call to use ResponseMessages.INVALID_UUID (the same constant referenced in other ParseUUIDPipe usages) so the message is consistent across methods like the other ParseUUIDPipe instances that already reference ResponseMessages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts`:
- Around line 393-437: Add an `@ApiQuery` decorator for the orgId query parameter
in the getEcosystemMemberInvitations controller so Swagger documents it:
annotate the orgId parameter (validated by ParseUUIDPipe in
getEcosystemMemberInvitations) with `@ApiQuery`({ name: 'orgId', required: true,
type: 'string', description: 'Organisation ID (UUID)' }) (matching the existing
pattern used for ecosystemId) so the orgId query is visible and required in the
OpenAPI spec.
- Around line 150-189: Add an `@ApiQuery` decorator to document the required orgId
query param for the createNewEcosystem handler: annotate the createNewEcosystem
method with `@ApiQuery`({ name: 'orgId', required: true, type: String,
description: 'Organization ID (UUID)', format: 'uuid' }) (place the decorator
above the `@Post`('/')/@ApiOperation decorators) so Swagger shows the required
UUID query parameter that is validated by the ParseUUIDPipe on the orgId
parameter.
In `@apps/ecosystem/src/ecosystem.service.ts`:
- Around line 79-84: The duplicate-check bypass occurs because
getPendingInvitationByEmail only returns records with ecosystemId === null so
the existing condition checking existingInvitation?.type === InviteType.MEMBER
never catches platform admin invites (InviteType.ECOSYSTEM); update the logic in
inviteUserToCreateEcosystem (or the caller that uses
getPendingInvitationByEmail) to detect existingInvitation?.type ===
InviteType.ECOSYSTEM as well and handle it: either throw the same RpcException
to block duplicate creation, or implement resend semantics by updating the
existing invitation record (an upsert/update of timestamp/status) and resending
the email instead of inserting a new row; ensure you reference and adjust
getPendingInvitationByEmail, InviteType.ECOSYSTEM, inviteUserToCreateEcosystem,
and existingInvitation handling accordingly.
---
Duplicate comments:
In `@apps/ecosystem/src/ecosystem.service.ts`:
- Around line 398-403: The call to
ecosystemRepository.updateEcosystemInvitationStatusByEmail has its arguments
swapped — currently passing (userEmail, orgId, ecosystemId, status) but the
repository expects (orgId, email, ecosystemId, status); update the call site in
ecosystem.service.ts to pass orgId first, then userEmail (i.e., (orgId,
userEmail, ecosystemId, status)) so the repository queries the correct email and
invitedOrg; verify the companion call in inviteMemberToEcosystem already matches
and run tests or add a unit test for accept/reject invitation flows to prevent
regressions.
---
Nitpick comments:
In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts`:
- Around line 366-372: Replace the hardcoded 'Invalid Uuid' string inside the
ParseUUIDPipe exceptionFactory used on the `@Query`('ecosystemId') parameter with
the shared ResponseMessages constant used elsewhere in the controller; update
the BadRequestException call to use ResponseMessages.INVALID_UUID (the same
constant referenced in other ParseUUIDPipe usages) so the message is consistent
across methods like the other ParseUUIDPipe instances that already reference
ResponseMessages.
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
pranalidhanavade
force-pushed
the
refactor/separate-intent-controller
branch
from
1a56edc to
473c0f1
Compare
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
|
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
4 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.
What
Summary by CodeRabbit
Refactor
New Features
Bug Fixes