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 - DO NOT REVIEW] Refactor feature managers #1942

Closed
wants to merge 33 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b5c8206
Remove unnecessary debug statement
gino-m Sep 27, 2023
b4fd8e7
Fix ktdoc
gino-m Sep 27, 2023
ef4607f
Readability improvements
gino-m Sep 27, 2023
3fab291
Style cluster icons
gino-m Sep 27, 2023
a4ecfb3
Refactor marker scaling
gino-m Sep 27, 2023
83df8f8
Minor rename
gino-m Sep 27, 2023
50fc83e
Update cluster marker image
gino-m Sep 27, 2023
771fc1e
Improve log message
gino-m Sep 27, 2023
28726a7
Refactor WIP
gino-m Sep 27, 2023
7ad5cc0
Add pin position
gino-m Sep 27, 2023
f08a510
Merge branch 'master' of https://github.com/google/ground-android int…
gino-m Sep 28, 2023
eb610d9
Remove unused
gino-m Sep 28, 2023
452fc6e
Finish implementing PointRenderer
gino-m Sep 28, 2023
99a41c1
Call remaining PointRenderer callback
gino-m Sep 28, 2023
211823e
Refactor renderers
gino-m Sep 28, 2023
0989dc4
Rename renderer to manager
gino-m Sep 28, 2023
3ce0b3f
Rename renderer to manager
gino-m Sep 28, 2023
36ca1af
Remove unused
gino-m Sep 28, 2023
f74dd6e
Init feature managers
gino-m Sep 28, 2023
f13f533
Disambiguate cap and joint types
gino-m Sep 28, 2023
0eddf0a
Remove hard coded default color
gino-m Sep 28, 2023
17432ca
TODO workaround
gino-m Sep 28, 2023
2244e93
Rename duplicate `TaskData` class
gino-m Oct 2, 2023
8b44e2f
Use correct color for drop a pin flow
gino-m Oct 2, 2023
3bbd93c
Merge branch 'master' of https://github.com/google/ground-android int…
gino-m Oct 2, 2023
ca9f247
Minor rename
gino-m Oct 2, 2023
b0e106b
Remove unused
gino-m Oct 2, 2023
ee4f070
Minor refactor
gino-m Oct 2, 2023
856124a
Apply style to polygon drawing
gino-m Oct 2, 2023
9da6475
Fix tests
gino-m Oct 2, 2023
5e88400
Remove unused
gino-m Oct 2, 2023
d878170
Fix detekt error
gino-m Oct 2, 2023
ec5e901
Small refactor
gino-m Oct 2, 2023
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
13 changes: 12 additions & 1 deletion ground/src/main/java/com/google/android/ground/model/job/Job.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,18 @@
*/
package com.google.android.ground.model.job

import android.graphics.Color
import com.google.android.ground.model.task.Task
import java.lang.IllegalArgumentException
import timber.log.Timber

