Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TreeDropdownField should use a generic GraphQL endpoint #2271

Open
chillu opened this issue Sep 21, 2018 · 0 comments
Open

TreeDropdownField should use a generic GraphQL endpoint #2271

chillu opened this issue Sep 21, 2018 · 0 comments

Comments

@chillu
Copy link
Member

chillu commented Sep 21, 2018

Overview

Routes within form fields (nested controllers) are inherently slow, since all the controller and form state up to this point needs to be recreated in order to execute the logic of the innermost controller. Depending on your form design and grid field use, this could create five forms with dozens of fields each before the actually desired logic kicks in. The best example of this is autocomplete in TagField, which we can't make fast because of this. Another example is TreeDropdownField.

There's two advantages to this nested controller approach in form fields:

  • Self-contained logic in one form field class
  • Access control linked to containing form(s): If you can't get to the form field, you can't execute the logic. We heavily rely on this in GridField for access control, which can often be different to the base canEdit() checks - e.g. through conditionals in getCMSFields().

In the medium term, we'll replace the tree with a React+GraphQL implementation, which has it's own challenges. In the short term, we could move towards this by rewriting TreeDropdownField to use a more limited version of this GraphQL feature (readonly).

Acceptance Criteria

  • TreeDropdownField uses a generic GraphQL endpoint for tree navigation and search results
  • The API works with multiple nesting levels starting from arbitrary root nodes
  • The API respects node_threshold
  • The API works with all objects implementing Hierarchy
  • The API can indicate if more records are available, allowing frontends to display "show more" hints

Notes

  • Ideally we can make this work through the standard GraphQL endpoints, e.g. readSiteTree
  • Search results on generic objects will require a standardized search params approach in GraphQL (rather than one-off implementations like readFiles)
  • It's not feasible to return arbitrarily nested data via GraphQL - you have to ask for it in the GraphQL query. If the client wants three levels, they have to repeat the same fields across three levels of GraphQL and their children fields. This allows clients to choose if they go flat or deep, but it means the server can't be proactive in sending deeply nested data if it would fit into the node_threshold
  • We'll likely cache GraphQL tree data in localStorage, which reduces the need to download large and deeply nested trees upfront. The client can be smarter about when and how to fetch data, leading to perceived performance increases.
  • The current search results UI shows a flat list (with breadcrumbs), rather than a tree. Building trees seems to be more expensive through the current MarkedSet approach, compared to simply listing results and computing their breadcrumbs. Returning arbitrarily deeply nested search results is also not feasible in a GraphQL implementation (see above) - we likely need to have a different GraphQL endpoint for searching.
  • Discussion about representing trees in GraphQL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants