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

NEW: DBField arguments, Enum, Composite support #398

Merged
merged 14 commits into from
Sep 6, 2021

Conversation

unclecheese
Copy link

@unclecheese unclecheese commented Jul 29, 2021

Resolves

Summary

This pull request adds three new plugins to better support DBField types.

Example

query ReadPage {
  readOnePage {
    content(format: FIRST_PARAGRAPH)
    introText(format: LIMIT_SENTENCES, limit: 5)
    lastEdited(format: NICE)
    publishDate(format: CUSTOM, customFormat: "YYYY")
  }
}

DBFieldArgsPlugin

  • Adds a new format argument to DBField types that have formatting methods, e.g. FirstParagraph. (content(format: FIRST_PARAGRAPH) )
  • When appropriate, a limit argument is added, e.g. content(format: LIMIT_SENTENCES, limit: 5)
  • New format argument for date / time fields, e.g. lastEdited(format: DAY_OF_WEEK)
  • All up, there are nine of these adapters (Text, Decimal, and Date ancestries)

DBFieldTypes plugin

  • DBEnum creates a proper Enum type in the schema, and gets assigned to the field
  • Composite fields, e.g. DBFile create proper object types in the schema and get assigned to the field

ScalarDBField plugin

  • Ensure DBFields are always returned as scalars (no longer done in the default DataObject resolver. see below)

New low-level stuff

  • Default DataObject resolver no longer normalises to scalars: The above changes required changing the default DataObject resolver to no longer return $dbField->getValue() (scalar) and rather leave the instance in tact so the other resolvers could act upon it (e.g. $obj->FirstParagraph()). There is now a plugin that runs at the end of the resolver stack that ensures if the object is DBField, it will run ->forTemplate() on it to extract its scalar value

  • ModelField now accepts a metadata property. This may be a bit overkill, but we needed a place to store state on how the field was created, e.g. from introspection of a DBField. (->getMetadata()->get('dataClass')). As yet there are no other uses for this storage, but it's conceivable that other plugins or future features will want to add arbitrary state to fields.

  • Schema config now has a parseShortcodes property as a global setting for DBHTMLText fields. (false in admin, true in default). This can also be set per field in myHTMLField(parseShortcodes: false)

  • Type DBFile no longer exists. It's created by the DBFieldTypes plugin automatically as DBFileComposite

Docs

silverstripe/silverstripe-framework#10039

Relevant, but not necessarily dependent

@unclecheese
Copy link
Author

Failed build is due to a branch pushed to origin that was later deleted.

Copy link
Member

@emteknetnz emteknetnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good

I don't have enough context to really challenge the approach taken here, so I'll just assume it's a great idea :-)

Allowing php formatting to be specified via a graphql query is certainly interesting. It does seem slightly wrong since usually that's supposed to happen client side, though I personally think it's a good convenience and it facilitates code reuse.

I've left some comments around some opportunities to tidy up the code

src/Schema/DataObject/DataObjectModel.php Outdated Show resolved Hide resolved
src/Schema/DataObject/Plugin/ScalarDBField.php Outdated Show resolved Hide resolved
src/Schema/DataObject/Resolver.php Outdated Show resolved Hide resolved
src/Schema/Field/ModelField.php Show resolved Hide resolved
src/Schema/Plugin/PaginationPlugin.php Outdated Show resolved Hide resolved
@emteknetnz emteknetnz merged commit d2a0318 into silverstripe:master Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants