-
-
Notifications
You must be signed in to change notification settings - Fork 918
Ignore property that does not exist for interfaces (identifiers for interfaces) #2086
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
Ignore property that does not exist for interfaces (identifiers for interfaces) #2086
Conversation
11851ca
to
5f88305
Compare
5f88305
to
6332272
Compare
I thought we already support virtual properties? |
@teohhanhui interesting. I was surprised to not find the feature. I'm curious to know where it is documented. |
@Nek- I think there's no documentation, not even tests for it. 😆 🙈 But looking at the code (and as I've heard from other's experience), it should work. (Not when using the |
In any case, it doesn't invalidate this feature as I understand. Are failures on CI related? (due to inversion of condition?) |
CI failures are unrelated and should be fixed after #2106 is merged. |
6332272
to
13eafa5
Compare
It's up to date and issues of phpstan non-related. |
src/Metadata/Property/Factory/ExtractorPropertyMetadataFactory.php
Outdated
Show resolved
Hide resolved
13eafa5
to
ea36a5c
Compare
I think that #2495 fixes this case lmk if it doesn't. Going to cherry pick the tests anyway! /note for EU-FOSSA Hackathon just cherry-pick the test |
@soyuka it does not. (I tried to add the test to the code base) |
ea36a5c
to
5155a3e
Compare
5155a3e
to
05b3709
Compare
src/Metadata/Property/Factory/ExtractorPropertyMetadataFactory.php
Outdated
Show resolved
Hide resolved
1. it makes no sens to check if an interface exists in case it's an interface 2. If the property is not recognized as one and there is no identifier, then api platform will not be able to call the DataProvider for a given item and interface as a resource is an incomplete feature.
It should work like this but sadly it does not ATM. This is some WIP and should not be tested for now.
05b3709
to
e05809e
Compare
Motivations
TL;DR: Before: no "item" route working because we can't define an identifier. After: we can define a property for an interface and it will not be considered as "not found property".
Usage
What's next?
If you like this kind of features, I will probably work on "virtual properties" next. I see something like
What do you think? Maybe you want a separate RFC issue to discuss it?