add maxZoom to quotients in /axis api #177
Conversation
WalkthroughThe changes in this pull request introduce a new field named Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range comments (1)
src/main/java/io/kontur/disasterninja/domain/Indicator.java (1)
Line range hint
24-24: Consider addressing existing technical debt.There are two comments indicating technical debt:
- A comment questioning the array of arrays design for the direction field
- A FIXME about the nested Unit class design
Consider creating separate issues to track and address these design concerns.
Would you like me to help create GitHub issues to track these technical debt items?
Also applies to: 29-29
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (6)
src/main/graphql/io/kontur/disasterninja/graphql/AxisList.graphql(1 hunks)src/main/graphql/io/kontur/disasterninja/graphql/schema.graphqls(2 hunks)src/main/java/io/kontur/disasterninja/domain/BivariateLegendQuotient.java(1 hunks)src/main/java/io/kontur/disasterninja/domain/Indicator.java(1 hunks)src/main/java/io/kontur/disasterninja/mapper/AxisListMapper.java(1 hunks)src/main/java/io/kontur/disasterninja/mapper/BivariateStatisticMapper.java(3 hunks)
🔇 Additional comments (4)
src/main/java/io/kontur/disasterninja/domain/Indicator.java (1)
20-21: Verify mappings between GraphQL and domain types.
The maxZoom field implementation looks consistent across files. Let's verify the mappings to ensure proper type conversion.
✅ Verification successful
Type mappings for maxZoom field are consistent and properly handled
The verification shows that the maxZoom field mappings are correctly implemented across the codebase:
- In
BivariateStatisticMapper.java, maxZoom is properly ignored in intermediate mappings and correctly mapped from GraphQL type usingmax_zoom()method - In
AxisListMapper.java, maxZoom is directly mapped from GraphQL quotient usingmaxZoom()method - The Integer type in the domain class matches with the GraphQL schema's type system
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for mapping implementations between GraphQL and domain types
# Look for mapping classes that handle these types
rg -l "BivariateLegendQuotient|Indicator" --type java | grep -i "mapper"
# Check for maxZoom field handling in mappers
rg "maxZoom" --type java | grep -i "mapper"
Length of output: 928
Script:
#!/bin/bash
# Let's check the actual type definitions and mappings in these files
cat src/main/java/io/kontur/disasterninja/mapper/BivariateStatisticMapper.java
cat src/main/java/io/kontur/disasterninja/mapper/AxisListMapper.java
Length of output: 21286
src/main/graphql/io/kontur/disasterninja/graphql/schema.graphqls (1)
71-71: LGTM! Consistent and backward-compatible field addition.
The maxZoom field is appropriately:
- Added to both
IndicatorandQuotienttypes - Defined as an optional
Inttype, maintaining backward compatibility - Named consistently across types
Also applies to: 212-212
src/main/java/io/kontur/disasterninja/mapper/AxisListMapper.java (1)
63-63: LGTM! Straightforward mapping implementation.
The maxZoom field mapping is implemented correctly and consistently with other field mappings in the class.
src/main/java/io/kontur/disasterninja/mapper/BivariateStatisticMapper.java (1)
63-63: Verify the design choice of ignoring maxZoom in these mappings.
The maxZoom field is consistently ignored in all bivariate statistic mappings. While this suggests a deliberate design choice to only populate maxZoom through the axis API, please confirm if this is the intended behavior.
Run the following script to verify the usage pattern:
Also applies to: 111-111, 183-183
✅ Verification successful
Let me analyze the code further to understand the maxZoom usage pattern.
Based on the gathered information, I can now provide the final response:
The design choice of ignoring maxZoom in bivariate mappings is correct
The codebase shows a clear pattern where maxZoom is:
- Only exposed through the AxisList GraphQL query where it's a property of quotients
- Not present in BivariateLayerLegend or BivariateMatrix GraphQL schemas (except in meta)
- Consistently ignored in BivariateStatisticMapper for bivariate-specific mappings
- Properly mapped in AxisListMapper where it belongs
This confirms that maxZoom is intentionally managed through the axis API and should be ignored in bivariate mappings.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if maxZoom is only used in axis-related queries/mutations
# and not in bivariate-related ones
# Search for maxZoom usage in GraphQL queries/mutations
rg -t graphql "maxZoom"
# Search for maxZoom in Java files to understand the access patterns
rg -t java "maxZoom"
Length of output: 2260
Script:
#!/bin/bash
# Check the implementation of BivariateLegendQuotient and related classes
ast-grep --pattern 'class BivariateLegendQuotient {
$$$
}'
# Check the implementation of BivariateStatisticMapper
ast-grep --pattern 'class BivariateStatisticMapper {
$$$
}'
# Check if there are any GraphQL schema files
fd -e graphql -e gql
Length of output: 669
Script:
#!/bin/bash
# Check maxZoom usage in GraphQL schema files
cat src/main/graphql/io/kontur/disasterninja/graphql/BivariateLayerLegend.graphql
cat src/main/graphql/io/kontur/disasterninja/graphql/BivariateMatrix.graphql
cat src/main/graphql/io/kontur/disasterninja/graphql/AxisList.graphql
# Check how maxZoom is used in LayersApiService
rg -A 5 -B 5 "maxZoom" src/main/java/io/kontur/disasterninja/service/layers/LayersApiService.java
Length of output: 6003
src/main/java/io/kontur/disasterninja/domain/BivariateLegendQuotient.java
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
maxZoomfield in thequotientsobject of thegetAxesquery, enhancing data detail.maxZoomfield to theIndicatorandQuotienttypes in the GraphQL schema.Bug Fixes
maxZoomfield appropriately, ensuring correct data transformation.Chores
maxZoomfield.