OCSController requires DataResponse #807
Conversation
|
I like it 👍 |
rullzer
force-pushed
the
ocs_dataresponse
branch
from
0d4c5f6 to
9b1af0f
Compare
| if ($data instanceof \OCP\AppFramework\OCS\DataResponse) { | ||
| $data = ['data' => $data->getData()]; | ||
| } else if ($data instanceof DataResponse) { | ||
| $data = $data->getData(); |
There was a problem hiding this comment.
Is there any reason now to keep the normal DataResponse? If not: Why not just move the logic from L96 to L98?
There was a problem hiding this comment.
Well before people could use the normal DataResponse. And I don't all of sudden want to break code that dit a: return DataResponse(['data' => $result]). The same like they can still return a normal array etc.
The OCS Controller requires a DataResponse object to be returned. This means that all error handling will have to be done via exceptions thrown and handling in the middleware.
rullzer
force-pushed
the
ocs_dataresponse
branch
from
9b1af0f to
4006763
Compare
rullzer
changed the title
|
OK PR changed after discussing with @schiessle and @LukasReschke The OCS Controller now requires users to return a DataResponse. |
|
First commit had to go into stable10 as well |
|
looks good 👍 |
|
👍 |
LukasReschke
added
4. to release
|
@rullzer any idea what happens with the tests here? |
|
Yes I think I know ;) |
|
Forgot to update one mroe OCSController ;) |
|
@rullzer do we need to backport this last commit as well? |
Comment from IRC: "no" |
OCSController requires DataResponse
Since 99% of the time when we return a DataResponse in an OCSController
we actually just want to set the data field to that we introduce the
OCS\DataResponse which is handled in the OCSController to do just that.
Nothing to fancy just cleaner.
CC: @LukasReschke @MorrisJobke @BernhardPosselt @schiessle