[MM-19193] Fix basename in paths to System Console #3885

mickmister · 2019-10-07T05:38:30Z

Summary

This PR fixes an issue where links to the System Console do not take into account the basename of the MM server. So servers serving only from subpaths would result in a 404.

Ticket Link

https://mattermost.atlassian.net/browse/MM-19193

This PR has changes to en.json

lindalumitchell

QA will test this after it's merged.

lieut-data

Makes sense -- just a request to use getBasePath throughout below.

lieut-data · 2019-10-08T19:46:59Z

components/announcement_bar/configuration_bar/configuration_bar.jsx

 }

- const siteURLMessage = formatMessage({id, defaultMessage});
+ let basename = window.basename;


Can we avoid using window.basename (i.e. the global) directly? And instead bind the result of getBasePath into the component?

lieut-data

Thanks!!

lindalumitchell · 2019-10-09T22:36:47Z

@mickmister I just inadvertently found a System Console link I hadn't seen before, but this one has a different problem. Should the following be a separate ticket, or should it be fixed along with these?

On a server that does not have Bot Accounts enabled (System Console > Integrations > Bot Accounts)
Go to main menu > Integrations > Bot Accounts, and click the System Console link in the helper text at the top
Observed: Just loads blank integrations page. Note the URL on hover shows superfluous %siteURL%7D between the actual site URL and /admin_console/integrations/bot_accounts
Expected: Opens Bot Accounts page in System Console

jasonblais · 2019-10-09T23:09:46Z

May be caused by #3632

@jfrerich wondering if it's due to the extra siteURL that was added to the System Console link?

jfrerich · 2019-10-10T02:33:53Z

@jasonblais, Just looking at this, I would assume this is an issue related to the PR I merged last night. It seems that {siteURL} must not be necessary afterall, although I found other instances of the same usage. I'll check this first thing tomorrow.

mickmister · 2019-10-10T02:53:10Z

I added a fix for the bot account link @jfrerich @lindalumitchell

It seems that {siteURL} must not be necessary afterall, although I found other instances of the same usage.

Since we need to take subpaths into account, we need to either explicitly include the basename (what this PR does), or include the siteURL. Both methods are essentially trying to form a URL the browser can process properly.

It turns out that getSiteURL doesn't even take the configured SiteURL into account. It's basically just origin + basename.

mattermost-webapp/utils/url.jsx

Lines 45 to 47 in e7c9f00

 export function getSiteURL() { 

 return getSiteURLFromWindowObject(window); 

 }

mickmister · 2019-10-10T03:42:54Z

After seeing the usage of siteURL over basename in en.json, I've decided to change the translations to use siteURL instead, as it is cleaner and less error-prone.

jfrerich · 2019-10-10T15:18:16Z

@mickmister. This was also cherry-picked into the release-5.16. Can you cherry-pick after this merge?

@lindalumitchell, @jasonblais I applied v5.16.0 milestone and CherryPick/Approved labels. This will need to be added to fix this commit 100e9dd in the release-5.16 branch.

lieut-data · 2019-10-15T13:10:08Z

components/plugin_marketplace/__snapshots__/marketplace_modal.test.jsx.snap

