codeant-ai-for-open-source[bot] commented on code in PR #41938:
URL: https://github.com/apache/superset/pull/41938#discussion_r3572665617
##########
superset-frontend/src/middleware/loggerMiddleware.ts:
##########
@@ -131,6 +136,12 @@ const logMessageQueue = new
DebouncedMessageQueue<LogEventData>({
delayThreshold: 1000,
});
+// Embedded dashboards are served from `/dashboard/:idOrSlug/embedded/` and
+// `/embedded/:uuid/`. Matching these specific shapes avoids false positives
+// for regular dashboards (e.g. a dashboard whose slug is "embedded").
+const EMBEDDED_ROUTE_REGEX =
+ /\/dashboard\/[^/]+\/embedded\/|\/embedded\/[^/]+\//;
Review Comment:
**Suggestion:** The embedded-route regex requires a trailing slash after the
UUID/slug, so valid embedded URLs without a final slash (which React Router can
still match) will not be classified as embedded and will be logged with the
wrong source. Make the trailing slash optional in both embedded route branches.
[incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Embedded dashboards without trailing slash logged without embedded
source.
- ⚠️ Event log segmentation between dashboard types becomes inconsistent.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start the embedded Superset frontend, which serves embedded dashboards
via React Router
paths `/dashboard/:idOrSlug/embedded/` and `/embedded/:uuid/` as defined in
`superset-frontend/src/embedded/index.tsx:21-22`.
2. Navigate the browser directly to an embedded dashboard URL without a
trailing slash,
e.g. `http://localhost/embedded/abc-def-uuid` or
`http://localhost/dashboard/123/embedded`, which still targets the embedded
routes but
omits the final `/`.
3. Trigger a logging event such as `LOG_ACTIONS_LOAD_CHART` (see test
harness in
`superset-frontend/src/middleware/logger.test.ts:50-59`), which is processed
by
`loggerMiddleware` in
`superset-frontend/src/middleware/loggerMiddleware.ts:147-174`;
because no SPA navigation event was just logged, `navPath` remains undefined
and `path` is
set from `window.location.href` containing the non-slashed embedded URL.
4. At `superset-frontend/src/middleware/loggerMiddleware.ts:139-143`,
`EMBEDDED_ROUTE_REGEX` requires a trailing slash after the
`:idOrSlug`/`:uuid`, and
`isEmbedded` is computed at line 179 via `EMBEDDED_ROUTE_REGEX.test(path ??
'')`; for URLs
without the final slash this test fails, so the condition at line 180 does
not set
`source: 'embedded_dashboard'` or `dashboard_id`, and events sent by
`sendBeacon` (lines
98-128) are logged without embedded dashboard metadata.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=489a6d8a81224b0c8a982dd731e79b4c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=489a6d8a81224b0c8a982dd731e79b4c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/middleware/loggerMiddleware.ts
**Line:** 142:143
**Comment:**
*Incorrect Condition Logic: The embedded-route regex requires a
trailing slash after the UUID/slug, so valid embedded URLs without a final
slash (which React Router can still match) will not be classified as embedded
and will be logged with the wrong source. Make the trailing slash optional in
both embedded route branches.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41938&comment_hash=5824158a90c5fe1796dc9e7dea140b88ed427c5ea3e09042f36102a42e5d7664&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41938&comment_hash=5824158a90c5fe1796dc9e7dea140b88ed427c5ea3e09042f36102a42e5d7664&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
