fix(content-item): rework move to avoid archiving skipped content or dependencies

src/commands/content-item/__mocks__/copy.ts

@@ -2,12 +2,14 @@ import { CopyItemBuilderOptions } from '../../../interfaces/copy-item-builder-op
 import { ConfigurationParameters } from '../../configure';
 import { Arguments } from 'yargs';
 
-let outputIds: string[];
+let exportIds: string[];
+let importIds: string[];
 let forceFail = false;
 export const calls: Arguments<CopyItemBuilderOptions & ConfigurationParameters>[] = [];
 
-export const setOutputIds = (ids: string[]): void => {
-  outputIds = ids;
+export const setOutputIds = (exports: string[], imports: string[]): void => {
+  exportIds = exports;
+  importIds = imports;
 };
 
 export const setForceFail = (fail: boolean): void => {
@@ -16,10 +18,15 @@ export const setForceFail = (fail: boolean): void => {
 
 export const handler = async (argv: Arguments<CopyItemBuilderOptions & ConfigurationParameters>): Promise<boolean> => {
   calls.push(argv);
-  const idOut = argv.exportedIds as string[];
+  const importedIds = argv.importedIds as string[];
+  const exportedIds = argv.exportedIds as string[];
 
-  if (idOut) {
-    idOut.push(...outputIds);
+  if (importedIds) {
+    importedIds.push(...importIds);
+  }
+
+  if (exportedIds) {
+    exportedIds.push(...exportIds);
   }
 
   return !forceFail;

src/commands/content-item/copy.spec.ts

@@ -250,13 +250,19 @@ describe('content-item copy command', () => {
         republish: true,
 
         excludeKeys: true,
-        media: true
+        media: true,
+
+        exportedIds: ['1'],
+        importedIds: ['2']
       };
       await handler(argv);
 
       expect(exportCalls.length).toEqual(1);
       expect(importCalls.length).toEqual(1);
 
+      expect(exportCalls[0].exportedIds).toEqual(['1']);
+      expect(importCalls[0].importedIds).toEqual(['2']);
+
       expect(exportCalls[0].clientId).toEqual(config.clientId);
       expect(exportCalls[0].clientSecret).toEqual(config.clientSecret);
       expect(exportCalls[0].hubId).toEqual(config.hubId);

src/commands/content-item/copy.ts

@@ -247,12 +247,16 @@ export const handler = async (argv: Arguments<CopyItemBuilderOptions & Configura
 
         media: argv.media,
         logFile: log,
-        revertLog: Promise.resolve(undefined)
+        revertLog: Promise.resolve(undefined),
+
+        importedIds: argv.importedIds
       });
 
       if (importResult) {
         log.appendLine('=== Done! ===');
         result = true;
+      } else {
+        log.appendLine('=== Importing content failed or was aborted. ===');
       }
     } catch (e) {
       log.appendLine('An unexpected error occurred: \n' + e.toString());

src/commands/content-item/export.spec.ts

@@ -804,7 +804,24 @@ describe('content-item export command', () => {
 
       const exists: ItemTemplate[] = [
         { id: 'id1', label: 'item2', repoId: 'repo1', typeSchemaUri: 'http://type', folderPath: 'exportedIds' },
-        { id: 'id2', label: 'item3', repoId: 'repo1', typeSchemaUri: 'http://type', folderPath: 'exportedIds/nested' }
+        {
+          id: 'id2',
+          label: 'item3',
+          repoId: 'repo1',
+          typeSchemaUri: 'http://type',
+          folderPath: 'exportedIds/nested',
+          body: dependsOn(['id3'])
+        },
+
+        // Dependency - Included in export, but _not_ present in exportedIds list (as it was not explicitly selected)
+        {
+          id: 'id3',
+          label: 'item5',
+          repoId: 'repo1',
+          typeSchemaUri: 'http://type',
+          folderPath: 'folder2',
+          dependancy: 'exportedIds'
+        }
       ];
 
       const skips: ItemTemplate[] = [
@@ -830,7 +847,7 @@ describe('content-item export command', () => {
       await itemsExist(`temp_${process.env.JEST_WORKER_ID}/export/`, exists);
       await itemsDontExist(`temp_${process.env.JEST_WORKER_ID}/export/`, skips);
 
-      expect(exportedIds).toEqual(exists.map(item => item.id as string));
+      expect(exportedIds).toEqual(exists.slice(0, 2).map(item => item.id as string));
 
       await rimraf(`temp_${process.env.JEST_WORKER_ID}/export/exportedIds/`);
     });

src/commands/content-item/export.ts

@@ -242,6 +242,10 @@ export const handler = async (argv: Arguments<ExportItemBuilderOptions & Configu
     }
   }
 
+  if (argv.exportedIds) {
+    argv.exportedIds.push(...items.map(({ item }) => item.id as string));
+  }
+
   log.appendLine('Scanning for dependancies.');
 
   const repoItems: RepositoryContentItem[] = items.map(item => ({ repo: dummyRepo, content: item.item }));
@@ -339,10 +343,6 @@ export const handler = async (argv: Arguments<ExportItemBuilderOptions & Configu
     log.appendLine(resolvedPath);
     await ensureDirectoryExists(directory);
 
-    if (argv.exportedIds) {
-      argv.exportedIds.push(item.id as string);
-    }
-
     writeJsonToFile(resolvedPath, item);
   }
 

src/commands/content-item/import.spec.ts

@@ -231,10 +231,17 @@ describe('content-item import command', () => {
       // Create content to import
 
       const templates: ItemTemplate[] = [
-        { label: 'item1', repoId: 'repo', typeSchemaUri: 'http://type' },
-        { label: 'item2', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest' },
-        { label: 'item3', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest', locale: 'en-us' },
-        { label: 'item4', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest/nested' }
+        { id: 'id1', label: 'item1', repoId: 'repo', typeSchemaUri: 'http://type' },
+        { id: 'id2', label: 'item2', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest' },
+        {
+          id: 'id3',
+          label: 'item3',
+          repoId: 'repo',
+          typeSchemaUri: 'http://type',
+          folderPath: 'folderTest',
+          locale: 'en-us'
+        },
+        { id: 'id4', label: 'item4', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest/nested' }
       ];
 
       await createContent(`temp_${process.env.JEST_WORKER_ID}/import/repo/`, templates, false);
@@ -248,7 +255,8 @@ describe('content-item import command', () => {
         ...config,
         dir: `temp_${process.env.JEST_WORKER_ID}/import/repo/`,
         mapFile: `temp_${process.env.JEST_WORKER_ID}/import/repo.json`,
-        baseRepo: 'targetRepo'
+        baseRepo: 'targetRepo',
+        importedIds: []
       };
       await handler(argv);
 
@@ -258,6 +266,7 @@ describe('content-item import command', () => {
 
       expect(matches.length).toEqual(templates.length);
       expect(mockContent.metrics.itemsLocaleSet).toEqual(1);
+      expect(argv.importedIds.sort()).toEqual(['id1', 'id2', 'id3', 'id4']);
 
       await rimraf(`temp_${process.env.JEST_WORKER_ID}/import/repo/`);
     });
@@ -515,17 +524,17 @@ describe('content-item import command', () => {
 
       const skipped: ItemTemplate[] = [
         // Repo 1 is missing, these should not be created.
-        { label: 'item1', repoId: 'repo', typeSchemaUri: 'http://type' },
-        { label: 'item2', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest' },
-        { label: 'item3', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest' },
-        { label: 'item4', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest/nested' }
+        { id: 'id1', label: 'item1', repoId: 'repo', typeSchemaUri: 'http://type' },
+        { id: 'id2', label: 'item2', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest' },
+        { id: 'id3', label: 'item3', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest' },
+        { id: 'id4', label: 'item4', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest/nested' }
       ];
 
       const added: ItemTemplate[] = [
-        { label: 'item5', repoId: 'repo', typeSchemaUri: 'http://type2' },
-        { label: 'item6', repoId: 'repo', typeSchemaUri: 'http://type2', folderPath: 'folderTest' },
-        { label: 'item7', repoId: 'repo', typeSchemaUri: 'http://type2', folderPath: 'folderTest' },
-        { label: 'item8', repoId: 'repo', typeSchemaUri: 'http://type2', folderPath: 'folderTest/special' }
+        { id: 'id5', label: 'item5', repoId: 'repo', typeSchemaUri: 'http://type2' },
+        { id: 'id6', label: 'item6', repoId: 'repo', typeSchemaUri: 'http://type2', folderPath: 'folderTest' },
+        { id: 'id7', label: 'item7', repoId: 'repo', typeSchemaUri: 'http://type2', folderPath: 'folderTest' },
+        { id: 'id8', label: 'item8', repoId: 'repo', typeSchemaUri: 'http://type2', folderPath: 'folderTest/special' }
       ];
 
       await createContent(`temp_${process.env.JEST_WORKER_ID}/import/missingSchema/`, skipped.concat(added), false);
@@ -546,7 +555,8 @@ describe('content-item import command', () => {
         ...config,
         dir: `temp_${process.env.JEST_WORKER_ID}/import/missingSchema/`,
         mapFile: `temp_${process.env.JEST_WORKER_ID}/import/missingSchema.json`,
-        baseRepo: 'targetRepo'
+        baseRepo: 'targetRepo',
+        importedIds: []
       };
       await handler(argv);
 
@@ -556,6 +566,9 @@ describe('content-item import command', () => {
 
       expect(matches.length).toEqual(added.length);
 
+      // Should only include content that wasn't skipped.
+      expect(argv.importedIds.sort()).toEqual(['id5', 'id6', 'id7', 'id8']);
+
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       expect((readline as any).responsesLeft()).toEqual(0); // All responses consumed.
 

src/commands/content-item/import.ts

@@ -1011,11 +1011,16 @@ export const handler = async (
     tree = await prepareContentForImport(client, hub, importRepos, null, mapping, log, argv);
   }
 
-  let result = true;
+  let result = tree != null;
 
   if (tree != null) {
+    const importIds = tree.all.map(dep => dep.owner.content.id);
     if (!validate) {
       result = await importTree(client, tree, mapping, log, argv);
+
+      if (result && argv.importedIds) {
+        argv.importedIds.push(...importIds);
+      }
     } else {
       log.appendLine('--validate was passed, so no content was imported.');
     }

src/commands/content-item/move.spec.ts

@@ -208,12 +208,17 @@ describe('content-item move command', () => {
       const copyCalls: Arguments<CopyItemBuilderOptions & ConfigurationParameters>[] = copierAny.calls;
 
       copierAny.setForceFail(false);
-      const exportIds = ['example-id', 'example-id2'];
-      copierAny.setOutputIds(exportIds);
+      const importIds = ['example-id', 'example-id2', 'example-id-skip1'];
+      const exportIds = ['example-id', 'example-id2', 'example-id-skip2'];
+      copierAny.setOutputIds(exportIds, importIds);
 
       const templates: ItemTemplate[] = [
         { id: 'example-id', label: 'item1', repoId: 'repo', typeSchemaUri: 'http://type' },
-        { id: 'example-id2', label: 'item2', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest' }
+        { id: 'example-id2', label: 'item2', repoId: 'repo', typeSchemaUri: 'http://type', folderPath: 'folderTest' },
+        // These two should not be archived on copy, due to being only in the export/import, not both.
+        // (indicates either a copied dependency, or a skipped content import)
+        { id: 'example-id-skip1', label: 'item1', repoId: 'repo', typeSchemaUri: 'http://type' },
+        { id: 'example-id-skip2', label: 'item1', repoId: 'repo', typeSchemaUri: 'http://type' }
       ];
 
       const mockContent = new MockContent(dynamicContentClientFactory as jest.Mock);
@@ -260,6 +265,7 @@ describe('content-item move command', () => {
       expect(copyCalls[0].skipIncomplete).toEqual(argv.skipIncomplete);
       expect(copyCalls[0].media).toEqual(argv.media);
 
+      expect(argv.importedIds).toEqual(importIds);
       expect(argv.exportedIds).toEqual(exportIds);
 
       expect(mockContent.metrics.itemsArchived).toEqual(2);
@@ -419,8 +425,8 @@ describe('content-item move command', () => {
       const copyCalls: Arguments<CopyItemBuilderOptions & ConfigurationParameters>[] = copierAny.calls;
 
       // These should not be archived
-      const exportIds = ['example-id', 'example-id2'];
-      copierAny.setOutputIds(exportIds);
+      const ids = ['example-id', 'example-id2'];
+      copierAny.setOutputIds(ids, ids);
       copierAny.setForceFail(true);
 
       // Create content, shouldn't be reverted.
@@ -452,8 +458,8 @@ describe('content-item move command', () => {
       const copyCalls: Arguments<CopyItemBuilderOptions & ConfigurationParameters>[] = copierAny.calls;
 
       copierAny.setForceFail(false);
-      const exportIds = ['example-id', 'example-id2'];
-      copierAny.setOutputIds(exportIds);
+      const ids = ['example-id', 'example-id2'];
+      copierAny.setOutputIds(ids, ids);
 
       const templates: ItemTemplate[] = [
         { id: 'example-id', label: 'item1', repoId: 'repo', typeSchemaUri: 'http://type' },
@@ -492,7 +498,8 @@ describe('content-item move command', () => {
 
       expect(copyCalls.length).toEqual(1);
 
-      expect(argv.exportedIds).toEqual(exportIds);
+      expect(argv.importedIds).toEqual(ids);
+      expect(argv.exportedIds).toEqual(ids);
 
       expect(mockContent.metrics.itemsArchived).toEqual(0);
     });

src/commands/content-item/move.ts

@@ -150,6 +150,7 @@ export const builder = (yargs: Argv): void => {
 
 export const handler = async (argv: Arguments<CopyItemBuilderOptions & ConfigurationParameters>): Promise<void> => {
   argv.exportedIds = [];
+  argv.importedIds = [];
 
   const { hubId, clientId, clientSecret } = argv;
 
@@ -231,13 +232,22 @@ export const handler = async (argv: Arguments<CopyItemBuilderOptions & Configura
       clientSecret: clientSecret
     });
 
-    // Only archive the result of the export once the copy has completed.
+    // Only archive the result of the import once the copy has completed.
+    // Just archive content that was selected by the export (not a dependency) and imported (not skipped by user action)
+    // All other content was not successfully "moved", or was copied due to being a dependency, so should not be archived.
     // This ensures the content is always active in one location if something goes wrong.
 
     const exported = argv.exportedIds;
+    const imported = new Set(argv.importedIds);
 
-    for (let i = 0; i < exported.length; i++) {
-      const item = await client.contentItems.get(exported[i]);
+    const ids = exported.filter(id => imported.has(id));
+
+    console.log(exported);
+    console.log(imported);
+    console.log(ids);
+
+    for (let i = 0; i < ids.length; i++) {
+      const item = await client.contentItems.get(ids[i]);
 
       try {
         await item.related.archive();

src/interfaces/copy-item-builder-options.interface.ts

@@ -29,4 +29,5 @@ export interface CopyItemBuilderOptions {
   excludeKeys?: boolean;
 
   exportedIds?: string[];
+  importedIds?: string[];
 }

src/interfaces/import-item-builder-options.interface.ts

@@ -15,4 +15,5 @@ export interface ImportItemBuilderOptions {
   logFile: FileLog;
 
   revertLog: Promise<FileLog | undefined>;
+  importedIds?: string[];
 }