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

Fix LocationOfInterest display name #2047

Merged
merged 7 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion ground/src/main/java/com/google/android/ground/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ object Config {

// Local db settings.
// TODO(#128): Reset version to 1 before releasing.
const val DB_VERSION = 109
const val DB_VERSION = 110
const val DB_NAME = "ground.db"

// Firebase Cloud Firestore settings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import com.google.android.ground.model.mutation.LocationOfInterestMutation
import com.google.android.ground.model.mutation.Mutation
import com.google.android.ground.model.mutation.Mutation.SyncStatus

/** Alias for a map of properties with string names. */
typealias LoiProperties = Map<String, Any>

/** User-defined locations of interest (LOI) shown on the map. */
data class LocationOfInterest(
/** A system-defined ID for this LOI. */
Expand All @@ -31,9 +34,7 @@ data class LocationOfInterest(
/** The job associated with this LOI. */
val job: Job,
/** A user-specified ID for this location of interest. */
val customId: String? = null,
/** A human readable caption for this location of interest. */
val caption: String? = null,
val customId: String = "",
/** User and time audit info pertaining to the creation of this LOI. */
val created: AuditInfo,
/** User and time audit info pertaining to the last modification of this LOI. */
Expand All @@ -51,7 +52,9 @@ data class LocationOfInterest(
* Whether this LOI was created opportunistically by the user through the Suggest LOI flow, or
* false if the LOI was created by the survey organizer.
*/
val isOpportunistic: Boolean = false
val isOpportunistic: Boolean = false,
/** Custom map of properties for this LOI. Corresponds to the properties field in GeoJSON */
val properties: LoiProperties = mapOf(),
) {

/**
Expand All @@ -66,11 +69,12 @@ data class LocationOfInterest(
surveyId = surveyId,
locationOfInterestId = id,
userId = userId,
customId = customId,
clientTimestamp = lastModified.clientTimestamp,
geometry = geometry,
caption = caption,
submissionCount = submissionCount,
ownerEmail = ownerEmail,
isOpportunistic = isOpportunistic
isOpportunistic = isOpportunistic,
properties = properties
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.android.ground.model.mutation

import com.google.android.ground.model.geometry.Geometry
import com.google.android.ground.model.locationofinterest.LoiProperties
import java.util.Date

data class LocationOfInterestMutation(
Expand All @@ -29,9 +30,10 @@ data class LocationOfInterestMutation(
override val retryCount: Long = 0,
override val lastError: String = "",
val jobId: String = "",
val customId: String = "",
val geometry: Geometry? = null,
val caption: String? = null,
val submissionCount: Int = 0,
val ownerEmail: String? = null,
val isOpportunistic: Boolean = false
val isOpportunistic: Boolean = false,
val properties: LoiProperties = mapOf(),
) : Mutation()
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.google.android.ground.Config
import com.google.android.ground.persistence.local.room.converter.GeometryWrapperTypeConverter
import com.google.android.ground.persistence.local.room.converter.JsonArrayTypeConverter
import com.google.android.ground.persistence.local.room.converter.JsonObjectTypeConverter
import com.google.android.ground.persistence.local.room.converter.LoiPropertiesMapConverter
import com.google.android.ground.persistence.local.room.converter.StyleTypeConverter
import com.google.android.ground.persistence.local.room.dao.*
import com.google.android.ground.persistence.local.room.entity.*
Expand Down Expand Up @@ -64,7 +65,8 @@ import com.google.android.ground.persistence.local.room.fields.*
MutationEntitySyncStatus::class,
OfflineAreaEntityState::class,
StyleTypeConverter::class,
TileSetEntityState::class
TileSetEntityState::class,
LoiPropertiesMapConverter::class,
)
abstract class LocalDatabase : RoomDatabase() {
abstract fun locationOfInterestDao(): LocationOfInterestDao
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,14 @@ fun LocationOfInterest.toLocalDataStoreObject() =
surveyId = surveyId,
jobId = job.id,
state = EntityState.DEFAULT,
caption = caption,
created = created.toLocalDataStoreObject(),
lastModified = lastModified.toLocalDataStoreObject(),
geometry = geometry.toLocalDataStoreObject(),
customId = customId,
submissionCount = submissionCount,
ownerEmail = ownerEmail,
isOpportunistic = isOpportunistic
isOpportunistic = isOpportunistic,
properties = properties,
)

fun LocationOfInterestEntity.toModelObject(survey: Survey): LocationOfInterest =
Expand All @@ -148,9 +149,10 @@ fun LocationOfInterestEntity.toModelObject(survey: Survey): LocationOfInterest =
surveyId = surveyId,
created = created.toModelObject(),
lastModified = lastModified.toModelObject(),
caption = caption,
customId = customId,
geometry = geometry.getGeometry(),
submissionCount = submissionCount,
properties = properties,
job = survey.getJob(jobId = jobId)
?: throw LocalDataConsistencyException(
"Unknown jobId ${this.jobId} in location of interest ${this.id}"
Expand All @@ -173,14 +175,15 @@ fun LocationOfInterestMutation.toLocalDataStoreObject(user: User): LocationOfInt
surveyId = surveyId,
jobId = jobId,
state = EntityState.DEFAULT,
caption = caption,
// TODO(#1562): Preserve creation audit info for UPDATE mutations.
created = auditInfo,
lastModified = auditInfo,
geometry = geometry?.toLocalDataStoreObject(),
customId = customId,
submissionCount = submissionCount,
ownerEmail = ownerEmail,
isOpportunistic = isOpportunistic
isOpportunistic = isOpportunistic,
properties = properties
)
}

Expand All @@ -191,13 +194,14 @@ fun LocationOfInterestMutation.toLocalDataStoreObject() =
jobId = jobId,
type = MutationEntityType.fromMutationType(type),
newGeometry = geometry?.toLocalDataStoreObject(),
caption = caption,
newCustomId = customId,
userId = userId,
locationOfInterestId = locationOfInterestId,
syncStatus = MutationEntitySyncStatus.fromMutationSyncStatus(syncStatus),
clientTimestamp = clientTimestamp.time,
lastError = lastError,
retryCount = retryCount
retryCount = retryCount,
newProperties = properties
)

fun LocationOfInterestMutationEntity.toModelObject() =
Expand All @@ -207,13 +211,14 @@ fun LocationOfInterestMutationEntity.toModelObject() =
jobId = jobId,
type = type.toMutationType(),
geometry = newGeometry?.getGeometry(),
caption = caption,
customId = newCustomId,
userId = userId,
locationOfInterestId = locationOfInterestId,
syncStatus = syncStatus.toMutationSyncStatus(),
clientTimestamp = Date(clientTimestamp),
lastError = lastError,
retryCount = retryCount,
properties = newProperties
)

fun MultipleChoiceEntity.toModelObject(optionEntities: List<OptionEntity>): MultipleChoice {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.android.ground.persistence.local.room.converter
shobhitagarwal1612 marked this conversation as resolved.
Show resolved Hide resolved

import androidx.room.TypeConverter
import com.google.common.reflect.TypeToken
import com.google.gson.Gson
import com.google.gson.JsonSyntaxException
import kotlinx.collections.immutable.toPersistentMap
import org.json.JSONException
import org.json.JSONObject
import timber.log.Timber

object LoiPropertiesMapConverter {
@TypeConverter
fun toString(properties: Map<String, Any>): String {
var jsonString = ""
try {
jsonString = JSONObject(properties).toString()
} catch (e: JSONException) {
Timber.e(e, "Error building JSON")
}
return jsonString
}

@TypeConverter
fun fromString(jsonString: String): Map<String, Any> {
var map = mutableMapOf<String, Any>()
try {
val type = object : TypeToken<Map<String, Any>>() {}.type
map = Gson().fromJson(jsonString, type)
} catch (e: JsonSyntaxException) {
Timber.e(e, "Error parsing JSON string")
}
return map.toPersistentMap()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.android.ground.persistence.local.room.entity

import androidx.room.*
import com.google.android.ground.model.locationofinterest.LoiProperties
import com.google.android.ground.persistence.local.room.fields.EntityState

/**
Expand All @@ -28,11 +29,12 @@ data class LocationOfInterestEntity(
@ColumnInfo(name = "survey_id") val surveyId: String,
@ColumnInfo(name = "job_id") val jobId: String,
@ColumnInfo(name = "state") val state: EntityState, // TODO: Rename to DeletionState.
@ColumnInfo(name = "caption") val caption: String?,
@Embedded(prefix = "created_") val created: AuditInfoEntity,
@Embedded(prefix = "modified_") val lastModified: AuditInfoEntity,
val geometry: GeometryWrapper?,
val customId: String,
val submissionCount: Int,
val ownerEmail: String?,
val isOpportunistic: Boolean
val isOpportunistic: Boolean,
val properties: LoiProperties,
)
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.android.ground.persistence.local.room.entity

import androidx.room.*
import com.google.android.ground.model.locationofinterest.LoiProperties
import com.google.android.ground.persistence.local.room.fields.MutationEntitySyncStatus
import com.google.android.ground.persistence.local.room.fields.MutationEntityType

Expand Down Expand Up @@ -48,7 +49,8 @@ data class LocationOfInterestMutationEntity(
@ColumnInfo(name = "client_timestamp") val clientTimestamp: Long,
@ColumnInfo(name = "location_of_interest_id") val locationOfInterestId: String,
@ColumnInfo(name = "job_id") val jobId: String,
@ColumnInfo(name = "caption") val caption: String?,
/** Non-null if the LOI's geometry was updated, null if unchanged. */
val newGeometry: GeometryWrapper?,
val newProperties: LoiProperties,
val newCustomId: String
)
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ object LoiConverter {
return LocationOfInterest(
id = loiId,
surveyId = survey.id,
customId = loiDoc.customId,
caption = loiDoc.caption,
customId = loiDoc.customId ?: "",
job = job,
created = AuditInfoConverter.toAuditInfo(created),
lastModified = AuditInfoConverter.toAuditInfo(lastModified),
// TODO(#929): Set geometry once LOI has been updated to use our own model.
geometry = geometry,
submissionCount = submissionCount
submissionCount = submissionCount,
properties = loiDoc.properties ?: mapOf()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.android.ground.persistence.remote.firebase.schema

import com.google.android.ground.model.locationofinterest.LoiProperties
import com.google.firebase.firestore.GeoPoint
import com.google.firebase.firestore.IgnoreExtraProperties

Expand All @@ -24,11 +25,11 @@ import com.google.firebase.firestore.IgnoreExtraProperties
data class LoiDocument(
val jobId: String? = null,
val customId: String? = null,
val caption: String? = null,
val location: GeoPoint? = null,
val geoJson: String? = null,
val geometry: Map<String, Any>? = null,
val created: AuditInfoNestedObject? = null,
val lastModified: AuditInfoNestedObject? = null,
val submissionCount: Int? = null
val submissionCount: Int? = null,
val properties: LoiProperties? = null,
)
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class LocationOfInterestHelper @Inject internal constructor(private val resource
locationOfInterest.map(::getLabel).orElse("")

fun getLabel(loi: LocationOfInterest): String {
val caption = loi.caption?.trim { it <= ' ' } ?: ""
// TODO(#2046): Reuse logic from card util to display LOI label
val caption = loi.customId?.trim { it <= ' ' } ?: ""
return caption.ifEmpty { getLocationOfInterestType(loi) }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,16 @@ import com.google.android.ground.util.isNotNullOrEmpty
object LoiCardUtil {

fun getDisplayLoiName(context: Context, loi: LocationOfInterest): String {
val caption = loi.caption
val customId = loi.customId
val loiId =
if (loi.customId.isNotNullOrEmpty()) loi.customId else loi.properties?.get("id")?.toString()
val geometry = loi.geometry
return if (caption.isNotNullOrEmpty() && customId.isNotNullOrEmpty()) {
"$caption ($customId)"
} else if (caption.isNotNullOrEmpty()) {
"$caption"
} else if (customId.isNotNullOrEmpty()) {
"${geometry.toType(context)} ($customId)"
val name: String? = loi.properties?.get("name")?.toString()
return if (name.isNotNullOrEmpty() && loiId.isNotNullOrEmpty()) {
"$name ($loiId)"
} else if (name.isNotNullOrEmpty()) {
"$name"
} else if (loiId.isNotNullOrEmpty()) {
"${geometry.toType(context)} ($loiId)"
} else {
geometry.toDefaultName(context)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,13 @@ class LoiLocalDataStoreConverterTest {
mockLoiDocumentSnapshot(
"loi001",
LoiDocument(
/* jobId */
"job001", /* customId */
null, /* caption */
null, /* location */
null, /* geoJson */
null, /* geometry */
null, /* created */
AUDIT_INFO_1_NESTED_OBJECT, /* lastModified */
AUDIT_INFO_2_NESTED_OBJECT
/* jobId */ "job001",
/* customId */ null,
/* location */ null,
/* geoJson */ null,
/* geometry */ null,
/* created */ AUDIT_INFO_1_NESTED_OBJECT,
/* lastModified */ AUDIT_INFO_2_NESTED_OBJECT
)
)
assertThat(toLocationOfInterest().isFailure).isTrue()
Expand All @@ -93,7 +91,6 @@ class LoiLocalDataStoreConverterTest {
null,
null,
null,
null,
noVerticesGeometry,
AUDIT_INFO_1_NESTED_OBJECT,
AUDIT_INFO_2_NESTED_OBJECT
Expand Down
Loading