From b02eb845a06c6095993fcffb36f17ef4f7802d48 Mon Sep 17 00:00:00 2001 From: Ylber Date: Fri, 24 Apr 2026 16:57:17 +0200 Subject: [PATCH 1/2] Fix DESCRIBE PAGE / ALTER PAGE column name mismatch (#116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deriveColumnNameBson skipped the intermediate TextTemplate.Template document when reading DataGrid2 column headers, so caption-only columns always fell back to col{N} instead of the name DESCRIBE PAGE shows. sanitizeColumnName also diverged from deriveColumnName on edge cases (leading/trailing underscores, all-special-char captions). Fix: traverse TextTemplate → Template → Items (with a nil fallback for older BSON), rewrite sanitizeColumnName to match deriveColumnName exactly (returns "" for all-special-char input so caller uses col{N}), and add 5 regression tests covering attribute binding, caption fallback, all-special-char caption, index fallback, and sanitization trimming. --- mdl/executor/alter_page_test.go | 135 ++++++++++++++++++++++++++++++++ mdl/executor/cmd_alter_page.go | 33 +++++--- 2 files changed, 156 insertions(+), 12 deletions(-) diff --git a/mdl/executor/alter_page_test.go b/mdl/executor/alter_page_test.go index 0bf81196..890068bb 100644 --- a/mdl/executor/alter_page_test.go +++ b/mdl/executor/alter_page_test.go @@ -700,3 +700,138 @@ func TestExtractWidgetScopeFromBSON_Nil(t *testing.T) { t.Error("Expected empty scope for nil input") } } + +// ============================================================================ +// deriveColumnNameBson regression tests (issue #116) +// ============================================================================ + +// makePropTypeID builds a primitive.Binary suitable for use as a TypePointer. +func makePropTypeID(b byte) primitive.Binary { + data := make([]byte, 16) + data[0] = b + return primitive.Binary{Subtype: 0x04, Data: data} +} + +// propKey builds the map key used by deriveColumnNameBson — the UUID string +// that extractBinaryIDFromDoc produces from a primitive.Binary TypePointer. +func propKey(id primitive.Binary) string { + return extractBinaryIDFromDoc(id) +} + +// TestDeriveColumnNameBson_AttributeBinding verifies attribute-bound columns +// produce the short attribute name (last segment after dot). +func TestDeriveColumnNameBson_AttributeBinding(t *testing.T) { + typeID := makePropTypeID(0x01) + propKeyMap := map[string]string{propKey(typeID): "attribute"} + + colDoc := bson.D{ + {Key: "Properties", Value: bson.A{ + int32(2), + bson.D{ + {Key: "TypePointer", Value: typeID}, + {Key: "Value", Value: bson.D{ + {Key: "AttributeRef", Value: "MyModule.Customer.Description"}, + }}, + }, + }}, + } + + got := deriveColumnNameBson(colDoc, propKeyMap, 0) + if got != "Description" { + t.Errorf("expected 'Description', got %q", got) + } +} + +// TestDeriveColumnNameBson_CaptionFallback verifies caption-only columns +// produce the sanitized caption. This exercises the TextTemplate → Template → +// Items[] path that was broken in issue #116. +func TestDeriveColumnNameBson_CaptionFallback(t *testing.T) { + typeID := makePropTypeID(0x02) + propKeyMap := map[string]string{propKey(typeID): "header"} + + colDoc := bson.D{ + {Key: "Properties", Value: bson.A{ + int32(2), + bson.D{ + {Key: "TypePointer", Value: typeID}, + {Key: "Value", Value: bson.D{ + {Key: "TextTemplate", Value: bson.D{ + {Key: "Template", Value: bson.D{ + {Key: "Items", Value: bson.A{ + int32(2), + bson.D{{Key: "Text", Value: "Order Status"}}, + }}, + }}, + }}, + }}, + }, + }}, + } + + got := deriveColumnNameBson(colDoc, propKeyMap, 0) + if got != "Order_Status" { + t.Errorf("expected 'Order_Status', got %q", got) + } +} + +// TestDeriveColumnNameBson_IndexFallback verifies that a column with neither +// attribute nor caption falls back to "col{N}" (1-based). +func TestDeriveColumnNameBson_IndexFallback(t *testing.T) { + colDoc := bson.D{{Key: "Properties", Value: bson.A{int32(2)}}} + got := deriveColumnNameBson(colDoc, map[string]string{}, 2) + if got != "col3" { + t.Errorf("expected 'col3', got %q", got) + } +} + +// TestSanitizeColumnName_TrimUnderscores verifies that leading/trailing +// underscores are trimmed to match deriveColumnName() on the DESCRIBE side. +func TestSanitizeColumnName_TrimUnderscores(t *testing.T) { + cases := []struct { + input string + want string + }{ + {" Description ", "Description"}, + {"!Order Status!", "Order_Status"}, + {"Hello World", "Hello_World"}, + {"___", ""}, // all special chars → empty → caller falls through to col{N} + } + for _, c := range cases { + got := sanitizeColumnName(c.input) + if got != c.want { + t.Errorf("sanitizeColumnName(%q) = %q, want %q", c.input, got, c.want) + } + } +} + +// TestDeriveColumnNameBson_AllSpecialCharCaption verifies that a caption +// composed entirely of non-identifier characters falls back to col{N}, +// matching deriveColumnName() on the DESCRIBE side. +func TestDeriveColumnNameBson_AllSpecialCharCaption(t *testing.T) { + typeID := makePropTypeID(0x03) + propKeyMap := map[string]string{propKey(typeID): "header"} + + colDoc := bson.D{ + {Key: "Properties", Value: bson.A{ + int32(2), + bson.D{ + {Key: "TypePointer", Value: typeID}, + {Key: "Value", Value: bson.D{ + {Key: "TextTemplate", Value: bson.D{ + {Key: "Template", Value: bson.D{ + {Key: "Items", Value: bson.A{ + int32(2), + bson.D{{Key: "Text", Value: "---"}}, + }}, + }}, + }}, + }}, + }, + }}, + } + + got := deriveColumnNameBson(colDoc, propKeyMap, 0) + if got != "col1" { + t.Errorf("expected 'col1' for all-special-char caption, got %q", got) + } +} diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index 9a247af3..f0ed2ff1 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -700,9 +700,15 @@ func deriveColumnNameBson(colDoc bson.D, propKeyMap map[string]string, index int attribute = dGetString(attrDoc, "Attribute") } case "header": - // Extract caption from TextTemplate + // TextTemplate → Template (Forms$Text) → Items[] → Translation{Text}. + // Must traverse the intermediate Template document — same path as + // extractDataGrid2Column on the DESCRIBE side. if tmpl := dGetDoc(valDoc, "TextTemplate"); tmpl != nil { - items := dGetArrayElements(dGet(tmpl, "Items")) + template := dGetDoc(tmpl, "Template") + if template == nil { + template = tmpl // fallback: some versions store Items directly + } + items := dGetArrayElements(dGet(template, "Items")) for _, item := range items { if itemDoc, ok := item.(bson.D); ok { if text := dGetString(itemDoc, "Text"); text != "" { @@ -720,22 +726,25 @@ func deriveColumnNameBson(colDoc bson.D, propKeyMap map[string]string, index int return parts[len(parts)-1] } if caption != "" { - return sanitizeColumnName(caption) + if name := sanitizeColumnName(caption); name != "" { + return name + } } return fmt.Sprintf("col%d", index+1) } -// sanitizeColumnName converts a caption string into a valid column identifier. +// sanitizeColumnName converts a caption string into a valid column identifier, +// matching deriveColumnName() in cmd_pages_describe_output.go exactly. +// Returns "" when the result would be all underscores so the caller can fall +// through to the col{N} index fallback. func sanitizeColumnName(caption string) string { - var result []rune - for _, r := range caption { - if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' { - result = append(result, r) - } else { - result = append(result, '_') + sanitized := strings.Map(func(r rune) rune { + if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || r == '_' { + return r } - } - return string(result) + return '_' + }, caption) + return strings.TrimFunc(sanitized, func(r rune) bool { return r == '_' }) } // columnPropertyAliases maps user-facing property names to internal column property keys. From 5c838ac3ec7a94b00c2270adb73059f56aa8c0d7 Mon Sep 17 00:00:00 2001 From: Ylber Date: Fri, 1 May 2026 09:50:20 +0200 Subject: [PATCH 2/2] Address PR #298 review: drop template fallback, add bug-test script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the `template = tmpl` fallback in deriveColumnNameBson — if Template is nil, DESCRIBE returns "" and falls to col{N}, so ALTER must do the same. The fallback had no test coverage and would have reintroduced a divergence. Add mdl-examples/bug-tests/116-datagrid2-column-name-mismatch.mdl per the PR checklist: covers attribute-bound, caption-only, and all-special-char column name roundtrip. --- .../116-datagrid2-column-name-mismatch.mdl | 72 +++++++++++++++++++ mdl/executor/cmd_alter_page.go | 19 +++-- 2 files changed, 81 insertions(+), 10 deletions(-) create mode 100644 mdl-examples/bug-tests/116-datagrid2-column-name-mismatch.mdl diff --git a/mdl-examples/bug-tests/116-datagrid2-column-name-mismatch.mdl b/mdl-examples/bug-tests/116-datagrid2-column-name-mismatch.mdl new file mode 100644 index 00000000..ed0e6539 --- /dev/null +++ b/mdl-examples/bug-tests/116-datagrid2-column-name-mismatch.mdl @@ -0,0 +1,72 @@ +-- ============================================================================ +-- Bug test: issue #116 — DESCRIBE PAGE / ALTER PAGE DataGrid2 column name mismatch +-- ============================================================================ +-- +-- Verifies that ALTER PAGE can address DataGrid2 columns using the names that +-- DESCRIBE PAGE produces — for both attribute-bound columns and caption-only +-- columns (the previously broken case). +-- +-- Affected functions (fixed in PR #298): +-- deriveColumnNameBson — now traverses TextTemplate → Template → Items +-- sanitizeColumnName — now matches deriveColumnName exactly +-- +-- To verify in Studio Pro: +-- 1. Run this script against a project. +-- 2. DESCRIBE PAGE Bug116.OrderList — column names in output should match +-- what ALTER PAGE uses below (Name, Order_Status, col3). +-- 3. Confirm no errors. +-- ============================================================================ + +-- ############################################################################ +-- PREREQUISITES +-- ############################################################################ + +CREATE MODULE Bug116; + +CREATE PERSISTENT ENTITY Bug116.Order ( + Name: String(200), + Status: String(50) +); + +-- ############################################################################ +-- PAGE WITH THREE COLUMN TYPES +-- +-- colAttr — attribute-bound: DESCRIBE yields "Name" +-- colCaption — caption-only (the previously broken case): DESCRIBE yields "Order_Status" +-- colNoName — no attribute, special-char caption only: DESCRIBE yields "col3" +-- ############################################################################ + +CREATE PAGE Bug116.OrderList ( + Title: 'Order List', + Layout: Atlas_Core.Atlas_Default +) { + DATAGRID dgOrders (DataSource: DATABASE Bug116.Order) { + COLUMN colAttr (Attribute: Name, Caption: 'Name') + COLUMN colCaption ( Caption: 'Order Status') + COLUMN colNoName ( Caption: '---') + } +}; + +-- ############################################################################ +-- ALTER PAGE USING DESCRIBE-DERIVED COLUMN NAMES +-- +-- H1 fix: colCaption uses TextTemplate → Template → Items path; +-- previously always fell back to col2 because Template was skipped. +-- H2 fix: sanitizeColumnName now trims underscores and falls through to col{N} +-- for all-special-char captions like '---'. +-- ############################################################################ + +-- Attribute-bound column — always worked; regression guard +ALTER PAGE Bug116.OrderList { + SET WIDGET dgOrders.Name (Visible: true) +}; + +-- Caption-only column — previously broken (derived col2 instead of Order_Status) +ALTER PAGE Bug116.OrderList { + SET WIDGET dgOrders.Order_Status (Visible: true) +}; + +-- All-special-char caption → falls back to col3 on both sides +ALTER PAGE Bug116.OrderList { + SET WIDGET dgOrders.col3 (Visible: true) +}; diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index f0ed2ff1..acc6f0b5 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -702,17 +702,16 @@ func deriveColumnNameBson(colDoc bson.D, propKeyMap map[string]string, index int case "header": // TextTemplate → Template (Forms$Text) → Items[] → Translation{Text}. // Must traverse the intermediate Template document — same path as - // extractDataGrid2Column on the DESCRIBE side. + // deriveColumnName on the DESCRIBE side. No Template → no caption, + // matching DESCRIBE's extractTextContent behaviour exactly. if tmpl := dGetDoc(valDoc, "TextTemplate"); tmpl != nil { - template := dGetDoc(tmpl, "Template") - if template == nil { - template = tmpl // fallback: some versions store Items directly - } - items := dGetArrayElements(dGet(template, "Items")) - for _, item := range items { - if itemDoc, ok := item.(bson.D); ok { - if text := dGetString(itemDoc, "Text"); text != "" { - caption = text + if template := dGetDoc(tmpl, "Template"); template != nil { + items := dGetArrayElements(dGet(template, "Items")) + for _, item := range items { + if itemDoc, ok := item.(bson.D); ok { + if text := dGetString(itemDoc, "Text"); text != "" { + caption = text + } } } }