Skip to content

fix: resolve lint errors for API consistency#205

Open
Cedarich wants to merge 2 commits intoStarShopCr:mainfrom
Cedarich:chore/api-consistency-response-normalization
Open

fix: resolve lint errors for API consistency#205
Cedarich wants to merge 2 commits intoStarShopCr:mainfrom
Cedarich:chore/api-consistency-response-normalization

Conversation

@Cedarich
Copy link

@Cedarich Cedarich commented Oct 3, 2025

📖 PR: Standardize Api Responses & Token Handling

📝 Description

  • Unifies Swagger response decorators across controllers using custom decorators:
    • ApiSuccessResponse
    • ApiErrorResponse
    • ApiAuthResponse
  • Normalizes auth token handling by using setToken(res, token) in UserController so ResponseInterceptor consistently includes token.

📂 Updated Controllers

  • src/modules/attributes/controllers/attributes.controller.ts
  • src/cache/controllers/cache.controller.ts
  • src/modules/notifications/controllers/notification.controller.ts
  • src/modules/escrow/controllers/escrow.controller.ts
  • src/modules/offers/offfer.controller.ts
  • src/modules/users/controllers/user.controller.ts

📊 Impact

  • Responses are standardized to:
    { "success": true, "data": {}, "token": "optional" }
    
    

Summary by CodeRabbit

  • New Features

    • Standardized success/error API responses across modules for clearer, consistent behavior.
    • Authentication responses now include a token field for easier client handling.
  • Bug Fixes

    • Corrected offer module linkage to ensure the Offer endpoints load reliably.
  • Documentation

    • Expanded and unified Swagger docs (tags, operations, auth/cookie metadata) across Auth, Users, Products, Stores, Seller, Offers, Escrow, Attributes, Coupons, Notifications, and Cache.
  • Tests

    • E2E tests updated to use a global /api/v1 prefix and a response wrapper interceptor, aligning tests with runtime behavior.

@coderabbitai
Copy link

coderabbitai bot commented Oct 3, 2025

Walkthrough

Standardized Swagger decorators across controllers, introducing ApiSuccessResponse/ApiErrorResponse (and ApiAuthResponse for auth). Adjusted auth service to return { token } instead of { access_token }. Added optional token to AuthResponseDto. Tests now use global prefix /api/v1 and ResponseInterceptor. Fixed an OfferController import path. Added explicit return types in EscrowController.

Changes

Cohort / File(s) Summary of Changes
Swagger decorator migration
src/cache/controllers/cache.controller.ts, src/modules/attributes/controllers/attributes.controller.ts, src/modules/auth/controllers/auth.controller.ts, src/modules/coupons/controllers/coupon.controller.ts, src/modules/notifications/controllers/notification.controller.ts, src/modules/products/controllers/product.controller.ts, src/modules/seller/controllers/seller.controller.ts, src/modules/stores/controllers/store.controller.ts, src/modules/users/controllers/user.controller.ts, src/modules/offers/offer.controller.ts
Replaced @ApiResponse with @ApiSuccessResponse/@ApiErrorResponse (and @ApiAuthResponse for auth). Updated imports accordingly. No business logic changes.
Escrow return types annotated
src/modules/escrow/controllers/escrow.controller.ts
Consolidated decorators to success/error variants and added explicit Promise return types for several methods; parameters/logic unchanged.
Auth token shape + DTO
src/modules/auth/services/auth.service.ts, src/modules/auth/dto/auth-response.dto.ts
authenticateUser now returns { token } instead of { access_token }. Added optional token?: string to AuthResponseDto.
Test harness updates
test/auth.e2e-spec.ts, src/modules/seller/tests/seller.e2e.spec.ts
Applied global prefix /api/v1 and registered ResponseInterceptor. Updated test routes and token/wallet extraction; adjusted role assignment endpoint.
Import path fix
src/modules/offers/offer.module.ts
Corrected controller import path from misspelled file to offer.controller.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor C as Client
  participant AC as AuthController
  participant AS as AuthService
  participant RI as ResponseInterceptor

  C->>AC: POST /api/v1/auth/login (wallet, signature)
  AC->>AS: authenticateUser(walletAddress)
  AS-->>AC: { token }
  note over AC: Controller returns DTO payload (no cookie set directly)
  AC-->>C: Response body (data)
  activate RI
  RI-->>C: Wraps response + sets token (e.g., header/cookie) per policy
  deactivate RI
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • Villarley

