MM-70095: Document Slack import account matching behavior - #9123
MM-70095: Document Slack import account matching behavior#9123edgarbellot wants to merge 5 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Slack migration guide documents email-based account matching. Administrator imports reuse matching Mattermost accounts. Non-administrator imports exclude matching Slack users. The guide updates placeholder-email and account activation guidance. ChangesSlack account matching
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/administration-guide/onboard/migrate-from-slack.rst`:
- Around line 353-361: Update the surrounding Slack import guidance to
distinguish administrator imports that reuse a matching Mattermost account from
imports that create new accounts. Revise the account-creation statement near the
referenced earlier section and the activation/password-reset guidance near the
later section so they apply only to newly created accounts, while preserving the
separate non-administrator behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0f4e326-a2b1-4812-8f1c-e806804e55e2
📒 Files selected for processing (1)
source/administration-guide/onboard/migrate-from-slack.rst
|
Newest code from mattermost has been published to preview environment for Git SHA 3830b6c |
|
Newest code from mattermost has been published to preview environment for Git SHA d171f7d |
|
@edgarbellot can you please assign a dev reviewer and also add the corresponding server release milestone? Thanks! cc// @Combs7th |
|
@carlisgg adding you since you reviewed the corresponding PR in the monorepo |
|
Newest code from mattermost has been published to preview environment for Git SHA 91f05d3 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
source/administration-guide/onboard/migrate-from-slack.rst (2)
355-355: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the
authenticatationtypo.Change
authenticatationtoauthenticationin the cross-reference label.As per coding guidelines, documentation must flag spelling and terminology errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/administration-guide/onboard/migrate-from-slack.rst` at line 355, Correct the misspelled “authenticatation” term in the cross-reference label for the Slack user migration instructions to “authentication,” leaving the surrounding documentation and reference target unchanged.Source: Coding guidelines
331-331: 🗄️ Data Integrity & Integration | 🟠 MajorQualify account creation and activation by match status.
The sentence “During the import process, the emails and usernames from Slack are used to create new Mattermost accounts” applies to every user. This conflicts with the matching rules at Lines 343-350.
The sentence “Slack users activate their new Mattermost accounts” also applies to matched administrator accounts and excluded non-administrator users.
Update both statements so they apply only when no existing Mattermost account matches.
Suggested wording
-During the import process, the emails and usernames from Slack are used to create new Mattermost accounts. +During the import process, the emails and usernames from Slack are used to create new Mattermost accounts when no existing Mattermost account matches. -* Slack users activate their new Mattermost accounts by using Mattermost's **Password Reset** screen with their email addresses from Slack to set new passwords for their Mattermost accounts. +* Slack users whose imports create new Mattermost accounts activate those accounts by using Mattermost's **Password Reset** screen with their email addresses from Slack to set new passwords.This repeats the previously reported account-creation and activation inconsistency. As per coding guidelines, documentation must describe feature behaviour accurately and provide corrected wording for inaccurate text.
Also applies to: 355-355
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/administration-guide/onboard/migrate-from-slack.rst` at line 331, Revise the account-creation statement and the Slack-user activation statement in the migration guide so they apply only when no existing Mattermost account matches. Keep the documented matching behavior for matched administrator accounts and excluded non-administrator users accurate, and update both occurrences identified near the account-creation and activation guidance.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@source/administration-guide/onboard/migrate-from-slack.rst`:
- Line 355: Correct the misspelled “authenticatation” term in the
cross-reference label for the Slack user migration instructions to
“authentication,” leaving the surrounding documentation and reference target
unchanged.
- Line 331: Revise the account-creation statement and the Slack-user activation
statement in the migration guide so they apply only when no existing Mattermost
account matches. Keep the documented matching behavior for matched administrator
accounts and excluded non-administrator users accurate, and update both
occurrences identified near the account-creation and activation guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bdbeafdf-0a08-4388-8b2e-57f551206b56
📒 Files selected for processing (1)
source/administration-guide/onboard/migrate-from-slack.rst
|
Newest code from mattermost has been published to preview environment for Git SHA 9850e71 |
|
@coderabbitai Both outside-diff points are addressed in 6e626f2:
|
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Newest code from mattermost has been published to preview environment for Git SHA 6e626f2 |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
See MM-70095.
PR: mattermost/mattermost#37818