/**
* @param suggestLoiTaskType the type of task used to suggest the LOI for this Job. Null if the job
* is already associated with an LOI.
*/
data class Job(
val id: String,
val style: Style,
val style: Style? = null,
val name: String? = null,
val tasks: Map<String, Task> = mapOf(),
val suggestLoiTaskType: Task.Type? = null,
Expand All @@ -35,3 +38,11 @@ data class Job(

fun hasData(): Boolean = tasks.isNotEmpty()
}

fun Job.getDefaultColor(): Int =
try {
Color.parseColor(style?.color ?: "")
} catch (e: IllegalArgumentException) {
Timber.w(e, "Invalid or missing color ${style?.color} in job $id")
0
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@
*/
package com.google.android.ground.model.job

data class Style @JvmOverloads constructor(val color: String = "#ff9131")
data class Style constructor(val color: String)
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ constructor(
// TODO: Move to strings.xml for i18n
val df = DecimalFormat("#.##")
df.roundingMode = RoundingMode.DOWN
return "${LatLngConverter.processCoordinates(geometry.coordinates)}\n" +
"Altitude: ${df.format(altitude)}m\n" +
"Accuracy: ${df.format(accuracy)}m"
val coordinatesString = LatLngConverter.formatCoordinates(geometry.coordinates)
val altitudeString = altitude?.let { df.format(it) } ?: "?"
val accuracyString = accuracy?.let { df.format(it) } ?: "?"
return "$coordinatesString\nAltitude: $altitudeString m\nAccuracy: $accuracyString m"
}

override fun isEmpty(): Boolean = geometry == null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,25 @@ fun Job.toLocalDataStoreObject(surveyId: String): JobEntity =
surveyId = surveyId,
name = name,
suggestLoiTaskType = suggestLoiTaskType?.toString(),
style = style.toLocalDataStoreObject()
style = style?.toLocalDataStoreObject()
)

fun JobEntityAndRelations.toModelObject(): Job {
val taskMap = taskEntityAndRelations.map { it.toModelObject() }.associateBy { it.id }
return Job(
jobEntity.id,
jobEntity.style.toModelObject(),
jobEntity.style?.toModelObject(),
jobEntity.name,
taskMap.toPersistentMap(),
jobEntity.suggestLoiTaskType?.let { Task.Type.valueOf(it) }
)
}

fun StyleEntity.toModelObject() = color?.let { Style(it) } ?: Style()
/**
* Returns the equivalent model object, setting the style color to #000 if it was missing in the
* local db.
*/
fun StyleEntity.toModelObject() = color?.let { Style(it) } ?: Style("#000000")

fun Style.toLocalDataStoreObject() = StyleEntity(color)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ data class JobEntity(
@ColumnInfo(name = "name") val name: String?,
@ColumnInfo(name = "survey_id") val surveyId: String?,
@ColumnInfo(name = "suggest_loi_task_type") val suggestLoiTaskType: String?,
@Embedded(prefix = "style_") val style: StyleEntity
@Embedded(prefix = "style_") val style: StyleEntity?
)
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,14 @@ class RoomSubmissionStore @Inject internal constructor() : LocalSubmissionStore
): List<SubmissionMutationEntity> =
submissionMutationDao.findByLocationOfInterestId(loidId, *states)

override suspend fun getPendingSubmissionCountByLocationOfInterestId(loiId: String): Int =
override suspend fun getPendingCreateCount(loiId: String): Int =
submissionMutationDao.getSubmissionMutationCount(
loiId,
MutationEntityType.CREATE,
MutationEntitySyncStatus.PENDING
)

override suspend fun getPendingSubmissionDeletionCountByLocationOfInterestId(loiId: String): Int =
override suspend fun getPendingDeleteCount(loiId: String): Int =
submissionMutationDao.getSubmissionMutationCount(
loiId,
MutationEntityType.DELETE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ interface LocalSubmissionStore : LocalMutationStore<SubmissionMutation, Submissi
vararg states: MutationEntitySyncStatus
): List<SubmissionMutationEntity>

