Skip to content

Conversation

@anandwana001
Copy link
Collaborator

Fixes #3148

This PR implements the new DrawGeometry task UI for Android, unifying the "Drop Pin" and "Capture Location" functionalities into a single, configurable task type. This change aligns the Android client with the updated DrawGeometry task model, allowing survey administrators to configure location lock requirements and accuracy thresholds.

Key Changes
Model & Schema:

  • Updated Task.Type: Added DRAW_GEOMETRY to the enum.
  • Updated Task: Integrated DrawGeometry configuration (including require_device_location and min_accuracy_meters).
  • Updated TaskConverter: Mapped DrawGeometry proto messages to Task.Type.DRAW_GEOMETRY, ensuring correct field population from the updated proto definition.

UI Implementation:

  • DrawGeometryTaskFragment: A new shared fragment that dynamically adapts its UI:
    • Capture Mode (require_device_location = true): Enforces location lock, disables map panning, and requires accuracy validation before capturing.
    • Drop Pin Mode (require_device_location = false): Allows map panning and pin placement at the camera center (legacy drop pin behavior).
  • DrawGeometryTaskViewModel: Manages task state, location updates, and data submission. It correctly generates either CaptureLocationTaskData or DropPinTaskData based on the configuration.
  • DrawGeometryTaskMapFragment: Handles map interactions, specifically disabling gestures when location lock is enforced.

Persistence:

  • ValueJsonConverter: Updated to handle persistence for DRAW_GEOMETRY tasks, supporting both Point (for drop pin) and CaptureLocation (for full location capture) data formats.

Tests:

  • DrawGeometryTaskViewModelTest: Added comprehensive unit tests to verify:
    - Location lock enforcement logic.
    - Correct generation of CaptureLocationTaskData vs. DropPinTaskData.
    - Location lock flow initialization.

Verification
Automated Tests:

  • Created and ran DrawGeometryTaskViewModelTest.
  • All tests passed successfully, verifying logical correctness and proto compatibility.

Manual Verification:

  • Verified the logic and flow of the new ViewModel and Fragment.
  • Confirmed that the updated Proto dependency is correctly integrated and fields are accessible.

@shobhitagarwal1612 PTAL?

@codecov
Copy link

codecov bot commented Dec 24, 2025

Codecov Report

❌ Patch coverage is 30.40936% with 119 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.16%. Comparing base (5ec702d) to head (8580206).

Files with missing lines Patch % Lines
...lection/tasks/geometry/DrawGeometryTaskFragment.kt 0.00% 72 Missing ⚠️
...tion/tasks/geometry/DrawGeometryTaskMapFragment.kt 0.00% 24 Missing ⚠️
...ection/tasks/geometry/DrawGeometryTaskViewModel.kt 74.13% 9 Missing and 6 partials ⚠️
...id/data/local/room/converter/ValueJsonConverter.kt 0.00% 7 Missing ⚠️
...droid/ui/datacollection/DataCollectionViewModel.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3421      +/-   ##
============================================
- Coverage     69.92%   69.16%   -0.77%     
- Complexity     1599     1615      +16     
============================================
  Files           319      323       +4     
  Lines          8598     8768     +170     
  Branches        949      967      +18     
============================================
+ Hits           6012     6064      +52     
- Misses         2010     2122     +112     
- Partials        576      582       +6     
Files with missing lines Coverage Δ
...droid/data/remote/firebase/schema/TaskConverter.kt 83.33% <100.00%> (+1.93%) ⬆️
.../groundplatform/android/model/task/DrawGeometry.kt 100.00% <100.00%> (ø)
...java/org/groundplatform/android/model/task/Task.kt 100.00% <100.00%> (ø)
...droid/ui/datacollection/DataCollectionViewModel.kt 78.60% <0.00%> (-0.37%) ⬇️
...id/data/local/room/converter/ValueJsonConverter.kt 56.66% <0.00%> (-7.49%) ⬇️
...ection/tasks/geometry/DrawGeometryTaskViewModel.kt 74.13% <74.13%> (ø)
...tion/tasks/geometry/DrawGeometryTaskMapFragment.kt 0.00% <0.00%> (ø)
...lection/tasks/geometry/DrawGeometryTaskFragment.kt 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@shobhitagarwal1612 shobhitagarwal1612 left a comment

Choose a reason for hiding this comment

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

Please attach a screen recording as well to the description.

Comment on lines +119 to +127
if (obj is JSONObject) {
(obj as JSONObject).toCaptureLocationTaskData()
} else {
DataStoreException.checkType(String::class.java, obj)
val geometry = GeometryWrapperTypeConverter.fromString(obj as String)?.getGeometry()
DataStoreException.checkNotNull(geometry, "Missing geometry in draw geometry task result")
DataStoreException.checkType(Point::class.java, geometry!!)
DropPinTaskData(geometry as Point)
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we define a new *taskData for DRAW_GEOMETRY data type?

Let's not reuse existing *taskData classes to prevent any confusions with legacy code.

Comment on lines +89 to +92
DrawGeometry(
task.drawGeometry.requireDeviceLocation,
task.drawGeometry.minAccuracyMeters,
)
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the proto, it also contains a allowed method field whose value can be one of "DROP_PIN" and "DRAW_AREA". Please add that field as well.

https://github.com/google/ground-platform/blob/b37eb6a7d1dc5c7a7e39b606102954823ceb6fed/proto/src/ground/v1beta1/job.proto#L220

*/
package org.groundplatform.android.model.task

data class DrawGeometry(val isLocationLockRequired: Boolean, val minAccuracyMeters: Float)
Copy link
Member

Choose a reason for hiding this comment

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

Please add ktdoc

import org.groundplatform.android.util.renderComposableDialog

@AndroidEntryPoint
class DrawGeometryTaskFragment @Inject constructor() :
Copy link
Member

Choose a reason for hiding this comment

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

Please add some ktdoc

@Inject lateinit var drawGeometryTaskMapFragmentProvider: Provider<DrawGeometryTaskMapFragment>

override fun onCreateTaskView(inflater: LayoutInflater): TaskView =
TaskViewFactory.createWithCombinedHeader(inflater, R.drawable.outline_pin_drop)
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, this task should be able to choose between DROP_PIN and DRAW_AREA depending upon the proto field and should not always default to DROP_PIN.

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.

Combine "capture location" and "drop pin" tasks

2 participants