Poem

I thump my paw—docs now gleam and shine,
Success on green, errors neatly align.
A token hops from service to be,
Wrapped by an interceptor—whee!
With routes prefixed, I sprint in glee,
Buggy imports fixed—more time for tea. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title “fix: resolve lint errors for API consistency” mischaracterizes the scope of the changes, which primarily standardize Swagger response decorators and normalize authentication token handling rather than addressing lint errors. It does not clearly convey the adoption of custom ApiSuccessResponse, ApiErrorResponse, and ApiAuthResponse decorators or the token emission updates. Consequently, it could confuse reviewers about the primary intent of the pull request. Rename the title to explicitly reflect the main changes, for example: “chore: standardize Swagger response decorators and normalize auth token handling.” This will make the PR purpose clear to reviewers.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3368ab3 and dd57769.

📒 Files selected for processing (4)
  • src/modules/auth/controllers/auth.controller.ts (8 hunks)
  • src/modules/auth/dto/auth-response.dto.ts (1 hunks)
  • src/modules/offers/offer.controller.ts (2 hunks)
  • src/modules/offers/offer.module.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/modules/offers/offer.module.ts
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.ts

📄 CodeRabbit inference engine (.cursorrules)

src/**/*.ts: Do not access environment variables via process.env directly; import and use the config object from src/config/env
Provide explicit return types for all functions
Do not use the any type; use unknown or specific types instead
Create interfaces for complex object shapes
Type all function parameters explicitly
Do not use console.log; use proper logging (e.g., NestJS Logger)
Remove unused imports
Do not leave commented-out code in commits
Wrap risky operations in try-catch and handle errors appropriately
Add JSDoc comments for complex logic

Files:

  • src/modules/auth/dto/auth-response.dto.ts
  • src/modules/offers/offer.controller.ts
  • src/modules/auth/controllers/auth.controller.ts
src/modules/**

📄 CodeRabbit inference engine (.cursorrules)

Follow NestJS module structure within src/modules/[module-name]/

Files:

  • src/modules/auth/dto/auth-response.dto.ts
  • src/modules/offers/offer.controller.ts
  • src/modules/auth/controllers/auth.controller.ts
src/modules/**/dto/**/*.dto.ts

📄 CodeRabbit inference engine (.cursorrules)

src/modules/**/dto/**/*.dto.ts: Use DTOs for data validation
Use class-validator decorators on DTOs

Files:

  • src/modules/auth/dto/auth-response.dto.ts
src/modules/**/controllers/**/*.controller.ts

📄 CodeRabbit inference engine (.cursorrules)

Handle HTTP in controllers only

Files:

  • src/modules/auth/controllers/auth.controller.ts
🧬 Code graph analysis (2)
src/modules/offers/offer.controller.ts (1)
src/common/decorators/api-response.decorator.ts (2)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/auth/controllers/auth.controller.ts (2)
src/common/decorators/api-response.decorator.ts (3)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
  • ApiAuthResponse (66-92)
src/modules/auth/dto/auth-response.dto.ts (4)
  • ChallengeResponseDto (3-23)
  • AuthResponseDto (51-81)
  • UserResponseDto (83-112)
  • LogoutResponseDto (114-126)
🔇 Additional comments (8)
src/modules/offers/offer.controller.ts (3)

2-3: LGTM! Import updates align with standardization effort.

The removal of ApiResponse and addition of custom decorators from the common module correctly supports the PR's goal of standardizing API response metadata across controllers.


17-19: LGTM! Decorator usage correctly standardizes response metadata.

The decorators are applied correctly with appropriate status codes (201 for creation, 400 for validation errors, 404 for not found) and descriptive messages. The Offer model is properly passed to ApiSuccessResponse for schema generation.


20-22: LGTM! Method signature follows coding guidelines.

The method has an explicit return type (Promise<Offer>), typed parameters (CreateOfferDto), and follows NestJS controller conventions.

src/modules/auth/dto/auth-response.dto.ts (1)

58-63: LGTM! This change addresses the previous review concern.

The addition of the optional token?: string field to AuthResponseDto properly aligns the DTO with the actual response shape produced by ResponseInterceptor. The field is correctly documented with @ApiProperty and marked as optional with required: false.

This change directly resolves the past review comment about the mismatch between the DTO definition and the interceptor-wrapped response format.

src/modules/auth/controllers/auth.controller.ts (4)

