Fix LocationOfInterest display name #2047
Conversation
* Include properties field in Android Loi dataclasses. * Propogate customId field to local database. * Remove caption field.
ground/src/main/java/com/google/android/ground/model/locationofinterest/LocationOfInterest.kt
Outdated
Show resolved
Hide resolved
| val isOpportunistic: Boolean = false | ||
| val isOpportunistic: Boolean = false, | ||
| /** Custom map of properties for this LOI. Corresponds to the properties field in GeoJSON */ | ||
| val properties: Map<String, Any>? = null, |
There was a problem hiding this comment.
Also, to avoid dual nil states (empty vs null), can we make this non-null and default to empty map in the local and remote Converters if not set in persistent storage?
ground/src/main/java/com/google/android/ground/model/mutation/LocationOfInterestMutation.kt
Outdated
Show resolved
Hide resolved
ground/src/main/java/com/google/android/ground/model/mutation/LocationOfInterestMutation.kt
Outdated
Show resolved
Hide resolved
...java/com/google/android/ground/persistence/local/room/converter/LoiPropertiesMapConverter.kt
Show resolved
Hide resolved
...java/com/google/android/ground/persistence/local/room/converter/LoiPropertiesMapConverter.kt
Outdated
Show resolved
Hide resolved
...java/com/google/android/ground/persistence/local/room/converter/LoiPropertiesMapConverter.kt
Outdated
Show resolved
Hide resolved
| var map = mutableMapOf<String, Any>() | ||
| try { | ||
| val type = object: TypeToken<Map<String, Any>>(){}.type | ||
| map = Gson().fromJson( |
There was a problem hiding this comment.
@JSunde introduced proto (de)serialization for local storage. Can we use that instead of packing/unpacking Json ourselves?
There was a problem hiding this comment.
Is this referring to JsonObjectTypeConverter?
There was a problem hiding this comment.
It's being done in GeometryWrapperTypeConverter usingProtoBuf.encodeToByteArray(). Can we use the same here?
There was a problem hiding this comment.
Hmmm so using ProtoBuf.encodeToByteArray() throws a Serialization Exception because kotlin doesn't have a serializer for the Any class http://screen/H4ovCcLxGR3MvZA
This medium article suggests either creating a custom serializer, or defining a stricter type, or using Gson. https://medium.com/@kerem_balic/serializer-for-class-any-not-found-how-do-we-fix-it-in-kotlin-cc196d92b7b3
What are your thoughts?
There was a problem hiding this comment.
Yikes, sorry I hadn't thought of that. Since we're adding properties to the model, perhaps now would be a good time to add stricter typing? Afaik there are only two possible types for properties values: String and Number.
Assuming the proto serializer can handle generics, can we define an interface PropertyValue<T> with one member T value, which is implemented by NumberValue: PropertyValue<Number> and StringValue: PropertyValue<String>?
There was a problem hiding this comment.
We chatted. We can make this change in a follow-up PR!
...nd/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/LoiDocument.kt
Outdated
Show resolved
Hide resolved
gino-m
changed the title
|
@rachaprince Looks like unit tests are failing. Relevant build log lines: |
Codecov Report
@@ Coverage Diff @@
## master #2047 +/- ##
============================================
+ Coverage 48.29% 48.47% +0.18%
- Complexity 1191 1201 +10
============================================
Files 317 318 +1
Lines 6508 6531 +23
Branches 644 648 +4
============================================
+ Hits 3143 3166 +23
+ Misses 3032 3029 -3
- Partials 333 336 +3
|
Fixes #1559
Update the way LOI customId and properties fields are used to display the LOI