Adds "spo tenant site get" command. Closes #6654 #6752

nanddeepn · 2025-06-08T10:54:32Z

Adds spo tenant site get command. Closes #6654

milanholemans · 2025-06-08T13:17:52Z

Thanks @nanddeepn, we'll try to review it ASAP!

milanholemans

This is already a nice start, let's change a few things before we do another review.

src/m365/spo/commands/tenant/tenant-site-get.ts

src/m365/spo/commands/tenant/tenant-site-get.spec.ts

docs/docs/cmd/spo/tenant/tenant-site-get.mdx

milanholemans

Thanks for addressing the previous feedback! I’ve added a few more comments on things we should still adjust. Let me know if anything’s unclear.

milanholemans · 2025-07-11T21:46:37Z

src/m365/spo/commands/tenant/tenant-site-get.ts

+
+const options = globalOptionsZod
+  .extend({
+    id: zod.alias('i', z.string())


Seems like there is an issue if you use another option than url

milanholemans · 2025-07-11T21:48:52Z

src/m365/spo/commands/tenant/tenant-site-get.ts

+
+const options = globalOptionsZod
+  .extend({
+    id: zod.alias('i', z.string())


Seems like the short options are not working properly

This is probably because you have to include all refiners logic into the alias function.

milanholemans · 2025-07-11T21:57:16Z

src/m365/spo/commands/tenant/tenant-site-get.ts

+      }
+      else if (args.options.title) {
+        const allSites: TenantSiteProperties[] = await spo.getAllSites(spoAdminUrl, logger, this.verbose, '', false, '');
+        const filterSites: TenantSiteProperties[] = allSites.filter(site => site.Title?.toLowerCase() === args.options.title?.toLowerCase());


Suggested change

const filterSites: TenantSiteProperties[] = allSites.filter(site => site.Title?.toLowerCase() === args.options.title?.toLowerCase());

const filterSites: TenantSiteProperties[] = allSites.filter(site => site.Title?.toLowerCase() === args.options.title!.toLowerCase());

milanholemans · 2025-07-11T21:57:31Z

src/m365/spo/commands/tenant/tenant-site-get.ts

+        const filterSites: TenantSiteProperties[] = allSites.filter(site => site.Title?.toLowerCase() === args.options.title?.toLowerCase());
+
+        if (filterSites.length === 0) {
+          throw `No site found with title '${args.options.title}'`;


Suggested change

throw `No site found with title '${args.options.title}'`;

throw `No site found with title '${args.options.title}'.`;

milanholemans · 2025-07-11T22:19:16Z

src/m365/spo/commands/tenant/tenant-site-get.ts

+          const resultAsKeyValuePair = formatting.convertArrayToHashTable('Url', filterSites);
+          const result = await cli.handleMultipleResultsFound<{ SiteId: string }>(`Multiple sites with title '${args.options.title}' found.`, resultAsKeyValuePair);
+
+          tenantSiteInfo = await this.getTenantSiteById(spoAdminUrl, formatting.extractCsomGuid(result.SiteId));


I noticed that the endpoint that retrieves the site by ID returns an empty site ID GUID as result. This is pretty annoying for the user of course. I suggest that we retrieve the site by URL since the result there seems more complete. Let's do the same for line 74.

milanholemans · 2025-07-21T18:15:03Z

src/utils/spo.spec.ts

+    assert.deepEqual(siteList, [
+      {
+        "_ObjectType_": "Microsoft.Online.SharePoint.TenantAdministration.SiteProperties", "_ObjectIdentity_": "487c379e-80f8-4000-80be-1d37a4995717|908bed80-a04a-4433-b4a0-883d9847d110:67753f63-bc14-4012-869e-f808a43fe023\nSiteProperties\nhttps%3a%2f%2fcontoso.sharepoint.com%2fsites%2fctest_101", "AllowDownloadingNonWebViewableFiles": false, "AllowEditing": false, "AllowSelfServiceUpgrade": true, "AverageResourceUsage": 0, "CommentsOnSitePagesDisabled": false, "CompatibilityLevel": 15, "ConditionalAccessPolicy": 0, "CurrentResourceUsage": 0, "DenyAddAndCustomizePages": 2, "DisableAppViews": 0, "DisableCompanyWideSharingLinks": 0, "DisableFlows": 0, "HasHolds": false, "LastContentModifiedDate": "\/Date(2017,11,17,4,12,28,997)\/", "Lcid": 1033, "LockIssue": null, "LockState": "Unlock", "NewUrl": "", "Owner": "", "OwnerEmail": null, "PWAEnabled": 0, "RestrictedToRegion": 3, "SandboxedCodeActivationCapability": 0, "SharingAllowedDomainList": null, "SharingBlockedDomainList": null, "SharingCapability": 1, "SharingDomainRestrictionMode": 0, "ShowPeoplePickerSuggestionsForGuestUsers": false, "SiteDefinedSharingCapability": 0, "Status": "Active", "StorageMaximumLevel": 26214400, "StorageQuotaType": null, "StorageUsage": 1, "StorageWarningLevel": 25574400, "Template": "STS#0", "TimeZoneId": 13, "Title": "Team site 101", "Url": "https:\u002f\u002fcontoso.sharepoint.com\u002fsites\u002fctest_101", "UserCodeMaximumLevel": 300, "UserCodeWarningLevel": 200, "WebsCount": 0
+      }, {
+        "_ObjectType_": "Microsoft.Online.SharePoint.TenantAdministration.SiteProperties", "_ObjectIdentity_": "487c379e-80f8-4000-80be-1d37a4995717|908bed80-a04a-4433-b4a0-883d9847d110:67753f63-bc14-4012-869e-f808a43fe023\nSiteProperties\nhttps%3a%2f%2fcontoso.sharepoint.com%2fsites%2fctest_1010", "AllowDownloadingNonWebViewableFiles": false, "AllowEditing": false, "AllowSelfServiceUpgrade": true, "AverageResourceUsage": 0, "CommentsOnSitePagesDisabled": false, "CompatibilityLevel": 15, "ConditionalAccessPolicy": 0, "CurrentResourceUsage": 0, "DenyAddAndCustomizePages": 2, "DisableAppViews": 0, "DisableCompanyWideSharingLinks": 0, "DisableFlows": 0, "HasHolds": false, "LastContentModifiedDate": "\/Date(2017,11,17,17,46,0,910)\/", "Lcid": 1033, "LockIssue": null, "LockState": "Unlock", "NewUrl": "", "Owner": "", "OwnerEmail": null, "PWAEnabled": 0, "RestrictedToRegion": 3, "SandboxedCodeActivationCapability": 0, "SharingAllowedDomainList": null, "SharingBlockedDomainList": null, "SharingCapability": 1, "SharingDomainRestrictionMode": 0, "ShowPeoplePickerSuggestionsForGuestUsers": false, "SiteDefinedSharingCapability": 0, "Status": "Active", "StorageMaximumLevel": 1048576, "StorageQuotaType": null, "StorageUsage": 1, "StorageWarningLevel": 1022361, "Template": "STS#0", "TimeZoneId": 13, "Title": "Team site 1010", "Url": "https:\u002f\u002fcontoso.sharepoint.com\u002fsites\u002fctest_1010", "UserCodeMaximumLevel": 300, "UserCodeWarningLevel": 200, "WebsCount": 0
+      }
+    ]);


Let's not check too many things in one test. Ideally, tests should test one specific thing each time. I suggest we create a separate test to check if the output is what we expect.

milanholemans · 2025-07-21T18:16:20Z

src/utils/spo.spec.ts

+    sinon.stub(spo, 'ensureFormDigest').resolves({ FormDigestValue: 'abc', FormDigestTimeoutSeconds: 1800, FormDigestExpiresAt: new Date(), WebFullUrl: 'https://contoso.sharepoint.com' });
+
+    sinon.stub(request, 'post').callsFake(async (opts) => {
+      if ((opts.url as string).indexOf(`/_vti_bin/client.svc/ProcessQuery`) > -1) {


Let's check for the entire URL instead of just checking for a small part of it.

milanholemans · 2025-07-21T18:17:38Z

src/utils/spo.spec.ts

+        if (opts.headers &&
+          opts.headers['X-RequestDigest'] &&
+          opts.headers['X-RequestDigest'] === 'abc' &&
+          opts.data === `<Request AddExpandoFieldTypeSuffix="true" SchemaVersion="15.0.0.0" LibraryVersion="16.0.0.0" ApplicationName="${config.applicationName}" xmlns="http://schemas.microsoft.com/sharepoint/clientquery/2009"><Actions><ObjectPath Id="2" ObjectPathId="1" /><ObjectPath Id="4" ObjectPathId="3" /><Query Id="5" ObjectPathId="3"><Query SelectAllProperties="true"><Properties /></Query><ChildItemQuery SelectAllProperties="true"><Properties /></ChildItemQuery></Query></Actions><ObjectPaths><Constructor Id="1" TypeId="{268004ae-ef6b-4e9b-8425-127220d84719}" /><Method Id="3" ParentId="1" Name="GetSitePropertiesFromSharePointByFilters"><Parameters><Parameter TypeId="{b92aeee2-c92c-4b67-abcc-024e471bc140}"><Property Name="Filter" Type="String"></Property><Property Name="IncludeDetail" Type="Boolean">false</Property><Property Name="IncludePersonalSite" Type="Enum">0</Property><Property Name="StartIndex" Type="String">0</Property><Property Name="Template" Type="String"></Property></Parameter></Parameters></Method></ObjectPaths></Request>`) {


I wouldn't test for headers and body. This will make the test really hard to see what actually went wrong.
Instead, I would just read out the stub and check how many times it has been invoked already.

milanholemans · 2025-07-21T18:18:05Z

src/utils/spo.spec.ts

+        if (opts.headers &&
+          opts.headers['X-RequestDigest'] &&
+          opts.headers['X-RequestDigest'] === 'abc' &&
+          opts.data === `<Request AddExpandoFieldTypeSuffix="true" SchemaVersion="15.0.0.0" LibraryVersion="16.0.0.0" ApplicationName="${config.applicationName}" xmlns="http://schemas.microsoft.com/sharepoint/clientquery/2009"><Actions><ObjectPath Id="2" ObjectPathId="1" /><ObjectPath Id="4" ObjectPathId="3" /><Query Id="5" ObjectPathId="3"><Query SelectAllProperties="true"><Properties /></Query><ChildItemQuery SelectAllProperties="true"><Properties /></ChildItemQuery></Query></Actions><ObjectPaths><Constructor Id="1" TypeId="{268004ae-ef6b-4e9b-8425-127220d84719}" /><Method Id="3" ParentId="1" Name="GetSitePropertiesFromSharePointByFilters"><Parameters><Parameter TypeId="{b92aeee2-c92c-4b67-abcc-024e471bc140}"><Property Name="Filter" Type="String">Url -like '<ctest>'</Property><Property Name="IncludeDetail" Type="Boolean">false</Property><Property Name="IncludePersonalSite" Type="Enum">1</Property><Property Name="StartIndex" Type="String">0</Property><Property Name="Template" Type="String"></Property></Parameter></Parameters></Method></ObjectPaths></Request>`) {


Also here, let's remove the header and body conditions.

milanholemans · 2025-07-21T18:19:28Z

src/utils/spo.spec.ts

+      if ((opts.url as string).indexOf(`/_vti_bin/client.svc/ProcessQuery`) > -1) {
+        if (opts.headers &&
+          opts.headers['X-RequestDigest'] &&
+          opts.headers['X-RequestDigest'] === 'abc' &&
+          opts.data === `<Request AddExpandoFieldTypeSuffix="true" SchemaVersion="15.0.0.0" LibraryVersion="16.0.0.0" ApplicationName="${config.applicationName}" xmlns="http://schemas.microsoft.com/sharepoint/clientquery/2009"><Actions><ObjectPath Id="2" ObjectPathId="1" /><ObjectPath Id="4" ObjectPathId="3" /><Query Id="5" ObjectPathId="3"><Query SelectAllProperties="true"><Properties /></Query><ChildItemQuery SelectAllProperties="true"><Properties /></ChildItemQuery></Query></Actions><ObjectPaths><Constructor Id="1" TypeId="{268004ae-ef6b-4e9b-8425-127220d84719}" /><Method Id="3" ParentId="1" Name="GetSitePropertiesFromSharePointByFilters"><Parameters><Parameter TypeId="{b92aeee2-c92c-4b67-abcc-024e471bc140}"><Property Name="Filter" Type="String">Url like 'test'</Property><Property Name="IncludeDetail" Type="Boolean">false</Property><Property Name="IncludePersonalSite" Type="Enum">0</Property><Property Name="StartIndex" Type="String">0</Property><Property Name="Template" Type="String"></Property></Parameter></Parameters></Method></ObjectPaths></Request>`) {


When validating error responses, we don't care whether the headers or body is correct. This is tested by other tests. The only thing we care about here is that we validate a clean error is shown for the user.

added command

119d8ee

milanholemans self-assigned this Jun 8, 2025

milanholemans requested changes Jun 24, 2025

View reviewed changes

milanholemans marked this pull request as draft June 24, 2025 22:20

nanddeepn added 7 commits June 29, 2025 11:53

review changes

703402c

Merge branch 'main' into issue-6654

6d2a1e2

Update tenant-site-list.ts

e6c4db7

Update tenant-site-list.spec.ts

e1257ba

Update tenant-site-list.spec.ts

c12988c

Update tenant-site-get.spec.ts

0c20857

Update tenant-site-get.spec.ts

400fd61

nanddeepn marked this pull request as ready for review June 29, 2025 13:07

milanholemans requested changes Jul 21, 2025

View reviewed changes

milanholemans marked this pull request as draft July 21, 2025 18:24

	const filterSites: TenantSiteProperties[] = allSites.filter(site => site.Title?.toLowerCase() === args.options.title?.toLowerCase());
	const filterSites: TenantSiteProperties[] = allSites.filter(site => site.Title?.toLowerCase() === args.options.title!.toLowerCase());

	throw `No site found with title '${args.options.title}'`;
	throw `No site found with title '${args.options.title}'.`;

Adds "spo tenant site get" command. Closes #6654 #6752

Are you sure you want to change the base?

Adds "spo tenant site get" command. Closes #6654 #6752

Uh oh!

Conversation

nanddeepn commented Jun 8, 2025

Uh oh!

milanholemans commented Jun 8, 2025

Uh oh!

milanholemans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

milanholemans left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!