Skip to content

Limit redirects to dashboard defined route#1709

Merged
joepavitt merged 4 commits into
mainfrom
1708-limit-redirtects
May 26, 2025
Merged

Limit redirects to dashboard defined route#1709
joepavitt merged 4 commits into
mainfrom
1708-limit-redirtects

Conversation

@Steve-Mcl

@Steve-Mcl Steve-Mcl commented May 18, 2025

Copy link
Copy Markdown
Contributor

closes #1708

Description

  1. uses config.path to scope the redirect middleware
  2. Changes the workbox service worker allowList to a denyList to prevent pre-caching of URLs where the pathname part contains a trailing slash - so that it hits the backend to be handled by the server.
    1. Prior to this, in dev mode, the main url was still being cached!
    2. Prior to this, pages ending with / were being precached and prevented the routing to reach the server

Related Issue(s)

#1708

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

@Steve-Mcl Steve-Mcl requested review from Copilot and joepavitt May 18, 2025 15:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR scopes client-side precaching and server-side redirect handling to the configured base path only.

  • Update Service Worker to use a denylist for dev mode and trailing-slash URLs under /dashboard
  • Scope the Express redirect middleware to config.path and adjust URL construction

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
ui/src/sw.js Replaces allowlist with denylist, prevents precaching in dev and for trailing-slash dashboard URLs
nodes/config/ui_base.js Scopes the trailing-slash redirect middleware to config.path and prefixes redirect URLs accordingly
Comments suppressed due to low confidence (1)

ui/src/sw.js:12

  • This JSDoc refers to allowlist which has been removed; update this annotation to reference denylist or remove it if it's no longer necessary.
/** @type {RegExp[] | undefined} */

Comment thread ui/src/sw.js Outdated
Comment thread nodes/config/ui_base.js

@joepavitt joepavitt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified and working locally

@joepavitt

Copy link
Copy Markdown
Contributor

I ran the E2E's locally just to be sure there isn't anything funky going on, and 2 x form tests are failing here too. Investigating further before merging and getting the new release out.

@joepavitt joepavitt force-pushed the 1708-limit-redirtects branch from 2621eb8 to 9e09dfb Compare May 26, 2025 09:52
@joepavitt joepavitt merged commit 916352e into main May 26, 2025
2 checks passed
@joepavitt joepavitt deleted the 1708-limit-redirtects branch May 26, 2025 10:26
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.

Redirect Loop for 3rd Party Projects

3 participants