Skip to content

Commit

Permalink
API Don't create "peripheral" operations by default
Browse files Browse the repository at this point in the history
When scaffolding Page with a read operation,
the default behaviour used to be the creation of readPage, readSiteTree, readRedirectorPage etc.
In fact, the readPage query is sufficient, since it returns a union of all these types. The rest is noise,
with the slight advantage that types at the end of an inheritance chain (without descendants)
get away without unions in the return type.

The default behaviour is a bit more useful on create and update operations,
since those require a specific input type - unions aren't allowed here.
So in order to fully create subclasses, you need createPage vs. createRedirectorPage.

I'm proposing that we make this well-documented opt-in behaviour,
with the admin/graphql schema opting in for each type on create and update.
I don't see a reason why anyone would want to opt in on read or delete.

For the more common case where scaffolding is used to create a more limited readonly public schema,
this reduces the mental overhead for devs, noise in the schema itself,
and cuts down schema generation time because less "helper types" like readRedirectorPageConnectionType are required.
  • Loading branch information
chillu committed Jan 30, 2019
1 parent bc5804d commit 365dd9d
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 28 deletions.
95 changes: 68 additions & 27 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1501,27 +1501,27 @@ SilverStripe\GraphQL\Manager:
##...
SilverStripe\CMS\Model\RedirectorPage:
fields: [ID, ExternalURL, Content]
Page:
fields: [MyCustomField]
operations:
read: true
create: true
Page:
fields: [MyCustomField]
```
**... Or with code**:
```php
$scaffolder
->type('SilverStripe\CMS\Model\RedirectorPage')
->addFields(['ID', 'ExternalURL', 'Content'])
->end()
->type('Page')
->addFields(['MyCustomField'])
->operation(SchemaScaffolder::READ)
->setName('readRedirectors')
->end()
->operation(SchemaScaffolder::CREATE)
->setName('createRedirector')
->end()
->end()
->type('Page')
->addFields(['MyCustomField'])
->end();
```

Expand Down Expand Up @@ -1554,44 +1554,84 @@ type SiteTreeWithDescendants {
SiteTree | Page | RedirectorPage
}

input RedirectorPageCreateInputType {
ExternalURL: String
RedirectionType: String
MyCustomField: String
Content: String
# all other fields from RedirectorPage, Page and SiteTree
}

input PageCreateInputType {
MyCustomField: String
Content: String
# all other fields from Page and SiteTree
}

input SiteTreeCreateInputType {
# all fields from SiteTree
query readPages {
PageWithDescendants
}

query readRedirectors {
RedirectorPage
mutation createPage {
PageCreateInputType
}
```

