fix: nestjs version update #1459
Conversation
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
WalkthroughRemoved explicit HTTP status codes from many Swagger error decorators; added and expanded multiple controller endpoints (cloud-wallet, connection, fido, etc.); refactored NATS client to use firstValueFrom and standardized headers/contextId; adjusted ClientProxy typings and DTO/interface refinements; introduced a pnpm catalog mapping for NestJS dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant GW as API Gateway (CloudWalletController)
participant S as CloudWalletService
participant N as NATSClient
participant Agent
User->>GW: Authenticated request (e.g., create wallet)
GW->>S: Delegate (attach userId/email)
S->>N: sendNatsMessage(pattern, payload)
N->>Agent: NATS request (headers include contextId)
Agent-->>N: Response
N-->>S: firstValueFrom(response)
S-->>GW: formatted data
GW-->>User: { statusCode, message, data }
sequenceDiagram
autonumber
actor User
participant C as ConnectionController
participant CS as ConnectionService
participant N as NATSClient
participant Agent
User->>C: GET /orgs/:orgId/connections/:connectionId
C->>C: Validate params (ParseUUID/Trim)
C->>CS: getConnectionsById(user, connectionId, orgId)
CS->>N: sendNatsMessage('get-connection-by-id', payload)
N->>Agent: NATS request
alt success
Agent-->>N: Data
N-->>CS: firstValueFrom(data)
CS-->>C: Details
C-->>User: { statusCode, message, data }
else error
Agent-->>N: Error
N-->>CS: Error
CS->>CS: Build RpcException (message, status fallback)
CS-->>C: throw
C-->>User: Error response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)apps/api-gateway/src/fido/fido.controller.ts (4)
🔇 Additional comments (6)
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 |
Signed-off-by: Krishna Waske <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
apps/organization/src/organization.service.ts (1)
1762-1780: Fix HttpException status, remove deprecated toPromise, and align with NATSClient
- Passing
error.erroras the HttpException status is incorrect; useerror.status/statusCodeor a safe default.toPromise()is deprecated; prefer Promise-returningNATSClientalready used elsewhere.- Avoid incorrect
(response: string)annotation; the response is an object/array.Apply this diff:
- return this.organizationServiceProxy - .send(pattern, userEmails) - .pipe( - map((response: string) => ({ - response - })) - ) - .toPromise() - .catch((error) => { - this.logger.error(`getUserKeycloakIdByEmail catch: ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error?.statusCode, - error: error?.message - }, - error.error - ); - }); + const response = await this.natsClient + .send(this.organizationServiceProxy, pattern, userEmails) + .catch((error) => { + this.logger.error(`getUserKeycloakIdByEmail catch: ${JSON.stringify(error)}`); + const status = error?.status ?? error?.statusCode ?? 500; + throw new HttpException({ status, error: error?.message }, status); + }); + return { response };apps/api-gateway/src/connection/connection.service.ts (1)
129-140: Same toPromise() issue here.Replace toPromise() with firstValueFrom.
Apply this diff:
- const message = await this.connectionServiceProxy.send(pattern, payload).toPromise(); + const message = await firstValueFrom(this.connectionServiceProxy.send(pattern, payload));libs/common/src/NATSClient.ts (2)
21-36: Fix header construction and replace deprecated toPromise().
- headers(): set the contextId explicitly on the headers object.
- Use firstValueFrom instead of toPromise.
Apply this diff:
- sendNats(serviceProxy, cmd: string, payload: any): Promise<any> { + sendNats(serviceProxy, cmd: string, payload: any): Promise<any> { this.logger.log(`Inside NATSClient for sendNats()`); const pattern = { cmd }; - const headers = nats.headers(1, this.contextStorageService.getContextId()); - const record = new NatsRecordBuilder(payload).setHeaders(headers).build(); + let contextId = this.contextStorageService.getContextId() || v4(); + const headers = nats.headers(); + headers.set('contextId', contextId); + const record = new NatsRecordBuilder(payload).setHeaders(headers).build(); - return serviceProxy - .send(pattern, record) - .pipe( - map((response: string) => ({ - response - })) - ) - .toPromise(); + return firstValueFrom( + serviceProxy.send(pattern, record).pipe(map((response: string) => ({ response }))) + ); }
48-61: Use proper header construction in generic send().Avoid passing args to nats.headers(); set header key explicitly.
Apply this diff:
send<T>(serviceProxy, pattern: object, payload: any): Promise<T> { let contextId = this.contextStorageService.getContextId(); if (!contextId) { contextId = v4(); } - const headers = nats.headers(1, contextId); + const headers = nats.headers(); + headers.set('contextId', contextId); const record = new NatsRecordBuilder(payload).setHeaders(headers).build(); - const result = serviceProxy.send(pattern, record); + const result = serviceProxy.send(pattern, record); return firstValueFrom(result); }apps/agent-service/src/agent-service.service.ts (1)
1859-1871: RxJS toPromise() is removed; use firstValueFrom.This will break on Nest 11/RxJS 7+. Replace the chain accordingly and import firstValueFrom.
Apply this diff:
- const updatedSchemaInfo = await this.agentServiceProxy - .send(pattern, payload) - .toPromise() - .catch((error) => { + const updatedSchemaInfo = await firstValueFrom(this.agentServiceProxy.send(pattern, payload)).catch((error) => { this.logger.error(`catch: ${JSON.stringify(error)}`); throw new HttpException( { status: error.status, error: error.message }, error.status ); });Add import near other rxjs imports:
- import { from } from 'rxjs'; + import { from, firstValueFrom } from 'rxjs';apps/cloud-wallet/src/cloud-wallet.service.ts (8)
156-159: Fix malformed URL: stray closing brace.Extra “}” corrupts the endpoint and will 404.
- const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}}`; + const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}`;
171-179: Fix malformed URL and query appending.
- Extra “}”
- Avoid adding “/” before a query string.
- const threadParam = threadId ? `?threadId=${threadId}` : ''; - const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${threadParam}}`; + const threadParam = threadId ? `?threadId=${threadId}` : ''; + const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}${threadParam}`;
256-261: Correct condition to validate createCloudWallet response.Use OR; current AND skips the missing-id case.
- if (!createCloudWalletResponse && !createCloudWalletResponse.id) { + if (!createCloudWalletResponse || !createCloudWalletResponse.id) { throw new InternalServerErrorException(ResponseMessages.cloudWallet.error.createCloudWallet, { cause: new Error(), description: ResponseMessages.errorMessages.serverError }); }
272-284: Await token encryption; currently storing a Promise.dataEncryption is async elsewhere; be consistent here to avoid persisting a Promise.
- const cloudWalletResponse: ICloudWalletDetails = { + const encryptedAgentToken = await this.commonService.dataEncryption(createCloudWalletResponse.token); + const cloudWalletResponse: ICloudWalletDetails = { createdBy: userId, label, lastChangedBy: userId, tenantId: createCloudWalletResponse.id, type: CloudWalletType.SUB_WALLET, userId, - agentApiKey: this.commonService.dataEncryption(createCloudWalletResponse.token), + agentApiKey: encryptedAgentToken, agentEndpoint, email, key: walletKey, connectionImageUrl };
287-291: Propagate errors after handling.Without rethrowing, callers receive undefined on failures.
} catch (error) { this.logger.error(`[createCloudWallet] - error in create cloud wallet: ${error}`); await this.commonService.handleError(error); - } + throw error; + }
331-334: Propagate errors after handling.Maintain consistent error flow.
} catch (error) { this.logger.error(`[createCloudWallet] - error in receive invitation: ${error}`); await this.commonService.handleError(error); - } + throw error; + }
374-377: Propagate errors after handling.Maintain consistent error flow.
} catch (error) { this.logger.error(`[receiveInvitationByUrl] - error in accept offer: ${error}`); await this.commonService.handleError(error); - } + throw error; + }
417-420: Propagate errors after handling.Maintain consistent error flow.
} catch (error) { this.logger.error(`[createDid] - error in create DID: ${error}`); await this.commonService.handleError(error); - } + throw error; + }
🧹 Nitpick comments (64)
apps/api-gateway/src/utilities/utilities.controller.ts (1)
55-79: Optional: Prefer framework-managed responses over manual@Res.Dropping
@Resenables interceptors/transformers/exception filters to work uniformly and simplifies testing. Not blocking if you need raw Express.apps/user/src/user.service.ts (1)
896-897: Consider adding proper error handling for the NATS send operation.The
userServiceProxy.send()call uses the older pattern without the newernatsClientwrapper that's used elsewhere in the service. This inconsistency could lead to different error handling behaviors.Consider updating this to use the consistent
natsClientpattern like other NATS calls in this service:- const getEcosystemConfigDetails = await this.userServiceProxy - .send(pattern, payload) + const getEcosystemConfigDetails = await this.natsClient + .send(this.userServiceProxy, pattern, payload)apps/api-gateway/src/platform/platform.service.ts (1)
1-15: Re-introduce a concrete type for platformServiceProxy (ClientProxy) to preserve type-safety.Dropping the type to implicit any weakens IDE help and compile-time checks. Typing the injected proxy keeps parity with NestJS patterns while still working with NATSClient.
Apply this diff:
-import { Injectable, Inject } from '@nestjs/common'; +import { Injectable, Inject } from '@nestjs/common'; +import { ClientProxy } from '@nestjs/microservices'; @@ - constructor( - @Inject('NATS_CLIENT') private readonly platformServiceProxy, - private readonly natsClient: NATSClient - ) { + constructor( + @Inject('NATS_CLIENT') private readonly platformServiceProxy: ClientProxy, + private readonly natsClient: NATSClient + ) {apps/organization/src/organization.service.ts (2)
74-74: Avoid implicit any: type the injected NATS client proxyTyping preserves compile‑time safety without changing runtime behavior.
Apply this diff for the constructor param:
- @Inject('NATS_CLIENT') private readonly organizationServiceProxy, + @Inject('NATS_CLIENT') private readonly organizationServiceProxy: ClientProxy,Add the missing import:
+import { ClientProxy } from '@nestjs/microservices';
1790-1794: Use NATSClient instead of raw ClientProxy + toPromiseThis aligns with the rest of the file, avoids RxJS dependency, and standardizes error handling.
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - const message = await this.organizationServiceProxy.send(pattern, payload).toPromise(); + const message = await this.natsClient.send(this.organizationServiceProxy, pattern, payload);apps/api-gateway/src/geo-location/geo-location.service.ts (2)
30-34: Fix misleading log message in getStatesByCountryId.It refers to “cities” and wrong method name.
- this.logger.log(`Finding cities for countryId= ${countryId},GeoLocationService::getCitiesByStateAndCountry`); + this.logger.log(`Finding states for countryId= ${countryId},GeoLocationService::getStatesByCountryId`);
8-11: Restore explicit type forserviceProxy(use ClientProxy)Add an explicit ClientProxy type to avoid an implicit any and keep the DI contract clear — tsconfig files do not enable "noImplicitAny", so this is a clarity/type-safety change (not a build blocker).
- constructor( - @Inject('NATS_CLIENT') private readonly serviceProxy, - private readonly natsClient: NATSClient - ) { + constructor( + @Inject('NATS_CLIENT') private readonly serviceProxy: ClientProxy, + private readonly natsClient: NATSClient + ) {import { Inject, Injectable } from '@nestjs/common'; +import { ClientProxy } from '@nestjs/microservices';apps/verification/src/verification.service.ts (7)
63-63: Type the injected NATS client to retain compile-time safety.Keeping this untyped weakens DI safety and downstream call signatures. Prefer explicit ClientProxy typing.
Apply this diff for the constructor param:
- @Inject('NATS_CLIENT') private readonly verificationServiceProxy, + @Inject('NATS_CLIENT') private readonly verificationServiceProxy: ClientProxy,Add/update the import (outside this range):
import { ClientProxy, RpcException } from '@nestjs/microservices';Verify the provider behind 'NATS_CLIENT' is still a ClientProxy in the upgraded NestJS setup.
993-1016: Replace deprecated toPromise and RxJS wrapping; make natsCall generic and await the Promise.natsClient.send already returns a Promise. This removes deprecated toPromise usage and unnecessary from/map, and fixes the overly narrow
response: stringtyping.- async natsCall( - pattern: object, - payload: object - ): Promise<{ - response: string; - }> { - return from(this.natsClient.send<string>(this.verificationServiceProxy, pattern, payload)) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { - this.logger.error(`catch: ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.statusCode, - error: error.error, - message: error.message - }, - error.error - ); - }); - } + async natsCall<T>( + pattern: object, + payload: object + ): Promise<{ response: T }> { + try { + const response = await this.natsClient.send<T>(this.verificationServiceProxy, pattern, payload); + return { response }; + } catch (error) { + this.logger.error(`catch: ${JSON.stringify(error)}`); + throw new HttpException( + { + status: error.statusCode, + error: error.error, + message: error.message + }, + error.error + ); + } + }Follow-ups (outside this range):
- Remove now-unused imports:
mapfrom 'rxjs/operators' andfromfrom 'rxjs'.- If the upgrade moved you to RxJS 8 anywhere else, this avoids build breaks from removed toPromise.
980-991: Fix boolean logic in verificationErrorHandling.Current condition uses ANDs, so it only triggers when error is falsy. Use ORs to handle partial/missing shapes.
- verificationErrorHandling(error): void { - if (!error && !error?.status && !error?.status?.message && !error?.status?.message?.error) { + verificationErrorHandling(error): void { + if (!error || !error?.status || !error?.status?.message || !error?.status?.message?.error) { throw new RpcException(error.response ? error.response : error); } else { throw new RpcException({ message: error?.status?.message?.error?.reason ? error?.status?.message?.error?.reason : error?.status?.message?.error, statusCode: error?.status?.code }); } }
513-540: Use else-if for mutually exclusive proof types to avoid accidental override of payload.Prevents a future edit from accidentally executing both branches.
- if (ProofRequestType.PRESENTATIONEXCHANGE === type) { + else if (ProofRequestType.PRESENTATIONEXCHANGE === type) { payload = { orgId: user.orgId, url, proofRequestPayload: { goalCode: outOfBandRequestProof.goalCode,
1043-1047: Drop unnecessary Promise.all over a synchronous increment.This is synchronous work; a simple loop is clearer and cheaper.
- await Promise.all( - deleteProofRecords.recordsToDelete.map(async (record) => { - statusCounts[record.state]++; - }) - ); + for (const record of deleteProofRecords.recordsToDelete) { + statusCounts[record.state]++; + }
680-687: Tighten the return type of _sendOutOfBandProofRequest.The caller accesses response.invitationUrl/outOfBandRecord/proofRecordThId. Reflect that in the signature to reduce any-leaks.
- async _sendOutOfBandProofRequest(payload: IProofRequestPayload): Promise<{ - response; - }> { + async _sendOutOfBandProofRequest(payload: IProofRequestPayload): Promise<{ + response: { invitationUrl: string; outOfBandRecord?: { id: string }; proofRecordThId?: string }; + }> {Alternatively, rely on the generic natsCall and pass a dedicated interface for the expected response.
964-977: Consider narrowing the type returned by _getOrgAgentApiKey.If the NATS response is a string API key, type it accordingly and avoid
any.- // eslint-disable-next-line @typescript-eslint/no-explicit-any - const message = await this.natsClient.send<any>(this.verificationServiceProxy, pattern, payload); + const message = await this.natsClient.send<string>(this.verificationServiceProxy, pattern, payload);Please confirm the actual response shape of cmd 'get-org-agent-api-key'.
libs/user-activity/repositories/index.ts (3)
50-55: Tighten txnMetadata typing to avoid usingobject.Use a structural type to improve safety and autocompletion.
- txnMetadata: object, + txnMetadata: Record<string, unknown>,
29-47: Guard and clamplimitto prevent invalid/abusive queries.Negative, NaN, or very large limits can error or stress the DB.
async getRecentActivities(userId: string, limit: number): Promise<IUsersActivity[]> { - return this.prisma.user_activity.findMany({ + const safeLimit = Number.isFinite(limit) && limit > 0 ? Math.min(100, Math.trunc(limit)) : 20; + return this.prisma.user_activity.findMany({ where: { userId }, select: { id: true, userId: true, orgId: true, action: true, details: true, createDateTime: true, lastChangedDateTime: true }, orderBy: { createDateTime: 'desc' }, - take: limit + take: safeLimit }); }
50-57: Public method naming: drop leading underscore.This is a public API used across services; leading underscore suggests private. Consider renaming to orgDeletedActivity (follow-up change across call sites).
apps/api-gateway/src/verification/verification.service.ts (2)
132-140: Same here: drop .toPromise(), use lastValueFrom, and remove unused ESLint disable.Keeps behavior while future‑proofing for RxJS 8.
Apply:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - const message = await this.verificationServiceProxy.send(pattern, payload).toPromise(); - return message; + const message = await lastValueFrom(this.verificationServiceProxy.send(pattern, payload)); + return message;Consider standardizing via NATSClient:
return this.natsClient.sendNatsMessage(this.verificationServiceProxy, 'post-webhook-response-to-webhook-url', payload);
118-126: Replace deprecated .toPromise() with lastValueFrom and remove ESLint disableRepo uses rxjs ^7.8.1 (toPromise deprecated but present) — switch to lastValueFrom to future-proof and drop the needless eslint-disable.
Apply:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - const message = await this.verificationServiceProxy.send(pattern, payload).toPromise(); - return message; + const message = await lastValueFrom(this.verificationServiceProxy.send(pattern, payload)); + return message;Add import:
import { lastValueFrom } from 'rxjs';Optional: for consistency with this service, consider using the existing NATSClient wrapper instead:
return this.natsClient.sendNatsMessage(this.verificationServiceProxy, 'get-webhookurl', payload);apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (8)
28-35: Document enum in Swagger and tighten type for sortFieldAdds accurate Swagger docs and compile-time safety.
@ApiProperty({ - required: false + required: false, + enum: SortFields }) @Transform(({ value }) => trim(value)) @IsOptional() @IsEnum(SortFields) - sortField: string = SortFields.CREATED_DATE_TIME; + sortField: SortFields = SortFields.CREATED_DATE_TIME;
36-43: Optional: type sortBy to the enumKeeps DTOs consistent and type-safe.
- sortBy: string = SortValue.DESC; + sortBy: SortValue = SortValue.DESC;
59-63: Normalize search text (trim) for consistency with other DTOsOther DTOs trim search text; mirror that here.
@ApiProperty({ required: false }) @IsOptional() + @Transform(({ value }) => trim(value)) @Type(() => String) searchByText: string = '';
72-79: Optional: type sortBy to SortValueConsistency with other DTOs.
- sortBy: string = SortValue.DESC; + sortBy: SortValue = SortValue.DESC;
87-93: Avoid duplicate Swagger decorators on ledgerIdUse ApiPropertyOptional with example directly.
- @ApiProperty({ example: '1a7eac11-ff05-40d7-8351-4d7467687cad' }) - @ApiPropertyOptional() + @ApiPropertyOptional({ example: '1a7eac11-ff05-40d7-8351-4d7467687cad' })
100-104: Normalize search text (trim) for platform DTOAligns with other DTOs.
@ApiProperty({ required: false }) @IsOptional() + @Transform(({ value }) => trim(value)) @Type(() => String) searchByText: string = '';
111-118: Document enum and tighten type for sorting; consider naming consistencyImproves Swagger and typing; consider aligning property name with sortField used elsewhere.
@ApiProperty({ - required: false + required: false, + enum: SortFields }) @Transform(({ value }) => trim(value)) @IsOptional() @IsEnum(SortFields) - sorting: string = SortFields.CREATED_DATE_TIME; + sorting: SortFields = SortFields.CREATED_DATE_TIME;
119-121: Add validation and enum docs for sortByValueCurrently unvalidated and not documented as enum.
- @ApiProperty({ required: false }) - @IsOptional() - sortByValue: string = SortValue.DESC; + @ApiProperty({ enum: [SortValue.DESC, SortValue.ASC], required: false }) + @Transform(({ value }) => trim(value)) + @IsOptional() + @IsEnum(SortValue) + sortByValue: SortValue = SortValue.DESC;apps/api-gateway/src/fido/fido.service.ts (3)
21-27: Prefer HttpException mapping for HTTP layer consistency (avoid RpcException here).Align error handling with AuthzService so controllers get proper HTTP status codes.
Apply this diff:
-import { RpcException } from '@nestjs/microservices'; +import { RpcException } from '@nestjs/microservices'; +import { HttpException, HttpStatus } from '@nestjs/common'; @@ async generateRegistrationOption(deviceFlag: boolean, email: string): Promise<{ response: object }> { try { const payload = { deviceFlag, email }; return await this.natsClient.sendNats(this.fidoServiceProxy, 'generate-registration-options', payload); } catch (error) { - throw new RpcException(error.response); + if (error?.status && error?.response) { + throw new HttpException(error.response, error.status); + } + throw new HttpException(error?.message ?? 'Internal error', HttpStatus.INTERNAL_SERVER_ERROR); } }
14-19: Optional typing: annotate the injected proxy to avoid implicit any.If strict mode is enabled, consider adding a minimal interface or
unknownand narrowing within NATSClient.Example:
- constructor( - @Inject('NATS_CLIENT') private readonly fidoServiceProxy, + constructor( + @Inject('NATS_CLIENT') private readonly fidoServiceProxy: unknown, private readonly natsClient: NATSClient ) {
23-23: Unify NATS helpers — sendNats and sendNatsMessage differ
- sendNats(...) wraps the broker reply into { response } and uses .toPromise(); sendNatsMessage(...) returns the raw reply via firstValueFrom (send also returns raw).
- Action: pick one API and standardize. Recommended: use the raw-returning helper (send/sendNatsMessage), update callers that expect .response (or make sendNatsMessage return the wrapper), and replace .toPromise() with firstValueFrom for consistency.
Locations: libs/common/src/NATSClient.ts (sendNats / sendNatsMessage / send); affected callers include apps/api-gateway/src/fido/fido.service.ts and apps/api-gateway/src/issuance/issuance.service.ts.
apps/api-gateway/src/authz/authz.service.ts (1)
28-29: Optional typing: annotate the injected proxy to avoid implicit any.Keeps strict typing clean without changing behavior.
- constructor( - @Inject('NATS_CLIENT') private readonly authServiceProxy, + constructor( + @Inject('NATS_CLIENT') private readonly authServiceProxy: unknown, private readonly natsClient: NATSClient ) {apps/api-gateway/src/utilities/utilities.service.ts (2)
12-12: Fix logger name to avoid misleading logs.Constructor passes 'OrganizationService' to BaseService in UtilitiesService.
Apply this diff:
- super('OrganizationService'); + super('UtilitiesService');
8-11: Optional typing: annotate the injected proxy.Same pattern as other services; add a non-any type if strict mode is on.
- constructor( - @Inject('NATS_CLIENT') private readonly serviceProxy, + constructor( + @Inject('NATS_CLIENT') private readonly serviceProxy: unknown, private readonly natsClient: NATSClient ) {libs/common/package.json (1)
19-21: Move @nestjs/testing to devDependencies.This is a test-only package and shouldn’t be in runtime deps.
"dependencies": { - "@nestjs/axios": "catalog:", - "@nestjs/common": "catalog:", - "@nestjs/core": "catalog:", - "@nestjs/testing": "catalog:", - "@nestjs/microservices": "catalog:", - "@nestjs/swagger": "catalog:", + "@nestjs/axios": "catalog:", + "@nestjs/common": "catalog:", + "@nestjs/core": "catalog:", + "@nestjs/microservices": "catalog:", + "@nestjs/swagger": "catalog:", @@ - "devDependencies": { + "devDependencies": { + "@nestjs/testing": "catalog:",Also applies to: 32-37
apps/api-gateway/src/revocation/revocation.service.ts (1)
13-15: Optional typing: annotate the injected proxy.Keeps strict typing clean; mirrors other services.
- constructor( - @Inject('NATS_CLIENT') private readonly revocationServiceProxy, + constructor( + @Inject('NATS_CLIENT') private readonly revocationServiceProxy: unknown, private readonly natsClient: NATSClient ) {apps/api-gateway/src/webhook/webhook.controller.ts (1)
70-78: Return the BadRequestException from exceptionFactory (don’t throw inside).Throwing inside exceptionFactory is unconventional; return the exception instance so Nest can throw it.
Apply this diff:
- new ParseUUIDPipe({ - exceptionFactory: (): Error => { - throw new BadRequestException(ResponseMessages.organisation.error.invalidOrgId); - } - }) + new ParseUUIDPipe({ + exceptionFactory: () => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + })apps/api-gateway/src/fido/fido.controller.ts (1)
162-175: Swagger status mismatch: decorator says 201, handler returns 200.Align to 200 OK (or change handler to 201). Suggest 200 for “options” reads.
Apply this diff:
- @ApiResponse({ status: HttpStatus.CREATED, description: 'Success', type: ApiResponseDto }) + @ApiResponse({ status: HttpStatus.OK, description: 'Success', type: ApiResponseDto }) @@ - const finalResponse: IResponseType = { - statusCode: HttpStatus.OK, + const finalResponse: IResponseType = { + statusCode: HttpStatus.OK,apps/api-gateway/src/connection/connection.service.ts (1)
23-26: Type the injected proxy (optional).Keeping this untyped promotes ‘any’ through call sites. Consider typing as ClientProxy.
Example:
import { ClientProxy } from '@nestjs/microservices'; // ... constructor(@Inject('NATS_CLIENT') private readonly connectionServiceProxy: ClientProxy, private readonly natsClient: NATSClient) { ... }apps/agent-service/src/agent-service.service.ts (1)
2063-2077: Simplify natsCall and drop toPromise().Use await on the Promise returned by NATSClient; mapping is trivial.
Apply this diff:
- try { - return from(this.natsClient.send<string>(this.agentServiceProxy, pattern, payload)) - .pipe(map((response) => ({ response }))) - .toPromise() - .catch((error) => { - this.logger.error(`catch: ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.statusCode, - error: error.message - }, - error.error - ); - }); - } catch (error) { + try { + const response = await this.natsClient.send<string>(this.agentServiceProxy, pattern, payload); + return { response }; + } catch (error) { this.logger.error(`[natsCall] - error in nats call : ${JSON.stringify(error)}`); throw error; }apps/api-gateway/src/notification/notification.service.ts (1)
9-14: Avoid implicit any on injected proxyExplicitly type the injected proxy (even as
any) to preserve type-safety and avoid noImplicitAny failures.Apply this diff:
- constructor( - @Inject('NATS_CLIENT') private readonly serviceProxy, - private readonly natsClient: NATSClient - ) { + constructor( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + @Inject('NATS_CLIENT') private readonly serviceProxy: any, + private readonly natsClient: NATSClient + ) {apps/connection/src/connection.service.ts (1)
40-40: Avoid implicit any on injected proxyAdd an explicit type (at least
any) to the injected NATS proxy.- @Inject('NATS_CLIENT') private readonly connectionServiceProxy, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + @Inject('NATS_CLIENT') private readonly connectionServiceProxy: any,apps/api-gateway/src/agent-service/agent-service.service.ts (1)
16-16: Avoid implicit any on injected proxyKeep an explicit type for
agentServiceProxy(evenany) to avoid losing type checks.- @Inject('NATS_CLIENT') private readonly agentServiceProxy, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + @Inject('NATS_CLIENT') private readonly agentServiceProxy: any,apps/ledger/src/schema/schema.service.ts (1)
65-65: Avoid implicit any on injected proxyAdd an explicit type for
schemaServiceProxyto avoid implicit any.- @Inject('NATS_CLIENT') private readonly schemaServiceProxy, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + @Inject('NATS_CLIENT') private readonly schemaServiceProxy: any,apps/api-gateway/src/credential-definition/credential-definition.controller.ts (2)
59-66: Use 200 OK for GET ApiResponseGET should document 200 OK, not 201 CREATED.
- @ApiResponse({ status: HttpStatus.CREATED, description: 'Success', type: ApiResponseDto }) + @ApiResponse({ status: HttpStatus.OK, description: 'Success', type: ApiResponseDto })
85-96: Fix route typo:/verifiation→/verificationThe path has a typo that will affect routing.
- @Get('/verifiation/cred-defs/:schemaId') + @Get('/verification/cred-defs/:schemaId')apps/api-gateway/src/schema/schema.service.ts (1)
20-24: Avoid implicit any on injected proxyAdd a type to
schemaServiceProxyto avoid implicit any.- constructor( - @Inject('NATS_CLIENT') private readonly schemaServiceProxy, - private readonly natsClient: NATSClient - ) { + constructor( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + @Inject('NATS_CLIENT') private readonly schemaServiceProxy: any, + private readonly natsClient: NATSClient + ) {apps/api-gateway/src/organization/organization.service.ts (1)
30-30: Avoid implicit any on injected proxyKeep an explicit type on
serviceProxy.- @Inject('NATS_CLIENT') private readonly serviceProxy, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + @Inject('NATS_CLIENT') private readonly serviceProxy: any,apps/issuance/src/issuance.service.ts (2)
101-101: Avoid implicit any on injected proxyType the injected proxy to avoid implicit any.
- @Inject('NATS_CLIENT') private readonly issuanceServiceProxy, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + @Inject('NATS_CLIENT') private readonly issuanceServiceProxy: any,
527-527: Unify NATS calls and add timeout handlingPrefer using the NATSClient wrapper consistently (it likely centralizes timeouts/retries). Also avoid Observable + toPromise; use async/await.
- .send(pattern, payload) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() + .send(pattern, payload) + .pipe(map((response) => ({ response }))) + .toPromise()Or, better, route through NATSClient:
- return this.issuanceServiceProxy - .send(pattern, payload) - .pipe(map((response) => ({ response }))) - .toPromise() + const resp = await this.natsClient.send(this.issuanceServiceProxy, pattern, payload); + return { response: resp };apps/api-gateway/src/credential-definition/credential-definition.service.ts (2)
13-15: Retain a concrete type for the injected NATS proxy.Dropping the explicit type weakens type-safety and IDE support. Consider typing this as ClientProxy (or a thin interface) for compile-time checks.
+import { ClientProxy } from '@nestjs/microservices'; ... - constructor( - @Inject('NATS_CLIENT') private readonly credDefServiceProxy, - private readonly natsClient: NATSClient - ) { + constructor( + @Inject('NATS_CLIENT') private readonly credDefServiceProxy: ClientProxy, + private readonly natsClient: NATSClient + ) {
24-27: Strongly type the return as ICredDef instead of object.The method fetches one credential definition; prefer ICredDef over object to preserve API guarantees.
- getCredentialDefinitionById(credentialDefinitionId: string, orgId: string): Promise<object> { + getCredentialDefinitionById(credentialDefinitionId: string, orgId: string): Promise<ICredDef> { const payload = { credentialDefinitionId, orgId }; return this.natsClient.sendNatsMessage(this.credDefServiceProxy, 'get-credential-definition-by-id', payload); }apps/api-gateway/src/schema/schema.controller.ts (3)
77-79: Validate orgId with ParseUUIDPipe for consistency.Other endpoints validate orgId; apply the same here to avoid inconsistent input handling.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => { + throw new BadRequestException(ResponseMessages.organisation.error.invalidOrgId); + } + }) + ) + orgId: string,
95-106: Fix minor typos in API descriptions.“Retrives” → “Retrieves”. Improves API docs polish.
- description: 'Retrives credential definition list by schema Id available on platform.' + description: 'Retrieves credential definition list by schema Id available on platform.'
116-129: Rename handler to follow camelCase and fix double f.Method name “getcredDeffListBySchemaId” is inconsistent; prefer “getCredDefListBySchemaId”. No route changes.
- async getcredDeffListBySchemaId( + async getCredDefListBySchemaId( ... - const credentialDefinitionList = await this.appService.getcredDefListBySchemaId( + const credentialDefinitionList = await this.appService.getcredDefListBySchemaId(apps/cloud-wallet/src/cloud-wallet.service.ts (1)
48-52: Remove unused injected proxy from constructor.cloudWalletServiceProxy is never used in this service; drop it to avoid dead dependencies.
export class CloudWalletService { constructor( private readonly commonService: CommonService, - @Inject('NATS_CLIENT') private readonly cloudWalletServiceProxy, private readonly cloudWalletRepository: CloudWalletRepository, private readonly logger: Logger, @Inject(CACHE_MANAGER) private cacheService: Cache ) {}apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts (2)
30-33: Keep the proxy typed for safer NATS usage.Type the injected proxy as ClientProxy (or a minimal interface) for better editor support and compile-time checks.
+import { ClientProxy } from '@nestjs/microservices'; ... - constructor( - @Inject('NATS_CLIENT') private readonly cloudWalletServiceProxy, - private readonly natsClient: NATSClient - ) { + constructor( + @Inject('NATS_CLIENT') private readonly cloudWalletServiceProxy: ClientProxy, + private readonly natsClient: NATSClient + ) {
100-113: Method names: prefer consistent camelCase (optional).Consider getConnectionById/getAllConnectionsById for readability.
apps/api-gateway/src/organization/organization.controller.ts (1)
579-589: Align documented status with actual response (201 vs 200).ApiResponse states 200 OK, but you return 201 Created. Align for accurate Swagger.
- @ApiResponse({ status: HttpStatus.OK, description: 'Success', type: ApiResponseDto }) + @ApiResponse({ status: HttpStatus.CREATED, description: 'Success', type: ApiResponseDto })apps/ledger/src/credential-definition/credential-definition.service.ts (3)
45-46: Consider adding null safety for the DID splitting logic.The DID string splitting logic assumes a specific format but doesn't validate the structure before accessing array elements. This could cause runtime errors if the DID format is unexpected.
Apply this defensive programming approach:
-const did = credDef.orgDid?.split(':').length >= 4 ? credDef.orgDid : orgDid; +const didParts = credDef.orgDid?.split(':'); +const did = didParts && didParts.length >= 4 ? credDef.orgDid : orgDid;
285-286: Fix error log message typo.The error message incorrectly states "creating credential definition" when this method is actually retrieving one.
-this.logger.error(`Error in creating credential definition : ${JSON.stringify(error)}`); +this.logger.error(`Error in getting credential definition : ${JSON.stringify(error)}`);
239-241: Add null safety for nested property access.The code accesses
credDefResponse.response.resolutionMetadata.errorwithout checking if intermediate properties exist.-if (credDefResponse.response.resolutionMetadata.error) { +if (credDefResponse?.response?.resolutionMetadata?.error) {apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts (3)
427-447: Fix typo in method name.The method name
getconnectionByIdshould follow camelCase convention asgetConnectionById.-async getconnectionById( +async getConnectionById(
462-479: Fix typo in method name.The method name
getAllconnectionByIdis misleading and has incorrect casing. It should begetAllConnectionssince it's retrieving multiple connections, not a single one by ID.-async getAllconnectionById( +async getAllConnections(
77-101: Consider adding error handling for base wallet configuration.The
configureBaseWalletendpoint doesn't have try-catch error handling unlike other endpoints. Consider wrapping the service call for consistency.async configureBaseWallet( @Res() res: Response, @Body() cloudBaseWalletConfigure: CloudBaseWalletConfigureDto, @User() user: user ): Promise<Response> { const { id, email } = user; cloudBaseWalletConfigure.userId = id; cloudBaseWalletConfigure.email = email; + try { const configureBaseWalletData = await this.cloudWalletService.configureBaseWallet(cloudBaseWalletConfigure); const finalResponse: IResponse = { statusCode: HttpStatus.CREATED, message: ResponseMessages.cloudWallet.success.configureBaseWallet, data: configureBaseWalletData }; return res.status(HttpStatus.CREATED).json(finalResponse); + } catch (error) { + this.logger.error(`Error in configureBaseWallet: ${JSON.stringify(error)}`); + throw error; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
apps/agent-service/src/agent-service.service.ts(1 hunks)apps/api-gateway/src/agent-service/agent-service.controller.ts(1 hunks)apps/api-gateway/src/agent-service/agent-service.service.ts(1 hunks)apps/api-gateway/src/agent/agent.controller.ts(11 hunks)apps/api-gateway/src/agent/agent.service.ts(1 hunks)apps/api-gateway/src/app.module.ts(1 hunks)apps/api-gateway/src/app.service.ts(1 hunks)apps/api-gateway/src/authz/authz.controller.ts(2 hunks)apps/api-gateway/src/authz/authz.service.ts(1 hunks)apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts(2 hunks)apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts(1 hunks)apps/api-gateway/src/connection/connection.controller.ts(8 hunks)apps/api-gateway/src/connection/connection.service.ts(3 hunks)apps/api-gateway/src/credential-definition/credential-definition.controller.ts(10 hunks)apps/api-gateway/src/credential-definition/credential-definition.service.ts(2 hunks)apps/api-gateway/src/dtos/save-roles-permissions.dto.ts(1 hunks)apps/api-gateway/src/fido/fido.controller.ts(2 hunks)apps/api-gateway/src/fido/fido.service.ts(1 hunks)apps/api-gateway/src/geo-location/geo-location.service.ts(1 hunks)apps/api-gateway/src/issuance/issuance.controller.ts(2 hunks)apps/api-gateway/src/issuance/issuance.service.ts(0 hunks)apps/api-gateway/src/main.ts(1 hunks)apps/api-gateway/src/notification/notification.controller.ts(2 hunks)apps/api-gateway/src/notification/notification.service.ts(1 hunks)apps/api-gateway/src/organization/organization.controller.ts(1 hunks)apps/api-gateway/src/organization/organization.service.ts(1 hunks)apps/api-gateway/src/platform/platform.service.ts(1 hunks)apps/api-gateway/src/revocation/revocation.service.ts(2 hunks)apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts(1 hunks)apps/api-gateway/src/schema/schema.controller.ts(8 hunks)apps/api-gateway/src/schema/schema.service.ts(3 hunks)apps/api-gateway/src/user/user.controller.ts(1 hunks)apps/api-gateway/src/user/user.service.ts(1 hunks)apps/api-gateway/src/utilities/utilities.controller.ts(3 hunks)apps/api-gateway/src/utilities/utilities.service.ts(2 hunks)apps/api-gateway/src/verification/verification.controller.ts(9 hunks)apps/api-gateway/src/verification/verification.service.ts(3 hunks)apps/api-gateway/src/webhook/webhook.controller.ts(3 hunks)apps/api-gateway/src/webhook/webhook.service.ts(1 hunks)apps/cloud-wallet/src/cloud-wallet.service.ts(1 hunks)apps/connection/src/connection.service.ts(2 hunks)apps/issuance/src/issuance.service.ts(3 hunks)apps/ledger/src/credential-definition/credential-definition.service.ts(4 hunks)apps/ledger/src/schema/schema.service.ts(2 hunks)apps/organization/src/organization.service.ts(4 hunks)apps/user/src/user.service.ts(1 hunks)apps/verification/src/verification.service.ts(2 hunks)libs/aws/package.json(1 hunks)libs/common/package.json(1 hunks)libs/common/src/NATSClient.ts(4 hunks)libs/user-activity/repositories/index.ts(1 hunks)package.json(3 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api-gateway/src/issuance/issuance.service.ts
🧰 Additional context used
🧬 Code graph analysis (20)
apps/api-gateway/src/agent/agent.controller.ts (2)
apps/api-gateway/src/dtos/wallet-details.dto.ts (1)
WalletDetailsDto(3-17)apps/agent-service/src/interface/agent-service.interface.ts (1)
IUserRequestInterface(237-250)
apps/api-gateway/src/credential-definition/credential-definition.controller.ts (4)
apps/api-gateway/src/agent-service/agent-service.controller.ts (1)
UseFilters(54-449)apps/api-gateway/src/organization/organization.controller.ts (1)
UseFilters(58-792)apps/api-gateway/src/schema/schema.controller.ts (1)
UseFilters(50-262)libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-518)
apps/api-gateway/src/schema/schema.controller.ts (6)
libs/common/src/cast.helper.ts (1)
TrimStringParamPipe(167-172)libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-518)apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (1)
GetCredentialDefinitionBySchemaIdDto(46-84)apps/api-gateway/src/schema/interfaces/index.ts (1)
IUserRequestInterface(3-16)libs/common/src/interfaces/response.interface.ts (1)
IResponse(8-13)apps/api-gateway/src/schema/dtos/update-schema-dto.ts (1)
UpdateSchemaDto(5-23)
apps/api-gateway/src/connection/connection.service.ts (4)
apps/issuance/src/issuance.service.ts (1)
Injectable(95-2074)apps/verification/src/verification.service.ts (1)
Injectable(58-1071)apps/webhook/src/webhook.service.ts (1)
Injectable(17-113)libs/service/base.service.ts (1)
BaseService(2-9)
apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts (3)
apps/api-gateway/src/agent-service/agent-service.service.ts (1)
Injectable(13-103)libs/service/base.service.ts (1)
BaseService(2-9)libs/common/src/interfaces/cloud-wallet.interface.ts (7)
ICloudBaseWalletConfigure(147-153)IGetStoredWalletInfo(72-78)IGetStoredWalletInfo(103-109)IWalletDetailsForDidList(192-195)IConnectionDetailsById(197-201)GetAllCloudWalletConnections(311-319)IBasicMessage(321-325)
apps/api-gateway/src/utilities/utilities.controller.ts (1)
apps/api-gateway/src/utilities/dtos/shortening-url.dto.ts (1)
StoreObjectDto(43-47)
apps/api-gateway/src/webhook/webhook.service.ts (2)
apps/webhook/interfaces/webhook.interfaces.ts (2)
IGetWebhookUrl(5-7)ICreateWebhookUrl(1-3)apps/api-gateway/src/webhook/dtos/register-webhook-dto.ts (1)
RegisterWebhookDto(6-16)
apps/api-gateway/src/fido/fido.service.ts (3)
apps/api-gateway/src/agent-service/agent-service.service.ts (1)
Injectable(13-103)libs/service/base.service.ts (1)
BaseService(2-9)apps/api-gateway/src/dtos/fido-user.dto.ts (4)
VerifyRegistrationDto(31-58)GenerateAuthenticationDto(74-78)VerifyAuthenticationDto(99-126)UpdateFidoUserDetailsDto(60-72)
apps/api-gateway/src/notification/notification.service.ts (3)
apps/api-gateway/src/notification/dtos/notification.dto.ts (2)
RegisterOrgWebhhookEndpointDto(29-49)SendNotificationDto(51-70)apps/api-gateway/src/notification/interfaces/notification.interfaces.ts (1)
INotification(1-5)apps/notification/interfaces/notification.interfaces.ts (1)
INotification(3-7)
apps/ledger/src/credential-definition/credential-definition.service.ts (4)
apps/ledger/src/credential-definition/interfaces/create-credential-definition.interface.ts (6)
CreateCredDefPayload(19-23)CredDefPayload(35-49)IPlatformCredDefs(25-33)GetCredDefPayload(6-17)GetAllCredDefsPayload(59-63)GetCredDefBySchemaId(65-67)apps/ledger/src/credential-definition/interfaces/credential-definition.interface.ts (3)
CreateCredDefAgentRedirection(1-12)GetCredDefAgentRedirection(20-29)CredDefSchema(35-38)libs/common/src/interfaces/cred-def.interface.ts (2)
IPlatformCredDefsData(30-32)ICredDefDetails(1-3)libs/common/src/interfaces/schema.interface.ts (1)
ISchemaDetail(48-67)
apps/api-gateway/src/schema/schema.service.ts (4)
apps/api-gateway/src/schema/interfaces/index.ts (1)
IUserRequestInterface(3-16)libs/common/src/interfaces/schema.interface.ts (3)
ISchemaData(14-16)ISchemasWithPagination(10-12)ICredDefWithPagination(82-84)apps/api-gateway/src/interfaces/ISchemaSearch.interface.ts (1)
ISchemaSearchPayload(4-13)apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (1)
GetCredentialDefinitionBySchemaIdDto(46-84)
apps/api-gateway/src/credential-definition/credential-definition.service.ts (4)
apps/api-gateway/src/credential-definition/dto/create-cred-defs.dto.ts (1)
CreateCredentialDefinitionDto(6-33)apps/api-gateway/src/credential-definition/interfaces/index.ts (3)
IUserRequestInterface(3-15)ICredDef(42-52)ICredDefs(54-57)apps/ledger/src/credential-definition/interfaces/create-credential-definition.interface.ts (1)
GetAllCredDefsDto(51-58)libs/common/src/interfaces/schema.interface.ts (1)
ICredDefData(74-80)
apps/api-gateway/src/agent/agent.service.ts (3)
apps/api-gateway/src/agent-service/agent-service.service.ts (1)
Injectable(13-103)libs/service/base.service.ts (1)
BaseService(2-9)apps/api-gateway/src/dtos/wallet-details.dto.ts (1)
WalletDetailsDto(3-17)
apps/api-gateway/src/connection/connection.controller.ts (2)
apps/api-gateway/src/connection/connection.service.ts (2)
receiveInvitationUrl(96-103)receiveInvitation(105-112)apps/connection/src/connection.service.ts (2)
receiveInvitationUrl(333-367)receiveInvitation(393-420)
apps/api-gateway/src/fido/fido.controller.ts (7)
libs/logger/src/logger.interface.ts (1)
Logger(6-20)apps/api-gateway/src/authz/decorators/roles.decorator.ts (1)
Roles(6-6)apps/api-gateway/src/fido/dto/fido-user.dto.ts (7)
Response(40-44)Response(77-82)GenerateRegistrationDto(5-15)VerifyRegistrationDto(17-38)GenerateAuthenticationDto(100-103)VerifyAuthenticationDto(54-75)UpdateFidoUserDetailsDto(88-98)libs/common/src/interfaces/response.interface.ts (1)
IResponseType(1-7)libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-518)apps/api-gateway/src/dtos/fido-user.dto.ts (5)
GenerateRegistrationDto(4-10)VerifyRegistrationDto(31-58)GenerateAuthenticationDto(74-78)VerifyAuthenticationDto(99-126)UpdateFidoUserDetailsDto(60-72)apps/api-gateway/src/fido/fido.service.ts (2)
verifyRegistration(29-32)verifyAuthentication(40-46)
apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts (5)
apps/api-gateway/src/cloud-wallet/dtos/configure-base-wallet.dto.ts (1)
CloudBaseWalletConfigureDto(6-26)apps/api-gateway/src/cloud-wallet/dtos/cloudWallet.dto.ts (7)
CreateCloudWalletDto(7-27)ReceiveInvitationUrlDTO(29-96)AcceptOfferDto(98-121)CreateCloudWalletDidDto(123-192)GetAllCloudWalletConnectionsDto(218-252)CredentialListDto(194-216)BasicMessageDTO(254-266)apps/cloud-wallet/src/cloud-wallet.service.ts (3)
acceptProofRequest(122-144)acceptOffer(341-377)createConnection(97-115)apps/api-gateway/src/cloud-wallet/dtos/accept-proof-request.dto.ts (1)
AcceptProofRequestDto(4-29)libs/common/src/interfaces/cloud-wallet.interface.ts (6)
IGetProofPresentationById(135-139)IGetProofPresentation(141-145)IWalletDetailsForDidList(192-195)IConnectionDetailsById(197-201)ICredentialDetails(211-215)IBasicMessage(321-325)
apps/api-gateway/src/notification/notification.controller.ts (6)
apps/api-gateway/src/notification/dtos/notification.dto.ts (2)
RegisterOrgWebhhookEndpointDto(29-49)SendNotificationDto(51-70)libs/common/src/interfaces/response.interface.ts (1)
IResponse(8-13)libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-518)apps/api-gateway/src/notification/notification.service.ts (1)
sendNotification(36-38)apps/notification/src/notification.controller.ts (1)
sendNotification(26-28)apps/notification/src/notification.service.ts (1)
sendNotification(43-77)
libs/user-activity/repositories/index.ts (6)
apps/issuance/src/issuance.service.ts (1)
Injectable(95-2074)apps/organization/src/organization.service.ts (1)
Injectable(69-2032)apps/agent-service/src/agent-service.service.ts (1)
Injectable(83-2092)apps/verification/src/verification.service.ts (1)
Injectable(58-1071)libs/user-activity/src/user-activity.service.ts (1)
Injectable(7-22)libs/user-activity/interface/index.ts (1)
IUsersActivity(1-9)
apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (1)
libs/common/src/cast.helper.ts (2)
trim(39-43)toNumber(58-76)
apps/api-gateway/src/platform/platform.service.ts (5)
apps/api-gateway/src/interfaces/ISchemaSearch.interface.ts (2)
ISchemaSearchPayload(4-13)ILedgers(27-39)apps/agent-service/src/interface/agent-service.interface.ts (2)
IUserRequestInterface(237-250)ILedgers(647-659)libs/common/src/interfaces/schema.interface.ts (2)
ISchemaDetails(18-20)INetworkUrl(91-93)apps/api-gateway/src/credential-definition/dto/get-all-platform-cred-defs.dto.ts (1)
GetAllPlatformCredDefsDto(8-30)libs/common/src/interfaces/cred-def.interface.ts (1)
IPlatformCredDefsData(30-32)
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
apps/api-gateway/src/agent/agent.service.ts (10)
14-19: Type the injected NATS client proxy to preserve type-safety and editor supportKeeping
agentServiceProxyasClientProxyavoids silent breakages and improves DX.Apply this diff:
- constructor( - @Inject('NATS_CLIENT') private readonly agentServiceProxy, - private readonly natsClient: NATSClient - ) { + constructor( + @Inject('NATS_CLIENT') private readonly agentServiceProxy: ClientProxy, + private readonly natsClient: NATSClient + ) {Add the import (outside the selected lines):
import { ClientProxy } from '@nestjs/microservices';
27-31: Use debug level for routine traces; optionally add explicit return typeThese are high-frequency calls; prefer
debugto reduce log noise. Consider: Promise<unknown>for the return type.- this.logger.log('**** getAllDid called...'); + this.logger.debug('**** getAllDid called...');
36-39: Demote to debug log for routine operationKeeps info logs clean while retaining traceability.
- this.logger.log('**** createLocalDid called...'); + this.logger.debug('**** createLocalDid called...');
50-53: Use debug for routine trace- this.logger.log('**** getPublicDid called...'); + this.logger.debug('**** getPublicDid called...');
65-76: Update JSDoc to match current params; consider aligning id type with assignPublicDidThe comment still mentions role/alias/verkey/did, but the method takes only
idanduser.- /** - * Description: Calling agent service for onboard-register-ledger - * @param role - * @param alias - * @param verkey - * @param did - */ + /** + * Description: Calling agent service to register NYM for an org + * @param id Organization identifier + * @param user Requesting user context + */Also verify whether
idhere should benumber(like assignPublicDid) orstring, and unify.
78-81: Narrow the action parameter to a union typePrevents invalid inputs at compile time.
- restartStopAgent(action: string, orgId: string) { + restartStopAgent(action: 'restart' | 'stop', orgId: string) {
83-85: Add parameter and return types for clarityMinimal type annotations improve readability without changing behavior.
- getAgentServerStatus(user) { + getAgentServerStatus(user: any): Promise<unknown> {
87-91: Use debug for heartbeat pingsThese are frequent and not actionable at info level.
- this.logger.log('**** pingServiceAgent called...'); + this.logger.debug('**** pingServiceAgent called...');
93-104: Prefer a DTO for pagination/filter params; demote log level to debugPassing many positional args is brittle; a DTO keeps it cohesive and future-proof. If snake_case is required by downstream, keep it within the DTO shape.
- this.logger.log('**** agentSpinupStatus called...'); + this.logger.debug('**** agentSpinupStatus called...');Optional DTO example (outside diff):
export interface AgentSpinupStatusDto { items_per_page: number; page: number; search_text: string; agentStatus: string; sortValue: string; user: any; }Then:
agentSpinupStatus(params: AgentSpinupStatusDto) { ... }Confirm whether snake_case keys are required over the wire; if not, prefer camelCase locally and map on publish.
27-104: Extract NATS subject strings to a central constant/enumReduces typo risk across methods and eases future renames.
Example (outside diff):
export const AgentSubjects = { GET_ALL_DID: 'get-all-did', CREATE_LOCAL_DID: 'create-local-did', WALLET_PROVISION: 'wallet-provision', GET_PUBLIC_DID: 'get-public-did', ASSIGN_PUBLIC_DID_ORG: 'assign-public-did-org', REGISTER_NYM_ORG: 'register-nym-org', RESTART_STOP_AGENT: 'restart-stop-agent', GET_AGENT_SERVER_STATUS: 'get-agent-server-status', PING_AGENT: 'ping-agent', GET_AGENT_SPINUP_STATUS: 'get-agent-spinup-status' } as const;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api-gateway/src/agent/agent.service.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api-gateway/src/agent/agent.service.ts (3)
apps/api-gateway/src/authz/authz.service.ts (1)
Injectable(22-109)libs/service/base.service.ts (1)
BaseService(2-9)apps/api-gateway/src/dtos/wallet-details.dto.ts (1)
WalletDetailsDto(3-17)
🔇 Additional comments (1)
apps/api-gateway/src/agent/agent.service.ts (1)
41-45: Drop unnecessary async and clarify safe log label (prior secret‑logging concern addressed)No
awaitis used;asyncis redundant. Also add a label to the wallet name in logs. Good job not logging sensitive fields.- async walletProvision(walletUserDetails: WalletDetailsDto, user: any) { - this.logger.log(`**** walletProvision called...${walletUserDetails.walletName}`); + walletProvision(walletUserDetails: WalletDetailsDto, user: any) { + this.logger.debug(`**** walletProvision called... walletName=${walletUserDetails.walletName}`); const payload = { walletUserDetails, user }; return this.natsClient.sendNats(this.agentServiceProxy, 'wallet-provision', payload); }
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
apps/cloud-wallet/src/cloud-wallet.controller.ts (1)
96-111: Inconsistent return type usage across endpoints.Several methods still return generic
Responsetypes (lines 63, 68, 73, 83, 88, 93, 98, 103, 108) whilegetDidListwas updated to use typedIProofRequestRes[]. For consistency and better type safety, consider updating the other endpoints to use appropriate typed interfaces.Consider updating other endpoints to use typed return values similar to
getDidList:-async receiveInvitationByUrl(ReceiveInvitationDetails: IReceiveInvitation): Promise<Response> { +async receiveInvitationByUrl(ReceiveInvitationDetails: IReceiveInvitation): Promise<IConnectionInvitationResponse> {Apply similar changes to other methods based on their actual return data structures.
apps/api-gateway/src/connection/connection.service.ts (8)
24-27: Type the injected NATS client for safety.
Loosening to any loses generics and compile‑time checks on send(). Prefer ClientProxy.Apply this change in the constructor:
- @Inject('NATS_CLIENT') private readonly connectionServiceProxy, + @Inject('NATS_CLIENT') private readonly connectionServiceProxy: ClientProxy,Add the import:
import { ClientProxy } from '@nestjs/microservices';
31-37: Harden error rethrow (error.response may be undefined).
Ensure a useful RpcException payload in all cases.- } catch (error) { - throw new RpcException(error.response); - } + } catch (error) { + throw new RpcException(error?.response ?? error?.message ?? error); + }
39-49: Align error handling with above.
Same concern in sendBasicMessage.- } catch (error) { - throw new RpcException(error.response); - } + } catch (error) { + throw new RpcException(error?.response ?? error?.message ?? error); + }
51-54: Prefer a concrete return type over Promise.
Consider defining/using a DTO or interface for the webhook response to keep the contract explicit.
93-95: Return type Promise — consider an explicit model.
Define a QuestionAnswersRecord interface for stronger contracts.
124-126: Avoid JSON.stringify(error) in logs.
Stringifying can throw on circular refs and loses stack. Prefer passing the error object.- this.logger.error(`catch: ${JSON.stringify(error)}`); + this.logger.error('catch', error);
137-139: Same logging concern here.
Use structured logging with the error object.- this.logger.error(`catch: ${JSON.stringify(error)}`); + this.logger.error('catch', error);
84-91: Unify duplicated user types: use a single IUserRequest across services.
I found both IUserRequest (libs/user-request/src/user-request.interface.ts) and IUserRequestInterface (apps/api-gateway/src/connection/interfaces/index.ts) with effectively the same fields; apps/api-gateway/src/connection/connection.service.ts imports/uses both — pick one canonical interface (recommend the shared libs/user-request) and update usages to avoid duplication and drift.📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api-gateway/src/connection/connection.service.ts(3 hunks)apps/cloud-wallet/src/cloud-wallet.controller.ts(4 hunks)apps/cloud-wallet/src/cloud-wallet.service.ts(3 hunks)🧰 Additional context used
🧬 Code graph analysis (2)
apps/cloud-wallet/src/cloud-wallet.controller.ts (1)
libs/common/src/interfaces/cloud-wallet.interface.ts (2)
IWalletDetailsForDidList(192-195)IProofRequestRes(162-173)apps/cloud-wallet/src/cloud-wallet.service.ts (1)
libs/common/src/interfaces/cloud-wallet.interface.ts (2)
IWalletDetailsForDidList(192-195)IProofRequestRes(162-173)🔇 Additional comments (12)
apps/cloud-wallet/src/cloud-wallet.controller.ts (2)
5-26: Import reorganization looks good.The imports have been reorganized into a cleaner multi-line format. This improves readability without affecting functionality.
78-78: Return type change aligns with interface standards.The change from generic
Responseto typedIProofRequestRes[]improves type safety and aligns with NestJS 11's enhanced typing capabilities. This matches the corresponding service implementation change.apps/cloud-wallet/src/cloud-wallet.service.ts (4)
48-48: Constructor type loosening aligns with NestJS upgrade patterns.The removal of explicit
ClientProxytyping follows the broader pattern across this PR for NestJS version updates. This provides better flexibility with dependency injection while maintaining runtime type safety through the@Inject('NATS_CLIENT')decorator.
427-427: Return type improvement enhances type safety.The change from
Promise<Response>toPromise<IProofRequestRes[]>provides better type safety and aligns with the controller interface change.
436-436: Good defensive programming with nullish coalescing.The use of nullish coalescing operator (
?? []) ensures a consistent array return type even when the HTTP GET request returns null or undefined. This prevents potential runtime errors in consuming code.
427-442: Return type mismatch — confirm /dids response and use a DID-specific interfaceIProofRequestRes (libs/common/src/interfaces/cloud-wallet.interface.ts) models proof requests but getDidList calls CommonConstants.URL_AGENT_GET_DID ('/dids') and no DID interface was found; confirm the agent actually returns IProofRequestRes[] — otherwise define an IDidRecord (or equivalent) and update signatures at apps/cloud-wallet/src/cloud-wallet.service.ts (getDidList), apps/cloud-wallet/src/cloud-wallet.controller.ts (getDidList), and apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts (getDidList).
apps/api-gateway/src/connection/connection.service.ts (6)
2-3: Nest imports look good.
No issues with these updates.
5-10: DTO imports consolidated — OK.
No concerns.
13-17: Interface imports — OK.
Looks consistent with usage below.
121-123: Replaced toPromise() with firstValueFrom — good fix.
This addresses the prior review note for RxJS 7+/Nest 11.
130-136: Replaced toPromise() with firstValueFrom — good fix (again).
Consistent with Nest 11/RxJS 7+.
56-65: getUrl: no change — sendNats already returns { response }
NATSClient.sendNats maps the reply into { response } (libs/common/src/NATSClient.ts), so getUrl's Promise<{ response: object }> matches the actual returned shape; the suggested wrapper is unnecessary.Likely an incorrect or invalid review comment.
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
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 (12)
libs/common/src/common.service.ts (1)
180-188: Remove duplicate error handling condition.Lines 170-178 and 180-188 contain identical error handling logic for
RESP_ERR_HTTP_ECONNREFUSED. This duplication should be removed to improve code maintainability.Apply this diff to remove the duplicate condition:
- if (error.message.toString().includes(CommonConstants.RESP_ERR_HTTP_ECONNREFUSED)) { - throw new HttpException( - { - statusCode: HttpStatus.NOT_FOUND, - error: error.message - }, - HttpStatus.NOT_FOUND - ); - }apps/issuance/src/issuance.service.ts (7)
88-90: Fix p-limit import to avoid runtime “is not a function”p-limit has a default export. Importing with
* aswill break at runtime.Apply:
-import * as pLimit from 'p-limit'; +import pLimit from 'p-limit';
692-699: Null-check before destructuring to prevent TypeError
agentDetailsis destructured before the null check. If it’s undefined, this throws before your NotFoundException.Apply:
- const { agentEndPoint } = agentDetails; - if (!agentDetails) { + if (!agentDetails) { throw new NotFoundException(ResponseMessages.issuance.error.agentEndPointNotFound); } + const { agentEndPoint } = agentDetails;
378-385: Null-check before destructuring (same issue here)Destructure
agentDetailsonly after verifying it exists.- const { agentEndPoint, organisation } = agentDetails; - - if (!agentDetails) { + if (!agentDetails) { throw new NotFoundException(ResponseMessages.issuance.error.agentEndPointNotFound); } + const { agentEndPoint, organisation } = agentDetails;
1264-1269: Papa Parse option name and email guard
- Option should be
transformHeader(camelCase).- Guard
email_identifierbeforetrim()to avoid NPE.- const parsedData = paParse(csvData, { + const parsedData = paParse(csvData, { header: true, skipEmptyLines: true, - transformheader: (header) => header.toLowerCase().replace('#', '').trim(), + transformHeader: (header) => header.toLowerCase().replace('#', '').trim(), complete: (results) => results.data }); … - const invalidEmails = parsedData.data.filter((entry) => !validateEmail(entry.email_identifier.trim())); + const invalidEmails = parsedData.data.filter((entry) => + !validateEmail(String(entry?.email_identifier ?? '').trim()) + );Also applies to: 1280-1284
1358-1363: Fix bad null-check and guard optional fields during search
cachedData === undefined || nullis always falsy when not undefined.item.namemay not exist; guard it.- if (cachedData === undefined || null) { + if (cachedData === undefined || cachedData === null) { throw new BadRequestException(ResponseMessages.issuance.error.previewCachedData); } … - filteredData = parsedData.filter( - (item) => - item.email_identifier.toLowerCase().includes(searchTerm) || item.name.toLowerCase().includes(searchTerm) - ); + filteredData = parsedData.filter((item) => { + const email = String(item?.email_identifier ?? '').toLowerCase(); + const name = String(item?.name ?? '').toLowerCase(); + return email.includes(searchTerm) || name.includes(searchTerm); + });Also applies to: 1367-1371
1591-1595: Wrong argument order: passing requestId where orgId is expectedThis will break ledger/agent lookups inside
uploadCSVTemplate.- await this.uploadCSVTemplate(reqPayload, requestId); + await this.uploadCSVTemplate(reqPayload, orgId, requestId);
527-544: Replace deprecated toPromise with firstValueFrom and add try/catchtoPromise is deprecated. Use firstValueFrom for RxJS 7+.
- return this.issuanceServiceProxy - .send(pattern, payload) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { - this.logger.error(`catch: ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.statusCode, - error: error.message - }, - error.error - ); - }); + try { + const response = await firstValueFrom( + this.issuanceServiceProxy.send(pattern, payload) + ); + return { response }; + } catch (error) { + this.logger.error(`catch: ${JSON.stringify(error)}`); + throw new HttpException( + { status: error.statusCode, error: error.message }, + error.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR + ); + }Also update imports:
-import { map } from 'rxjs'; +import { firstValueFrom } from 'rxjs';apps/connection/src/connection.service.ts (4)
189-197: Null-check before destructuring to prevent TypeErrorThese blocks destructure
agentDetailsbefore verifying it exists.Example fix pattern (apply to each occurrence):
- const { agentEndPoint } = agentDetails; - if (!agentDetails) { + if (!agentDetails) { throw new NotFoundException(ResponseMessages.issuance.error.agentEndPointNotFound); } + const { agentEndPoint } = agentDetails;Also applies to: 255-261, 280-286, 339-345, 399-405, 444-449, 712-718
632-649: Use firstValueFrom instead of toPromise for RxJSModernize
natsCalland avoid chained.catch()on the Promise.- return from(this.natsClient.send<string>(this.connectionServiceProxy, pattern, payload)) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { - this.logger.error(`catch: ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.statusCode, - error: error.message - }, - error.error - ); - }); + try { + const response = await firstValueFrom( + from(this.natsClient.send<string>(this.connectionServiceProxy, pattern, payload)) + ); + return { response }; + } catch (error) { + this.logger.error(`catch: ${JSON.stringify(error)}`); + throw new HttpException( + { status: error.statusCode, error: error.message }, + error?.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR + ); + }And update imports:
-import { from, map } from 'rxjs'; +import { from, firstValueFrom } from 'rxjs';
287-289: Return normalized response shape
_getQuestionAnswersRecordreturns{ response }. Expose theresponsefor callers.- const record = await this._getQuestionAnswersRecord(url, orgId); - return record; + const record = await this._getQuestionAnswersRecord(url, orgId); + return record.response;
118-126: Fix getUrl: return stored invitation URL (not referenceId)getShorteningUrl returns a shortening_url record (schema stores invitationPayload as JSON; there is no url column). getUrl currently returns urlDetails.referenceId — that returns the key, not the invitation URL. Either return the actual invitation URL (e.g. urlDetails.invitationPayload or the URL field inside it) and adjust the return type/docstring, or change the documentation/signature to explicitly return the referenceId.
Locations:
- apps/connection/src/connection.service.ts — getUrl (returns referenceId)
- apps/connection/src/connection.repository.ts — getShorteningUrl (returns shortening_url)
- libs/prisma-service/prisma/schema.prisma — model shortening_url (invitationPayload present; url column removed)
🧹 Nitpick comments (9)
libs/common/src/common.service.ts (1)
27-36: Migrate deprecated.toPromise()→firstValueFrom/lastValueFromRepo contains many
.toPromise()calls and package.json shows "rxjs": "^7.8.1" (toPromise is deprecated in RxJS v7 and removed in v8). Replace patterns like:
- await observable.toPromise() → await firstValueFrom(observable) (preferred for single-emission observables such as HttpService and ClientProxy.send)
- use lastValueFrom(observable) only if you need the last emission after completion
Representative affected files: libs/common/src/common.service.ts, libs/common/src/NATSClient.ts, apps/api-gateway/, apps/organization/, apps/ledger/, apps/agent-service/, apps/issuance/, apps/connection/, apps/user/, apps/verification/
apps/issuance/src/issuance.service.ts (3)
504-511: HttpException status should be numeric
error.errormay be a message, not a status code. Use a safe fallback.- throw new HttpException( - { - status: error.statusCode, - error: error.message - }, - error.error - ); + throw new HttpException( + { status: error.statusCode, error: error.message }, + error?.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR + );If upstream guarantees a numeric
error.error, confirm and ignore this.
1491-1535: p-limit usage here is a no-op
limit(() => job)just returns objects; no async work is being limited. Either removep-limithere or apply it where actual async occurs (e.g., chunking addBulk or upstream writes).
1810-1848: Socket created but never closed
io(...)returns synchronously;awaitis unnecessary and the socket isn’t disconnected, which can leak connections.- const socket = await io(`${process.env.SOCKET_HOST}`, { + const socket = io(`${process.env.SOCKET_HOST}`, { reconnection: true, reconnectionDelay: 5000, reconnectionAttempts: Infinity, autoConnect: true, transports: ['websocket'] }); … + socket.disconnect();Repeat in the catch block below as well.
apps/connection/src/connection.service.ts (3)
656-664: handleError signature/typeThis never resolves; it throws. Remove
asyncand returnnever(orvoid) for clarity.- handleError(error): Promise<void> { + handleError(error): never { if (error?.status?.message?.error) { throw new RpcException({ message: error.status.message.error.reason || error.status.message.error, statusCode: error.status?.code ?? HttpStatus.INTERNAL_SERVER_ERROR }); } throw new RpcException(error.response || error); }
709-716: Typo: sendBasicMesageMinor naming nit. Consider renaming to
sendBasicMessage(API ripple).
463-470: Consistent error wrapping fallbackGood normalization with
error?.status?.message?.error. Ensure fallback status uses a numeric code.- statusCode: error.status?.code ?? HttpStatus.INTERNAL_SERVER_ERROR + statusCode: Number(error?.status?.code) || HttpStatus.INTERNAL_SERVER_ERRORAlso applies to: 729-736
apps/ledger/src/credential-definition/credential-definition.service.ts (2)
119-129: Harden response shape accessIf agent shape changes, these nested property reads can throw. Use optional chaining with safe defaults.
- } else if ('finished' === response.credentialDefinition.state) { - credDefData.tag = response.credentialDefinition.credentialDefinition.tag; + } else if ('finished' === response?.credentialDefinition?.state) { + credDefData.tag = response?.credentialDefinition?.credentialDefinition?.tag; …
421-428: HttpException status fallbackUse numeric status fallback to avoid throwing with undefined/non-numeric codes.
- throw new HttpException( - { - status: error.status, - error: error.message - }, - error.status - ); + throw new HttpException( + { status: error?.status, error: error?.message }, + error?.status ?? HttpStatus.INTERNAL_SERVER_ERROR + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/connection/src/connection.service.ts(5 hunks)apps/issuance/src/issuance.service.ts(4 hunks)apps/ledger/src/credential-definition/credential-definition.service.ts(4 hunks)apps/ledger/src/schema/schema.service.ts(3 hunks)libs/common/src/common.service.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/ledger/src/schema/schema.service.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/ledger/src/credential-definition/credential-definition.service.ts (4)
apps/ledger/src/credential-definition/interfaces/create-credential-definition.interface.ts (6)
CreateCredDefPayload(19-23)CredDefPayload(35-49)IPlatformCredDefs(25-33)GetCredDefPayload(6-17)GetAllCredDefsPayload(59-63)GetCredDefBySchemaId(65-67)apps/ledger/src/credential-definition/interfaces/credential-definition.interface.ts (3)
CreateCredDefAgentRedirection(1-12)GetCredDefAgentRedirection(20-29)CredDefSchema(35-38)libs/common/src/interfaces/cred-def.interface.ts (2)
IPlatformCredDefsData(30-32)ICredDefDetails(1-3)libs/common/src/interfaces/schema.interface.ts (1)
ISchemaDetail(48-67)
apps/issuance/src/issuance.service.ts (1)
libs/logger/src/logger.service.ts (1)
error(51-54)
🔇 Additional comments (3)
libs/common/src/common.service.ts (1)
241-247: LGTM! Error handling logic correctly streamlined.The simplified error handling logic is cleaner and maintains the same functionality. The conditional check has been appropriately streamlined to only verify the essential path (
error?.status?.message?.error), and the fallback behavior remains consistent.apps/ledger/src/credential-definition/credential-definition.service.ts (2)
136-145: LGTM: simplified error handlingThis aligns with prior review guidance and standardizes RpcException construction.
156-174: Replace toPromise with firstValueFrom and unify error wrappingModernize RxJS usage and avoid Promise
.catch()chains for consistency.-import { from } from 'rxjs'; +import { firstValueFrom } from 'rxjs'; … - const credDefResponse = await from(this.natsClient.send<string>(this.credDefServiceProxy, pattern, payload)) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { - this.logger.error(`Catch : ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.statusCode, - error: error.message - }, - error.error - ); - }); + try { + const response = await firstValueFrom(this.natsClient.send<string>(this.credDefServiceProxy, pattern, payload)); + return { response }; + } catch (error) { + this.logger.error(`Catch : ${JSON.stringify(error)}`); + throw new HttpException( + { status: error?.statusCode ?? error?.status, error: error?.message }, + error?.statusCode ?? error?.status ?? HttpStatus.INTERNAL_SERVER_ERROR + ); + }And for
_getSchemaDetails:- const getSchemaDetails = await this.credDefServiceProxy - .send(pattern, payload) - .toPromise() - .catch((error) => { - this.logger.error(`catch: ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.status, - error: error.message - }, - error.status - ); - }); - return getSchemaDetails; + try { + return await firstValueFrom(this.credDefServiceProxy.send(pattern, payload)); + } catch (error) { + this.logger.error(`catch: ${JSON.stringify(error)}`); + throw new HttpException( + { status: error?.status, error: error?.message }, + error?.status ?? HttpStatus.INTERNAL_SERVER_ERROR + ); + }Also applies to: 260-277, 327-341
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
libs/common/src/interfaces/schema.interface.ts (2)
1-1: Use a type-only import to avoid runtime dependency and satisfy strict TS configs.Since
ICredDefDatais only used for typing, preferimport type(helps withverbatimModuleSyntax/importsNotUsedAsValues).-import { ICredDefData } from './cred-def.interface'; +import type { ICredDefData } from './cred-def.interface';
76-76: Use a type alias instead of an empty interface extendingPick<>.This is purely an alias; a
typeis clearer here. Consider keeping the current name to minimize churn.-export interface PartialICredDefData extends Pick<ICredDefData, 'createdBy' | 'schemaId' | 'orgId'> {} +export type PartialICredDefData = Pick<ICredDefData, 'createdBy' | 'schemaId' | 'orgId'>;Optional: The name “PartialICredDefData” can be confusing (it suggests optional fields). Consider
ICredDefSummaryin a follow-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/common/src/interfaces/schema.interface.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
libs/common/src/interfaces/schema.interface.ts (1)
libs/common/src/interfaces/cred-def.interface.ts (1)
ICredDefData(5-14)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/common/src/interfaces/schema.interface.ts (2)
1-1: Reduce cross-module coupling: avoid importing cred-def types into schema interfacesBringing
ICredDefDataintoschema.interface.tsincreases coupling between schema and cred-def modules. Consider relocating the cred-def–related result shapes (ICredDefWithPagination,ICredDefWithCount, and the picked type) intocred-def.interface.tsand importing them where needed. This keeps boundaries cleaner and reduces the risk of circular deps over time.Please confirm there are no circular imports between
schema.interface.tsandcred-def.interface.tsafter this change.
76-76: Rename for clarity and prefer a type alias overinterface extends Pick<...>“PartialICredDefData” suggests optional fields (like
Partial<T>), but this is a strict subset. A clearer name (e.g.,ICredDefSummary) and a type alias are more idiomatic with utility types.Apply this diff:
-export interface PartialICredDefData extends Pick<ICredDefData, 'createdBy' | 'schemaId' | 'orgId'> {} +export type ICredDefSummary = Pick<ICredDefData, 'createdBy' | 'schemaId' | 'orgId'>;Follow-ups are provided below to update usages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/common/src/interfaces/schema.interface.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
libs/common/src/interfaces/schema.interface.ts (1)
libs/common/src/interfaces/cred-def.interface.ts (1)
ICredDefData(5-14)
🔇 Additional comments (1)
libs/common/src/interfaces/schema.interface.ts (1)
79-85: Verify downstream usage after narrowing the cred-def list item shapeChanging list items from
ICredDefData[]to a 3-field subset is a breaking type change. Any consumer reading fields likecredentialDefinitionId,tag,schemaLedgerId,revocable, orcreateDateTimefromdata[]orcredDefResult[]will now break.
- If UIs/services link to a specific cred-def, consider including
credentialDefinitionIdin the picked set to avoid extra lookups.- If you accept the rename in the earlier comment, also update the references here:
export interface ICredDefWithPagination extends IPaginationDetails { - data: PartialICredDefData[]; + data: ICredDefSummary[]; } export interface ICredDefWithCount { credDefCount: number; - credDefResult: PartialICredDefData[]; + credDefResult: ICredDefSummary[]; }Run this script to surface potential breakages where removed fields may still be accessed on these collections:
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/api-gateway/src/platform/platform.service.ts (3)
12-17: Restore explicit ClientProxy typing for DI to retain type safetyKeeping the injected proxy typed avoids
anyleakage and preserves IDE help; Nest v10 still exports ClientProxy.Apply within this block:
- constructor( - @Inject('NATS_CLIENT') private readonly platformServiceProxy, - private readonly natsClient: NATSClient - ) { + constructor( + @Inject('NATS_CLIENT') private readonly platformServiceProxy: ClientProxy, + private readonly natsClient: NATSClient + ) { super('PlatformService'); }Add import at the top of the file:
import { ClientProxy } from '@nestjs/microservices';
19-22: Inline the payload to reduce noiseMinor cleanup; no behavior change.
- async getAllSchema(schemaSearchCriteria: ISchemaSearchPayload, user: IUserRequestInterface): Promise<ISchemaDetails> { - const schemaSearch = { schemaSearchCriteria, user }; - return this.natsClient.sendNatsMessage(this.platformServiceProxy, 'get-all-schemas', schemaSearch); - } + async getAllSchema(schemaSearchCriteria: ISchemaSearchPayload, user: IUserRequestInterface): Promise<ISchemaDetails> { + return this.natsClient.sendNatsMessage(this.platformServiceProxy, 'get-all-schemas', { schemaSearchCriteria, user }); + }
24-30: Consistent style: inline the cred-defs payloadMatches the pattern suggested above; optional readability tweak.
async getAllPlatformCredDefs( getAllPlatformCredDefs: GetAllPlatformCredDefsDto, user: IUserRequestInterface ): Promise<IPlatformCredDefsData> { - const credDefs = { ...getAllPlatformCredDefs, user }; - return this.natsClient.sendNatsMessage(this.platformServiceProxy, 'get-all-platform-cred-defs', credDefs); + return this.natsClient.sendNatsMessage(this.platformServiceProxy, 'get-all-platform-cred-defs', { ...getAllPlatformCredDefs, user }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api-gateway/src/platform/platform.service.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api-gateway/src/platform/platform.service.ts (5)
apps/api-gateway/src/interfaces/ISchemaSearch.interface.ts (2)
ISchemaSearchPayload(4-13)ILedgers(27-39)apps/agent-service/src/interface/agent-service.interface.ts (2)
IUserRequestInterface(237-250)ILedgers(647-659)libs/common/src/interfaces/schema.interface.ts (1)
INetworkUrl(87-89)apps/api-gateway/src/credential-definition/dto/get-all-platform-cred-defs.dto.ts (1)
GetAllPlatformCredDefsDto(8-30)libs/common/src/interfaces/cred-def.interface.ts (1)
IPlatformCredDefsData(30-32)
🔇 Additional comments (3)
apps/api-gateway/src/platform/platform.service.ts (3)
32-35: Inline empty payload; remove temp var (already flagged earlier)This was suggested previously; good to tidy up now.
async getAllLedgers(): Promise<ILedgers[]> { - const payload = {}; - return this.natsClient.sendNatsMessage(this.platformServiceProxy, 'get-all-ledgers', payload); + return this.natsClient.sendNatsMessage(this.platformServiceProxy, 'get-all-ledgers', {}); }
37-39: LGTM: network URL retrieval is correctly typedReturn type matches INetworkUrl shape; call pattern looks correct.
41-44: Clarify/align return type and nullability for get-shortening-urlEarlier reviews noted the handler may return an object or null. The current signature
Promise<string>may be inaccurate.Please confirm the handler’s return shape and adjust the signature (e.g.,
Promise<string | null>or a DTO likePromise<ShorteningUrlDto | null>). To verify quickly:#!/bin/bash # Locate the NATS handler(s) and inspect return types/shapes. rg -n -C3 -P "get-shortening-url" rg -n -C3 -P "getShorteningUrlById\\("
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/api-gateway/src/fido/fido.controller.ts (3)
254-261: Decode credentialId to match other endpoints and handle URL-encoded IDs.Consistent with updateFidoUser.
- const updateDeviceName = await this.fidoService.updateFidoUserDeviceName(credentialId, deviceName); + const updateDeviceName = await this.fidoService.updateFidoUserDeviceName( + decodeURIComponent(credentialId), + deviceName + );
281-286: Decode credentialId before service call (delete).Handles URL-encoded IDs consistently.
- const deleteFidoUser = await this.fidoService.deleteFidoUserDevice(credentialId); + const deleteFidoUser = await this.fidoService.deleteFidoUserDevice(decodeURIComponent(credentialId));
224-233: Variable naming nit: avoid misleading “verifyRegistration” in updateFidoUser.Minor clarity improvement.
- const verifyRegistration = await this.fidoService.updateFidoUser( + const updateUserDetails = await this.fidoService.updateFidoUser( updateFidoUserDetailsDto, decodeURIComponent(credentialId) ); const finalResponse: IResponseType = { statusCode: HttpStatus.OK, message: ResponseMessages.fido.success.updateUserDetails, - data: verifyRegistration.response + data: updateUserDetails.response };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api-gateway/src/fido/fido.controller.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api-gateway/src/fido/fido.controller.ts (4)
libs/common/src/interfaces/response.interface.ts (1)
IResponseType(1-7)libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-518)apps/api-gateway/src/dtos/fido-user.dto.ts (5)
GenerateRegistrationDto(4-10)VerifyRegistrationDto(31-58)GenerateAuthenticationDto(74-78)VerifyAuthenticationDto(99-126)UpdateFidoUserDetailsDto(60-72)apps/api-gateway/src/fido/fido.service.ts (2)
verifyRegistration(29-32)verifyAuthentication(40-46)
🔇 Additional comments (7)
apps/api-gateway/src/fido/fido.controller.ts (7)
27-45: Import missing guards (AuthGuard, OrgRolesGuard).Required to actually enforce @roles.
import { ApiResponseDto } from '../dtos/apiResponse.dto'; import { BadRequestErrorDto } from '../dtos/bad-request-error.dto'; +import { AuthGuard } from '@nestjs/passport'; +import { OrgRolesGuard } from '../authz/guards/org-roles.guard';
1-15: Enforce @roles with guards — add UseGuards to imports.Without UseGuards, @roles is not enforced. Import UseGuards.
import { Body, Controller, Delete, Get, HttpStatus, Logger, Param, Post, Put, Query, Request, Res, - UseFilters + UseFilters, + UseGuards } from '@nestjs/common';
62-73: Protect fetch endpoint with guards.@roles without guards leaves the route public.
@Get('/passkey/:email') @Roles(OrgRoles.OWNER, OrgRoles.ADMIN, OrgRoles.HOLDER, OrgRoles.ISSUER, OrgRoles.SUPER_ADMIN, OrgRoles.MEMBER) +@UseGuards(AuthGuard('jwt'), OrgRolesGuard) @ApiBearerAuth()
243-248: Protect update device-name endpoint with guards.@roles is currently ineffective.
@Put('/passkey/:credentialId') @Roles(OrgRoles.OWNER, OrgRoles.ADMIN, OrgRoles.HOLDER, OrgRoles.ISSUER, OrgRoles.SUPER_ADMIN, OrgRoles.MEMBER) +@UseGuards(AuthGuard('jwt'), OrgRolesGuard) @ApiBearerAuth()
274-279: Protect delete device endpoint with guards.@roles is currently ineffective.
@Delete('/passkey/:credentialId') @Roles(OrgRoles.OWNER, OrgRoles.ADMIN, OrgRoles.HOLDER, OrgRoles.ISSUER, OrgRoles.SUPER_ADMIN, OrgRoles.MEMBER) +@UseGuards(AuthGuard('jwt'), OrgRolesGuard) @ApiBearerAuth()
139-142: Use the method param instead of req.params for email.Safer and consistent with other handlers.
const verifyRegistration = await this.fidoService.verifyRegistration( verifyRegistrationDto, - req.params.email.toLowerCase() + email.toLowerCase() );
195-198: Use the method param instead of req.params for email (verify auth).Consistent param usage.
const verifyAuthentication = await this.fidoService.verifyAuthentication( verifyAuthenticationDto, - req.params.email.toLowerCase() + email.toLowerCase() );
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
…url) instead of a string(url) Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
libs/common/src/NATSClient.ts (1)
33-34: Nit: Fix the typo in the inline comment.Please tweak the comment to say “response” instead of “reposne” for readability.
apps/api-gateway/src/agent/agent.service.ts (2)
6-6: Restore explicit ClientProxy typing for agentServiceProxy.Dropping the
ClientProxyannotation turns the injected proxy intoany, which removes the compile-time safety we rely on when calling microservice APIs. NestJS v11 still exportsClientProxy, so we can keep the strong typing without issue.-import { Injectable, Inject } from '@nestjs/common'; +import { Injectable, Inject } from '@nestjs/common'; +import { ClientProxy } from '@nestjs/microservices'; @@ - @Inject('NATS_CLIENT') private readonly agentServiceProxy, + @Inject('NATS_CLIENT') private readonly agentServiceProxy: ClientProxy,Also applies to: 14-19
48-57: Update registerNym docblock to match its parameters.The comment still references role/alias/verkey/did even though the method only accepts
idanduser. Please refresh the documentation so it reflects the actual signature./** * Description: Calling agent service for onboard-register-ledger - * @param role - * @param alias - * @param verkey - * @param did + * @param id Organization identifier forwarded to the agent + * @param user Authenticated user context attached to the request */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api-gateway/src/agent/agent.controller.ts(9 hunks)apps/api-gateway/src/agent/agent.service.ts(1 hunks)apps/api-gateway/src/fido/fido.controller.ts(2 hunks)apps/api-gateway/src/platform/platform.service.ts(1 hunks)apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts(1 hunks)apps/api-gateway/src/webhook/webhook.controller.ts(3 hunks)libs/common/src/NATSClient.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/api-gateway/src/agent/agent.controller.ts (2)
apps/api-gateway/src/dtos/wallet-details.dto.ts (1)
WalletDetailsDto(3-17)apps/agent-service/src/interface/agent-service.interface.ts (1)
IUserRequestInterface(237-250)
apps/api-gateway/src/webhook/webhook.controller.ts (2)
libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-518)apps/api-gateway/src/authz/decorators/roles.decorator.ts (1)
Roles(6-6)
apps/api-gateway/src/platform/platform.service.ts (5)
apps/api-gateway/src/interfaces/ISchemaSearch.interface.ts (2)
ISchemaSearchPayload(4-13)ILedgers(27-39)apps/agent-service/src/interface/agent-service.interface.ts (2)
IUserRequestInterface(237-250)ILedgers(647-659)libs/common/src/interfaces/schema.interface.ts (2)
ISchemaDetails(20-22)INetworkUrl(87-89)apps/api-gateway/src/credential-definition/dto/get-all-platform-cred-defs.dto.ts (1)
GetAllPlatformCredDefsDto(8-30)libs/common/src/interfaces/cred-def.interface.ts (1)
IPlatformCredDefsData(30-32)
apps/api-gateway/src/fido/fido.controller.ts (3)
libs/common/src/interfaces/response.interface.ts (1)
IResponseType(1-7)apps/api-gateway/src/dtos/fido-user.dto.ts (4)
GenerateRegistrationDto(4-10)VerifyRegistrationDto(31-58)GenerateAuthenticationDto(74-78)VerifyAuthenticationDto(99-126)apps/api-gateway/src/fido/fido.service.ts (2)
verifyRegistration(29-32)verifyAuthentication(40-46)
apps/api-gateway/src/agent/agent.service.ts (4)
libs/common/src/NATSClient.ts (1)
Injectable(11-62)apps/api-gateway/src/agent-service/agent-service.service.ts (1)
Injectable(13-103)libs/service/base.service.ts (1)
BaseService(2-9)apps/api-gateway/src/dtos/wallet-details.dto.ts (1)
WalletDetailsDto(3-17)
apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (1)
libs/common/src/cast.helper.ts (2)
trim(39-43)toNumber(58-76)
🔇 Additional comments (12)
apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (7)
64-71: Type the sortField property as CredDefSortFieldsRuntime validation now uses
CredDefSortFields, but the property remains typed asstring, leaving room for invalid assignments and misaligned API metadata. Updating the type tightens both TypeScript and Swagger contracts.- sortField: string = CredDefSortFields.CREATED_DATE_TIME; + sortField: CredDefSortFields = CredDefSortFields.CREATED_DATE_TIME;
16-20: Restore default pageNumber when query param is absent
toNumberreturnsundefinedwhenvalueis missing, which overwrites thepageNumber = 1initializer. That nullifies the default pagination and breaks downstream expectations. Please supply the default/min options to keep the fallback intact.- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 1, min: 1 }))
22-26: Preserve pageSize default when the client omits itSame issue here:
toNumberwithout defaults can returnundefined, wiping outpageSize = 10. Pass the default and minimum so pagination never devolves toundefined.- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 10, min: 1 }))
47-51: Keep GetCredentialDefinitionBySchemaIdDto.pageNumber defaulting to 1For this DTO, missing
pageNumberalso becomesundefinedpost-transform. Mirror the fix suggested above so the initializer survives and validators work on a concrete value.- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 1, min: 1 }))
53-57: Stop GetCredentialDefinitionBySchemaIdDto.pageSize from becoming undefinedApply the same default/min guard on
pageSize; otherwise, omitting the query param wipes out the intended default of 10.- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 10, min: 1 }))
95-99: Maintain GetAllSchemaByPlatformDto.pageNumber defaultHere too, omitting the query param yields
undefined. Please propagate the default/min options so pagination stays deterministic.- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 1, min: 1 }))
106-110: Retain GetAllSchemaByPlatformDto.pageSize defaultWithout the default/min guard,
pageSizeloses its fallback of 10. Align this transform with the rest to avoidundefinedleaking downstream.- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 10, min: 1 }))apps/api-gateway/src/webhook/webhook.controller.ts (1)
106-113: Great to see OrgRolesGuard applied hereThanks for closing the roles-enforcement gap on this endpoint; the guard pairing now matches the roles declared.
apps/api-gateway/src/agent/agent.controller.ts (1)
46-49: No action needed: CommonService is already wired into AgentModule
AgentModule imports CommonModule and lists CommonService in its providers.libs/common/src/NATSClient.ts (1)
25-28: Context headers now consistent—nice catch.Thanks for standardizing the contextId header creation across all three send helpers; this keeps downstream correlation logic intact.
Also applies to: 42-45, 54-55
apps/api-gateway/src/fido/fido.controller.ts (2)
63-85: Secure @Roles-protected routes with guards.Every endpoint annotated with
@Roles(...)is still missing@UseGuards(AuthGuard('jwt'), OrgRolesGuard), so the role checks never execute and these passkey operations remain publicly callable. Please wire the guards back in (and add the required imports) before shipping.@@ @Get('/passkey/:email') @Roles(OrgRoles.OWNER, OrgRoles.ADMIN, OrgRoles.HOLDER, OrgRoles.ISSUER, OrgRoles.SUPER_ADMIN, OrgRoles.MEMBER) +@UseGuards(AuthGuard('jwt'), OrgRolesGuard) @@ @Put('/passkey/:credentialId') @Roles(OrgRoles.OWNER, OrgRoles.ADMIN, OrgRoles.HOLDER, OrgRoles.ISSUER, OrgRoles.SUPER_ADMIN, OrgRoles.MEMBER) +@UseGuards(AuthGuard('jwt'), OrgRolesGuard) @@ @Delete('/passkey/:credentialId') @Roles(OrgRoles.OWNER, OrgRoles.ADMIN, OrgRoles.HOLDER, OrgRoles.ISSUER, OrgRoles.SUPER_ADMIN, OrgRoles.MEMBER) +@UseGuards(AuthGuard('jwt'), OrgRolesGuard)Remember to extend the top-level imports accordingly:
@@ - Res, - UseFilters + Res, + UseFilters, + UseGuards+import { AuthGuard } from '@nestjs/passport'; +import { OrgRolesGuard } from '../authz/guards/org-roles.guard';Also applies to: 243-262, 274-288
164-176: Align HTTP status with documented contract.The handler body advertises
statusCode: HttpStatus.OKwhile the decorator and actual response emit 201 Created. This mismatch is confusing for clients. Please return 200 across the board (or adjust both decorator and payload to 201) so the status codes line up.- @ApiResponse({ status: HttpStatus.CREATED, description: 'Success', type: ApiResponseDto }) + @ApiResponse({ status: HttpStatus.OK, description: 'Success', type: ApiResponseDto }) @@ - return res.status(HttpStatus.CREATED).json(finalResponse); + return res.status(HttpStatus.OK).json(finalResponse);
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/connection/src/connection.service.ts (1)
118-125: Return the stored invitation payload instead of echoing the reference ID
getShorteningUrl()already receives the reference ID; sending the same value back makes the endpoint useless. Callers rely on this method to retrieve the stored (shortened) invitation/URL that was persisted alongside the reference. Echoing the reference ID breaks that flow and regresses existing clients.Please return the persisted invitation field (e.g.,
connectionInvitation/connectionInvitationUrl) from the repository result.- const urlDetails = await this.connectionRepository.getShorteningUrl(referenceId); - return urlDetails.referenceId; + const urlDetails = await this.connectionRepository.getShorteningUrl(referenceId); + return urlDetails.connectionInvitation;apps/api-gateway/src/connection/connection.service.ts (1)
56-64: Align with the revertedsendNatsreturn type.Because
sendNatsnow continues to return the raw value, this helper should consume that raw payload. Changing the return type here will keep the public API consistent with existing clients.- getUrl(referenceId: string): Promise<{ - response: object; - }> { + getUrl(referenceId: string): Promise<object> {
🧹 Nitpick comments (13)
apps/api-gateway/src/notification/notification.service.ts (1)
22-29: Fix the lingeringwebhhooktypo in the parameter name.The method name is now
registerOrgWebhookEndpoint, but the parameter still carries the oldwebhhookspelling. Renaming the variable keeps the codebase consistent and avoids propagating the typo.- async registerOrgWebhookEndpoint( - registerOrgWebhhookEndpointDto: RegisterOrgWebhhookEndpointDto - ): Promise<INotification> { + async registerOrgWebhookEndpoint( + registerOrgWebhookEndpointDto: RegisterOrgWebhhookEndpointDto + ): Promise<INotification> { return this.natsClient.sendNatsMessage( this.serviceProxy, 'register-org-webhook-endpoint-for-notification', - registerOrgWebhhookEndpointDto + registerOrgWebhookEndpointDto ); }apps/api-gateway/src/verification/verification.service.ts (2)
16-16: ImportfirstValueFromfor the new promise conversion.Since we should move off the deprecated
toPromise()path, please pull infirstValueFromfromrxjsso the updated calls compile.Apply this diff:
-import { ClientProxy } from '@nestjs/microservices'; +import { ClientProxy } from '@nestjs/microservices'; +import { firstValueFrom } from 'rxjs';
119-136: Replace deprecated.toPromise()withfirstValueFrom.Lines 121 and 135 still rely on the deprecated
.toPromise()helper. NestJS 11 ships with RxJS 7.8+, wheretoPromiseis slated for removal; sticking with it makes future upgrades brittle and can trigger linting/TS errors. Switching tofirstValueFromkeeps the same semantics and aligns with RxJS guidance.Apply this diff:
- const message = await this.verificationServiceProxy.send(pattern, payload).toPromise(); + const message = await firstValueFrom(this.verificationServiceProxy.send(pattern, payload));Make the same change for the call on Line 135.
apps/api-gateway/src/agent/agent.service.ts (1)
48-58: Sync the JSDoc with the current signature.The comment still references role/alias/verkey/did parameters, but the method now only accepts
{ id, user }. Please update the doc so future readers don’t chase nonexistent args.apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (8)
28-35: Expose enum in Swagger and type the fieldMake the OpenAPI enum explicit and use the enum type instead of string.
Apply:
- @ApiProperty({ - required: false - }) + @ApiProperty({ + required: false, + enum: SortFields + }) @Transform(({ value }) => trim(value)) @IsOptional() @IsEnum(SortFields) - sortField: string = SortFields.CREATED_DATE_TIME; + sortField: SortFields = SortFields.CREATED_DATE_TIME;
36-43: Type sortBy as SortValueTighten typing; Swagger already lists the enum values.
Apply:
@IsEnum(SortValue) - sortBy: string = SortValue.DESC; + sortBy: SortValue = SortValue.DESC;
64-71: Type sortField as CredDefSortFieldsValidation is enum-based; align the property type for safety and clearer OpenAPI.
Apply:
@IsEnum(CredDefSortFields) - sortField: string = CredDefSortFields.CREATED_DATE_TIME; + sortField: CredDefSortFields = CredDefSortFields.CREATED_DATE_TIME;
73-80: Type sortBy as SortValueSame reasoning as above.
Apply:
@IsEnum(SortValue) - sortBy: string = SortValue.DESC; + sortBy: SortValue = SortValue.DESC;
82-84: Add validation/Swagger for schemaId and orgIdThese are bare fields; add ApiProperty and validators (UUID assumed—adjust if not).
Apply:
- schemaId: string; + @ApiProperty({ description: 'Schema ID', example: '1a7eac11-ff05-40d7-8351-4d7467687cad' }) + @IsUUID('4', { message: 'Invalid format of schemaId' }) + schemaId: string; - orgId: string; + @ApiProperty({ description: 'Organization ID', example: '3c69d3a2-9b1d-4d5e-b9b0-0c3f8b9b6cde' }) + @IsUUID('4', { message: 'Invalid format of orgId' }) + orgId: string;
88-93: Remove redundant Swagger decoratorsUse only ApiPropertyOptional and keep the example.
Apply:
- @ApiProperty({ example: '1a7eac11-ff05-40d7-8351-4d7467687cad' }) - @ApiPropertyOptional() + @ApiPropertyOptional({ example: '1a7eac11-ff05-40d7-8351-4d7467687cad' })
112-119: Expose enum in Swagger and type the field (sorting)Align typing with validation and show enum in OpenAPI.
Apply:
- @ApiProperty({ - required: false - }) + @ApiProperty({ + required: false, + enum: SortFields + }) @Transform(({ value }) => trim(value)) @IsOptional() @IsEnum(SortFields) - sorting: string = SortFields.CREATED_DATE_TIME; + sorting: SortFields = SortFields.CREATED_DATE_TIME;
120-123: Validate and document sortByValueCurrently unvalidated; enforce enum and document values (or merge with sortBy for consistency).
Apply:
- @ApiProperty({ required: false }) - @IsOptional() - sortByValue: string = SortValue.DESC; + @ApiProperty({ required: false, enum: [SortValue.DESC, SortValue.ASC] }) + @IsOptional() + @IsEnum(SortValue) + sortByValue: SortValue = SortValue.DESC;apps/issuance/src/issuance.service.ts (1)
527-543: Replace deprecated.toPromise()withfirstValueFrom.Everywhere else in this upgrade we standardised on
firstValueFrom. Leaving.toPromise()here keeps the deprecated API in play and will bite us the moment RxJS 8 lands. Please switch this call tofirstValueFromand add the corresponding import at the top.- return this.issuanceServiceProxy - .send(pattern, payload) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { + return firstValueFrom( + this.issuanceServiceProxy.send(pattern, payload).pipe( + map((response) => ({ + response + })) + ) + ).catch((error) => { this.logger.error(`catch: ${JSON.stringify(error)}`); throw new HttpException( { status: error.statusCode, error: error.message }, error.error ); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
apps/api-gateway/src/agent-service/agent-service.service.ts(1 hunks)apps/api-gateway/src/agent/agent.service.ts(1 hunks)apps/api-gateway/src/app.service.ts(1 hunks)apps/api-gateway/src/authz/authz.service.ts(1 hunks)apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts(1 hunks)apps/api-gateway/src/connection/connection.controller.ts(8 hunks)apps/api-gateway/src/connection/connection.service.ts(3 hunks)apps/api-gateway/src/credential-definition/credential-definition.service.ts(2 hunks)apps/api-gateway/src/fido/fido.service.ts(1 hunks)apps/api-gateway/src/geo-location/geo-location.service.ts(1 hunks)apps/api-gateway/src/issuance/issuance.service.ts(1 hunks)apps/api-gateway/src/notification/notification.service.ts(1 hunks)apps/api-gateway/src/organization/organization.service.ts(1 hunks)apps/api-gateway/src/platform/platform.service.ts(1 hunks)apps/api-gateway/src/revocation/revocation.service.ts(2 hunks)apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts(1 hunks)apps/api-gateway/src/schema/schema.service.ts(2 hunks)apps/api-gateway/src/user/user.service.ts(1 hunks)apps/api-gateway/src/utilities/utilities.controller.ts(3 hunks)apps/api-gateway/src/utilities/utilities.service.ts(2 hunks)apps/api-gateway/src/verification/verification.service.ts(3 hunks)apps/api-gateway/src/webhook/webhook.service.ts(1 hunks)apps/cloud-wallet/src/cloud-wallet.service.ts(3 hunks)apps/connection/src/connection.service.ts(3 hunks)apps/issuance/src/issuance.service.ts(2 hunks)apps/ledger/src/credential-definition/credential-definition.service.ts(4 hunks)apps/ledger/src/schema/schema.service.ts(2 hunks)apps/organization/src/organization.service.ts(2 hunks)libs/common/src/NATSClient.ts(1 hunks)libs/user-activity/repositories/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/api-gateway/src/schema/schema.service.ts
- apps/api-gateway/src/organization/organization.service.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/api-gateway/src/authz/authz.service.ts
- apps/api-gateway/src/revocation/revocation.service.ts
- apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts
- apps/api-gateway/src/issuance/issuance.service.ts
- apps/api-gateway/src/webhook/webhook.service.ts
- apps/api-gateway/src/user/user.service.ts
- apps/cloud-wallet/src/cloud-wallet.service.ts
- apps/api-gateway/src/geo-location/geo-location.service.ts
- apps/api-gateway/src/utilities/utilities.service.ts
- apps/api-gateway/src/app.service.ts
- apps/organization/src/organization.service.ts
- apps/ledger/src/schema/schema.service.ts
🧰 Additional context used
🧬 Code graph analysis (11)
apps/api-gateway/src/credential-definition/credential-definition.service.ts (2)
apps/api-gateway/src/agent-service/agent-service.service.ts (1)
Injectable(14-104)libs/service/base.service.ts (1)
BaseService(2-9)
apps/api-gateway/src/utilities/utilities.controller.ts (1)
apps/api-gateway/src/utilities/dtos/shortening-url.dto.ts (1)
StoreObjectDto(43-47)
apps/ledger/src/credential-definition/credential-definition.service.ts (5)
apps/ledger/src/credential-definition/interfaces/create-credential-definition.interface.ts (6)
CreateCredDefPayload(19-23)CredDefPayload(35-49)IPlatformCredDefs(25-33)GetCredDefPayload(6-17)GetAllCredDefsPayload(59-63)GetCredDefBySchemaId(65-67)apps/ledger/src/credential-definition/interfaces/credential-definition.interface.ts (3)
CreateCredDefAgentRedirection(1-12)GetCredDefAgentRedirection(20-29)CredDefSchema(35-38)libs/common/src/interfaces/cred-def.interface.ts (2)
IPlatformCredDefsData(30-32)ICredDefDetails(1-3)libs/common/src/interfaces/schema.interface.ts (1)
ISchemaDetail(50-69)apps/ledger/src/schema/schema.service.ts (1)
getSchemaDetails(530-537)
apps/api-gateway/src/fido/fido.service.ts (4)
apps/api-gateway/src/agent-service/agent-service.service.ts (1)
Injectable(14-104)apps/api-gateway/src/connection/connection.service.ts (1)
Injectable(22-155)libs/service/base.service.ts (1)
BaseService(2-9)apps/api-gateway/src/dtos/fido-user.dto.ts (4)
VerifyRegistrationDto(31-58)GenerateAuthenticationDto(74-78)VerifyAuthenticationDto(99-126)UpdateFidoUserDetailsDto(60-72)
apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (1)
libs/common/src/cast.helper.ts (2)
trim(39-43)toNumber(58-76)
apps/api-gateway/src/platform/platform.service.ts (5)
apps/api-gateway/src/utilities/utilities.service.ts (1)
Injectable(7-25)libs/service/base.service.ts (1)
BaseService(2-9)apps/api-gateway/src/interfaces/ISchemaSearch.interface.ts (2)
ISchemaSearchPayload(4-13)ILedgers(27-39)libs/common/src/interfaces/schema.interface.ts (2)
ISchemaDetails(20-22)INetworkUrl(87-89)libs/common/src/interfaces/cred-def.interface.ts (1)
IPlatformCredDefsData(30-32)
apps/api-gateway/src/agent/agent.service.ts (2)
libs/service/base.service.ts (1)
BaseService(2-9)apps/api-gateway/src/dtos/wallet-details.dto.ts (1)
WalletDetailsDto(3-17)
apps/api-gateway/src/notification/notification.service.ts (4)
libs/service/base.service.ts (1)
BaseService(2-9)apps/api-gateway/src/notification/dtos/notification.dto.ts (2)
RegisterOrgWebhhookEndpointDto(29-49)SendNotificationDto(51-70)apps/notification/interfaces/notification.interfaces.ts (1)
INotification(3-7)apps/api-gateway/src/notification/interfaces/notification.interfaces.ts (1)
INotification(1-5)
libs/user-activity/repositories/index.ts (5)
apps/issuance/src/issuance.service.ts (1)
Injectable(95-2070)apps/organization/src/organization.service.ts (1)
Injectable(69-2032)apps/verification/src/verification.service.ts (1)
Injectable(58-1071)apps/agent-service/src/agent-service.service.ts (1)
Injectable(83-2092)libs/user-activity/src/user-activity.service.ts (1)
Injectable(7-22)
apps/issuance/src/issuance.service.ts (1)
libs/logger/src/logger.service.ts (1)
error(51-54)
apps/api-gateway/src/connection/connection.controller.ts (2)
libs/common/src/cast.helper.ts (1)
TrimStringParamPipe(167-172)apps/api-gateway/src/connection/connection.service.ts (2)
receiveInvitationUrl(97-104)receiveInvitation(106-113)
🔇 Additional comments (16)
apps/api-gateway/src/agent-service/agent-service.service.ts (1)
12-12: Import alignment looks goodPulling
ClientProxyfrom@nestjs/microservicesmatches the NestJS 11 catalog shift and keeps the dependency surface tidy. 👍apps/api-gateway/src/utilities/utilities.controller.ts (1)
68-74: Boolean param parsing fix LGTMApplying
ParseBoolPipetopersistcloses the string-truthiness gap and keeps the downstream service call clean. Nicely done.apps/api-gateway/src/platform/platform.service.ts (1)
42-45: Return type and null handling for shortening URL still wrongThe NATS handler can return
null, and callers expect a shaped DTO (e.g.,referenceId,invitationPayload). Typing this asPromise<object>both hides the real structure and forbids representingnull, so we can still explode at runtime when the record is missing. Please introduce a proper interface (or reuse an existing DTO) and returnPromise<IShorteningUrl | null>(or throw a typed exception when absent), updating downstream code to handle the null/exception path.apps/api-gateway/src/credential-definition/credential-definition.service.ts (1)
7-34: Return type mismatch forgetAllCredDefs.
sendNatsMessageresolves to the paginated wrapper (ICredDefDetails), not a singleICredDefData. The wrong signature leaks all the way up the controller and misleads callers about the shape they receive. Please align the import and return type with what the ledger service actually sends.-import { ICredDefData } from '@credebl/common/interfaces/cred-def.interface'; +import { ICredDefDetails } from '@credebl/common/interfaces/cred-def.interface'; @@ - ): Promise<ICredDefData> { + ): Promise<ICredDefDetails> {apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (10)
10-14: LGTM on searchByText setupOptional string with trim + Type is fine.
59-63: LGTM on searchByTextConsistent with other DTOs.
101-105: LGTM on searchByTextConsistent and harmless.
124-131: LGTM on schemaTypeEnum validation + optional looks good.
16-21: Fix pagination default: toNumber(undefined) overrides pageNumber=1Provide a default and min to preserve 1 when query param is absent.
Apply:
- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 1, min: 1 }))
22-27: Fix pagination default: toNumber(undefined) overrides pageSize=10Add default and min to keep 10 when omitted.
Apply:
- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 10, min: 1 }))
47-51: Fix pagination default: pageNumberSame undefined override; add default + min.
Apply:
- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 1, min: 1 }))
53-57: Fix pagination default: pageSizeSame undefined override; add default + min.
Apply:
- @Transform(({ value }) => toNumber(value)) + @Transform(({ value }) => toNumber(value, { default: 10, min: 1 }))
95-100: Fix pagination default: pageNumber (platform DTO)Default lost when omitted; set default + min.
Apply:
- @Transform(({ value }) => toNumber(value, { min: 1 })) + @Transform(({ value }) => toNumber(value, { default: 1, min: 1 }))
106-111: Fix pagination default: pageSize (platform DTO)Provide default + min to preserve 10.
Apply:
- @Transform(({ value }) => toNumber(value, { min: 1 })) + @Transform(({ value }) => toNumber(value, { default: 10, min: 1 }))apps/api-gateway/src/connection/connection.controller.ts (2)
378-395: Don't swallow failures when persisting connection webhooksBy catching the promise and only logging at
debug, we still respond with 201 even when saving the webhook data (or forwarding it) fails. That silently drops webhook payloads and breaks downstream processing.Let the exception surface (or rethrow after logging at
error) so the caller gets a failure and the issue can be monitored.- const connectionData = await this.connectionService.getConnectionWebhook(connectionDto, orgId).catch((error) => { - this.logger.debug(`error in saving connection webhook ::: ${JSON.stringify(error)}`); - }); + const connectionData = await this.connectionService.getConnectionWebhook(connectionDto, orgId).catch((error) => { + this.logger.error(`error in saving connection webhook ::: ${JSON.stringify(error)}`, error); + throw error; + }); @@ - const webhookUrl = await this.connectionService - ._getWebhookUrl(connectionDto?.contextCorrelationId, orgId) - .catch((error) => { - this.logger.debug(`error in getting webhook url ::: ${JSON.stringify(error)}`); - }); + const webhookUrl = await this.connectionService + ._getWebhookUrl(connectionDto?.contextCorrelationId, orgId) + .catch((error) => { + this.logger.error(`error in getting webhook url ::: ${JSON.stringify(error)}`, error); + throw error; + }); @@ - await this.connectionService._postWebhookResponse(webhookUrl, { data: connectionDto }).catch((error) => { - this.logger.debug(`error in posting webhook response to webhook url ::: ${JSON.stringify(error)}`); - }); + await this.connectionService._postWebhookResponse(webhookUrl, { data: connectionDto }).catch((error) => { + this.logger.error(`error in posting webhook response to webhook url ::: ${JSON.stringify(error)}`, error); + throw error; + });
425-435: Surface webhook delivery failures instead of hiding themThe question-answer webhook path repeats the same silent
.catchpattern, returning 201 even if delivery fails. That masks outages and drops webhook events.Let the error bubble (or rethrow after logging at
error) so callers can retry and we can monitor failures.- const webhookUrl = await this.connectionService - ._getWebhookUrl(questionAnswerWebhookDto?.contextCorrelationId, orgId) - .catch((error) => { - this.logger.debug(`error in getting webhook url ::: ${JSON.stringify(error)}`); - }); + const webhookUrl = await this.connectionService + ._getWebhookUrl(questionAnswerWebhookDto?.contextCorrelationId, orgId) + .catch((error) => { + this.logger.error(`error in getting webhook url ::: ${JSON.stringify(error)}`, error); + throw error; + }); @@ - await this.connectionService - ._postWebhookResponse(webhookUrl, { data: questionAnswerWebhookDto }) - .catch((error) => { - this.logger.debug(`error in posting webhook response to webhook url ::: ${JSON.stringify(error)}`); - }); + await this.connectionService + ._postWebhookResponse(webhookUrl, { data: questionAnswerWebhookDto }) + .catch((error) => { + this.logger.error(`error in posting webhook response to webhook url ::: ${JSON.stringify(error)}`, error); + throw error; + });
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/api-gateway/src/fido/fido.controller.ts (2)
143-146: Use the routedreq.params.You already receive the email via
@Param('email'), but still dereferencereq.params. Relying on the decorator keeps the handler framework-agnostic and avoids subtle bugs when adapters differ.- const verifyRegistration = await this.fidoService.verifyRegistration( - verifyRegistrationDto, - req.params.email.toLowerCase() - ); + const verifyRegistration = await this.fidoService.verifyRegistration( + verifyRegistrationDto, + email.toLowerCase() + );
199-202: Same as above—lean on thereq.params.Stick to the decorated argument to keep the handler consistent across adapters and avoid Express-only assumptions.
- const verifyAuthentication = await this.fidoService.verifyAuthentication( - verifyAuthenticationDto, - req.params.email.toLowerCase() - ); + const verifyAuthentication = await this.fidoService.verifyAuthentication( + verifyAuthenticationDto, + email.toLowerCase() + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api-gateway/src/fido/fido.controller.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api-gateway/src/fido/fido.controller.ts (1)
apps/api-gateway/src/fido/fido.service.ts (2)
verifyRegistration(29-32)verifyAuthentication(40-46)
🔇 Additional comments (1)
apps/api-gateway/src/fido/fido.controller.ts (1)
167-179: Fix status code mismatch (payload says 200 but response returns 201).The handler builds a payload with
statusCode: HttpStatus.OKyet returnsres.status(HttpStatus.CREATED). Clients receive a 201 response whose body claims 200, and Swagger advertises 201 as well. Align everything to 200 to keep the transport and documented contract consistent.Apply this diff:
- @ApiResponse({ status: HttpStatus.CREATED, description: 'Success', type: ApiResponseDto }) + @ApiResponse({ status: HttpStatus.OK, description: 'Success', type: ApiResponseDto }) @@ - return res.status(HttpStatus.CREATED).json(finalResponse); + return res.status(HttpStatus.OK).json(finalResponse);
Signed-off-by: Krishna Waske <[email protected]>
…ntroller Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api-gateway/src/fido/fido.controller.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api-gateway/src/fido/fido.controller.ts (4)
apps/api-gateway/src/authz/decorators/roles.decorator.ts (1)
Roles(6-6)libs/common/src/interfaces/response.interface.ts (1)
IResponseType(1-7)libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-518)apps/api-gateway/src/fido/fido.service.ts (2)
verifyRegistration(29-32)verifyAuthentication(40-46)
|
* fix: update nest version and add them to catalog in pnpm Signed-off-by: Krishna Waske <[email protected]> * fix: nest version update changes for services Signed-off-by: Krishna Waske <[email protected]> * fix: add remaining packages in catalog Signed-off-by: Krishna Waske <[email protected]> * fix: logging and unnecessary await Signed-off-by: Krishna Waske <[email protected]> * fix: inconsistent return type Signed-off-by: Krishna Waske <[email protected]> * fix: remove toPromise in favour of firstValueFrom Signed-off-by: Krishna Waske <[email protected]> * fix: toPromise to lastValuFrom Signed-off-by: Krishna Waske <[email protected]> * fix: update error message Signed-off-by: Krishna Waske <[email protected]> * fix: update duplicate credDefData interface Signed-off-by: Krishna Waske <[email protected]> * fix: get all ledgers Signed-off-by: Krishna Waske <[email protected]> * fix: throw error Signed-off-by: Krishna Waske <[email protected]> * fix: proper use of params Signed-off-by: Krishna Waske <[email protected]> * fix: path excluded properly Signed-off-by: Krishna Waske <[email protected]> * fix: api-response code Signed-off-by: Krishna Waske <[email protected]> * fix: remove unwanted controllers, with no matching nats call available Signed-off-by: Krishna Waske <[email protected]> * fix: header setting for nats Signed-off-by: Krishna Waske <[email protected]> * fix: add missing roleguard Signed-off-by: Krishna Waske <[email protected]> * fix: return type is actually an object(including referenceId and the url) instead of a string(url) Signed-off-by: Krishna Waske <[email protected]> * fix: update correct enum Signed-off-by: Krishna Waske <[email protected]> * fix: min value for transformation Signed-off-by: Krishna Waske <[email protected]> * fix: parsing string as a boolean to handle boolean values correctly Signed-off-by: Krishna Waske <[email protected]> * fix: add required typing Signed-off-by: Krishna Waske <[email protected]> * fix: error handling Signed-off-by: Krishna Waske <[email protected]> * fix: add dependency for loggerModule Signed-off-by: Krishna Waske <[email protected]> * fix: add typing and todo for the nested response issue Signed-off-by: Krishna Waske <[email protected]> * fix: add roleguards where role level access is expected Signed-off-by: Krishna Waske <[email protected]> * fix: interface type Signed-off-by: Krishna Waske <[email protected]> * fix: adding a todo for unsure changes regarding authguards in fido controller Signed-off-by: Krishna Waske <[email protected]> * fix: use params directly instead of through req object Signed-off-by: Krishna Waske <[email protected]> --------- Signed-off-by: Krishna Waske <[email protected]>
What
Summary by CodeRabbit
New Features
Refactor
Breaking Changes
Documentation
Chores