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

Cleanup remaining usages of getDetailsText() from TaskData #2950

Merged
merged 18 commits into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
3df3240
Stop using getDetailsText() for getting photo task url
shobhitagarwal1612 Dec 20, 2024
fd820b8
Stop using getDetailsText() for getting user displayed text for numbe…
shobhitagarwal1612 Dec 20, 2024
59e97a4
Stop using getDetailsText() for getting user displayed text for text …
shobhitagarwal1612 Dec 20, 2024
bcd537d
Update remaining test usages of responseText live data
shobhitagarwal1612 Dec 20, 2024
0d837b2
Remove LiveData of getDetailsText() from Abstract view model
shobhitagarwal1612 Dec 20, 2024
77b71ba
Remove obsolete getDetailsText() method from all extended classes
shobhitagarwal1612 Dec 20, 2024
426cc29
Use childFragmentManager when start child map fragment. This fixes th…
shobhitagarwal1612 Dec 22, 2024
0938f68
Add tests for info card shown in CaptureLocation task
shobhitagarwal1612 Dec 22, 2024
5fbfd4d
Update DateTaskFragmentTest to not use internal states for verifying …
shobhitagarwal1612 Dec 22, 2024
64f1042
Make CaptureLocationTaskData not a GeometryTaskData since we are stor…
shobhitagarwal1612 Dec 22, 2024
fad8c70
Use childFragment manager in drop pin and draw area tasks (similar to…
shobhitagarwal1612 Dec 22, 2024
d8b0ef9
Remove unnecessary toString() for TextTaskData
shobhitagarwal1612 Dec 22, 2024
ea85df2
Reuse assertInfoCardHidden on other map based tasks tests
shobhitagarwal1612 Dec 22, 2024
5006520
Remove usages of internal states for verifying user visible content i…
shobhitagarwal1612 Dec 22, 2024
a330407
Update method names as per suggestion
shobhitagarwal1612 Dec 23, 2024
5194258
Merge branch 'master' into cleanup-taskData
shobhitagarwal1612 Dec 23, 2024
49820e3
Automatically added GitHub issue links to TODOs
shobhitagarwal1612 Dec 23, 2024
ce8abd7
Merge branch 'master' into cleanup-taskData
shobhitagarwal1612 Dec 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ data class SubmissionMutation(

override fun toString(): String = super.toString() + "deltas= $deltas"

fun updateSyncStatus(status: SyncStatus) = this.copy(syncStatus = status)

fun getPhotoData(): List<PhotoTaskData> =
deltas.map { it.newTaskData }.filterIsInstance<PhotoTaskData>().filter { !it.isEmpty() }
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,41 +15,15 @@
*/
package com.google.android.ground.model.submission

import android.location.Location
import com.google.android.ground.model.geometry.Coordinates
import com.google.android.ground.model.geometry.Point
import com.google.android.ground.util.toDmsFormat
import java.math.RoundingMode
import java.text.DecimalFormat
import com.google.android.ground.proto.Task

/** User-provided response to a "capture location" data collection [Task]. */
data class CaptureLocationTaskData(
val location: Point,
val altitude: Double?, // in metres
val accuracy: Double?, // in metres
) : GeometryTaskData(location) {
override fun getDetailsText(): String {
// TODO: Move to strings.xml for i18n
// Issue URL: https://github.com/google/ground-android/issues/1733
val df = DecimalFormat("#.##")
df.roundingMode = RoundingMode.DOWN
val coordinatesString = location.coordinates.toDmsFormat()
val altitudeString = altitude?.let { df.format(it) } ?: "?"
val accuracyString = accuracy?.let { df.format(it) } ?: "?"
return "$coordinatesString\nAltitude: $altitudeString m\nAccuracy: $accuracyString m"
}
) : TaskData {

override fun isEmpty(): Boolean = false

companion object {
fun Location.toCaptureLocationResult(): CaptureLocationTaskData {
val altitude = if (hasAltitude()) altitude else null
val accuracy = if (hasAccuracy()) accuracy else null
return CaptureLocationTaskData(
Point(Coordinates(latitude, longitude)),
altitude,
accuracy?.toDouble(),
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import kotlinx.serialization.Serializable
@Serializable
data class DateTimeTaskData(val timeInMillis: Long) : TaskData {

override fun getDetailsText(): String = ""

override fun isEmpty(): Boolean = timeInMillis == 0L

companion object {
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,24 @@ package com.google.android.ground.model.submission

import com.google.android.ground.model.geometry.Geometry
import com.google.android.ground.model.geometry.LineString
import com.google.android.ground.model.geometry.LinearRing
import com.google.android.ground.model.geometry.MultiPolygon
import com.google.android.ground.model.geometry.Point
import com.google.android.ground.model.geometry.Polygon
import com.google.android.ground.model.task.Task

/** A user-provided response to a geometry-based task ("drop a pin" or "draw an area"). */
abstract class GeometryTaskData(val geometry: Geometry) : TaskData {

// TODO: Move strings to view layer.
// Issue URL: https://github.com/google/ground-android/issues/1733
override fun getDetailsText(): String =
when (geometry) {
is Point -> "Point data"
is Polygon -> "Polygon data"
is LinearRing -> "LinearRing data"
is LineString -> "LineString data"
is MultiPolygon -> "MultiPolygon data"
}
sealed class GeometryTaskData(val geometry: Geometry) : TaskData

/** User-provided response to a "drop a pin" data collection [Task]. */
data class DropPinTaskData(val location: Point) : GeometryTaskData(location) {
override fun isEmpty(): Boolean = false
}

/** User-provided response to a "draw an area" data collection [Task]. */
data class DrawAreaTaskData(val area: Polygon) : GeometryTaskData(area) {
override fun isEmpty(): Boolean = area.isEmpty()
}

/** User-provided "ongoing" response to a "draw an area" data collection [Task]. */
data class DrawAreaTaskIncompleteData(val lineString: LineString) : GeometryTaskData(lineString) {
override fun isEmpty(): Boolean = lineString.isEmpty()
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import kotlinx.serialization.Serializable

/** User responses to a select-one (radio) or select-multiple (checkbox) field. */
@Serializable
class MultipleChoiceTaskData(
data class MultipleChoiceTaskData(
private val multipleChoice: MultipleChoice?,
val selectedOptionIds: List<String>,
) : TaskData {
Expand All @@ -40,13 +40,6 @@ class MultipleChoiceTaskData(
suffix = MultipleChoiceTaskViewModel.OTHER_SUFFIX,
) ?: ""

override fun getDetailsText(): String =
selectedOptionIds
.mapNotNull { multipleChoice?.getOptionById(it) }
.map { it.label }
.sorted()
.joinToString()

override fun isEmpty(): Boolean = selectedOptionIds.isEmpty()

override fun equals(other: Any?): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@ import kotlinx.serialization.Serializable

/** A user provided response to a number question task. */
@Serializable
data class NumberTaskData(private val number: String) : TaskData {
data class NumberTaskData(val number: String) : TaskData {
val value: Double
get() = number.toDouble()

override fun getDetailsText(): String = number

override fun isEmpty(): Boolean = number.isEmpty()

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import kotlinx.serialization.Serializable
/** Represents a single photo associated with a given [LocationOfInterest] and [Job]. */
@Serializable
class PhotoTaskData(val remoteFilename: String) : TaskData {
override fun getDetailsText(): String = remoteFilename

override fun isEmpty(): Boolean = remoteFilename.isEmpty()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,5 @@ import kotlinx.serialization.Serializable
@Serializable
class SkippedTaskData : TaskData {

override fun getDetailsText(): String = ""

override fun isEmpty(): Boolean = true
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package com.google.android.ground.model.submission

/** User-provided value for a single data collection [Task]. */
interface TaskData {
fun getDetailsText(): String

fun isEmpty(): Boolean
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,8 @@ import kotlinx.serialization.Serializable
/** A user-provided value to a text question task. */
@Serializable
data class TextTaskData(val text: String) : TaskData {
override fun getDetailsText(): String = text

override fun isEmpty(): Boolean = text.trim { it <= ' ' }.isEmpty()

override fun toString(): String = text

companion object {
fun fromString(text: String): TaskData? = if (text.isEmpty()) null else TextTaskData(text)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,4 @@ constructor(
SELECT_ONE,
SELECT_MULTIPLE,
}

fun getOptionById(id: String): Option? = options.firstOrNull { it.id == id }
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
JSONObject().apply {
put(ACCURACY_KEY, accuracy)
put(ALTITUDE_KEY, altitude)
put(GEOMETRY_KEY, GeometryWrapperTypeConverter.toString(geometry))
put(GEOMETRY_KEY, GeometryWrapperTypeConverter.toString(location))

Check warning on line 33 in ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/CaptureLocationResultConverter.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/CaptureLocationResultConverter.kt#L33

Added line #L33 was not covered by tests
}

fun JSONObject.toCaptureLocationTaskData(): CaptureLocationTaskData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import com.google.android.ground.R
import com.google.android.ground.databinding.MapTaskFragBinding
import com.google.android.ground.model.submission.CaptureLocationTaskData.Companion.toCaptureLocationResult
import com.google.android.ground.ui.datacollection.DataCollectionViewModel
import com.google.android.ground.ui.map.CameraPosition
import com.google.android.ground.ui.map.MapFragment
import com.google.android.ground.ui.map.gms.getAccuracyOrNull
import com.google.android.ground.ui.map.gms.toCoordinates
import com.google.android.ground.util.toDmsFormat
import java.math.RoundingMode
import java.text.DecimalFormat
Expand Down Expand Up @@ -66,12 +67,11 @@ abstract class AbstractMapFragmentWithControls : AbstractMapContainerFragment()
viewLifecycleOwner.lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.STARTED) {
getMapViewModel().location.collect {
val taskData = it?.toCaptureLocationResult()
val locationText = taskData?.location?.coordinates?.toDmsFormat()
val locationText = it?.toCoordinates()?.toDmsFormat()

val df = DecimalFormat("#.##")
df.roundingMode = RoundingMode.DOWN
val accuracyText = taskData?.accuracy?.let { value -> df.format(value) + "m" } ?: "?"
val accuracyText = it?.getAccuracyOrNull()?.let { value -> df.format(value) + "m" } ?: "?"

updateLocationInfoCard(R.string.current_location, locationText, accuracyText)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,16 @@
*/
package com.google.android.ground.ui.datacollection.tasks

import androidx.lifecycle.LiveData
import androidx.lifecycle.asLiveData
import com.google.android.ground.R
import com.google.android.ground.model.job.Job
import com.google.android.ground.model.submission.SkippedTaskData
import com.google.android.ground.model.submission.TaskData
import com.google.android.ground.model.submission.isNullOrEmpty
import com.google.android.ground.model.task.Task
import com.google.android.ground.ui.common.AbstractViewModel
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.map

/** Defines the state of an inflated [Task] and controls its UI. */
open class AbstractTaskViewModel internal constructor() : AbstractViewModel() {
Expand All @@ -37,22 +33,13 @@ open class AbstractTaskViewModel internal constructor() : AbstractViewModel() {
private val _taskDataFlow: MutableStateFlow<TaskData?> = MutableStateFlow(null)
val taskTaskData: StateFlow<TaskData?> = _taskDataFlow.asStateFlow()

/** Transcoded text to be displayed for the current [TaskData]. */
val responseText: LiveData<String>

lateinit var task: Task

init {
responseText = detailsTextFlow().asLiveData()
}

open fun initialize(job: Job, task: Task, taskData: TaskData?) {
this.task = task
setValue(taskData)
}

private fun detailsTextFlow(): Flow<String> = taskTaskData.map { it?.getDetailsText() ?: "" }

/** Checks if the current value is valid and updates error value. */
fun validate(): Int? = validate(task, taskTaskData.value)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class CaptureLocationTaskFragment @Inject constructor() :
val args = Bundle()
args.putString(TASK_ID_FRAGMENT_ARG_KEY, taskId)
fragment.arguments = args
parentFragmentManager
childFragmentManager
.beginTransaction()
.add(rowLayout.id, fragment, CaptureLocationTaskMapFragment::class.java.simpleName)
.commit()
Expand Down
Loading
Loading