21-21: LGTM! Custom decorators imported for consistent API documentation.

The import of ApiSuccessResponse, ApiErrorResponse, and ApiAuthResponse from the custom decorator file standardizes Swagger documentation across the controller and aligns with the PR's goal of API response consistency.


51-52: LGTM! Swagger decorators standardized for challenge endpoint.

The replacement of @ApiResponse with custom @ApiSuccessResponse and @ApiErrorResponse decorators provides clear, standardized documentation for the challenge generation endpoint. The decorators correctly reference ChallengeResponseDto and document the expected success and error scenarios.


115-117: LGTM! Consistent decorator and type improvements.

The registration endpoint follows the same improvement pattern as the login endpoint:

  1. Decorator standardization (lines 115-117): Proper use of @ApiAuthResponse for auth responses and @ApiErrorResponse for validation and conflict scenarios.

  2. Type safety (lines 135-147): The return statement is properly typed without as any casts, maintaining consistency with the updated AuthResponseDto.

Also applies to: 135-147


163-164: LGTM! Success/error decorators applied to remaining endpoints.

The updates to the getMe and logout endpoints complete the standardization effort:

  • Both endpoints now use @ApiSuccessResponse and @ApiErrorResponse for clear documentation.
  • Each endpoint correctly references its corresponding response DTO (UserResponseDto and LogoutResponseDto).
  • Error responses appropriately document authentication requirements with 401 status codes.

Also applies to: 200-201


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/modules/auth/services/auth.service.ts (2)

52-58: Replace console.log with NestJS Logger.

Direct console.log usage violates coding guidelines. Use the NestJS Logger for proper structured logging.

As per coding guidelines, apply this pattern throughout the file:

