When selecting a channel at the end of the setup flow, archived channels are visible in the drop down list

[DHaussermann]

trafficstars

The channel selection used to select which channel will receive the welcome message includes Archived channels.

These should be removed from the listing.

note It's unclear at this time if there is a broader solution or if each plugin must be fixed individually.

Steps:

• Archive a channel and make note of it
• Install and enable GitLab
• Run the setup command
• Configure the oAuth in the flow
• Configure the webhook or skip
• Or if available - Just run /gitlab setup announcement
• Select the option to send the welcome message Observed: Archive channel is available to select from

[hanzei]

This behavior is not GitLab specific. All plugins using interactive dialog behave this way. It would require toolkit changes to fix this.

1/5 let's make this a HW in the server repo.

[hanzei]

@catalintomai Was there a decision about if this should be a HW?

[catalintomai]

@catalintomai Was there a decision about if this should be a HW?

@hanzei - we assigned it to Demansol team. Could not assign it directly to Raju for some reason, but they are aware of it and they work on Gitlab nowadays anyway.

[sibasankarnayak]

@hanzei wanted to check before starting should this fix be in mattermost-server ?

[hanzei]

Well, this is not a bug from a framework perspective. Channel lists for interactive dialogs include archived channels. If an integration wants a filtered list, we need a new select type.

Is it really worth adding such type given that interactive dialogs is something we want to move away from?

[raghavaggarwal2308]

@hanzei @DHaussermann I think we can add a configurable boolean field (Like "include_archived_channels") for the "select" type to include/exclude archived channels. What are your opinions on this?

[hanzei]

Sounds good to me :+1:

[raghavaggarwal2308]

I think we can add a configurable boolean field (Like "include_archived_channels") for the "select" type to include/exclude archived channels. What are your opinions on this?

@hanzei @mickmister To implement this we need to make the changes in the MM server and webapp. Should we do the fix on the least supported version on Gitlab (i.e. v7.1.0) or the latest MM repo containing the code of both server and webapp?

[hanzei]

A select type would be a new feature and hence needs to go into the latest MM server version.

[mickmister]

Just to surface how this currently works, it is using the select type and dataSource: channels

@raghavaggarwal2308 @hanzei I don't think this archived channel issue is something the GitLab plugin should be concerned about. I certainly don't want to require a bump in min_server_version if we deem this a requirement.

One route would be to treat this as a "bug" in the webapp, and simply remove the option to select archived channels. This would technically be a breaking change, so maybe we should also include the include_archived_channels prop to allow for integrations to adapt, but I personally don't think it's likely that any integration wants archived channels to show in the dropdown.

I'm inclined to just avoid showing archived channels in the webapp, or close this as not planned. I don't see much risk in just removing the archived channels, even if it's technically a breaking change. We've had to make calls like this in the past, and I think this is more benign than some that we've had to do in the past.

[raghavaggarwal2308]

@mickmister Just a clarification, there are two ways to do this:

1. Handling this on the webapp side and not displaying the deleted/archived channels in the dropdown conditionally. Also, if we are going to do this, should this be done in the latest MM repo?
2. The logic to fetch the channels contains a flag includeDeleted which decides whether the archived channels will be returned in the API call or not. We can hardcode this flag to be false. If we do this when a user calls the API to fetch channels for a team, he won't get the archived channels in response.

Which one are you suggesting? Also, if we think this is not a concerning issue, we can also close it.

[mickmister]

@raghavaggarwal2308 The second option seems cleaner and more performant

Also, if we are going to do this, should this be done in the latest MM repo?

Yes we'll just introduce the change to the master branch of the mattermost repo and the task will be done after that

[raghavaggarwal2308]

@mickmister No one is assigned to review the attached PR https://github.com/mattermost/mattermost/pull/25508 with this issue. Can you please look into that?

[hanzei]

Requested two reviews, thanks for the reminder :+1:

[mickmister]

FIxed with https://github.com/mattermost/mattermost/pull/25508