@tsteur opened this Pull Request on June 5th 2020 Member
@sgiehl commented on June 5th 2020 Member

@tsteur Guess I did that as I thought it should be the smaller change, as the JSON format actually used the PHP format before. But guess it should be fine to use original as well.

@tsteur commented on June 8th 2020 Member

fyi tried using &format=original&serialize=0

there are 2 problems:

  1. It slightly changes formatting for XML when there is eg a map returned:

image

  1. The test_getBulkRequest_IsAbleToHandleManyDifferentRequests fails because the first request is a permission error which throws an error. The original formatter won't catch the error and format it in any way meaning the exception is passed on to the getBulkRequest method meaning you only get one error compared to 1 error and 3 valid results within the bulk request response. So as soon as one request fails, all would fail.

When using serialize=1 with the original handler then we see the class names in the response:
image

So far there are four solutions I can think of:

  1. Add PHP handler back (but try to use it only within getBulkRequest to keep BC but that might be difficult and sounds like possible future issues as only bulk request uses it so it might not be as much tested etc)
  2. Document the breaking change and keep existing code. It probably be hard to find out all possible changes. Maybe there aren't any further changes, maybe there are. Would need to create more tests.
  3. A third option be in getBulkRequest to convert back an {value: 5} response to 5. But not sure that would always work. Eg if an API returned actually return array('value' => 5). I don't think that would really work.
  4. Have an option in JSON renderer (not sure where this is actually done) to somehow ignore the value conversion in JSON datatable renderer but not quite sure how.
    image.

#4 sounds best but not really achievable unless we make some bigger changes.

It's also bit hard to know what regressions there are in other formats. Tempted to go with #1 or #2 .

Any ideas @sgiehl ?

@sgiehl commented on June 8th 2020 Member

@tsteur I'd go with 2. Readding the PHP handler wouldn't make things better. Also I think the bulk request feature shouldn't be used frequently and breaking the BC here shouldn't hurt too many people.

@tsteur commented on June 8th 2020 Member
This Pull Request was closed on June 8th 2020
Powered by GitHub Issue Mirror