fix(backend-github): improve workflow migration edge cases/messaging (#3319)

cypress/integration/common/editorial_workflow_migrations.js

@@ -41,8 +41,7 @@ export default function({ entries, getUser }) {
       cy.reload();
 
       // allow migration code to run for 5 minutes
-      Cypress.config('defaultCommandTimeout', 5 * 60 * 1000);
+      publishWorkflowEntry(entries[2], 5 * 60 * 1000);
-      publishWorkflowEntry(entries[2]);
       publishWorkflowEntry(entries[1]);
       publishWorkflowEntry(entries[0]);
       goToCollections();

cypress/utils/steps.js

@@ -73,8 +73,8 @@ function updateWorkflowStatus({ title }, fromColumnHeading, toColumnHeading) {
   assertNotification(notifications.updated);
 }
 
-function publishWorkflowEntry({ title }) {
+function publishWorkflowEntry({ title }, timeout) {
-  cy.contains('h2', workflowStatus.ready)
+  cy.contains('h2', workflowStatus.ready, { timeout })
     .parent()
     .within(() => {
       cy.contains('a', title)

packages/netlify-cms-backend-github/src/API.ts

@@ -1,6 +1,7 @@
 import { Base64 } from 'js-base64';
 import semaphore, { Semaphore } from 'semaphore';
 import { initial, last, partial, result, trimStart, trim } from 'lodash';
+import { oneLine } from 'common-tags';
 import {
   getAllResponses,
   APIError,
@@ -149,6 +150,8 @@ const getTreeFiles = (files: GitHubCompareFiles) => {
   return treeFiles;
 };
 
+let migrationNotified = false;
+
 export default class API {
   apiRoot: string;
   token: string;
@@ -672,9 +675,14 @@ export default class API {
     const newContentKey = `${metadata.collection}/${oldContentKey}`;
     const newBranchName = `cms/${newContentKey}`;
 
-    // create new branch and pull request in new format
+    // retrieve or create new branch and pull request in new format
-    await this.createBranch(newBranchName, pullRequest.head.sha as string);
+    const branch = await this.getBranch(newBranchName).catch(() => undefined);
-    const pr = await this.createPR(pullRequest.title, newBranchName);
+    if (!branch) {
+      await this.createBranch(newBranchName, pullRequest.head.sha as string);
+    }
+    const pr =
+      (await this.getPullRequests(newBranchName, PullRequestState.All, () => true))[0] ||
+      (await this.createPR(pullRequest.title, newBranchName));
 
     // store new metadata
     const newMetadata = {
@@ -703,9 +711,9 @@ export default class API {
     await this.deleteMetadata(contentKey);
   }
 
-  async migratePullRequest(pullRequest: GitHubPull) {
+  async migratePullRequest(pullRequest: GitHubPull, countMessage: string) {
     const { number } = pullRequest;
-    console.log(`Migrating Pull Request '${number}'`);
+    console.log(`Migrating Pull Request '${number}' (${countMessage})`);
     const contentKey = contentKeyFromBranch(pullRequest.head.ref);
     let metadata = await this.retrieveMetadataOld(contentKey).catch(() => undefined);
 
@@ -768,8 +776,18 @@ export default class API {
         PullRequestState.Open,
         withoutCmsLabel,
       );
+      let prCount = 0;
       for (const pr of pullRequests) {
-        await this.migratePullRequest(pr);
+        if (!migrationNotified) {
+          migrationNotified = true;
+          alert(oneLine`
+            Netlify CMS is adding labels to ${pullRequests.length} of your Editorial Workflow
+            entries. The "Workflow" tab will be unavailable during this migration. You may use other
+            areas of the CMS during this time. Note that closing the CMS will pause the migration.
+          `);
+        }
+        prCount = prCount + 1;
+        await this.migratePullRequest(pr, `${prCount} of ${pullRequests.length}`);
       }
       const cmsPullRequests = await this.getPullRequests(
         undefined,

packages/netlify-cms-backend-github/src/__tests__/API.spec.js

@@ -372,9 +372,11 @@ describe('github API', () => {
 
       const newBranch = { ref: 'refs/heads/cms/posts/2019-11-11-post-title' };
       api.createBranch = jest.fn().mockResolvedValue(newBranch);
+      api.getBranch = jest.fn().mockRejectedValue(new Error('Branch not found'));
 
       const newPr = { ...pr, number: 2 };
       api.createPR = jest.fn().mockResolvedValue(newPr);
+      api.getPullRequests = jest.fn().mockResolvedValue([]);
 
       api.storeMetadata = jest.fn();
       api.closePR = jest.fn();
@@ -403,9 +405,17 @@ describe('github API', () => {
         pullRequest: newPr,
       });
 
+      expect(api.getBranch).toHaveBeenCalledTimes(1);
+      expect(api.getBranch).toHaveBeenCalledWith('cms/posts/2019-11-11-post-title');
       expect(api.createBranch).toHaveBeenCalledTimes(1);
       expect(api.createBranch).toHaveBeenCalledWith('cms/posts/2019-11-11-post-title', 'pr_head');
 
+      expect(api.getPullRequests).toHaveBeenCalledTimes(1);
+      expect(api.getPullRequests).toHaveBeenCalledWith(
+        'cms/posts/2019-11-11-post-title',
+        'all',
+        expect.any(Function),
+      );
       expect(api.createPR).toHaveBeenCalledTimes(1);
       expect(api.createPR).toHaveBeenCalledWith('pr title', 'cms/posts/2019-11-11-post-title');
 
@@ -424,6 +434,153 @@ describe('github API', () => {
       expect(api.deleteMetadata).toHaveBeenCalledTimes(1);
       expect(api.deleteMetadata).toHaveBeenCalledWith('2019-11-11-post-title');
     });
+
+    it('should not create new branch if exists', async () => {
+      const api = new API({ branch: 'master', repo: 'owner/repo' });
+
+      const pr = {
+        head: { ref: 'cms/2019-11-11-post-title', sha: 'pr_head' },
+        title: 'pr title',
+        number: 1,
+        labels: [],
+      };
+
+      const newBranch = { ref: 'refs/heads/cms/posts/2019-11-11-post-title' };
+      api.createBranch = jest.fn();
+      api.getBranch = jest.fn().mockResolvedValue(newBranch);
+
+      const newPr = { ...pr, number: 2 };
+      api.createPR = jest.fn().mockResolvedValue(newPr);
+      api.getPullRequests = jest.fn().mockResolvedValue([]);
+
+      api.storeMetadata = jest.fn();
+      api.closePR = jest.fn();
+      api.deleteBranch = jest.fn();
+      api.deleteMetadata = jest.fn();
+
+      const branch = 'cms/2019-11-11-post-title';
+      const metadata = {
+        branch,
+        type: 'PR',
+        pr: { head: pr.head.sha },
+        commitMessage: 'commitMessage',
+        collection: 'posts',
+      };
+
+      const expectedMetadata = {
+        type: 'PR',
+        pr: { head: newPr.head.sha, number: 2 },
+        commitMessage: 'commitMessage',
+        collection: 'posts',
+        branch: 'cms/posts/2019-11-11-post-title',
+        version: '1',
+      };
+      await expect(api.migrateToVersion1(pr, metadata)).resolves.toEqual({
+        metadata: expectedMetadata,
+        pullRequest: newPr,
+      });
+
+      expect(api.getBranch).toHaveBeenCalledTimes(1);
+      expect(api.getBranch).toHaveBeenCalledWith('cms/posts/2019-11-11-post-title');
+      expect(api.createBranch).toHaveBeenCalledTimes(0);
+
+      expect(api.getPullRequests).toHaveBeenCalledTimes(1);
+      expect(api.getPullRequests).toHaveBeenCalledWith(
+        'cms/posts/2019-11-11-post-title',
+        'all',
+        expect.any(Function),
+      );
+      expect(api.createPR).toHaveBeenCalledTimes(1);
+      expect(api.createPR).toHaveBeenCalledWith('pr title', 'cms/posts/2019-11-11-post-title');
+
+      expect(api.storeMetadata).toHaveBeenCalledTimes(1);
+      expect(api.storeMetadata).toHaveBeenCalledWith(
+        'posts/2019-11-11-post-title',
+        expectedMetadata,
+      );
+
+      expect(api.closePR).toHaveBeenCalledTimes(1);
+      expect(api.closePR).toHaveBeenCalledWith(pr.number);
+
+      expect(api.deleteBranch).toHaveBeenCalledTimes(1);
+      expect(api.deleteBranch).toHaveBeenCalledWith('cms/2019-11-11-post-title');
+
+      expect(api.deleteMetadata).toHaveBeenCalledTimes(1);
+      expect(api.deleteMetadata).toHaveBeenCalledWith('2019-11-11-post-title');
+    });
+
+    it('should not create new pr if exists', async () => {
+      const api = new API({ branch: 'master', repo: 'owner/repo' });
+
+      const pr = {
+        head: { ref: 'cms/2019-11-11-post-title', sha: 'pr_head' },
+        title: 'pr title',
+        number: 1,
+        labels: [],
+      };
+
+      const newBranch = { ref: 'refs/heads/cms/posts/2019-11-11-post-title' };
+      api.createBranch = jest.fn();
+      api.getBranch = jest.fn().mockResolvedValue(newBranch);
+
+      const newPr = { ...pr, number: 2 };
+      api.createPR = jest.fn();
+      api.getPullRequests = jest.fn().mockResolvedValue([newPr]);
+
+      api.storeMetadata = jest.fn();
+      api.closePR = jest.fn();
+      api.deleteBranch = jest.fn();
+      api.deleteMetadata = jest.fn();
+
+      const branch = 'cms/2019-11-11-post-title';
+      const metadata = {
+        branch,
+        type: 'PR',
+        pr: { head: pr.head.sha },
+        commitMessage: 'commitMessage',
+        collection: 'posts',
+      };
+
+      const expectedMetadata = {
+        type: 'PR',
+        pr: { head: newPr.head.sha, number: 2 },
+        commitMessage: 'commitMessage',
+        collection: 'posts',
+        branch: 'cms/posts/2019-11-11-post-title',
+        version: '1',
+      };
+      await expect(api.migrateToVersion1(pr, metadata)).resolves.toEqual({
+        metadata: expectedMetadata,
+        pullRequest: newPr,
+      });
+
+      expect(api.getBranch).toHaveBeenCalledTimes(1);
+      expect(api.getBranch).toHaveBeenCalledWith('cms/posts/2019-11-11-post-title');
+      expect(api.createBranch).toHaveBeenCalledTimes(0);
+
+      expect(api.getPullRequests).toHaveBeenCalledTimes(1);
+      expect(api.getPullRequests).toHaveBeenCalledWith(
+        'cms/posts/2019-11-11-post-title',
+        'all',
+        expect.any(Function),
+      );
+      expect(api.createPR).toHaveBeenCalledTimes(0);
+
+      expect(api.storeMetadata).toHaveBeenCalledTimes(1);
+      expect(api.storeMetadata).toHaveBeenCalledWith(
+        'posts/2019-11-11-post-title',
+        expectedMetadata,
+      );
+
+      expect(api.closePR).toHaveBeenCalledTimes(1);
+      expect(api.closePR).toHaveBeenCalledWith(pr.number);
+
+      expect(api.deleteBranch).toHaveBeenCalledTimes(1);
+      expect(api.deleteBranch).toHaveBeenCalledWith('cms/2019-11-11-post-title');
+
+      expect(api.deleteMetadata).toHaveBeenCalledTimes(1);
+      expect(api.deleteMetadata).toHaveBeenCalledWith('2019-11-11-post-title');
+    });
   });
 
   describe('migrateToPullRequestLabels', () => {

packages/netlify-cms-core/src/backend.ts

@@ -557,7 +557,7 @@ export class Backend {
           const entry = createEntry(collectionName, loadedEntry.slug, loadedEntry.file.path, {
             raw: loadedEntry.data,
             isModification: loadedEntry.isModification,
-            label: selectFileEntryLabel(collection, loadedEntry.slug!),
+            label: collection && selectFileEntryLabel(collection, loadedEntry.slug!),
           });
           entry.metaData = loadedEntry.metaData;
           return entry;
@@ -569,6 +569,10 @@ export class Backend {
           const collection = collections.get(entry.collection);
           if (collection) {
             acc.push(this.entryWithFormat(collection)(entry) as EntryValue);
+          } else {
+            console.warn(
+              `Missing collection '${entry.collection}' for entry with path '${entry.path}'`,
+            );
           }
           return acc;
         }, [] as EntryValue[]),