fix(event-log): match embedded routes precisely in logger middleware#41942
Merged
michael-s-molina merged 1 commit intoJul 13, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41942 +/- ##
=======================================
Coverage 64.79% 64.79%
=======================================
Files 2740 2740
Lines 153125 153126 +1
Branches 35124 35125 +1
=======================================
+ Hits 99219 99220 +1
Misses 52009 52009
Partials 1897 1897
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
luizotavio32
changed the title
9 tasks
Contributor
Code Review Agent Run #539aa5Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
michael-s-molina
approved these changes
Jul 13, 2026
luizotavio32
added a commit
to luizotavio32/superset
that referenced
this pull request
Jul 13, 2026
Cherry-picks the AI-suggested fix from apache#41942. The embedded-dashboard detection classified events by checking whether the URL contained the substring `/embedded/`, which misclassified a regular dashboard whose slug is literally `embedded` (URL `/dashboard/embedded/`). Replace the generic substring check with a regex that matches only the actual embedded route shapes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9 tasks
luizotavio32
added a commit
to luizotavio32/superset
that referenced
this pull request
Jul 13, 2026
React Router matches the embedded routes without a trailing slash too, so `/dashboard/:idOrSlug/embedded` and `/embedded/:uuid` are valid embedded URLs. The regex introduced in apache#41942 required a trailing slash, so those events were classified with the wrong source. Make the trailing slash optional in both branches and add test coverage for the no-slash variants. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
This implements a suggestion raised by a review bot on PR #41938.
The embedded-dashboard detection in
loggerMiddlewareclassified events by checking whether the URL contained the substring/embedded/. That check is too broad: a regular dashboard whose slug is literallyembedded(URL/dashboard/embedded/) also contains/embedded/, so its events were incorrectly tagged withsource: 'embedded_dashboard'instead ofsource: 'dashboard'.This replaces the generic substring check with a regular expression that matches only the actual embedded route shapes:
/dashboard/:idOrSlug/embedded//embedded/:uuid/As a result, regular dashboard events are no longer misclassified as embedded, while genuine embedded routes continue to be detected correctly.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — internal event-logging metadata change only.
TESTING INSTRUCTIONS
Unit tests were added in
superset-frontend/src/middleware/logger.test.tscovering source classification across paths:/dashboard/123/embedded/->embedded_dashboard/embedded/abc-def-uuid/->embedded_dashboard/dashboard/123/->dashboard/dashboard/embedded/(slug is literallyembedded) ->dashboardRun:
ADDITIONAL INFORMATION