zfcampus / zf-versioning

BSD 3-Clause "New" or "Revised" License
11 stars 7 forks source link

Use zf_ver_version instead of version #18

Closed TomHAnderson closed 6 years ago

TomHAnderson commented 8 years ago

The route parameter version is a reserved word because of the handling of the version in this module. This fixes the named route parameter to zf_ver_version so version may be a field in a Doctrine entity. If the reserved word version is also a field name in an entity it will break: https://github.com/zfcampus/zf-apigility-doctrine/issues/275

cc @jensstalder

weierophinney commented 7 years ago

I'm afraid I don't understand: how exactly does a route parameter affect a Doctrine entity? Those are usually going to be based on the request body, not the URI. Can you please elaborate?

TomHAnderson commented 7 years ago

I believe it is https://github.com/zfcampus/zf-apigility-doctrine/blob/master/src/Server/Resource/DoctrineResource.php#L704 + https://github.com/zfcampus/zf-versioning/blob/master/src/PrototypeRouteListener.php#L127 where version is injected into the route parameters.

So these route parameters are used in zf-apigility-doctrine here: https://github.com/zfcampus/zf-apigility-doctrine/blob/master/src/Server/Resource/DoctrineResource.php#L718 and 'version' is now a route parameter. This causes https://github.com/zfcampus/zf-apigility-doctrine/blob/master/src/Server/Resource/DoctrineResource.php#L739 to assign a query builder equals of 'version' to 1.

This is not a problem unless the entity has a version field.

I would like to see route parameter filtering of entities removed from zf-apigility-doctrine as it was added at the request of the community long ago and I believe writing an API in that way is incorrect and should be done on individual and individual looking resources.

michalbundyra commented 6 years ago

It's no longer needed. Please see: https://github.com/zfcampus/zf-apigility-doctrine/pull/303

weierophinney commented 6 years ago

Closed with zfcampus/zf-apigility-doctrine#303.