sc/qa/unit/data/xlsx/pivot.xlsx       |binary
 sc/qa/unit/subsequent_export-test.cxx |   61 +++++++++++++++++++++++++-
 sc/source/filter/excel/xepivotxml.cxx |   79 +++++++++++++++++++++++++++-------
 3 files changed, 123 insertions(+), 17 deletions(-)

New commits:
commit 084a1fc85f2e9099b89b7a0bc519a8d482645155
Author: Bartosz Kosiorek <gan...@poczta.onet.pl>
Date:   Tue Aug 1 05:40:34 2017 +0200

    tdf#89139 tdf#109016 Fix PivotCache fields according to OOXML specification
    
    Apply changes to fields:
     - XML_containsInteger
     - XML_containsBlank
     - XML_containsMixedTypes
     - XML_containsSemiMixedTypes
     - XML_count
    According to OOXML specification
    
https://technet.microsoft.com/en-us/library/documentformat.openxml.spreadsheet.shareditems.aspx
    
    Generally OOXML specification allows listing items for mixed items 
(example: STRING + NUMBERS).
    This patch is fixing that.
    Example of mixed types:
            <cacheField name="pwdLastSet" numFmtId="0">
                <sharedItems containsDate="1" containsBlank="1" 
containsMixedTypes="1" minDate="2014-07-07T09:30:31" 
maxDate="2017-03-24T08:38:46" count="4">
                    <m/>
                    <d v="2016-06-15T12:18:34"/>
                    <s v=""/>
                    <d v="2017-03-21T10:27:39"/>
                </sharedItems>
            </cacheField>
    
    Cherry-picked from Change-Id: I02b07c79bea60890e3c995dd70cb5c72901a3d4a
    
    Change-Id: I384629de7dc64dd1c62e1fab24df5595b8d6f03e
    Reviewed-on: https://gerrit.libreoffice.org/40970
    Reviewed-by: Andras Timar <andras.ti...@collabora.com>
    Tested-by: Andras Timar <andras.ti...@collabora.com>

diff --git a/sc/qa/unit/data/xlsx/pivot.xlsx b/sc/qa/unit/data/xlsx/pivot.xlsx
new file mode 100644
index 000000000000..e6297a91777b
Binary files /dev/null and b/sc/qa/unit/data/xlsx/pivot.xlsx differ
diff --git a/sc/qa/unit/subsequent_export-test.cxx 
b/sc/qa/unit/subsequent_export-test.cxx
index 9deb04ba8be0..514f08c68a54 100644
--- a/sc/qa/unit/subsequent_export-test.cxx
+++ b/sc/qa/unit/subsequent_export-test.cxx
@@ -108,7 +108,7 @@ public:
     void testCellNoteExportXLS();
     void testFormatExportODS();
 
-
+    void testPivotExportXLSX();
     void testCommentExportXLSX();
     void testCustomColumnWidthExportXLSX();
     void testXfDefaultValuesXLSX();
@@ -212,6 +212,7 @@ public:
     CPPUNIT_TEST(testCellNoteExportXLS);
     CPPUNIT_TEST(testFormatExportODS);
 
+    CPPUNIT_TEST(testPivotExportXLSX);
     CPPUNIT_TEST(testCommentExportXLSX);
     CPPUNIT_TEST(testCustomColumnWidthExportXLSX);
     CPPUNIT_TEST(testXfDefaultValuesXLSX);
@@ -504,6 +505,64 @@ void ScExportTest::testFormatExportODS()
     xDocSh->DoClose();
 }
 
