chillu commented 7 years ago

Overview

Since forms don't always have a 1:1 mapping to DataObjects, there's a need for a "form state" abstraction. While forms can be submitted through GraphQL (see #10 and #17), they're not returning direct GraphQL types in a form submission response. This means we can't rely on GraphQL's built-in cache updating logic.

If we separate form submissions from DataObject GraphQL types, we need way for the submission handling to indicate to the client which data has gone stale (either delivering new data in the response, or triggering a fetch from the client). The PoC simply does a refetch() of the GraphQL query bound to <AssetAdmin>, which only works because it happens to query data which is changed through the form (in order to list files and folders). That's not a strong correlation though, e.g. a ModelAdmin form builder component won't necessarily query the DataObject GraphQL type which is submitted through the form. The Apollo client does not have an API to identify which queries track a particular object as identified by its client-side Apollo identifier (see dataObjectFromId), so we don't know which queries would need to be retriggered. This also rules out the updateQueries and refetchQueries option in mutate().

Form submissions can't easily return arbitrary GraphQL representations of changed objects because there's no API to specifically update Apollo's stored objects outside of it's own query/mutation response processing. See updateQueries by id (dataIdFromObject) discussion. There's some ideas around a cache reducer to invalidate or update individual objects.

Note that this needs to be resolved even if we don't submit forms through GraphQL - the submission will still alter/invalidate existing GraphQL caches. For example, if we retrieve the page tree via GraphQL, publishing a page will alter the "published" status of its page tree node.

Where this is currently at: We're not going to be doing form submissions through GraphQL mutations at this stage due to larger form refactor (https://github.com/silverstripe/silverstripe-framework/issues/6362). The code for doing such mutations has been developed to a certain extent (#17) and could be used in the future if needed...

The decided approach for updating record data outside of using mutations is to have the FormSchema return a list of any modified records off the web server then use a reducer to re-perform any work on store. This will require changes to the FormSchema API.

Part 1 - FormSchema providing a list of changed records

The server needs to let Apollo know what typenames are new, deleted and updated.

"schema": {
     "fields": {
          // ...
    },
     "records": {
          "created": ['Member:2'],
          "changed": ['Group:1'],
          "deleted": []
    }
}

Initially this FormSchema API will be manual, developers will need to manually flag changed and created records, however this API would be the starting block and could be automated through DataObject::write, DataObject::delete.

interface GraphQLDocument {
public function getDataIdFromObject();
}

class DataObjectGraphQLExtension extends DataExtension implements GraphQLDocument {
    public function getDataIdFromObject() {
        return sprintf("%s:%s", $this->owner->ClassName, $this->owner->ID);
    }
}

$form->markRecordCreated(GraphQLDocument $record);
$form->markRecordDeleted(GraphQLDocument $record);
$form->markRecordChanged(GraphQLDocument $record);

Part 2 - GraphQL taking that information and updating it.

Given a list of changes off the server we need to update the React store outside of a mutation / updateQueries reducer. The store action changes which this requires is happening in Apollo at the moment (see https://github.com/apollostack/apollo-client/issues/951#issuecomment-266650406) however they say it's likely to be early 2017 before this is released. In the mean time, refetch() and manual updateQueries per query might be the way to go for asset admin. At the moment we can’t used updateQueries unless we perform the form submission through the mutation.

Tasks

[ ] Review https://github.com/apollostack/apollo-client/issues/951, once implemented then build.
[ ] PHP API.
[ ] Nomenclature for record vs document vs object? what is consistent in PHP API as this will be public.

chillu commented 7 years ago

I've read through https://github.com/apollostack/apollo-client/issues/951, lots of interesting discussion happening there!

The Apollo maintainers want to make the internal cache structure part of the API, which is encouraging for our use case (e.g. "find all queries related to data object id"). Many of the advanced caching strategies would be a "nice to have" for us, I'd expect most of our logic to simply call refetch() on relevant queries. Some queries might be expensive enough that advanced caching routines make sense - for example updating the page tree after saving a new page title.

The important issue for us is decoupling: Both individual queries and third party modules should be able to influence state and query invalidation based on our result objects returned from mutations. Here's a sketch what this could look like for a straight-up invalidation. I've changed the response signature to recordUpdatesById for simplicity (rather than created, updated and deleted), not too fussed though.

const saveFormMutation = gql`mutation saveForm($formId:!String, $data:FormData) {
  recordUpdatesById {
    type
    id
    updateType <-- created | updated | deleted
  }
}`;

const getPageTreeQuery = gql`query getPageTree() {
  treeNodes {
    id
    title
  }
}`;

function invalidateQuery(state, queryName) {
  const newState = state;
  // loop through state and delete all related entries
  // e.g. "$ROOT_QUERY.readFiles({id:0}).
  // Would require a higher level abstraction in Apollo
  return newState;
}

const cacheReducers = {
  // Could be registered by third party code, through a registry system
  pageTreeReducer: (args, prev, action) => {
    if (action.type === 'APOLLO_MUTATION_RESULT' && action.result.recordUpdatesById) {
      const hasApplicableRecords = action.result.recordUpdatesById.filter(update => type === 'Page').length;
      if (hasApplicableRecords) {
        // Invalidate queries by removing their data based on their name
        return invalidateQuery(prev, 'getPageTree')
      } else {
        return prev;
      }
    } else {
      return prev;
    }
  }
};

const client = new ApolloClient({
  cacheReducers,
});

Some potential use cases from the current UI implementation:

Current member ("Hello Ingo") needs to update on based on edits in admin/myprofile or admin/security
Page tree node needs to update based on page edits
Campaigns and Reports need to update based on all kinds of record edits in other sections (unless we hard refresh between sections)
Page tree needs to update based on batch actions (publish, delete)
Currently edited page needs to update based on batch actions (publish, delete) - modifying displayed fields, or closing the view in case of a delete
User needs to refresh group associations when adding/removing/editing groups

Overall, I think we can go a long way with strategically placed refetch() calls for now, particularly if we keep hard refresh between sections. The main restriction here is that components need to know about each other for this to work (child component causes a mutation, which calls a handler on parent component, which owns a related query and calls refetch())

chillu commented 7 years ago

Moving this out of the milestone since it's blocked by pending work in the upstream library. Craeted a spike card to reflect the effort involved so far: https://github.com/silverstripe/silverstripe-graphql/issues/33

silverstripe / silverstripe-graphql

Update GraphQL models after form submission #14

Overview

Related

Part 1 - FormSchema providing a list of changed records

Part 2 - GraphQL taking that information and updating it.

Tasks