Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tutorial enhancement and bug: PATCH doesn't work #228

Open
geoidesic opened this issue Jul 20, 2020 · 7 comments
Open

Tutorial enhancement and bug: PATCH doesn't work #228

geoidesic opened this issue Jul 20, 2020 · 7 comments

Comments

@geoidesic
Copy link

geoidesic commented Jul 20, 2020

Hi,

The tutorial doesn't seem to cover PATCH. It would be great to see:

  1. a simple PATCH request that edits a restaurant
  2. a PATCH request that adds / removes related data to / from a restaurant

I notice that there's some information in at the end of the readme, but it doesn't seem to work. I get errors.
Here's my code:

computed: {
    restaurantById: "restaurants/byId"
},
methods:{
      ...mapActions({updateRestaurant: "restaurants/update"}),
      async handleUpdateRestaurant() {
        const restaurant = this.restaurantById({ id: this.id });
        restaurant.attributes.name = _.clone(this.name);
        restaurant.attributes.address = _.clone(this.address);
        const response = await this.updateRestaurant(restaurant);
        this.clearRestaurant();
        return response;
      },
}

This is as per the docs, however it generates the following errors:

[Vue warn]: Error in callback for watcher "function () { return this._data.$$state }": "Error: [vuex] do not mutate vuex store state outside mutation handlers."

(found in <Root>)
vue.runtime.esm.js?5593:1888 Error: [vuex] do not mutate vuex store state outside mutation handlers.
    at assert (vuex.esm.js?94e4:94)
    at Vue.store._vm.$watch.deep (vuex.esm.js?94e4:834)
    at Watcher.run (vue.runtime.esm.js?5593:4568)
    at Watcher.update (vue.runtime.esm.js?5593:4542)
    at Dep.notify (vue.runtime.esm.js?5593:730)
    at Object.reactiveSetter [as name] (vue.runtime.esm.js?5593:1055)
    at _callee4$ (poc.vue?e5e6:281)
    at tryCatch (runtime.js?ad1f:45)
    at Generator.invoke [as _invoke] (runtime.js?ad1f:274)
    at Generator.prototype.<computed> [as next] (runtime.js?ad1f:97)
xhr.js?e38e:160 PATCH https://sandbox.howtojsonapi.com/restaurants/285 400 (Bad Request)
vue.runtime.esm.js?5593:619 [Vue warn]: Error in v-on handler (Promise/async): "[object Object]"

found in

---> <POC> at src/pages/poc.vue
       <QPageContainer>
         <QLayout>
           <DefaultLoggedIn> at src/layouts/DefaultLoggedIn.vue
             <App> at src/App.vue
               <Root>
vue.runtime.esm.js?5593:1888 
{data: {…}, status: 400, statusText: "Bad Request", headers: {…}, config: {…}, …}
config: {transformRequest: {…}, transformResponse: {…}, timeout: 0, xsrfCookieName: "XSRF-TOKEN", adapter: ƒ, …}
data:
errors: Array(1)
0:
code: "105"
detail: "links is not allowed."
status: "400"
title: "Param not allowed"
__proto__: Object
length: 1
__proto__: Array(0)
__proto__: Object
headers:
cache-control: "no-cache"
content-type: "application/vnd.api+json"
__proto__: Object
request: XMLHttpRequest {readyState: 4, timeout: 0, withCredentials: false, upload: XMLHttpRequestUpload, onreadystatechange: ƒ, …}
status: 400
statusText: "Bad Request"
__proto__: Object

I also tried it a different way; creating a new object without links or relationship (as both these properties are rejected by the HowToJSON server):

async handleUpdateRestaurant() {
        const rVuex = this.restaurantById({ id: this.id });
        const nuR = {
          attributes: {
            name: this.name,
            address: this.address
          },
          id: this.id,
          type: "restaurants"
        };
        const response = await this.updateRestaurant(nuR);
        this.clearRestaurant();
        return response;
      },

This actually works (i.e. the PATCH executes) but also results in a different error:

vuex.esm.js?94e4:438 [vuex] unknown action type: undefined/storeRelated
dispatch @ vuex.esm.js?94e4:438
boundDispatch @ vuex.esm.js?94e4:347
local.dispatch @ vuex.esm.js?94e4:720
eval @ reststate-vuex.js?61e3:335
Promise.then (async)
update @ reststate-vuex.js?61e3:322
wrappedActionHandler @ vuex.esm.js?94e4:792
dispatch @ vuex.esm.js?94e4:457
boundDispatch @ vuex.esm.js?94e4:347
mappedAction @ vuex.esm.js?94e4:1005
_callee4$ @ poc.vue?e5e6:291
tryCatch @ runtime.js?ad1f:45
invoke @ runtime.js?ad1f:274
prototype.<computed> @ runtime.js?ad1f:97
asyncGeneratorStep @ asyncToGenerator.js?a338:5
_next @ asyncToGenerator.js?a338:27
eval @ asyncToGenerator.js?a338:34
F @ _export.js?fe90:36
eval @ asyncToGenerator.js?a338:23
handleUpdateRestaurant @ poc.vue?e5e6:306
_callee3$ @ poc.vue?e5e6:256
tryCatch @ runtime.js?ad1f:45
invoke @ runtime.js?ad1f:274
prototype.<computed> @ runtime.js?ad1f:97
asyncGeneratorStep @ asyncToGenerator.js?a338:5
_next @ asyncToGenerator.js?a338:27
eval @ asyncToGenerator.js?a338:34
F @ _export.js?fe90:36
eval @ asyncToGenerator.js?a338:23
submitRestaurant @ poc.vue?e5e6:267
submit @ poc.vue?ca56:182
invokeWithErrorHandling @ vue.runtime.esm.js?5593:1854
invoker @ vue.runtime.esm.js?5593:2179
original._wrapper @ vue.runtime.esm.js?5593:6917

