Skip to content

NEW Generate description from class_description when scaffolding types from DataObject#633

Open
maxime-rainville wants to merge 1 commit into
silverstripe:6from
maxime-rainville:pull/6/auto-generate-type-description
Open

NEW Generate description from class_description when scaffolding types from DataObject#633
maxime-rainville wants to merge 1 commit into
silverstripe:6from
maxime-rainville:pull/6/auto-generate-type-description

Conversation

@maxime-rainville

Copy link
Copy Markdown
Contributor

Description

Update the schema DatObject scaffolding logic to read the description from DataObjects and use that to populate the GraphQL schema description for the resulting types.

Manual testing steps

  • Add class_description to a Test DataObject.
  • Set up your default graphql schema for your project and your test DataObject to your schema.
  • Generate your schema.
  • Use the GraphQL dev tools to access your schema via /dev/graphql/ide
  • Use the Schema doc view to see the description for your generated type.

Screenshot from 2025-04-19 21-30-17

Issues

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
    • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • Code changes follow our coding conventions
  • This change is covered with tests (or tests aren't necessary for this change)
  • Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
  • CI is green

@maxime-rainville maxime-rainville left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I considered allowing developers to override the description in their schema definition, but opted against it because it felt like subverting the class_description API. It would have look something like that.

SilverStripe\FrameworkTest\Model\Company:
  description: "My overridden description"
  fields: '*'
  operations:
    '*': true

I also considered provided description for the scaffolded queries and mutations, but opted against it because the description would have been so painfully obvious, it didn't seem worth it. e.g. readOneCompany: Retrieve a Company

/**
* Implementors of this interface can provide a description for a model type
*/
interface ModelTypeDescription

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This probably should be in SchemaModelInterface but that would be a breaking change.

array_map('strtolower', $model->getBlacklistedFields()) :
[];

if ($model instanceof ModelTypeDescription) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This mimics the approach above that is used with ModelBlacklist.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I didn't find any obviously great place to park those test. The existing tests don't seem to be targeted at specific classes.


public function getTypeDescription(): ?string
{
return $this->dataObject->classDescription() ?? null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
return $this->dataObject->classDescription() ?? null;
return $this->dataObject->i18nClassDescription() ?? null;

Unless there's a good reason to not use i18n here?

@maxime-rainville maxime-rainville May 1, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not overly fuss either way.

I would argue that the description in the GraphQL schema is more like developer doc than CMS user doc. We don't normally localise the dev doc.

We also don't provide a way to localise the description when manually defining types or queries.

types:
  Monkey:
    description: "This description will only be available in English and can't be translated."
    fields:
      name:
        type: String
      age:
        type: Int
      species:
        type: String

It's worth pointing out that the description is set when you build your schema. The description will then be stored in the .graphql-generated folder. So if Friedrich has his CMS locale set to German and decides to rebuild the schema, everyone else will get their GraphQL schema description in German whether they like or not.

[];

if ($model instanceof ModelTypeDescription) {
$this->setDescription($model->getTypeDescription());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems fine - if $config['description'] has been set this will be overridden in applyConfig which gives developers a way to override this if they super duper want to.

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