Skip to content

Conversation

@akshaydeo
Copy link
Contributor

@akshaydeo akshaydeo commented Dec 17, 2025

Summary

Improved the enterprise feature redirection in the proxy configuration page by using client-side navigation instead of server-side redirects.

Changes

  • Replaced redirect() from Next.js navigation with useRouter().replace() for client-side redirection
  • Added a useEffect hook to handle the redirection logic
  • Added a null return when not in enterprise mode to prevent flash of content
  • Fixed semicolon consistency in the file

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (Next.js)
  • Docs

How to test

  1. Run the application in non-enterprise mode:
cd ui
pnpm i
pnpm dev
  1. Navigate to /workspace/config/proxy
  2. Verify you are redirected to /workspace/config/client-settings without any errors or flash of content

Breaking changes

  • Yes
  • No

Related issues

N/A

Security considerations

No security implications as this is a UI navigation change only.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 17, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed redirect behavior to improve navigation consistency and reliability.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

The redirect logic in the proxy configuration page is refactored from render-time execution to a client-side effect using useEffect. When IS_ENTERPRISE is false, the redirect now occurs asynchronously via the effect hook. The component conditionally renders ProxyView only when IS_ENTERPRISE is true.

Changes

Cohort / File(s) Summary
Client-side redirect refactoring
ui/app/workspace/config/proxy/page.tsx
Migrates redirect logic from synchronous render-time execution to async client-side effect using useEffect. Adds router integration and conditional ProxyView rendering based on IS_ENTERPRISE flag.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the useEffect dependency array is correct and prevents infinite redirect loops
  • Confirm router usage follows project conventions
  • Check that the conditional render (IS_ENTERPRISE check) aligns with intended behavior for non-enterprise instances
  • Ensure no race conditions during page transitions

Poem

🐰 A hop from render to effect we take,
Client-side redirects now gently awake,
Enterprise flows smooth, the logic refined,
Async redirects leave stale paths behind!

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 12-17-adds_router_based_redirect_to_proxy_view

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b24de3 and b98d355.

📒 Files selected for processing (1)
  • ui/app/workspace/config/proxy/page.tsx (1 hunks)

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

@akshaydeo akshaydeo marked this pull request as ready for review December 17, 2025 15:43
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #1125

@akshaydeo akshaydeo merged commit f324fb5 into main Dec 17, 2025
10 of 11 checks passed
@akshaydeo akshaydeo deleted the 12-17-adds_router_based_redirect_to_proxy_view branch December 17, 2025 15:45
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