Use PATCH response to update fields data

[mullema]

Describe the bug
When a field is updated and saved, the content store keeps track of the change. This works for the fields value but not for other props/computed properties created in the fields php file. The new computed properties from the php file are received as response to the PATCH request but are not used to update the content store.

To Reproduce
Steps to reproduce the behavior:

1. Add this simple field plugin to your Kirby 3.3 installation https://github.com/mullema/patch-test It is an extended text field with a computed property value2 which is dependent on value.
2. Add the field to your blueprint

   fields:
   patch:
   type: text
   label: patch test

3. see that the field is empty and the text reads "in the box"
4. write "Kirby" into the text input, save the changes
5. see that it still says "in the box"
6. reload the page or navigate in the panel and come back
7. see that it says now "Kirby in the box"

Expected behavior
The response to the PATCH request should be parsed and used to update the model in the content store.

Kirby Version
3.3

[distantnative]

This isn't really a bug, but I understand that it can be confusing since on the PHP side it's also called computed. However, a PATCH request only sends the new values to the API, it never request updated data from the API. The value doesn't get updated through the API, it is only send there and then the store replaces the original value with the new value itself. Changing it any other way would result in worse performance, since all fields would have to be reloaded from the API. I am not sure if we are going to do this ever, probably at least not that soon (my gut feeling).

However, for your use case, it would make much more sense to directly implement it on the Vue side (and then it works):

panel.plugin("test/patch", {
  fields: {
    text: {
      computed: {
        value2: function () {
          return this.value + " in a box";
        }
      },
      extends: 'k-text-field',
      template: `...`
    }
  }
});

[mullema]

The example above was a very simple one to show the issue.

The actual use case is a patch request which would expect new thumbnail URLs to show images in the panel. I don't think implementing the Filename class in javascript is a good/maintainable solution.

a PATCH request only sends the new values to the API, it never request updated data from the API

The entire field data (props and computed) is sent in the response body to a patch request though.

The only solution I see is an additional request to the api to fetch the thumbnail URLs and then update the store with that data.

[distantnative]

The entire field data (props and computed) is sent in the response body to a patch request though.

I don't think so. That's the response to a PATCH request when saving:

[distantnative]

I think it would be very inefficient if all fields refresh their data on save. However, you could listen in your field for that event and then make a separate API request to update your specific data.

[mullema]

I understand your concern about refreshing all the fields.

I don't think so. That's the response to a PATCH request when saving

On my system the data.content node contains all field data of that page. My test page contains 2 files fields, one structure field including another 2 files fields. The PATCH request takes 200ms and responds with 23kb of data (all the fields and page data). It seems unnecessary to respond with that much data when it is not used in any way.

The PATCH request responds with the most current model from the backend. However this data is not used and therefore the "backend-model" and the "panel-model" become inconsistent.

Maybe the response could be:

• a http code 204 and not include any message body
• the changed data only (which triggers a content store update)

-- Regarding the initial issue/question: I am using a separate request in my field plugin to get the computed values and dispatch a content/update to the store.

[distantnative]

@mullema I can't reproduce that. I'm testing it on a note page in the starterkit. PATCH request response is 1.9 kb. The data.content contains all values of that page, but nothing else of the field definitions (no computed, no props....).

[mullema]

Maybe it is a specific issue with the files field. Also my wording was not correct. There are no computed values. But a lot of other data.

[distantnative]

What you are looking at is not the response for the PATCH request, but the reload request that files sections send on the model.update event (so after saving): https://github.com/getkirby/kirby/blob/master/panel/src/components/Sections/FilesSection.vue#L83

Wondering whether we need a bit more granular events. At least can't remember right now why the files section needs to be refreshed on save as well.

[distantnative]

Closing in favor of https://github.com/getkirby/kirby/issues/2361

[mullema]

It seems that I am not fully understanding the way the panel handels these requests.

*edit: I just realized you were talking about files sections. My example is about a files field.

One last comment to this: I cannot see a reload request (if by request you mean http request). Firefox Devtools do only log a PATCH request with all the payload.

[distantnative]

Ah sorry, thought you meant files section.

[distantnative]

I'm going to revert everything I said XD

Yes, $api.patch() for form values returns all fields again. Yes, we should use this to update all field sections.

The plan is:

1. Extract this to its own mapFields method: https://github.com/getkirby/kirby/blob/master/panel/src/components/Sections/FieldsSection.vue#L60-L67
2. Add an update method to the FiedsSection component. The method receives a full object of fields. It checks for each of its own fields (included in this section) whether data in the object is available and updates accordingly.
3. In the FieldsSection component we register an $event listener for content.update that calls the new update method.
4. Here (https://github.com/getkirby/kirby/blob/master/panel/src/store/modules/content.js#L337) we actually take the response and emit the event content.update, passing the response object. We emit the event via this._vm.$events.$emit() (since we are in the store).

this._vm.$library.

[distantnative]

Actually, we only get values for most fields The picker fields are an exception. So my previous thoughts make more sense than today's - I guess we have to close this issue.