query readPages {
PageWithDescendants
By default, types are created for the full inheritance tree.
In certain situations, you may also want to create operations for all these ancestors and descendants.
For example, `createPage` only allows you to create an object of type `Page`.
In order to define a `createRedirectorPage` equivalent to create an object of type `RedirectorPage`,
you need to opt in via the `cloneable` configuration setting:


**Via YAML**:
```yaml
SilverStripe\GraphQL\Manager:
schemas:
default:
scaffolding:
types:
MyProject\Post:
##...
SilverStripe\CMS\Model\RedirectorPage:
fields: [ID, ExternalURL, Content]
Page:
fields: [MyCustomField]
operations:
read: true
create:
cloneable: true
```
**... Or with code**:
```php
$scaffolder
->type('SilverStripe\CMS\Model\RedirectorPage')
->addFields(['ID', 'ExternalURL', 'Content'])
->end()
->type('Page')
->addFields(['MyCustomField'])
->operation(SchemaScaffolder::READ)
->setName('readRedirectors')
->setCloneable(true)
->end()
->operation(SchemaScaffolder::CREATE)
->setName('createRedirector')
->end()
->end();
```

This will add the following fields to your schema:

```graphql
input RedirectorPageCreateInputType {
ExternalURL: String
RedirectionType: String
MyCustomField: String
Content: String
# all other fields from RedirectorPage, Page and SiteTree
}

query readSiteTrees {
SiteTreeWithDescendants
input SiteTreeCreateInputType {
# all fields from SiteTree
}

mutation createRedirector {
RedirectorPageCreateInputType
}

mutation createPage {
PageCreateInputType
}

mutation createSiteTree {
SiteTreeCreateInputType
}
Expand All @@ -1604,15 +1644,16 @@ when `RedirectorPage` is also exposed), a union is returned, and you will need t
with the `...on {type}` GraphQL syntax.

```graphql
query readSiteTrees {
readSiteTrees {
query readPages {
readPages {
edges {
node {
__typename
...on Page {
ID
}
...on RedirectorPage {
ID
RedirectionType
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/Scaffolding/Scaffolders/DataObjectScaffolder.php
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,10 @@ public function cloneTo(DataObjectScaffolder $target)
}
}
foreach ($this->getOperations() as $op) {
if (!$op->getCloneable()) {
continue;
}

$identifier = OperationScaffolder::getIdentifier($op);
$target->operation($identifier);
}
Expand Down
30 changes: 30 additions & 0 deletions src/Scaffolding/Scaffolders/OperationScaffolder.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,30 @@ abstract class OperationScaffolder implements ConfigurationApplier
*/
protected $args = [];

/**
* @var bool Indicates that this operation should be
* included in clones to other types.
*/
protected $cloneable = false;

/**
* @return bool
*/
public function getCloneable()
{
return $this->cloneable;
}

/**
* @param bool $cloneable
*/
public function setCloneable($cloneable)
{
$this->cloneable = $cloneable;

return $this;
}

/**
* @param string $name
* @return string|null
Expand Down Expand Up @@ -399,13 +423,19 @@ public function applyConfig(array $config)
}
}
}

if (isset($config['resolver'])) {
$this->setResolver($config['resolver']);
}

if (isset($config['name'])) {
$this->setName($config['name']);
}

if (isset($config['cloneable'])) {
$this->setCloneable((bool)$config['cloneable']);
}

return $this;
}

Expand Down
2 changes: 2 additions & 0 deletions src/Scaffolding/Scaffolders/SchemaScaffolder.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class SchemaScaffolder implements ManagerMutatorInterface
*/
public static function createFromConfig($config)
{
/** @var self $scaffolder */
$scaffolder = Injector::inst()->get(self::class);
if (isset($config['types'])) {
if (!ArrayLib::is_associative($config['types'])) {
Expand Down Expand Up @@ -284,6 +285,7 @@ public function getMutations()
public function addToManager(Manager $manager)
{
$this->registerFixedTypes($manager);

$this->registerPeripheralTypes($manager);

$this->extend('onBeforeAddToManager', $manager);
Expand Down
17 changes: 16 additions & 1 deletion tests/Scaffolding/Scaffolders/DataObjectScaffolderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,22 @@ public function testCloneTo()

$this->assertContains('Title', $target->getFields()->column('Name'));
$this->assertNotContains('RedirectionType', $target->getFields()->column('Name'));
$this->assertCount(2, $target->getOperations());
$this->assertCount(0, $target->getOperations(), 'Does not clone operations by default');
}

public function testCloneToWithCloneable()
{
$scaffolder = new DataObjectScaffolder(FakeSiteTree::class);
$scaffolder->addFields(['Title', 'ID']);
$scaffolder->operation(SchemaScaffolder::READ);
$scaffolder->operation(SchemaScaffolder::UPDATE)->setCloneable(true);

$target = new DataObjectScaffolder(FakeRedirectorPage::class);
$target = $scaffolder->cloneTo($target);
$this->assertEquals(
['updateSilverStripeFakeRedirectorPage'],
$target->getOperations()->column('getName')
);
}

/**
Expand Down

0 comments on commit 365dd9d

Please sign in to comment.