Editorial Workflow support for GitLab #1817

ghost · 2018-10-18T14:05:48Z

Summary

Editorial Workflow support for GitLab

It's essentially the same as for GitHub with slightly different API calls.
I tried to keep the code as close as possible between the backends, to make refactoring common things easier. (please check that for reference)
It's been years since the last time I used JavaScript, so I'm open for suggestions about coding style and best practices.

I added a couple of TODOs about things I was unsure about.
I didn't implement resolving conflicts or rebasing branches.

Fixes half of #568 (please see my comments there)

Test plan

We tried positive scenarios manually (CMS changes to branches, merge requests, metadata).
We didn't try every negative scenario (external changes to branches, merge requests, metadata).

We manually migrated our existing draft entries to unpublished entries.

Simple use case:

Create new unpublished entry
[GitLab] Check that the branch and merge requests is created
Publish unpublished entry
[GitLab] Check that the branch is deleted and merge requests is merged

Complex use case:

Edit existing published entry
[GitLab] Check that the branch and merge requests is created
Edit unpublished entry
[GitLab] Check that the branch and merge requests is updated
Delete unpublished entry
[GitLab] Check that the branch is deleted and merge requests is closed

A picture of a cute animal

GitLab - About Us:

Our Tanuki (Japanese for raccoon dog) logo symbolizes this with a smart animal that works in a group to achieve a common goal.

verythorough · 2018-10-18T14:10:36Z

Deploy preview for netlify-cms-www ready!

Built with commit 0b661eb

https://deploy-preview-1817--netlify-cms-www.netlify.com

verythorough · 2018-10-18T14:12:21Z

Deploy preview for cms-demo ready!

Built with commit 0b661eb

https://deploy-preview-1817--cms-demo.netlify.com

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

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

ghost · 2018-10-18T14:19:10Z

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

+    // 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.


TODO - rudolf - Update README / Installation and Configuration.

ghost · 2018-10-18T14:19:36Z

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

+            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?


TODO - rudolf - Why does it matter whether an asset store is in use?

When you upload an image to the repo, it creates a commit direct against the configured branch. If you save a workflow entry without publishing it, and then upload and use a new image in that entry, saving again without publishing, the entry will reference an image that doesn't exist in the PR (because the new image was committed direct to the base branch). Therefore, if images are being stored in the repo (no asset store), we rebase the pull request onto the latest commit on the base branch every time the unpublished entry is saved (but only if the base branch has changed).

And yes, this is silly and shouldn't be the case, but it currently is.

ghost · 2018-10-18T14:21:24Z

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

