tdf#138181 Chart OOXML: fix deleted legend entries of pie charts

The legend of the pie chart showed deleted legend entries too
when VaryColorsByPoint was false.

Change-Id: I6fc978af0db6e2d39d7f451e765d7ef81c73a05c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105943
Tested-by: Jenkins
Tested-by: László Németh <nemeth@numbertext.org>
Reviewed-by: László Németh <nemeth@numbertext.org>

6 files changed, 70 insertions, 53 deletions

diff --git a/chart2/qa/extras/chart2dump/chart2dump.cxx b/chart2/qa/extras/chart2dump/chart2dump.cxx
index 544eec6b5373..7fd7c4a8da61 100644
--- a/chart2/qa/extras/chart2dump/chart2dump.cxx
+++ b/chart2/qa/extras/chart2dump/chart2dump.cxx
@@ -10,7 +10,6 @@
 #include <charttest.hxx>
 #include <com/sun/star/chart2/XChartDocument.hpp>
 #include <com/sun/star/chart/XChartDocument.hpp>
-#include <com/sun/star/container/XNamed.hpp>
 #include <com/sun/star/text/XText.hpp>
 #include <com/sun/star/drawing/HomogenMatrix3.hpp>
 #include <com/sun/star/drawing/LineDash.hpp>
@@ -252,28 +251,6 @@ protected:
             OUString::number(rTransform.Line3.Column1) + ";" + OUString::number(rTransform.Line3.Column2) + ";" + OUString::number(rTransform.Line3.Column3);
     }
 
-    uno::Reference<drawing::XShape> getShapeByName(const uno::Reference<drawing::XShapes>& rShapes, const OUString& rName, bool (*pCondition)(const uno::Reference<drawing::XShape>&) = nullptr)
-    {
-        for (sal_Int32 i = 0; i < rShapes->getCount(); ++i)
-        {
-            uno::Reference<drawing::XShapes> xShapes(rShapes->getByIndex(i), uno::UNO_QUERY);
-            if (xShapes.is())
-            {
-                uno::Reference<drawing::XShape> xRet = getShapeByName(xShapes, rName, pCondition);
-                if (xRet.is())
-                    return xRet;
-            }
-            uno::Reference<container::XNamed> xNamedShape(rShapes->getByIndex(i), uno::UNO_QUERY);
-            if (xNamedShape->getName() == rName)
-            {
-                uno::Reference<drawing::XShape> xShape(xNamedShape, uno::UNO_QUERY);
-                if (pCondition == nullptr || (*pCondition)(xShape))
-                    return xShape;
-            }
-        }
-        return uno::Reference<drawing::XShape>();
-    }
-
 private:
     OUString         m_sTestFileName;
     bool             m_bDumpMode;
diff --git a/chart2/qa/extras/chart2export.cxx b/chart2/qa/extras/chart2export.cxx
index 9637aebd4e53..a39b36234862 100644
--- a/chart2/qa/extras/chart2export.cxx
+++ b/chart2/qa/extras/chart2export.cxx
@@ -183,6 +183,7 @@ public:
     void testDataLabelPlacementPieChart();
     void testTdf137917();
     void testTdf138204();
+    void testTdf138181();
 
     CPPUNIT_TEST_SUITE(Chart2ExportTest);
     CPPUNIT_TEST(testErrorBarXLSX);
@@ -328,6 +329,7 @@ public:
     CPPUNIT_TEST(testDataLabelPlacementPieChart);
     CPPUNIT_TEST(testTdf137917);
     CPPUNIT_TEST(testTdf138204);
+    CPPUNIT_TEST(testTdf138181);
 
     CPPUNIT_TEST_SUITE_END();
 
@@ -2970,7 +2972,6 @@ void Chart2ExportTest::testDataLabelPlacementPieChart()
     sal_Int32 nLabelPlacement = 0;
     CPPUNIT_ASSERT(aAny >>= nLabelPlacement);
     CPPUNIT_ASSERT_EQUAL(chart::DataLabelPlacement::OUTSIDE, nLabelPlacement);
