Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Offline sync status] Duplicate entries shown in status screen #2389

Closed
gino-m opened this issue Mar 17, 2024 · 7 comments · Fixed by #2609
Closed

[Offline sync status] Duplicate entries shown in status screen #2389

gino-m opened this issue Mar 17, 2024 · 7 comments · Fixed by #2609
Assignees
Labels
type: bug Something isn't working
Milestone

Comments

@gino-m
Copy link
Collaborator

gino-m commented Mar 17, 2024

Describe the bug
When adding a new LOI, two entries appear, one of which never completes.

To Reproduce
Steps to reproduce the behavior:

  1. Add a new LOI and collect data
  2. Open sync status screen

Expected behavior
There is only one entry shown for the LOI, submission, and photo.

Actual behavior
Two entries are shown, one of which never finishes.

Screenshots

Screenshot_20240317-105913.png

Screenshot_20240317-105937.png

Screenshot_20240317-105952.png

@scolsen PTAL? @jcqli for tracking.

@gino-m
Copy link
Collaborator Author

gino-m commented May 28, 2024

@shobhitagarwal1612 Could you PTAL this one, related to sync status screen?

@shobhitagarwal1612 shobhitagarwal1612 self-assigned this May 28, 2024
@shobhitagarwal1612
Copy link
Member

shobhitagarwal1612 commented Jun 10, 2024

During my investigation, I discovered that we only mark the status of SubmissionMutations as "COMPLETE" in MediaUploadWorker, which disregards LoiMutations.

There are two potential solutions:

  1. Assuming LOI Mutations will never contain media, we can directly set their status as done, eliminating the "Pending for media upload" state.
  2. Assuming LOI Mutations might include media in the future, we must update the media upload worker to handle them.

Recommendation: Since LoiMutations also consist of tasks, it's possible to incorporate a media upload task in the LOI creation flow. However, option 2 requires more effort than option 1. Hence, if LOIs will never have media tasks, option 1 is sensible. Otherwise, option 2 should be chosen.

Additionally, we need to introduce another text field in the displayed card to distinguish between "Submission" and "LOI" mutations.

@gino-m and @sufyanAbbasi, please share your thoughts on these options.

@gino-m
Copy link
Collaborator Author

gino-m commented Jun 13, 2024

Additionally, we need to introduce another text field in the displayed card to distinguish between "Submission" and "LOI" mutations.

Thank you @shobhitagarwal1612 for the thorough investigation. The user doesn't think of the data collection flow as "LOI, Submission, and Media" mutations, but rather as a single flow. In that case, would be possible to group all three types by some ID (a new data collection flow ID?) so that they can be correctly grouped as a single action? I believe that would make several of the questions you mentioned moot. Lmkwyt.

@shobhitagarwal1612
Copy link
Member

But from implementation point of view, do we ever expect LoiMutations to contain media related tasks?

@gino-m gino-m removed their assignment Jun 14, 2024
@gino-m
Copy link
Collaborator Author

gino-m commented Jun 14, 2024

But from implementation point of view, do we ever expect LoiMutations to contain media related tasks?

No. :)

@shobhitagarwal1612
Copy link
Member

The user doesn't think of the data collection flow as "LOI, Submission, and Media" mutations, but rather as a single flow. In that case, would be possible to group all three types by some ID (a new data collection flow ID?) so that they can be correctly grouped as a single action?

One LOI (predefined or adhoc) can have multiple mutations. So, it is not feasible to group mutations by LOI. Possible options:

  1. Keep the LOI mutation but add a different label to it
  2. Hide the LOI mutation once the sync is complete
  3. Don't show the LOI mutation at all in sync status screen
  4. Create a separate screen or section for LOI mutations

@gino-m
Copy link
Collaborator Author

gino-m commented Jun 17, 2024

If it's not much more work, my preference would be to link them all by some common, new "collection" or "add/edit" ID that is assigned to each time to data collection flow is opened. That way if we add "edit" in the future, changes can be grouped by user action (i.e. the user opened "collect" or "modify data", went through the flow, and hit submit/save). Is that possible?

@jcqli jcqli assigned scolsen and unassigned shobhitagarwal1612 Jul 19, 2024
scolsen added a commit to scolsen/ground-android that referenced this issue Aug 7, 2024
Fixes google#2389.

When a user collects data, they may need to add a new LOI depending on the job configuration. We treat the addition of the LOI and the eventual submission of data against it as two distinct data mutations. This resulted in duplicate mutation details in sync status for what, to the user, is perceived as a single instance of data collection/submission, which can prove confusing.

To resolve this, we now associate mutations together using a "collection ID" which indicates whether or not two distinct mutations originated from the same data collection flow. When we read mutations out of the local store, we then combine them using this identifier, taking only the more recent mutation as indicative of the overall status for the submission.
@gino-m gino-m closed this as completed in 3b77400 Aug 8, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Ground Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants