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

Add empty waypoint names to HistoryEventMapper#mapNavigationRoute as old records may not include them #5627

Merged
merged 1 commit into from
Mar 30, 2022

Conversation

Guardiola31337
Copy link
Contributor

Description

Adds empty waypoint names to HistoryEventMapper#mapNavigationRoute as old records may not include them, causing parsing issues when replaying old history files.

Follow up from #5614

This mapbox/mapbox-java#1386 is not needed anymore.

cc @SiarheiFedartsou @danpat @mskurydin

@Guardiola31337 Guardiola31337 added bug Defect to be fixed. Core Work related to core navigation and integrations. labels Mar 29, 2022
@Guardiola31337 Guardiola31337 requested a review from a team as a code owner March 29, 2022 19:55
@Guardiola31337 Guardiola31337 self-assigned this Mar 29, 2022
@Guardiola31337
Copy link
Contributor Author

This mapbox/mapbox-java#1386 is not needed anymore.

Revert PR mapbox/mapbox-java#1387

@Guardiola31337 Guardiola31337 force-pushed the pg-history-event-mapper-waypoint-names branch from b4bfddf to 32e6622 Compare March 30, 2022 14:41
@Guardiola31337 Guardiola31337 added the skip changelog Should not be added into version changelog. label Mar 30, 2022
Comment on lines 127 to 137
private fun addWaypointNames(response: String): String {
val gson = GsonBuilder().create()
val jsonObject = gson.fromJson(response, JsonObject::class.java)
val waypoints = jsonObject?.getAsJsonArray(WAYPOINTS_JSON_KEY)
val isNameNotIncluded = waypoints?.get(0)?.asJsonObject?.get(NAME_JSON_KEY) == null
if (isNameNotIncluded) {
waypoints?.forEachIndexed { index, _ ->
jsonObject.getAsJsonArray(WAYPOINTS_JSON_KEY)
.get(index).asJsonObject.addProperty(NAME_JSON_KEY, "")
}
}
return jsonObject.toString()
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private fun addWaypointNames(response: String): String {
val gson = GsonBuilder().create()
val jsonObject = gson.fromJson(response, JsonObject::class.java)
val waypoints = jsonObject?.getAsJsonArray(WAYPOINTS_JSON_KEY)
val isNameNotIncluded = waypoints?.get(0)?.asJsonObject?.get(NAME_JSON_KEY) == null
if (isNameNotIncluded) {
waypoints?.forEachIndexed { index, _ ->
jsonObject.getAsJsonArray(WAYPOINTS_JSON_KEY)
.get(index).asJsonObject.addProperty(NAME_JSON_KEY, "")
}
}
return jsonObject.toString()
}
private fun addWaypointNames(response: String): String {
val gson = GsonBuilder().create()
val jsonObject = gson.fromJson(response, JsonObject::class.java)
val waypoints = jsonObject?.getAsJsonArray(WAYPOINTS_JSON_KEY)?.map { it.asJsonObject }
waypoints?.forEach { waypoint ->
if (!waypoint.has(NAME_JSON_KEY)) {
waypoint.addProperty(NAME_JSON_KEY, "")
}
}
return jsonObject.toString()
}

Copy link
Member

Choose a reason for hiding this comment

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

Simplifying this a little bit (please double check), otherwise, LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It worked :shipit:

… records may not include them causing parsing issues when replaying old history files
@Guardiola31337 Guardiola31337 force-pushed the pg-history-event-mapper-waypoint-names branch from 32e6622 to 581b0a2 Compare March 30, 2022 15:27
@Guardiola31337 Guardiola31337 enabled auto-merge (squash) March 30, 2022 15:28
@Guardiola31337 Guardiola31337 merged commit 73f7d7b into main Mar 30, 2022
@Guardiola31337 Guardiola31337 deleted the pg-history-event-mapper-waypoint-names branch March 30, 2022 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defect to be fixed. Core Work related to core navigation and integrations. skip changelog Should not be added into version changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants