pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
911 stars 319 forks source link

New command: `teams message remove` #5859

Closed milanholemans closed 2 months ago

milanholemans commented 7 months ago

Usage

m365 teams message remove [options]

Description

Removes a message from a channel in a Microsoft Teams team

Options

Option Description
--teamId [teamId] ID of the Microsoft Teams team. Specify either teamId or teamName but not both.
--teamName [teamName] Name of the Microsoft Teams team. Specify either teamId or teamName but not both.
--channelId [channelId] Channel ID of the Microsoft Teams team. Specify either channelId or channelName but not both.
--channelName [channelName] Channel name of the Microsoft Teams team. Specify either channelId or channelName but not both.
-i, --id <id> The ID of the Teams message.
-f, --force Don't prompt for confirmation.

Examples

Remove a message by using IDs

m365 teams message remove --teamId 5f5d7b71-1161-44d8-bcc1-3da710eb4171 --channelId 19:4a95f7d8db4c4e7fae857bcebe0623e6@thread.tacv2 --id 1540747442203

Remove a message by using display names

m365 teams message remove --teamName Marketing --channelName Branding --id 1540747442203

Default properties

No response

Additional Info

Remarks:

API: https://learn.microsoft.com/en-us/graph/api/chatmessage-softdelete?view=graph-rest-1.0&tabs=http

Adam-it commented 7 months ago

LGTM šŸ‘ let's do it šŸš€

MathijsVerbeeck commented 7 months ago

Can I work on this?

MathijsVerbeeck commented 7 months ago

@milanholemans I have noticed that we use an interface ExtendedGroup to retrieve the Team and confirm that we are trying to execute an action on a Microsoft 365 Team. However, we use the same piece of code for this across 12 commands.

Would it be handy if I put this into a util (or extend the entraGroup util)?

The code that I'm referring to is the following:

private async getTeamId(args: CommandArgs): Promise<string> {
    if (args.options.teamId) {
      return args.options.teamId;
    }

    const group = await entraGroup.getGroupByDisplayName(args.options.teamName!);
    if ((group as ExtendedGroup).resourceProvisioningOptions.indexOf('Team') === -1) {
      throw 'The specified team does not exist in the Microsoft Teams';
    }

    return group.id!;
  }

Same goes for retrieving the channel by the channelName option, so I think that a teams util could come in handy.

milanholemans commented 7 months ago

I think it would make sense to centralize ExtendedGroup instead of copy/pasting it over and over again. Good catch! About the retrieval of Teams groups and channels by name. I'm all into centralizing this as well. This enables us to throw a uniform error in multiple commands and introduce interactivity when multiple teams with the same name exist. Let's include this in a new teams util. Great suggestions šŸ‘

MathijsVerbeeck commented 7 months ago

Hi @milanholemans

Unfortunately, I have noticed that currently, we haven't got enough permissions to execute the request to the Graph API, as we require the ChannelMessage.ReadWrite permission to be able to delete a message in a channel.

Is it possible that it gets added to the PnP Application Registration? In the meantime, I will continue developing using my own application.

milanholemans commented 7 months ago

Yes, be sure to mention this in your PR so we don't forget it.

MathijsVerbeeck commented 7 months ago

@milanholemans I think for the refactoring of all the other commands, it might be handy that we create a different issue for this. Otherwise, this PR will become too big I feel.

In that issue, we could possibly also refactor all the commands so that they support both teamId and teamName, and channelId and teamName, as currently this is quite inconsistent.

Does that seem fine for you?

milanholemans commented 7 months ago

There are a bunch of commands that lack multiple options like id and displayName in Teams. We probably have to split it in multiple issues then.

MathijsVerbeeck commented 7 months ago

Makes sense. Let's start with this command, and then continue with the rest I suppose šŸ˜„