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

[WIP] Integrate TaskSequenceHandler with DataCollectionViewModel #2986

Draft
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

shobhitagarwal1612
Copy link
Member

@shobhitagarwal1612 shobhitagarwal1612 commented Jan 7, 2025

Fixes #2958

@... PTAL?

Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 63.41463% with 15 lines in your changes missing coverage. Please review.

Project coverage is 63.33%. Comparing base (d33d1d1) to head (ac16b9f).

Files with missing lines Patch % Lines
...round/ui/datacollection/DataCollectionViewModel.kt 61.53% 11 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2986      +/-   ##
============================================
- Coverage     63.55%   63.33%   -0.23%     
+ Complexity     1242     1228      -14     
============================================
  Files           267      267              
  Lines          6506     6455      -51     
  Branches        911      905       -6     
============================================
- Hits           4135     4088      -47     
+ Misses         1785     1782       -3     
+ Partials        586      585       -1     
Files with missing lines Coverage Δ
...ground/ui/datacollection/DataCollectionFragment.kt 38.54% <ø> (-0.64%) ⬇️
...id/ground/ui/datacollection/TaskSequenceHandler.kt 100.00% <ø> (ø)
...tion/tasks/location/CaptureLocationTaskFragment.kt 54.34% <100.00%> (-5.27%) ⬇️
.../datacollection/tasks/point/DropPinTaskFragment.kt 76.92% <100.00%> (-3.08%) ⬇️
...round/ui/datacollection/DataCollectionViewModel.kt 77.16% <61.53%> (-2.14%) ⬇️

@shobhitagarwal1612 shobhitagarwal1612 marked this pull request as ready for review January 8, 2025 15:00
@shobhitagarwal1612
Copy link
Member Author

Let's hold this one until the latest release candidate is cut.

@shobhitagarwal1612 shobhitagarwal1612 marked this pull request as draft January 8, 2025 16:21
@shobhitagarwal1612
Copy link
Member Author

The problem with current existing and proposed is that we aren't caching the value of sequence. It getting generated 5-10 times during each screen. Ideally, we only need to generate the sequence when data is updated.

@shobhitagarwal1612 shobhitagarwal1612 force-pushed the ashobhit/2958/task-sequence-2 branch from af94eb3 to a511ecb Compare January 9, 2025 09:21
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.

[Code health] Move task sequence calculation / handling into a separate handler
1 participant