-
-
Notifications
You must be signed in to change notification settings - Fork 921
feat(metadata): use PHP file as resource format #7017
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
e44b66c
to
10224da
Compare
@dunglas thoughts ? I quite like the idea. As folluw-up you'd be able to overwrite sylius resources using something like this: return $syliusProductResource->withShortName('overriden'); ? |
I like the idea too! |
10224da
to
0d3b24a
Compare
@soyuka I know you'll be a speaker during Sylius party in Toulouse. I'll be not there physically but maybe we could work on this together (remotely) during the hackaton if you are interested too. WDYT? |
Not sure there's much more tbd, I'd flag this new functionality as @experimental, rebase, make sure tests are green and I'll do a review then we can merge this no? |
f5f8e08
to
4a76d19
Compare
4a76d19
to
397ffca
Compare
thanks @loic425 ! |
That's a very good news, I'll try to work on the customization soon, but that's already a cool feature. |
| Q | A | --------------- | ----- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Related tickets | | License | MIT Same feature as I did on api-platform/core#7017
| Q | A | --------------- | ----- | Bug fix? | no | New feature? | yes | BC breaks? | no/yes | Deprecations? | no/yes <!-- don't forget to update the UPGRADE-*.md file --> | Related tickets | | License | MIT It replaces #1005 The goal of this PR is to achieve part of what have been done on api-platform/core#7017
| Q | A | --------------- | ----- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Related tickets | fixes #1029 | License | MIT Very similar to api-platform/core#7017 but in Sylius resource context (for HTML routing) ```php <?php // config/sylius/resources/speaker.php declare(strict_types=1); use Sylius\Resource\Metadata\Create; use Sylius\Resource\Metadata\ResourceMetadata; use Sylius\Resource\Metadata\Update; use Sylius\Resource\Metadata\Operations; use App\Entity\Speaker; return new ResourceMetadata() ->withClass(Speaker::class) ->withOperations(new Operations([ new Create(), new Update(), ])) ; ```
The idea of this PoC is to allow configuring resources and their operations with external PHP files instead of XML/YAML files, which will be useful for projects such as Sylius.
This PR is not ready to merge, there are no PHPUnit tests, I've just used a minimalist Symfony app to test it.
The goal of this PR is to open discussions and make an ADR later.
I already know there are also some discussions about customizing the existing operations but that could be based on this PHP file resource extractor. (I already made a PoC about that too in a CustomResourceMetadataCollectionFactory on the Sylius resource side).
The main issue is that we cannot load PHP files that are in the autoloader, cause it will throw an error, indicating the class already exist. That's the reason I added an "imports" part to the mapping configuration to solve the issue for the PoC.
we instanciate the ApiResource, so it's the same DX as we already have when configuring the entity with the attributes.
Here, it's just to show the current implementation with attributes still works.
We both have our Conference Entity with the ApiResource attribute and our Speaker entity configured with an external PHP file.
I've tried to get the collection and to add a new speaker and it works perfectly.