fix(backend-github): prepend collection name

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

@@ -18,6 +18,7 @@ import {
   APIError,
   EditorialWorkflowError,
   filterPromisesWith,
+  flowAsync,
   localForage,
   onlySuccessfulPromises,
   resolvePromiseProperties,
@@ -148,9 +149,7 @@ export default class API {
 
   generateContentKey(collectionName, slug) {
     if (!this.useOpenAuthoring) {
-      // this doesn't use the collection, but we need to leave it that way for backwards
-      // compatibility
-      return slug;
+      return `${collectionName}/${slug}`;
     }
 
     return `${this.repo}/${collectionName}/${slug}`;
@@ -225,6 +224,29 @@ export default class API {
     );
   }
 
+  deleteMetadata(key) {
+    if (!this._metadataSemaphore) {
+      this._metadataSemaphore = semaphore(1);
+    }
+    return new Promise(resolve =>
+      this._metadataSemaphore.take(async () => {
+        try {
+          const branchData = await this.checkMetadataRef();
+          const file = { path: `${key}.json`, sha: null };
+
+          const changeTree = await this.updateTree(branchData.sha, [file]);
+          const { sha } = await this.commit(`Deleting “${key}” metadata`, changeTree);
+          await this.patchRef('meta', '_netlify_cms', sha);
+          this._metadataSemaphore.leave();
+          resolve();
+        } catch (err) {
+          this._metadataSemaphore.leave();
+          resolve();
+        }
+      }),
+    );
+  }
+
   retrieveMetadata(key) {
     const cache = localForage.getItem(`gh.meta.${key}`);
     return cache.then(cached => {
@@ -394,27 +416,21 @@ export default class API {
       });
   }
 
-  getPRsForBranchName = ({
-    branchName,
-    state,
-    base = this.branch,
-    repoURL = this.repoURL,
-    usernameOfFork,
-  } = {}) => {
+  getPRsForBranchName = branchName => {
     // Get PRs with a `head` of `branchName`. Note that this is a
     // substring match, so we need to check that the `head.ref` of
     // at least one of the returned objects matches `branchName`.
-    return this.requestAllPages(`${repoURL}/pulls`, {
+    return this.requestAllPages(`${this.repoURL}/pulls`, {
       params: {
-        head: usernameOfFork ? `${usernameOfFork}:${branchName}` : branchName,
-        ...(state ? { state } : {}),
-        base,
+        head: branchName,
+        state: 'open',
+        base: this.branch,
       },
     });
   };
 
-  branchHasPR = async ({ branchName, ...rest }) => {
-    const prs = await this.getPRsForBranchName({ branchName, ...rest });
+  branchHasPR = async branchName => {
+    const prs = await this.getPRsForBranchName(branchName);
     return prs.some(pr => pr.head.ref === branchName);
   };
 
@@ -459,14 +475,55 @@ export default class API {
     return metadata;
   };
 
+  async migrateToVersion1(oldContentKey, metaData) {
+    const newContentKey = this.generateContentKey(metaData.collection, oldContentKey);
+    const newBranchName = this.generateBranchName(newContentKey);
+
+    // create new branch and pull request in new format
+    const newBranch = await this.createBranch(newBranchName, metaData.pr.head);
+    const pr = await this.createPR(metaData.commitMessage, newBranchName);
+
+    // store new metadata
+    await this.storeMetadata(newContentKey, {
+      ...metaData,
+      pr: {
+        number: pr.number,
+        head: pr.head.sha,
+      },
+      branch: newBranchName,
+      version: '1',
+    });
+
+    // remove old data
+    await this.closePR(metaData.pr);
+    await this.deleteBranch(metaData.branch);
+    await this.deleteMetadata(oldContentKey);
+
+    return newBranch;
+  }
+
+  async migrateBranch(branch) {
+    const contentKey = this.contentKeyFromRef(branch.ref);
+    const metadata = await this.retrieveMetadata(contentKey);
+    if (!metadata.version) {
+      // migrate branch from cms/slug to cms/collection/slug
+      branch = await this.migrateToVersion1(contentKey, metadata);
+    }
+
+    return branch;
+  }
+
   async listUnpublishedBranches() {
     console.log(
       '%c Checking for Unpublished entries',
       'line-height: 30px;text-align: center;font-weight: bold',
     );
-    const onlyBranchesWithOpenPRs = filterPromisesWith(({ ref }) =>
-      this.branchHasPR({ branchName: this.branchNameFromRef(ref), state: 'open' }),
-    );
+    const onlyBranchesWithOpenPRs = flowAsync([
+      filterPromisesWith(({ ref }) => this.branchHasPR(this.branchNameFromRef(ref))),
+      map(branch => this.migrateBranch(branch)),
+      onlySuccessfulPromises,
+    ]);
+
     const getUpdatedOpenAuthoringBranches = flow([
       map(async branch => {
         const contentKey = this.contentKeyFromRef(branch.ref);
@@ -621,6 +678,7 @@ export default class API {
           files: mediaFilesList,
         },
         timeStamp: new Date().toISOString(),
+        version: '1',
       });
     } else {
       // Entry is already on editorial review workflow - just update metadata and commit to existing branch
@@ -862,6 +920,7 @@ export default class API {
       this.retrieveMetadata(contentKey)
         .then(metadata => (metadata && metadata.pr ? this.closePR(metadata.pr) : Promise.resolve()))
         .then(() => this.deleteBranch(branchName))
+        .then(() => this.deleteMetadata(contentKey))
         // If the PR doesn't exist, then this has already been deleted -
         // deletion should be idempotent, so we can consider this a
         // success.
@@ -881,6 +940,7 @@ export default class API {
     const metadata = await this.retrieveMetadata(contentKey);
     await this.mergePR(metadata.pr, metadata.objects);
     await this.deleteBranch(branchName);
+    await this.deleteMetadata(contentKey);
 
     return metadata;
   }

packages/netlify-cms-backend-github/src/GraphQLAPI.js

@@ -228,7 +228,8 @@ export default class GraphQLAPI extends API {
           branches.push({ ref: `${headRef.prefix}${headRef.name}` });
         });
       });
-      return branches;
+
+      return await Promise.all(branches.map(branch => this.migrateBranch(branch)));
     } else {
       console.log(
         '%c No Unpublished entries',

packages/netlify-cms-backend-github/src/implementation.js

@@ -387,8 +387,8 @@ export default class GitHub {
         const promises = [];
         branches.map(({ ref }) => {
           promises.push(
-            new Promise(resolve => {
-              const contentKey = ref.split('refs/heads/cms/').pop();
+            new Promise(async resolve => {
+              const contentKey = this.api.contentKeyFromRef(ref);
               const slug = contentKey.split('/').pop();
               return sem.take(() =>
                 this.api

packages/netlify-cms-lib-util/src/index.js

@@ -14,6 +14,7 @@ import {
   filterPromisesWith,
   onlySuccessfulPromises,
   resolvePromiseProperties,
+  flowAsync,
   then,
 } from './promise';
 import unsentRequest from './unsentRequest';
@@ -44,6 +45,7 @@ export const NetlifyCmsLibUtil = {
   filterPromisesWith,
   onlySuccessfulPromises,
   resolvePromiseProperties,
+  flowAsync,
   then,
   unsentRequest,
   filterByPropExtension,
@@ -69,6 +71,7 @@ export {
   filterPromisesWith,
   onlySuccessfulPromises,
   resolvePromiseProperties,
+  flowAsync,
   then,
   unsentRequest,
   filterByPropExtension,

packages/netlify-cms-lib-util/src/promise.js

@@ -33,3 +33,6 @@ export const onlySuccessfulPromises = flow([
   then(Promise.all.bind(Promise)),
   then(filter(maybeValue => maybeValue !== filterPromiseSymbol)),
 ]);
+
+const wrapFlowAsync = fn => async arg => fn(await arg);
+export const flowAsync = fns => flow(fns.map(fn => wrapFlowAsync(fn)));