-
 }
 
 void Chart2ExportTest::testTdf137917()
@@ -3011,6 +3012,32 @@ void Chart2ExportTest::testTdf138204()
     //CPPUNIT_ASSERT_EQUAL(OUString("67.5%"), aFields[0]->getString()); TODO: Not implemented yet
 }
 
+void Chart2ExportTest::testTdf138181()
+{
+    load("/chart2/qa/extras/data/xlsx/", "piechart_deleted_legendentry.xlsx");
+    Reference<chart::XChartDocument> xChartDoc(getChartDocFromSheet(0, mxComponent),
+                                               UNO_QUERY_THROW);
+    Reference<drawing::XDrawPageSupplier> xDrawPageSupplier(xChartDoc, UNO_QUERY_THROW);
+    Reference<drawing::XDrawPage> xDrawPage(xDrawPageSupplier->getDrawPage(), UNO_SET_THROW);
+    Reference<drawing::XShapes> xShapes(xDrawPage->getByIndex(0), UNO_QUERY_THROW);
+    Reference<drawing::XShape> xLegendEntry1, xLegendEntry2, xLegendEntry3;
+
+    // first legend entry is visible
+    xLegendEntry1
+        = getShapeByName(xShapes, "CID/MultiClick/D=0:CS=0:CT=0:Series=0:Point=0:LegendEntry=0");
+    CPPUNIT_ASSERT(xLegendEntry1.is());
+
+    // second legend entry is not visible
+    xLegendEntry2
+        = getShapeByName(xShapes, "CID/MultiClick/D=0:CS=0:CT=0:Series=0:Point=1:LegendEntry=0");
+    CPPUNIT_ASSERT(!xLegendEntry2.is());
+
+    // third legend entry is visible
+    xLegendEntry3
+        = getShapeByName(xShapes, "CID/MultiClick/D=0:CS=0:CT=0:Series=0:Point=2:LegendEntry=0");
+    CPPUNIT_ASSERT(xLegendEntry3.is());
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(Chart2ExportTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/chart2/qa/extras/chart2import.cxx b/chart2/qa/extras/chart2import.cxx
index 8e8f0886d50f..a0b9c00dd24d 100644
--- a/chart2/qa/extras/chart2import.cxx
+++ b/chart2/qa/extras/chart2import.cxx
@@ -27,7 +27,6 @@
 #include <com/sun/star/chart2/TickmarkStyle.hpp>
 #include <com/sun/star/chart2/SymbolStyle.hpp>
 #include <com/sun/star/chart2/Symbol.hpp>
-#include <com/sun/star/container/XNamed.hpp>
 #include <com/sun/star/chart2/data/XTextualDataSequence.hpp>
 #include <com/sun/star/chart/DataLabelPlacement.hpp>
 #include <com/sun/star/text/XTextRange.hpp>
@@ -299,31 +298,6 @@ private:
 
 };
 
-static uno::Reference<drawing::XShape>
-getShapeByName(const uno::Reference<drawing::XShapes>& rShapes, const OUString& rName,
-               const std::function<bool(const uno::Reference<drawing::XShape>&)>& pCondition
-               = nullptr)
-{
-    for (sal_Int32 i = 0; i < rShapes->getCount(); ++i)
-    {
-        uno::Reference<drawing::XShapes> xShapes(rShapes->getByIndex(i), uno::UNO_QUERY);
-        if (xShapes.is())
-        {
-            uno::Reference<drawing::XShape> xRet = getShapeByName(xShapes, rName, pCondition);
-            if (xRet.is())
-                return xRet;
-        }
-        uno::Reference<container::XNamed> xNamedShape(rShapes->getByIndex(i), uno::UNO_QUERY);
-        if (xNamedShape->getName() == rName)
-        {
-            uno::Reference<drawing::XShape> xShape(xNamedShape, uno::UNO_QUERY);
-            if (pCondition == nullptr || pCondition(xShape))
-                return xShape;
-        }
-    }
-    return uno::Reference<drawing::XShape>();
-}
-
 // error bar import
 // split method up into smaller chunks for more detailed tests
 void Chart2ImportTest::Fdo60083()
diff --git a/chart2/qa/extras/charttest.hxx b/chart2/qa/extras/charttest.hxx
index 04794bbeb607..b696231a6c54 100644
--- a/chart2/qa/extras/charttest.hxx
+++ b/chart2/qa/extras/charttest.hxx
@@ -19,6 +19,7 @@
 
 #include <com/sun/star/sheet/XSpreadsheetDocument.hpp>
 #include <com/sun/star/container/XIndexAccess.hpp>
+#include <com/sun/star/container/XNamed.hpp>
 #include <com/sun/star/table/XTableChartsSupplier.hpp>
 #include <com/sun/star/table/XTableChart.hpp>
 #include <com/sun/star/table/XTablePivotChartsSupplier.hpp>
@@ -614,4 +615,29 @@ awt::Size ChartTest::getSize(css::uno::Reference<chart2::XDiagram> xDiagram, con
     return aSize;
 }
 
+uno::Reference<drawing::XShape>
+getShapeByName(const uno::Reference<drawing::XShapes>& rShapes, const OUString& rName,
+               const std::function<bool(const uno::Reference<drawing::XShape>&)>& pCondition
+               = nullptr)
+{
+    for (sal_Int32 i = 0; i < rShapes->getCount(); ++i)
+    {
+        uno::Reference<drawing::XShapes> xShapes(rShapes->getByIndex(i), uno::UNO_QUERY);
+        if (xShapes.is())
+        {
+            uno::Reference<drawing::XShape> xRet = getShapeByName(xShapes, rName, pCondition);
+            if (xRet.is())
+                return xRet;
+        }
+        uno::Reference<container::XNamed> xNamedShape(rShapes->getByIndex(i), uno::UNO_QUERY);
+        if (xNamedShape->getName() == rName)
+        {
+            uno::Reference<drawing::XShape> xShape(xNamedShape, uno::UNO_QUERY);
+            if (pCondition == nullptr || pCondition(xShape))
+                return xShape;
+        }
+    }
+    return uno::Reference<drawing::XShape>();
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/chart2/qa/extras/data/xlsx/piechart_deleted_legendentry.xlsx b/chart2/qa/extras/data/xlsx/piechart_deleted_legendentry.xlsx
new file mode 100644
index 000000000000..8428686ff6ab
--- /dev/null
+++ b/chart2/qa/extras/data/xlsx/piechart_deleted_legendentry.xlsx
diff --git a/chart2/source/view/charttypes/VSeriesPlotter.cxx b/chart2/source/view/charttypes/VSeriesPlotter.cxx
index 132903ba4d30..f405396e113c 100644
--- a/chart2/source/view/charttypes/VSeriesPlotter.cxx
+++ b/chart2/source/view/charttypes/VSeriesPlotter.cxx
@@ -2669,9 +2669,22 @@ std::vector< ViewLegendEntry > VSeriesPlotter::createLegendEntriesForSeries(
         ViewLegendEntry aEntry;
         OUString aLabelText;
         bool bVaryColorsByPoint = rSeries.isVaryColorsByPoint();
-        if (bVaryColorsByPoint
-            || m_xChartTypeModel->getChartType().equalsIgnoreAsciiCase(
-                CHART2_SERVICE_NAME_CHARTTYPE_PIE))
+        bool bIsPie = m_xChartTypeModel->getChartType().equalsIgnoreAsciiCase(
+            CHART2_SERVICE_NAME_CHARTTYPE_PIE);
+        try
+        {
+            if (bIsPie && m_xChartTypeModelProps.is())
+            {
+                bool bDonut = false;
+                if ((m_xChartTypeModelProps->getPropertyValue("UseRings") >>= bDonut) && bDonut)
+                    bIsPie = false;
+            }
+        }
+        catch (const uno::Exception&)
+        {
+        }
+
+        if (bVaryColorsByPoint || bIsPie)
         {
             Sequence< OUString > aCategoryNames;
             if( m_pExplicitCategoriesProvider )