UI-8742 - More robust MDX cleanup #115
Conversation
| @@ -60,7 +60,7 @@ describe("_cleanupDescendants", () => { | |||
| NON EMPTY Hierarchize( | |||
| Descendants( | |||
| { | |||
| [Booking].[Desk].[ALL].[AllMember].[LegalEntityA] | |||
| [Booking].[Desk].[ALL].[AllMember].[LegalEntityA].[BusinessUnitA].[DeskA].[0] | |||
There was a problem hiding this comment.
We were actually checking something invalid with the previous input:
Descendants(
{
[Booking].[Desk].[ALL].[AllMember].[LegalEntityA]
},
1
)
should NOT be replaced by just LegalEntityA.
Indeed the level at distance 1 from LegalEntityA is BusinessUnit.
However in the new test input:
Descendants(
{
[Booking].[Desk].[ALL].[AllMember].[LegalEntityA].[BusinessUnitA].[DeskA].[0]
},
1
)
the Descendants node should be replaced by the 0 BookId, since it doesn't have descendants anyway.
There was a problem hiding this comment.
Can we also keep a test for the LegalEntityA case which seems to have been broken but that you fixed in this PR (right?)
|
|
||
| const levelsInSet = getLevels(set, { | ||
| cube, | ||
| }).filter((levelCorodinates) => |
There was a problem hiding this comment.
| }).filter((levelCorodinates) => | |
| }).filter((levelCoordinates) => |
| @@ -17,15 +21,26 @@ const canDrilldownLevelBeReplacedByItsFirstArgument = ( | |||
| drilldownLevelNode: MdxFunction, | |||
| cube: Cube, | |||
| ) => { | |||
There was a problem hiding this comment.
This function seems to have been improved in some cases. Don't clean up drilldownlevel if:
- it has several arguments
- something about the levels expressed in the set that are not on the same hierarchy as the first hierarchy found in the set (not sure what)
Can you add unit tests in order to illustrate and test these improvements?
There was a problem hiding this comment.
Replace if both these conditions are met:
- the function has a single argument
- in the input set, the deepest level expressed in the first hierarchy (which is the one being implicitly drilled) is a leaf level
I'll add a comment.
| @@ -60,7 +60,7 @@ describe("_cleanupDescendants", () => { | |||
| NON EMPTY Hierarchize( | |||
| Descendants( | |||
| { | |||
| [Booking].[Desk].[ALL].[AllMember].[LegalEntityA] | |||
| [Booking].[Desk].[ALL].[AllMember].[LegalEntityA].[BusinessUnitA].[DeskA].[0] | |||
There was a problem hiding this comment.
Can we also keep a test for the LegalEntityA case which seems to have been broken but that you fixed in this PR (right?)
Co-authored-by: Benjamin Amelot <[email protected]>
Co-authored-by: Benjamin Amelot <[email protected]>
antoinetissier
deleted the
personal/a.tissier/UI-8742-more-robust-mdx-cleanup
branch
* UI-8742 - More robust MDX cleanup * Bump to latest versions of the SDK * add robustness * format test snapshot * PR feedback * Update src/4.3_to_5.0/_cleanupDrilldownLevel.ts Co-authored-by: Benjamin Amelot <[email protected]> * Update src/4.3_to_5.0/_cleanupDrilldownLevel.ts Co-authored-by: Benjamin Amelot <[email protected]> --------- Co-authored-by: Benjamin Amelot <[email protected]>
* UI-8742 - More robust MDX cleanup * Bump to latest versions of the SDK * add robustness * format test snapshot * PR feedback * Update src/4.3_to_5.0/_cleanupDrilldownLevel.ts Co-authored-by: Benjamin Amelot <[email protected]> * Update src/4.3_to_5.0/_cleanupDrilldownLevel.ts Co-authored-by: Benjamin Amelot <[email protected]> --------- Co-authored-by: Benjamin Amelot <[email protected]>
getIndexOfDeepestLevelExpressedInDescendantsNodewhich has been improved on the SDK side, and allows not to reinvent the wheel here