Skip to content

Conversation

@AntonioABLima
Copy link
Collaborator

@AntonioABLima AntonioABLima commented Oct 13, 2025

This PR replaces history-based navigation -1 as To with a dynamic URL using globalThis.location.origin.
This ensures consistent navigation behavior when users access the Settings page directly and click the Back button.
The change happens inside the PageLayout component, specifically in the backTo property.

Motivation

  • Fix inconsistent Back button behavior when opening Settings directly.
  • Improve user experience.
  • Remove dependency on browser history.
  • Make navigation environment-independent.

How to Test

✅ You should be redirected to the Langflow home page instead of the browser’s default previous page.

Summary by CodeRabbit

  • Bug Fixes
    • Updated Settings page back navigation to return users to the Flows page instead of doing nothing.
    • Ensures consistent and predictable back behavior across the app for a smoother UX.
    • Uses the current site URL to build the destination, preventing incorrect redirects across environments.
    • No changes to general settings display or other page behavior.

…ng window.location.origin in SettingsPage

- Replace backTo={-1 as To} with window.location.origin
- Previously, if the user opened the Settings page directly and clicked Back, they would be redirected to the browser's default page (e.g., home page or previous site).
- Now, they will always be redirected to the Langflow home page.
- This ensures that the Back button works correctly across different environments (development, staging, production).
- Maintains the same functionality while making navigation environment-independent.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

Walkthrough

Updates Settings page back navigation: adds a flowsUrl derived from window.location.origin and passes it to PageLayout’s backTo prop, changing the back action to navigate to the Flows page. No other logic or exports modified.

Changes

Cohort / File(s) Summary of changes
Frontend Settings back navigation
src/frontend/src/pages/SettingsPage/index.tsx
Introduces flowsUrl = ${window.location.origin}/flows and replaces backTo={-1} with backTo={flowsUrl} to redirect back action to Flows page. No other logic altered.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant SP as SettingsPage
  participant PL as PageLayout
  participant B as Browser

  U->>SP: Open Settings
  SP->>PL: Render with backTo = flowsUrl (origin + "/flows")
  U->>PL: Click Back
  PL->>B: Navigate to flowsUrl (/flows on current origin)
  Note over PL,B: Back target changed from -1 (no-op) to explicit /flows
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

bug

Suggested reviewers

  • edwinjosechittilappilly
  • deon-sanchez
  • mfortman11

Pre-merge checks and finishing touches

❌ Failed checks (1 error, 2 warnings)
Check name Status Explanation Resolution
Test Coverage For New Implementations ❌ Error The PR modifies only src/frontend/src/pages/SettingsPage/index.tsx, introducing no accompanying test updates or new test files. Given that this is a bug fix aimed at ensuring the Settings page back navigation routes correctly, a regression test (likely a frontend test such as a Cypress or React Testing Library spec) should be added to cover the new behavior. Without such a test, the change lacks automated verification, so the custom check fails. Please add an automated test that verifies the Settings page Back button now navigates to /flows when the page is accessed directly, following the project’s frontend test conventions (e.g. *.test.ts). Once that regression coverage is in place, rerun this check.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Quality And Coverage ⚠️ Warning I inspected the updated SettingsPage implementation and found no accompanying unit, integration, or end-to-end tests verifying the new navigation behavior; existing tests remain untouched, so the change to the Back button destination is untested and there is no coverage ensuring the client-side routing regression is prevented. Please add a frontend test (following the project’s standard framework) that renders the Settings page and asserts the Back button targets “/flows” via client-side navigation, then re-run the check.
✅ Passed checks (4 passed)
Check name Status Explanation
Test File Naming And Structure ✅ Passed A repository-wide search shows backend tests named with the required test_*.py pattern using pytest constructs, while frontend tests consistently use the .test.tsx suffix with Playwright-style structure; sampled files include descriptive test names, clear setup, and both positive and negative scenarios where applicable, with integration tests stored in dedicated directories, so the project already meets the requested conventions and this PR does not regress them.
Excessive Mock Usage Warning ✅ Passed I inspected the repository’s test files and found no usage of frameworks like Jest or Vitest that would allow spy or mock helpers, and the codebase contains no instances of mock, spyOn, or similar mocking constructs. Since there are no mocks present, there is no evidence of excessive mock usage obscuring test intent or substituting for real interactions. Consequently, the custom check’s criteria are satisfied.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly summarizes the primary change of replacing history-based back navigation with a dynamic URL in the SettingsPage component and specifies the implementation detail of using globalThis.location.origin. It is a single concise sentence that directly reflects the main purpose of the changeset without extraneous file names or vague terminology, ensuring that teammates reviewing the PR understand its focus at a glance.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
Copy link
Contributor

@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: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf387bf and fa3c98e.

📒 Files selected for processing (1)
  • src/frontend/src/pages/SettingsPage/index.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/frontend/src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)

src/frontend/src/**/*.{ts,tsx,js,jsx}: All frontend TypeScript and JavaScript code should be located under src/frontend/src/ and organized into components, pages, icons, stores, types, utils, hooks, services, and assets directories as per the specified directory layout.
Use React 18 with TypeScript for all UI components in the frontend.
Format all TypeScript and JavaScript code using the make format_frontend command.
Lint all TypeScript and JavaScript code using the make lint command.

Files:

  • src/frontend/src/pages/SettingsPage/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Run Frontend Tests / Determine Test Suites and Shard Distribution
  • GitHub Check: Test Docker Images / Test docker images
  • GitHub Check: Test Starter Templates

Comment on lines 18 to 19
const flowsUrl = `${window.location.origin}/flows`;

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential base path issue: use a relative path instead.

Constructing the URL with window.location.origin will fail if the application is deployed under a base path (e.g., example.com/langflow/), as it would navigate to example.com/flows instead of example.com/langflow/flows. Additionally, using a full URL may trigger a full page reload rather than client-side routing.

Apply this diff to use a relative path:

-  const flowsUrl = `${window.location.origin}/flows`;
+  const flowsUrl = "/flows";

Or pass the path directly to the backTo prop:

-  const flowsUrl = `${window.location.origin}/flows`;
-
   // Hides the General settings if there is nothing to show
   const showGeneralSettings = ENABLE_PROFILE_ICONS || hasStore || !autoLogin;

   const sidebarNavItems: {

And update line 94:

     <PageLayout
-      backTo={flowsUrl}
+      backTo="/flows"
       title="Settings"
       description="Manage the general settings for Langflow."
     >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const flowsUrl = `${window.location.origin}/flows`;
@@ src/frontend/src/pages/SettingsPage/index.tsx:18
const flowsUrl = "/flows";
@@ src/frontend/src/pages/SettingsPage/index.tsx:94
backTo="/flows"
🤖 Prompt for AI Agents
In src/frontend/src/pages/SettingsPage/index.tsx around lines 18-19, the code
builds flowsUrl using window.location.origin which breaks when the app is served
under a base path and forces a full-page reload; replace the absolute URL with a
relative path (e.g., '/flows' or just 'flows' as appropriate) and pass that
relative path directly to the backTo prop so client-side routing is used instead
of constructing a full origin-based URL.

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
@sonarqubecloud
Copy link

@AntonioABLima AntonioABLima changed the title fix: replace history-based navigation (-1 as To) with dynamic URL using window.location.origin in SettingsPage fix: replace history-based navigation (-1 as To) with dynamic URL using globalThis.location.origin in SettingsPage Oct 13, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant