CDA-41: Add Location Alias Inclusion Flag to getOne #1313
Conversation
|
I'm curious about LocationAlias switching from the locationLevelId field to categoryId-groupId. How and when are they different, what does that change do? |
|
The categoryId-groupId alias field is the correct format, as seen in the example input of the initial issue. It can also been seen here in code: https://github.com/USACE/cwms-data-api/blame/e56603d0de8c393326c1dc089250177c7c44163b/cwms-data-api/src/main/java/cwms/cda/data/dao/LocationsDaoImpl.java#L576. I mistakenly set this value to the locationId, so this was a fix. I also applied it to my recent alias change for location levels. |
MikeNeilson
left a comment
MikeNeilson
left a comment
There was a problem hiding this comment.
I'm with Ryan. Field should be null and not render if aliases not requested and empty array if requested and no aliases.
…check to integration test.
MikeNeilson
left a comment
MikeNeilson
left a comment
There was a problem hiding this comment.
Some thoughts, but nothing that rises to the level of "we absolutely must make the change."
| retVal.setPropertyNamingStrategy(PropertyNamingStrategies.KEBAB_CASE); | ||
| retVal.setSerializationInclusion(JsonInclude.Include.NON_NULL); | ||
| retVal.registerModule(new JavaTimeModule()); | ||
| retVal.registerModule(new Jdk8Module()); |
There was a problem hiding this comment.
excellent, saved by a simple plugin.
| if (first.getAliases().isPresent() && second.getAliases().isPresent()) { | ||
| assertEquals(first.getAliases().get().size(), second.getAliases().get().size(), | ||
| "Alias list sizes do not match"); | ||
| for (LocationAlias alias : first.getAliases().get()) { |
There was a problem hiding this comment.
Not that it matters given these arrays are small but I can't help but think there's a cleaner way to do this.
Though perhaps the Aliases should be a Set, and not even an ordered one, might make the comparison easier, I'm pretty sure we can't have duplicate alias names.
But that doesn't need to happen this PR. and the duplication here make sense as you're providing more specific asserts for diagnostics.
3 participants
Fixes #1036
Adds support for including aliases in location getOne results.
Includes alias formatting fixes for location level DAO and tests.