suspend fun getPendingSubmissionCountByLocationOfInterestId(
suspend fun getPendingCreateCount(
loiId: String,
): Int

suspend fun getPendingSubmissionDeletionCountByLocationOfInterestId(
suspend fun getPendingDeleteCount(
loiId: String,
): Int
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ internal object JobConverter {
}
return Job(
id,
obj.defaultStyle.toStyle(),
obj.defaultStyle?.toStyle(),
obj.name,
taskMap.toPersistentMap(),
TaskConverter.toSuggestLoiTaskType(obj.suggestLoiTaskType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import com.google.firebase.firestore.IgnoreExtraProperties
/** Firestore representation of map layers. */
@IgnoreExtraProperties
data class JobNestedObject(
val defaultStyle: StyleNestedObject = StyleNestedObject(),
val defaultStyle: StyleNestedObject? = null,
val name: String? = null,
val tasks: Map<String, TaskNestedObject>? = null,
val suggestLoiTaskType: String? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ import com.google.android.ground.model.job.Style
import com.google.firebase.firestore.IgnoreExtraProperties

/** Firestore representation of map layers. */
@IgnoreExtraProperties data class StyleNestedObject(val color: String = "#ff9131")
@IgnoreExtraProperties data class StyleNestedObject(val color: String = "")

fun StyleNestedObject.toStyle(): Style = Style(color)
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import com.google.android.ground.model.mutation.LocationOfInterestMutation
import com.google.android.ground.model.mutation.Mutation.SyncStatus
import com.google.android.ground.persistence.local.room.fields.MutationEntitySyncStatus
import com.google.android.ground.persistence.local.stores.LocalLocationOfInterestStore
import com.google.android.ground.persistence.local.stores.LocalSubmissionStore
import com.google.android.ground.persistence.local.stores.LocalSurveyStore
import com.google.android.ground.persistence.remote.NotFoundException
import com.google.android.ground.persistence.remote.RemoteDataStore
Expand All @@ -33,15 +32,11 @@ import com.google.android.ground.persistence.uuid.OfflineUuidGenerator
import com.google.android.ground.rx.annotations.Cold
import com.google.android.ground.system.auth.AuthenticationManager
import com.google.android.ground.ui.map.Bounds
import com.google.android.ground.ui.map.Feature
import com.google.android.ground.ui.map.FeatureType
import com.google.android.ground.ui.map.gms.GmsExt.contains
import io.reactivex.Flowable
import io.reactivex.Single
import javax.inject.Inject
import javax.inject.Singleton
import kotlinx.collections.immutable.toPersistentSet
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.reactive.awaitFirst

/**
Expand All @@ -55,7 +50,6 @@ class LocationOfInterestRepository
constructor(
private val localSurveyStore: LocalSurveyStore,
private val localLoiStore: LocalLocationOfInterestStore,
private val localSubmissionStore: LocalSubmissionStore,
private val remoteDataStore: RemoteDataStore,
private val mutationSyncWorkManager: MutationSyncWorkManager,
private val authManager: AuthenticationManager,
Expand Down Expand Up @@ -133,31 +127,7 @@ constructor(
survey: Survey
): Flowable<Set<LocationOfInterest>> = localLoiStore.getLocationsOfInterestOnceAndStream(survey)

private fun findLocationsOfInterest(survey: Survey) =
localLoiStore.findLocationsOfInterest(survey)

fun findLocationsOfInterestFeatures(survey: Survey) =
findLocationsOfInterest(survey).map { toLocationOfInterestFeatures(it) }

private suspend fun toLocationOfInterestFeatures(
locationsOfInterest: Set<LocationOfInterest>
): Set<Feature> = // TODO: Add support for polylines similar to mapPins.
locationsOfInterest
.map {
val pendingSubmissions =
localSubmissionStore.getPendingSubmissionCountByLocationOfInterestId(it.id) -
localSubmissionStore.getPendingSubmissionDeletionCountByLocationOfInterestId(it.id)
val submissionCount = it.submissionCount + pendingSubmissions
Feature(
id = it.id,
type = FeatureType.LOCATION_OF_INTEREST.ordinal,
flag = submissionCount > 0,
geometry = it.geometry,
style = it.job.style,
clusterable = true
)
}
.toPersistentSet()
fun getLocationsOfInterest(survey: Survey) = localLoiStore.findLocationsOfInterest(survey)

/** Returns a list of geometries associated with the given [Survey]. */
suspend fun getAllGeometries(survey: Survey): List<Geometry> =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,10 @@ constructor(
MutationEntitySyncStatus.FAILED
)
}

suspend fun getPendingCreateCount(loiId: String) =
localSubmissionStore.getPendingCreateCount(loiId)

suspend fun getPendingDeleteCount(loiId: String) =
localSubmissionStore.getPendingDeleteCount(loiId)
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ abstract class AbstractMapFragmentWithControls : AbstractMapContainerFragment()
return
}
val target = position.target
val processedCoordinates = LatLngConverter.processCoordinates(target)
val processedCoordinates = LatLngConverter.formatCoordinates(target)
setCurrentLocationAsInfoCard(processedCoordinates)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ internal constructor(
}
val viewModel = viewModelFactory.create(getViewModelClass(task.type))
// TODO(#1146): Pass in the existing taskData if there is one
viewModel.initialize(task, null)
viewModel.initialize(job, task, null)
addTaskViewModel(viewModel)
return viewModel
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.toLiveData
import androidx.lifecycle.viewModelScope
import com.google.android.ground.R
import com.google.android.ground.model.job.Job
import com.google.android.ground.model.submission.TaskData
import com.google.android.ground.model.submission.isNullOrEmpty
import com.google.android.ground.model.task.Task
Expand Down Expand Up @@ -65,7 +66,7 @@ open class AbstractTaskViewModel internal constructor(private val resources: Res
}

// TODO: Add a reference of Task in TaskData for simplification.
fun initialize(task: Task, taskData: TaskData?) {
open fun initialize(job: Job, task: Task, taskData: TaskData?) {
this.task = task
setResponse(taskData)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ package com.google.android.ground.ui.datacollection.tasks.point
import android.content.res.Resources
import androidx.lifecycle.MutableLiveData
import com.google.android.ground.model.geometry.Point
import com.google.android.ground.model.job.Job
import com.google.android.ground.model.job.getDefaultColor
import com.google.android.ground.model.submission.GeometryData
import com.google.android.ground.model.submission.TaskData
import com.google.android.ground.model.task.Task
import com.google.android.ground.persistence.uuid.OfflineUuidGenerator
import com.google.android.ground.rx.annotations.Hot
import com.google.android.ground.ui.datacollection.tasks.AbstractTaskViewModel
Expand All @@ -32,9 +36,15 @@ class DropAPinTaskViewModel
constructor(resources: Resources, private val uuidGenerator: OfflineUuidGenerator) :
AbstractTaskViewModel(resources) {

private var pinColor: Int = 0
private var lastCameraPosition: CameraPosition? = null
val features: @Hot MutableLiveData<Set<Feature>> = MutableLiveData()

override fun initialize(job: Job, task: Task, taskData: TaskData?) {
super.initialize(job, task, taskData)
pinColor = job.getDefaultColor()
}

fun updateCameraPosition(position: CameraPosition) {
lastCameraPosition = position
}
Expand All @@ -55,6 +65,8 @@ constructor(resources: Resources, private val uuidGenerator: OfflineUuidGenerato
id = uuidGenerator.generateUuid(),
type = FeatureType.USER_POINT.ordinal,
geometry = point,
// TODO: Set correct pin color.
style = Feature.Style(pinColor),
clusterable = false
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import kotlin.math.abs
object LatLngConverter {

/** Converts the given coordinates in decimal format to D°M′S″ format. */
fun processCoordinates(coordinates: Coordinates?): String? =
fun formatCoordinates(coordinates: Coordinates?): String? =
coordinates?.let { "${convertLatToDMS(it.lat)} ${convertLongToDMS(it.lng)}" }

private fun convertLatToDMS(lat: Double): String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import com.google.android.ground.model.geometry.LineString
import com.google.android.ground.model.geometry.LinearRing
import com.google.android.ground.model.geometry.Point
import com.google.android.ground.model.geometry.Polygon
import com.google.android.ground.model.job.Job
import com.google.android.ground.model.job.getDefaultColor
import com.google.android.ground.model.submission.GeometryData
import com.google.android.ground.model.submission.TaskData
import com.google.android.ground.model.task.Task
import com.google.android.ground.persistence.uuid.OfflineUuidGenerator
import com.google.android.ground.ui.common.SharedViewModel
import com.google.android.ground.ui.datacollection.tasks.AbstractTaskViewModel
Expand Down Expand Up @@ -61,6 +65,13 @@ internal constructor(private val uuidGenerator: OfflineUuidGenerator, resources:
/** Represents whether the user has completed drawing the polygon or not. */
private var isMarkedComplete: Boolean = false

private var strokeColor: Int = 0

override fun initialize(job: Job, task: Task, taskData: TaskData?) {
super.initialize(job, task, taskData)
strokeColor = job.getDefaultColor()
}

fun isMarkedComplete(): Boolean = isMarkedComplete

/**
Expand Down Expand Up @@ -147,15 +158,16 @@ internal constructor(private val uuidGenerator: OfflineUuidGenerator, resources:
}

/** Returns a set of [Feature] to be drawn on map for the given [Polygon]. */
private fun refreshFeatures(points: List<Point>, isMarkedComplete: Boolean) {
private fun refreshFeatures(vertices: List<Point>, isMarkedComplete: Boolean) {
featureFlow.value =
if (points.isEmpty()) {
if (vertices.isEmpty()) {
null
} else {
Feature(
id = uuidGenerator.generateUuid(),
type = FeatureType.USER_POLYGON.ordinal,
geometry = createGeometry(points, isMarkedComplete),
geometry = createGeometry(vertices, isMarkedComplete),
style = Feature.Style(strokeColor, Feature.VertexStyle.CIRCLE),
clusterable = false
)
}
Expand Down
Loading