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

Fallback attributes in the @ApiResource annotation #1788

Merged
merged 2 commits into from Mar 29, 2018

Conversation

Projects
None yet
6 participants
@meyerbaptiste
Member

meyerbaptiste commented Mar 22, 2018

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

This PR adds the ability to configure resource attributes directly at the first level of the @ApiResource annotation. This improves the DX by adding autocompletion on annotation properties with modern IDEs.

Attribute keys are normally in snake_case but we use lowerCamelCase at the first level of the @ApiResource annotation. So I made the choice to define them also in lowerCamelCase here but they are then normalized to snake_case.

About private properties (see the second commit), they are "virtual" because of this issue: Haehnchen/idea-php-annotation-plugin#112. I don't like that, so if you have a better idea, just tell me.

In the same way, what about priorities? For example, actually: validationGroups={"foo"}, attributes={"validation_groups"={"bar"}} = ['bar'].
Are you agree? Or do you want ['foo'] or ['foo', 'bar'] instead?

/**
* @var string
*/
private $accessControl;

This comment has been minimized.

@dunglas

dunglas Mar 23, 2018

Member

Can you add a reference to the PHPStorm plugin issue, so we'll not forget and drop those properties later :)

This comment has been minimized.

@meyerbaptiste
* @Attribute("shortName", type="string"),
* @Attribute("subresourceOperations", type="array"),
* @Attribute("validationGroups", type="mixed")
* )

This comment has been minimized.

@soyuka

soyuka Mar 23, 2018

Member

FTR I'm using vim and all I see is noise 😈

(it's friday I'm allowed to rage coz I'm a hater)

This comment has been minimized.

@meyerbaptiste

meyerbaptiste Mar 23, 2018

Member

Dear @soyuka,

I encourage you to download a real IDE. Here is the link to PHPStorm: https://www.jetbrains.com/phpstorm/download/.

I hope this could help you to find the truth!

Yours sincerely.

This comment has been minimized.

@meyerbaptiste

meyerbaptiste Mar 23, 2018

Member

More seriously, @Attribute annotations are needed when using a constructor in your annotation class. The noise here are the private properties.

meyerbaptiste added some commits Mar 21, 2018

private $routePrefix;
/**
* @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112

This comment has been minimized.

@Simperfit

Simperfit Mar 27, 2018

Member

Why not having it on the class level since it's on every property?

This comment has been minimized.

@dunglas

dunglas Mar 29, 2018

Member

Because we may introduce other properties for valid reasons later.

@dunglas dunglas merged commit d2b657e into api-platform:master Mar 29, 2018

4 checks passed

Scrutinizer Analysis: 1 updated code elements – Tests: passed
Details
brigade Brigade build passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dunglas

This comment has been minimized.

Member

dunglas commented Mar 29, 2018

@bendavies

This comment has been minimized.

Contributor

bendavies commented Mar 29, 2018

I wonder if these could be fully documented in the docs?

@meyerbaptiste meyerbaptiste deleted the meyerbaptiste:fallback_attributes_api_resource_annotation branch Mar 29, 2018

@meyerbaptiste

This comment has been minimized.

Member

meyerbaptiste commented Mar 29, 2018

Yes it will be @bendavies, it's on my todo list 😉

@teohhanhui

This comment has been minimized.

Member

teohhanhui commented Jul 8, 2018

Didn't see this earlier, but I also think it's bad, as the attributes were arbitrary keys we used, and many of them were specific to the Symfony integration. But now they've been elevated to a more important status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment