Skip to content

fix(event-log): require segment boundary after 'embedded' in route regex#42011

Closed
luizotavio32 wants to merge 1 commit into
apache:6.1from
luizotavio32:fix-embedded-route-boundary-6.1
Closed

fix(event-log): require segment boundary after 'embedded' in route regex#42011
luizotavio32 wants to merge 1 commit into
apache:6.1from
luizotavio32:fix-embedded-route-boundary-6.1

Conversation

@luizotavio32

Copy link
Copy Markdown
Contributor

SUMMARY

Backports to 6.1 the segment-boundary fix for embedded-route classification in loggerMiddleware.

6.1 already carries the earlier embedded-route work (the regex with an optional trailing slash). However, the first branch of that regex does not enforce an end-of-segment boundary after embedded, so EMBEDDED_ROUTE_REGEX.test(...) also matches prefixes like /dashboard/123/embeddedXYZ, misclassifying those events with source: 'embedded_dashboard' instead of source: 'dashboard'.

This requires an end-of-segment boundary (/, ?, #, or end of string) after embedded:

-  /\/dashboard\/[^/]+\/embedded\/?|\/embedded\/[^/]+\/?/;
+  /\/dashboard\/[^/]+\/embedded(?:[/?#]|$)|\/embedded\/[^/]+\/?/;

The second branch (/embedded/:uuid) is unchanged — its [^/]+ already treats the UUID as a full segment.

This mirrors the fix under review for master in #42005.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — internal event-logging metadata change only.

TESTING INSTRUCTIONS

Unit tests in superset-frontend/src/middleware/logger.test.ts add coverage for the prefix case:

  • /dashboard/123/embeddedXYZ/ -> dashboard

alongside the existing embedded/regular-dashboard cases.

Run:

cd superset-frontend
npm run test -- src/middleware/logger.test.ts

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

The optional trailing slash allowed the first branch to match prefixes
like `/dashboard/123/embeddedXYZ`, misclassifying them as embedded. Require
an end-of-segment boundary (`/`, `?`, `#`, or end of string) after
`embedded` and add test coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dosubot dosubot Bot added the change:frontend Requires changing the frontend label Jul 13, 2026
@bito-code-review

bito-code-review Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Bito Automatic Review Skipped - Branch Excluded

Bito didn't auto-review because the source or target branch is excluded from automatic reviews.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the branch exclusion settings here, or contact your Bito workspace admin at evan@preset.io.

@luizotavio32 luizotavio32 marked this pull request as draft July 13, 2026 21:09
@netlify

netlify Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 5d86444
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a5553ab21d88900080c534c
😎 Deploy Preview https://deploy-preview-42011--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@luizotavio32 luizotavio32 deleted the fix-embedded-route-boundary-6.1 branch July 14, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant