New command: spo eventreceiver remove

[MathijsVerbeeck]

Usage

spo eventreceiver remove [options]

Description

This command will remove an event receiver from a list or web

Options

Option	Description
-u, --webUrl <webUrl>	The URL of the web of which to remove the event receiver.
--listTitle [listTitle]	The title of the list of which to remove the event receiver, if the event receiver should be removed from a list. Specify either listTitle, listId or listUrl.
--listId [listId]	The id of the list for which to remove the event receivers, if the event receiver should be removed from a list. Specify either listTitle, listId or listUrl.
--listUrl [listUrl]	The url of the list for which to remove the event receiver, if the event receiver should be removed from a list. Specify either listTitle, listId or listUrl.
-n, --name [name]	The name of the event receiver to remove. Specify --name or --id but not both.
-i, --id [id]	The id of the event receiver to remove. Specify --name or --id but not both.
--scope [scope]	The scope on which to retrieve the Event Receiver. Can be either "Site" or "Web". Defaults to "Web". Only applicable when not specifying any of the list properties
--confirm	Don't prompt for confirmation

Additional Info

If the user refers to the event receiver by name and the API returns multiple matches, the command prints a list of found event receivers and their IDs and asks the user to select the event receiver by ID.

[martinlingstuyl]

Thanks for the suggestion @MathijsVerbeeck. 🚀 I like the idea.

Please update the specs so it's clear that the optional list options are mutually exclusive. Also, please rename it to spo eventreceiver remove to align it with the verbs we use. And please add a ---confirm option for suppressing the confirmation prompt.

Any @pnp/cli-for-microsoft-365-maintainers with ideas about this?

[MathijsVerbeeck]

@martinlingstuyl Thanks for the feedback. I've updated the options.

[martinlingstuyl]

Oh, and What about adding a --scope option? (Site/Web) (on all 4 eventreceiver commands)

[MathijsVerbeeck]

@martinlingstuyl As for as I know, a remote event receiver can only be set on Web level (/_api/web('webUrl')/eventreceivers) and not on Site level. Obviously, it can also be set on a list level. (_api/web('webUrl')/lists('listId')/eventReceivers)

[Adam-it]

@MathijsVerbeeck thanks for this great suggestion 🤩 similar to eventreceiver add issue here I was also thinking we may split this into two commands (one for web other for list) like:

• spo web eventreceiver remove
• spo list eventreceiver remove That way we may avoid resolving the 'scope problem' in the single command. The command name is self descriptive and shows at what level (scope) we plan to add the event receiver. The naming will follow our standard. Let me know what do you think? Have a great day 👍

[martinlingstuyl]

You do have Site / Web scope @MathijsVerbeeck. Check this link

This scope is (for example) relevant for event receiver types like SiteDeleting. (See this full list)

update: ...so either we'd need to determine the scope within the CLI, based on the event types, or we'd need it as an option. I prefer the latter one. This is much clearer for users. PnP.PowerShell does it like that as well by the way, see here.

[martinlingstuyl]

I like the idea @Adam-it. Don't you think though that that would 'spread' the eventreceiver commands too much? Bundling them would be better for usability in my opinion (for example when reading the docs.)

update: I see you're comparing it to the roleinheritance commands in issue #3306. 😉 I do think this is something different here, as an eventreceiver is actually an entity in sharepoint, whereas roleinheritance is more of like a functionality of existing entities. I still think we need to keep scope as an option 🤣 but let's hear what other @pnp/cli-for-microsoft-365-maintainers have to say about it...

[MathijsVerbeeck]

@Adam-it Thanks for the feedback. I liked to keep this "compact", because the code would basically be the same accross the 2 commands, except that we would get a list on which we would add the Event Receivers, that's why I kept it in one command.

@martinlingstuyl I was unaware of that. I will change the options and add a "--scope" parameter. Thanks for enlightening me about this.

[Adam-it]

@martinlingstuyl @MathijsVerbeeck I totally agree my approach would 'spread' the code and in some cases would lead to code duplications 😉. In the end I think how we code is one thing (important for sure) but what's most important is how it will be easier/nicer/better for the user that just uses CLI commands (not actually cares about the code behind it). TBH I am also not sure what is better for the user in this case😋:

• to have separate self descriptive commands for each scope🤔
• or have one well documented command which has more functionality and parameters 🤔 ... I see pros and cons in both cases. Let's wait for more comments on this command and make a decision later on 👍 anyway thanks for the group investigation/analysis on this issue 🤩 you rock 👍

[waldekmastykarz]

I'd opt for not splitting into separate commands and sticking to the current spec. See my comment for more info

[waldekmastykarz]

In #3308 we allow retrieving event receiver by its name or id. Shouldn't we allow deleting it by name as well, rather than just by id?

[MathijsVerbeeck]

@waldekmastykarz I could add that option. However, I do believe that it is possible to have multiple event receivers with the same name, but a different event type. So we should return an error if multiple matches are found.

[martinlingstuyl]

That's fine 👍, if you find multiple event receivers, the user can be asked to choose one.

[martinlingstuyl]

I've updated it @MathijsVerbeeck. Let's open it up for contribution.

[waldekmastykarz]

@waldekmastykarz I could add that option. However, I do believe that it is possible to have multiple event receivers with the same name, but a different event type. So we should return an error if multiple matches are found.

Good catch! I've added this to the spec

[MathijsVerbeeck]

I could work on this one.

[Adam-it]

Thanks @MathijsVerbeeck for all of your awesome work 👍. I assigned you to the issue. Can't wait to see the PR 😍. You rock 🤩

[martinlingstuyl]

Hi @MathijsVerbeeck, are you still working on this? Let us know if you need any help!

[MathijsVerbeeck]

@martinlingstuyl I've written all the code. I only have to add the code that would handle multiple event receivers with the same name (the selection part). I'll try and finish this this week.

[milanholemans]

@martinlingstuyl I've written all the code. I only have to add the code that would handle multiple event receivers with the same name (the selection part). I'll try and finish this this week.

In that case we should just throw an error like we do in other commands. Example: https://github.com/pnp/cli-microsoft365/blob/f956c5da7fec2c629abd18947a88f6bd149f9974/src/utils/planner.ts#L50-L56