I think the bug is here:

// remove old relationships first

Maybe it shouldn't update relationships if the submitted record doesn't contain any? I'm not sure... looking at getRelationshipType it seems to be looking for a data property in the relationships object... but looking at the vuex data loaded from even those restaurants that do have related dishes, none of them have a data node in the relationships property. Seems spurious to me.

@geoidesic geoidesic changed the title Tutorial enhancement: PATCH Tutorial enhancement and bug: PATCH doesn't work Jul 20, 2020
@geoidesic
Copy link
Author

geoidesic commented Jul 22, 2020

So on further analyses, there are a few issues here.

  1. HowToJson doesn't support PATCH for relationships.
  2. Even if you get a back-end that does support PATCH for relationships via the primary entity, the spec does not allow all related entities to be removed via this URL. Editing of relationships has better support via this part of the spec: https://jsonapi.org/format/#crud-updating-to-many-relationships (which reststate does not currently support).
  3. The documentation regarding PATCH for reststate is wrong.

The first issue can be ignored if you have your own back-end.
The second issue can't be ignored and must be fixed, otherwise it is not possible to remove all related items.
Third should be updated.

This code works for adding or editing (but not removing all) related entities:

    methods: {
      ...mapActions({
        updateRestaurant: "restaurants/update",
      }),
      
      async handleUpdateRestaurant() {
        const restaurantData = {
          attributes: {
            name: this.name,
            address: this.address
          },
          id: this.id,
          type: "restaurants"
        };
        const $relationships = {
          dishes: {
            data: []
          }
        };
        this.selectedDishes.forEach(dish => {
          $relationships.dishes.data.push({
            type: "dishes",
            id: dish.id
          });
        });
        restaurantData.relationships = $relationships;
        const response = await this.updateRestaurant(restaurantData);
        return response;
      },

This is notably different from what's in the docs in that it doesn't edit vuex state directly and thus does not cause errors, whereas the PATCH examples currently in the docs fall into that trap.

@geoidesic
Copy link
Author

Relates to: #187

@CodingItWrong
Copy link
Contributor

There's a lot going on in this issue; there are probably several issues under the hood. I'll need to ask some questions to even understand what's going on.

When you say:

I notice that there's some information in at the end of the readme, but it doesn't seem to work. I get errors.

What exact information are you seeing where?

Even if you get a back-end that does support PATCH for relationships via the primary entity, the spec does not allow all related entities to be removed via this URL.

Where does the spec say this?

The documentation regarding PATCH for reststate is wrong.

I'm going to need more details than this to evaluate whether something is wrong and update it. Please provide details.

@geoidesic
Copy link
Author

geoidesic commented Jul 23, 2020

What exact information are you seeing where?

https://vuex.reststate.codingitwrong.com/writing-data.html#update

Where does the spec say this?

On re-reading it perhaps it doesn't. I assumed it did because my back-end JSONAPI service does not support it and also because the spec only shows an example of removeAll for relationhips PATCH requests (in https://jsonapi.org/format/#crud-updating-to-many-relationships).

I'm going to need more details than this to evaluate whether something is wrong and update it. Please provide details.

The documentation (https://vuex.reststate.codingitwrong.com/writing-data.html#update) shows this:

const widget = this.$store.getters['widgets/byId']({ id: 42 });
widget.attributes.title = 'Updated Title';
this.$store.dispatch('widgets/update', widget);

However this will cause a vuex warning as it is directly editing state without going via a mutator when it does this widget.attributes.title = 'Updated Title';

@daryledesilva
Copy link
Contributor

+1 also getting this for patches with empty relationships

@mspiderv
Copy link

mspiderv commented Nov 13, 2020

I experience the same error after a siple one-attribute update action.

[vuex] unknown action type: undefined/storeRelated

I found out that this piece of the core is bugged:

// remove old relationships first
if (oldRecord && oldRecord.relationships) {
  for (const entry of Object.entries(oldRecord.relationships)) {
    const [relationship, entity] = entry;
    const type = getRelationshipType(entity);
    const paramsToStore = {
      relationship,
      parent: getResourceIdentifier(oldRecord),
    };

    dispatch(
      `${type}/storeRelated`,
      {
        params: paramsToStore,
        relatedIds: null,
      },
      { root: true },
    );
  }
}

When the original record had loaded some relationship and you are doing update operation, then your server will most likely will NOT respond with that relationship. That will cause the type variable to be undefined. Whole update action works perfectly even with that error in the console.

EDIT: The solution here is imho to not update the relationships we had in original record but only those ones which server responded. If server doesn't respond with any relationships we should not even call the dispatch(something/storeRelated ...

@CodingItWrong
Copy link
Contributor

FYI, this library will be unmaintained going forward.

If you need new features or fixes, I recommend forking the repo and making changes, or finding an alternate library that meets your needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants