fix param converter to handle correctly isOptional parameter #150

Merged
merged 3 commits into from May 7, 2012

Conversation

Projects
None yet
2 participants
Member

jaugustin commented May 7, 2012

Hi,

This allow to have a paramConverter on an optional parameter.

I didn't run tests, wait for travisbot ;)

Edit: No travis bot damned :'( Build Status

@willdurand willdurand commented on an outdated diff May 7, 2012

Request/ParamConverter/PropelParamConverter.php
@@ -29,8 +29,13 @@ public function apply(Request $request, ConfigurationInterface $configuration)
// find by Pk
if (in_array('id', $exclude) || false === $object = $this->findPk($classQuery, $request)) {
// find by criteria
- if (false === $object = $this->findOneBy($classQuery, $request, $exclude)) {
- throw new \LogicException('Unable to guess how to get a Propel object from the request information.');
+ if (false === ($object = $this->findOneBy($classQuery, $request, $exclude))) {
@willdurand

willdurand May 7, 2012

Owner

you can avoid parenthesis near $object.

@willdurand willdurand commented on an outdated diff May 7, 2012

Request/ParamConverter/PropelParamConverter.php
@@ -29,8 +29,13 @@ public function apply(Request $request, ConfigurationInterface $configuration)
// find by Pk
if (in_array('id', $exclude) || false === $object = $this->findPk($classQuery, $request)) {
// find by criteria
- if (false === $object = $this->findOneBy($classQuery, $request, $exclude)) {
- throw new \LogicException('Unable to guess how to get a Propel object from the request information.');
+ if (false === ($object = $this->findOneBy($classQuery, $request, $exclude))) {
+ if ($configuration->isOptional()) {
+ //we find nothing but the obejct is optional
@willdurand

willdurand May 7, 2012

Owner

s/obejct/object

Owner

willdurand commented May 7, 2012

What do you expect then? Just a null object? That means your route doesn't point a resource, right?

Member

jaugustin commented May 7, 2012

A route like /page/{id} or /page/{slug}
if there is an Id or a slug the converter load the $category but if the id or slug is not provided it will just be a null object that you handle in your controller or view

Yes the route didn't point to the optional resource

@willdurand willdurand added a commit that referenced this pull request May 7, 2012

@willdurand willdurand Merge pull request #150 from jaugustin/fix-param-converter-optional
fix param converter to handle correctly isOptional parameter
c02e7ce

@willdurand willdurand merged commit c02e7ce into propelorm:1.1 May 7, 2012

Owner

willdurand commented May 7, 2012

Thanks!

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