Add sleep between calls to add user to channels #124
Conversation
server/welcomebot.go
Outdated
| @@ -113,6 +113,7 @@ func (p *Plugin) renderWelcomeMessage(messageTemplate MessageTemplate, configMes | |||
|
|
|||
| for _, channelName := range configAction.ChannelsAddedTo { | |||
| p.joinChannel(action, channelName) | |||
| time.Sleep(time.Second) | |||
There was a problem hiding this comment.
@hanzei 0/5 on if this should be done this way. Lots of cons come with this:
- Channel actions may not be used on a server, so this sleep would be for naught
- The sleep may be unnecessarily long, especially when we're adding the user to a lot of channels
- It may not solve the problem in every case
There was a problem hiding this comment.
I'm in favour of doing it this way, as it should fix the issue out-of-the-box. A config setting seems cumbersome and confusing for admins.
This code only runs once per user, i.e. on their creation. Having a delay is justifiable. Even with 60 channels (!), that is just one minute of wait time.
There was a problem hiding this comment.
Though I wonder if the user is pinged with a "ding" every second. I haven't actually seen this in action yet
There was a problem hiding this comment.
Dear Michael @mickmister and Ben @hanzei, thanks for providing this fix and your thoughts. I agree with the concerns, though. This wait-time as the only measure is rather unreliable. You can never guess an optimal wait-time for all cases. Sometimes you bother users with unexpected long wait-times, sometimes you bother them with duplicate categories.
Suggestions:
-
Is there a way to determine if a category needs to be created, but creation has not yet been triggered (this might be the tricky part), check if the category already exists, wait-retry if is it not yet created but triggered, add member if it is already created?
-
Is there a way to wait only if a category assignment is configured in the channel actions?
What do you think about these suggestions?
That's of course more challenging to implement, but you might have an idea how to achieve that. Thank you very much for taking these considerations into account!
There was a problem hiding this comment.
@mickmister Are you looking into this or we should?
There was a problem hiding this comment.
@hanzei Note that there is already a property on the WelcomeMessage struct called DelayInSeconds, which allows the admin to set a delay between certain operations, so introducing another part of the config with this pattern would not be out of the ordinary. 0/5 but in this case it may make sense to allow the admin to set this, so we don't have to revisit it if the wait we choose is not high enough.
This reverts commit af24fe5.
Summary
There is an issue occurring with the Welcomebot and the Playbooks plugin's Channel Actions "automatically add to channel category" feature, where the user is inadvertently having duplicate channel categories created with the same name. We believe is due to a race condition of adding the user to multiple channels semi-simultaneously. More info in the tickets linked below.
This PR adds a sleep in between adding a user to multiple channels, to avoid this situation. A more holistic solution of handling this issue in the core product is discussed in the Jira ticket below, though this PR can resolve this before the core issue is addressed.
Ticket Link
Fixes #120
https://mattermost.atlassian.net/browse/MM-54003