Enhances spo site set with ability to adding owner without having permissions, Closes #4600

[nicodecleyre]

Closes #4600

[milanholemans]

Thank you @nicodecleyre! We'll review it ASAP!

[martinlingstuyl]

Okay, very nicely done! Let's fix some things before we merge..

[martinlingstuyl]

Question @nicodecleyre: how does this perform in an environment with many sites? I'm asking myself if we should call the admin endpoint all the time or only if the regular endpoint fails.

[nicodecleyre]

Question @nicodecleyre: how does this perform in an environment with many sites? I'm asking myself if we should call the admin endpoint all the time or only if the regular endpoint fails.

Don't know... I will take a look if i can test it on a tenant with 5000+ sites...

[martinlingstuyl]

Hi @nicodecleyre,whats the status of this PR? Is it ready for review?

[nicodecleyre]

Hi @nicodecleyre,whats the status of this PR? Is it ready for review?

Not yet, gonna test it with a tenant with 5000 sites next week

[nicodecleyre]

Test looking good! (added some extra logging for this test) Let me know if you want me to test anything else

[martinlingstuyl]

Test looking good! (added some extra logging for this test) Let me know if you want me to test anything else

How about speed of execution @nicodecleyre? How big was the tenant you tested this on and did you notice performance differences between the old and new way?

[martinlingstuyl]

@nicodecleyre, did you have an answer on my question already?

[nicodecleyre]

@nicodecleyre, did you have an answer on my question already?

sorry about that! Tested it on a tenant with 5050 site collections.
I don't notice any change in speed. Just the fact that the command works with the enhancement as seen below (see 2nd output, first output is as is)

[martinlingstuyl]

Hi @nicodecleyre,
It's been a while since I've responded to this one.
Reason is I've been thinking about the setup.

Retrieving all tenant sites using the new endpoint has a few drawbacks: You appear to cannot retrieve OneDrive sites like we do with the other endpoint, plus its interface is different from the CSOM request we are normally doing.
And the siteId cannot be found on other endpoints. It's not ideal.

So I thought: why are we even retrieving the siteId. And it turns out we only use it in one place. On the UpdateGroupPropertiesBySiteId function. Well, it appears there's an alias function that does not need the siteId: UpdateGroupProperties.

The following request updates the site title just as well:

POST https://contoso-admin.sharepoint.com/_api/SPOGroup/UpdateGroupProperties
{
  groupId: this.groupId,
  displayName: args.options.title
}

So I suggest we change it up a bit:

• Use the getSiteGroupId function in site-remove.ts to get the group ID through the admin endpoint
• Update to using the UpdateGroupProperties function.

Together this should be enough, and we can remove the getTenantSites util and implementation entirely. The change will be quite small.

Could you update the PR with this implementation? Thanks again for all the work!

[martinlingstuyl]

Hi @nicodecleyre, how r u doing on this? Just checking 😀

[nicodecleyre]

Hi @martinlingstuyl,

Did the changes as asked. If i'm not mistaken we still have to use getTenantSites to get the groupId. Or do I see this wrong and do you want to revert this to the usage of /_api/site?$select=GroupId?

[martinlingstuyl]

Hi @nicodecleyre,
Nice, you've been busy!

About your question: You're right, we still need the groupId. but I'd like us to get it using the list on the admin site: DO_NOT_DELETE_SPLIST_TENANTADMIN_AGGREGATED_SITECOLLECTIONS, checkout hubsite get for example.

This seems to me more straightforward than using RenderAdminListData after all. For one we can use an odata $filter `SiteUrl eq '', and also we can reuse code.

Now you've been building this util in another PR. I suggest we wait a bit for a conclusion on the discussion there as to how we're going to handle the util PR's.

[martinlingstuyl]

Waiting on the conclusion here

[mkm17]

Hi @nicodecleyre @martinlingstuyl , I see this long discussion here, and I was wondering if the issue mentioned in the topic of this PR has already been resolved with the spo site admin add --asAdmin --primary command.

I noticed there were some doubts about using the admin endpoint (the https://contoso-admin.sharepoint.com/_api/SPO.Tenant/ endpoint is used in site admin add). Is there something I should reconsider in this command?

[martinlingstuyl]

Hi @mkm17, That command does make this PR less urgent, yes...

I don't think I had doubts about using admin endpoints, but more about what endpoint to use so that it covers all types of sites (onedrives, regular sites, group sites) and where the group Id might be found if needed.

[mkm17]

@martinlingstuyl ok, thank you for confirmation.