Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Edit In Excel: Fix metadata generation bug when caption name uses en dash instead of em dash #2032

Merged
merged 7 commits into from
Sep 23, 2024
Merged

Edit In Excel: Fix metadata generation bug when caption name uses en dash instead of em dash #2032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
75015f3
Fix metadata generation bug
DenLilleMand Sep 13, 2024
7075ac5
Add a test case
DenLilleMand Sep 16, 2024
962dca4
Fixup test a bit
DenLilleMand Sep 16, 2024
f36255b
Merge branch 'main' into private/mattinielsen/fix-metadata-generation…
DenLilleMand Sep 18, 2024
7369ce4
...
DenLilleMand Sep 20, 2024
ea85047
Merge branch 'main' into private/mattinielsen/fix-metadata-generation…
DenLilleMand Sep 20, 2024
6506607
cleanup everything
DenLilleMand Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions src/System Application/App/Edit in Excel/src/EditinExcelImpl.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ codeunit 1482 "Edit in Excel Impl."
ExcelFileNameTxt: Text;
XmlByteEncodingTok: Label '_x00%1_%2', Locked = true;
XmlByteEncoding2Tok: Label '%1_x00%2_%3', Locked = true;
XmlByteEncoding3Tok: Label '%1_%2_%3', Locked = true;

procedure EditPageInExcel(PageCaption: Text[240]; PageId: Integer; EditinExcelFilters: Codeunit "Edit in Excel Filters"; FileName: Text)
var
Expand Down Expand Up @@ -100,7 +101,7 @@ codeunit 1482 "Edit in Excel Impl."
VarFieldRef := VarKeyRef.FieldIndex(KeyFieldNumber);

if not AddedFields.Contains(VarFieldRef.Number) then begin // Make sure we don't add the same field twice
// Add missing key fields at the beginning
// Add missing key fields at the beginning
EditinExcelWorkbook.InsertColumn(0, VarFieldRef.Caption, ExternalizeODataObjectName(VarFieldRef.Name));
AddedFields.Add(VarFieldRef.Number);
end;
Expand Down Expand Up @@ -204,6 +205,7 @@ codeunit 1482 "Edit in Excel Impl."
StartStr: Text;
EndStr: Text;
ByteValue: DotNet Byte;
IsByteValueUnderscore: Dictionary of [Integer, Boolean];
begin
ConvertedName := Name;

Expand All @@ -224,17 +226,33 @@ codeunit 1482 "Edit in Excel Impl."
CurrentPosition := 1;