+
+  // eslint-disable-next-line no-unused-vars
+  forceMergeMR(mr, branch, objects) {
+    // TODO - rudolf - Why is this needed? How are merge conflicts possible?


TODO - rudolf - Why is this needed? How are merge conflicts possible? (see mergeMr)

Actually, this is the current documented behaviour:

If merge request is unable to be accepted (ie: Work in Progress, Closed, Pipeline Pending Completion, or Failed while requiring Success) - you’ll get a 405 and the error message ‘Method Not Allowed’

If it has some conflicts and can not be merged - you’ll get a 406 and the error message ‘Branch cannot be merged’

If the sha parameter is passed and does not match the HEAD of the source - you’ll get a 409 and the error message ‘SHA does not match HEAD of source branch’

If you don’t have permissions to accept this merge request - you’ll get a 401

ghost · 2018-10-18T14:25:51Z

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

+
+  // 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?


TODO - rudolf - Why is this needed? How are assets affected by branches? (see editorialWorkflowGit)

Depending on the setting for a GitLab repository, it might be only allowed to merge if a fast forward merge is possible.
That means if someone has merged something while you've created a new merge request you need to rebase in order to merge your own changes.

ghost · 2018-10-18T14:29:29Z

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

+    const metadataPath = `${key}.json`;
+    return this.checkMetadataBranch().then(() =>
+      this.uploadAndCommit(
+        null, // entry is not used


I assumed that entry is not used in uploadAndCommit, but we probably should use a separate method for metadata.

ghost · 2018-10-18T14:39:43Z

Test results

Commits:

Merge Request - Closed:

Merge Request - Merged:

CMS - 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

erquhart · 2018-10-26T19:52:49Z

This is crazy awesome @rudolf-zivver!! We're working on reviewing now, stay tuned.

praveev · 2018-11-25T04:04:39Z

+1 any update on this feature?

erquhart · 2018-11-27T02:24:23Z

We've given this some review, but it needs more - specifically testing. Merging changes to backends is shaky business without tests, as a small mistake could result in very big problems.

For folks in the community that want this, we could definitely use more eyes and reviews.

@rudolf-zivver has your team been using this in production? Any discoveries?

Yttrium-tYcLief · 2018-12-01T16:46:38Z

I hate to chime in where it's possibly unwelcome, but as the reference above shows, if I could get #1298 (or #2 or #1268) functional, then I could very extensively test this PR in production with a userbase of at least 20 or so users.

ghost · 2018-12-04T11:38:11Z

Let me get back to this PR next week. (deliverables, deadlines, etc.) ⏳ 🙁

Edit: It works for us in production, but we have a quite simple setup.

jamezrin · 2019-02-08T21:40:20Z

The original MR author deleted its account, is there a chance for this to get assigned to someone else if that's even possible?

erquhart · 2019-02-08T22:01:33Z

Sure is, just let us know.

timaschew · 2019-03-11T11:25:03Z

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

+    //   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


Why it's so important to create an orphan branch?

I mean it looks cleaner okay, but if this is not supported by GitLab, I don't see any problem to create just a normal branch or based on the first commit in the repo or even on the current master.

Definitely, we're aware that orphan refs are GitHub only - we're actually moving away from this model entirely and using more obvious methods like PR/MR labels to track status.

Here's an issue outlining that: #1669

And here's a WIP PR implementing it for GitHub: #1961

verythorough · 2019-03-11T17:17:03Z

Adding this feature will require an update to the docs. Since they're in the same repo, you might as well include the changes in here. :)

Here's where you'll need to update:

Thanks!

timaschew · 2019-03-11T21:40:07Z

@Yttrium-tYcLief @jamezrin I got it running with a self hosted GitLab.
The happy path is working fine, but if there is a merge conflict or discussions hasn't been resolved yet then the publish (merge to master) doesn't work as well. If you're interested I can publish it in my fork.

@erquhart Any plans to show the current state of a merge request in the workflow, like pipeline/job status, if discussions/reviews has been resolved or not?

erquhart · 2019-03-15T14:57:07Z

Currently we're only handling workflow status (draft/pending review/pending publish) and providing links to deploy previews when available. More robust status representation definitely makes sense, but it's not something we would take a PR for at the moment, needs a fair amount of product, arch, and design before we even know how to approach it.

rico-ocepek · 2019-04-30T11:51:50Z

Any news on this one? Would love to finally use this with my gitlab setup

zomars · 2019-06-24T21:32:38Z

Almost two months without update? Is there something missing?

erquhart · 2019-06-26T15:07:03Z

This is an abandoned PR.

#1817 (comment)

erquhart · 2019-06-26T15:58:06Z

As the branch for this PR can no longer be updated, I've opened a new PR to continue development and discussion, see #2409.

GitLab - Editorial Workflow

f8683ce

ghost commented Oct 18, 2018

View reviewed changes

packages/netlify-cms-backend-gitlab/src/implementation.js Show resolved Hide resolved

ghost commented Oct 18, 2018

View reviewed changes

packages/netlify-cms-backend-gitlab/src/implementation.js Outdated Show resolved Hide resolved

ghost commented Oct 18, 2018

View reviewed changes

packages/netlify-cms-backend-gitlab/src/API.js Show resolved Hide resolved

ghost commented Oct 18, 2018

View reviewed changes

Yttrium-tYcLief mentioned this pull request Dec 1, 2018

Add allow_publish flag for editorial workflow #1298

Closed

erquhart mentioned this pull request Dec 13, 2018

Determine testing strategies #1013

Closed

28 tasks

tomrutgers mentioned this pull request Jan 14, 2019

Error: The GitLab backend does not support the Editorial Workflow. #2003

Closed

emilio-martinez mentioned this pull request Mar 4, 2019

Add Editorial Workflow support for GitLab and BitBucket backends #568

Closed

timaschew reviewed Mar 11, 2019

View reviewed changes

This was referenced Jun 26, 2019

Editorial Workflow support for GitLab #2408

Closed

Editorial Workflow support for GitLab #2409

Closed

erquhart closed this Jun 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editorial Workflow support for GitLab #1817

Editorial Workflow support for GitLab #1817

ghost commented Oct 18, 2018 •

edited by ghost

Loading

verythorough commented Oct 18, 2018 •

edited

Loading

verythorough commented Oct 18, 2018 •

edited

Loading

ghost Oct 18, 2018 •

edited by ghost

Loading

ghost Oct 18, 2018 •

edited by ghost

Loading

erquhart Nov 26, 2018 •

edited

Loading

erquhart Nov 26, 2018

ghost Oct 18, 2018

timaschew Mar 11, 2019

ghost Oct 18, 2018

timaschew Mar 11, 2019

ghost Oct 18, 2018

ghost commented Oct 18, 2018 •

edited by ghost

Loading

erquhart commented Oct 26, 2018

praveev commented Nov 25, 2018

erquhart commented Nov 27, 2018

Yttrium-tYcLief commented Dec 1, 2018

ghost commented Dec 4, 2018 •

edited by ghost

Loading

jamezrin commented Feb 8, 2019

erquhart commented Feb 8, 2019

timaschew Mar 11, 2019 •

edited

Loading

erquhart Mar 11, 2019

verythorough commented Mar 11, 2019

timaschew commented Mar 11, 2019 •

edited

Loading

erquhart commented Mar 15, 2019

rico-ocepek commented Apr 30, 2019

zomars commented Jun 24, 2019

erquhart commented Jun 26, 2019

erquhart commented Jun 26, 2019

Editorial Workflow support for GitLab #1817

Editorial Workflow support for GitLab #1817

Conversation

ghost commented Oct 18, 2018 • edited by ghost Loading

verythorough commented Oct 18, 2018 • edited Loading

verythorough commented Oct 18, 2018 • edited Loading

ghost Oct 18, 2018 • edited by ghost Loading

Choose a reason for hiding this comment

ghost Oct 18, 2018 • edited by ghost Loading

Choose a reason for hiding this comment

erquhart Nov 26, 2018 • edited Loading

Choose a reason for hiding this comment

erquhart Nov 26, 2018

Choose a reason for hiding this comment

ghost Oct 18, 2018

Choose a reason for hiding this comment

timaschew Mar 11, 2019

Choose a reason for hiding this comment

ghost Oct 18, 2018

Choose a reason for hiding this comment

timaschew Mar 11, 2019

Choose a reason for hiding this comment

ghost Oct 18, 2018

Choose a reason for hiding this comment

ghost commented Oct 18, 2018 • edited by ghost Loading

erquhart commented Oct 26, 2018

praveev commented Nov 25, 2018

erquhart commented Nov 27, 2018

Yttrium-tYcLief commented Dec 1, 2018

ghost commented Dec 4, 2018 • edited by ghost Loading

jamezrin commented Feb 8, 2019

erquhart commented Feb 8, 2019

timaschew Mar 11, 2019 • edited Loading

Choose a reason for hiding this comment

erquhart Mar 11, 2019

Choose a reason for hiding this comment

verythorough commented Mar 11, 2019

timaschew commented Mar 11, 2019 • edited Loading

erquhart commented Mar 15, 2019

rico-ocepek commented Apr 30, 2019

zomars commented Jun 24, 2019

erquhart commented Jun 26, 2019

erquhart commented Jun 26, 2019

ghost commented Oct 18, 2018 •

edited by ghost

Loading

verythorough commented Oct 18, 2018 •

edited

Loading

verythorough commented Oct 18, 2018 •

edited

Loading

ghost Oct 18, 2018 •

edited by ghost

Loading

ghost Oct 18, 2018 •

edited by ghost

Loading

erquhart Nov 26, 2018 •

edited

Loading

ghost commented Oct 18, 2018 •

edited by ghost

Loading

ghost commented Dec 4, 2018 •

edited by ghost

Loading

timaschew Mar 11, 2019 •

edited

Loading

timaschew commented Mar 11, 2019 •

edited

Loading