Skip to content

Commit

Permalink
Fixed error when deleting external defname from the sheet while conne…
Browse files Browse the repository at this point in the history 
…ction still exist

Fixed the refersTo string parser to get the correct sheet name for defname

Added tests
  • Loading branch information
@DimitryOrlov
DimitryOrlov committed Jan 17, 2025
1 parent b88d82c commit b89185d
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 27 deletions.
65 changes: 39 additions & 26 deletions cell/model/Workbook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14417,7 +14417,7 @@
let sheetContainer = fOld.wb && fOld.wb.dependencyFormulas && fOld.wb.dependencyFormulas.sheetListeners && fOld.wb.dependencyFormulas.sheetListeners[wsId];

if (sheetContainer) {
if (Object.keys(sheetContainer.cellMap).length === 0 && Object.keys(sheetContainer.areaMap).length === 0) {
if (Object.keys(sheetContainer.cellMap).length === 0 && Object.keys(sheetContainer.areaMap).length === 0 && Object.keys(sheetContainer.defName3d).length === 0) {
hasListeners = false;
} else {
hasListeners = true;
Expand Down Expand Up @@ -23569,17 +23569,29 @@
externalDefName = eReference.DefinedNames[i];
if (externalDefName.SheetId !== null) {
externalSheetName = eReference.SheetNames[eReference.DefinedNames[i].SheetId];
} else if (!externalDefName.SheetId && externalDefName.RefersTo) {
// parse string
let refString = externalDefName.RefersTo,
// regex to find a sheet name enclosed in single quotes
reg = /'([\S]+)'/gi,
regMatch = reg.exec(refString);

if (regMatch && regMatch[1]) {
externalSheetName = regMatch[1];
}
else if (!externalDefName.SheetId && externalDefName.RefersTo) {
// RefersTo differs from the wsName and may contain several exclamation marks,
// so we use a condition and a regular expression to get the correct name

let refString = externalDefName.RefersTo;
let exclamationMarkIndex = refString.lastIndexOf("!");

refString = refString.slice(0, exclamationMarkIndex);
refString = refString[0] === "=" ? refString.substring(1) : refString;
externalSheetName = refString;

// regex to find string enclosed in single qoutes
let regex = /^'(.*)'$/;
let match = regex.exec(refString);
if (match && match[1]) {
externalSheetName = match[1];
} else if (refString) {
externalSheetName = refString.split("!")[0];
let externalWB = eReference.getWb();
let depFormulas = externalWB && externalWB.dependencyFormulas;
if (depFormulas && depFormulas.defNames && depFormulas.defNames.wb && depFormulas.defNames.wb[receivedDefName]) {
externalSheetName = refString;
}
}
}
break;
Expand All @@ -23602,7 +23614,7 @@

// we check whether sheetName is part of the document or is it a short link to external data
if (local && !externalLink && receivedShortLink) {
// если существует лист с таким же названием, ссылаемся на него, иначе создаем внешнюю ссылку(сокращенную)
// if there is a sheet with the same name, we link to it, otherwise we create an external link (shortened)
let innerSheet = t.wb.getWorksheetByName(sheetName);
if (!innerSheet) {
let eReference = t.wb.getExternalLinkByName(sheetName);
Expand All @@ -23613,20 +23625,21 @@
if (externalDefName.SheetId !== null) {
externalSheetName = eReference.SheetNames[eReference.DefinedNames[i].SheetId];
} else if (!externalDefName.SheetId && externalDefName.RefersTo) {
// parse string
let refString = externalDefName.RefersTo,
// regex to find a sheet name enclosed in single quotes
reg = /'([\S]+)'/gi,
regMatch = reg.exec(refString);

if (regMatch && regMatch[1]) {
externalSheetName = regMatch[1];
} else if (refString) {
let externalWB = eReference.getWb();
let depFormulas = externalWB && externalWB.dependencyFormulas;
if (depFormulas && depFormulas.defNames && depFormulas.defNames.wb && depFormulas.defNames.wb[receivedDefName]) {
externalSheetName = refString.split("!")[0];
}
// RefersTo differs from the wsName and may contain several exclamation marks,
// so we use a condition and a regular expression to get the correct name

let refString = externalDefName.RefersTo;
let exclamationMarkIndex = refString.lastIndexOf("!");

refString = refString.slice(0, exclamationMarkIndex);
refString = refString[0] === "=" ? refString.substring(1) : refString;
externalSheetName = refString;

// regex to find string enclosed in single qoutes
let regex = /^'(.*)'$/;
let match = regex.exec(refString);
if (match && match[1]) {
externalSheetName = match[1];
}
}
break;
Expand Down
82 changes: 81 additions & 1 deletion tests/cell/spreadsheet-calculation/ExternalReference.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ $(function () {
assert.strictEqual(wb.externalReferences.length, 0);
});

QUnit.test("Test: \"Check short links\"", function (assert) {
QUnit.test("Test: \"Check short links parse\"", function (assert) {
// create ext link
// check parser formula - simulate reading a string like [linkIndex] + "SheetName" + "!" + "ReferenceTo"
let fileName = window["Asc"]["editor"].DocInfo && window["Asc"]["editor"].DocInfo.get_Title();
Expand Down Expand Up @@ -1256,6 +1256,86 @@ $(function () {
assert.strictEqual(wb.externalReferences.length, 0);
});


QUnit.test("Test: \"Read and init external reference data\"", function (assert) {
// create external link
let cellWithFormula = new AscCommonExcel.CCellWithFormula(ws, 1, 0);
let parseResult = new AscCommonExcel.ParseResult([]);

oParser = new parserFormula("[book.xlsx]Sheet1!_s1", cellWithFormula, ws);
assert.ok(oParser.parse(true, null, parseResult), "book.xlsx!_s1");

// set extrefs to 0
wb.externalReferences.length = 0;

assert.strictEqual(wb.externalReferences.length, 0, 'External reference length before add');
wb.addExternalReferencesAfterParseFormulas(parseResult.externalReferenesNeedAdd);
assert.strictEqual(wb.externalReferences.length, 1, 'External reference length after add');

let eR = wb.externalReferences[0];
let externalWb = eR.getWb();
let externalWs;

initDefinedName(eR, "Sheet1", "A1:A2", "_s1");
initReference(eR, "Sheet1", "A1:A2", [["10"],["20"]], true);

let newDefname = eR.DefinedNames[0].clone();
newDefname.RefersTo = "='Sheet1'!$A$1:$A$2";
newDefname.SheetId = null;

eR.DefinedNames[0] = newDefname;

externalWs = createExternalWorksheet("Sheet1");
externalWs.getRange2("A1").setValue("10");
externalWs.getRange2("A2").setValue("20");

eR.updateData([externalWs]);

// defname listeners check
assert.strictEqual(Object.keys(wb.dependencyFormulas.defNameListeners).length, 0, 'Defname listeners before setValue into cell');
ws.getRange2("A1").setValue("='book.xlsx'!_s1");
assert.strictEqual(Object.keys(wb.dependencyFormulas.defNameListeners).length, 1, 'Defname listeners after setValue into cell');

// defnames in external wb check
assert.strictEqual(Object.keys(externalWb.dependencyFormulas.defNames.wb).length, 0, 'Defnames before init');
eR.initPostOpen();
assert.strictEqual(Object.keys(externalWb.dependencyFormulas.defNames.wb).length, 1, 'Defnames after init');


/* add second sheet with '!' in name */
externalWs = createExternalWorksheet(" 'Sheet!'!1");
eR.addSheet(externalWs);

// initReference(eR, " 'Sheet!'!1", "A1", [[""]], true);
initDefinedName(eR, " 'Sheet!'!1", "A1:A2", "_s2");
initReference(eR, " 'Sheet!'!1", "A1:A2", [["40"],["80"]], true);

newDefname = eR.DefinedNames[1].clone();
newDefname.RefersTo = "=' 'Sheet!'!1'!$A$1:$A$2";
newDefname.SheetId = null;

eR.DefinedNames[1] = newDefname;

// externalWs.getRange2("A1").setValue("400");
// externalWs.getRange2("A2").setValue("800");

eR.updateData([externalWs]);

assert.strictEqual(Object.keys(externalWb.dependencyFormulas.defNames.wb).length, 1, 'Defnames before second init');
eR.initPostOpen();
assert.strictEqual(Object.keys(externalWb.dependencyFormulas.defNames.wb).length, 2, 'Defnames after second init');

assert.strictEqual(Object.keys(wb.dependencyFormulas.defNameListeners).length, 1, 'Defname listeners before setValue into cell');
ws.getRange2("A2").setValue("='book.xlsx'!_s2");
assert.strictEqual(Object.keys(wb.dependencyFormulas.defNameListeners).length, 2, 'Defname listeners after setValue into cell');

//remove external reference and clear cells
ws.getRange2("A1:A2").setValue("");

wb.removeExternalReferences([eR.getAscLink()]);
assert.strictEqual(wb.externalReferences.length, 0);
});

// Mocks for API Testing
Asc.spreadsheet_api.prototype._init = function () {
this._loadModules();
Expand Down

0 comments on commit b89185d

Please sign in to comment.