Analytics API: Add Options aggregation and disaggregation #19111
Conversation
…)" labels [2.39-DHIS2-17955-backport]
…gation [2.42-DHIS2-18368]
|
| @@ -861,6 +867,17 @@ public boolean isOutputFormat(OutputFormat format) { | |||
| return this.outputFormat != null && this.outputFormat == format; | |||
| } | |||
|
|
|||
| public boolean hasOptionSetInDimensionItems() { | |||
| return dimensions.stream() | |||
There was a problem hiding this comment.
Please, simplify this logic by using less streams.
| // choose options from an option set and display those as disaggregated data items. | ||
| // This selection is non-relative and will not automatically include | ||
| // any options added to the option set in the future. | ||
| ABSOLUTE; |
There was a problem hiding this comment.
I did not understand the ABSOLUTE... We need to cover only AGGREGATED and DISAGGREGATED params.
| private static final Pattern COMPOSITE_DIM_OBJECT_PATTERN = | ||
| Pattern.compile("(?<id1>\\w+)\\.(?<id2>\\w+|\\*)(\\.(?<id3>\\w+|\\*))?"); | ||
| Pattern.compile( | ||
| "(?<id1>\\w+)\\.(?<id2>\\w+|\\*)(\\.(?<id3>\\w+|\\*))?(-?(?<suffix>AGGREGATED|DISAGGREGATED|ABSOLUTE)?)?"); |
There was a problem hiding this comment.
Again, what's the purpose of accepting ABSOLUTE?
| @@ -751,6 +757,24 @@ protected String getAggregateClause(EventQueryParams params) { | |||
| } | |||
| } | |||
|
|
|||
| private AggregationType getAggregationType(EventQueryParams params) { | |||
|
|
|||
There was a problem hiding this comment.
Do not need this blank line.
| @@ -751,6 +757,24 @@ protected String getAggregateClause(EventQueryParams params) { | |||
| } | |||
| } | |||
|
|
|||
| private AggregationType getAggregationType(EventQueryParams params) { | |||
|
|
|||
| if (params.getValue() instanceof DataElement | |||
There was a problem hiding this comment.
We can write it cleaner.
This seems too much to be inside an IF statement:
params
.getOptionSetSelectionCriteria()
.getOptionSetSelectionModes()
.get(
params.getValue().getUid()
+ "."
+ ((DataElement) params.getValue()).getOptionSet().getUid()
| @@ -90,6 +90,13 @@ public List<EventQueryParams> planAggregateQuery(EventQueryParams params) { | |||
| return withTableNameAndPartitions(queries); | |||
| } | |||
|
|
|||
| @Override | |||
| public List<EventQueryParams> planQuery(EventQueryParams params) { | |||
| List<EventQueryParams> queries = Lists.newArrayList(params); | |||
There was a problem hiding this comment.
Could we use plain Java instead of the Guava API?
| @@ -105,6 +105,8 @@ | |||
| @Slf4j | |||
| @Service("org.hisp.dhis.analytics.AnalyticsTableManager") | |||
| public class JdbcAnalyticsTableManager extends AbstractJdbcTableManager { | |||
| // deo.optionsetuid as optionsetuid, | |||
There was a problem hiding this comment.
Do not forget to clean-up it.
|
|
||
| OptionSet optionSet = dataElement.getOptionSet(); | ||
| if (optionSet != null) { | ||
| map.put( |
There was a problem hiding this comment.
Please, make it more readable. Create the variables (key/value) before adding to the map, so we can have map.put(key, value);
larshelge
changed the title
TBD