Skip to content

Fixes 'spo eventreceiver remove' command#4034

Closed
milanholemans wants to merge 1 commit intopnp:mainfrom
milanholemans:sporerremove-4033
Closed

Fixes 'spo eventreceiver remove' command#4034
milanholemans wants to merge 1 commit intopnp:mainfrom
milanholemans:sporerremove-4033

Conversation

@milanholemans
Copy link
Copy Markdown
Contributor

@milanholemans milanholemans commented Nov 10, 2022

Fixes 'spo eventreceiver remove' command. Closes #4033

Remarks

Wrong endpoints were called. Reworked the entire command.

@milanholemans milanholemans mentioned this pull request Nov 10, 2022
Closed
@Adam-it Adam-it self-assigned this Nov 11, 2022
Adam-it
Adam-it approved these changes Nov 11, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.
will do some local testing before I merge 👍

@milanholemans
Copy link
Copy Markdown
Contributor Author

milanholemans commented Nov 12, 2022 •
edited
Loading

@Adam-it one thing I forgot to update, I updated the telemetry scope option, removed the anonymous value since it's a choice field. However, scope is an optional option so right now it's possible that undefined is logged in the telemetry.
I'm unable to fix it myself right now since I'm on mobile, maybe you can do it before you merge it? 🙂

@Adam-it
Copy link
Copy Markdown
Member

Adam-it commented Nov 12, 2022

@Adam-it one thing I forgot to update, I updated the telemetry scope option, removed the anonymous value since it's a choice field. However, scope is an optional option so right now it's possible that undefined is logged in the telemetry. I'm unable to fix it myself right now since I'm on mobile, maybe you can do it before you merge it? 🙂

sure

Adam-it
Adam-it reviewed Nov 12, 2022
View reviewed changes
Comment thread src/m365/spo/commands/eventreceiver/eventreceiver-remove.ts
listTitle: typeof args.options.listTitle !== 'undefined',
listUrl: typeof args.options.listUrl !== 'undefined',
scope: typeof args.options.scope !== 'undefined',
scope: args.options.scope,
Copy link
Copy Markdown
Member

@Adam-it Adam-it Nov 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
scope: args.options.scope,
scope: args.options.scope || 'web',

@milanholemans when not defined I will log the web scope as this seems to be the default one when this is left undefined.

Copy link
Copy Markdown
Contributor Author

@milanholemans milanholemans Nov 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I check the docs, web should be the default value.

The scope. Can be either "site" or "web". Defaults to "web". Only applicable when not specifying any of the list properties.

Thank you for fixing this again ❤️
I totally forgot to fix it 😁

Copy link
Copy Markdown
Member

@Adam-it Adam-it Nov 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry... web... totally got that the other way around looking at the old conditions before your change 😜.
no problem 👍

@Adam-it
Copy link
Copy Markdown
Member

Adam-it commented Nov 12, 2022

added fix up
merged manually
thanks @milanholemans for your awesome work 👍
your rock 🤩

@Adam-it Adam-it closed this Nov 12, 2022
@milanholemans milanholemans deleted the sporerremove-4033 branch November 12, 2022 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Adam-it Adam-it Adam-it approved these changes

Assignees

@Adam-it Adam-it

Labels

pr-bugfix pr-merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug report: Does Not Support HTTP DELETE Error on spo eventreceiver remove

2 participants

@milanholemans @Adam-it