+void ScExportTest::testPivotExportXLSX()
+{
+    //tdf#89139 FILESAVE  xlsx pivot table corrupted after save with LO and 
re-open with MS Office
+    ScDocShellRef xShell = loadDoc("pivot.", FORMAT_XLSX);
+    CPPUNIT_ASSERT(xShell.Is());
+
+    std::shared_ptr<utl::TempFile> pXPathFile = 
ScBootstrapFixture::exportTo(&(*xShell), FORMAT_XLSX);
+    xmlDocPtr pSheet = XPathHelper::parseExport(pXPathFile, m_xSFactory, 
"xl/pivotCache/pivotCacheDefinition1.xml");
+    CPPUNIT_ASSERT(pSheet);
+
+    assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField", 
5);
+
+    // Four strings and one empty field
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]", "name", 
"imieinazwisko");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", 
"containsBlank", "1");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", 
"containsMixedTypes");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", 
"containsSemiMixedTypes");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", 
"containsString");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", 
"containsNumber");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", 
"containsInteger");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", 
"minValue");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", 
"maxValue");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "count", 
"5");
+
+    // Two integers and one empty field
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]", "name", "wartosc");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", 
"containsBlank", "1");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", 
"containsMixedTypes");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", 
"containsSemiMixedTypes");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", 
"containsString", "0");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", 
"containsNumber", "1");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", 
"containsInteger", "1");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", 
"minValue", "111");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", 
"maxValue", "222");
+
+    // Five integers
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]", "name", "druga 
wartosc");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", 
"containsBlank");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", 
"containsMixedTypes");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", 
"containsSemiMixedTypes", "0");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", 
"containsString", "0");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", 
"containsNumber", "1");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", 
"containsInteger", "1");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", 
"minValue", "1111");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", 
"maxValue", "5555");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "count");
+
+    // Three integers and one string
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]", "name", "trzecia 
wartosc");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", 
"containsBlank");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", 
"containsMixedTypes", "1");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", 
"containsSemiMixedTypes");
+    assertXPathNoAttribute(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", 
"containsString");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", 
"containsNumber", "1");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", 
"containsInteger", "1");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", 
"minValue", "1234");
+    assertXPath(pSheet, 
"/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", 
"maxValue", "5678");
+}
 
 void ScExportTest::testCommentExportXLSX()
 {
diff --git a/sc/source/filter/excel/xepivotxml.cxx 
b/sc/source/filter/excel/xepivotxml.cxx
index 37b94168e193..80a85316d01c 100644
--- a/sc/source/filter/excel/xepivotxml.cxx
+++ b/sc/source/filter/excel/xepivotxml.cxx
@@ -234,6 +234,8 @@ void XclExpXmlPivotCaches::SavePivotCacheXml( 
XclExpXmlStream& rStrm, const Entr
 
         std::set<ScDPItemData::Type> aDPTypes;
         double fMin = std::numeric_limits<double>::infinity(), fMax = 
-std::numeric_limits<double>::infinity();
+        bool isValueInteger = true;
+        double intpart;
         for (; it != itEnd; ++it)
         {
             ScDPItemData::Type eType = it->GetType();
@@ -243,31 +245,76 @@ void XclExpXmlPivotCaches::SavePivotCacheXml( 
XclExpXmlStream& rStrm, const Entr
                 double fVal = it->GetValue();
                 fMin = std::min(fMin, fVal);
                 fMax = std::max(fMax, fVal);
+                // Check if all values are integers
+                if (isValueInteger && (modf(fVal, &intpart) != 0.0))
+                {
+                    isValueInteger = false;
+                }
             }
         }
 
         auto aDPTypeEnd = aDPTypes.cend();
 
         auto pAttList = sax_fastparser::FastSerializerHelper::createAttrList();
-        // tdf#89139: Only create item list for string-only fields.
-        // Using containsXXX attributes in this case makes Excel think the 
file is corrupted.
-        // OTOH listing items for e.g. number fields also triggers "corrupted" 
warning in Excel.
-        bool bListItems = aDPTypes.size() == 1 && 
aDPTypes.find(ScDPItemData::String) != aDPTypeEnd;
-        if (bListItems)
+
+        bool bListItems = true;
+
+        std::set<ScDPItemData::Type> aDPTypesWithoutBlank = aDPTypes;
+        aDPTypesWithoutBlank.erase(ScDPItemData::Empty);
+
+        bool isContainsMoreThanOneType = aDPTypesWithoutBlank.size() > 1;
+        // XML_containsMixedType possible values:
+        // 1 - field contains more than one data type
+        // 0 - (Default) only one data type. The field can still contain blank 
values (that's why we are using aDPTypesWithoutBlank)
+        if (isContainsMoreThanOneType)
+            pAttList->add(XML_containsMixedTypes, XclXmlUtils::ToPsz10(true));
+
+        bool isContainsString = 
aDPTypesWithoutBlank.find(ScDPItemData::String) != aDPTypesWithoutBlank.end();
+        // XML_containsSemiMixedTypes possible values:
+        // 1 - (Default) at least one text value, or can also contain a mix of 
other data types and blank values
+        // 0 - the field does not have a mix of text and other values
+        if (!(isContainsString || (aDPTypes.size() > 1)))
+            pAttList->add(XML_containsSemiMixedTypes, 
XclXmlUtils::ToPsz10(false));
+
+        // default for containsString field is true, so we are writing only 
when is false
+        if (!isContainsString)
+            pAttList->add(XML_containsString, XclXmlUtils::ToPsz10(false));
+
+        bool isContainsBlank = aDPTypes.find(ScDPItemData::Empty) != 
aDPTypeEnd;
+        if (isContainsBlank)
+            pAttList->add(XML_containsBlank, XclXmlUtils::ToPsz10(true));
+
+        bool isContainsNumber = aDPTypesWithoutBlank.find(ScDPItemData::Value) 
!= aDPTypesWithoutBlank.end();
+        if (isContainsNumber)
+            pAttList->add(XML_containsNumber, XclXmlUtils::ToPsz10(true));
+
+        if (isValueInteger && isContainsNumber)
+            pAttList->add(XML_containsInteger, XclXmlUtils::ToPsz10(true));
+
+
+        // Number type fields could be mixed with blank types, and it 
shouldn't be treated as listed items.
+        // Example:
+        //    <cacheField name="employeeID" numFmtId="0">
+        //        <sharedItems containsString="0" containsBlank="1" 
containsNumber="1" containsInteger="1" minValue="35" maxValue="89"/>
+        //    </cacheField>
+        if (isContainsNumber)
         {
-            pAttList->add(XML_count, 
OString::number(static_cast<long>(rFieldItems.size())));
+            pAttList->add(XML_minValue, OString::number(fMin));
+            pAttList->add(XML_maxValue, OString::number(fMax));
+            // If there is only numeric types, then we shouldn't list all items
+            if  (aDPTypesWithoutBlank.size() == 1)
+                bListItems = false;
         }
-        else
+
+        if (isContainsBlank && (aDPTypes.size() == 1))
         {
-            pAttList->add(XML_containsMixedTypes, 
XclXmlUtils::ToPsz10(aDPTypes.size() > 1));
-            pAttList->add(XML_containsSemiMixedTypes, 
XclXmlUtils::ToPsz10(aDPTypes.size() > 1));
-            pAttList->add(XML_containsString, 
XclXmlUtils::ToPsz10(aDPTypes.find(ScDPItemData::String) != aDPTypeEnd));
-            if (aDPTypes.find(ScDPItemData::Value) != aDPTypeEnd)
-            {
-                pAttList->add(XML_containsNumber, XclXmlUtils::ToPsz10(true));
-                pAttList->add(XML_minValue, OString::number(fMin));
-                pAttList->add(XML_maxValue, OString::number(fMax));
-            }
+            // If all items are blank, then we shouldn't list all items
+            bListItems = false;
+        }
+
+        if (bListItems)
+        {
+            pAttList->add(XML_count, 
OString::number(static_cast<long>(rFieldItems.size())));
         }
         sax_fastparser::XFastAttributeListRef xAttributeList(pAttList);
 
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to