Fix a problem when a composite identifier is missing #1182
Conversation
|
Failures are not related, I'll fix those in a new PR. |
| try { | ||
| $identifiers = $this->normalizeIdentifiers($id, $manager, $resourceClass); | ||
| } catch (PropertyNotFoundException $e) { | ||
| throw new BadRequestHttpException($e->getMessage()); |
There was a problem hiding this comment.
The exception should be InvalidArgumentException.
|
Thanks for the patch @raoulclais ! May you rebase on 2.0 instead of merging? For example: You may have to reset to skip the merge commit from above. Another solution is to submit your patch without merging and we will take care of rebasing :). Thanks! |
raoulclais
force-pushed
the
hotfix/missing-composite-identifier
branch
2 times, most recently
from
9fa9efa
to
b1c81fe
Compare
raoulclais
force-pushed
the
hotfix/missing-composite-identifier
branch
from
8c269bc
to
985710e
Compare
|
Thanks @raoulclais |
| Scenario: Get the first composite relation with a missing identifier | ||
| Given there are Composite identifier objects | ||
| When I send a "GET" request to "/composite_relations/compositeLabel=1;" | ||
| Then the response status code should be 400 |
There was a problem hiding this comment.
It should be 404, not 400...
There was a problem hiding this comment.
The resource could never be found because the composite is invalid, why 404 ?
There was a problem hiding this comment.
The server did not even search it in the DB ^^
There was a problem hiding this comment.
10.4.5 404 Not Found
The server has not found anything matching the Request-URI.
This is by definition what happened here no? Though, one could argue that the url is malformed (hence 400).
There was a problem hiding this comment.
This is totally my point ;)
There was a problem hiding this comment.
so @teohhanhui @soyuka what do we do ? :o
…omposite-identifier Fix a problem when a composite identifier is missing
This PR fixes a bug when a resource is requested with a wrong composite identifier, like 'ida=1;' instead of 'ida=1;idb=2'.