GitLab backend built with cursor API #1343

Benaiah · 2018-05-07T22:09:07Z

This PR contains the commits from #517, rebased onto the cursor API. This will now be the primary development PR for GitLab.

The best summary of the changes is the commit log, but a quick list of the most major aspects are here. (Once this PR has been created, I'll be doing much more detailed code review comments to request input on specific questions):

A new cursor API (WIP - Cursor API #1301) has been added, which allows backends to advertise support of an arbitrary set of pagination actions. It is entirely backwards-compatible, and existing backends require zero changes to maintain their current behavior. The test backend has been modified to use cursor-based pagination.
Integrations have been moved out of action creators and into backend.js (except for media library integrations).
The GitLab backend has been refactored using unsentRequest.js, and now uses cursor-based pagination.
An internal backend.js method has been added to retrieve all entries of a collection regardless of its pagination, for use in local search and querying.

Todo before merge:

Merge @tech4him1's implicit auth (WIP: Implicit OAuth for GitLab #1314) into this branch.
Revert changes to example/config.yml, remove remaining debug code
Re-enable infinite scroll when the old pagination API (based on incrementing indexes) is in use.¹
Refactor the cursor API to be immutable.
Finish ensuring integration changes work correctly with Smashing's setup.²
Create tests for cursors and unsentRequest.¹
Update docs (Add GitLab backend to docs #1413)

Todo after merge (some of these are not GitLab-specific, but general backend work):

Decouple API pagination from UI pagination, so the first page of a collection isn't the shortest one (this would also enable UI pagination of backends that don't actually paginate the API results).
Support editorial workflow.
Support pagination and cursors in the media library.
Move media library integrations into backend.js.
Finish git cache to reduce number of requests and cache cursors, entries, and completly-known trees (e.g., trees returned from listAllEntries). Currently I have a smart git cache working, but clunky - I'm doing some work to make it asynchronous and support partial knowledge of objects (for instance, truncated trees).

¹ This could also be done by making integration pagination use cursors, but that may take longer than the simple fix.
² These steps may end up being done after merge, depending on how we decide to release this.

verythorough · 2018-05-07T22:10:41Z

Deploy preview for netlify-cms-www ready!

Built with commit d86c610

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

Benaiah · 2018-05-07T22:11:40Z

src/valueObjects/Cursor.js

@@ -0,0 +1,52 @@
+import isArray from "lodash/isArray";


Should this file be moved/renamed? Cursor is not actually a class, just a definition and validation function for a specified set of properties on any object.

Any reason not to provide a Cursor factory?

Benaiah · 2018-05-07T22:12:28Z

src/reducers/cursors.js

+// Since pagination can be used for a variety of views (collections
+// and searches are the most common examples), we namespace cursors by
+// their type before storing them in the state.
+export const collectionEntriesCursorKey = collectionName => `collectionEntries.${ collectionName }`;


This allows us to use pagination only for collections at the moment, while still using a Redux structure that will support other uses of cursors.

Benaiah · 2018-05-07T22:13:33Z

src/reducers/search.js

@@ -38,7 +38,7 @@ const entries = (state = defaultState, action) => {
        map.set('isFetching', false);
        map.set('page', page);
        map.set('term', searchTerm);
-        map.set('entryIds', page === 0 ? entryIds : map.get('entryIds', List()).concat(entryIds));
+        map.set('entryIds', (!page || page === 0) ? entryIds : map.get('entryIds', List()).concat(entryIds));


When we're using cursor-based pagination, we do not want the entryIds to be appended to, since we only model knowledge of one set of entries and one cursor, which we retrieve from the backend.

Benaiah · 2018-05-07T22:17:51Z

src/lib/promiseHelper.js

@@ -18,3 +18,5 @@ export const resolvePromiseProperties = (obj) => {
    // resolved values
    Object.assign({}, obj, zipObject(promiseKeys, resolvedPromises)));
 };
+
+export const then = func => p => ((p && p.then) ? p.then(func) : func(p));


This is a very simple function that lets you do something like the following (flow being from lodash):

const fn = flow([ then(x => x + 2), // this will work whether or not x is a Promise then(x => x * 2), ]) fn(2) // -> 8 fn(Promise.resolve(2)) // -> Promise resolving to 8

One notable question: should this return synchronously if it doesn't receive a Promise (as it does currently), or should it be rewritten as:

export const then = func => p => Promise.resolve(p).then(func) const fn = flow([ then(x => x + 2), // this will work whether or not x is a Promise then(x => x * 2), ]) fn(2) // -> Promise resolving to 8 fn(Promise.resolve(2)) // -> Promise resolving to 8

The advantage of this would be that then would always return a Promise, regardless of what was passed in, which seems more in line with the name and analogy to the .then method it's wrapping.

Benaiah · 2018-05-07T22:21:39Z

src/backends/backend.js

+  // returns all the collected entries. Used to retrieve all entries
+  // for local searches and queries.
+  async listAllEntries(collection) {
+    if (collection.get("folder") && this.implementation.allEntriesByFolder) {


I'm not very happy with how this check turned out, but it's basically necessary due to the use of two methods for listing collection (entriesByFiles and entriesByFolder). Refactoring the backend API should make this much neater by unifying the two.

Note that test-repo does not have a special implementation of allEntriesByFolder - it simply returns a cursor, and listing all entries can be done using just the cursor (as long as it supports the "next" action). GitLab has a particular implementation of this for performance reasons (it raises the files-per-page to GitLab's maximum, and doesn't list the entries in reverse).

Benaiah · 2018-05-07T22:25:20Z

src/actions/search.js

@@ -1,10 +1,6 @@
 import fuzzy from 'fuzzy';
 import { currentBackend } from 'Backends/backend';
-import { getIntegrationProvider } from 'Integrations';
-import { selectIntegration, selectEntries } from 'Reducers';
-import { selectInferedField } from 'Reducers/collections';
 import { WAIT_UNTIL_ACTION } from 'Redux/middleware/waitUntilAction';


TODO: remove this import, as well as fuzzy, which are not used in the file.

verythorough · 2018-05-08T05:36:22Z

Deploy preview for cms-demo ready!

Built with commit d86c610

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

Benaiah · 2018-05-09T03:07:46Z

Update: implicit grant authentication support (#1314) has been added to this branch.

talves · 2018-05-10T20:49:32Z

Built current changes and tested local, production. Media, post update, creation and delete working.

@Benaiah let me know if there is something specific we need to test. Great work!

erquhart

Awesome work @Benaiah. My comments are all around consolidation of Cursor logic so cursor interactions are discreet across the codebase, as we discussed.

erquhart · 2018-05-16T19:21:11Z

src/actions/entries.js

+    // structure.
+    .then(response => ((!response.cursor) || validateCursor(response.cursor)
+      ? response
+      : Promise.reject(invalidCursorError(response.cursor))))


Nitpick: multiple negatives make this conditional a bit confusing to read, consider:

(response.cursor && !validateCursor(response.cursor)) ? Promise.reject(..) : response

If there's no cursor or if the cursor is not valid

versus

If there's a cursor and the cursor is not valid

erquhart · 2018-05-16T21:05:36Z

src/components/Collection/Entries/EntriesCollection.js

+  getCursorActionFunctions = (cursor, handler) => cursor && cursor.actions
+    ? cursor.actions.reduce((acc, action) => ({ ...acc, [action]: handler(action) }), {})
+    : {};
+


Cursor interaction functions like the ones added to this file should be moved to the Cursor.js library and imported.

erquhart · 2018-05-16T21:07:24Z

src/components/Collection/Entries/EntriesCollection.js

@@ -62,11 +78,15 @@ function mapStateToProps(state, ownProps) {
  const entries = selectEntries(state, collection.get('name'));
  const isFetching = state.entries.getIn(['pages', collection.get('name'), 'isFetching'], false);

-  return { publicFolder, collection, page, entries, isFetching, viewStyle };
+  const rawCursor = state.cursors.get(collectionEntriesCursorKey(collection.get('name')));
+  const cursor = rawCursor && pick(rawCursor, ["meta", "actions"]);


This logic could probably be moved to a selector in the Cursor reducer.

jaredmorgs · 2018-05-25T03:49:11Z

Very excited to see this PR move forward to merged, so the BitBucket work can flow on from there. When Netlify supports all three major VCS for the CMS side, it will be a superb day for all.

Keep up the great work!

Benaiah · 2018-05-25T21:04:58Z

This PR is now ready for review! Notable changes for those who have already looked at the code prior:

The old style of pagination (which simply uses integer page numbers) is turned into cursors at the root of the entry list (EntriesCollection or EntriesSearch), so other UI elements deal only with cursors.
Cursors have been entirely rewritten, and now use a fluent (but immutable) class-based API.

Benaiah · 2018-06-06T23:33:23Z

This PR has been updated with preliminary git-gateway support for GitLab, pending netlify/git-gateway#17.

verythorough

I added a checkbox to the task list to add docs before this gets merged. I'm happy to make the necessary updates and PR them against this branch, but I haven't had a chance to test the GitLab backend myself, so I need to figure out if there's anything different in implementation. (@erquhart has offered to look into this.)

I'm also checking into doc updtes for git-gateway, both at the repo level and in Netlify's docs.

src/backends/gitlab/AuthenticationPage.js

+    if (authType === "implicit") {
+      this.auth = new ImplicitAuthenticator({
+        auth_url: this.props.config.getIn(['backend', 'auth_url'], 'https://gitlab.com/oauth/authorize'),
+        appid: this.props.config.getIn(['backend', 'appid']),


Benaiah · 2018-06-07T21:08:15Z

appid has been changed to app_id, and the fixes from #1093 have been copied over to the GitLab implementation (thanks for the catch @tech4him1).

tech4him1 · 2018-06-08T00:47:06Z

src/backends/gitlab/implementation.js

+      throw new Error("The GitLab backend does not support the Editorial Workflow.")
+    }
+
+    if (!options.proxied && config.getIn(["backend", "repo"]) == null) {


Shouldn't this be this.options.proxied?

Good point - options should also have a default value of {} (or L17 should do a null check before using the spread operator).

verythorough · 2018-06-08T02:45:23Z

Docs PR, against this branch: #1413

tech4him1 · 2018-06-12T00:14:19Z

src/lib/implicit-oauth.js

-    this.auth_url = config.auth_url;
+    const baseURL = trimEnd(config.base_url, '/');
+    const authEndpoint = trim(config.auth_endpoint, '/');
+    this.auth_url = `${ config.base_url }/${ config.auth_endpoint }`;


Typo here -- we want to use the trimmed values, not the ones from the config.

mryanb · 2018-06-12T22:33:56Z

Through some testing today, we did run into a scenario with a protected branch. The user made a change and published, it stated that the publish was successful but nothing happened. Upon further review it was discovered the branch was protected and the user didn't have permission to push. This error should probably be surfaced to the user.

erquhart · 2018-06-13T00:15:57Z

@mryanb GitLab support is merged and released now, can you open a new issue for this bug? Want to make sure it gets addressed.

_{Sent with GitHawk}

Benaiah commented May 7, 2018

View reviewed changes

Benaiah mentioned this pull request May 7, 2018

(WIP) Add GitLab Backend #517

Closed

10 tasks

Benaiah force-pushed the gitlab-benaiah-rebased-onto-cursor-api branch 2 times, most recently from be2aeed to 61fef08 Compare May 8, 2018 05:34

Benaiah force-pushed the gitlab-benaiah-rebased-onto-cursor-api branch 2 times, most recently from cb0c93e to d7bedc9 Compare May 8, 2018 22:31

Benaiah mentioned this pull request May 9, 2018

WIP: Implicit OAuth for GitLab #1314

Closed

talves self-requested a review May 10, 2018 20:46

erquhart mentioned this pull request May 14, 2018

(WIP) Bitbucket integration #525

Closed

Benaiah force-pushed the gitlab-benaiah-rebased-onto-cursor-api branch from ac0b0da to a35dd3d Compare May 15, 2018 22:44

erquhart suggested changes May 16, 2018

View reviewed changes

Benaiah force-pushed the gitlab-benaiah-rebased-onto-cursor-api branch 2 times, most recently from be8e092 to c125416 Compare May 18, 2018 21:29

Benaiah mentioned this pull request May 22, 2018

WIP - Cursor API #1301

Closed

3 tasks

Benaiah force-pushed the gitlab-benaiah-rebased-onto-cursor-api branch from c125416 to cce0e3c Compare May 22, 2018 17:19

verythorough mentioned this pull request May 24, 2018

Gitlab as backend. #57

Closed

Benaiah force-pushed the gitlab-benaiah-rebased-onto-cursor-api branch 2 times, most recently from ed05f5f to f7fb1dc Compare May 25, 2018 20:58

Benaiah changed the title ~~WIP: GitLab backend built with cursor API~~ GitLab backend built with cursor API May 25, 2018

Benaiah and others added 3 commits June 6, 2018 16:13

Filter folders from file-listing functions in GitLab API

3bfe9e3

print error messages in error boundary component

bf410a4

Git Gateway: lazy loading and GitLab support

40438e8

Benaiah force-pushed the gitlab-benaiah-rebased-onto-cursor-api branch from 7e05ec3 to 40438e8 Compare June 6, 2018 23:18

verythorough suggested changes Jun 7, 2018

View reviewed changes

Benaiah commented Jun 7, 2018

View reviewed changes

Benaiah added 2 commits June 7, 2018 13:32

Rename appid to app_id

28d6ce5

GitLab: fix collection failure if entry loading fails

1669f29

tech4him1 reviewed Jun 8, 2018

View reviewed changes

Benaiah and others added 4 commits June 8, 2018 15:09

Update default git-gateway endpoint

0fb3ebf

Fix instantiation of proxied backend implementation classes

c4c403e

Add missing auth_endpoint to GitLab's Netlify auth.

16ebe26

Remove console.logs

3678e5d

Benaiah mentioned this pull request Jun 10, 2018

Gitlab backport to v1 #1421

Closed

Fix integrations pagination

c92e39e

tech4him1 reviewed Jun 12, 2018

View reviewed changes

Benaiah added 2 commits June 11, 2018 17:41

Change auth_url to base_url+auth_endpoint for GitLab implicit auth

117e8b1

Fix querying through integrations

d86c610

Benaiah force-pushed the gitlab-benaiah-rebased-onto-cursor-api branch from 844912f to d86c610 Compare June 12, 2018 00:43

erquhart approved these changes Jun 12, 2018

View reviewed changes

erquhart merged commit b65f68e into master Jun 12, 2018

erquhart pushed a commit to erquhart/netlify-cms that referenced this pull request Jun 12, 2018

GitLab backend built with cursor API (decaporg#1343)

c0dc8f3

tech4him1 deleted the gitlab-benaiah-rebased-onto-cursor-api branch July 24, 2018 14:00

tech4him1 added a commit to tech4him1/netlify-cms that referenced this pull request Aug 1, 2018

chore(lint): cleanup from decaporg#1343

0f01b9c

tech4him1 added a commit to tech4him1/netlify-cms that referenced this pull request Aug 1, 2018

chore(lint): cleanup from decaporg#1343

ee5d7a3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitLab backend built with cursor API #1343

GitLab backend built with cursor API #1343

Benaiah commented May 7, 2018 •

edited by verythorough

Loading

verythorough commented May 7, 2018 •

edited

Loading

Benaiah May 7, 2018

erquhart May 16, 2018

Benaiah May 7, 2018

Benaiah May 7, 2018

Benaiah May 7, 2018 •

edited

Loading

Benaiah May 7, 2018 •

edited

Loading

Benaiah May 7, 2018

verythorough commented May 8, 2018 •

edited

Loading

Benaiah commented May 9, 2018

talves commented May 10, 2018 •

edited

Loading

erquhart left a comment •

edited

Loading

erquhart May 16, 2018

erquhart May 16, 2018

erquhart May 16, 2018

jaredmorgs commented May 25, 2018

Benaiah commented May 25, 2018

Benaiah commented Jun 6, 2018

verythorough left a comment

This comment was marked as resolved.

Benaiah commented Jun 7, 2018

tech4him1 Jun 8, 2018

Benaiah Jun 8, 2018

verythorough commented Jun 8, 2018

tech4him1 Jun 12, 2018

mryanb commented Jun 12, 2018

erquhart commented Jun 13, 2018

GitLab backend built with cursor API #1343

GitLab backend built with cursor API #1343

Conversation

Benaiah commented May 7, 2018 • edited by verythorough Loading

verythorough commented May 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Benaiah May 7, 2018 • edited Loading

Choose a reason for hiding this comment

Benaiah May 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

verythorough commented May 8, 2018 • edited Loading

Benaiah commented May 9, 2018

talves commented May 10, 2018 • edited Loading

erquhart left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredmorgs commented May 25, 2018

Benaiah commented May 25, 2018

Benaiah commented Jun 6, 2018

verythorough left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Benaiah commented Jun 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

verythorough commented Jun 8, 2018

Choose a reason for hiding this comment

mryanb commented Jun 12, 2018

erquhart commented Jun 13, 2018

Benaiah commented May 7, 2018 •

edited by verythorough

Loading

verythorough commented May 7, 2018 •

edited

Loading

Benaiah May 7, 2018 •

edited

Loading

Benaiah May 7, 2018 •

edited

Loading

verythorough commented May 8, 2018 •

edited

Loading

talves commented May 10, 2018 •

edited

Loading

erquhart left a comment •

edited

Loading