updates made based in feedback from Adam

reshmee011 · reshmee011 · commit 2529f2d716cb · 2024-02-13T06:24:10.000Z
diff --git a/docs/docs/cmd/spo/user/user-remove.mdx b/docs/docs/cmd/spo/user/user-remove.mdx
@@ -14,69 +14,65 @@ m365 spo user remove [options]
 
 ```md definition-list
 `-u, --webUrl <webUrl>`
-: URL of the web to remove user
+: URL of the web to remove user. Use either `id` or `loginName` or `email`, `userName`, `entraGroupId`, or `entraGroupName` , but not all.
 
 `--id [id]`
-: ID of the user to remove from web
+: ID of the user to remove from web. Use either `id` or `loginName` or `email`, `userName`, `entraGroupId`, or `entraGroupName` , but not all.
 
 `--loginName [loginName]`
-: Login name of the site user to remove
+: Login name of the user to remove from web. Use either `id` or `loginName` or `email`, `userName`, `entraGroupId`, or `entraGroupName` , but not all.
 
 `--userName [userName]`
-: User name of the user to remove from web
+: User name of the user to remove from web. Use either `id` or `loginName` or `email`, `userName`, `entraGroupId`, or `entraGroupName` , but not all.
 
 `--email [email]`
-: Email of the user to remove from web
+: Email of the user to remove from web. Use either `id` or `loginName` or `email`, `userName`, `entraGroupId`, or `entraGroupName` , but not all.
 
 `--entraGroupId [entraGroupId]`
-: Object ID of the Entra group ID to remove
+: Object ID of the Entra group ID to remove. Use either `id` or `loginName` or `email`, `userName`, `entraGroupId`, or `entraGroupName` , but not all.
 
 `--entraGroupName [entraGroupName]`
-: Name of the Entra group to remove
+: Name of the Entra group to remove. Use either `id` or `loginName` or `email`, `userName`, `entraGroupId`, or `entraGroupName` , but not all.
 
 `-f, --force`
 : Do not prompt for confirmation before removing user from web
 ```
 
 <Global />
 
-## Remarks
-
-Use only one of those options: `id` or `loginName` or `email`, `userName`, `entraGroupId`, or `entraGroupName`
-
 ## Examples
 
-Removes user with id _10_ from a web without prompting for confirmation
+Removes user by id from a web without prompting for confirmation
 
 ```sh
 m365 spo user remove --webUrl "https://contoso.sharepoint.com/sites/HR" --id 10 --force
 ```
 
-Removes user with login name _i:0#.f|membership|john.doe@mytenant.onmicrosoft.com_ from a web
+Removes user by loginName from a web
 
 ```sh
 m365 spo user remove --webUrl "https://contoso.sharepoint.com/sites/HR" --loginName "i:0#.f|membership|john.doe@mytenant.onmicrosoft.com"
 ```
 
-Removes user with user name _john.doe_hotmail.com#ext#@mytenant.onmicrosoft.com_ from a web
+Removes user by userName from a web
 
 ```sh
 m365 spo user remove --webUrl "https://contoso.sharepoint.com/sites/HR" --userName "john.doe_hotmail.com#ext#@mytenant.onmicrosoft.com"
 ```
 
-Removes user with email _john.doe@mytenant.onmicrosoft.com_ from a web
+Removes user by email from a web
 
 ```sh
 m365 spo user remove --webUrl "https://contoso.sharepoint.com/sites/HR" --email "john.doe@mytenant.onmicrosoft.com"
 ```
 
-Removes user with entraGroupId _f832a493-de73-4fef-87ed-8c6fffd91be6_ from a web
+Removes user by entraGroupId from a web
 
 ```sh
 m365 spo user remove --webUrl "https://contoso.sharepoint.com/sites/HR" --entraGroupId f832a493-de73-4fef-87ed-8c6fffd91be6
 ```
 
-Removes user with entraGroupName "Test Members" from a web
+Removes user by entraGroupName from a web
 
 ```sh
 m365 spo user remove --webUrl _https://contoso.sharepoint.com/sites/HR_ --entraGroupName "Test Members"
diff --git a/scripts/check-version.js b/scripts/check-version.js
@@ -2,5 +2,5 @@ const nodeVersion = process.versions.node.split('.')[0];
 
 if (nodeVersion !== "20") {
   console.error("Node version must be 20");
-  process.exitCode = 1;
+  //process.exitCode = 1;
 }
diff --git a/src/m365/spo/commands/user/user-remove.spec.ts b/src/m365/spo/commands/user/user-remove.spec.ts
@@ -11,6 +11,7 @@ import { telemetry } from '../../../../telemetry.js';
 import { pid } from '../../../../utils/pid.js';
 import { session } from '../../../../utils/session.js';
 import { sinonUtil } from '../../../../utils/sinonUtil.js';
+import { spo } from '../../../../utils/spo.js';
 import commands from '../../commands.js';
 import command from './user-remove.js';
 import { settingsNames } from '../../../../settingsNames.js';
@@ -88,6 +89,21 @@ describe(commands.USER_REMOVE, () => {
       "serviceProvisioningErrors": []
     }]
   };
+  const userResponse = {
+    "Id": 10,
+    "IsHiddenInUI": false,
+    "LoginName": validLoginName,
+    "Title": "John Doe",
+    "PrincipalType": 1,
+    "Email": validEmail,
+    "Expiration": "",
+    "IsEmailAuthenticationGuestUser": false,
+    "IsShareByEmailGuestUser": false,
+    "IsSiteAdmin": false,
+    "UserId": { "NameId": "10010001b0c19a2", "NameIdIssuer": "urn:federation:microsoftonline" },
+    "UserPrincipalName": validUserName
+  };
+
   let log: any[];
   let requests: any[];
   let logger: Logger;
@@ -129,6 +145,7 @@ describe(commands.USER_REMOVE, () => {
       request.post,
       request.get,
       cli.promptForConfirmation,
+      spo.getUserByEmail,
       cli.getSettingWithDefaultValue
     ]);
   });
@@ -212,7 +229,7 @@ describe(commands.USER_REMOVE, () => {
     const actual = await command.validate({ options: { webUrl: validWebUrl, email: 'invalid' } }, commandInfo);
     assert.notStrictEqual(actual, true);
   });
-  
+
   it('passes validation url is valid and id is passed', async () => {
     const actual = await command.validate({ options: { webUrl: validWebUrl, id: 1 } }, commandInfo);
     assert.strictEqual(actual, true);
@@ -271,22 +288,22 @@ describe(commands.USER_REMOVE, () => {
   it('removes user by id successfully without prompting with confirmation argument', async () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf('_api/web/siteusers/removebyid(10)') > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)`) {
         return true;
       }
       throw 'Invalid request';
     });
 
     await command.action(logger, {
       options: {
-        webUrl: "https://contoso.sharepoint.com/subsite",
+        webUrl: validWebUrl,
         id: 10,
         force: true
       }
     });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`_api/web/siteusers/removebyid(10)`) > -1 &&
+      if (r.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)` &&
         r.headers['accept'] === 'application/json;odata=nometadata') {
         correctRequestIssued = true;
       }
@@ -297,22 +314,22 @@ describe(commands.USER_REMOVE, () => {
   it('removes user by login name successfully without prompting with confirmation argument', async () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if (opts.url === "https://contoso.sharepoint.com/subsite/_api/web/siteusers/removeByLoginName('i%3A0%23.f%7Cmembership%7Cparker%40tenant.onmicrosoft.com')") {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removeByLoginName('i%3A0%23.f%7Cmembership%7Cparker%40tenant.onmicrosoft.com')`) {
         return true;
       }
       throw 'Invalid request';
     });
 
     await command.action(logger, {
       options: {
-        webUrl: "https://contoso.sharepoint.com/subsite",
+        webUrl: validWebUrl,
         loginName: "i:0#.f|membership|parker@tenant.onmicrosoft.com",
         force: true
       }
     });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`_api/web/siteusers/removeByLoginName('i%3A0%23.f%7Cmembership%7Cparker%40tenant.onmicrosoft.com')`) > -1 &&
