-
Notifications
You must be signed in to change notification settings - Fork 62
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
Auto-expand transformation #293
Open
HeikoTheissen
wants to merge
35
commits into
main
Choose a base branch
from
auto-expand
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 15 commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
4750db8
auto-expand transformation
HeikoTheissen 53368e3
Avoid EnumType
HeikoTheissen 64e5ae7
Instance annotations for auto-expanded result sets
HeikoTheissen b5a6f34
Reprased definitions
HeikoTheissen 9238702
SortOrder -> SortOrderType
HeikoTheissen 34b5a64
Avoid <dt>
HeikoTheissen 443f99c
After alignment with @uhlmannm
HeikoTheissen 2fb8ff2
rephrasing
HeikoTheissen f5dea9f
Make `LeafLevel` mandatory
HeikoTheissen 41b289a
Don't call out instance annotation
HeikoTheissen 2df14e4
Merge remote-tracking branch 'origin/main' into auto-expand
HeikoTheissen defafdf
Allowed values for DrillState
HeikoTheissen 8584dfe
Merge remote-tracking branch 'origin/main' into auto-expand
HeikoTheissen f27abe8
auto-refreshed
HeikoTheissen 3ca799d
@uhlmannm's suggestion
HeikoTheissen 92f1abd
Simplifications agreed in V4 table meeting
HeikoTheissen bb78f7d
Example request
HeikoTheissen 4181573
Rephrased
HeikoTheissen b227e7f
Typos
HeikoTheissen e390b0c
@uhlmannm's comments
HeikoTheissen b9ecf4f
explained example
HeikoTheissen b587137
expand N levels
HeikoTheissen ae774a6
renamed type
HeikoTheissen 3cb5082
typo
HeikoTheissen c9bbadf
more links
HeikoTheissen 3e6379a
corrected LeavesCount, ResultEntriesCount
HeikoTheissen f98c999
example formatting
HeikoTheissen 64f96eb
save lines
HeikoTheissen bc35559
round parentheses for in
HeikoTheissen 9f24039
More elegance
HeikoTheissen 8c7b80b
reword: deeper -> finer-grained
HeikoTheissen 4dfd1c6
Sort order like Common.SortOrderType
HeikoTheissen 5036eba
As agreed in meeting on 2024-05-24
HeikoTheissen 192cc17
Explain D vs. A
HeikoTheissen fbe94b9
Merge remote-tracking branch 'origin/main' into auto-expand
HeikoTheissen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,9 @@ | |
}, | ||
"https://sap.github.io/odata-vocabularies/vocabularies/Common.json": { | ||
"$Include": [{ "$Namespace": "com.sap.vocabularies.Common.v1", "$Alias": "Common" }] | ||
}, | ||
"https://sap.github.io/odata-vocabularies/vocabularies/Hierarchy.json": { | ||
"$Include": [{ "$Namespace": "com.sap.vocabularies.Hierarchy.v1", "$Alias": "Hierarchy" }] | ||
} | ||
}, | ||
"com.sap.vocabularies.Analytics.v1": { | ||
|
@@ -161,6 +164,113 @@ | |
"@[email protected]": "Adding a list of other terms that can be annotated to it.", | ||
"@Validation.ApplicableTerms": ["Common.Label"] | ||
} | ||
}, | ||
"AutoExpand": [ | ||
{ | ||
"$Kind": "Function", | ||
"$EntitySetPath": "InputSet", | ||
"$IsBound": true, | ||
"@Common.Experimental": true, | ||
"@Core.IsSignature": true, | ||
"@Core.Description": "`$apply` transformation that expands an unnamed leveled hierarchy with custom aggregation of certain properties", | ||
"$Parameter": [ | ||
{ | ||
"$Name": "InputSet", | ||
"$Collection": true, | ||
"$Type": "Edm.EntityType", | ||
"@Core.Description": "Entity set to be processed" | ||
}, | ||
{ | ||
"$Name": "Levels", | ||
"$Collection": true, | ||
"$Type": "Analytics.AutoExpandLevel", | ||
"@Core.Description": "Collection of aggregation levels forming the visible part of a leveled hierarchy", | ||
"@Core.LongDescription": "Each element in the collection defines the property names for one level.\n A property must not referenced by more than one level.\n The first element in the collection defines the property names of the coarsest level,\n the following elements define the property names of consecutively finer-grained aggregation levels.\n The function result comprises the leveled hierarchy with these levels in preorder.\n All referenced properties must be groupable." | ||
}, | ||
{ | ||
"$Name": "LeafLevel", | ||
"$Collection": true, | ||
"@Core.Description": "A possibly empty set of property names that constitute, together with the property names from `Levels`, the leaf level of the leveled hierarchy", | ||
"@Core.LongDescription": "Property names that occur in `Levels` must not be repeated here.\n All referenced properties must be groupable." | ||
}, | ||
{ | ||
"$Name": "Aggregation", | ||
HeikoTheissen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"$Collection": true, | ||
"@Core.Description": "Properties to aggregate for all result entries on all levels", | ||
"@Core.LongDescription": "All properties in this collection must be custom aggregates." | ||
}, | ||
{ | ||
"$Name": "SiblingOrder", | ||
"$Collection": true, | ||
"$Type": "Analytics.AutoExpandSiblingOrder", | ||
"@Core.Description": "Sort specification to apply to all direct descendants of a given entry in the resulting leveled hierarchy" | ||
}, | ||
{ | ||
"$Name": "BeforeAggregationFilter", | ||
"@Core.Description": "Expression valid for `filter` transformation to restrict the input set before any further procressing", | ||
"@Core.OptionalParameter": {} | ||
}, | ||
{ | ||
"$Name": "AggregatedValuesLeafFilter", | ||
"@Core.Description": "Expression valid for `filter` transformation to restrict the input set at the most detailed grouping level with conditions on aggregated values", | ||
"@Core.OptionalParameter": {} | ||
}, | ||
{ | ||
"$Name": "Skip", | ||
"$Type": "Edm.Int64", | ||
"@Core.Description": "Number of entries to skip from the top of the fully ordered result", | ||
"@Core.OptionalParameter": { "DefaultValue": "0" } | ||
}, | ||
{ | ||
"$Name": "Top", | ||
"$Type": "Edm.Int64", | ||
"@Core.Description": "Number of entries to return from the result set after any skipping (absent means all)", | ||
"@Core.OptionalParameter": {} | ||
}, | ||
{ | ||
"$Name": "ResultEntriesCount", | ||
"$Type": "Edm.Bool", | ||
"@Core.Description": "Whether to return the total number of entries in the result independent of Skip/Top", | ||
"@Core.OptionalParameter": { "DefaultValue": "false" } | ||
} | ||
], | ||
"$ReturnType": { | ||
"$Collection": true, | ||
"$Type": "Edm.EntityType", | ||
"@Core.Description": "Output set including the instance annotation [`LevelInformation`](#LevelInformation)" | ||
} | ||
} | ||
], | ||
"AutoExpandLevel": { | ||
"$Kind": "ComplexType", | ||
"@Common.Experimental": true, | ||
"P": { "$Collection": true, "@Core.Description": "A non-empty set of property names constituting a level" } | ||
HeikoTheissen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
"AutoExpandSiblingOrder": { | ||
"$Kind": "ComplexType", | ||
"@Common.Experimental": true, | ||
"Property": { "@Core.Description": "Property by which to sort" }, | ||
"Order": { | ||
"$Type": "Analytics.SortOrderType", | ||
"$DefaultValue": "Analytics.SortOrderType/Asc", | ||
"@Core.Description": "Sorting direction" | ||
} | ||
}, | ||
"SortOrderType": { | ||
"$Kind": "TypeDefinition", | ||
"$UnderlyingType": "Edm.String", | ||
"@Common.Experimental": true, | ||
"@Validation.AllowedValues": [ | ||
{ "Value": "asc", "@Core.Description": "Sort in ascending order" }, | ||
{ "Value": "desc", "@Core.Description": "Sort in descending order" } | ||
] | ||
}, | ||
"LevelInformation": { | ||
"$Kind": "Term", | ||
"$Type": "Hierarchy.HierarchyType", | ||
"$AppliesTo": ["EntityType"], | ||
"@Common.Experimental": true, | ||
"@Core.Description": "Information about grouping levels in the result set of a request including the [`AutoExpand`](#AutoExpand) transformation" | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder how that would work. If
LeafLevel
is empty then the last level inLevels
is not expanded, cannot be expanded and could as well be written intoLeafLevel
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't
LeafLevel
simply the last entry inLevels
, then?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There can be several properties used for grouping on the leaf level that are not used for visual grouping / group headers.
The interesting part is that we have three things:
I think we had decided somewhen in the past that 3. is not necessary for the AutoExpand function as one can simply as collapsed visual groups to 2. But I wonder whether that decision is still good when introducing ExpandLevels. Should the backend then not know which visual groups are introduced, how grouping is done on leaf level and how deep to expand initially?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The leaf level is just the deepest grouping level, therefore we only need the parameter
Levels
, notLeafLevel
.In the current definition, nodes not mentioned in
ExpandLevels
are completely expanded. If you want the default to be "expand N levels", we need an additional parameter for that N.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can do it like that. But the question is how to denote the leaf level. Is it always the last of the Levels? And hence this last level is not and cannot be expanded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be stated explicitly in the Levels description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look at the FE examples, then on the leaf level there are individual (unaggregated) records. If that is the target state, one could also argue that the last level can still be expanded and below this last level are the unaggregated records.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
odata-vocabularies/vocabularies/Analytics.xml
Line 224 in 8c7b80b