You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "martyngigg (via GitHub)" <gi...@apache.org> on 2025/11/10 12:04:07 UTC
[PR] fix(RightMenu): Ensure User Info link in RightMenu contains app root [superset]
martyngigg opened a new pull request, #36054:
URL: https://github.com/apache/superset/pull/36054
### SUMMARY
Includes app root in the link to the user information on the right menu.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
Refs #35027
- [x] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(RightMenu): Ensure User Info link in RightMenu contains app root [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #36054:
URL: https://github.com/apache/superset/pull/36054#discussion_r2510275539
##########
superset-frontend/src/features/home/RightMenu.tsx:
##########
@@ -477,7 +477,7 @@ const RightMenu = ({
userItems.push({
key: 'info',
label: (
- <Typography.Link href={navbarRight.user_info_url}>
+ <Typography.Link href={ensureAppRoot(navbarRight.user_info_url)}>
Review Comment:
### Missing null check before ensureAppRoot call <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The ensureAppRoot function is called on navbarRight.user_info_url without null/undefined checking, which could cause a runtime error if user_info_url is null or undefined.
###### Why this matters
If navbarRight.user_info_url is null or undefined, passing it to ensureAppRoot could result in unexpected behavior or runtime errors, potentially breaking the user info link functionality.
###### Suggested change β *Feature Preview*
Add a null check before calling ensureAppRoot:
```typescript
<Typography.Link href={navbarRight.user_info_url ? ensureAppRoot(navbarRight.user_info_url) : '#'}>
```
Or ensure the conditional check above properly handles the null case by changing the condition to check for truthy values:
```typescript
if (navbarRight.user_info_url) {
userItems.push({
key: 'info',
label: (
<Typography.Link href={ensureAppRoot(navbarRight.user_info_url)}>
{t('Info')}
</Typography.Link>
),
});
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bb00183-9793-4495-8e31-93055903eea6/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bb00183-9793-4495-8e31-93055903eea6?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bb00183-9793-4495-8e31-93055903eea6?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bb00183-9793-4495-8e31-93055903eea6?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bb00183-9793-4495-8e31-93055903eea6)
</details>
<sub>
π¬ Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:ea842276-653d-41ba-8d71-42e05b960a54 -->
[](ea842276-653d-41ba-8d71-42e05b960a54)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(RightMenu): Ensure links in RightMenu contains app root when applicable [superset]
Posted by "martyngigg (via GitHub)" <gi...@apache.org>.
martyngigg closed pull request #36054: fix(RightMenu): Ensure links in RightMenu contains app root when applicable
URL: https://github.com/apache/superset/pull/36054
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(RightMenu): Ensure User Info link in RightMenu contains app root [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #36054:
URL: https://github.com/apache/superset/pull/36054#issuecomment-3511209533
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(RightMenu): Ensure links in RightMenu contains app root when applicable [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #36054:
URL: https://github.com/apache/superset/pull/36054#issuecomment-3512371301
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/36054?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 48.43%. Comparing base ([`c42e3c6`](https://app.codecov.io/gh/apache/superset/commit/c42e3c68373f9a90b2b9f8e7f3341bf13e2fe37a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`56b136d`](https://app.codecov.io/gh/apache/superset/commit/56b136d28a66cd636b4e353a44d9aab5d7ac434d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #36054 +/- ##
===========================================
+ Coverage 0 48.43% +48.43%
===========================================
Files 0 622 +622
Lines 0 45738 +45738
Branches 0 4978 +4978
===========================================
+ Hits 0 22154 +22154
- Misses 0 22959 +22959
- Partials 0 625 +625
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/36054/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/36054/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `44.16% <ΓΈ> (?)` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/36054/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.71% <ΓΈ> (?)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/36054/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.43% <ΓΈ> (?)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/36054/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ΓΈ> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/36054?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(RightMenu): Ensure links in RightMenu contains app root when applicable [superset]
Posted by "martyngigg (via GitHub)" <gi...@apache.org>.
martyngigg commented on PR #36054:
URL: https://github.com/apache/superset/pull/36054#issuecomment-3512624479
Closing this in favour of wider changes to fix app root issues.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org