search

farmOS / farmOS.js

A JavaScript library for working with farmOS data structures and interacting with farmOS servers.
MIT License
17 stars 13 forks source link

More robust conflict handling. #27

Closed jgaehring closed 3 years ago

jgaehring commented 3 years ago

Opening this as a place to investigate the general effectiveness of how we handle conflicts in mergeEntities. As I mentioned in https://github.com/farmOS/farmOS-client/issues/418#issuecomment-813618913, I believe the logic is relatively sound, but I think it might be helpful to kick the tires on this a little more, once we actually add a UI for conflict resolution to Field Kit.

One question that occurs to me now is, what do we do if a particular conflicting change has already been stored in the conflicts array of a field's metadata? We're not matching against those currently, but perhaps we should, so we don't get uncessary duplication.

jgaehring commented 3 years ago

I should probably note, too, that we should hold off on this until #26 is completed.

jgaehring commented 3 years ago

Closing.

I'm far more confident in the conflict handling in farmOS.js itself than I was when I opened this, especially after my last few commits, 9019f6f and 5bfb5df, which fine-tuned how the merge and resolve methods work. Issues may arise in the course of farmOS/farmOS-client#190, but they can be dealt with separately when the time comes.

And with regards to this particular concern:

One question that occurs to me now is, what do we do if a particular conflicting change has already been stored in the conflicts array of a field's metadata?

This is moot now, because I added a check that will throw if you try to merge an entity with pre-existing conflicts:

https://github.com/farmOS/farmOS.js/blob/9019f6f513a7786a689d9bab77de520b38253fa6/src/model/merge.js#L50-L53

So duplicates shouldn't be possible.