-
-
Notifications
You must be signed in to change notification settings - Fork 903
Add Subresource to swagger docs #1188
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
Conversation
|
||
namespace ApiPlatform\Core\Metadata\Resource; | ||
|
||
final class SubResourceMetadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm this is a good idea. I didn't wanted to introduce a new Metadata class when I worked on subresources but this will give more power to subresource declarations (ping @teohhanhui ^^).
WDYT about going a bit further, so that PropertyMetadataFactory
will create the SubResourceMetadata
when it's declared as such? This is what the outcome may be in the code below:
$propertyMetadata = $this->propertyMetadataFactory->create($parentResourceClass, $property);
if ($propertyMetadata->hasSubresource()) {
$subResourceMetadata = $propertyMetadata->getSubresource(); // this is an instance of SubResourceMetadata
}
With something like this we can avoid repeating the following code (for example also in ApiLoader
):
$isCollection = $propertyMetadata->getType()->isCollection();
$resourceClass = $isCollection ? $propertyMetadata->getType()->getCollectionValueType()->getClassName() : $propertyMetadata->getType()->getClassName();
ping @api-platform/core-team
IMO we need to agree on this change first because it may involve many changes ^^. Anyway, if we introduce this new metadata class, it'd be a shame not to use it everywhere we can :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's worth it to introduce yet another metadata class.
In fact, I'd argue for the removal of PropertyMetadata::$subresource
property. Just use an attribute.
$options = isset($serializerContext['groups']) ? ['serializer_groups' => $serializerContext['groups']] : []; | ||
foreach ($this->propertyNameCollectionFactory->create($parentResourceClass, $options) as $property) { | ||
$propertyMetadata = $this->propertyMetadataFactory->create($parentResourceClass, $property); | ||
if ($propertyMetadata->hasSubresource()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use a guard clause to make the code easier to read?
if (!$propertyMetadata->hasSubresource()) {
continue;
}
Closing in favor of #1225 |
Display SubResources in the swagger docs