From e114894d1bbcea8940cf14486fc336aa8d112da7 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Tue, 7 Jul 2020 11:01:28 +1000 Subject: [PATCH 1/7] Factor out list utils (#3234) --- .changeset/lucky-jokes-warn.md | 5 ++ packages/keystone/lib/ListTypes/list.js | 95 +++--------------------- packages/keystone/lib/ListTypes/utils.js | 95 ++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 85 deletions(-) create mode 100644 .changeset/lucky-jokes-warn.md create mode 100644 packages/keystone/lib/ListTypes/utils.js diff --git a/.changeset/lucky-jokes-warn.md b/.changeset/lucky-jokes-warn.md new file mode 100644 index 00000000000..80236304c43 --- /dev/null +++ b/.changeset/lucky-jokes-warn.md @@ -0,0 +1,5 @@ +--- +'@keystonejs/keystone': patch +--- + +Factored out utility `List` functions into a separate module. diff --git a/packages/keystone/lib/ListTypes/list.js b/packages/keystone/lib/ListTypes/list.js index 8e9da992091..4103bb2f494 100644 --- a/packages/keystone/lib/ListTypes/list.js +++ b/packages/keystone/lib/ListTypes/list.js @@ -12,100 +12,25 @@ const { flatten, zipObj, createLazyDeferred, - upcase, } = require('@keystonejs/utils'); const { parseListAccess } = require('@keystonejs/access-control'); const { logger } = require('@keystonejs/logger'); - -const graphqlLogger = logger('graphql'); -const keystoneLogger = logger('keystone'); - +const { + preventInvalidUnderscorePrefix, + keyToLabel, + labelToPath, + labelToClass, + opToType, + mapNativeTypeToKeystoneType, + getDefautlLabelResolver, +} = require('./utils'); const { LimitsExceededError, ValidationFailureError, throwAccessDenied, } = require('./graphqlErrors'); -const preventInvalidUnderscorePrefix = str => str.replace(/^__/, '_'); - -const keyToLabel = str => { - let label = str - .replace(/([a-z])([A-Z])/g, '$1 $2') - .split(/\s|_|\-/) - .filter(i => i) - .map(upcase) - .join(' '); - - // Retain the leading underscore for auxiliary lists - if (str[0] === '_') { - label = `_${label}`; - } - return label; -}; - -const labelToPath = str => - str - .split(' ') - .join('-') - .toLowerCase(); - -const labelToClass = str => str.replace(/\s+/g, ''); - -const opToType = { - read: 'query', - create: 'mutation', - update: 'mutation', - delete: 'mutation', -}; - -const mapNativeTypeToKeystoneType = (type, listKey, fieldPath) => { - const { Text, Checkbox, Float } = require('@keystonejs/fields'); - - const nativeTypeMap = new Map([ - [ - Boolean, - { - name: 'Boolean', - keystoneType: Checkbox, - }, - ], - [ - String, - { - name: 'String', - keystoneType: Text, - }, - ], - [ - Number, - { - name: 'Number', - keystoneType: Float, - }, - ], - ]); - - if (!nativeTypeMap.has(type)) { - return type; - } - - const { name, keystoneType } = nativeTypeMap.get(type); - - keystoneLogger.warn( - { nativeType: type, keystoneType, listKey, fieldPath }, - `Mapped field ${listKey}.${fieldPath} from native JavaScript type '${name}', to '${keystoneType.type.type}' from the @keystonejs/fields package.` - ); - - return keystoneType; -}; - -const getDefautlLabelResolver = labelField => item => { - const value = item[labelField || 'name']; - if (typeof value === 'number') { - return value.toString(); - } - return value || item.id; -}; +const graphqlLogger = logger('graphql'); module.exports = class List { constructor( diff --git a/packages/keystone/lib/ListTypes/utils.js b/packages/keystone/lib/ListTypes/utils.js new file mode 100644 index 00000000000..5380dc9d708 --- /dev/null +++ b/packages/keystone/lib/ListTypes/utils.js @@ -0,0 +1,95 @@ +const { upcase } = require('@keystonejs/utils'); +const { logger } = require('@keystonejs/logger'); + +const keystoneLogger = logger('keystone'); + +const preventInvalidUnderscorePrefix = str => str.replace(/^__/, '_'); + +const keyToLabel = str => { + let label = str + .replace(/([a-z])([A-Z])/g, '$1 $2') + .split(/\s|_|\-/) + .filter(i => i) + .map(upcase) + .join(' '); + + // Retain the leading underscore for auxiliary lists + if (str[0] === '_') { + label = `_${label}`; + } + return label; +}; + +const labelToPath = str => + str + .split(' ') + .join('-') + .toLowerCase(); + +const labelToClass = str => str.replace(/\s+/g, ''); + +const opToType = { + read: 'query', + create: 'mutation', + update: 'mutation', + delete: 'mutation', +}; + +const mapNativeTypeToKeystoneType = (type, listKey, fieldPath) => { + const { Text, Checkbox, Float } = require('@keystonejs/fields'); + + const nativeTypeMap = new Map([ + [ + Boolean, + { + name: 'Boolean', + keystoneType: Checkbox, + }, + ], + [ + String, + { + name: 'String', + keystoneType: Text, + }, + ], + [ + Number, + { + name: 'Number', + keystoneType: Float, + }, + ], + ]); + + if (!nativeTypeMap.has(type)) { + return type; + } + + const { name, keystoneType } = nativeTypeMap.get(type); + + keystoneLogger.warn( + { nativeType: type, keystoneType, listKey, fieldPath }, + `Mapped field ${listKey}.${fieldPath} from native JavaScript type '${name}', to '${keystoneType.type.type}' from the @keystonejs/fields package.` + ); + + return keystoneType; +}; + +const getDefautlLabelResolver = labelField => item => { + const value = item[labelField || 'name']; + if (typeof value === 'number') { + return value.toString(); + } + return value || item.id; +}; + +module.exports = { + preventInvalidUnderscorePrefix, + keyToLabel, + labelToPath, + labelToClass, + opToType, + mapNativeTypeToKeystoneType, + getDefautlLabelResolver, +}; From caf3021295c4136ea48585d1687ad6fbf58ed4aa Mon Sep 17 00:00:00 2001 From: Amandeep Singh Date: Tue, 7 Jul 2020 12:42:20 +1000 Subject: [PATCH 2/7] Upgrade meetup demo usage of NextJS latest pattern (#3229) Ref: https://github.com/vercel/next.js/blob/canary/examples/with-apollo The summary of the changes are: - Less boilerplate code for setting/initializing Apollo Client - Update home, events and about page to opt in for SSG - Removed MyApp.getIntialProps as it will stop the Auto Static optimization. Ref: (https://nextjs.org/docs/api-reference/data-fetching/getInitialProps#caveats) - Handle the auth loading client side before checking of authentication Co-authored-by: Aman Singh --- .changeset/light-llamas-wonder.md | 11 +++++ .../meetup/site/components/Navbar.js | 6 +-- demo-projects/meetup/site/lib/apolloClient.js | 42 +++++++++++++++++++ demo-projects/meetup/site/pages/_app.js | 37 +++------------- demo-projects/meetup/site/pages/about.js | 16 +++++++ demo-projects/meetup/site/pages/events.js | 15 +++++++ demo-projects/meetup/site/pages/index.js | 21 ++++++++-- 7 files changed, 111 insertions(+), 37 deletions(-) create mode 100644 .changeset/light-llamas-wonder.md create mode 100644 demo-projects/meetup/site/lib/apolloClient.js diff --git a/.changeset/light-llamas-wonder.md b/.changeset/light-llamas-wonder.md new file mode 100644 index 00000000000..63a1fcc1b01 --- /dev/null +++ b/.changeset/light-llamas-wonder.md @@ -0,0 +1,11 @@ +--- +'@keystonejs/demo-project-meetup': patch +--- + +Updated usage of Apollo based on Next.js [example](https://github.com/vercel/next.js/blob/canary/examples/with-apollo). + +The **key changes** are: +- Less boilerplate code for setting/initializing Apollo Client +- Update home, events and about page to opt in for SSG +- Removed MyApp.getIntialProps as it will stop the [Auto Static optimization](https://nextjs.org/docs/api-reference/data-fetching/getInitialProps#caveats). +- Handle the authentication in client-side only. diff --git a/demo-projects/meetup/site/components/Navbar.js b/demo-projects/meetup/site/components/Navbar.js index 6964c6af54b..85a923a3cc3 100644 --- a/demo-projects/meetup/site/components/Navbar.js +++ b/demo-projects/meetup/site/components/Navbar.js @@ -6,7 +6,7 @@ import { jsx } from '@emotion/core'; import Link from 'next/link'; import { useAuth } from '../lib/authentication'; -import { SignoutIcon } from '../primitives'; +import { SignoutIcon, Loading } from '../primitives'; import { getForegroundColor, useLogoDimension } from '../helpers'; import { mq } from '../helpers/media'; import { fontSizes, gridSize, shadows } from '../theme'; @@ -143,7 +143,7 @@ const AnonActions = () => { }; const Navbar = ({ background = 'white', ...props }) => { - const { isAuthenticated, user } = useAuth(); + const { isAuthenticated, user, isLoading } = useAuth(); const { logoWidth, logoHeight, logoWidthSm, logoHeightSm } = useLogoDimension(); const foreground = getForegroundColor(background); @@ -171,7 +171,7 @@ const Navbar = ({ background = 'white', ...props }) => { About Events - {isAuthenticated ? : } + {isLoading ? : isAuthenticated ? : } ); diff --git a/demo-projects/meetup/site/lib/apolloClient.js b/demo-projects/meetup/site/lib/apolloClient.js new file mode 100644 index 00000000000..794ad661646 --- /dev/null +++ b/demo-projects/meetup/site/lib/apolloClient.js @@ -0,0 +1,42 @@ +// Derived from https://github.com/vercel/next.js/blob/canary/examples/with-apollo/lib/apolloClient.js +import { useMemo } from 'react'; +import { ApolloClient } from 'apollo-client'; +import { InMemoryCache } from 'apollo-cache-inmemory'; +import { createUploadLink } from 'apollo-upload-client'; + +let apolloClient; + +function createApolloClient(req) { + return new ApolloClient({ + ssrMode: typeof window === 'undefined', + link: createUploadLink({ + // TODO: server-side requests must have an absolute URI. We should find a way + // to make this part of the project config, seems highly opinionated here + uri: 'http://localhost:3000/admin/api', + credentials: 'same-origin', // Additional fetch() options like `credentials` or `headers` + headers: req && req.headers, + }), + cache: new InMemoryCache(), + }); +} + +export function initializeApollo(initialState = null, req) { + const _apolloClient = apolloClient ?? createApolloClient(req); + + // If your page has Next.js data fetching methods that use Apollo Client, the initial state + // gets hydrated here + if (initialState) { + _apolloClient.cache.restore(initialState); + } + // For SSG and SSR always create a new Apollo Client + if (typeof window === 'undefined') return _apolloClient; + // Create the Apollo Client once in the client + if (!apolloClient) apolloClient = _apolloClient; + + return _apolloClient; +} + +export function useApollo(initialState) { + const store = useMemo(() => initializeApollo(initialState), [initialState]); + return store; +} diff --git a/demo-projects/meetup/site/pages/_app.js b/demo-projects/meetup/site/pages/_app.js index d18676489bb..2623f22ecdd 100644 --- a/demo-projects/meetup/site/pages/_app.js +++ b/demo-projects/meetup/site/pages/_app.js @@ -1,19 +1,17 @@ import React from 'react'; import Head from 'next/head'; -import gql from 'graphql-tag'; -import { ApolloProvider } from '@apollo/react-hooks'; import { ToastProvider } from 'react-toast-notifications'; - -import withApollo from '../lib/withApollo'; import { AuthProvider } from '../lib/authentication'; import StylesBase from '../primitives/StylesBase'; import GoogleAnalytics from '../components/GoogleAnalytics'; - -const MyApp = ({ Component, pageProps, apolloClient, user }) => { +import { useApollo } from '../lib/apolloClient'; +import { ApolloProvider } from '@apollo/react-hooks'; +const MyApp = ({ Component, pageProps }) => { + const apolloClient = useApollo(pageProps.initialApolloState); return ( - + { ); }; -MyApp.getInitialProps = async ({ Component, ctx }) => { - let pageProps = {}; - - const data = await ctx.apolloClient.query({ - query: gql` - query { - authenticatedUser { - id - name - isAdmin - } - } - `, - fetchPolicy: 'network-only', - }); - - if (Component.getInitialProps) { - pageProps = await Component.getInitialProps(ctx); - } - - return { pageProps, user: data.data ? data.data.authenticatedUser : undefined }; -}; - -export default withApollo(MyApp); +export default MyApp; diff --git a/demo-projects/meetup/site/pages/about.js b/demo-projects/meetup/site/pages/about.js index 711b017a01b..370142dcac4 100644 --- a/demo-projects/meetup/site/pages/about.js +++ b/demo-projects/meetup/site/pages/about.js @@ -11,6 +11,7 @@ import Meta from '../components/Meta'; import { colors, gridSize } from '../theme'; import { GET_ORGANISERS } from '../graphql/organisers'; import { mq } from '../helpers/media'; +import { initializeApollo } from '../lib/apolloClient'; const { publicRuntimeConfig } = getConfig(); @@ -121,3 +122,18 @@ const Organiser = ({ organiser }) => ( ); const Content = props =>
; + +export async function getStaticProps() { + const apolloClient = initializeApollo(); + + await apolloClient.query({ + query: GET_ORGANISERS, + }); + + return { + props: { + initialApolloState: apolloClient.cache.extract(), + }, + unstable_revalidate: 1, + }; +} diff --git a/demo-projects/meetup/site/pages/events.js b/demo-projects/meetup/site/pages/events.js index b7481f2f943..911432745a8 100644 --- a/demo-projects/meetup/site/pages/events.js +++ b/demo-projects/meetup/site/pages/events.js @@ -10,6 +10,7 @@ import Navbar from '../components/Navbar'; import Footer from '../components/Footer'; import Meta from '../components/Meta'; import { gridSize } from '../theme'; +import { initializeApollo } from '../lib/apolloClient'; import { GET_ALL_EVENTS } from '../graphql/events'; @@ -38,3 +39,17 @@ export default function Events() { ); } +export async function getStaticProps() { + const apolloClient = initializeApollo(); + + await apolloClient.query({ + query: GET_ALL_EVENTS, + }); + + return { + props: { + initialApolloState: apolloClient.cache.extract(), + }, + unstable_revalidate: 1, + }; +} diff --git a/demo-projects/meetup/site/pages/index.js b/demo-projects/meetup/site/pages/index.js index 347be873377..946b59d2a79 100644 --- a/demo-projects/meetup/site/pages/index.js +++ b/demo-projects/meetup/site/pages/index.js @@ -11,6 +11,7 @@ import Meta from '../components/Meta'; import { GET_CURRENT_EVENTS } from '../graphql/events'; import { GET_EVENT_RSVPS } from '../graphql/rsvps'; import { GET_SPONSORS } from '../graphql/sponsors'; +import { initializeApollo } from '../lib/apolloClient'; import Talks from '../components/Talks'; import Rsvp from '../components/Rsvp'; @@ -328,9 +329,23 @@ const Home = ({ now }) => { ); }; -Home.getInitialProps = async () => ({ - now: new Date().toISOString(), -}); +export async function getStaticProps() { + const apolloClient = initializeApollo(); + + const now = new Date().toISOString(); + await apolloClient.query({ + query: GET_CURRENT_EVENTS, + variables: { now }, + }); + + return { + props: { + initialApolloState: apolloClient.cache.extract(), + now, + }, + unstable_revalidate: 1, + }; +} export default Home; From 06f86c6f5c573411f0efda565a269d1d7ccb3c66 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Tue, 7 Jul 2020 13:14:21 +1000 Subject: [PATCH 3/7] Update access control functions (#3233) --- .changeset/pretty-radios-obey.md | 5 +++ packages/keystone/lib/Keystone/index.js | 17 +++++++--- packages/keystone/lib/ListTypes/list.js | 36 +++++++++++++++------ packages/keystone/lib/providers/listAuth.js | 5 ++- packages/keystone/tests/List.test.js | 5 +-- 5 files changed, 50 insertions(+), 18 deletions(-) create mode 100644 .changeset/pretty-radios-obey.md diff --git a/.changeset/pretty-radios-obey.md b/.changeset/pretty-radios-obey.md new file mode 100644 index 00000000000..eb80163cd1f --- /dev/null +++ b/.changeset/pretty-radios-obey.md @@ -0,0 +1,5 @@ +--- +'@keystonejs/keystone': patch +--- + +Updated internal access control functions to directly accept access control definition. diff --git a/packages/keystone/lib/Keystone/index.js b/packages/keystone/lib/Keystone/index.js index 1d056f289f9..4bc1a4efee6 100644 --- a/packages/keystone/lib/Keystone/index.js +++ b/packages/keystone/lib/Keystone/index.js @@ -128,9 +128,15 @@ module.exports = class Keystone { ); const getListAccessControlForUser = memoize( - async (listKey, originalInput, operation, { gqlName, itemId, itemIds, context } = {}) => { + async ( + access, + listKey, + originalInput, + operation, + { gqlName, itemId, itemIds, context } = {} + ) => { return validateListAccessControl({ - access: this.lists[listKey].access[schemaName], + access: access[schemaName], originalInput, operation, authentication, @@ -146,6 +152,7 @@ module.exports = class Keystone { const getFieldAccessControlForUser = memoize( async ( + access, listKey, fieldKey, originalInput, @@ -154,7 +161,7 @@ module.exports = class Keystone { { gqlName, itemId, itemIds, context } = {} ) => { return validateFieldAccessControl({ - access: this.lists[listKey].fieldsByPath[fieldKey].access[schemaName], + access: access[schemaName], originalInput, existingItem, operation, @@ -171,9 +178,9 @@ module.exports = class Keystone { ); const getAuthAccessControlForUser = memoize( - async (listKey, { gqlName, context } = {}) => { + async (access, listKey, { gqlName, context } = {}) => { return validateAuthAccessControl({ - access: this.lists[listKey].access[schemaName], + access: access[schemaName], authentication, listKey, gqlName, diff --git a/packages/keystone/lib/ListTypes/list.js b/packages/keystone/lib/ListTypes/list.js index 4103bb2f494..8458d97c0d6 100644 --- a/packages/keystone/lib/ListTypes/list.js +++ b/packages/keystone/lib/ListTypes/list.js @@ -302,6 +302,7 @@ module.exports = class List { // Check access const operation = 'read'; const access = await context.getFieldAccessControlForUser( + field.access, this.key, field.path, undefined, @@ -335,6 +336,7 @@ module.exports = class List { for (const field of fields) { const access = await context.getFieldAccessControlForUser( + field.access, this.key, field.path, data, @@ -355,11 +357,17 @@ module.exports = class List { } async checkListAccess(context, originalInput, operation, { gqlName, ...extraInternalData }) { - const access = await context.getListAccessControlForUser(this.key, originalInput, operation, { - gqlName, - context, - ...extraInternalData, - }); + const access = await context.getListAccessControlForUser( + this.access, + this.key, + originalInput, + operation, + { + gqlName, + context, + ...extraInternalData, + } + ); if (!access) { graphqlLogger.debug( { operation, access, gqlName, ...extraInternalData }, @@ -1341,14 +1349,22 @@ module.exports = class List { // declarative syntax) getAccess: () => ({ getCreate: () => - context.getListAccessControlForUser(this.key, undefined, 'create', { context }), + context.getListAccessControlForUser(this.access, this.key, undefined, 'create', { + context, + }), getRead: () => - context.getListAccessControlForUser(this.key, undefined, 'read', { context }), + context.getListAccessControlForUser(this.access, this.key, undefined, 'read', { + context, + }), getUpdate: () => - context.getListAccessControlForUser(this.key, undefined, 'update', { context }), + context.getListAccessControlForUser(this.access, this.key, undefined, 'update', { + context, + }), getDelete: () => - context.getListAccessControlForUser(this.key, undefined, 'delete', { context }), - getAuth: () => context.getAuthAccessControlForUser(this.key, { context }), + context.getListAccessControlForUser(this.access, this.key, undefined, 'delete', { + context, + }), + getAuth: () => context.getAuthAccessControlForUser(this.access, this.key, { context }), }), getSchema: () => { diff --git a/packages/keystone/lib/providers/listAuth.js b/packages/keystone/lib/providers/listAuth.js index bfee9af35ff..cfbc9e4e1fb 100644 --- a/packages/keystone/lib/providers/listAuth.js +++ b/packages/keystone/lib/providers/listAuth.js @@ -138,7 +138,10 @@ class ListAuthProvider { async checkAccess(context, type, { gqlName }) { const operation = 'auth'; - const access = await context.getAuthAccessControlForUser(this.list.key, { gqlName, context }); + const access = await context.getAuthAccessControlForUser(this.list.access, this.list.key, { + gqlName, + context, + }); if (!access) { graphqlLogger.debug({ operation, access, gqlName }, 'Access statically or implicitly denied'); graphqlLogger.info({ operation, gqlName }, 'Access Denied'); diff --git a/packages/keystone/tests/List.test.js b/packages/keystone/tests/List.test.js index bd98a331ece..504a7c76358 100644 --- a/packages/keystone/tests/List.test.js +++ b/packages/keystone/tests/List.test.js @@ -25,7 +25,7 @@ Relationship.adapters['mock'] = {}; const context = { getListAccessControlForUser: () => true, - getFieldAccessControlForUser: (listKey, fieldPath, originalInput, existingItem) => + getFieldAccessControlForUser: (access, listKey, fieldPath, originalInput, existingItem) => !(existingItem && existingItem.makeFalse && fieldPath === 'name'), getAuthAccessControlForUser: () => true, authedItem: { @@ -786,7 +786,8 @@ test('checkListAccess', async () => { const newContext = { ...context, - getListAccessControlForUser: (listKey, originalInput, operation) => operation === 'update', + getListAccessControlForUser: (access, listKey, originalInput, operation) => + operation === 'update', }; await expect( list.checkListAccess(newContext, originalInput, 'update', { gqlName: 'testing' }) From 5ad84ccd8d008188e293629e90a4d7e7fde55333 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Tue, 7 Jul 2020 13:43:49 +1000 Subject: [PATCH 4/7] Factor out hooks handling code (#3235) --- .changeset/big-llamas-sparkle.md | 5 + packages/keystone/lib/ListTypes/hooks.js | 166 ++++++++++++++ packages/keystone/lib/ListTypes/list.js | 266 +++++++---------------- packages/keystone/lib/ListTypes/utils.js | 13 +- 4 files changed, 258 insertions(+), 192 deletions(-) create mode 100644 .changeset/big-llamas-sparkle.md create mode 100644 packages/keystone/lib/ListTypes/hooks.js diff --git a/.changeset/big-llamas-sparkle.md b/.changeset/big-llamas-sparkle.md new file mode 100644 index 00000000000..e88322b78ac --- /dev/null +++ b/.changeset/big-llamas-sparkle.md @@ -0,0 +1,5 @@ +--- +'@keystonejs/keystone': patch +--- + +Refactored hook management into a separate module. diff --git a/packages/keystone/lib/ListTypes/hooks.js b/packages/keystone/lib/ListTypes/hooks.js new file mode 100644 index 00000000000..2122e187057 --- /dev/null +++ b/packages/keystone/lib/ListTypes/hooks.js @@ -0,0 +1,166 @@ +const { omitBy, arrayToObject } = require('@keystonejs/utils'); +const { mapToFields } = require('./utils'); +const { ValidationFailureError } = require('./graphqlErrors'); + +class HookManager { + constructor({ fields, hooks, listKey }) { + this.fields = fields; + this.hooks = hooks; + this.listKey = listKey; + this.fieldsByPath = arrayToObject(this.fields, 'path'); + } + + _fieldsFromObject(obj) { + return Object.keys(obj) + .map(fieldPath => this.fieldsByPath[fieldPath]) + .filter(field => field); + } + + _throwValidationFailure({ errors, operation, originalInput }) { + throw new ValidationFailureError({ + data: { + messages: errors.map(e => e.msg), + errors: errors.map(e => e.data), + listKey: this.listKey, + operation, + }, + internalData: { errors: errors.map(e => e.internalData), data: originalInput }, + }); + } + + async resolveInput({ resolvedData, existingItem, context, operation, originalInput }) { + const args = { resolvedData, existingItem, context, originalInput, operation }; + + // First we run the field type hooks + // NOTE: resolveInput is run on _every_ field, regardless if it has a value + // passed in or not + resolvedData = await mapToFields(this.fields, field => field.resolveInput(args)); + + // We then filter out the `undefined` results (they should return `null` or + // a value) + resolvedData = omitBy(resolvedData, key => typeof resolvedData[key] === 'undefined'); + + // Run the schema-level field hooks, passing in the results from the field + // type hooks + resolvedData = { + ...resolvedData, + ...(await mapToFields( + this.fields.filter(field => field.hooks.resolveInput), + field => field.hooks.resolveInput({ ...args, resolvedData }) + )), + }; + + // And filter out the `undefined`s again. + resolvedData = omitBy(resolvedData, key => typeof resolvedData[key] === 'undefined'); + + if (this.hooks.resolveInput) { + // And run any list-level hook + resolvedData = await this.hooks.resolveInput({ ...args, resolvedData }); + if (typeof resolvedData !== 'object') { + throw new Error( + `Expected ${ + this.listKey + }.hooks.resolveInput() to return an object, but got a ${typeof resolvedData}: ${resolvedData}` + ); + } + } + + // Finally returning the amalgamated result of all the hooks. + return resolvedData; + } + + async validateInput({ resolvedData, existingItem, context, operation, originalInput }) { + const args = { resolvedData, existingItem, context, originalInput, operation }; + // Check for isRequired + const fieldValidationErrors = this.fields + .filter( + field => + field.isRequired && + !field.isRelationship && + ((operation === 'create' && + (resolvedData[field.path] === undefined || resolvedData[field.path] === null)) || + (operation === 'update' && + Object.prototype.hasOwnProperty.call(resolvedData, field.path) && + (resolvedData[field.path] === undefined || resolvedData[field.path] === null))) + ) + .map(f => ({ + msg: `Required field "${f.path}" is null or undefined.`, + data: { resolvedData, operation, originalInput }, + internalData: {}, + })); + if (fieldValidationErrors.length) { + this._throwValidationFailure({ errors: fieldValidationErrors, operation, originalInput }); + } + + const fields = this._fieldsFromObject(resolvedData); + await this._validateHook({ args, fields, operation, hookName: 'validateInput' }); + } + + async validateDelete({ existingItem, context, operation }) { + const args = { existingItem, context, operation }; + const fields = this.fields; + await this._validateHook({ args, fields, operation, hookName: 'validateDelete' }); + } + + async _validateHook({ args, fields, operation, hookName }) { + const { originalInput } = args; + const fieldValidationErrors = []; + // FIXME: Can we do this in a way where we simply return validation errors instead? + args.addFieldValidationError = (msg, _data = {}, internalData = {}) => + fieldValidationErrors.push({ msg, data: _data, internalData }); + await mapToFields(fields, field => field[hookName](args)); + await mapToFields( + fields.filter(field => field.hooks[hookName]), + field => field.hooks[hookName](args) + ); + if (fieldValidationErrors.length) { + this._throwValidationFailure({ errors: fieldValidationErrors, operation, originalInput }); + } + + if (this.hooks[hookName]) { + const listValidationErrors = []; + await this.hooks[hookName]({ + ...args, + addValidationError: (msg, _data = {}, internalData = {}) => + listValidationErrors.push({ msg, data: _data, internalData }), + }); + if (listValidationErrors.length) { + this._throwValidationFailure({ errors: listValidationErrors, operation, originalInput }); + } + } + } + + async beforeChange({ resolvedData, existingItem, context, operation, originalInput }) { + const args = { resolvedData, existingItem, context, originalInput, operation }; + await this._runHook({ args, fieldObject: resolvedData, hookName: 'beforeChange' }); + } + + async beforeDelete({ existingItem, context, operation }) { + const args = { existingItem, context, operation }; + await this._runHook({ args, fieldObject: existingItem, hookName: 'beforeDelete' }); + } + + async afterChange({ updatedItem, existingItem, context, operation, originalInput }) { + const args = { updatedItem, originalInput, existingItem, context, operation }; + await this._runHook({ args, fieldObject: updatedItem, hookName: 'afterChange' }); + } + + async afterDelete({ existingItem, context, operation }) { + const args = { existingItem, context, operation }; + await this._runHook({ args, fieldObject: existingItem, hookName: 'afterDelete' }); + } + + async _runHook({ args, fieldObject, hookName }) { + // Used to apply hooks that only produce side effects + const fields = this._fieldsFromObject(fieldObject); + await mapToFields(fields, field => field[hookName](args)); + await mapToFields( + fields.filter(field => field.hooks[hookName]), + field => field.hooks[hookName](args) + ); + + if (this.hooks[hookName]) await this.hooks[hookName](args); + } +} + +module.exports = { HookManager }; diff --git a/packages/keystone/lib/ListTypes/list.js b/packages/keystone/lib/ListTypes/list.js index 8458d97c0d6..cf9f9cd0b61 100644 --- a/packages/keystone/lib/ListTypes/list.js +++ b/packages/keystone/lib/ListTypes/list.js @@ -1,6 +1,5 @@ const pluralize = require('pluralize'); const { - resolveAllKeys, mapKeys, omit, omitBy, @@ -8,7 +7,6 @@ const { intersection, mergeWhereClause, objMerge, - arrayToObject, flatten, zipObj, createLazyDeferred, @@ -23,12 +21,10 @@ const { opToType, mapNativeTypeToKeystoneType, getDefautlLabelResolver, + mapToFields, } = require('./utils'); -const { - LimitsExceededError, - ValidationFailureError, - throwAccessDenied, -} = require('./graphqlErrors'); +const { HookManager } = require('./hooks'); +const { LimitsExceededError, throwAccessDenied } = require('./graphqlErrors'); const graphqlLogger = logger('graphql'); @@ -58,7 +54,7 @@ module.exports = class List { ) { this.key = key; this._fields = fields; - this.hooks = hooks; + this._hooks = hooks; this.schemaDoc = schemaDoc; this.adminDoc = adminDoc; @@ -225,6 +221,11 @@ module.exports = class List { this.views = mapKeys(sanitisedFieldsConfig, ({ type }, path) => this.fieldsByPath[path].extendAdminViews({ ...type.views }) ); + this.hookManager = new HookManager({ + fields: this.fields, + hooks: this._hooks, + listKey: this.key, + }); } getAdminMeta({ schemaName }) { @@ -611,32 +612,6 @@ module.exports = class List { } // Mutation resolvers - _throwValidationFailure(errors, operation, data = {}) { - throw new ValidationFailureError({ - data: { - messages: errors.map(e => e.msg), - errors: errors.map(e => e.data), - listKey: this.key, - operation, - }, - internalData: { - errors: errors.map(e => e.internalData), - data, - }, - }); - } - - _mapToFields(fields, action) { - return resolveAllKeys(arrayToObject(fields, 'path', action)).catch(error => { - if (!error.errors) { - throw error; - } - const errorCopy = new Error(error.message || error.toString()); - errorCopy.errors = Object.values(error.errors); - throw errorCopy; - }); - } - _fieldsFromObject(obj) { return Object.keys(obj) .map(fieldPath => this.fieldsByPath[fieldPath]) @@ -645,7 +620,7 @@ module.exports = class List { async _resolveRelationship(data, existingItem, context, getItem, mutationState) { const fields = this._fieldsFromObject(data).filter(field => field.isRelationship); - const resolvedRelationships = await this._mapToFields(fields, async field => { + const resolvedRelationships = await mapToFields(fields, async field => { const { create, connect, disconnect, currentValue } = await field.resolveNestedOperations( data[field.path], existingItem, @@ -688,7 +663,7 @@ module.exports = class List { field => typeof originalInput[field.path] === 'undefined' ); - const defaultValues = await this._mapToFields(fieldsWithoutValues, field => + const defaultValues = await mapToFields(fieldsWithoutValues, field => field.getDefaultValue(args) ); @@ -698,140 +673,6 @@ module.exports = class List { }; } - async _resolveInput(resolvedData, existingItem, context, operation, originalInput) { - const args = { resolvedData, existingItem, context, originalInput, operation }; - - // First we run the field type hooks - // NOTE: resolveInput is run on _every_ field, regardless if it has a value - // passed in or not - resolvedData = await this._mapToFields(this.fields, field => field.resolveInput(args)); - - // We then filter out the `undefined` results (they should return `null` or - // a value) - resolvedData = omitBy(resolvedData, key => typeof resolvedData[key] === 'undefined'); - - // Run the schema-level field hooks, passing in the results from the field - // type hooks - resolvedData = { - ...resolvedData, - ...(await this._mapToFields( - this.fields.filter(field => field.hooks.resolveInput), - field => field.hooks.resolveInput({ ...args, resolvedData }) - )), - }; - - // And filter out the `undefined`s again. - resolvedData = omitBy(resolvedData, key => typeof resolvedData[key] === 'undefined'); - - if (this.hooks.resolveInput) { - // And run any list-level hook - resolvedData = await this.hooks.resolveInput({ ...args, resolvedData }); - if (typeof resolvedData !== 'object') { - throw new Error( - `Expected ${ - this.key - }.hooks.resolveInput() to return an object, but got a ${typeof resolvedData}: ${resolvedData}` - ); - } - } - - // Finally returning the amalgamated result of all the hooks. - return resolvedData; - } - - async _validateInput(resolvedData, existingItem, context, operation, originalInput) { - const args = { resolvedData, existingItem, context, originalInput, operation }; - // Check for isRequired - const fieldValidationErrors = this.fields - .filter( - field => - field.isRequired && - !field.isRelationship && - ((operation === 'create' && - (resolvedData[field.path] === undefined || resolvedData[field.path] === null)) || - (operation === 'update' && - Object.prototype.hasOwnProperty.call(resolvedData, field.path) && - (resolvedData[field.path] === undefined || resolvedData[field.path] === null))) - ) - .map(f => ({ - msg: `Required field "${f.path}" is null or undefined.`, - data: { resolvedData, operation, originalInput }, - internalData: {}, - })); - if (fieldValidationErrors.length) { - this._throwValidationFailure(fieldValidationErrors, operation, originalInput); - } - - const fields = this._fieldsFromObject(resolvedData); - await this._validateHook(args, fields, operation, 'validateInput'); - } - - async _validateDelete(existingItem, context, operation) { - const args = { existingItem, context, operation }; - const fields = this.fields; - await this._validateHook(args, fields, operation, 'validateDelete'); - } - - async _validateHook(args, fields, operation, hookName) { - const { originalInput } = args; - const fieldValidationErrors = []; - // FIXME: Can we do this in a way where we simply return validation errors instead? - args.addFieldValidationError = (msg, _data = {}, internalData = {}) => - fieldValidationErrors.push({ msg, data: _data, internalData }); - await this._mapToFields(fields, field => field[hookName](args)); - await this._mapToFields( - fields.filter(field => field.hooks[hookName]), - field => field.hooks[hookName](args) - ); - if (fieldValidationErrors.length) { - this._throwValidationFailure(fieldValidationErrors, operation, originalInput); - } - - if (this.hooks[hookName]) { - const listValidationErrors = []; - await this.hooks[hookName]({ - ...args, - addValidationError: (msg, _data = {}, internalData = {}) => - listValidationErrors.push({ msg, data: _data, internalData }), - }); - if (listValidationErrors.length) { - this._throwValidationFailure(listValidationErrors, operation, originalInput); - } - } - } - - async _beforeChange(resolvedData, existingItem, context, operation, originalInput) { - const args = { resolvedData, existingItem, context, originalInput, operation }; - await this._runHook(args, resolvedData, 'beforeChange'); - } - - async _beforeDelete(existingItem, context, operation) { - const args = { existingItem, context, operation }; - await this._runHook(args, existingItem, 'beforeDelete'); - } - - async _afterChange(updatedItem, existingItem, context, operation, originalInput) { - const args = { updatedItem, originalInput, existingItem, context, operation }; - await this._runHook(args, updatedItem, 'afterChange'); - } - - async _afterDelete(existingItem, context, operation) { - const args = { existingItem, context, operation }; - await this._runHook(args, existingItem, 'afterDelete'); - } - - async _runHook(args, fieldObject, hookName) { - // Used to apply hooks that only produce side effects - const fields = this._fieldsFromObject(fieldObject); - await this._mapToFields(fields, field => field[hookName](args)); - await this._mapToFields( - fields.filter(field => field.hooks[hookName]), - field => field.hooks[hookName](args) - ); - - if (this.hooks[hookName]) await this.hooks[hookName](args); - } - async _nestedMutation(mutationState, context, mutation) { // Set up a fresh mutation state if we're the root mutation const isRootMutation = !mutationState; @@ -909,22 +750,34 @@ module.exports = class List { mutationState ); - resolvedData = await this._resolveInput( + resolvedData = await this.hookManager.resolveInput({ resolvedData, existingItem, context, operation, - originalInput - ); + originalInput, + }); - await this._validateInput(resolvedData, existingItem, context, operation, originalInput); + await this.hookManager.validateInput({ + resolvedData, + existingItem, + context, + operation, + originalInput, + }); - await this._beforeChange(resolvedData, existingItem, context, operation, originalInput); + await this.hookManager.beforeChange({ + resolvedData, + existingItem, + context, + operation, + originalInput, + }); - let newItem; + let updatedItem; try { - newItem = await this.adapter.create(resolvedData); - createdPromise.resolve(newItem); + updatedItem = await this.adapter.create(resolvedData); + createdPromise.resolve(updatedItem); // Wait until next tick so the promise/micro-task queue can be flushed // fully, ensuring the deferred handlers get executed before we move on await new Promise(res => process.nextTick(res)); @@ -938,9 +791,15 @@ module.exports = class List { } return { - result: newItem, + result: updatedItem, afterHook: () => - this._afterChange(newItem, existingItem, context, operation, originalInput), + this.hookManager.afterChange({ + updatedItem, + existingItem, + context, + operation, + originalInput, + }), }; }); } @@ -991,28 +850,53 @@ module.exports = class List { ); } - async _updateSingle(id, data, existingItem, context, mutationState) { + async _updateSingle(id, originalInput, existingItem, context, mutationState) { const operation = 'update'; return await this._nestedMutation(mutationState, context, async mutationState => { let resolvedData = await this._resolveRelationship( - data, + originalInput, existingItem, context, undefined, mutationState ); - resolvedData = await this._resolveInput(resolvedData, existingItem, context, operation, data); + resolvedData = await this.hookManager.resolveInput({ + resolvedData, + existingItem, + context, + operation, + originalInput, + }); - await this._validateInput(resolvedData, existingItem, context, operation, data); + await this.hookManager.validateInput({ + resolvedData, + existingItem, + context, + operation, + originalInput, + }); - await this._beforeChange(resolvedData, existingItem, context, operation, data); + await this.hookManager.beforeChange({ + resolvedData, + existingItem, + context, + operation, + originalInput, + }); - const newItem = await this.adapter.update(id, resolvedData); + const updatedItem = await this.adapter.update(id, resolvedData); return { - result: newItem, - afterHook: () => this._afterChange(newItem, existingItem, context, operation, data), + result: updatedItem, + afterHook: () => + this.hookManager.afterChange({ + updatedItem, + existingItem, + context, + operation, + originalInput, + }), }; }); } @@ -1055,15 +939,15 @@ module.exports = class List { const operation = 'delete'; return await this._nestedMutation(mutationState, context, async () => { - await this._validateDelete(existingItem, context, operation); + await this.hookManager.validateDelete({ existingItem, context, operation }); - await this._beforeDelete(existingItem, context, operation); + await this.hookManager.beforeDelete({ existingItem, context, operation }); await this.adapter.delete(existingItem.id); return { result: existingItem, - afterHook: () => this._afterDelete(existingItem, context, operation), + afterHook: () => this.hookManager.afterDelete({ existingItem, context, operation }), }; }); } diff --git a/packages/keystone/lib/ListTypes/utils.js b/packages/keystone/lib/ListTypes/utils.js index 5380dc9d708..f731b8c0788 100644 --- a/packages/keystone/lib/ListTypes/utils.js +++ b/packages/keystone/lib/ListTypes/utils.js @@ -1,4 +1,4 @@ -const { upcase } = require('@keystonejs/utils'); +const { upcase, resolveAllKeys, arrayToObject } = require('@keystonejs/utils'); const { logger } = require('@keystonejs/logger'); const keystoneLogger = logger('keystone'); @@ -84,6 +84,16 @@ const getDefautlLabelResolver = labelField => item => { return value || item.id; }; +const mapToFields = (fields, action) => + resolveAllKeys(arrayToObject(fields, 'path', action)).catch(error => { + if (!error.errors) { + throw error; + } + const errorCopy = new Error(error.message || error.toString()); + errorCopy.errors = Object.values(error.errors); + throw errorCopy; + }); + module.exports = { preventInvalidUnderscorePrefix, keyToLabel, @@ -92,4 +102,5 @@ module.exports = { opToType, mapNativeTypeToKeystoneType, getDefautlLabelResolver, + mapToFields, }; From 61cdafe20e0a22b5a1f9b6a2dcc4aefa45a26902 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Wed, 8 Jul 2020 15:14:01 +1000 Subject: [PATCH 5/7] Fix existingItem in updateManyMutation hooks (#3241) --- .changeset/famous-pumpkins-raise.md | 5 +++++ packages/keystone/lib/ListTypes/list.js | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 .changeset/famous-pumpkins-raise.md diff --git a/.changeset/famous-pumpkins-raise.md b/.changeset/famous-pumpkins-raise.md new file mode 100644 index 00000000000..c288a88100a --- /dev/null +++ b/.changeset/famous-pumpkins-raise.md @@ -0,0 +1,5 @@ +--- +'@keystonejs/keystone': patch +--- + +Fixed a bug where `existingItem` had the wrong value in hooks during an `updateManyMutation`. diff --git a/packages/keystone/lib/ListTypes/list.js b/packages/keystone/lib/ListTypes/list.js index cf9f9cd0b61..3e75b137bbc 100644 --- a/packages/keystone/lib/ListTypes/list.js +++ b/packages/keystone/lib/ListTypes/list.js @@ -10,6 +10,7 @@ const { flatten, zipObj, createLazyDeferred, + arrayToObject, } = require('@keystonejs/utils'); const { parseListAccess } = require('@keystonejs/access-control'); const { logger } = require('@keystonejs/logger'); @@ -833,9 +834,10 @@ module.exports = class List { const access = await this.checkListAccess(context, data, operation, extraData); const existingItems = await this.getAccessControlledItems(ids, access); + const existingItemsById = arrayToObject(existingItems, 'id'); const itemsToUpdate = zipObj({ - existingItem: existingItems, + existingItem: ids.map(id => existingItemsById[id]), id: ids, // itemId is taken from here in checkFieldAccess data: data.map(d => d.data), }); From 65659e657f515a7618b9e7192d867068c50508a0 Mon Sep 17 00:00:00 2001 From: Matheus Chimelli Date: Wed, 8 Jul 2020 21:33:53 -0300 Subject: [PATCH 6/7] Docs/Passport: Add a how to use the serviceProfile (#3105) * Add a how to use the serviceProfile Add a how to use the serviceProfile on resolveCreateData to save the data provided on the auth provider * Fix formatting on example Co-authored-by: Jess Telford --- packages/auth-passport/README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/auth-passport/README.md b/packages/auth-passport/README.md index 54aa03649dc..d22490c207a 100644 --- a/packages/auth-passport/README.md +++ b/packages/auth-passport/README.md @@ -307,7 +307,23 @@ keystone process.exit(1); }); ``` +Sometimes you just need to get the profile data provided by the auth provider. In that case, you +have to call the `resolveCreateData` with `serviceProfile` property. See an example below: +```javascript +// Here we wait for our serviceProfile to get the data provided by the auth provider +resolveCreateData: ({ createData, serviceProfile }) => { + // Once we had our seviceProfile, we set it on our list fields + + // Google will return the profile data inside the _json key + // Each provider can return the user profile data in a different way. + // Check how it's returned on your provider documentation + createData.name = serviceProfile._json.name + createData.profilePicture = serviceProfile._json.picture + + return createData; +} +``` ## Using other Passport strategies You can create your own strategies to work with Keystone by extending the From 391b7d0702c542d7a127c9e7adba5603d9238b93 Mon Sep 17 00:00:00 2001 From: "allcontributors[bot]" <46447321+allcontributors[bot]@users.noreply.github.com> Date: Thu, 9 Jul 2020 10:34:37 +1000 Subject: [PATCH 7/7] docs: add matheuschimelli as a contributor (#3244) * docs: update CONTRIBUTING.md [skip ci] * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> --- .all-contributorsrc | 9 +++++++++ CONTRIBUTING.md | 6 +----- README.md | 6 +----- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index 13f7396fe3c..8e8264cc564 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -438,6 +438,15 @@ "contributions": [ "code" ] + }, + { + "login": "matheuschimelli", + "name": "Matheus Chimelli", + "avatar_url": "https://avatars0.githubusercontent.com/u/10470074?v=4", + "profile": "https://github.com/matheuschimelli", + "contributions": [ + "doc" + ] } ], "contributorsPerLine": 7, diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a27c64e734..ed6bfa3e9ab 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -275,11 +275,8 @@ We also build commonjs builds to run in node (for testing with jest or etc.) and Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/docs/en/emoji-key)): - - - @@ -340,13 +337,12 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d +

Jed Watson
💻
frank10gm
💻
mbrodt
📖
Misha Zamkevich
💻
Matheus Chimelli
📖
- - This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! diff --git a/README.md b/README.md index 21769a77de5..f5ef33fbb61 100644 --- a/README.md +++ b/README.md @@ -77,11 +77,8 @@ We'd like to start by thanking all our wonderful contributors: ([emoji key](https://allcontributors.org/docs/en/emoji-key)): - - - @@ -142,13 +139,12 @@ We'd like to start by thanking all our wonderful contributors: +

Jed Watson
💻
frank10gm
💻
mbrodt
📖
Misha Zamkevich
💻
Matheus Chimelli
📖
- - ### Demo Projects