fix: redirect swagger fixed - #1572
Conversation
Signed-off-by: joaoMiraya <joaomiraya01@gmail.com>
|
Warning Review limit reached
Next review available in: 57 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe API gateway now conditionally redirects the root route to ChangesSwagger redirect
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized redirect change has no actionable merge-blocking risk remaining beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
@joaoMiraya - Thanks for your contribution. I have one suggestion. Instead of limiting the Swagger UI redirect to development mode only, I think it would be better to make it configurable through some different flag in .env file. That way, users can decide which environments they want to expose the Swagger endpoint in. For example, if someone wants to enable it in production for their own use case, they should be able to do so. This keeps the default behavior safe while giving users the flexibility to configure it based on their needs. |
Signed-off-by: joaoMiraya <joaomiraya01@gmail.com>
Its looks great, i gonna make it |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api-gateway/src/app.controller.ts (1)
16-17: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid a permanent redirect for a runtime-configurable feature.
A
301is heuristically cacheable; clients or proxies can retain/apiafterENABLE_SWAGGERis later disabled. Use a temporary redirect such as302/307, or explicitly prevent caching. (rfc-editor.org)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api-gateway/src/app.controller.ts` around lines 16 - 17, Update the root route’s `@Redirect` in the controller to use a temporary redirect status, such as 302 or 307, instead of the permanent 301 so runtime configuration changes are not cached by clients or proxies.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.sample:
- Around line 275-277: Update the ENABLE_SWAGGER setting in the generic
environment sample to false or remove it, ensuring Swagger is not enabled by
default; retain true only in explicitly demo or development templates.
In `@libs/config/src/config.service.ts`:
- Around line 29-30: Update the isSwaggerAvailable getter in ConfigService to
normalize the ENABLE_SWAGGER environment value at runtime, converting string
values such as "false" to boolean false before returning it. Preserve true
behavior for enabled values and ensure the root redirect does not treat the
string "false" as truthy.
---
Outside diff comments:
In `@apps/api-gateway/src/app.controller.ts`:
- Around line 16-17: Update the root route’s `@Redirect` in the controller to use
a temporary redirect status, such as 302 or 307, instead of the permanent 301 so
runtime configuration changes are not cached by clients or proxies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ef8721b9-401b-4977-8785-1923ddd96c4b
📒 Files selected for processing (4)
.env.demo.env.sampleapps/api-gateway/src/app.controller.tslibs/config/src/config.service.ts
Signed-off-by: joaoMiraya <joaomiraya01@gmail.com>
|
Hi @joaoMiraya, |
Signed-off-by: joaoMiraya <joaomiraya01@gmail.com>
Signed-off-by: João Victor B Miraya <108498460+joaoMiraya@users.noreply.github.com>
|
Hello, It's done |
3 participants
Redirect http://localhost:5000 -> http://localhost:5000/api when's development mode
Issue
Summary by CodeRabbit