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

Add feature to update swagger context for properties #1433

Merged

Conversation

mab05k
Copy link
Contributor

@mab05k mab05k commented Oct 13, 2017

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR api-platform/docs#315

@mab05k
Copy link
Contributor Author

mab05k commented Oct 13, 2017

@dunglas Pull request for documentation is at api-platform/docs#315

@mab05k mab05k changed the title add feature to update swagger context for properties Add feature to update swagger context for properties Oct 13, 2017
@mab05k mab05k force-pushed the mab05k/feature/property-swagger-context branch from e5675aa to c606cd9 Compare October 13, 2017 00:28
*
* @return array
*/
private function getPropertyAttributes(array $attributes): array
Copy link
Member

Choose a reason for hiding this comment

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

Is this new method necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought here was to have that method there to add additional functionality in the future. In case you wanted to do more with the "attributes" on properties.

@@ -532,7 +537,22 @@ private function getPropertySchema(PropertyMetadata $propertyMetadata, \ArrayObj

$valueSchema = $this->getType($builtinType, $isCollection, $className, $propertyMetadata->isReadableLink(), $definitions, $serializerContext);

return new \ArrayObject((array) $propertySchema + $valueSchema);
return new \ArrayObject((array) $propertySchema + $valueSchema + $attributeSchema);
Copy link
Member

@dunglas dunglas Oct 13, 2017

Choose a reason for hiding this comment

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

This should be enough to get rid of all other lines:

$attributes = $propertyMetadata->getAttributes();

return new \ArrayObject((array) $propertySchema + $valueSchema + $attributes['swagger_context'] ?? []);

@mab05k mab05k force-pushed the mab05k/feature/property-swagger-context branch from c606cd9 to e8df5cd Compare October 13, 2017 23:22
@mab05k
Copy link
Contributor Author

mab05k commented Oct 13, 2017

@dunglas Updated per your comments.


return new \ArrayObject((array) $propertySchema + $valueSchema);
return new \ArrayObject((array) $propertySchema + $valueSchema + ($attributeSchema['swagger_context'] ?? []));
Copy link
Member

Choose a reason for hiding this comment

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

$attributeSchema is useless. ($propertyMetadata->getAttributes()['swagger_context'] ?? []) should do the trick.

@mab05k mab05k force-pushed the mab05k/feature/property-swagger-context branch from e8df5cd to 9ba21ef Compare October 20, 2017 00:20
@mab05k
Copy link
Contributor Author

mab05k commented Oct 20, 2017

@meyerbaptiste Updated.

@meyerbaptiste meyerbaptiste merged commit c07b770 into api-platform:2.1 Oct 20, 2017
@meyerbaptiste
Copy link
Member

Thanks @mab05k!

@greg0ire
Copy link
Contributor

Maybe I just don't understand the feature, but I can't get it to work because of this guard clause:

if (null === $type = $propertyMetadata->getType()) {
return $propertySchema;
}

Because of it, the code in the PR is never reached, and I can't get the type to be anything else than null.

Here is what my resource definition looks like:

resources:

    EMA\MD\Interfaces\Work\Dto:
        shortName: Work
        description: Work DTO
        attributes:
            filters:
                - App\Infrastructure\ApiPlatform\DataProvider\WorkCollectionDataProvider
        properties:
            contentId:
                identifier: true
                type: string
                attributes:
                    swagger_context:
                        type: string
                        enum: ['one', 'two']
                        example: 'one'
        itemOperations:
            get:
                method: 'GET'
                path: /works/{id}
        collectionOperations:
            search:
                method: 'GET'
                path: /works

@mab05k
Copy link
Contributor Author

mab05k commented Oct 30, 2017

My guess is there must be something with the entity class you are using that is causing the $propertyMetadata to resolve to a type of "null". I would need to see the rest of what you're working with to give you a better answer.

@greg0ire
Copy link
Contributor

It's a DTO, maybe this could explain a lot of things?

namespace EMA\MD\Interfaces\Work;

final class Dto
{
    public $identifier;
    public $contentId;
    public $runLength;
    public $releaseYear;
    public $releaseDate;
    public $workType;
    public $originalTitle;
    public $countryOfOrigin;
    public $title;
}

@greg0ire
Copy link
Contributor

I wrote #1466 to fix this.

@mab05k
Copy link
Contributor Author

mab05k commented Oct 30, 2017

I actually have used a DTO for this. Try adding annotations of the types. And make sure you type hint your setter.

/*
 * @var string|null
 */
private $contentId;

@greg0ire
Copy link
Contributor

Thanks, using the comment works!

hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
…rty-swagger-context

Add feature to update swagger context for properties
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.

None yet

5 participants