Update conflict logic for offline entities

[matthew-white]

Problem description

In #669, I thought that offline entities wouldn't requires us to change much (if anything) about the conflict property. Specifically, I thought:

When Central processes an entity update with a run index > 1 [branchBaseVersion > trunkVersion], then the conflict type of the resulting entity version is null if the current server version of the entity is the prior local version.

However, it's looking like that isn't the ideal behavior after all. Even if the current server version is the previous version from the branch, the update being processed may conflict with a separate update between the trunk version and the previous version. For example:

• Create an entity (v1).
• Download the entity to Collect, then create 2 offline updates to the entity. Don't send the updates yet.
  • For the 1st update, update one property.
  • For the 2nd update, update a different property.
• Before sending the updates, update the entity from Central (creating v2 on the server). Update the same property as the 2nd offline update.
• Now send the offline updates (creating v3 and v4 on the server).
• Example on staging: https://staging.getodk.cloud/#/projects/95/entity-lists/people/entities/46593005-5cf3-4620-b1eb-c7aa76aec59b

As things are currently implemented, v3 will be marked as a soft conflict because there was an update between it and its base version. However, v4 is not marked as a conflict because it immediately follows its base version (the previous version from the branch). There are a few issues that stem from the fact that v4 is not marked as a conflict:

• Another way to describe a conflict is as a "parallel update". v3 is a conflict because it was made in parallel with (without knowledge of) v2. But if v3 can be called a parallel update, then so can v4. It was also made in parallel with / without knowledge of v2.
• v3 is a conflict, but it is only a soft conflict. v2 and v3 were made in parallel, but they updated different properties, so there was no hard conflict. However, v4 does feel like more of a hard conflict. v2 and v4 were made in parallel, and they both updated the same property.
• The conflict summary table on the entity detail page lists all properties that were specified in updates that are conflicts (this is the allReceived logic in the EntityConflictTable component). If v4 is not marked as a conflict, then the only version that is a conflict is v3, and only the one property that v3 updated will be shown in the table. That means that the property that v2 and v4 updated will not be shown in the table. Upon seeing the conflict summary table, the user might double-check all the properties listed in the table. If we don't list the property that v2 and v4 updated, the user might not realize that it also warrants a double-check.
• The diff for v4 has the potential to be misunderstood. Because v4 is not marked as a conflict, it only shows Central's view of the diff. Let's say that this property was 10 in v1, 20 in v2, and 30 in v4. From the author's view, v4 updated the property from 10 to 30. However, the diff will say that v4 updated the property from 20 to 30. Without the user thinking about it carefully, I don't think it's clear that there's any problem involving the property. It looks like it went through a series of simple changes, from 10 to 20, then from 20 to 30, when in actuality, there were two conflicting changes, one from 10 to 20 and one from 10 to 30.
  • There's only so much we can do here without some effort. Even if we mark v4 as a conflict, the author's view will still say that the property changed from 20 to 30 (the same as Central's view). However, the user would at least see the warning that the author's view might not be accurate — a sign that they should look more closely at the property. If v4 is not marked as a conflict, then no warning about accuracy will be shown.

Expected behavior

As I wrote on Slack:

[A]n offline update should be considered a conflict whenever it’s not contiguous with the trunk version, even if it immediately follows its base version.

In the example above, v3 is not contiguous with the trunk version (v1), because v2 is between the two. The same is true of v4. This rule would cause v4 to be marked as a conflict.

I think this is a good rule, though I am slightly concerned about the ramifications. I would guess that there are places in Backend and/or Frontend where we consider something a conflict if and only if it does not immediately follow its base version (version !== baseVersion + 1). If we implement the proposed rule, that logic won't be true anymore. If anything needs to change in this area about Frontend, I think I could do so quickly.

I also wrote the following on Slack, but I no longer think that it's always true:

[I]f one offline update in a branch is a conflict, then all updates that follow it in the branch should also be considered in conflict, as they too were made in parallel with an update from another source.

I do think this is true if the updates are processed in order. However, if an update is force-processed out of order, it may be a conflict even if an update that follows it is not a conflict. For example:

• Create an entity (v1).
• Download the entity to Collect, then create 4 offline updates to the entity. Don't send the updates yet.
• Send the 2nd update first. Wait 5 days or force-process the update immediately. The update will be v2 on the server. It is not a conflict: its exact base version was not found, so it was applied on top of the latest version.
• Now send the 1st update. It will be immediately applied, as its base version (v1) is on the server. The update will be v3 on the server. It is a conflict because there was another version between it and its base version.
• Now send the 4th update. Wait 5 days or force-process the update immediately. The update will be v4 on the server. It is not a conflict, for the same reasons as v2.

In this example, even though v3 and v4 are from the same branch, and v3 is a conflict, we do not expect v4 to be a conflict. v2, v3, and v4 are all contiguous with the trunk version (v1): there's no conflict with some interweaving update from another source, as in the first example above. To summarize: if the versions from an offline branch were force-processed out of order, then even if all the versions are contiguous with their trunk version, it may be the case that an earlier version from the branch is a conflict while a later version is not.

So when evaluating whether an offline update being processed is a conflict, I think the question should be, "Are there any versions after the trunk version that are from outside the branch?" The question should not be, "Was the previous version in the branch a conflict?"

In terms of figuring out whether the conflict is soft or hard (which properties are conflicting, if any), I'm not so sure about the logic. We can't just compare the offline update being processed against its trunk version, because if two updates from the same branch change the same property and are processed in the correct order, there should not be a conflict: in that case, everything happened in sequence, not in parallel. It's only if there was an update from outside the branch that changed the same property as an update from the branch that there should be a hard conflict, as in the first example above.

[matthew-white]

In terms of figuring out whether the conflict is soft or hard (which properties are conflicting, if any), I'm not so sure about the logic.

Even if we can't figure out this part, I personally think it'd be an improvement from the status quo to mark these cases as soft conflicts, not even worrying about calculating the conflicting properties. As long as we mark an offline update that is not contiguous with its trunk version as a conflict, the properties it specified will appear in the conflict summary table. The update will appear as a conflict in the feed. All in all, the user will have a better understanding of what they need to review.

[matthew-white]

I think this is a good rule, though I am slightly concerned about the ramifications. I would guess that there are places in Backend and/or Frontend where we consider something a conflict if and only if it does not immediately follow its base version (version !== baseVersion + 1). If we implement the proposed rule, that logic won't be true anymore. If anything needs to change in this area about Frontend, I think I could do so quickly.

I glanced through Frontend, and I didn't see anything that obviously needed to change. Mostly the code just looks at the conflict property and doesn't compare a version number to the base version. I searched the codebase for the following strings to try to find logic that needed to change:

baseVersion
ersion + 1
ersion - 1
conflict

I may have missed something, but if so, hopefully we'll spot it while testing this issue.

[matthew-white]

@getodk/testers, we still need to do some follow-up work related to this issue, so it's not quite ready for testing. I can let you know once that follow-up work is done.

[srujner]

@matthew-white Hey what's the status with the follow-up work?

[matthew-white]

I've filed an issue for that follow-up work: #805. We're not planning to work on that this week, but we'll take a look at it next week.

[matthew-white]

@lognaturel wrote on Slack:

We also have decided not to take on #805 about offline entity conflicts for now. Given that, we also think you can call testing of #698 done for now. We think that behavior is very isolated and relatively unlikely to be common. We will decide how much more time we want to invest in related issues once we have some analytics related to offline Entities.

I'll go ahead and remove the "needs testing" label.