while CurrentPosition <= StrLen(ConvertedName) do begin
if ConvertedName[CurrentPosition] in ['''', '+'] then begin
ByteValue := Convert.ToByte(ConvertedName[CurrentPosition]);
StartStr := CopyStr(ConvertedName, 1, CurrentPosition - 1);
EndStr := CopyStr(ConvertedName, CurrentPosition + 1);
ConvertedName := StrSubstNo(XmlByteEncoding2Tok, StartStr, Convert.ToString(ByteValue, 16), EndStr);
// Notice in the following line that – (en dash) is not a normal dash (em dash).
// We need to handle this here because at least the norwegian translation uses en dash.
if ConvertedName[CurrentPosition] in ['''', '+', '–'] then begin
DenLilleMand marked this conversation as resolved.
Show resolved Hide resolved
if ConvertedName[CurrentPosition] in ['–'] then begin
StartStr := CopyStr(ConvertedName, 1, CurrentPosition - 1);
EndStr := CopyStr(ConvertedName, CurrentPosition + 1);
ConvertedName := StrSubstNo(XmlByteEncoding3Tok, StartStr, 'x2013', EndStr);
// length of _x00nn_ minus one that will be added later
end else begin
ByteValue := Convert.ToByte(ConvertedName[CurrentPosition]);
StartStr := CopyStr(ConvertedName, 1, CurrentPosition - 1);
EndStr := CopyStr(ConvertedName, CurrentPosition + 1);
ConvertedName := StrSubstNo(XmlByteEncoding2Tok, StartStr, Convert.ToString(ByteValue, 16), EndStr);
end;
// length of _x00nn_ minus one that will be added later
CurrentPosition += 6;

IsByteValueUnderscore.Add(CurrentPosition, true);
end else
if ConvertedName[CurrentPosition] in [' ', '\', '/', '"', '.', '(', ')', '-', ':'] then
if CurrentPosition > 1 then begin
if ConvertedName[CurrentPosition - 1] = '_' then begin
// The only cases where we allow 2 underscores in succession is when
// we have substituted a symbol with its byte value and when we have an actual underscore
// prefixed with a symbol that should be replaced with underscore.
// This code below removes duplicate underscores but
// needs to not remove underscores that was added via a byte value.
if (ConvertedName[CurrentPosition - 1] = '_') and not IsByteValueUnderscore.ContainsKey(CurrentPosition - 1) then begin
ConvertedName := DelStr(ConvertedName, CurrentPosition, 1);
CurrentPosition -= 1;
end else
Expand Down
39 changes: 39 additions & 0 deletions src/System Application/Test/Edit in Excel/src/EditInExcelTest.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,55 @@ codeunit 132525 "Edit in Excel Test"
PlusFieldName: Text;
RegularFieldName: Text;
FieldNameStartingWDigit: Text;
EnDashFieldName: Text;
EnDashFieldName2: Text;
EnDashFieldName3: Text;
EnDashFieldName4: Text;
ForwardSlashesFieldName: Text;
ManyForwardSlashesFieldName: Text;
ForwardSlashesEmDashesAndUnderscoresFieldName: Text;
begin
Init();
FieldNameStartingWDigit := EditinExcelTestLibrary.ExternalizeODataObjectName('3field');
RegularFieldName := EditinExcelTestLibrary.ExternalizeODataObjectName('field');
ApostropheFieldName := EditinExcelTestLibrary.ExternalizeODataObjectName('new vendor''s name');
PlusFieldName := EditinExcelTestLibrary.ExternalizeODataObjectName('c+c field');

// Both spaces will be converted to underscore and the `en dash` will be converted to _x2013_
EnDashFieldName := EditinExcelTestLibrary.ExternalizeODataObjectName('lager – reklassfication field');

// The special symbol `en dash` will be converted to _x2013_ and the first prefixed space will be a converted
// to an underscore.
EnDashFieldName2 := EditinExcelTestLibrary.ExternalizeODataObjectName('lager –reklassfication field');

// The special symbol `en dash` will be converted to _x2013_.
EnDashFieldName3 := EditinExcelTestLibrary.ExternalizeODataObjectName('lager–reklassfication field');

// The two forward slashes will be converted to underscores and the `en dash` will be converted to _x2013_.
EnDashFieldName4 := EditinExcelTestLibrary.ExternalizeODataObjectName('lager/–/reklassfication field');

// Two forward slashes will have the first replaced with an underscore and the second removed.
ForwardSlashesFieldName := EditinExcelTestLibrary.ExternalizeODataObjectName('lager//reklassfication field');

// When we have a lot of forward slashes it only replaces the first one with an underscore and the rest are truncated.
ManyForwardSlashesFieldName := EditinExcelTestLibrary.ExternalizeODataObjectName('lager////////reklassfication field');

// The first forward slash will be converted to an underscore, the next underscore does not hit any special case so just
// stays as an underscore, the next forward slash is removed because it follows the rule of not allowing two underscores
// when converting from a special symbol to underscore(unless that special character is translated to a byte value).
ForwardSlashesEmDashesAndUnderscoresFieldName := EditinExcelTestLibrary.ExternalizeODataObjectName('lager/_/-reklassfication field');

LibraryAssert.AreEqual('field', RegularFieldName, 'Conversion alters name that does not begin with a string');
LibraryAssert.AreEqual('_x0033_field', FieldNameStartingWDigit, 'Did not convert the name with number correctly');
LibraryAssert.AreEqual('new_vendor_x0027_s_name', ApostropheFieldName, 'Did not convert the name with an apostrophe correctly');
LibraryAssert.AreEqual('c_x002b_c_field', PlusFieldName, 'Did not convert the name with a plus correctly');
LibraryAssert.AreEqual('lager__x2013__reklassfication_field', EnDashFieldName, 'Did not convert the name with an `en dash` with two surrounding spaces correctly');
LibraryAssert.AreEqual('lager__x2013_reklassfication_field', EnDashFieldName2, 'Did not convert the name with a space before an `en dash` correctly');
LibraryAssert.AreEqual('lager_x2013_reklassfication_field', EnDashFieldName3, 'Did not convert the name with an `en dash` correctly');
LibraryAssert.AreEqual('lager__x2013__reklassfication_field', EnDashFieldName4, 'Did not convert the name with forward slashes around `en dash` correctly');
LibraryAssert.AreEqual('lager_reklassfication_field', ForwardSlashesFieldName, 'Did not convert the name with 2 forward slashes correctly');
LibraryAssert.AreEqual('lager_reklassfication_field', ManyForwardSlashesFieldName, 'Did not convert the name with many forward slashes correctly');
LibraryAssert.AreEqual('lager__reklassfication_field', ForwardSlashesEmDashesAndUnderscoresFieldName, 'Did not convert the name with forward slashes, em dash and underscores correctly');
end;

[Test]
Expand Down
Loading