search

mysticfall / pivot4j

Pivot4J provides a common API for OLAP servers which can be used to build an analytical service frontend with pivot style GUI.
Other
128 stars 101 forks source link

Wrong table columns rendered with property column #220

Closed d-amelin closed 6 years ago

d-amelin commented 6 years ago

There is mdx querу: select [Dimension1], [Dimension2] on COLUMNS ... Dimension1 members has property "Property1". TableRenderer.propertyCollector setted to rendered "Property1" as table column ).

So, i expects rendered column headers: Dimension1, Property1, Dimension2 But it was: Dimension1, Property1, Property1

Same bug was not noticed in HtmlRendererIT.testComplexMemberProperties. The test expects headers: ... Has coffee bar, Street address, Street address ... But must expects headers: ... Has coffee bar, Street address, Promotion Media ... see https://github.com/mysticfall/pivot4j/blob/master/pivot4j-core/src/test/resources/org/pivot4j/ui/html/complex-member-properties-false-true-false-result.html#L49 https://github.com/mysticfall/pivot4j/blob/master/pivot4j-core/src/test/resources/org/pivot4j/ui/html/complex-member-properties-true-true-false-result.html#L68

The reason is TableRenderer.renderHeaderCorner:915: https://github.com/mysticfall/pivot4j/blob/master/pivot4j-core/src/main/java/org/pivot4j/ui/table/TableRenderer.java#L915 On Dimension1 iteration context.property setted (context.setProperty(property);). But context.property is not cleared after iteration. So, next iterations will render the property caption as header content because getLabel(context) will return context.getProperty().getCaption().

we need clear context.property after property headers rendered.

There are one more bug: property header rendrered at wrong index.

as result Property1 rendered with index 3, but correct index is 2.

we need increment colIndex (1) after property headers rendered, not before.

I'll make PR.

mysticfall commented 6 years ago

Good catch! Thanks much for pointing this out and sending me the fix :) Additional optimization is also nice and I agree with the approach you've taken.

Recently, the CI job has been failing for some reason, so I'll need to fix it first to make a build. I'll try to update the marketplace version when I do that.

Thanks again for the contribution!