@@ -10,7 +10,7 @@ exports[`components/MarketplaceModal should match the snapshot, error banner is
 className="error-bar"
 dangerouslySetInnerHTML={
 Object {
- "__html": "<p>Error connecting to the marketplace server. Please check your settings in the <a class=\\"theme markdown__link\\" href=\\"/admin_console/plugins/plugin_management\\" rel=\\"noreferrer\\" data-link=\\"/admin_console/plugins/plugin_management\\">System Console</a>.</p>",
+ "__html": "<p>Error connecting to the marketplace server. Please check your settings in the <a class=\\"theme markdown__link\\" href=\\"http:https://example.com/admin_console/plugins/plugin_management\\" rel=\\"noreferrer\\" target=\\"_blank\\">System Console</a>.</p>",


Just one comment re: change to target here, but otherwise looks good.

mattermod · 2019-10-15T14:52:34Z

@mickmister
Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
Fetching origin
+++ Updating remotes done...
!!! 'upstream/release-5.17' not found. The second argument should be something like upstream/release-0.21.
    (In particular, it needs to be a valid, existing remote branch that I can 'git checkout'.)

mickmister · 2019-10-17T14:56:17Z

/cherry-pick release-1.25

mattermod · 2019-10-17T14:56:19Z

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
Fetching origin
+++ Updating remotes done...
!!! 'upstream/release-1.25' not found. The second argument should be something like upstream/release-0.21.
    (In particular, it needs to be a valid, existing remote branch that I can 'git checkout'.)

mickmister · 2019-10-17T14:57:10Z

/cherry-pick release-5.17

mattermod · 2019-10-17T14:57:13Z

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
Fetching origin
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-#3885-upstream-release-5.17-1571324232
Branch 'automated-cherry-pick-of-#3885-upstream-release-5.17-1571324232' set up to track remote branch 'release-5.17' from 'upstream'.
+++ Downloading patch to /tmp/3885.patch (in case you need to do this again)

+++ About to attempt cherry pick of PR. To reattempt:
  $ git am -3 /tmp/3885.patch

Applying: fix basename in paths to System Console
Applying: update to use getBasePath
Applying: fix bot account i18n site url
Using index info to reconstruct a base tree...
M	components/integrations/bots/bots.jsx
Falling back to patching base and 3-way merge...
Auto-merging components/integrations/bots/bots.jsx
CONFLICT (content): Merge conflict in components/integrations/bots/bots.jsx
Patch failed at 0003 fix bot account i18n site url
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

+++ Conflicts detected:

UU components/integrations/bots/bots.jsx

+++ Aborting in-progress git am.

+++ Returning you to the master branch and cleaning up.

* Clarify bot account creation flow * fix basename in paths to System Console

* fix basename in paths to System Console * update to use getBasePath * fix bot account i18n site url * prefer siteURL over basename * update tests * use FormattedMarkdownMessage * remove unnecessary import

fix basename in paths to System Console

7514b8d

mickmister added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Oct 7, 2019

mickmister requested review from lieut-data, deanwhillier and lindalumitchell October 7, 2019 05:38

lindalumitchell approved these changes Oct 8, 2019

View reviewed changes

lindalumitchell added QA Review Done and removed 3: QA Review Requires review by a QA tester labels Oct 8, 2019

deanwhillier approved these changes Oct 8, 2019

View reviewed changes

lieut-data suggested changes Oct 8, 2019

View reviewed changes

update to use getBasePath

e593b88

mickmister requested a review from lieut-data October 8, 2019 20:50

lieut-data approved these changes Oct 9, 2019

View reviewed changes

Merge branch 'master' into MM-19193-fix-admin-console-basename

78f4877

fix bot account i18n site url

53aaf61

prefer siteURL over basename

1d5a579

mickmister requested a review from lieut-data October 10, 2019 03:42

update tests

93b3076

jfrerich added this to the v5.16.0 milestone Oct 10, 2019

jfrerich added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Oct 10, 2019

Merge branch 'master' into MM-19193-fix-admin-console-basename

e39c872

lieut-data approved these changes Oct 15, 2019

View reviewed changes

amyblais removed this from the v5.16.0 milestone Oct 15, 2019

amyblais added this to the v5.17.0 milestone Oct 15, 2019

mickmister added 2 commits October 15, 2019 10:20

use FormattedMarkdownMessage

fbd2c27

remove unnecessary import

d5b6631

mickmister added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Oct 15, 2019

mickmister merged commit b4b04d9 into master Oct 15, 2019

mickmister deleted the MM-19193-fix-admin-console-basename branch October 15, 2019 14:52

mickmister self-assigned this Oct 15, 2019

mickmister mentioned this pull request Oct 17, 2019

Cherry pick of #3885 Fix basename in paths to System Console #3992

Merged

mickmister added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Oct 17, 2019

mickmister added a commit that referenced this pull request Oct 17, 2019

Cherry pick of #3885 Fix basename in paths to System Console (#3992)

64532f1

* Clarify bot account creation flow * fix basename in paths to System Console

amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Oct 17, 2019

lindy65 added the Tests/Done Release tests have been written label Oct 21, 2019

sakshm-aurora mentioned this pull request Mar 27, 2024

[Snyk] Security upgrade katex from 0.13.2 to 0.16.10 sakshm-aurora/mattermost-webapp#65

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-19193] Fix basename in paths to System Console #3885

[MM-19193] Fix basename in paths to System Console #3885

mickmister commented Oct 7, 2019 •

edited

Loading

lindalumitchell left a comment

lieut-data left a comment

lieut-data Oct 8, 2019

lieut-data left a comment

lindalumitchell commented Oct 9, 2019 •

edited

Loading

jasonblais commented Oct 9, 2019

jfrerich commented Oct 10, 2019

mickmister commented Oct 10, 2019 •

edited

Loading

mickmister commented Oct 10, 2019

jfrerich commented Oct 10, 2019 •

edited

Loading

lieut-data Oct 15, 2019

mattermod commented Oct 15, 2019

mickmister commented Oct 17, 2019

mattermod commented Oct 17, 2019

mickmister commented Oct 17, 2019

mattermod commented Oct 17, 2019

[MM-19193] Fix basename in paths to System Console #3885

[MM-19193] Fix basename in paths to System Console #3885

Conversation

mickmister commented Oct 7, 2019 • edited Loading

Summary

Ticket Link

lindalumitchell left a comment

Choose a reason for hiding this comment

lieut-data left a comment

Choose a reason for hiding this comment

lieut-data Oct 8, 2019

Choose a reason for hiding this comment

lieut-data left a comment

Choose a reason for hiding this comment

lindalumitchell commented Oct 9, 2019 • edited Loading

jasonblais commented Oct 9, 2019

jfrerich commented Oct 10, 2019

mickmister commented Oct 10, 2019 • edited Loading

mickmister commented Oct 10, 2019

jfrerich commented Oct 10, 2019 • edited Loading

lieut-data Oct 15, 2019

Choose a reason for hiding this comment

mattermod commented Oct 15, 2019

mickmister commented Oct 17, 2019

mattermod commented Oct 17, 2019

mickmister commented Oct 17, 2019

mattermod commented Oct 17, 2019

mickmister commented Oct 7, 2019 •

edited

Loading

lindalumitchell commented Oct 9, 2019 •

edited

Loading

mickmister commented Oct 10, 2019 •

edited

Loading

jfrerich commented Oct 10, 2019 •

edited

Loading