Hide "Collect data" button when the job has no more tasks to complete

[sufyanAbbasi]

Fixes #2539

Don't start data collection and instead throw an error if there are not more valid tasks for the job.

• Detects when there are no tasks or there is only an LOI task for pre-defined submission and displays an error.
• Defensively put the user back to the beginning of the task list if for any reason the startId is not found.
• Couldn't find any good place to test the code, sadly... And I'm unfortunately in a time crunch right now to fix that.

• Created a job that only has the LOI task using the local web server.
• Started the job in the app and created an LOI (only task).
• Pressed "Collect data" on the task, shows an error instead.

This state should be impossible but is nice to have to prevent crashes:

@shobhitagarwal1612 PTAL!

[codecov]

Codecov Report

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

Project coverage is 57.84%. Comparing base (473427f) to head (dcabbd9).

Files	Patch %	Lines
...ome/mapcontainer/HomeScreenMapContainerFragment.kt	0.00%	11 Missing and 1 partial ⚠️
...in/java/com/google/android/ground/model/job/Job.kt	0.00%	1 Missing ⚠️
...round/ui/datacollection/DataCollectionViewModel.kt	90.00%	0 Missing and 1 partial ⚠️
...round/ui/home/mapcontainer/cards/MapCardAdapter.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2598      +/-   ##
============================================
- Coverage     57.86%   57.84%   -0.03%     
- Complexity     1379     1381       +2     
============================================
  Files           318      318              
  Lines          7191     7206      +15     
  Branches        892      896       +4     
============================================
+ Hits           4161     4168       +7     
- Misses         2511     2519       +8     
  Partials        519      519              

Flag	Coverage Δ
service	57.84% <37.50%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...in/java/com/google/android/ground/model/job/Job.kt	59.25% <0.00%> (-2.28%)	⬇️
...round/ui/datacollection/DataCollectionViewModel.kt	77.65% <90.00%> (+1.79%)	⬆️
...round/ui/home/mapcontainer/cards/MapCardAdapter.kt	10.95% <0.00%> (ø)
...ome/mapcontainer/HomeScreenMapContainerFragment.kt	21.48% <0.00%> (-1.54%)	⬇️

[gino-m]

Thank you, @sufyanAbbasi ! Are missing tests easy to add, or should we disable these warnings for now?

[sufyanAbbasi]

Unfortunately, the tests are really hard to add since we don't have any good Fragment or ViewModel tests on these components, just a small one that checks for the map type dialog opens. There's a ton we'd have to do get that fixed.... I have tested this locally on the emulator, if you'll at least believe that the crash won't happen.

[gino-m]

LGTM!

[gino-m]

Unfortunately, the tests are really hard to add since we don't have any good Fragment or ViewModel tests on these components, just a small one that checks for the map type dialog opens. There's a ton we'd have to do get that fixed.... I have tested this locally on the emulator, if you'll at least believe that the crash won't happen.

Ack. Please stamp and merge #2610 and update this branch to rerun checks!

[gino-m]

What does it mean for a job to have no more tasks to complete? A job with no tasks is an invalid job and shouldn't be shown, right?

[gino-m]

@sufyanAbbasi On second thought - the correct thing to do would be to hide the "Add data" button altogether for jobs which only have a single addLoi task. Can you update this PR accordingly?

[sufyanAbbasi]

What does it mean for a job to have no more tasks to complete? A job with no tasks is an invalid job and shouldn't be shown, right?

Good question, definitely we can/should workshop the UX a little bit here, as I was mostly going for the simplest solution possible. For all intents and purposes, there are tasks on the job, it's just that it's only the one LOI task, so if you are trying to submit to a pre-defined LOI task, then there are technically no MORE tasks on the job, hence the message. I think a survey organizer would be able to understand why their user saw that; they configured the survey to only have the LOI task so if they are trying to submit to a predefined task, there's nothing the user can do with that.

I did see that it's possible to put the card in a read-only state, but it doesn't provide any feedback to the user about why they can't do anything with it. From a UX perspective, we prefer not to disable submit buttons, instead, let users click them and get feedback about why it doesn't work, and they can escalate to the survey organizers with a clear enough message.

If you have any suggestions for a better error message, please let me know!

[gino-m]

It's quite clear imo - if your only task is to add an LOI (e.g, a survey where the users just need to drop pins on the map), "add data to an existing LOI" has no meaning, and as such it would be a bug to show the CTA in the UI. "No more tasks" would be confusing without context - "How do I get more tasks? Do I need to do something else? Or is this a bug"? Would it be hard to just remove it completely in these cases?

[sufyanAbbasi]

Implemented the fix to remove the "Collect Data" button. I did keep the "No task" message even though now it's impossible to get to since the button doesn't render, but since we do the same thing for user permission, I figured it would be worth keeping around to prevent crashes.

[gino-m]

LGTM! Please just update the PR desc before merging!