From f8683ce5241230cb4bd7e2a8faee96bc4a29f6fc Mon Sep 17 00:00:00 2001 From: Rudolf Rakos Date: Fri, 12 Oct 2018 19:08:57 +0200 Subject: [PATCH 1/2] GitLab - Editorial Workflow --- .../netlify-cms-backend-gitlab/src/API.js | 537 ++++++++++++++++-- .../src/implementation.js | 104 +++- 2 files changed, 563 insertions(+), 78 deletions(-) diff --git a/packages/netlify-cms-backend-gitlab/src/API.js b/packages/netlify-cms-backend-gitlab/src/API.js index f14ffb3eef86..8dd2838cde7e 100644 --- a/packages/netlify-cms-backend-gitlab/src/API.js +++ b/packages/netlify-cms-backend-gitlab/src/API.js @@ -1,7 +1,12 @@ -import { localForage, unsentRequest, then, APIError, Cursor } from 'netlify-cms-lib-util'; +import { localForage, then, unsentRequest } from 'netlify-cms-lib-util'; import { Base64 } from 'js-base64'; import { fromJS, List, Map } from 'immutable'; -import { flow, partial, result } from 'lodash'; +import { flow, get, partial, result, uniq } from 'lodash'; +import { filterPromises, resolvePromiseProperties } from 'netlify-cms-lib-util'; +import { APIError, Cursor, EditorialWorkflowError } from 'netlify-cms-lib-util'; + +export const CMS_BRANCH_PREFIX = 'cms/'; +const CMS_METADATA_BRANCH = '_netlify_cms'; export default class API { constructor(config) { @@ -10,16 +15,29 @@ export default class API { this.branch = config.branch || 'master'; this.repo = config.repo || ''; this.repoURL = `/projects/${encodeURIComponent(this.repo)}`; + this.squash_merges = config.squash_merges || false; + this.initialWorkflowStatus = config.initialWorkflowStatus; } withAuthorizationHeaders = req => unsentRequest.withHeaders(this.token ? { Authorization: `Bearer ${this.token}` } : {}, req); + withBody = body => req => + flow( + body + ? [ + r => unsentRequest.withHeaders({ 'Content-Type': 'application/json' }, r), + r => unsentRequest.withBody(JSON.stringify(body), r), + ] + : [], + )(req); + buildRequest = req => flow([ unsentRequest.withRoot(this.api_root), - this.withAuthorizationHeaders, unsentRequest.withTimestamp, + this.withAuthorizationHeaders, + this.withBody(req.body), ])(req); request = async req => @@ -72,9 +90,14 @@ export default class API { responseToJSON = res => this.parseResponse(res, { expectingFormat: 'json' }); responseToBlob = res => this.parseResponse(res, { expectingFormat: 'blob' }); responseToText = res => this.parseResponse(res, { expectingFormat: 'text' }); + requestJSON = req => this.request(req).then(this.responseToJSON); + requestBlob = req => this.request(req).then(this.responseToBlob); requestText = req => this.request(req).then(this.responseToText); + toBase64 = str => Promise.resolve(Base64.encode(str)); + fromBase64 = str => Promise.resolve(Base64.decode(str)); + user = () => this.requestJSON('/user'); WRITE_ACCESS = 30; @@ -90,22 +113,35 @@ export default class API { return false; }); - readFile = async (path, sha, { ref = this.branch, parseText = true } = {}) => { - const cacheKey = parseText ? `gl.${sha}` : `gl.${sha}.blob`; - const cachedFile = sha ? await localForage.getItem(cacheKey) : null; - if (cachedFile) { - return cachedFile; + readFile(path, sha, { ref = this.branch, parseText = true } = {}) { + if (sha) { + return this.getBlob(sha, path, ref, parseText); + } else { + return this.fetchBlob(path, ref, parseText); } - const result = await this.request({ + } + + getBlob(sha, path, ref, parseText) { + const cacheKey = parseText ? `gl.${sha}` : `gl.${sha}.blob`; + return localForage.getItem(cacheKey).then(cached => { + if (cached) { + return cached; + } + + return this.fetchBlob(path, ref, parseText).then(result => { + localForage.setItem(cacheKey, result); + return result; + }); + }); + } + + fetchBlob(path, ref, parseText) { + return flow([parseText ? this.requestText : this.requestBlob])({ url: `${this.repoURL}/repository/files/${encodeURIComponent(path)}/raw`, params: { ref }, cache: 'no-store', - }).then(parseText ? this.responseToText : this.responseToBlob); - if (sha) { - localForage.setItem(cacheKey, result); - } - return result; - }; + }); + } getCursorFromHeaders = headers => { // indices and page counts are assumed to be zero-based, but the @@ -224,58 +260,443 @@ export default class API { return entries.filter(({ type }) => type === 'blob'); }; - toBase64 = str => Promise.resolve(Base64.encode(str)); - fromBase64 = str => Base64.decode(str); - uploadAndCommit = async ( - item, - { commitMessage, updateFile = false, branch = this.branch, author = this.commitAuthor }, - ) => { - const content = await result(item, 'toBase64', partial(this.toBase64, item.raw)); - const file_path = item.path.replace(/^\//, ''); - const action = updateFile ? 'update' : 'create'; - const encoding = 'base64'; - - const commitParams = { - branch, - commit_message: commitMessage, - actions: [{ action, file_path, content, encoding }], - }; + toCommitParams(commit_message, branch, author) { + const commitParams = { branch, commit_message }; if (author) { - const { name, email } = author; - commitParams.author_name = name; - commitParams.author_email = email; + return { + ...commitParams, + author_email: author.email, + author_name: author.name, + }; + } else { + return commitParams; } + } - await this.request({ - url: `${this.repoURL}/repository/commits`, + async uploadAndCommit( + entry, + items, + { commitMessage, newEntry = true, branch = this.branch, author = this.commitAuthor }, + ) { + const commitParams = this.toCommitParams(commitMessage, branch, author); + const action = newEntry ? 'create' : 'update'; + const actions = await Promise.all( + items.map(async item => ({ + action, + file_path: item.path.replace(/^\//, ''), + content: await result(item, 'toBase64', partial(this.toBase64, item.raw)), + encoding: 'base64', + ...(item.sha ? { last_commit_id: item.sha } : {}), + })), + ); + const response = await this.requestJSON({ method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(commitParams), + url: `${this.repoURL}/repository/commits`, + body: { + ...commitParams, + actions, + }, }); + return items.map(item => ({ + ...item, + sha: response.id, + uploaded: true, + })); + } - return { ...item, uploaded: true }; - }; + persistFiles(entry, mediaFiles, options) { + const files = entry ? [entry, ...mediaFiles] : [...mediaFiles]; + if (!options.useWorkflow) { + return this.uploadAndCommit(entry, files, options); + } else { + return this.editorialWorkflowGit(entry, files, options); + } + } + + deleteFile(path, commitMessage, { branch = this.branch, author = this.commitAuthor }) { + const commitParams = this.toCommitParams(commitMessage, branch, author); + return this.request({ + method: 'DELETE', + url: `${this.repoURL}/repository/files/${encodeURIComponent(path)}`, + params: commitParams, + // last_commit_id: ???, + }); + } + + // Metadata - persistFiles = (files, { commitMessage, newEntry }) => - Promise.all( - files.map(file => - this.uploadAndCommit(file, { commitMessage, updateFile: newEntry === false }), + checkMetadataBranch() { + return this.getBranch(CMS_METADATA_BRANCH).catch(error => { + if (error instanceof APIError && error.status === 404) { + return this.createMetadataBranch(); + } + throw error; + }); + } + + createMetadataBranch() { + // The Branches API doesn't support creating orphan branches. + // The Commits API doesn't support creating orphan branches. + // https://gitlab.com/gitlab-org/gitlab-ce/issues/2420 only works for empty repositories. + throw new Error(`Not Implemented - GitLab API does not support creating orphan branches.'`); + // TODO - rudolf - Update README / Installation and Configuration. + // Git could be used for creating orphan branches. + // git checkout --orphan _netlify_cms + // git rm --cached * + // echo "# Netlify CMS\n\nThis branch is used by Netlify CMS to store metadata information for files, branches, merge requests.\n" > README.md + // git add README.md + // git commit -m "Initial commit by Netlify CMS" + // git push --set-upstream origin _netlify_cms + } + + retrieveMetadata(key) { + const metadataKey = `gl.meta.${key}`; + const metadataPath = `${key}.json`; + return localForage.getItem(metadataKey).then(cached => { + if (cached && cached.expires > Date.now()) { + return cached.data; + } + console.log( + '%c Checking for MetaData files of %s', + 'line-height: 30px;text-align: center;font-weight: bold', + key, + ); + return this.fetchBlob(metadataPath, CMS_METADATA_BRANCH, true) + .then(raw => JSON.parse(raw)) + .catch(error => { + if (error instanceof APIError && error.status === 404) { + console.log( + '%c %s does not have metadata', + 'line-height: 30px;text-align: center;font-weight: bold', + key, + ); + throw error; + } + throw error; + }); + }); + } + + storeMetadata(key, data, { create = false } = {}) { + const metadataKey = `gl.meta.${key}`; + const metadataPath = `${key}.json`; + return this.checkMetadataBranch().then(() => + this.uploadAndCommit( + null, // entry is not used + [ + { + path: metadataPath, + raw: JSON.stringify(data), + }, + ], + { + commitMessage: (create ? 'Create' : 'Update') + ` '${key}' metadata`, + newEntry: create, + branch: CMS_METADATA_BRANCH, + }, + ).then(() => + localForage.setItem(metadataKey, { + expires: Date.now() + 300000, // in 5 minutes + data, + }), ), ); + } - deleteFile = (path, commit_message, options = {}) => { - const branch = options.branch || this.branch; - const commitParams = { commit_message, branch }; - if (this.commitAuthor) { - const { name, email } = this.commitAuthor; - commitParams.author_name = name; - commitParams.author_email = email; + deleteMetadata(key) { + const metadataKey = `gl.meta.${key}`; + const metadataPath = `${key}.json`; + return this.deleteFile(metadataPath, `Delete '${key}' metadata`, { + branch: CMS_METADATA_BRANCH, + }).then(() => localForage.removeItem(metadataKey)); + } + + // Editorial Workflow + + editorialWorkflowGit(entry, items, options) { + const contentKey = entry.slug; + const branchName = this.generateBranchName(contentKey); + const unpublished = options.unpublished || false; + if (!unpublished) { + // Open new editorial review workflow for this entry - create new metadata and commit to new branch + // Note that the metadata branch is checked to fail fast and avoid inconsistent state. + return this.checkMetadataBranch().then(() => + this.createBranch(branchName).then(branch => + this.uploadAndCommit(entry, items, { ...options, branch: branch.name }).then( + updatedItems => + this.createMR(options.commitMessage, branch.name).then(mr => { + const sha = updatedItems[0].sha; + const filesList = updatedItems.map(item => ({ path: item.path, sha: item.sha })); + const user = mr.author; + const metadata = { + type: 'MR', + mr: { + number: mr.iid, + head: sha, // === mr.sha + }, + user: user.name || user.username, + status: this.initialWorkflowStatus, + branch: branch.name, + collection: options.collectionName, + title: options.parsedData && options.parsedData.title, + description: options.parsedData && options.parsedData.description, + objects: { + // NOTE - rudolf - It looks like `entry.sha` is always `undefined`. + entry: { path: entry.path, sha: entry.sha }, + files: filesList, + }, + timeStamp: new Date().toISOString(), + }; + return this.storeMetadata(contentKey, metadata, { create: true }); + }), + ), + ), + ); + } else { + // Entry is already on editorial review workflow - update metadata and commit to existing branch + return this.retrieveMetadata(contentKey).then(metadata => + this.getBranch(branchName).then(branch => + this.uploadAndCommit(entry, items, { ...options, branch: branch.name }).then( + updatedItems => { + const sha = updatedItems[0].sha; + const filesList = updatedItems.map(item => ({ path: item.path, sha: item.sha })); + // TODO - rudolf - Why does it matter whether an asset store is in use? + // if (options.hasAssetStore) { + // /** + // * If an asset store is in use, assets are always accessible, so we + // * can just finish the persist operation here. + // */ + const metadataFiles = get(metadata.objects, 'files', []); + const files = [...metadataFiles, ...filesList]; + const updatedMetadata = { + ...metadata, + mr: { + ...metadata.mr, + head: sha, + }, + title: options.parsedData && options.parsedData.title, + description: options.parsedData && options.parsedData.description, + objects: { + // NOTE - rudolf - It looks like `entry.sha` is always `undefined`. + entry: { path: entry.path, sha: entry.sha }, + files: uniq(files), + }, + timeStamp: new Date().toISOString(), + }; + return this.storeMetadata(contentKey, updatedMetadata); + // } else { + // /** + // * If no asset store is in use, assets are being stored in the content + // * repo, which means pull requests opened for editorial workflow + // * entries must be rebased if assets have been added or removed. + // */ + // return this.rebaseMR(mr.number, branch.name, contentKey, metadata, sha); + // } + }, + ), + ), + ); } - return flow([ - unsentRequest.withMethod('DELETE'), - // TODO: only send author params if they are defined. - unsentRequest.withParams(commitParams), - this.request, - ])(`${this.repoURL}/repository/files/${encodeURIComponent(path)}`); - }; + } + + listUnpublishedBranches() { + console.log( + '%c Checking for Unpublished entries', + 'line-height: 30px;text-align: center;font-weight: bold', + ); + // Get branches with a `name` matching `CMS_BRANCH_PREFIX`. + // Note that this is a substring match, so we need to check that the `branch.name` was generated by us. + return this.requestJSON({ + url: `${this.repoURL}/repository/branches`, + params: { search: CMS_BRANCH_PREFIX }, + }) + .then(branches => + branches.filter(branch => this.assertBranchName(branch.name) && !branch.merged), + ) + .then(branches => + filterPromises(branches, branch => + // Get MRs with a `source_branch` of `branch.name`. + // Note that this is *probably* a substring match, so we need to check that + // the `source_branch` of at least one of the returned objects matches `branch.name`. + this.requestJSON({ + url: `${this.repoURL}/merge_requests`, + params: { + state: 'opened', + source_branch: branch.name, + target_branch: this.branch, + }, + }).then(mrs => mrs.some(mr => mr.source_branch === branch.name)), + ), + ) + .catch(error => { + console.log( + '%c No Unpublished entries', + 'line-height: 30px;text-align: center;font-weight: bold', + ); + throw error; + }); + } + + readUnpublishedBranchFile(contentKey) { + const metaDataPromise = this.retrieveMetadata(contentKey).then( + data => (data.objects.entry.path ? data : Promise.reject(null)), + ); + return resolvePromiseProperties({ + metaData: metaDataPromise, + fileData: metaDataPromise.then(data => + this.readFile(data.objects.entry.path, null, { ref: data.branch }), + ), + isModification: metaDataPromise.then(data => + this.isUnpublishedEntryModification(data.objects.entry.path, this.branch), + ), + }).catch(() => { + throw new EditorialWorkflowError('content is not under editorial workflow', true); + }); + } + + isUnpublishedEntryModification(path, branch) { + return this.readFile(path, null, { ref: branch }) + .then(() => true) + .catch(error => { + if (error instanceof APIError && error.status === 404) { + return false; + } + throw error; + }); + } + + updateUnpublishedEntryStatus(collection, slug, status) { + const contentKey = slug; + return this.retrieveMetadata(contentKey) + .then(metadata => ({ + ...metadata, + status, + })) + .then(updatedMetadata => this.storeMetadata(contentKey, updatedMetadata)); + } + + deleteUnpublishedEntry(collection, slug) { + const contentKey = slug; + const branchName = this.generateBranchName(contentKey); + return ( + this.retrieveMetadata(contentKey) + .then(metadata => this.closeMR(metadata.mr)) + .then(() => this.deleteBranch(branchName)) + .then(() => this.deleteMetadata(contentKey)) + // If the MR doesn't exist, then this has already been deleted. + // Deletion should be idempotent, so we can consider this a success. + .catch(error => { + if (error instanceof APIError && error.status === 404) { + return Promise.resolve(); + } + throw error; + }) + ); + } + + publishUnpublishedEntry(collection, slug) { + const contentKey = slug; + const branchName = this.generateBranchName(contentKey); + return this.retrieveMetadata(contentKey) + .then(metadata => this.mergeMR(metadata.mr, metadata.branch, metadata.objects)) + .then(() => this.deleteBranch(branchName)) + .then(() => this.deleteMetadata(contentKey)); + } + + // Branches + + generateBranchName(name) { + return `${CMS_BRANCH_PREFIX}${name}`; + } + + assertBranchName(name) { + return name.startsWith(CMS_BRANCH_PREFIX); + } + + getBranch(branch = this.branch) { + return this.requestJSON(`${this.repoURL}/repository/branches/${encodeURIComponent(branch)}`); + } + + createBranch(branch, ref = this.branch) { + return this.requestJSON({ + method: 'POST', + url: `${this.repoURL}/repository/branches`, + params: { branch, ref }, + }); + } + + deleteBranch(branch) { + return this.request({ + method: 'DELETE', + url: `${this.repoURL}/repository/branches/${encodeURIComponent(branch)}`, + }); + } + + // Merge Requests + + getMR(mrNumber) { + return this.requestJSON( + `${this.repoURL}/repository/merge_requests/${encodeURIComponent(mrNumber)}`, + ); + } + + createMR(title, source_branch, target_branch = this.branch) { + console.log('%c Creating MR', 'line-height: 30px;text-align: center;font-weight: bold'); + return this.requestJSON({ + method: 'POST', + url: `${this.repoURL}/merge_requests`, + body: { + source_branch, + target_branch, + title, + description: 'Generated by Netlify CMS', + remove_source_branch: false, + squash: this.squash_merges, + }, + }); + } + + closeMR(mr) { + const mrNumber = mr.number; + console.log('%c Deleting MR', 'line-height: 30px;text-align: center;font-weight: bold'); + return this.requestJSON({ + method: 'PUT', + url: `${this.repoURL}/merge_requests/${encodeURIComponent(mrNumber)}`, + body: { + state_event: 'close', + }, + }); + } + + mergeMR(mr, branch, objects) { + const mrNumber = mr.number; + const sha = mr.head; + console.log('%c Merging MR', 'line-height: 30px;text-align: center;font-weight: bold'); + return this.requestJSON({ + method: 'PUT', + url: `${this.repoURL}/merge_requests/${encodeURIComponent(mrNumber)}/merge`, + body: { + merge_commit_message: + 'Merged by Netlify CMS\n' + `Merge branch '${branch}' into '${this.branch}'`, + sha, + }, + }).catch(error => { + if (error instanceof APIError && error.status === 405) { + return this.forceMergeMR(mr, branch, objects); + } + throw error; + }); + } + + // eslint-disable-next-line no-unused-vars + forceMergeMR(mr, branch, objects) { + // TODO - rudolf - Why is this needed? How are merge conflicts possible? + throw new Error(`Not Implemented`); + } + + // eslint-disable-next-line no-unused-vars + rebaseMR(mrNumber, branch, contentKey, metadata, sha) { + // TODO - rudolf - Why is this needed? How are assets affected by branches? + throw new Error(`Not Implemented`); + } } diff --git a/packages/netlify-cms-backend-gitlab/src/implementation.js b/packages/netlify-cms-backend-gitlab/src/implementation.js index cf7e7c9ca888..fc209044ebc2 100644 --- a/packages/netlify-cms-backend-gitlab/src/implementation.js +++ b/packages/netlify-cms-backend-gitlab/src/implementation.js @@ -3,6 +3,7 @@ import semaphore from 'semaphore'; import { CURSOR_COMPATIBILITY_SYMBOL } from 'netlify-cms-lib-util'; import AuthenticationPage from './AuthenticationPage'; import API from './API'; +import { CMS_BRANCH_PREFIX } from './API'; const MAX_CONCURRENT_DOWNLOADS = 10; @@ -15,10 +16,6 @@ export default class GitLab { ...options, }; - if (this.options.useWorkflow) { - throw new Error('The GitLab backend does not support the Editorial Workflow.'); - } - if (!this.options.proxied && config.getIn(['backend', 'repo']) == null) { throw new Error('The GitLab backend needs a "repo" in the backend configuration.'); } @@ -29,6 +26,7 @@ export default class GitLab { this.branch = config.getIn(['backend', 'branch'], 'master'); this.api_root = config.getIn(['backend', 'api_root'], 'https://gitlab.com/api/v4'); this.token = ''; + this.squash_merges = config.getIn(['backend', 'squash_merges']); } authComponent() { @@ -46,6 +44,8 @@ export default class GitLab { branch: this.branch, repo: this.repo, api_root: this.api_root, + squash_merges: this.squash_merges, + initialWorkflowStatus: this.options.initialWorkflowStatus, }); return this.api.user().then(user => this.api.hasWriteAccess(user).then(isCollab => { @@ -68,21 +68,31 @@ export default class GitLab { } entriesByFolder(collection, extension) { - return this.api.listFiles(collection.get('folder')).then(({ files, cursor }) => - this.fetchFiles(files.filter(file => file.name.endsWith('.' + extension))).then( - fetchedFiles => { + return this.api + .listFiles(collection.get('folder')) + .then(({ files, cursor }) => ({ + files: files.filter(file => file.name.endsWith('.' + extension)), + cursor, + })) + .then(({ files, cursor }) => + this.fetchFiles(files).then(fetchedFiles => { const returnedFiles = fetchedFiles; returnedFiles[CURSOR_COMPATIBILITY_SYMBOL] = cursor; return returnedFiles; - }, - ), - ); + }), + ); } allEntriesByFolder(collection, extension) { return this.api .listAllFiles(collection.get('folder')) - .then(files => this.fetchFiles(files.filter(file => file.name.endsWith('.' + extension)))); + .then(files => files.filter(file => file.name.endsWith('.' + extension))) + .then(files => + this.fetchFiles(files).then(fetchedFiles => { + const returnedFiles = fetchedFiles; + return returnedFiles; + }), + ); } entriesByFiles(collection) { @@ -107,13 +117,12 @@ export default class GitLab { .readFile(file.path, file.id) .then(data => { resolve({ file, data }); - sem.leave(); }) .catch((error = true) => { - sem.leave(); console.error(`failed to load file from GitLab: ${file.path}`); resolve({ error }); - }), + }) + .finally(() => sem.leave()), ), ), ); @@ -133,7 +142,6 @@ export default class GitLab { getMedia() { const sem = semaphore(MAX_CONCURRENT_DOWNLOADS); - return this.api.listAllFiles(this.config.get('media_folder')).then(files => files.map(({ id, name, path }) => { const getBlobPromise = () => @@ -145,24 +153,23 @@ export default class GitLab { .finally(() => sem.leave()), ), ); - return { id, name, getBlobPromise, path }; }), ); } - async persistEntry(entry, mediaFiles, options = {}) { - return this.api.persistFiles([entry], options); + persistEntry(entry, mediaFiles = [], options = {}) { + return this.api.persistFiles(entry, mediaFiles, options); } async persistMedia(mediaFile, options = {}) { - await this.api.persistFiles([mediaFile], options); + await this.api.persistFiles(null, [mediaFile], options); const { value, path, fileObj } = mediaFile; const getBlobPromise = () => Promise.resolve(fileObj); return { name: value, size: fileObj.size, getBlobPromise, path: trimStart(path, '/') }; } - deleteFile(path, commitMessage, options) { + deleteFile(path, commitMessage, options = {}) { return this.api.deleteFile(path, commitMessage, options); } @@ -174,4 +181,61 @@ export default class GitLab { cursor: newCursor, })); } + + // Editorial Workflow + + unpublishedEntries() { + return this.api.listUnpublishedBranches().then(branches => { + const sem = semaphore(MAX_CONCURRENT_DOWNLOADS); + const promises = []; + branches.forEach(branch => { + promises.push( + new Promise(resolve => { + // Strip the `CMS_BRANCH_PREFIX` from the `branch.name` to get the `slug`. + const slug = branch.name.split(CMS_BRANCH_PREFIX).pop(); + return sem.take(() => + this.unpublishedEntry(null, slug) + .then(resolve) + .catch((error = true) => { + console.error(`failed to load unpublished file from GitLab: ${slug}`); + resolve({ error }); + }) + .finally(() => sem.leave()), + ); + }), + ); + }); + return Promise.all(promises).then(loadedEntries => + loadedEntries.filter(loadedEntry => !loadedEntry.error), + ); + }); + } + + unpublishedEntry(collection, slug) { + return this.api.readUnpublishedBranchFile(slug).then(data => { + if (!data) { + return null; + } else { + return { + slug, + file: { path: data.metaData.objects.entry.path }, + data: data.fileData, + metaData: data.metaData, + isModification: data.isModification, + }; + } + }); + } + + updateUnpublishedEntryStatus(collection, slug, newStatus) { + return this.api.updateUnpublishedEntryStatus(collection, slug, newStatus); + } + + deleteUnpublishedEntry(collection, slug) { + return this.api.deleteUnpublishedEntry(collection, slug); + } + + publishUnpublishedEntry(collection, slug) { + return this.api.publishUnpublishedEntry(collection, slug); + } } From 0b661eb0b09a13ebc1305342748d607b44a909ca Mon Sep 17 00:00:00 2001 From: Rudolf Rakos Date: Fri, 26 Oct 2018 16:37:52 +0200 Subject: [PATCH 2/2] GitLab - Editorial Workflow Branches - Included `collection` and `slug` in branch name, to avoid conflicts between collections Merge Requests - Changes to the branch are allowed, so HEAD is not checked against metadata Edge - Replaced `finally` with `then` and `recover`, as it is not (yet) supported --- .../netlify-cms-backend-gitlab/src/API.js | 56 ++++++++++--------- .../src/implementation.js | 39 ++++++++----- 2 files changed, 57 insertions(+), 38 deletions(-) diff --git a/packages/netlify-cms-backend-gitlab/src/API.js b/packages/netlify-cms-backend-gitlab/src/API.js index 8dd2838cde7e..cdf1d7dcc682 100644 --- a/packages/netlify-cms-backend-gitlab/src/API.js +++ b/packages/netlify-cms-backend-gitlab/src/API.js @@ -5,7 +5,7 @@ import { flow, get, partial, result, uniq } from 'lodash'; import { filterPromises, resolvePromiseProperties } from 'netlify-cms-lib-util'; import { APIError, Cursor, EditorialWorkflowError } from 'netlify-cms-lib-util'; -export const CMS_BRANCH_PREFIX = 'cms/'; +const CMS_BRANCH_PREFIX = 'cms/'; const CMS_METADATA_BRANCH = '_netlify_cms'; export default class API { @@ -319,6 +319,7 @@ export default class API { method: 'DELETE', url: `${this.repoURL}/repository/files/${encodeURIComponent(path)}`, params: commitParams, + // Note that changes to the file are allowed, so `last_commit_id` is not checked against `item.sha`. // last_commit_id: ???, }); } @@ -414,8 +415,7 @@ export default class API { // Editorial Workflow editorialWorkflowGit(entry, items, options) { - const contentKey = entry.slug; - const branchName = this.generateBranchName(contentKey); + const branchName = this.generateBranchName(options.collectionName, entry.slug); const unpublished = options.unpublished || false; if (!unpublished) { // Open new editorial review workflow for this entry - create new metadata and commit to new branch @@ -447,14 +447,14 @@ export default class API { }, timeStamp: new Date().toISOString(), }; - return this.storeMetadata(contentKey, metadata, { create: true }); + return this.storeMetadata(branchName, metadata, { create: true }); }), ), ), ); } else { // Entry is already on editorial review workflow - update metadata and commit to existing branch - return this.retrieveMetadata(contentKey).then(metadata => + return this.retrieveMetadata(branchName).then(metadata => this.getBranch(branchName).then(branch => this.uploadAndCommit(entry, items, { ...options, branch: branch.name }).then( updatedItems => { @@ -483,14 +483,14 @@ export default class API { }, timeStamp: new Date().toISOString(), }; - return this.storeMetadata(contentKey, updatedMetadata); + return this.storeMetadata(branchName, updatedMetadata); // } else { // /** // * If no asset store is in use, assets are being stored in the content // * repo, which means pull requests opened for editorial workflow // * entries must be rebased if assets have been added or removed. // */ - // return this.rebaseMR(mr.number, branch.name, contentKey, metadata, sha); + // return this.rebaseMR(mr.number, branchName, metadata, sha); // } }, ), @@ -537,8 +537,9 @@ export default class API { }); } - readUnpublishedBranchFile(contentKey) { - const metaDataPromise = this.retrieveMetadata(contentKey).then( + readUnpublishedBranchFile(collection, slug) { + const branchName = this.generateBranchName(collection, slug); + const metaDataPromise = this.retrieveMetadata(branchName).then( data => (data.objects.entry.path ? data : Promise.reject(null)), ); return resolvePromiseProperties({ @@ -566,23 +567,22 @@ export default class API { } updateUnpublishedEntryStatus(collection, slug, status) { - const contentKey = slug; - return this.retrieveMetadata(contentKey) + const branchName = this.generateBranchName(collection, slug); + return this.retrieveMetadata(branchName) .then(metadata => ({ ...metadata, status, })) - .then(updatedMetadata => this.storeMetadata(contentKey, updatedMetadata)); + .then(updatedMetadata => this.storeMetadata(branchName, updatedMetadata)); } deleteUnpublishedEntry(collection, slug) { - const contentKey = slug; - const branchName = this.generateBranchName(contentKey); + const branchName = this.generateBranchName(collection, slug); return ( - this.retrieveMetadata(contentKey) + this.retrieveMetadata(branchName) .then(metadata => this.closeMR(metadata.mr)) .then(() => this.deleteBranch(branchName)) - .then(() => this.deleteMetadata(contentKey)) + .then(() => this.deleteMetadata(branchName)) // If the MR doesn't exist, then this has already been deleted. // Deletion should be idempotent, so we can consider this a success. .catch(error => { @@ -595,18 +595,24 @@ export default class API { } publishUnpublishedEntry(collection, slug) { - const contentKey = slug; - const branchName = this.generateBranchName(contentKey); - return this.retrieveMetadata(contentKey) + const branchName = this.generateBranchName(collection, slug); + return this.retrieveMetadata(branchName) .then(metadata => this.mergeMR(metadata.mr, metadata.branch, metadata.objects)) .then(() => this.deleteBranch(branchName)) - .then(() => this.deleteMetadata(contentKey)); + .then(() => this.deleteMetadata(branchName)); } // Branches - generateBranchName(name) { - return `${CMS_BRANCH_PREFIX}${name}`; + generateBranchName(collection, slug) { + return `${CMS_BRANCH_PREFIX}${collection}/${slug}`; + } + + deconstructBranchName(name) { + return name + .split(CMS_BRANCH_PREFIX) + .pop() + .split('/'); } assertBranchName(name) { @@ -670,7 +676,6 @@ export default class API { mergeMR(mr, branch, objects) { const mrNumber = mr.number; - const sha = mr.head; console.log('%c Merging MR', 'line-height: 30px;text-align: center;font-weight: bold'); return this.requestJSON({ method: 'PUT', @@ -678,7 +683,8 @@ export default class API { body: { merge_commit_message: 'Merged by Netlify CMS\n' + `Merge branch '${branch}' into '${this.branch}'`, - sha, + // Note that changes to the branch are allowed, so HEAD is not checked against `mr.head`. + // sha: mr.head, }, }).catch(error => { if (error instanceof APIError && error.status === 405) { @@ -695,7 +701,7 @@ export default class API { } // eslint-disable-next-line no-unused-vars - rebaseMR(mrNumber, branch, contentKey, metadata, sha) { + rebaseMR(mrNumber, branch, metadata, sha) { // TODO - rudolf - Why is this needed? How are assets affected by branches? throw new Error(`Not Implemented`); } diff --git a/packages/netlify-cms-backend-gitlab/src/implementation.js b/packages/netlify-cms-backend-gitlab/src/implementation.js index fc209044ebc2..8a58c10985aa 100644 --- a/packages/netlify-cms-backend-gitlab/src/implementation.js +++ b/packages/netlify-cms-backend-gitlab/src/implementation.js @@ -1,9 +1,9 @@ +import { Map } from 'immutable'; import trimStart from 'lodash/trimStart'; import semaphore from 'semaphore'; import { CURSOR_COMPATIBILITY_SYMBOL } from 'netlify-cms-lib-util'; import AuthenticationPage from './AuthenticationPage'; import API from './API'; -import { CMS_BRANCH_PREFIX } from './API'; const MAX_CONCURRENT_DOWNLOADS = 10; @@ -117,12 +117,13 @@ export default class GitLab { .readFile(file.path, file.id) .then(data => { resolve({ file, data }); + sem.leave(); }) .catch((error = true) => { console.error(`failed to load file from GitLab: ${file.path}`); resolve({ error }); - }) - .finally(() => sem.leave()), + sem.leave(); + }), ), ), ); @@ -149,8 +150,14 @@ export default class GitLab { sem.take(() => this.api .readFile(path, id, { parseText: false }) - .then(resolve, reject) - .finally(() => sem.leave()), + .then(data => { + resolve(data); + sem.leave(); + }) + .catch(error => { + reject(error); + sem.leave(); + }), ), ); return { id, name, getBlobPromise, path }; @@ -191,16 +198,20 @@ export default class GitLab { branches.forEach(branch => { promises.push( new Promise(resolve => { - // Strip the `CMS_BRANCH_PREFIX` from the `branch.name` to get the `slug`. - const slug = branch.name.split(CMS_BRANCH_PREFIX).pop(); + const [collection, slug] = this.api.deconstructBranchName(branch.name); return sem.take(() => - this.unpublishedEntry(null, slug) - .then(resolve) + this.unpublishedEntry(collection, slug) + .then(data => { + resolve(data); + sem.leave(); + }) .catch((error = true) => { - console.error(`failed to load unpublished file from GitLab: ${slug}`); + console.error( + `failed to load unpublished file from GitLab: ${collection}/${slug}`, + ); resolve({ error }); - }) - .finally(() => sem.leave()), + sem.leave(); + }), ); }), ); @@ -212,7 +223,9 @@ export default class GitLab { } unpublishedEntry(collection, slug) { - return this.api.readUnpublishedBranchFile(slug).then(data => { + // NOTE - rudolf - It looks like `collection` could be `Map` or `String`. + const collectionName = Map.isMap(collection) ? collection.get('name') : collection; + return this.api.readUnpublishedBranchFile(collectionName, slug).then(data => { if (!data) { return null; } else {