Skip to content

Commit 35c8728

Browse files
authored
feat: import Slack bot users as Mattermost bot accounts (#79)
* feat: e2e testing boilerplate * fix: allow custom image and version on e2e tests * refactor: move slack fixtures to testhelper package Prevents test utilities from being compiled and published with the services/slack package while keeping them importable by any test. * fix: address resource leaks, missing validation, and data correctness issues - Terminate orphaned containers on error in Postgres and Mattermost setup - Validate Docker Hub HTTP status before decoding response - Add json:"-" tag to SlackChannel.Type to clarify it is computed - Only set DeleteAt on user export when user is actually deleted * fix: resolve gofmt and staticcheck lint errors * rename: BasicExport -> SlackBasicExport * e2e: added e2e tests * chore: support GO_TEST_FLAGS in `make test` * fix: reset cobra flags between e2e subtests to prevent state leakage Reset all flag values and Changed state on the shared RootCmd before each Execute() call, preventing flag pollution across subtests. Also fix duplicate step numbering in test comments. * feat: import Slack bot users as Mattermost bot accounts Slack bot users are now exported as native Mattermost bot records (type "bot" with BotImportData) instead of regular user accounts. This ensures bots get proper Bot records with IsBot=true, avoids email validation issues for bots, and correctly attributes bot posts. Adds --bot-owner CLI flag (required when bots are present) to set the owner of all imported bots. Bots are skipped during Sanitise and PopulateUserMemberships. Placeholder bots created for missing bot references in posts are also exported as bot type. Includes unit tests for all new functions and e2e tests verifying bot import, bot posts, --bot-owner validation, and deleted bots. * fix: resolve variable shadowing lint error in bot e2e test * docs: regenerate CLI docs to include --bot-owner flag * fix: improve test helper robustness for cross-platform and cancellation - Set ImagePlatform to linux/amd64 for the Mattermost container so Docker pulls the correct image on ARM hosts (no arm64 variant exists) - Normalize ZIP entry paths with filepath.ToSlash for cross-platform compliance - Respect context cancellation in the import job polling loop * ci: disable coderabbit automatic reviews * fix: detect bot messages by BotId field and harden bot import validation - Change IsBotMessage() to check BotId != "" instead of relying on subtype, ensuring posts from bots are correctly routed regardless of their subtype field - Move bot message case before plain message in TransformPosts switch to prevent bot posts from being misattributed to regular users - Guard against empty BotID in TransformUsers, falling back to user ID - Trim whitespace on --bot-owner flag to reject whitespace-only values - Add defensive validation in ExportUsers to fail fast if bots exist but botOwner is empty - Add regression test for empty BotID fallback behavior - Use transformLogFile constant in e2e test cleanup - Fix GetBotByUsername docstring to reflect actual error propagation - Remove Description from bot export (no longer set) * test: add e2e test for non-existent bot owner username Mattermost's importBot silently accepts unknown owner usernames by falling back to plugin-owner semantics (storing the raw string as OwnerId). This test documents that behaviour so we notice if server-side semantics change. * ci: trigger CI run * fix: log username instead of email for imported Slack users Bot users may not have email addresses, making the previous log message unhelpful. Username is always available and more useful for identifying imported users. * fix: handle bot messages with empty BotId by falling back to User/BotUsername fields Some Slack exports contain messages with subtype "bot_message" but no BotId. Expand IsBotMessage() to also match on SubType and add a fallback chain (User → BotUsername → timestamp placeholder) so these messages are no longer silently dropped during import. * fix: correct misspelling of 'existent' in e2e test
1 parent 614cb5f commit 35c8728

File tree

13 files changed

+1175
-63
lines changed

13 files changed

+1175
-63
lines changed

.coderabbit.yaml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
11
language: en-US
22
reviews:
33
auto_review:
4-
enabled: true
5-
drafts: false
6-
base_branches:
7-
- master
4+
enabled: false

commands/transform.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func init() {
4949
TransformSlackCmd.Flags().BoolP("allow-download", "l", false, "Allows downloading the attachments for the import file")
5050
TransformSlackCmd.Flags().BoolP("discard-invalid-props", "p", false, "Skips converting posts with invalid props instead discarding the props themselves")
5151
TransformSlackCmd.Flags().Bool("debug", false, "Whether to show debug logs or not")
52+
TransformSlackCmd.Flags().String("bot-owner", "", "Username of the Mattermost user who will own all imported bots. Required if the Slack export contains bot users.")
5253

5354
TransformCmd.AddCommand(
5455
TransformSlackCmd,
@@ -71,6 +72,7 @@ func transformSlackCmdF(cmd *cobra.Command, args []string) error {
7172
allowDownload, _ := cmd.Flags().GetBool("allow-download")
7273
discardInvalidProps, _ := cmd.Flags().GetBool("discard-invalid-props")
7374
debug, _ := cmd.Flags().GetBool("debug")
75+
botOwner, _ := cmd.Flags().GetString("bot-owner")
7476

7577
// convert team name to lowercase since Mattermost expects all team names to be lowercase
7678
team = strings.ToLower(team)
@@ -140,7 +142,20 @@ func transformSlackCmdF(cmd *cobra.Command, args []string) error {
140142
return err
141143
}
142144

143-
if err = slackTransformer.Export(outputFilePath); err != nil {
145+
// Validate that --bot-owner is provided if there are bot users
146+
hasBots := false
147+
for _, user := range slackTransformer.Intermediate.UsersById {
148+
if user.IsBot {
149+
hasBots = true
150+
break
151+
}
152+
}
153+
botOwner = strings.TrimSpace(botOwner)
154+
if hasBots && botOwner == "" {
155+
return fmt.Errorf("the Slack export contains bot users but --bot-owner was not specified. Please provide the username of a Mattermost user who will own the imported bots")
156+
}
157+
158+
if err = slackTransformer.Export(outputFilePath, botOwner); err != nil {
144159
return err
145160
}
146161

0 commit comments

Comments
 (0)