+import { Injectable, UnauthorizedException, Inject, forwardRef, Logger } from '@nestjs/common';
+
 @Injectable()
 export class AuthService {
+  private readonly logger = new Logger(AuthService.name);
   private readonly CHALLENGE_MESSAGE = 'StarShop Authentication Challenge';

   // In verifyStellarSignature method:
-      // eslint-disable-next-line no-console
-      console.log('Development mode: Bypassing signature verification for testing');
+      this.logger.warn('Development mode: Bypassing signature verification for testing');

-    // eslint-disable-next-line no-console
-    console.error('Signature verification error:', error);
+    this.logger.error('Signature verification error:', error);

Also applies to: 67-68


162-163: Replace console.error with NestJS Logger.

This console.error also violates coding guidelines and should use the Logger instance.

-      // eslint-disable-next-line no-console
-      console.error('Failed to create default store for seller:', error);
+      this.logger.error('Failed to create default store for seller:', error);
src/modules/coupons/controllers/coupon.controller.ts (1)

48-50: Restore a valid decorator here (compile error right now)

@ApiResponse(...) is still present, but you removed the ApiResponse import. That leaves an undefined identifier and TypeScript blows up. Either put the import back or (preferably) convert this block to the new ApiSuccessResponse/ApiErrorResponse pattern the rest of the file uses.

-  @ApiResponse({ status: 200, description: 'Coupon found', type: Coupon })
-  @ApiResponse({ status: 404, description: 'Coupon not found' })
+  @ApiSuccessResponse(200, 'Coupon found', Coupon)
+  @ApiErrorResponse(404, 'Coupon not found')
🧹 Nitpick comments (1)
src/modules/users/controllers/user.controller.ts (1)

70-71: Replace Object as any with the real auth response DTO

Passing Object as any throws away the OpenAPI schema you just tried to standardize. Reuse the concrete AuthResponseDto (the same one the AuthController exposes) or create a dedicated DTO so the decorator can emit the contract for clients. Right now the documentation regresses to an untyped blob.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b80d6d8 and 3368ab3.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (14)
  • src/cache/controllers/cache.controller.ts (5 hunks)
  • src/modules/attributes/controllers/attributes.controller.ts (7 hunks)
  • src/modules/auth/controllers/auth.controller.ts (8 hunks)
  • src/modules/auth/services/auth.service.ts (2 hunks)
  • src/modules/coupons/controllers/coupon.controller.ts (4 hunks)
  • src/modules/escrow/controllers/escrow.controller.ts (5 hunks)
  • src/modules/notifications/controllers/notification.controller.ts (3 hunks)
  • src/modules/offers/offfer.controller.ts (2 hunks)
  • src/modules/products/controllers/product.controller.ts (6 hunks)
  • src/modules/seller/controllers/seller.controller.ts (4 hunks)
  • src/modules/seller/tests/seller.e2e.spec.ts (15 hunks)
  • src/modules/stores/controllers/store.controller.ts (10 hunks)
  • src/modules/users/controllers/user.controller.ts (7 hunks)
  • test/auth.e2e-spec.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.ts

📄 CodeRabbit inference engine (.cursorrules)

src/**/*.ts: Do not access environment variables via process.env directly; import and use the config object from src/config/env
Provide explicit return types for all functions
Do not use the any type; use unknown or specific types instead
Create interfaces for complex object shapes
Type all function parameters explicitly
Do not use console.log; use proper logging (e.g., NestJS Logger)
Remove unused imports
Do not leave commented-out code in commits
Wrap risky operations in try-catch and handle errors appropriately
Add JSDoc comments for complex logic

Files:

  • src/modules/auth/services/auth.service.ts
  • src/modules/stores/controllers/store.controller.ts
  • src/modules/coupons/controllers/coupon.controller.ts
  • src/modules/offers/offfer.controller.ts
  • src/modules/seller/controllers/seller.controller.ts
  • src/modules/auth/controllers/auth.controller.ts
  • src/modules/products/controllers/product.controller.ts
  • src/modules/notifications/controllers/notification.controller.ts
  • src/modules/seller/tests/seller.e2e.spec.ts
  • src/cache/controllers/cache.controller.ts
  • src/modules/escrow/controllers/escrow.controller.ts
  • src/modules/users/controllers/user.controller.ts
  • src/modules/attributes/controllers/attributes.controller.ts
src/modules/**

📄 CodeRabbit inference engine (.cursorrules)

Follow NestJS module structure within src/modules/[module-name]/

Files:

  • src/modules/auth/services/auth.service.ts
  • src/modules/stores/controllers/store.controller.ts
  • src/modules/coupons/controllers/coupon.controller.ts
  • src/modules/offers/offfer.controller.ts
  • src/modules/seller/controllers/seller.controller.ts
  • src/modules/auth/controllers/auth.controller.ts
  • src/modules/products/controllers/product.controller.ts
  • src/modules/notifications/controllers/notification.controller.ts
  • src/modules/seller/tests/seller.e2e.spec.ts
  • src/modules/escrow/controllers/escrow.controller.ts
  • src/modules/users/controllers/user.controller.ts
  • src/modules/attributes/controllers/attributes.controller.ts
src/modules/**/services/**/*.service.ts

📄 CodeRabbit inference engine (.cursorrules)

Place business logic in services

Files:

  • src/modules/auth/services/auth.service.ts
src/modules/**/controllers/**/*.controller.ts

📄 CodeRabbit inference engine (.cursorrules)

Handle HTTP in controllers only

Files:

  • src/modules/stores/controllers/store.controller.ts
  • src/modules/coupons/controllers/coupon.controller.ts
  • src/modules/seller/controllers/seller.controller.ts
  • src/modules/auth/controllers/auth.controller.ts
  • src/modules/products/controllers/product.controller.ts
  • src/modules/notifications/controllers/notification.controller.ts
  • src/modules/escrow/controllers/escrow.controller.ts
  • src/modules/users/controllers/user.controller.ts
  • src/modules/attributes/controllers/attributes.controller.ts
src/**/*.spec.ts

📄 CodeRabbit inference engine (.cursorrules)

src/**/*.spec.ts: Create tests for new features
Mock external dependencies in tests
Test both success and error cases
Ensure tests are deterministic (no flakiness)

Files:

  • src/modules/seller/tests/seller.e2e.spec.ts
🧬 Code graph analysis (12)
src/modules/stores/controllers/store.controller.ts (2)
src/common/decorators/api-response.decorator.ts (1)
  • ApiSuccessResponse (12-43)
src/modules/stores/dto/store.dto.ts (1)
  • StoreResponseDto (229-295)
src/modules/coupons/controllers/coupon.controller.ts (1)
src/common/decorators/api-response.decorator.ts (2)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/offers/offfer.controller.ts (1)
src/common/decorators/api-response.decorator.ts (2)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/seller/controllers/seller.controller.ts (3)
src/common/decorators/api-response.decorator.ts (2)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/seller/dto/build-register.dto.ts (1)
  • BuildRegisterResponseDto (17-32)
src/modules/seller/dto/submit-register.dto.ts (1)
  • SubmitRegisterResponseDto (14-30)
src/modules/auth/controllers/auth.controller.ts (2)
src/common/decorators/api-response.decorator.ts (3)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
  • ApiAuthResponse (66-92)
src/modules/auth/dto/auth-response.dto.ts (4)
  • ChallengeResponseDto (3-23)
  • AuthResponseDto (51-74)
  • UserResponseDto (76-105)
  • LogoutResponseDto (107-119)
src/modules/products/controllers/product.controller.ts (1)
src/common/decorators/api-response.decorator.ts (2)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/notifications/controllers/notification.controller.ts (1)
src/common/decorators/api-response.decorator.ts (1)
  • ApiSuccessResponse (12-43)
src/modules/seller/tests/seller.e2e.spec.ts (1)
src/common/interceptors/index.ts (1)
  • ResponseInterceptor (1-1)
src/cache/controllers/cache.controller.ts (1)
src/common/decorators/api-response.decorator.ts (2)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/escrow/controllers/escrow.controller.ts (2)
src/common/decorators/api-response.decorator.ts (2)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/escrow/dto/release-funds-response.dto.ts (3)
  • ReleaseFundsResponseDto (5-29)
  • EscrowAccountDto (63-93)
  • MilestoneDto (31-61)
src/modules/users/controllers/user.controller.ts (2)
src/modules/auth/controllers/auth.controller.ts (1)
  • ApiTags (34-202)
src/common/decorators/api-response.decorator.ts (3)
  • ApiAuthResponse (66-92)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/attributes/controllers/attributes.controller.ts (2)
src/common/decorators/api-response.decorator.ts (2)
  • ApiSuccessResponse (12-43)
  • ApiErrorResponse (50-58)
src/modules/attributes/dto/attribute-response.dto.ts (2)
  • AttributeResponseDto (18-43)
  • PaginatedAttributesResponseDto (45-62)
🔇 Additional comments (8)
test/auth.e2e-spec.ts (1)

6-6: LGTM! Test environment now mirrors production setup.

The addition of ResponseInterceptor and global API prefix aligns the test environment with production behavior, ensuring tests validate the actual response format clients will receive.

Also applies to: 20-22

src/modules/auth/services/auth.service.ts (1)

313-313: LGTM! Token field rename improves API consistency.

The rename from access_token to token aligns with the PR's objective to standardize response formats across the API.

Also applies to: 353-353

src/modules/seller/tests/seller.e2e.spec.ts (1)

11-11: LGTM! Test setup properly aligned with production.

The test configuration now correctly applies the global API prefix and ResponseInterceptor, and token extraction has been updated to match the interceptor's response structure. Path updates throughout the file are consistent.

Also applies to: 38-40, 45-58

src/modules/auth/controllers/auth.controller.ts (1)

21-21: LGTM! Decorator migration standardizes API documentation.

The replacement of ApiResponse with ApiSuccessResponse, ApiErrorResponse, and ApiAuthResponse decorators provides consistent, well-structured API documentation across all authentication endpoints.

Also applies to: 51-52, 74-76, 112-114, 157-158, 194-195

src/modules/notifications/controllers/notification.controller.ts (1)

2-3: LGTM! Decorator migration is consistent.

The replacement of ApiResponse with ApiSuccessResponse aligns with the PR's standardization objectives. No business logic changes, only API documentation improvements.

Also applies to: 31-31, 48-48

src/modules/offers/offfer.controller.ts (1)

2-3: LGTM! Decorator migration follows established pattern.

The replacement of ApiResponse decorators with ApiSuccessResponse and ApiErrorResponse is consistent with changes across other controllers.

Also applies to: 17-19

src/cache/controllers/cache.controller.ts (1)

2-3: LGTM! All cache endpoints now use standardized decorators.

The migration from ApiResponse to ApiSuccessResponse and ApiErrorResponse is complete and consistent across all four cache management endpoints.

Also applies to: 19-20, 29-30, 40-41, 51-52

src/modules/stores/controllers/store.controller.ts (1)

16-17: LGTM! Comprehensive decorator standardization across all store endpoints.

All eight store endpoints now use ApiSuccessResponse and/or ApiErrorResponse decorators. The proper use of the isArray parameter for list endpoints (lines 56, 125, 137) ensures accurate OpenAPI documentation for array responses.

Also applies to: 36-36, 56-56, 71-71, 88-88, 108-108, 125-125, 137-137, 156-156, 176-176

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments