Updated as per review comments

pnp · Oct 16, 2024 · 2fbd5a3 · 2fbd5a3
1 parent 4fe9dde
commit 2fbd5a3
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 64 deletions.
diff --git a/docs/docs/cmd/spo/list/list-remove.mdx b/docs/docs/cmd/spo/list/list-remove.mdx
@@ -23,7 +23,7 @@ m365 spo list remove [options]
 : Title of the list to remove. Specify either `id` or `title` but not both.
 
 `--recycle`
-: Recycle the list
+: Instead of permanently deleting, send the list to the recycle bin.
 
 `-f, --force`
 : Don't prompt for confirming removing the list.
@@ -45,7 +45,7 @@ Remove the list with a specific title located in a specific site.
 m365 spo list remove --webUrl https://contoso.sharepoint.com/sites/project-x --title 'List 1'
 ```
 
-Remove the list with a specific ID located in a specific site and send it to recycle bin
+Remove a list specified by id by sending it to the recycle bin instead of permanently removing it
 
 ```sh
 m365 spo list remove --webUrl https://contoso.sharepoint.com/sites/project-x --id 0cd891ef-afce-4e55-b836-fce03286cccf --recycle

diff --git a/src/m365/spo/commands/list/list-remove.spec.ts b/src/m365/spo/commands/list/list-remove.spec.ts
@@ -15,6 +15,10 @@ import command from './list-remove.js';
 import { settingsNames } from '../../../../settingsNames.js';
 
 describe(commands.LIST_REMOVE, () => {
+  const listId = 'b2307a39-e878-458b-bc90-03bc578531d6';
+  const webUrl = 'https://contoso.sharepoint.com';
+  const listTitle = 'Documents';
+
   let log: any[];
   let logger: Logger;
   let commandInfo: CommandInfo;
@@ -44,9 +48,10 @@ describe(commands.LIST_REMOVE, () => {
       }
     };
     requests = [];
-    sinon.stub(cli, 'promptForConfirmation').callsFake(() => {
+    sinon.stub(cli, 'promptForConfirmation').callsFake(async () => {
       promptIssued = true;
-      return Promise.resolve(false);
+      //return Promise.resolve(false);
+      return false;
     });
     promptIssued = false;
   });
@@ -95,14 +100,10 @@ describe(commands.LIST_REMOVE, () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
 
-      if ((opts.url as string).indexOf(`/_api/web/lists(guid'`) > -1) {
-        if (opts.headers &&
-          opts.headers.accept &&
-          (opts.headers.accept as string).indexOf('application/json') === 0) {
-          return;
-        }
-      }
 
+      if (opts.url === `${webUrl}/_api/web/lists(guid'${listId}')`) {
+        return;
+      }
       throw 'Invalid request';
     });
 
@@ -111,9 +112,7 @@ describe(commands.LIST_REMOVE, () => {
     await command.action(logger, { options: { id: 'b2307a39-e878-458b-bc90-03bc578531d6', webUrl: 'https://contoso.sharepoint.com' } });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`/_api/web/lists(guid'`) > -1 &&
-        r.headers.accept &&
-        r.headers.accept.indexOf('application/json') === 0) {
+      if (r.url === `${webUrl}/_api/web/lists(guid'${listId}')`) {
         correctRequestIssued = true;
       }
     });