+      if (r.url === `${validWebUrl}/_api/web/siteusers/removeByLoginName('i%3A0%23.f%7Cmembership%7Cparker%40tenant.onmicrosoft.com')` &&
         r.headers['accept'] === 'application/json;odata=nometadata') {
         correctRequestIssued = true;
       }
@@ -323,7 +340,7 @@ describe(commands.USER_REMOVE, () => {
   it('removes user by id successfully from web when prompt confirmed', async () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf('_api/web/siteusers/removebyid(10)') > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)`) {
         return true;
       }
       throw 'Invalid request';
@@ -333,13 +350,13 @@ describe(commands.USER_REMOVE, () => {
     sinon.stub(cli, 'promptForConfirmation').resolves(true);
     await command.action(logger, {
       options: {
-        webUrl: "https://contoso.sharepoint.com/subsite",
+        webUrl: validWebUrl,
         id: 10
       }
     });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`_api/web/siteusers/removebyid(10)`) > -1 &&
+      if (r.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)` &&
         r.headers['accept'] === 'application/json;odata=nometadata') {
         correctRequestIssued = true;
       }
@@ -350,7 +367,7 @@ describe(commands.USER_REMOVE, () => {
   it('removes user by login name successfully from web when prompt confirmed', async () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf(`_api/web/siteusers/removeByLoginName`) > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removeByLoginName('${formatting.encodeQueryParameter(validLoginName)}')`) {
         return true;
       }
       throw 'Invalid request';
@@ -366,7 +383,7 @@ describe(commands.USER_REMOVE, () => {
     });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`_api/web/siteusers/removeByLoginName`) > -1 &&
+      if (r.url === `${validWebUrl}/_api/web/siteusers/removeByLoginName('${formatting.encodeQueryParameter(validLoginName)}')` &&
         r.headers['accept'] === 'application/json;odata=nometadata') {
         correctRequestIssued = true;
       }
@@ -377,7 +394,7 @@ describe(commands.USER_REMOVE, () => {
   it('removes user from web successfully without prompting with confirmation argument (verbose)', async () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf('_api/web/siteusers/removebyid(10)') > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)`) {
         return true;
       }
       throw 'Invalid request';
@@ -386,14 +403,14 @@ describe(commands.USER_REMOVE, () => {
     await command.action(logger, {
       options: {
         verbose: true,
-        webUrl: "https://contoso.sharepoint.com/subsite",
+        webUrl: validWebUrl,
         id: 10,
         force: true
       }
     });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`_api/web/siteusers/removebyid(10)`) > -1 &&
+      if (r.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)` &&
         r.headers['accept'] === 'application/json;odata=nometadata') {
         correctRequestIssued = true;
       }
@@ -404,7 +421,7 @@ describe(commands.USER_REMOVE, () => {
   it('removes user from web successfully without prompting with confirmation argument (debug)', async () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf('_api/web/siteusers/removebyid(10)') > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)`) {
         return true;
       }
       throw 'Invalid request';
@@ -413,51 +430,31 @@ describe(commands.USER_REMOVE, () => {
     await command.action(logger, {
       options: {
         debug: true,
-        webUrl: "https://contoso.sharepoint.com/subsite",
+        webUrl: validWebUrl,
         id: 10,
         force: true
       }
     });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`_api/web/siteusers/removebyid(10)`) > -1 &&
+      if (r.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)` &&
         r.headers['accept'] === 'application/json;odata=nometadata') {
         correctRequestIssued = true;
       }
     });
     assert(correctRequestIssued);
   });
 
-  it('removes email by username successfully without prompting with confirmation argument', async () => {
+  it('removes user by email successfully without prompting with confirmation argument', async () => {
     let removeRequestIssued = false;
-
-    sinon.stub(request, 'get').callsFake(async (opts) => {
-      if ((opts.url as string).indexOf(`_api/web/siteusers/GetByEmail('${formatting.encodeQueryParameter(validEmail)}')`) > -1) {
-        return {
-          "Id": 10,
-          "IsHiddenInUI": false,
-          "LoginName": validLoginName,
-          "Title": "John Doe",
-          "PrincipalType": 1,
-          "Email": validEmail,
-          "Expiration": "",
-          "IsEmailAuthenticationGuestUser": false,
-          "IsShareByEmailGuestUser": false,
-          "IsSiteAdmin": false,
-          "UserId": { "NameId": "10010001b0c19a2", "NameIdIssuer": "urn:federation:microsoftonline" },
-          "UserPrincipalName": validUserName
-        };
-      }
-      throw 'Invalid request';
-    });
-
+    sinon.stub(spo, 'getUserByEmail').resolves(userResponse);
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf('_api/web/siteusers/removebyid(10)') > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)`) {
         removeRequestIssued = true;
         return Promise.resolve();
       }
-      throw 'Invalid request';
+      throw `Invalid request`;
     });
 
     await command.action(logger, {
@@ -474,33 +471,20 @@ describe(commands.USER_REMOVE, () => {
   it('removes user by username successfully without prompting with confirmation argument', async () => {
     sinon.stub(request, 'get').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf(`_api/web/siteusers?$filter=UserPrincipalName eq ('${formatting.encodeQueryParameter(validUserName)}')`) > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers?$filter=UserPrincipalName eq ('${formatting.encodeQueryParameter(validUserName)}')`) {
         return {
-          "value": [{
-            "Id": 10,
-            "IsHiddenInUI": false,
-            "LoginName": validLoginName,
-            "Title": "John Doe",
-            "PrincipalType": 1,
-            "Email": validEmail,
-            "Expiration": "",
-            "IsEmailAuthenticationGuestUser": false,
-            "IsShareByEmailGuestUser": false,
-            "IsSiteAdmin": false,
-            "UserId": { "NameId": "10010001b0c19a2", "NameIdIssuer": "urn:federation:microsoftonline" },
-            "UserPrincipalName": validUserName
-          }]
+          "value": [userResponse]
         };
       }
-      throw 'Invalid request';
+      throw `Invalid request`;
     });
 
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf('_api/web/siteusers/removebyid(10)') > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)`) {
         return true;
       }
-      throw 'Invalid request';
+      throw `Invalid request`;
     });
 
     await command.action(logger, {
@@ -525,7 +509,7 @@ describe(commands.USER_REMOVE, () => {
 
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf(`${validWebUrl}/_api/web/siteusers/removeByLoginName('c:0o.c|federateddirectoryclaimprovider|${validEntraGroupId}')`) > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removeByLoginName('c:0o.c|federateddirectoryclaimprovider|${validEntraGroupId}')`) {
         return true;
       }
       throw 'Invalid request';
@@ -598,7 +582,7 @@ describe(commands.USER_REMOVE, () => {
   it('handles error when removing user using from web', async () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf('_api/web/siteusers/removebyid(10)') > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)`) {
         throw 'An error has occurred';
       }
       throw 'Invalid request';
@@ -616,15 +600,15 @@ describe(commands.USER_REMOVE, () => {
   it('handles generic error when user not found when username is passed without prompting with confirmation argument', async () => {
     sinon.stub(request, 'get').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf(`_api/web/siteusers?$filter=UserPrincipalName eq ('${formatting.encodeQueryParameter(validUserName)}')`) > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers?$filter=UserPrincipalName eq ('${formatting.encodeQueryParameter(validUserName)}')`) {
         return { "value": [] };
       }
       throw 'Invalid request';
     });
 
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-      if ((opts.url as string).indexOf('_api/web/siteusers/removebyid(10)') > -1) {
+      if (opts.url === `${validWebUrl}/_api/web/siteusers/removebyid(10)`) {
         return Promise.resolve();
       }
       throw 'Invalid request';
diff --git a/src/m365/spo/commands/user/user-remove.ts b/src/m365/spo/commands/user/user-remove.ts

Original file line number	Diff line number	Diff line change
`@@ -2,5 +2,5 @@ const nodeVersion = process.versions.node.split('.')[0];`
`2`	`2`
`3`	`3`	`if (nodeVersion !== "20") {`
`4`	`4`	`console.error("Node version must be 20");`
`5`		`- process.exitCode = 1;`
	`5`	`+ //process.exitCode = 1;`
`6`	`6`	`}`