Output format plugin #129
Output format plugin #129
Conversation
…put-format-plugin Conflicts: restful.module
|
The approach makes sense. I saw some minor stuff, but I'll wait to review once it's ready. One thing though, is that I think the FormatterBase class could be our Json+HAL, instead of an unused one. |
|
Makes sense |
Conflicts: modules/restful_token_auth/tests/RestfulTokenAuthenticationTestCase.test plugins/restful/RestfulEntityBase.php plugins/restful/RestfulEntityBaseMultipleBundles.php plugins/restful/RestfulFilesUpload.php restful.info tests/RestfulEntityAndPropertyAccessTestCase.test tests/RestfulEntityUserAccessTestCase.test tests/RestfulGetHandlersTestCase.test tests/RestfulHookMenuTestCase.test tests/RestfulListEntityMultipleBundlesTestCase.test tests/RestfulListTestCase.test tests/RestfulRenderCacheTestCase.test tests/RestfulViewEntityTestCase.test
|
@amitaibu this is ready to be reviewed now. I realized in the way that dealing with the special case of one item not being an array is a pain of if/else both in the back-end and in the front-end. So everything is an array now inside of |
| ->getQueryCount() | ||
| ->execute(); | ||
| $entity_type = $this->getEntityType(); | ||
| if (!empty($count_results[$entity_type])) { |
There was a problem hiding this comment.
count() will return 0 or the number, so this can be
return $this
->getQueryCount()
->execute();In that case maybe we can deprecate getQueryCount() and move logic here.
|
Really great PR, fun to review! :) |
|
Tests are green again. |
|
|
||
| /** | ||
| * @file | ||
| * Contains RestfulExampleArticlesResource__1_5. |
There was a problem hiding this comment.
RestfulExampleArticlesResource__1_5 => RestfulExampleArticlesResource__1_6
|
@amitaibu I think this is ready to be merged. |
| } | ||
| } | ||
| else { | ||
| $formatter_name = variable_get('restful_default_output_formatter', 'hal_json'); |
There was a problem hiding this comment.
https://github.com/Gizra/restful/pull/129/files#r17127841
Something like this:
else {
// Use a simple JSON formatter, for non RESTful handler calls, such as a CSRF token.
$formatter_name = variable_get('restful_default_output_formatter', 'json');
}There was a problem hiding this comment.
or instead, for simplicity, just keep the delivery callback from CSRF token as it was with restful_json_output?
Seems simpler :)
There was a problem hiding this comment.
I'd rather not. It makes no sense to have YAML for everything but csrf tokens. It forces clients to support JSON, and that is against the spirit of this pr.
I'll have a custom delivery that skips the prepare method.
There was a problem hiding this comment.
I'll have a custom delivery that skips the prepare method.
Or maybe in the plugin we can have "formatter_skip_prepare"?
|
@mateu-aguilo-bosch Seems ready. Can I merge, or still in work? |
|
Mergeable. |
|
@mateu-aguilo-bosch I see something wrong here -- I query for a single entity ID and I get a
|
|
Merged by ce02e44. Created a follow up for the |
This is early work. I just wanted you to have a peek to the direction this is turning into.