@@ -123,13 +122,8 @@ describe(commands.LIST_REMOVE, () => {
   it('removes the list when prompt confirmed by option', async () => {
     sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-
-      if ((opts.url as string).indexOf(`/_api/web/lists(guid'`) > -1) {
-        if (opts.headers &&
-          opts.headers.accept &&
-          (opts.headers.accept as string).indexOf('application/json') === 0) {
-          return;
-        }
+      if (opts.url === `${webUrl}/_api/web/lists(guid'${listId}')`) {
+        return;
       }
 
       throw 'Invalid request';
@@ -138,64 +132,41 @@ describe(commands.LIST_REMOVE, () => {
     await command.action(logger, { options: { id: 'b2307a39-e878-458b-bc90-03bc578531d6', webUrl: 'https://contoso.sharepoint.com', force: true } });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`/_api/web/lists(guid'`) > -1 &&
-        r.headers.accept &&
-        r.headers.accept.indexOf('application/json') === 0) {
+      if (r.url === `${webUrl}/_api/web/lists(guid'${listId}')`) {
         correctRequestIssued = true;
       }
     });
     assert(correctRequestIssued);
   });
 
-  it('recycles the list when prompt confirmed', async () => {
-    sinon.stub(request, 'post').callsFake((opts) => {
+  it('uses correct API url when recycle option is passed', async () => {
+    sinon.stub(request, 'post').callsFake(async (opts) => {
       requests.push(opts);
-
-      if ((opts.url as string).indexOf(`/recycle()`) > -1) {
-        if (opts.headers &&
-          opts.headers.accept &&
-          (opts.headers.accept as string).indexOf('application/json') === 0) {
-          return Promise.resolve();
-        }
+      if (opts.url === `${webUrl}/_api/web/lists/GetByTitle('${listTitle}')/recycle`) {
+        return 'Correct URL';
       }
 
-      return Promise.reject('Invalid request');
+      throw 'Invalid request';
     });
-
     sinonUtil.restore(cli.promptForConfirmation);
     sinon.stub(cli, 'promptForConfirmation').resolves(true);
-    await command.action(logger, { options: { id: 'b2307a39-e878-458b-bc90-03bc578531d6', webUrl: 'https://contoso.sharepoint.com', recycle: true } });
+    await command.action(logger, {
+      options: {
+        title: 'Documents',
+        recycle: true,
+        webUrl: 'https://contoso.sharepoint.com'
+      }
+    });
     let correctRequestIssued = false;
     requests.forEach(r => {
-      if (r.url.indexOf(`/recycle()`) > -1 &&
-        r.headers.accept &&
-        r.headers.accept.indexOf('application/json') === 0) {
+      if (r.url === `${webUrl}/_api/web/lists/GetByTitle('${listTitle}')/recycle`) {
         correctRequestIssued = true;
       }
     });
 
     assert(correctRequestIssued);
   });
 
-  it('uses correct API url when recycle option is passed', async () => {
-    sinon.stub(request, 'post').callsFake((opts) => {
-      if ((opts.url as string).indexOf('/recycle()') > -1) {
-        return Promise.resolve('Correct Url');
-      }
-
-      return Promise.reject('Invalid request');
-    });
-
-    await command.action(logger, {
-      options: {
-        title: 'Documents',
-        recycle: true,
-        webUrl: 'https://contoso.sharepoint.com',
-        force: true
-      }
-    });
-  });
-
   it('command correctly handles list get reject request', async () => {
     const error = {
       error: {
@@ -208,7 +179,8 @@ describe(commands.LIST_REMOVE, () => {
       }
     };
     sinon.stub(request, 'post').callsFake(async (opts) => {
-      if ((opts.url as string).indexOf('/_api/web/lists/GetByTitle(') > -1) {
+
+      if (opts.url === `${webUrl}/_api/web/lists/GetByTitle('${listTitle}')`) {
         throw error;
       }
 

diff --git a/src/m365/spo/commands/list/list-remove.ts b/src/m365/spo/commands/list/list-remove.ts
@@ -33,17 +33,18 @@ class SpoListRemoveCommand extends SpoCommand {
 
     this.#initTelemetry();
     this.#initOptions();
+    this.#initTypes();
     this.#initValidators();
     this.#initOptionSets();
   }
 
   #initTelemetry(): void {
     this.telemetry.push((args: CommandArgs) => {
       Object.assign(this.telemetryProperties, {
-        id: (!(!args.options.id)).toString(),
-        title: (!(!args.options.title)).toString(),
-        force: (!(!args.options.force)).toString(),
-        recycle: (!(!args.options.recycle)).toString()
+        id: typeof args.options.id !== 'undefined',
+        title: typeof args.options.title !== 'undefined',
+        force: !!args.options.force,
+        recycle: !!args.options.recycle
       });
     });
   }
@@ -90,6 +91,11 @@ class SpoListRemoveCommand extends SpoCommand {
     this.optionSets.push({ options: ['id', 'title'] });
   }
 
+  #initTypes(): void {
+    this.types.string.push('id', 'title', 'webUrl');
+    this.types.boolean.push('force', 'recycle');
+  }
+
   public async commandAction(logger: Logger, args: CommandArgs): Promise<void> {
     const removeList = async (): Promise<void> => {
       if (this.verbose) {
@@ -106,7 +112,7 @@ class SpoListRemoveCommand extends SpoCommand {
       }
 
       if (args.options.recycle) {
-        requestUrl += `/recycle()`;
+        requestUrl += `/recycle`;
       }
 
       const requestOptions: CliRequestOptions = {