Added new enum-based sortBy query argument

keystonejs · Apr 6, 2020 · 208919b · 208919b
1 parent 660fb19
commit 208919b
Show file tree

Hide file tree

Showing 18 changed files with 140 additions and 50 deletions.
diff --git a/.changeset/stupid-ducks-explain.md b/.changeset/stupid-ducks-explain.md
@@ -0,0 +1,10 @@
+---
+'@keystonejs/adapter-knex': major
+'@keystonejs/adapter-mongoose': major
+'@keystonejs/keystone': major
+'@keystonejs/mongo-join-builder': major
+---
+
+Added new `sortBy` query argument.
+
+Each list now has an additional `Sort<List>By` enum type that refresents the valid sorting options for all orderable fields in the list. `sortBy` takes one or more of these enum types, allowing for multi-field/column sorting.
diff --git a/api-tests/queries/limits.test.js b/api-tests/queries/limits.test.js
@@ -67,7 +67,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
           query {
             allUsers(
               where: { name_contains: "J" },
-              orderBy: "name_ASC",
+              sortBy: name_ASC,
             ) {
               name
             }
@@ -232,10 +232,10 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
                   { title: "Two authors" },
                 ]
               }
-              orderBy: "title_ASC",
+              sortBy: title_ASC,
             ) {
               title
-              author(orderBy: "name_ASC") {
+              author(sortBy: name_ASC) {
                 name
               }
             }
@@ -319,7 +319,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
               title
               author {
                 posts {
-                  title 
+                  title
                 }
               }
             }

diff --git a/api-tests/queries/relationships.test.js b/api-tests/queries/relationships.test.js
@@ -180,7 +180,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
               feed_some: { title_contains: "J" }
             }) {
               id
-              feed(orderBy: "title_ASC") {
+              feed(sortBy: title_ASC) {
                 title
               }
             }
@@ -299,7 +299,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
             }) {
               id
               name
-              feed(orderBy: "title_ASC") {
+              feed(sortBy: title_ASC) {
                 id
                 title
               }
@@ -331,7 +331,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
             allUsers(where: {
               feed_none: { title_contains: "J" }
             },
-            orderBy: "id_ASC") {
+            sortBy: id_ASC) {
               id
               feed {
                 title

diff --git a/api-tests/relationships/filtering/nested.test.js b/api-tests/relationships/filtering/nested.test.js
@@ -100,7 +100,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
         query {
           allUsers {
             id
-            posts (first: 1, orderBy: "content_ASC") {
+            posts (first: 1, sortBy: content_ASC) {
               id
               content
             }

diff --git a/api-tests/relationships/nested-mutations/create-many.test.js b/api-tests/relationships/nested-mutations/create-many.test.js
@@ -82,7 +82,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
             notes: { create: [{ content: "${noteContent}" }] }
           }) {
             id
-            notes(orderBy: "content_ASC") {
+            notes(sortBy: content_ASC) {
               id
               content
             }
@@ -118,7 +118,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
             }
           }) {
             id
-            notes(orderBy: "content_ASC") {
+            notes(sortBy: content_ASC) {
               id
               content
             }
@@ -245,7 +245,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
             }
           ) {
             id
-            notes(orderBy: "content_ASC") {
+            notes(sortBy: content_ASC) {
               id
               content
             }

diff --git a/api-tests/relationships/nested-mutations/reconnect-many-to-one.test.js b/api-tests/relationships/nested-mutations/reconnect-many-to-one.test.js
@@ -46,7 +46,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
                   notes: { connect: [{ id: "${noteA.id}" }, { id: "${noteB.id}" }] }
                 }) {
                   id
-                  notes(orderBy: "title_ASC") { id title }
+                  notes(sortBy: title_ASC) { id title }
                 }
               }
           `,
@@ -60,7 +60,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
                   notes: { connect: [{ id: "${noteC.id}" }, { id: "${noteD.id}" }] }
                 }) {
                   id
-                  notes(orderBy: "title_ASC") { id title }
+                  notes(sortBy: title_ASC) { id title }
                 }
               }
           `,
@@ -82,7 +82,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
                   notes: { connect: [{ id: "${noteB.id}" }] }
                 }) {
                   id
-                  notes(orderBy: "title_ASC") { id title }
+                  notes(sortBy: title_ASC) { id title }
                 }
               }
           `,
@@ -119,7 +119,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
             {
               User(where: { id: "${alice.createUser.id}"}) {
                 id
-                notes(orderBy: "title_ASC") { id title }
+                notes(sortBy: title_ASC) { id title }
               }
             }
             `,

diff --git a/api-tests/relationships/nested-mutations/two-way-backreference/to-many.test.js b/api-tests/relationships/nested-mutations/two-way-backreference/to-many.test.js
@@ -204,7 +204,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
               }
             ) {
               id
-              teachers(orderBy: "id_ASC") {
+              teachers(sortBy: id_ASC) {
                 id
               }
             }

diff --git a/demo-projects/meetup/site/graphql/events.js b/demo-projects/meetup/site/graphql/events.js
@@ -30,13 +30,13 @@ export const GET_CURRENT_EVENTS = gql`
   query GetCurrentEvents($now: DateTime!) {
     upcomingEvents: allEvents(
       where: { startTime_not: null, status: active, startTime_gte: $now }
-      orderBy: "startTime_DESC"
+      sortBy: startTime_DESC
     ) {
       ...EventData
     }
     previousEvents: allEvents(
       where: { startTime_not: null, status: active, startTime_lte: $now }
-      orderBy: "startTime_ASC"
+      sortBy: startTime_ASC
     ) {
       ...EventData
     }
@@ -46,7 +46,7 @@ export const GET_CURRENT_EVENTS = gql`
 
 export const GET_ALL_EVENTS = gql`
   {
-    allEvents(where: { startTime_not: null, status: active }, orderBy: "startTime_DESC") {
+    allEvents(where: { startTime_not: null, status: active }, sortBy: startTime_DESC) {
       ...EventData
     }
   }

diff --git a/docs/guides/intro-to-graphql.md b/docs/guides/intro-to-graphql.md
@@ -224,7 +224,8 @@ When executing queries and mutations there are a number of ways you can filter,
 - `search`
 - `skip`
 - `first`
-- `orderby`
+- `sortBy`
+- `orderby` (deprecated)
 
 ### `where`
 
@@ -320,8 +321,22 @@ query {
 }
 ```
 
+### `sortBy`
+
+Order results. Accepts one or more list sort enums in the format `<field>_<ASC|DESC>`. For example, to order by name descending (alphabetical order, A -> Z):
+
+```graphql
+query {
+  allUsers(sortBy: name_DESC) {
+    id
+  }
+}
+```
+
 ### `orderBy`
 
+> **Warning:** This argument is deprecated. Use `sortBy` instead.
+
 Order results. The orderBy string should match the format `<field>_<ASC|DESC>`. For example, to order by name descending (alphabetical order, A -> Z):
 
 ```graphql

diff --git a/packages/adapter-knex/lib/adapter-knex.js b/packages/adapter-knex/lib/adapter-knex.js
@@ -484,7 +484,7 @@ class KnexListAdapter extends BaseListAdapter {
 class QueryBuilder {
   constructor(
     listAdapter,
-    { where = {}, first, skip, orderBy, search },
+    { where = {}, first, skip, sortBy, orderBy, search },
     { meta = false, from = {} }
   ) {
     this._tableAliases = {};
@@ -561,6 +561,17 @@ class QueryBuilder {
         const sortKey = listAdapter.fieldAdaptersByPath[orderField].sortKey || orderField;
         this._query.orderBy(sortKey, orderDirection);
       }
+      if (sortBy !== undefined) {
+        // SELECT ... ORDER BY <orderField>[, <orderField>, ...]
+        this._query.orderBy(
+          sortBy.map(s => {
+            const [orderField, orderDirection] = s.split('_');
+            const sortKey = listAdapter.fieldAdaptersByPath[orderField].sortKey || orderField;
+
+            return { column: sortKey, order: orderDirection };
+          })
+        );
+      }
     }
   }
 

diff --git a/packages/adapter-mongoose/lib/adapter-mongoose.js b/packages/adapter-mongoose/lib/adapter-mongoose.js
@@ -241,7 +241,10 @@ class MongooseListAdapter extends BaseListAdapter {
           ? graphQlQueryToMongoJoinQuery(whereElement) // Recursively traverse relationship fields
           : whereElement
       ),
-      ...mapKeyNames(pick(modifiers, ['search', 'orderBy', 'skip', 'first']), key => `$${key}`),
+      ...mapKeyNames(
+        pick(modifiers, ['search', 'sortBy', 'orderBy', 'skip', 'first']),
+        key => `$${key}`
+      ),
     });
     let query;
     try {

diff --git a/packages/fields/src/types/DateTime/test-fixtures.js b/packages/fields/src/types/DateTime/test-fixtures.js
@@ -199,11 +199,11 @@ export const filterTests = withKeystone => {
   );
 
   test(
-    'Sorting: orderBy: lastOnline_ASC',
+    'Sorting: sortBy: lastOnline_ASC',
     withKeystone(({ keystone, adapterName }) =>
       match(
         keystone,
-        'orderBy: "lastOnline_ASC"',
+        'sortBy: lastOnline_ASC',
         adapterName === 'mongoose'
           ? [
               { name: 'person5', lastOnline: null },
@@ -225,11 +225,11 @@ export const filterTests = withKeystone => {
   );
 
   test(
-    'Sorting: orderBy: lastOnline_DESC',
+    'Sorting: sortBy: lastOnline_DESC',
     withKeystone(({ keystone, adapterName }) =>
       match(
         keystone,
-        'orderBy: "lastOnline_DESC"',
+        'sortBy: lastOnline_DESC',
         adapterName === 'mongoose'
           ? [
               { name: 'person2', lastOnline: '2000-01-20T00:08:00.000+10:00' },

diff --git a/packages/keystone/lib/List/index.js b/packages/keystone/lib/List/index.js
@@ -186,6 +186,7 @@ module.exports = class List {
       listQueryName: `all${_listQueryName}`,
       listQueryMetaName: `_all${_listQueryName}Meta`,
       listMetaName: preventInvalidUnderscorePrefix(`_${_listQueryName}Meta`),
+      listSortName: `Sort${_listQueryName}By`,
       deleteMutationName: `delete${_itemQueryName}`,
       updateMutationName: `update${_itemQueryName}`,
       createMutationName: `create${_itemQueryName}`,
@@ -356,10 +357,18 @@ module.exports = class List {
       .filter(field => field.access[schemaName][access]); // If it's globally set to false, makes sense to never let it be updated
   }
 
+  /** Equivalent to getFieldsWithAccess but includes `id` fields. */
+  getAllFieldsWithAccess({ schemaName, access }) {
+    return this.fields.filter(field => field.access[schemaName][access]);
+  }
+
   getGqlTypes({ schemaName }) {
     const schemaAccess = this.access[schemaName];
-    // https://github.com/opencrud/opencrud/blob/master/spec/2-relational/2-2-queries/2-2-3-filters.md#boolean-expressions
     const types = [];
+
+    // We want to include `id` fields
+    // If read is globally set to false, makes sense to never show it
+    const readFields = this.getAllFieldsWithAccess({ schemaName, access: 'read' });
     if (
       schemaAccess.read ||
       schemaAccess.create ||
@@ -381,33 +390,43 @@ module.exports = class List {
           """
           _label_: String
           ${flatten(
-            this.fields
-              .filter(field => field.access[schemaName].read) // If it's globally set to false, makes sense to never show it
-              .map(field =>
-                field.schemaDoc
-                  ? `""" ${field.schemaDoc} """ ${field.gqlOutputFields({ schemaName })}`
-                  : field.gqlOutputFields({ schemaName })
-              )
+            readFields.map(field =>
+              field.schemaDoc
+                ? `""" ${field.schemaDoc} """ ${field.gqlOutputFields({ schemaName })}`
+                : field.gqlOutputFields({ schemaName })
+            )
           ).join('\n')}
-        }
-      `,
+        }`,
+
+        // https://github.com/opencrud/opencrud/blob/master/spec/2-relational/2-2-queries/2-2-3-filters.md#boolean-expressions
         `
         input ${this.gqlNames.whereInputName} {
           AND: [${this.gqlNames.whereInputName}]
           OR: [${this.gqlNames.whereInputName}]
 
-          ${flatten(
-            this.fields
-              .filter(field => field.access[schemaName].read) // If it's globally set to false, makes sense to never show it
-              .map(field => field.gqlQueryInputFields({ schemaName }))
-          ).join('\n')}
+          ${flatten(readFields.map(field => field.gqlQueryInputFields({ schemaName }))).join('\n')}
         }`,
         // TODO: Include other `unique` fields and allow filtering by them
         `
         input ${this.gqlNames.whereUniqueInputName} {
           id: ID!
         }`
       );
+
+      const sortOptions = flatten(
+        readFields.map(({ path, isOrderable }) =>
+          // Explicitly allow sorting by id
+          isOrderable || path === 'id' ? [`${path}_ASC`, `${path}_DESC`] : []
+        )
+      );
+
+      if (sortOptions.length) {
+        types.push(`
+          enum ${this.gqlNames.listSortName} {
+            ${sortOptions.join('\n')}
+          }
+        `);
+      }
     }
 
     const updateFields = this.getFieldsWithAccess({ schemaName, access: 'update' });
@@ -446,6 +465,7 @@ module.exports = class List {
     return [
       `where: ${this.gqlNames.whereInputName}`,
       `search: String`,
+      `sortBy: [${this.gqlNames.listSortName}!]`,
       `orderBy: String`,
       `first: Int`,
       `skip: Int`,
-Original file line number
+Diff line change
@@ Expand Up / @@ -100,7 +100,7 @@ multiAdapterRunners().map(({ runner, adapterName }) => @@
             query {
               allUsers {
                 id
-                posts (first: 1, orderBy: "content_ASC") {
+                posts (first: 1, sortBy: content_ASC) {
                   id
                   content
                 }
@@ Expand Down @@