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

ResourceEvent should be optional in query filters and providers #7

Open
weierophinney opened this issue Dec 31, 2019 · 3 comments
Open
Labels
Enhancement New feature or request Question Further information is requested

Comments

@weierophinney
Copy link
Contributor

As a concrete example, let's say an entity has to be created from an RPC controller. A DoctrineResource::create method is then called. Thus, when calling it, data is filtered through a ZF\Apigility\Doctrine\Server\Query\CreateFilter\DefaultCreateFilter (by default, should no extra configuration provided) as shown here

  • In this concrete class (DefaultCreateFilter), the event is not used at all.
  • Although filtering is mandatory through the DoctrineResource::create implementation, the event may not be available.
  • That should not stopped the process of creating an entity through its dedicated Resource.

Originally posted by @jguittard at zfcampus/zf-apigility-doctrine#280

@weierophinney weierophinney added Enhancement New feature or request Question Further information is requested labels Dec 31, 2019
@weierophinney
Copy link
Contributor Author

@jguittard DoctrineResource extends AbstractResourceListener and method create is not called directly but via dispatch method so the event is there always available.

Can you show example when the event is not available in create method?


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#280 (comment)

@weierophinney
Copy link
Contributor Author

I think he meant if you call DoctrineResource::create() directly since it's a public function. Maybe the API for the class can be simplified to avoid confusion by making the methods of that type protected in the future (but it's a BC break)


Originally posted by @gsomoza at zfcampus/zf-apigility-doctrine#280 (comment)

@weierophinney
Copy link
Contributor Author

Oh, yeah, this is what he meant probably :) Thanks @gsomoza. I misunderstood. Sorry @jguittard :)

I have a lot of concerns about PR #281 and it seems to be also BC Break.
We can't change these methods to protected - as these are from ZF\Rest\AbstractResourceListener.

I'm afraid that the library doesn't support RPC endpoints at all right now. There is something in codebase already (DoctrineRpcServiceModel, ...), so before somebody has it in mind. Maybe we can add it.

Of course it will be nice to resolve also this issue somehow, to allow usage create/fetch/patch/... methods without the event. Hm... what if we override getEvent method to returns always ResourceEvent?

@jguittard @gsomoza thoughts?


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#280 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant