Implement Model.withoutWrapping to return full response when set to false

[peterquentin]

Fixes #54

You can disable response unwrapping by setting one simple var:

Model.withoutWrapping = false

Tests included, please let me know if you can merge it 👍

Edit: Kind of a big change to support nested unwrapping of data.
Example API response, Model.withoutWrapping to false returns the same result:

{
  data: [
    {
      id: 1,
      someId: 'ap319-0nh56',
      firstname: 'John',
      lastname: 'Doe',
      age: 25,
      comments: {
        data: [
          {
            id: 1,
            post_id: 1,
            someId: 'ma018-9ha12',
            text: 'Hello'
          },
          {
            id: 2,
            post_id: 1,
            someId: 'mw012-7ha19',
            text: 'How are you?'
          }
        ]
      }
    },
    {
      id: 1,
      someId: 'by887-0nv66',
      firstname: 'Mary',
      lastname: 'Doe',
      age: 25,
      comments: {
        data: [
          {
            id: 1,
            post_id: 1,
            someId: 'ma018-9ha12',
            text: 'Hello'
          },
          {
            id: 2,
            post_id: 1,
            someId: 'mw012-7ha19',
            text: 'How are you?'
          }
        ]
      }
    }
  ]
}

When Model.withoutWrapping is set to true (or not defined) we return it like this:

[
    {
      id: 1,
      someId: 'ap319-0nh56',
      firstname: 'John',
      lastname: 'Doe',
      age: 25,
      comments: [
          {
            id: 1,
            post_id: 1,
            someId: 'ma018-9ha12',
            text: 'Hello'
          },
          {
            id: 2,
            post_id: 1,
            someId: 'mw012-7ha19',
            text: 'How are you?'
          }
      ]
    },
    {
      id: 1,
      someId: 'by887-0nv66',
      firstname: 'Mary',
      lastname: 'Doe',
      age: 25,
      comments: [
          {
            id: 1,
            post_id: 1,
            someId: 'ma018-9ha12',
            text: 'Hello'
          },
          {
            id: 2,
            post_id: 1,
            someId: 'mw012-7ha19',
            text: 'How are you?'
          }
      ]
    }
]

[codecov]

Codecov Report

Merging #55 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #55   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         252    256    +4     
  Branches       45     47    +2     
=====================================
+ Hits          252    256    +4

Impacted Files	Coverage Δ
src/Model.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e5ec0d...0156329. Read the comment docs.

[robsontenorio]

@peterquentin It is a good feature. But, once you do Model.withoutWrapping = true or false , this withoutWrapping attribute will belong to the new object instance, and for sure we don't want to pollute our object.

Maybe another strategy is just simply to do it like get() does: just check for a data attribute on root level from json response, and if it exists just get next element from response (the main content).

So, there is no need for withoutWrapping configs.

[peterquentin]

@robsontenorio i see what you mean, basically you always want to alter the response in order to correctly consume it. In that case we would just need to remove 1 single check in resolveResponse.

Please let me know if you agree so I can make the last changes.

Edit: Also I would extract the response formatters to a helper class of some sorts instead of methods on the Model class. I got the polluting part. 👍

[robsontenorio]

Basically, all the logic can be summarized in a simple check like this

response.data.data || response.data

For find() and first() methods.

[peterquentin]

@robsontenorio how does that help exactly with the nested relation "data" problem? In this example post.comments.data

[derekphilipau]

I understand the reasoning not to pollute the Model object, but to preserve backwards compatibility I think something must be done, perhaps it's best just to extend Model into a new class with this unwrapping behaviour.

[peterquentin]

@robsontenorio Any thoughts on the Model name that this functionality will have?

• ModelUnwrapped
• UnwrappedModel
• ModelDataUnwrapped

Not so sure about the name for this, please share your insights

[robsontenorio]

@peterquentin Please, give a try at this new branch and tell me if everything is working as expected

https://github.com/robsontenorio/vue-api-query/tree/find-wrap

[peterquentin]

@robsontenorio Nested "data" params are not removed. See my sample post.comments.data

Ps: the find part was clear to me, but for the nested part you want a seperate model. Or did I not understand this correctly?

[robsontenorio]

Yes, i have focused only on find() part.

About remove data from nested content, although possible, I am afraid it can not be part of this package. Because we don’t know if is it intended to exists on main data.

In another languages data has a different meaning (E.g PT-BR means “date”) and it can be part of a business rule in that nested content.

[peterquentin]

@robsontenorio, I think it could be if it's a seperate Model. Plus I check if the data type is an array. If its not possible, I think it would be usefull for newcomers to show them how they can best implement it on their own.

Final check, curious on your viewpoint on this one.

[robsontenorio]

If you want to get ride off all nested “data” wrapper just implement a helper class on your frontend app, after getting a response from backend. Just like you did on PR. It is the same concept.

The best scenario is to make the backend compliant, by removing that “data” wrapper.

[derekphilipau]

I am fine with implementing my own interface on top of this package to get rid of the data keys. But I must strongly disagree to the suggestion that we change our API to conform to your client software. As I mentioned originally, Laravel's JSON Resources actually default to using data keys in single resource responses. The JSON Api standard allows for nested data keys. So this is not an unusual requirement. Most importantly, we often are working with third-party API's that cannot be modified, or we are developing API's that are already being used by third parties. To change our API is not realistic.

[robsontenorio]

As I said it would be a “best scenario” for a new brand API. If not, you need to handle it by your own on frontend app.

Edit: as mentioned on Readme, this package does not try to follow strictly JSon API specs. There are specifically packages for that.