Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frontend: Fix broken links in /plugins when pathname has a trailing slash #64348

Merged
merged 1 commit into from Mar 10, 2023

Conversation

gassiss
Copy link
Contributor

@gassiss gassiss commented Mar 7, 2023

This PR fixes a bug that makes all plugin links in the plugins page 404.

When on /plugins/ route (with the trailing slash), the links get broken:

image

Which makes every plugin link 404:

image

@gassiss gassiss requested a review from a team as a code owner March 7, 2023 17:58
@gassiss gassiss requested review from jackw and removed request for a team March 7, 2023 17:58
@leventebalogh
Copy link
Contributor

leventebalogh commented Mar 8, 2023

@andresmgot just helped me, I was missing the trailing / in the URL (was testing with /plugins instead of /plugins/), now I can reproduce it 🤦‍♂️

Hey @gassiss!

Which version are you looking at?
I was trying to reproduce it from the latest main, but couldn't (both with top-nav on and off) 🤔

Screenshot 2023-03-08 at 8 48 03

Copy link
Contributor

@leventebalogh leventebalogh left a comment

Choose a reason for hiding this comment

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

Nice catch 👏

const location = useLocation();
const pathName = config.appSubUrl + location.pathname;
const { pathname } = useLocation();
const pathName = config.appSubUrl + (pathname.endsWith('/') ? pathname.slice(0, -1) : pathname);
Copy link
Contributor

Choose a reason for hiding this comment

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

Although I think probably the best would be if the last item in the chain (PluginListItem) would do the validation - so whatever gets passed in it renders the correct links -, this change does the job as well. 👍

@gassiss gassiss added no-changelog Skip including change in changelog/release notes no-backport Skip backport of PR labels Mar 9, 2023
@leventebalogh leventebalogh added this to the 9.5.0 milestone Mar 10, 2023
@leventebalogh leventebalogh added backport v9.3.x backport v9.4.x Mark PR for automatic backport to v9.4.x add to changelog backport v9.0.x backport v9.1.x Bot will automatically open backport PR backport v9.2.x Mark PR for automatic backport to v9.2.x and removed no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes backport v9.0.x backport v9.1.x Bot will automatically open backport PR backport v9.2.x Mark PR for automatic backport to v9.2.x backport v9.3.x labels Mar 10, 2023
@gassiss gassiss changed the title Fix broken links in /plugins when pathname has a trailing slash Frontend: Fix broken links in /plugins when pathname has a trailing slash Mar 10, 2023
@leventebalogh leventebalogh merged commit a05cb1e into main Mar 10, 2023
34 checks passed
@leventebalogh leventebalogh deleted the plugins-fix-links branch March 10, 2023 13:18
grafanabot pushed a commit that referenced this pull request Mar 10, 2023
…lash (#64348)

Fix broken links in /plugins when pathname has a trailing slash

(cherry picked from commit a05cb1e)
eleijonmarck pushed a commit that referenced this pull request Mar 13, 2023
…lash (#64348)

Fix broken links in /plugins when pathname has a trailing slash
leventebalogh pushed a commit that referenced this pull request Mar 13, 2023
…railing slash (#64608)

Frontend: Fix broken links in /plugins when pathname has a trailing slash (#64348)

Fix broken links in /plugins when pathname has a trailing slash

(cherry picked from commit a05cb1e)

Co-authored-by: William Assis <35489495+gassiss@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants