Cli-microsoft365: New command: Delete Microsoft Teams team channel

[waldekmastykarz]

Deletes the specified Microsoft Teams team channel:

• command: teams channel remove -c|--channelId [channelId] -n|--channelName [channelName] -i|--teamId <teamId> --confirm
• channelId: The ID of the channel to remove. Specify channelId or channelName but not both
• channelName: The name of the channel to remove. Specify channelId or channelName but not both
• teamId: The ID of the team to which the channel to remove belongs
• confirm: Don't prompt for confirmation

[rabwill]

Hey @waldekmastykarz should we make this command accept channeName too? Incase people want to delete a channel with id or name but not both are passed.

[waldekmastykarz]

Yes! Very nice suggestion @rabwill, spec updated

[thomyg]

Would like to put this on my backlog.

[waldekmastykarz]

Awesome @thomyg!

[thomyg]

Would need some noobie help it seems @waldekmastykarz . I have the command up and running for the -c option.

If using -n I'm already able to call the graph to get a list of all the channels and to query for the right one. I have the correct channel as a Channel object in my code, but whenever I want to access channel.id I get undefined although if I log the object I see the id.

Example code:

const channelToDelete: Channel = (res.value.filter((i: any) => i.displayName === args.options.channelName));
          const channelToDeleteId: string = channelToDelete.id;
          if (this.debug) {
            cmd.log('channelName:')
            cmd.log(args.options.channelName)
            cmd.log('channelToDelete:')
            cmd.log(channelToDelete)
            cmd.log('channelToDelete.id:')
            cmd.log(channelToDelete.id)
            cmd.log('channelToDelete.id as string:')
            cmd.log(channelToDeleteId)
          }

Example output:

channelName:
DeletePls
channelToDelete:
description: null
displayName: DeletePls
email      :
id         : 19:[email protected]
webUrl     : https://teams.microsoft.com/l/channel/19%3a93ef06e61f59418088a54431ca152857%40thread.skype/DeletePls?groupId=851bce33-1196-4ca6-9358-25139ebd2975&tenantId=a1d5f937-b756-46d7-b92f-464629a6d985

channelToDelete.id:
undefined
channelToDelete.id as string:
undefined

Any tips?

[waldekmastykarz]

res.value.filter returns an array so const channelToDelete: Channel is in fact const channelToDelete: Channel[]. Because you're trying to get .id of an array, it comes back as undefined. Instead of filter you could use res.value.find which returns Channel | undefined but I wonder if there is no way for us to retrieve the right channel directly from the Graph instead of having to retrieve all of them and then filter the whole array locally.

[thomyg]

Thx Waldek for the fast response, will look for a more direct way. Will keep you posted.

[thomyg]

Thx for the hint Waldek -> https://graph.microsoft.com/v1.0/teams/851bce33-1196-4ca6-9358-25139ebd2975/channels?$filter=displayName eq 'DeletePls' does the trick. Will use that for getting the correct channel by name.

[thomyg]

@waldekmastykarz & @VelinGeorgiev the first version of a teams base class is accessible at https://github.com/thomyg/office365-cli/blob/DeleteTeamsChannel/src/o365/graph/commands/teams/teams-base.ts and I use it at the teams-channel-remove command here https://github.com/thomyg/office365-cli/blob/DeleteTeamsChannel/src/o365/graph/commands/teams/teams-channel-remove.ts

May I ask you for a short review if I'm heading in the right direction? The code is not polished but working, I'm able to delete a channel by only providing the name. If this is the way to go I would add error handling and more validation, but I just want to make sure the effort is done in the right direction.

Second more important question: if we implement such a base class, wouldn't it be necessary to move more graph calls there? When I think of the future feature of "export team as json" I will need to call all the different endpoints to get the team definition. If all of those calls would be in the base class we only need them once and the commands can share them, otherwise, we end up with code duplication, correct?

[waldekmastykarz]

1) The code looks good so far. One thing I'd suggest changing is to replace <Object> with <string> (I assume channelId is a string)

2) Export team is a unique case as it doesn't have a corresponding API and basically means we need to manually extract all different artifacts ourselves. Something's telling me (not based on any inside knowledge, but just a hunch) that at one point such API will be available so I wouldn't mind a temporary code duplication. The problem with centralizing code is that if one of the commands will need something specific, we'll need to generalize the central method and potentially refactor all other commands which would lead to unnecessary effort.

[waldekmastykarz]

Hey @thomyg, are you still working on it?

[waldekmastykarz]

Setting as available to pick up due to lack of response

[garrytrinder]

@waldekmastykarz I'd like to pick this up as my first contribution

[VelinGeorgiev]

All yours Garry (@garrytrinder), thanks for helping !

[garrytrinder]

Just a quick note to say that I am slowly making progress on this command and hope to have some code to review soon enough.

[garrytrinder]

Hey @waldekmastykarz & @VelinGeorgiev

I've implemented this command, see https://github.com/pnp/office365-cli/compare/dev...garrytrinder:teams-channel-remove but I have left out the --channelName parameter for now as I think it might require discussion.

As I've been learning by examples to create this command (first contribution), I've been making sure I stick to existing implementations for consistency, however I noticed that --channelName isn't used on any other command as an optional required parameter from what I can see, e.g. the teams team remove command only takes teamId and not teamName.

If there is an example already in the CLI that I have overlooked that would point me in the right direction, please let me know.

[waldekmastykarz]

It seems like we could work a bit on consistency: channel get expects the id, but channel set the name. Ideally, we'd support both, so if one or the other are missing in some commands, we should add separate issues to close these gaps.

[garrytrinder]

I'd say that passing the id should be the base level of implementation as this also fits the underlying Graph API call /teams/{id}/channels/{id} also keeps the implementation simple.

How would you like to proceed? I can push what I currently have without --channelName as its ready and tested, then raise an enhancement for --channelName and raise issues against the others for obtaining consistency?

[waldekmastykarz]

Sounds good to me. 👍

Keeping the implementation simple is one thing. What I find more important is for us to make using the CLI convenient. If I want to delete a channel, I likely know its name but not the ID. If we can simplify the usage by automatically translating the name to ID, then we should definitely do it. In the end we should support both use cases and starting with ID and extending it later to support name is a good idea 👍

[garrytrinder]

Great, I’ll get the PR raised later today 😁

_{Sent with GitHawk}