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

Sync: Fix double mutation detail rendering in sync status. #2609

Merged
merged 4 commits into from
Aug 8, 2024

Conversation

scolsen
Copy link
Contributor

@scolsen scolsen commented Aug 7, 2024

Fixes #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.

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.
@scolsen
Copy link
Contributor Author

scolsen commented Aug 7, 2024

Many of these changes a simple downstream consequences of adding a new field to the mutations. The core logical change is in ground/src/main/java/com/google/android/ground/repository/MutationRepository.kt, which we use only to read off mutations for the status screen. Note, importantly, that the actual workers do not use this logic. We generate the value for this field at submission time in the data collection view model.

Copy link
Collaborator

@gino-m gino-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add edit functionality we may want to expand the meaning and scope of collection to be something like operation, but for now this LGTM!

@gino-m
Copy link
Collaborator

gino-m commented Aug 7, 2024

@scolsen

> [ktfmt] Found 2 files that are not properly formatted:
  src/test/java/com/google/android/ground/ui/datacollection/DataCollectionFragmentTest.kt
  src/test/java/com/google/android/ground/persistence/remote/firebase/protobuf/LoiMutationConverterTest.kt

@scolsen
Copy link
Contributor Author

scolsen commented Aug 7, 2024

formatting is fixed but it looks like there's some other issue... I can't see the logs unfortunately

@scolsen
Copy link
Contributor Author

scolsen commented Aug 7, 2024

...looks like it may have been checkcode stuff

Copy link
Contributor

@sufyanAbbasi sufyanAbbasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix!!!

@gino-m
Copy link
Collaborator

gino-m commented Aug 8, 2024

...looks like it may have been checkcode stuff

Actually it's no longer finding the protoc java plugin for some reason. I was able to repro this locally in master as well.. 🤷‍♂️ Not sure why it's using java and not java-lite, and what would have changed to cause this, but specifying the dep manually seems to fix this. Fix in #2620.

@gino-m gino-m merged commit 3b77400 into google:master Aug 8, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Offline sync status] Duplicate entries shown in status screen
3 participants