tdf#124736: Sort group field items

Excel expects the group field items to be in ascending order starting
from "<01/02/2010", then "Jan", "Feb", ..., then end with ">01/02/2020".

Change-Id: I29e9b55f43091ed007f59e10dec64f46a37c7d5f
Reviewed-on: https://gerrit.libreoffice.org/70800
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/70815
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>

2 files changed, 115 insertions, 40 deletions

diff --git a/sc/qa/unit/pivottable_filters_test.cxx b/sc/qa/unit/pivottable_filters_test.cxx
index 4f1134c7c686..eea645d7d3c7 100644
--- a/sc/qa/unit/pivottable_filters_test.cxx
+++ b/sc/qa/unit/pivottable_filters_test.cxx
@@ -90,6 +90,7 @@ public:
     void testTdf123923();
     void testTdf123939();
     void testTdf124651();
+    void testTdf124736();
 
     CPPUNIT_TEST_SUITE(ScPivotTableFiltersTest);
 
@@ -134,6 +135,7 @@ public:
     CPPUNIT_TEST(testTdf123923);
     CPPUNIT_TEST(testTdf123939);
     CPPUNIT_TEST(testTdf124651);
+    CPPUNIT_TEST(testTdf124736);
 
     CPPUNIT_TEST_SUITE_END();
 
@@ -2477,6 +2479,60 @@ void ScPivotTableFiltersTest::testTdf124651()
     assertXPath(pDoc, "/x:pivotTableDefinition/x:dataFields/x:dataField", "name", "");
 }
 
