farmLog.mergeLogFromServer produces more conflicts than expected

[jgaehring]

I was working on the farm-log-demo and noticed some unexpected behavior. In demo script 3, we change the log name both locally and on the server to produce a conflict on the name prop; however, several other conflicts are produced by this action, as seen below:

{
        "name": [
                {
                        "changed": 1600260778000,
                        "data": "Update from the server!"
                }
        ],
        "notes": [
                {
                        "changed": 1600260778000,
                        "data": null
                }
        ],
        "movement": [
                {
                        "changed": 1600260778000,
                        "data": {
                                "area": [],
                                "geometry": null
                        }
                }
        ],
        "log_owner": [
                {
                        "changed": 1600260778000,
                        "data": [
                                {
                                        "uri": "http://localhost/user/1",
                                        "id": "1",
                                        "resource": "user"
                                }
                        ]
                }
        ],
        "data": [
                {
                        "changed": 1600260778000,
                        "data": null
                }
        ]
}

With the exception of the name and log_owner prop (the latter being the result of the owner only being assigned after the initial POST request), these conflicts only emerge because of a discrepancy between the data_schema props in resources. Here are the entries for those props under resources, for comparison:

{
  notes: {
    label: 'Notes',
    type: 'text_long',
    required: 0,
    data_schema: { value: '', format: 'farm_format' },
  },
  movement: {
    label: 'Movement',
    type: 'field_collection',
    required: 0,
    data_schema: { area: [{ id: null }], geometry: '' },
  },
  data: {
    label: 'Data',
    type: 'text_long',
    required: 0,
    data_schema: '',
  },
},

@mstenta, I'm curious if this is something that could be corrected on the server, or if we should have a workaround in farmLog instead.

[jgaehring]

This will be critical for #190, which is slated for the 0.8.0 milestone, unless we want to punt on that until farmOS 2.x (and hence JSON Schema) is ready to use.

[mstenta]

Is it the addition of uri and resource that you're referring to? eg: in the log_owner field that is returned from the server

Those are added automatically to any "reference" fields (fields that reference another entity).

Other than those, I'm not sure what differences you mean. I assume you're not talking about the fact that they are wrapped in an object with changed and data? (Those are Field Kit's additions - but I thought they went away with https://github.com/farmOS/farmOS-client/issues/358?)

[jgaehring]

I believe the problem is that the data_schema is saying that data, movement.geometry and notes.value should be an empty string (''), whereas the server is sending them back as null's. That's the discrepancy. The question is, if we send those null's back, will the server reject them? I know that to be the case for notes and other text_long types, which is why we have some special hard-coded logic for that. I'll have to test with the data and movement fields to see if those too get rejected.

[mstenta]

I believe that technically, '' and null are both valid and have different meaning. '' means "an empty value is saved". null means "no value is saved". This is a server detail, and I'm not sure it is helpful at all in client apps to know that. (Also I haven't tested to confirm this, but I think it's true.) The data and notes.value fields may not make much difference, but with movement.geometry it may be more important to ensure that null is set when there is no geometry, otherwise it may result in a Movement Field Collection being created on the server when it doesn't need to be. That wouldn't hurt anything, but it would be cleaner to not do that.

The question is, if we send those null's back, will the server reject them?

I would be curious of this too - my assumption is that they will be accepted.

If they are accepted, does that make this issue any easier to resolve? Or harder?

[jgaehring]

Ah, ok, that helps to know about the server value of null, but seems like all the more reason why we should try to fix the discrepancy somehow.

@mstenta, can you link again to the place in farmOS where those '' values are being generated for the data_schema props that get posted in /farm.json? I think I'm going to circle back to this issue after I finish some of the UI stuff I'm currently engaged in, so not urgent, but perhaps I could take a closer look at that time and try to identify the source of the discrepancy.

[mstenta]

Oh yes! I forgot...

https://github.com/farmOS/farmOS/blob/d28b681056be3ac71fc826355e313f63f380b33e/modules/farm/farm_ui/farm_ui.module#L49

[jgaehring]

Gonna move this to the 2.0.0-beta1 milestone, since there isn't much sense in dealing with this now until we are working with JSON Schema. Perhaps that will even resolve this w/o any further work.

[jgaehring]

I think this is mostly resolved by jgaehring/farmOS.js@9354af9, particularly with the addition of the eqFields function, which has special logic for relationships so that it's only comparing by the relationship's id, irrespective of the rest of the relationship's JSON structure.

I'm going to close this for that matter, b/c I don't think the potential bugs described in this issue will continue to be a problem. I'll open a more general issue in farmOS.js, however, to address general improvements to our conflict handling, and I'll mention this issue for future reference.