[Add LOI tasks] Inconsistent behavior of Add LOI tasks about ground-android HOT 5 CLOSED

It seems like, for some reason, while the $addLoi task appears in the task list from Firestore, when we're retrieving the taks lists from the JobEntityAndRelations in the local DB, that $addLoi task is no where to be found. Investigating further.

Hypothesis: maybe because the task ID, $addLoi is shared, the key is not unique so it gets wiped out by another task?

from ground-android.

After debugging some more, by changing the $addLoi ID to something like $addLoi123 then setting the target LOI task ID to $addLoi123, the feature began to work again as expected. With breakpoints, I can see that as we iterate over the surveys and store them into the DAO, when the same LOI task ID key is encountered, we store it there.

Given that the taskDao uses the taskId as the primary key, it gets overridden by the other tasks when loaded into the DAO. Inspecting the DB revealed the issue:

Proposed fixes:

1. Change it from $addLoi to $addLoi<JOB ID> which maintains the unique task ID and we can use a simple startsWith() check.
2. Do this transformation on the local DB side and remember to unparse it later at fetch time.
3. Reconfigure the DB to use both a Primary and Secondary key, the task and job ID, something something?

from ground-android.

Great sleuthing, Sufy! What if we just go back to using the "addLoiTask" boolean? That way clients would be able to count on task IDs being UUIDs. @rfontanarosa does this complicate things much?

from ground-android.

I wanted to report that option 3, having two primary keys, is a completely viable option: I modified the task table, multiple choice table, and option table to use the task ID and job ID as primary keys. I have no crashes and as long as task IDs are unique within a job, the solution actually works REALLY well. Furthermore, consider the scenario in which a sufficiently lazy survey creator wanted to just copy and paste a survey schema. With this solution, they could theoretically JUST change the job ID and the two surveys could coexist on the device. In the status quo scenario, doing the copy operation would be unsafe.

The cost is a table schema update and more complicated key structures which would get in the way of migration.

from ground-android.

Update: We reevaluated this issue, and agreed instead on the solution outlined in google/ground-platform#1530. For now, no changes are needed Android-side. We'll eventually deprecate addLoiTask boolean, but not in this version, so closing this for now.

from ground-android.