+void ScPivotTableFiltersTest::testTdf124736()
+{
+    ScDocShellRef xDocSh = loadDoc("pivot-table/shared-dategroup.", FORMAT_XLSX);
+    CPPUNIT_ASSERT(xDocSh.is());
+
+    std::shared_ptr<utl::TempFile> pXPathFile
+        = ScBootstrapFixture::exportTo(xDocSh.get(), FORMAT_XLSX);
+    xDocSh->DoClose();
+
+    xmlDocPtr pTable = XPathHelper::parseExport(pXPathFile, m_xSFactory,
+                                                "xl/pivotCache/pivotCacheDefinition1.xml");
+    CPPUNIT_ASSERT(pTable);
+
+    assertXPath(pTable,
+                "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:fieldGroup/x:groupItems",
+                "count", "45");
+    // Group items must start with "<05/16/1958", then years sorted ascending, then ">06/11/2009"
+    // They used to have years in the beginning, then "<05/16/1958", then ">06/11/2009".
+    // The "<" and ">" date strings are locale-dependent, so test depends on en_US locale
+    assertXPath(
+        pTable,
+        "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:fieldGroup/x:groupItems/x:s[1]",
+        "v", "<05/16/1958");
+    for (int i = 2; i <= 44; ++i)
+        assertXPath(
+            pTable,
+            "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:fieldGroup/x:groupItems/x:s["
+                + OString::number(i) + "]",
+            "v", OUString::number(1963 + i));
+    assertXPath(
+        pTable,
+        "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:fieldGroup/x:groupItems/x:s[45]",
+        "v", ">06/11/2009");
+
+    // Now check that table references these in correct order (document-dependent, so this is how
+    // it should be in this specific testdoc which shows "<" and ">" values in the end)
+    pTable = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/pivotTables/pivotTable1.xml");
+    CPPUNIT_ASSERT(pTable);
+    assertXPath(pTable, "/x:pivotTableDefinition/x:pivotFields/x:pivotField[1]/x:items", "count",
+                "46");
+    const int vals[] = { 1,  2,  3,  4,  5,  6,  7,  8,  9,  10, 11, 12, 13, 14, 15,
+                         16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
+                         31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 0,  44 };
+    for (size_t i = 0; i < SAL_N_ELEMENTS(vals); ++i)
+    {
+        assertXPath(pTable,
+                    "/x:pivotTableDefinition/x:pivotFields/x:pivotField[1]/x:items/x:item["
+                        + OString::number(i + 1) + "]",
+                    "x", OUString::number(vals[i]));
+    }
+    assertXPath(pTable, "/x:pivotTableDefinition/x:pivotFields/x:pivotField[1]/x:items/x:item[46]",
+                "t", "default");
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(ScPivotTableFiltersTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sc/source/filter/excel/xepivotxml.cxx b/sc/source/filter/excel/xepivotxml.cxx
index 8648f27e7539..694eee0469d3 100644
--- a/sc/source/filter/excel/xepivotxml.cxx
+++ b/sc/source/filter/excel/xepivotxml.cxx
@@ -197,7 +197,42 @@ OUString GetExcelFormattedDate( double fSerialDateTime, SvNumberFormatter& rForm
     ::sax::Converter::convertDateTime(sBuf, aUDateTime, nullptr, true);
     return sBuf.makeStringAndClear();
 }
+
+// Excel seems to expect different order of group item values; we need to rearrange elements
+// to output "<date1" first, then elements, then ">date2" last.
+// Since ScDPItemData::DateFirst is -1, ScDPItemData::DateLast is 10000, and other date group
+// items would fit between those in order (like 0 = Jan, 1 = Feb, etc.), we can simply sort
+// the items by value.
+std::vector<OUString> SortGroupItems(const ScDPCache& rCache, long nDim)
+{
+    struct ItemData
+    {
+        sal_Int32 nVal;
+        const ScDPItemData* pData;
+    };
+    std::vector<ItemData> aDataToSort;
+    ScfInt32Vec aGIIds;
+    rCache.GetGroupDimMemberIds(nDim, aGIIds);
+    for (sal_Int32 id : aGIIds)
+    {
+        const ScDPItemData* pGIData = rCache.GetItemDataById(nDim, id);
+        if (pGIData->GetType() == ScDPItemData::GroupValue)
+        {
+            auto aGroupVal = pGIData->GetGroupValue();
+            aDataToSort.push_back({ aGroupVal.mnValue, pGIData });
+        }
+    }
+    std::sort(aDataToSort.begin(), aDataToSort.end(),
+              [](const ItemData& a, const ItemData& b) { return a.nVal < b.nVal; });
+
+    std::vector<OUString> aSortedResult;
+    for (const auto& el : aDataToSort)
+    {
+        aSortedResult.push_back(rCache.GetFormattedString(nDim, *el.pData, false));
+    }
+    return aSortedResult;
 }
+} // namespace
 
 void XclExpXmlPivotCaches::SavePivotCacheXml( XclExpXmlStream& rStrm, const Entry& rEntry, sal_Int32 nCounter )
 {
@@ -296,17 +331,11 @@ void XclExpXmlPivotCaches::SavePivotCacheXml( XclExpXmlStream& rStrm, const Entr
         pDefStrm->singleElement(XML_rangePr, pGroupAttList);
 
         // groupItems element
-        ScfInt32Vec aGIIds;
-        rCache.GetGroupDimMemberIds(i, aGIIds);
-        pDefStrm->startElement(XML_groupItems, XML_count, OString::number(aGIIds.size()), FSEND);
-        for (auto nGIId : aGIIds)
+        auto aElemVec = SortGroupItems(rCache, i);
+        pDefStrm->startElement(XML_groupItems, XML_count, OString::number(aElemVec.size()), FSEND);
+        for (const auto& sElem : aElemVec)
         {
-            const ScDPItemData* pGIData = rCache.GetItemDataById(i, nGIId);
-            if (pGIData->GetType() == ScDPItemData::GroupValue)
-            {
-                OUString sVal = rCache.GetFormattedString(i, *pGIData, false);
-                pDefStrm->singleElement(XML_s, XML_v, sVal.toUtf8(), FSEND);
-            }
+            pDefStrm->singleElement(XML_s, XML_v, sElem.toUtf8(), FSEND);
         }
         pDefStrm->endElement(XML_groupItems);
         pDefStrm->endElement(XML_fieldGroup);
@@ -894,50 +923,40 @@ void XclExpXmlPivotTables::SavePivotTableXml( XclExpXmlStream& rStrm, const ScDP
             dpo.GetMembers(i, dpo.GetUsedHierarchy(i), aMembers);
         }
 
-        std::vector<const ScDPItemData*> rCacheFieldItems;
+        std::vector<OUString> aCacheFieldItems;
         if (i < rCache.GetFieldCount() && !rCache.GetGroupType(i))
+        {
             for (const auto& it : rCache.GetDimMemberValues(i))
-                rCacheFieldItems.push_back(&it);
+            {
+                OUString sFormattedName;
+                if (it.HasStringData() || it.IsEmpty())
+                    sFormattedName = it.GetString();
+                else
+                    sFormattedName = const_cast<ScDPObject&>(rDPObj).GetFormattedString(
+                        pDim->GetName(), it.GetValue());
+                aCacheFieldItems.push_back(sFormattedName);
+            }
+        }
         else
         {
-            ScfInt32Vec aGIIds;
-            rCache.GetGroupDimMemberIds(i, aGIIds);
-            for (const sal_Int32 id : aGIIds)
-                rCacheFieldItems.push_back(rCache.GetItemDataById(i, id));
+            aCacheFieldItems = SortGroupItems(rCache, i);
         }
-        const auto iCacheFieldItems_begin = rCacheFieldItems.begin(), iCacheFieldItems_end = rCacheFieldItems.end();
         // The pair contains the member index in cache and if it is hidden
         std::vector< std::pair<size_t, bool> > aMemberSequence;
         std::set<size_t> aUsedCachePositions;
         for (const auto & rMember : aMembers)
         {
-            for (auto it = iCacheFieldItems_begin; it != iCacheFieldItems_end; ++it)
+            auto it = std::find(aCacheFieldItems.begin(), aCacheFieldItems.end(), rMember.maName);
+            if (it != aCacheFieldItems.end())
             {
-                OUString sFormattedName;
-                if ((*it)->GetType() == ScDPItemData::GroupValue)
-                {
-                    sFormattedName = rCache.GetFormattedString(i, **it, false);
-                }
-                else if ((*it)->HasStringData() || (*it)->IsEmpty())
-                {
-                    sFormattedName = (*it)->GetString();
-                }
-                else
-                {
-                    sFormattedName = const_cast<ScDPObject&>(rDPObj).GetFormattedString(pDim->GetName(), (*it)->GetValue());
-                }
-                if (sFormattedName == rMember.maName)
-                {
-                    size_t nCachePos = it - iCacheFieldItems_begin;
-                    auto aInserted = aUsedCachePositions.insert(nCachePos);
-                    if (aInserted.second)
-                        aMemberSequence.emplace_back(std::make_pair(nCachePos, !rMember.mbVisible));
-                    break;
-                }
+                size_t nCachePos = static_cast<size_t>(std::distance(aCacheFieldItems.begin(), it));
+                auto aInserted = aUsedCachePositions.insert(nCachePos);
+                if (aInserted.second)
+                    aMemberSequence.emplace_back(std::make_pair(nCachePos, !rMember.mbVisible));
             }
         }
         // Now add all remaining cache items as hidden
-        for (size_t nItem = 0; nItem < rCacheFieldItems.size(); ++nItem)
+        for (size_t nItem = 0; nItem < aCacheFieldItems.size(); ++nItem)
         {
             if (aUsedCachePositions.find(nItem) == aUsedCachePositions.end())
                 aMemberSequence.emplace_back(nItem, true);