Skip to content

[MM-67519] Remove clean path name - #3699

Open
devinbinnie wants to merge 3 commits into
masterfrom
MM-67519
Open

[MM-67519] Remove clean path name#3699
devinbinnie wants to merge 3 commits into
masterfrom
MM-67519

Conversation

@devinbinnie

@devinbinnie devinbinnie commented Feb 17, 2026

Copy link
Copy Markdown
Member

Summary

In cases where a team name is the same as the subpath name in a URL (eg. https://foo.bar/foo/foo/test, where foo is the team name and test is the channel name), the Desktop App browser history logic gets confused and will treat foo as the channel name and remove the second foo, resulting in a Team not found error.

We added this because Boards uses a different browser history module, and it does not account for the basename when setting up the navigation. However, we will be changing that over, so this PR will remove that extra path cleaning logic altogether.

Ticket Link

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

Fixed an issue where subpath servers might show a "Team not found" error in some cases

Change Impact: 🟡 Medium

Regression Risk: The change affects browser-history behavior in the Desktop App. The removed automated test reduces coverage for subpath server navigation.

QA Recommendation: Perform manual testing with subpath servers and team names that match server subpaths. Test Boards navigation and previous Boards versions.
Generated by CodeRabbitAI

@devinbinnie
devinbinnie requested review from a team and agarciamontoro and removed request for a team February 17, 2026 19:11
@devinbinnie devinbinnie added 2: Dev Review Requires review by a core committer Do Not Merge Should not be merged until this label is removed labels Feb 17, 2026
@devinbinnie devinbinnie added the Build Apps for PR Builds signed builds for testing label Feb 17, 2026
@devinbinnie

Copy link
Copy Markdown
Member Author

@Rajat-Dabade this has the sample build for testing the Boards history module

We should not merge this until we've decided when we should introduce a breaking change for previous Boards versions.

@yasserfaraazkhan yasserfaraazkhan added the E2E/Run Run Desktop E2E Tests label Apr 16, 2026
@mm-cloud-bot

Copy link
Copy Markdown

❌ E2E Test Setup Failed

Failed to create E2E test instances: failed to create installation: failed with status code 409

@mattermost-build

Copy link
Copy Markdown
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@devinbinnie

Copy link
Copy Markdown
Member Author

/update-branch

@github-actions github-actions Bot added E2E/Run Run Desktop E2E Tests and removed E2E/Run Run Desktop E2E Tests labels Aug 4, 2026
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f635ee9d-71a1-47f9-a5e0-60b166380e97

📥 Commits

Reviewing files that changed from the base of the PR and between 44fd925 and e436d73.

📒 Files selected for processing (2)
  • src/app/navigationManager.test.js
  • src/app/navigationManager.ts
💤 Files with no reviewable changes (1)
  • src/app/navigationManager.test.js

📝 Walkthrough

Walkthrough

handleBrowserHistoryPush now forwards the original browser-history pathname to the renderer. The test for server-subpath removal was deleted.

Changes

Browser-history path forwarding

Layer / File(s) Summary
Forward the original pathname
src/app/navigationManager.ts, src/app/navigationManager.test.js
handleBrowserHistoryPush no longer strips the server pathname. The test for pathname cleanup was removed.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: removing browser-history path cleaning logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch MM-67519

Comment @coderabbitai help to get the list of available commands.

@devinbinnie devinbinnie added Build Apps for PR Builds signed builds for testing and removed Build Apps for PR Builds signed builds for testing labels Aug 4, 2026
@github-actions github-actions Bot removed the E2E/Run Run Desktop E2E Tests label Aug 4, 2026
@devinbinnie

Copy link
Copy Markdown
Member Author

/update-branch

@github-actions github-actions Bot added the E2E/Run Run Desktop E2E Tests label Aug 11, 2026
@devinbinnie devinbinnie removed Lifecycle/1:stale Do Not Merge Should not be merged until this label is removed labels Aug 11, 2026
@devinbinnie devinbinnie added this to the v6.4.0 milestone Aug 11, 2026
@devinbinnie devinbinnie added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Aug 11, 2026
@github-actions github-actions Bot removed the E2E/Run Run Desktop E2E Tests label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: Reviews Complete All reviewers have approved the pull request Build Apps for PR Builds signed builds for testing release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants