Cli-microsoft365: Enhancement: extend 'spo site remove' to support groupified sites

[waldekmastykarz]

Extend the spo site remove command to support groupified sites.

• Start with determining the type of site
• If the site is not groupified, used. the existing removal logic. If the site is groupified, use the Graph site delete API
• When the user specified theskipRecycleBin or fromRecycleBin options with a groupified site, we should show a warning that these options don't apply, after determining the type of site

Big thanks to @arjunumenon for research for the spec

[ktskumar]

Hi Waldek, May I take this!

[waldekmastykarz]

Please do! Thank you! 👏

[garrytrinder]

Hey @ktskumar 👋🏻

Are you still working on this command?

Please don't hesitate to reach out if you need some assistance.

[waldekmastykarz]

Ping @ktskumar, just checking if you're still working on this

[waldekmastykarz]

Settings as available for picking due to lack of response

[arjunumenon]

Hey @waldekmastykarz - Do you mind if I take this enhancement up? I have couple of PR's in pipeline and hence I can squeeze myself for working on this if you are fine with this.

[waldekmastykarz]

Appreciate your help @arjunumenon 👏

[arjunumenon]

Hello @waldekmastykarz - Before I go ahead with the implementation, I have couple of questions.

1. We already have a command spo site remove which currently supports the deletion of all the sites except the O365 Group connected site. I am assuming we would be enhancing the same command with the support for deletion of site connected to O365 groups as well. Is that correct?
   _Side Note: As per the issue #1494 remove commands for classic and modern were merged_
   If my understanding is correct, do you mind updating the specs please?

2. Assuming we are going ahead with the above approach, we would be using Graph API Delete for the operation, is that correct?

It will be great if you can confirm the direction so that I can go ahead with the implementation.

[waldekmastykarz]

1. You're right. Good catch and sorry for the confusion
2. If that's the API to do the job, let's use it 👍

[arjunumenon]

Thanks @waldekmastykarz . No need to be sorry it all..
This is the least we can do from our side considering the major chunk which you already doing from your side. 💪💪

[arjunumenon]

Hello @waldekmastykarz - Just want to confirm on the implementation design before I go ahead with development. It will be great if you can confirm that so that I can go ahead with the implementation.

Implementation Plan

• Before we go ahead with the removal REST Call implemented currently, we would be initiating a call to get the GroupIdfield using client.svc which will fetch the needed field called GroupId for the entered site

• Then we will check whether the GroupIdis valid?

• Groupid would be valid if the returned GroupIdIS NOT EQUAL TO 00000000-0000-0000-0000-000000000000

• If it is valid GroupId, we will be initiating the deletion of the Group using Graph API Delete.

• If the NOT valid Groupid, currently implemented process will be continued.

Existing Parameters

Another point to be noted here is that, If the entered site is groupified, operation will not respect the available parameters like, skipRecycleBin, fromRecycleBin.

1. Do you want this to be given as instructions in the Remarks section.
2. Or do you want to validate the existence of the valid guid and return false validation if those parameters are entered during the validate() method?

If you agree to the above assumption, do you think it make sense to update current specs with these information. Especially over here, we are going ahead with enhancement of the current command rather than a new command.

Hopefully this would I hope this will be by last set of questions before I am done with the implementation 🤞

[waldekmastykarz]

Implementation plan is 👍

As for the parameters, ideally, we'd do it during validation, but at that stage we can't execute any requests, because auth has not set up yet. So for now, let's do it as a pre-check after determining the type of site. Also, like you mentioned, let's include it in the remarks so that users are aware of it.

I've update the spec like you suggested.

I'm starting to sound like a broken record, but I really appreciate your thoroughness 👏

[arjunumenon]

Thanks @waldekmastykarz for the direction and the updates. Now everything is definitely on place.

ideally, we'd do it during validation, but at that stage we can't execute any requests, because auth has not set up yet.

I had not realized that part. Thanks for highlighting that part which I hadn't thought of...

I'm starting to sound like a broken record,

Not at all. Music which record plays is always amazing.. 👏
All I need is your high level guidance which is golden always. I will be more than happy to fill in the details for you.

[waldekmastykarz]

Pleasure to be working with you, sir 👏

[arjunumenon]

Pleasure to be working with you, sir 👏

💓. Like always, you are being humble. Pleasure is definitely mine... 🕺

[arjunumenon]

Hello @waldekmastykarz - Got couple of questions before I initiate the Pull Request _(though I had told there wouldn't be any more questions 😑. Couldn't keep up that word_.

WaitParameter

There is a parameter called wait which also is not relevant to the Group deletion since we use Graph API group deletion which does not follow the conventional site deletion API which we are using. Adding to that, as per my understanding even if the Office 365 Group is deleted, respective SharePoint site will be deleted later via timer jobs behind the scenes. Henceforth, the timeout feature which we use will nevertheless be relevant.
Can we give the similar instructions if we the parameter is entered for the Groupified site

Existing code refactored

I have also have done refactoring for the existing code by creating a new method so that code would be organized. It will be great if you can let us know whether it is not recommended during your review so that I can revert it back if that approach is not recommended.

Thanks again for patiently responding to all my questions.

[waldekmastykarz]

You're absolutely right about wait. If it doesn't work in this case, it makes perfect sense to let the user know about it upfront in the docs.

Re refactoring: if it simplifies the code and maintenance, let's do it. Let's see what you have in mind and take it from there 👍