Document getModelsFromResponse

[aeksco]

Hi there! Playing around with VueMC to see if it's a good fit for the project I'm working on. I'm working with a server who's responses are of the following format:

// GET /api/widgets
{
    "page": 0,
    "per_page": 10,
    "items": [
        {
            "_id": "5a01fea69d8a3a4ce8a70bbd",
            "label": "FOO"
        },
        {
            "_id": "5a01feab9d8a3a4ce8a70bbe",
            "label": "BAR"
        }
    ]
}

Using Collection.fetch(), I've been seeing the following error thrown:

"Expected an array of models in fetch response"

I went ahead and overwrote the getModelsFromResponse method in my Collection class definition, but I had to do a bit of digging to get there:

  getModelsFromResponse (response) {
    let models = response.getData().items
    ...
  }

Is this the preferred approach with VueMC? With Backbone.js one typically defines a parse method on Models or Collections to handle variations in response form - I want to ensure that the overwriting the getModelsFromResponse method is the VueJS analogue to this approach. If so, I think it would be a valuable addition to the documentation and I would be happy to help with that endeavor :)

Also - as a side note - if this.isPaginated() does return true, it would be nice if the user could specify an alternative to the data field without overwriting the entire method:

    // We're making an assumption here that paginated models are returned
    // within the "data" field of the response.
    if (this.isPaginated()) {
      return _.get(models, 'data', models)
    }

[rtheunissen]

@aeksco agreed all round, there should be a more obvious way to adjust this and be made clear in the documentation. There are only a few options to go with here:

• A: Create options for modelPath and paginatedPath, so that you don't need to override.
• B: Keep getModelsFromResponse but document it.
• C: Keep getModelsFromResponse, document it, and pass "paginated" as an arg to it.

We could do all of the above also. Options are nice because you don't have to override anything, but the downside is you have to set the option for every instance. It therefore makes more sense to me to leave the method as is, but document it clearly and make sure it maintains backward compatibility.

The loose intention was that any method that doesn't start with an underscore is considered part of the public API and therefore affects semver.

[rtheunissen]

For what it's worth, pagination is not battle tested and there may be much nicer ways to handle it. Feel free to suggest any improvements you come across. Nice digging by the way. ⛏ ✨

[aeksco]

@rtheunissen Thanks for the prompt reply! In the short-term I'll write up some documentation for the getModelsFromResponse method so it's more accessible to other developers - I think the best approach would be to maintain the integrity of the public API and simply emphasize that some methods are intended to be overwritten when subclassing the VueMC Collection definition.

In the long term I'm definitely interested in exploring what improvements could be made to the getModelsFromResponse, as well as the pagination functionality. Backbone.PaginatedCollection takes a clean approach by defining both parse and parseState methods to extract the models and the pagination state respectively, though admittedly there are many other aspects of that library that are quite brittle.

[rtheunissen]

getModelsFromResponse is clear in what it's trying to do, where parse isn't immediately obvious. I'd like to keep the API as is and update the documentation, so we're on the same page here. @aeksco do you plan on making the change to the documentation? Happy to do it otherwise. 👍

[aeksco]

I'll make the change to the documentation - it would give me a good opportunity to get familiar with the structure of the codebase and the house-style for documentation. I should have an opportunity to get that PR open sometime this weekend - I'll keep in touch!

Thanks for all the fantastic work on VueMC 😄

[majdal]

Hi folks - I have an API that returns GeoJSON formatted responses, so getModelsFromResponse is very useful for me. That being said, two things to mention:

1. I don't think this has gone into the docs yet, has it? I can't seem to find it. I'm sure it's very useful for devs other than me.
2. Is there an equivalent for models? Something like getAttributesFromResponse? The closest I can find is onFetchSuccess, which doesn't seem to call any parsing code other than response.getData().

Thanks!

[ryanrapini]

I'm still wondering about this, any updates? Do you guys take pull requests?

[rodriigomedeiros]

Some idea for this? This is very similar to the one mentioned in #30 . Unfortunately sometimes the API is developed by third parties and we need to adapt. Sorry for coming back to this.

[ryanrapini]

I overrode the onFetchSuccess() method as described above.

export class CustomModel extends Model {
    onFetchSuccess(response) {
        let attributes = response.getData();

        // A fetch request must receive *some* data in return.
        if (isEmpty(attributes)) {
            throw new ResponseError('No data in fetch response', response);
        }

        this.assign(attributes.data); // This is the line that is different in my API (nested under a data variable)

        Vue.set(this, 'fatal',   false);
        Vue.set(this, 'loading', false);

        this.emit('fetch', {error: null});
    }
}

[enbacode]

Any progress on this? I'm also facing this problem. @ryanrapini 's solution didnt work forme, instead I overrode assign, checking wether attributes.data is set (which means the attributes come from a request) or not (meaning data comes from constructor(?)):

assign(attributes) {
        if(attributes.data)
            this.set(defaults({}, attributes.data, cloneDeep(this.defaults())));
        else
        this.set(defaults({}, attributes, cloneDeep(this.defaults())));
        this.sync();
    }

However, this is not an elegant solution. I'd love to have a getModelsFromResponse for the Model itself (or something similar).