silverstripe / silverstripe-admin

Silverstripe Admin Component
BSD 3-Clause "New" or "Revised" License
25 stars 94 forks source link

Make all GraphQL core queries injectable #611

Closed chillu closed 6 years ago

chillu commented 6 years ago

Overview

We've done ground work on customiseable GraphQL queries a few months ago. The sole purpose of this was to make the CMS UI extensible. Yet somehow we ended up with AssetAdmin still having hardcoded queries (see readFilesQuery.js and AssetAdmin.js)

Extract from the ACs:

The following use cases are made possible through the API. This can be demonstrated by sample code and recipes rather than full implementations.

  • Show a "pending approval" indicator on a file gallery tile in asset admin

I'm wondering how this was achieved without making the readFiles query injectable?

Acceptance Criteria

Pull Requests

robbieaverill commented 6 years ago

Other examples of these types of queries:

These queries are all applied via registerTransforms.js injector calls for what it's worth.

bergice commented 6 years ago

Discussed this with @unclecheese.

He proposes that we move away from the DI on the front-end and instead move this to the back-end to be composed and presented using FormSchema to the front end as this should make it easier to extend and less abstract, but at the cost of coupling the front-end with the back-end.

The concern is that the way you would extend this using DI on the front-end would be too abstract and not able to scale.

We should look into a quick and clean way to make the asset-admin query extensible based on the requirements of the TNZ project as this should be the current priority.

unclecheese commented 6 years ago

POC is up here, (https://github.com/silverstripeltd/tourism1/tree/feature/injectable-queries) using two different forks -- one for admin/ and one for asset-admin/. There are bigger implications for how this will work that I've discussed with @chillu (out of scope for this card), but these changes get the story over the line.

unclecheese commented 6 years ago

Blocking based on all kinds of stuff up in the air on whether this is the right approach or not. Impossible to call this ready for peer review at this point, but done enough to satisfy edge case.

robbieaverill commented 6 years ago

I've reviewed all PRs and tested it out, all working nicely. Each PR could do with being rebased against 1.3 and have a good squash and re-label. I also expect someone else is going to review this as well =)

robbieaverill commented 6 years ago

All PRs merged