refactor(be): remove deprecated endpoints #44
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces several modifications across multiple controller files and a constants file. In Possibly related PRs
Warning Rate limit exceeded@krystxf has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/backend/src/modules/departure/departure.controller.ts (3)
Line range hint
21-21: Fix incorrect cache TTL valueThe
@CacheTTLdecorator expects a value in seconds. Currently,4 * 1000sets the TTL to 4000 seconds (approximately 66 minutes), which might not be intended. If you meant to cache the data for 4 seconds, please adjust the value accordingly.Apply this diff to correct the cache TTL:
-@CacheTTL(4 * 1000) +@CacheTTL(4)
Line range hint
29-58: Refactor to eliminate duplicate code ingetDeparturesV1andgetDeparturesV2The implementations of
getDeparturesV1andgetDeparturesV2are nearly identical, differing only in the service instance used (departureServiceV1vsdepartureServiceV2). To adhere to the DRY (Don't Repeat Yourself) principle and enhance maintainability, consider extracting the shared logic into a private method.Here is a suggested refactor:
Define a private method to handle the common logic:
private async handleGetDepartures( query, service: DepartureServiceV1 | DepartureServiceV2, ): Promise<DepartureSchema[]> { const schema = z.object({ metroOnly: metroOnlySchema, platform: z.string().array().optional().default([]), stop: z.string().array().optional().default([]), }); const parsed = schema.safeParse(query); if (!parsed.success) { throw new BadRequestException('Invalid query params'); } const parsedQuery = parsed.data; if (parsedQuery.platform.length + parsedQuery.stop.length === 0) { throw new BadRequestException('At least one platform or stop ID must be provided'); } const departures = await service.getDepartures({ stopIds: parsedQuery.stop, platformIds: parsedQuery.platform, metroOnly: parsedQuery.metroOnly, }); return departureSchema.array().parse(departures); }Update the versioned methods to utilize the new private method:
@Get() @Version([EndpointVersion.v1]) async getDeparturesV1(@Query() query): Promise<DepartureSchema[]> { - // Existing implementation... + return this.handleGetDepartures(query, this.departureServiceV1); } @Get() @Version([EndpointVersion.v2]) async getDeparturesV2(@Query() query): Promise<DepartureSchema[]> { - // Existing implementation... + return this.handleGetDepartures(query, this.departureServiceV2); }Also applies to: 61-90
Line range hint
40-43: Use specific exception classes for clarityInstead of using
HttpExceptionwith an HTTP status code, consider using specific exception classes likeBadRequestExceptionprovided by NestJS. This enhances readability and aligns with NestJS best practices.Apply this diff to improve exception handling:
- throw new HttpException( - "Invalid query params", - HttpStatus.BAD_REQUEST, - ); + throw new BadRequestException("Invalid query params");and
- throw new HttpException( - "At least one platform or stop ID must be provided", - HttpStatus.BAD_REQUEST, - ); + throw new BadRequestException("At least one platform or stop ID must be provided");Also applies to: 47-50, 72-75, 79-82
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
apps/backend/src/constants/swagger.const.ts(0 hunks)apps/backend/src/modules/departure/departure.controller.ts(1 hunks)apps/backend/src/modules/platform/platform.controller.ts(0 hunks)apps/backend/src/modules/stop/stop.controller.ts(0 hunks)
💤 Files with no reviewable changes (3)
- apps/backend/src/constants/swagger.const.ts
- apps/backend/src/modules/platform/platform.controller.ts
- apps/backend/src/modules/stop/stop.controller.ts
krystxf
force-pushed
the
refactor/remove-deprecated-endpoints
branch
from
4f95d29
to
e82e6b5
Compare
krystxf
force-pushed
the
refactor/remove-deprecated-endpoints
branch
from
e82e6b5
to
ed69b16
Compare
krystxf
force-pushed
the
refactor/remove-deprecated-endpoints
branch
from
ed69b16
to
49efb29
Compare
Changes
/departure/platform(it's been replaced by/departurewith search params)Warning
BREAKING CHANGES
This PR should be merged after the majority of users update to the app version
0.3