sc/inc/globstr.hrc | 2 sc/qa/uitest/calc_tests8/dataProvider.py | 52 +++++++++++++++ sc/qa/uitest/data/tdf169049_Source.csv | 9 ++ sc/qa/uitest/data/tdf169049_WithLabel.ods |binary sc/source/ui/inc/dataproviderdlg.hxx | 2 sc/source/ui/miscdlgs/dataproviderdlg.cxx | 103 +++++++++++++++++++++--------- 6 files changed, 138 insertions(+), 30 deletions(-)
New commits: commit 4069facf79f37df2e6b9039c18b3ed78ab9c4d2a Author: Regina Henschel <[email protected]> AuthorDate: Sun Dec 7 20:03:53 2025 +0100 Commit: Regina Henschel <[email protected]> CommitDate: Fri Dec 19 21:39:31 2025 +0100 tdf#169049 ensure valid range for preview document The data provider uses a temporary document for the preview. The sorting transformation needs a range on which the sorting is done. To determine this, the process started with maximum range of 0|0 to MaxCol|MaxRow. It then tried to shrink it to the actual used area by own getLastRow and getLastCol methods. But both always return 0 in case of the preview. The getLastRow and getLastCol methods were only used by the sort transformation. I have removed them and use instead the generally known ShrinkToDataArea method of the document. This method detects, that no valid range exists and returns false in that case. The preview uses now the size of the database range that the user selects in the dialog. For that purpose an additional database range 'internalhelper' is introduced that has the size of the selected range, and a new member is added to the dialog, that helds a pointer to the selected range. The additional database range is introduced, because the transformation control has no access to the selected database range of the destination document. The user might have defined a huge range, thus the size is later restricted for sorting. The sort results in the preview are not correct in that case, but otherwise it would seem as if LibreOffice hangs. The chosen values can be discussed. This additional database range is used in addition to transport the flag, whether a label row exists or not. The sort parameter has a member for it, although in ODF it only exists as attribute of the database range. In this patch the flag is only evaluated for sorting. Later it can be used for the aggegrate transformation of type 'average' as well. It has the problem tracked in tdf#169545. The process covered by UITest test_brokenCSV not longer exists. Now it is earlier detected, that no database range was specified in the dialog. Thus the orcus error in the CSV import is no longer produced. I have added a comment in that test. Nevertheless the original fix is still meaningful, but I don't know how to trigger an error in the orcus CSV import in a different way to adapt the test. Change-Id: I208de42a78c1d90f08749c4271d50e5b209c7057 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195203 Tested-by: Jenkins Reviewed-by: Regina Henschel <[email protected]> diff --git a/sc/inc/globstr.hrc b/sc/inc/globstr.hrc index 0a7c84dd4f05..f077019fff07 100644 --- a/sc/inc/globstr.hrc +++ b/sc/inc/globstr.hrc @@ -588,7 +588,7 @@ #define STR_SENSITIVITY_SHADOWPRICE NC_("STR_SENSITIVITY_SHADOWPRICE", "Shadow Price") #define STR_SENSITIVITY_RHS NC_("STR_SENSITIVITY_RHS", "Constraint R.H. Side") #define STD_ERR_CSV_PARSE NC_("STD_ERR_CSV_PARSE", "An error occurred while parsing the CSV file.") - +#define STR_DATAPROVIDER_NODATABASERANGE NC_("STR_DATAPROVIDER_NODATABASERANGE", "A Database Range is missing.") #define STR_SHEET_VIEW_DEFAULT_VIEW_NAME NC_("STR_SHEET_VIEW_DEFAULT_VIEW_NAME", "Default View") #define STR_SHEET_VIEW_TEMPORARY_NAME_TEMPLATE NC_("STR_SHEET_VIEW_TEMPORARY_NAME_TEMPLATE", "Temporary View %1") diff --git a/sc/qa/uitest/calc_tests8/dataProvider.py b/sc/qa/uitest/calc_tests8/dataProvider.py index 8ee9b5e60082..4f5060a23e02 100644 --- a/sc/qa/uitest/calc_tests8/dataProvider.py +++ b/sc/qa/uitest/calc_tests8/dataProvider.py @@ -42,6 +42,9 @@ class DataProvider(UITestCase): self.assertEqual("CSV", get_state_as_dict(xProvider)['DisplayText']) # tdf#165658: without the fix in place, it would have crashed here + # The fix for error 169049 changes the process slightly. The problem of no database + # range being specified is now detected earlier and an error message is displayed to + # the user. with self.ui_test.execute_blocking_action( xApply.executeAction, args=('CLICK', ()), close_button="close") as dialog: pass @@ -186,4 +189,53 @@ class DataProvider(UITestCase): self.assertEqual("Name", get_cell_by_position(document, 1, 9, 4).getString()) self.assertEqual("Sales", get_cell_by_position(document, 1, 10, 4).getString()) + def test_consider_labels_when_sorting(self): + + with self.ui_test.load_file(get_url_for_data_file('tdf169049_WithLabel.ods')) as document: + + with self.ui_test.execute_dialog_through_command(".uno:DataProvider") as xDialog: + xDB = xDialog.getChild("select_db_range") + select_by_text(xDB, "withheader") + self.assertEqual("withheader", get_state_as_dict(xDB)['DisplayText']) + + xProvider = xDialog.getChild("provider_lst") + select_by_text(xProvider, "CSV") + self.assertEqual("CSV", get_state_as_dict(xProvider)['DisplayText']) + + xURL = xDialog.getChild("ed_url") + xBrowse = xDialog.getChild("browse") + with self.ui_test.execute_blocking_action( + xBrowse.executeAction, args=('CLICK', ()), close_button="") as dialog: + xFileName = dialog.getChild("file_name") + xFileName.executeAction( + "TYPE", mkPropertyValues({"TEXT": get_url_for_data_file("tdf169049_Source.csv")})) + xOpen = dialog.getChild("open") + xOpen.executeAction("CLICK", tuple()) + self.assertEqual(get_url_for_data_file("tdf169049_Source.csv"), + get_state_as_dict(xURL)['Text']) + + xTransformation = xDialog.getChild("transformation_box") + select_by_text(xTransformation, "Sort Columns") + self.assertEqual("Sort Columns", + get_state_as_dict(xTransformation)['DisplayText']) + + xAdd = xDialog.getChild("add_transformation") + xAdd.executeAction("CLICK", tuple()) + xAscending = xDialog.getChild("ed_ascending") + select_by_text(xAscending, "Ascending Order") + self.assertEqual("Ascending Order", get_state_as_dict(xAscending)['DisplayText']) + + xByColumn = xDialog.getChild("ed_columns") + xByColumn.executeAction("TYPE", mkPropertyValues({"TEXT":"3"})) + + # The transformation is only performed after clicking the Apply button. + xApply = xDialog.getChild("apply") + xApply.executeAction("CLICK", tuple()) + + # Without the fix in place, this test would have failed with + # AssertionError: 'Name' != 'Ali' + self.assertEqual("Name", get_cell_by_position(document, 0, 3, 1).getString()) + self.assertEqual("Ali", get_cell_by_position(document, 0, 3, 2).getString()) + self.assertEqual("Frieda", get_cell_by_position(document, 0, 3, 9).getString()) + # vim: set shiftwidth=4 softtabstop=4 expandtab: diff --git a/sc/qa/uitest/data/tdf169049_Source.csv b/sc/qa/uitest/data/tdf169049_Source.csv new file mode 100644 index 000000000000..def3281fdb5a --- /dev/null +++ b/sc/qa/uitest/data/tdf169049_Source.csv @@ -0,0 +1,9 @@ +ID,Region,Name,Sales +1,West,Anne,8 +2,East,Bob,5 +3,East,Claudia,4 +4,West,Ali,8 +5,West,Doris,3 +6,East,Frieda,6 +7,East,Edith,4 +8,East,Betty,2 diff --git a/sc/qa/uitest/data/tdf169049_WithLabel.ods b/sc/qa/uitest/data/tdf169049_WithLabel.ods new file mode 100644 index 000000000000..8133f4e54b24 Binary files /dev/null and b/sc/qa/uitest/data/tdf169049_WithLabel.ods differ diff --git a/sc/source/ui/inc/dataproviderdlg.hxx b/sc/source/ui/inc/dataproviderdlg.hxx index afb42f2e4cf9..a90d7e6bc4f0 100644 --- a/sc/source/ui/inc/dataproviderdlg.hxx +++ b/sc/source/ui/inc/dataproviderdlg.hxx @@ -23,6 +23,7 @@ class ScDocument; class ScDataTransformationBaseControl; class ScDBData; +class ScDBCollection; class ScDataProviderDlg : public weld::GenericDialogController { @@ -54,6 +55,7 @@ private: sal_uInt32 mnIndex; ScDBData* pDBData; + ScDBCollection* mpDestDBCollection; DECL_LINK(StartMenuHdl, const OString&, void); DECL_LINK(ColumnMenuHdl, const weld::ComboBox&, void); diff --git a/sc/source/ui/miscdlgs/dataproviderdlg.cxx b/sc/source/ui/miscdlgs/dataproviderdlg.cxx index 03beba8d6222..f71cf6acd676 100644 --- a/sc/source/ui/miscdlgs/dataproviderdlg.cxx +++ b/sc/source/ui/miscdlgs/dataproviderdlg.cxx @@ -40,29 +40,8 @@ public: void updateIndex(sal_uInt32 nIndex) { mnIndex = nIndex; } virtual std::shared_ptr<sc::DataTransformation> getTransformation() = 0; - - static SCROW getLastRow(const ScDocument& rDoc); - static SCCOL getLastCol(const ScDocument& rDoc); }; -SCROW ScDataTransformationBaseControl::getLastRow(const ScDocument& rDoc) -{ - SCROW nEndRow = rDoc.MaxRow(); - return rDoc.GetLastDataRow(0, 0, 0, nEndRow); -} - -SCCOL ScDataTransformationBaseControl::getLastCol(const ScDocument& rDoc) -{ - for (SCCOL nCol = 1; nCol <= rDoc.MaxCol(); ++nCol) - { - if (rDoc.GetCellType(nCol, 0, 0) == CELLTYPE_NONE) - { - return static_cast<SCCOL>(nCol - 1 ); - } - } - return rDoc.MaxCol(); -} - ScDataTransformationBaseControl::ScDataTransformationBaseControl(weld::Container* pParent, const OUString& rUIFile, sal_uInt32 nIndex) : mxBuilder(Application::CreateBuilder(pParent, rUIFile)) , mxGrid(mxBuilder->weld_container(u"grid"_ustr)) @@ -274,10 +253,44 @@ std::shared_ptr<sc::DataTransformation> ScSortTransformationControl::getTransfor aColumn = nCol - 1; // translate from 1-based column notations to internal Calc one ScSortParam aSortParam; - aSortParam.nRow1=0; - aSortParam.nRow2=getLastRow(*mpDoc); - aSortParam.nCol1=0; - aSortParam.nCol2=getLastCol(*mpDoc); + SCCOL nEndCol = mpDoc->MaxCol(); + SCROW nEndRow = mpDoc->MaxRow(); + SCCOL nStartCol = 0; + SCROW nStartRow = 0; + SCTAB nTab = 0; // for internal doc in case of Apply and clipboard in case of OK + if (!mpDoc->ShrinkToDataArea(nTab, nStartCol, nStartRow, nEndCol, nEndRow)) + { + // tdf#169515 The preview document does not show the sorted data. Reason: ShrinkToDataArea + // fails. ToDo: Why? + // Workaround: We use the size of the "internalhelper" database range instead. In case the + // user has defined a huge range, LibreOffice seems to hang. Thus the size is restricted here + // although the preview does not show the final result in that case. + ScDBCollection::NamedDBs& rLocalDBs = mpDoc->GetDBCollection()->getNamedDBs(); + ScDBData* pDB = rLocalDBs.findByName(u"internalhelper"_ustr); + if (pDB) + { + pDB->GetArea(nTab, nStartCol, nStartRow, nEndCol, nEndRow); + nEndCol = std::min<SCCOL>(nEndCol, nStartCol + 30); + nEndRow = std::min<SCROW>(nEndRow, nStartRow + 10000); + } + else + { // should not happen + nEndCol = 30; + nEndRow = 10000; + } + } + aSortParam.nCol1 = nStartCol; + aSortParam.nRow1 = nStartRow; + aSortParam.nCol2 = nEndCol; + aSortParam.nRow2 = nEndRow; + { + ScDBCollection::NamedDBs& rLocalDBs = mpDoc->GetDBCollection()->getNamedDBs(); + ScDBData* pDB = rLocalDBs.findByName(u"internalhelper"_ustr); + if (pDB) + aSortParam.bHasHeader = pDB->HasHeader(); + else + aSortParam.bHasHeader = true; + } aSortParam.maKeyState[0].bDoSort = true; aSortParam.maKeyState[0].nField = aColumn; aSortParam.maKeyState[0].bAscending = aIsAscending; @@ -786,8 +799,8 @@ ScDataProviderDlg::ScDataProviderDlg(weld::Window* pParent, std::shared_ptr<ScDo mxBox->set_size_request(aPrefSize.Width(), aPrefSize.Height()); mxTable->Show(); - ScDBCollection* pDBCollection = pDocument->GetDBCollection(); - auto& rNamedDBs = pDBCollection->getNamedDBs(); + mpDestDBCollection = pDocument->GetDBCollection(); + auto& rNamedDBs = mpDestDBCollection->getNamedDBs(); for (auto& rNamedDB : rNamedDBs) { mxDBRanges->append_text(rNamedDB->GetName()); @@ -1007,7 +1020,7 @@ void ScDataProviderDlg::swapRowsTransformation() namespace { -bool hasDBName(const OUString& rName, ScDBCollection* pDBCollection) +bool lcl_hasDBName(const OUString& rName, ScDBCollection* pDBCollection) { if (pDBCollection->getNamedDBs().findByUpperName(ScGlobal::getCharClass().uppercase(rName))) return true; @@ -1029,6 +1042,38 @@ void ScDataProviderDlg::clearTablePreview() void ScDataProviderDlg::import(ScDocument& rDoc, bool bInternal) { + OUString sDestDBUpperName = ScGlobal::getCharClass().uppercase(mxDBRanges->get_active_text()); + ScDBData* pDestDBRange = mpDestDBCollection->getNamedDBs().findByUpperName(sDestDBUpperName); + if (!pDestDBRange) + { + if (bInternal) + { + std::unique_ptr<weld::MessageDialog> xMsgBox( + Application::CreateMessageDialog(m_xDialog.get(), VclMessageType::Error, + VclButtonsType::Close, ScResId(STR_DATAPROVIDER_NODATABASERANGE))); + xMsgBox->run(); + } + return; + } + + if (bInternal) + { + // Use an additional database range in the temporary document to provide infos for transformations. + // Currently used for ersatz area in sorting and for HasHeader flag. + ScRange aDestRange; + pDestDBRange->GetArea(aDestRange); + SCCOL nHelperCol = aDestRange.aEnd.Col() - aDestRange.aStart.Col(); + SCROW nHelperRow = aDestRange.aEnd.Row() - aDestRange.aStart.Row(); + ScDBData* pDBHelper = new ScDBData(u"internalhelper"_ustr, 0, 0, 0, nHelperCol, nHelperRow); + pDBHelper->SetHeader(pDestDBRange->HasHeader()); + ScDBCollection::NamedDBs& rLocalDBs = rDoc.GetDBCollection()->getNamedDBs(); + auto iter = rLocalDBs.findByUpperName2(u"INTERNALHELPER"_ustr); + if (iter != rLocalDBs.end()) + rLocalDBs.erase(iter); + bool bSuccess = rLocalDBs.insert(std::unique_ptr<ScDBData>(pDBHelper)); + SAL_WARN_IF(!bSuccess, "sc", "Could not generate DBRange."); + } + sc::ExternalDataSource aSource = getDataSource(&rDoc); for (size_t i = 0; i < maControls.size(); ++i) @@ -1041,7 +1086,7 @@ void ScDataProviderDlg::import(ScDocument& rDoc, bool bInternal) else { aSource.setDBData(mxDBRanges->get_active_text()); - if (!hasDBName(aSource.getDBName(), rDoc.GetDBCollection())) + if (!lcl_hasDBName(aSource.getDBName(), rDoc.GetDBCollection())) return; rDoc.GetExternalDataMapper().insertDataSource(aSource); }
