From 5b7cbb8794e3ea9d074cfe8034bf4283a903cd9b Mon Sep 17 00:00:00 2001 From: scrummer Date: Tue, 8 Sep 2026 13:21:45 +0200 Subject: [PATCH] Bugfix: Handle non-numeric `oid` query parameter in `GetClassDefinitionForColumnConfigPayload` --- CHANGELOG.md | 3 + public/js/opendxp/object/helpers/classTree.js | 55 ++++++------ .../object/helpers/gridConfigDialog.js | 90 +++++++++---------- ...tClassDefinitionForColumnConfigPayload.php | 2 +- ...ssDefinitionForColumnConfigPayloadTest.php | 71 +++++++++++++++ 5 files changed, 147 insertions(+), 74 deletions(-) create mode 100644 tests/Unit/Handler/DataObject/ClassDef/GetClassDefinitionForColumnConfig/GetClassDefinitionForColumnConfigPayloadTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e177622..5b3ad825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog / Upgrade Notes +## 1.4.1 +* Bugfix: Tolerate empty / non-integer `oid` query parameter in `getClassDefinitionForColumnConfig` (regression from #91) [#115](https://github.com/open-dxp/admin-bundle/pull/115) + ## 1.4.0 * **New Feature**: Replace controller logic with dedicated handler and service classes (CQRS-Lite) [#91](https://github.com/open-dxp/admin-bundle/pull/91) * New Feature: Add "use original recipients" option, fix resend and forward recipients [#102](https://github.com/open-dxp/admin-bundle/pull/102) diff --git a/public/js/opendxp/object/helpers/classTree.js b/public/js/opendxp/object/helpers/classTree.js index cc45b275..6f19ebf9 100644 --- a/public/js/opendxp/object/helpers/classTree.js +++ b/public/js/opendxp/object/helpers/classTree.js @@ -11,7 +11,7 @@ * @license https://www.gnu.org/licenses/gpl-3.0.html GNU General Public License version 3 (GPLv3) */ -opendxp.registerNS("opendxp.object.helpers.classTree"); +opendxp.registerNS('opendxp.object.helpers.classTree'); /** * @private */ @@ -72,7 +72,7 @@ opendxp.object.helpers.classTree = Class.create({ ); var filterButton = new Ext.button.Button({ - iconCls: "opendxp_icon_search" + iconCls: 'opendxp_icon_search' }); var headerConfig = { @@ -87,16 +87,16 @@ opendxp.object.helpers.classTree = Class.create({ title: t('class_attributes'), iconCls: 'opendxp_icon_gridconfig_class_attributes', tbar: headerConfig, - region: "center", + region: 'center', autoScroll: true, rootVisible: false, bufferedRenderer: false, animate: false, width: 300, root: { - id: "0", + id: '0', root: true, - text: t("base"), + text: t('base'), allowDrag: false, leaf: true, isTarget: true @@ -106,7 +106,7 @@ opendxp.object.helpers.classTree = Class.create({ ptype: 'treeviewdragdrop', enableDrag: true, enableDrop: false, - ddGroup: "columnconfigelement" + ddGroup: 'columnconfigelement' } } }); @@ -121,10 +121,10 @@ opendxp.object.helpers.classTree = Class.create({ }); filterField.on( - "keyup", + 'keyup', Ext.Function.createBuffered(this.updateFilter.bind(this, tree, filterField), 300) ); - filterButton.on("click", this.updateFilter.bind(this, tree, filterField)); + filterButton.on('click', this.updateFilter.bind(this, tree, filterField)); return tree; }, @@ -141,19 +141,19 @@ opendxp.object.helpers.classTree = Class.create({ var brickDescriptor = {}; - if (data[keys[i]].nodeType == "objectbricks") { + if (data[keys[i]].nodeType == 'objectbricks') { brickDescriptor = { insideBrick: true, brickType: data[keys[i]].nodeLabel, brickField: data[keys[i]].brickField }; - text = t(data[keys[i]].nodeLabel) + " " + t("columns"); + text = t(data[keys[i]].nodeLabel) + ' ' + t('columns'); } var baseNode = { - type: "layout", + type: 'layout', allowDrag: false, - iconCls: "opendxp_icon_" + data[keys[i]].nodeType, + iconCls: 'opendxp_icon_' + data[keys[i]].nodeType, text: text, originalText: text }; @@ -162,7 +162,7 @@ opendxp.object.helpers.classTree = Class.create({ for (var j = 0; j < data[keys[i]].children.length; j++) { baseNode.appendChild(this.recursiveAddNode(data[keys[i]].children[j], baseNode, brickDescriptor, this.config)); } - if (data[keys[i]].nodeType == "object") { + if (data[keys[i]].nodeType == 'object') { baseNode.expand(true); } else { // baseNode.collapse(); @@ -177,7 +177,7 @@ opendxp.object.helpers.classTree = Class.create({ var fn = null; var newNode = null; - if (con.fieldtype == "localizedfields") { + if (con.fieldtype == 'localizedfields') { // create a copy because we have to pop this state brickDescriptor = Ext.clone(brickDescriptor); Ext.apply(brickDescriptor, { @@ -185,10 +185,9 @@ opendxp.object.helpers.classTree = Class.create({ }); } - if (con.datatype == "layout") { + if (con.datatype == 'layout') { fn = this.addLayoutChild.bind(scope, con.fieldtype, con); - } - else if (con.datatype == "data") { + } else if (con.datatype == 'data') { fn = this.addDataChild.bind(scope, con.fieldtype, con, this.showFieldName, brickDescriptor, config); } @@ -216,11 +215,11 @@ opendxp.object.helpers.classTree = Class.create({ } var newNode = { - type: "layout", + type: 'layout', expanded: true, expandable: initData.children.length, allowDrag: false, - iconCls: "opendxp_icon_" + type, + iconCls: 'opendxp_icon_' + type, text: t(nodeLabel), originalText: nodeLabel }; @@ -231,12 +230,12 @@ opendxp.object.helpers.classTree = Class.create({ }, addDataChild: function (type, initData, showFieldname, brickDescriptor, config) { - if (type != "objectbricks" && (!initData.invisible || config.showInvisible)) { + if (type != 'objectbricks' && (!initData.invisible || config.showInvisible)) { var isLeaf = true; var draggable = true; // localizedfields can be a drop target - if (type == "localizedfields") { + if (type == 'localizedfields') { isLeaf = false; draggable = false; } @@ -249,31 +248,31 @@ opendxp.object.helpers.classTree = Class.create({ containerKey: brickDescriptor.brickType, fieldname: brickDescriptor.brickField, brickfield: key - } - key = "?" + Ext.encode(parts) + "~" + key; + }; + key = '?' + Ext.encode(parts) + '~' + key; } else { - key = brickDescriptor.brickType + "~" + key; + key = brickDescriptor.brickType + '~' + key; } } var text = t(initData.title); if (showFieldname) { if (brickDescriptor && brickDescriptor.insideBrick && brickDescriptor.insideLocalizedFields) { - text = text + "(" + brickDescriptor.brickType + "." + initData.name + ")"; + text = text + '(' + brickDescriptor.brickType + '.' + initData.name + ')'; } else { - text = text + " (" + key.replace("~", ".") + ")"; + text = text + ' (' + key.replace('~', '.') + ')'; } } var newNode = { text: text, key: key, name: initData.name, - type: "data", + type: 'data', layout: initData, leaf: isLeaf, allowDrag: draggable, dataType: type, - iconCls: "opendxp_icon_" + type, + iconCls: 'opendxp_icon_' + type, expanded: true, brickDescriptor: brickDescriptor, originalText: text diff --git a/public/js/opendxp/object/helpers/gridConfigDialog.js b/public/js/opendxp/object/helpers/gridConfigDialog.js index fd2ef945..0991cf79 100644 --- a/public/js/opendxp/object/helpers/gridConfigDialog.js +++ b/public/js/opendxp/object/helpers/gridConfigDialog.js @@ -11,7 +11,7 @@ * @license https://www.gnu.org/licenses/gpl-3.0.html GNU General Public License version 3 (GPLv3) */ -opendxp.registerNS("opendxp.object.helpers.gridConfigDialog"); +opendxp.registerNS('opendxp.object.helpers.gridConfigDialog'); /** * @private */ @@ -28,8 +28,8 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g this.brickKeys = []; this.leftPanel = new Ext.Panel({ - cls: "opendxp_panel_tree opendxp_gridconfig_leftpanel", - region: "center", + cls: 'opendxp_panel_tree opendxp_gridconfig_leftpanel', + region: 'center', split: true, width: 300, minSize: 175, @@ -43,7 +43,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g layoutConfig: { animate: false }, - hideMode: "offsets", + hideMode: 'offsets', items: items }); } @@ -95,13 +95,13 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g }.bind(this)); } - var user = opendxp.globalmanager.get("user"); + var user = opendxp.globalmanager.get('user'); if (this.showSaveAndShareTab) { this.settings = Ext.apply(this.settings, this.settingsForm.getForm().getFieldValues()); } - if (this.showSaveAndShareTab && user.isAllowed("share_configurations")) { + if (this.showSaveAndShareTab && user.isAllowed('share_configurations')) { if (this.settings.sharedUserIds != null) { this.settings.sharedUserIds = this.settings.sharedUserIds.join(); @@ -169,20 +169,20 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g url: Routing.generate('opendxp_admin_dataobject_dataobject_gridproxy', {classId: this.previewSettings.classId, folderId: this.previewSettings.objectId}), method: 'POST', params: { - "fields[]": keys, + 'fields[]': keys, language: language, limit: 1, csvMode: csvMode, specificId: this.previewSettings.specificId, - context : Ext.encode(this.context) + context: Ext.encode(this.context) }, success: function (response) { let responseData = Ext.decode(response.responseText); if (responseData && responseData.data && responseData.data.length == 1) { - let rootNode = this.selectionPanel.getRootNode() + let rootNode = this.selectionPanel.getRootNode(); let childNodes = rootNode.childNodes; let previewItem = responseData.data[0]; - let store = this.selectionPanel.getStore() + let store = this.selectionPanel.getStore(); let i; let count = childNodes.length; @@ -195,7 +195,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g let value = previewItem[columnKey]; let record = store.getById(nodeId); - record.set("preview", value, { + record.set('preview', value, { commit: true }); } @@ -222,19 +222,19 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g } else { let text = t(nodeConf.label); - const keyText = ` (${nodeConf.key.replace("~", ".")})`; - if (nodeConf.dataType !== "system" && this.showFieldname && nodeConf.key && !text.includes(keyText)) { + const keyText = ` (${nodeConf.key.replace('~', '.')})`; + if (nodeConf.dataType !== 'system' && this.showFieldname && nodeConf.key && !text.includes(keyText)) { text = text + keyText; } var child = { text: text, key: nodeConf.key, - type: "data", + type: 'data', dataType: nodeConf.dataType, leaf: true, layout: nodeConf.layout, - iconCls: "opendxp_icon_" + nodeConf.dataType + iconCls: 'opendxp_icon_' + nodeConf.dataType }; if (nodeConf.width) { child.width = nodeConf.width; @@ -254,17 +254,17 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var store = new Ext.data.TreeStore({ fields: [{ - name: "text" + name: 'text' }, { - name: "preview", + name: 'preview', persist: false } ], root: { - id: "0", + id: '0', root: true, - text: t("selected_grid_columns"), + text: t('selected_grid_columns'), leaf: false, isTarget: true, expanded: true, @@ -296,12 +296,12 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var key = record.data.key; record.data.inheritedFields = {}; - if (key == "modificationDate" || key == "creationDate") { + if (key == 'modificationDate' || key == 'creationDate') { var timestamp = intval(value) * 1000; var date = new Date(timestamp); return Ext.Date.format(date, opendxp.globalmanager.get('localeDateTime').getShortDateTimeFormat()); - } else if (key == "published") { + } else if (key == 'published') { return Ext.String.format('
', value ? '-checked' : ''); } else { var layout = Ext.clone(record.data.layout) || {}; @@ -330,7 +330,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g console.log(e); } - if (typeof value == "string") { + if (typeof value == 'string') { value = '
' + value + '
'; } return value; @@ -348,7 +348,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g viewConfig: { plugins: { ptype: 'treeviewdragdrop', - ddGroup: "columnconfigelement" + ddGroup: 'columnconfigelement' }, listeners: { beforedrop: function (node, data, overModel, dropPosition, dropHandlers, eOpts) { @@ -359,7 +359,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var record = data.records[0]; var isOperator = record.data.isOperator; var realOverModel = overModel; - if (dropPosition == "before" || dropPosition == "after") { + if (dropPosition == 'before' || dropPosition == 'after') { realOverModel = overModel.parentNode; } @@ -367,7 +367,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g let copy = this.handleOperator(record); data.records = [copy]; // assign the copy as the new dropNode } else { - if (this.selectionPanel.getRootNode().findChild("key", record.data.key)) { + if (this.selectionPanel.getRootNode().findChild('key', record.data.key)) { dropHandlers.cancelDrop(); } else { var copy = Ext.apply({}, record.data); @@ -376,7 +376,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var ownerTree = this.selectionPanel; - if (record.data.dataType == "classificationstore") { + if (record.data.dataType == 'classificationstore') { setTimeout(function () { var ccd = new opendxp.object.classificationstore.columnConfigDialog(); ccd.getConfigDialog(ownerTree, copy, this.selectionPanel); @@ -390,7 +390,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var record = data.records[0]; var isOperator = record.data.isOperator; var realOverModel = overModel; - if (dropPosition == "before" || dropPosition == "after") { + if (dropPosition == 'before' || dropPosition == 'after') { realOverModel = overModel.parentNode; } @@ -430,26 +430,26 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g if (sourceNode.data.isOperator) { var realOverModel = targetNode; - if (dropPosition == "before" || dropPosition == "after") { + if (dropPosition == 'before' || dropPosition == 'after') { realOverModel = realOverModel.parentNode; } var allowed = true; - if (typeof realOverModel.data.isChildAllowed == "function") { - console.log("no child allowed"); + if (typeof realOverModel.data.isChildAllowed == 'function') { + console.log('no child allowed'); allowed = allowed && realOverModel.data.isChildAllowed(realOverModel, sourceNode); } - if(realOverModel.data.maxChildCount) { + if (realOverModel.data.maxChildCount) { if (realOverModel.childNodes.length >= realOverModel.data.maxChildCount) { allowed = false; } } - if (typeof sourceNode.data.isParentAllowed == "function") { - console.log("parent not allowed"); + if (typeof sourceNode.data.isParentAllowed == 'function') { + console.log('parent not allowed'); allowed = allowed && sourceNode.data.isParentAllowed(realOverModel, sourceNode); } @@ -460,21 +460,21 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var allowed = true; if (this.parentIsOperator(targetNode)) { - if (dropPosition == "before" || dropPosition == "after") { + if (dropPosition == 'before' || dropPosition == 'after') { targetNode = targetNode.parentNode; } - if (typeof targetNode.data.isChildAllowed == "function") { + if (typeof targetNode.data.isChildAllowed == 'function') { allowed = allowed && targetNode.data.isChildAllowed(targetNode, sourceNode); } - if(targetNode.data.maxChildCount) { + if (targetNode.data.maxChildCount) { if (targetNode.childNodes.length >= targetNode.data.maxChildCount) { allowed = false; } } - if (typeof sourceNode.data.isParentAllowed == "function") { + if (typeof sourceNode.data.isParentAllowed == 'function') { allowed = allowed && sourceNode.data.isParentAllowed(targetNode, sourceNode); } @@ -511,7 +511,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g return this.selectionPanel; }, - handleOperator: function(record) { + handleOperator: function (record) { var attr = record.data; if (record.data.configAttributes) { attr = record.data.configAttributes; @@ -547,18 +547,18 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var classTreeHelper = new opendxp.object.helpers.classTree(this.showFieldname); var tree = classTreeHelper.getClassTree(url, classId, objectId); - tree.addListener("itemdblclick", function (tree, record, item, index, e, eOpts) { - if (!record.data.root && record.data.type != "layout" + tree.addListener('itemdblclick', function (tree, record, item, index, e, eOpts) { + if (!record.data.root && record.data.type != 'layout' && record.data.dataType != 'localizedfields') { var copy = Ext.apply({}, record.data); - if (this.selectionPanel && !this.selectionPanel.getRootNode().findChild("key", record.data.key)) { + if (this.selectionPanel && !this.selectionPanel.getRootNode().findChild('key', record.data.key)) { delete copy.id; copy = this.selectionPanel.getRootNode().appendChild(copy); var ownerTree = this.selectionPanel; - if (record.data.dataType == "classificationstore") { + if (record.data.dataType == 'classificationstore') { var ccd = new opendxp.object.classificationstore.columnConfigDialog(); ccd.getConfigDialog(ownerTree, copy, this.selectionPanel); } else { @@ -581,13 +581,13 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var nodeConfig = opendxp.object.gridcolumn.operator[operator].prototype; var configTreeNode = nodeConfig.getConfigTreeNode(); - var operatorGroup = nodeConfig.operatorGroup ? nodeConfig.operatorGroup : "other"; + var operatorGroup = nodeConfig.operatorGroup ? nodeConfig.operatorGroup : 'other'; if (!operatorGroups[operatorGroup]) { operatorGroups[operatorGroup] = []; } - var groupName = nodeConfig.group || "other"; + var groupName = nodeConfig.group || 'other'; if (!operatorGroups[operatorGroup][groupName]) { operatorGroups[operatorGroup][groupName] = []; } @@ -608,7 +608,7 @@ opendxp.object.helpers.gridConfigDialog = Class.create(opendxp.element.helpers.g var operatorGroupName = operatorGroupKeys[i]; var groupNodes = operatorGroups[operatorGroupName]; let operatorTree = this.getOperatorTree(operatorGroupName, groupNodes); - operatorTree.addListener("itemdblclick", function (tree, record, item, index, e, eOpts) { + operatorTree.addListener('itemdblclick', function (tree, record, item, index, e, eOpts) { var copy = this.handleOperator(record); this.selectionPanel.getRootNode().appendChild(copy); }.bind(this)); diff --git a/src/Handler/DataObject/ClassDef/GetClassDefinitionForColumnConfig/GetClassDefinitionForColumnConfigPayload.php b/src/Handler/DataObject/ClassDef/GetClassDefinitionForColumnConfig/GetClassDefinitionForColumnConfigPayload.php index 1a3d8d75..55ff60aa 100644 --- a/src/Handler/DataObject/ClassDef/GetClassDefinitionForColumnConfig/GetClassDefinitionForColumnConfigPayload.php +++ b/src/Handler/DataObject/ClassDef/GetClassDefinitionForColumnConfig/GetClassDefinitionForColumnConfigPayload.php @@ -31,7 +31,7 @@ public static function fromRequest(Request $request): static { return new static( id: $request->query->getString('id') ?: null, - objectId: $request->query->getInt('oid'), + objectId: ($v = $request->query->get('oid')) !== null && is_numeric($v) ? (int) $v : 0, ); } } diff --git a/tests/Unit/Handler/DataObject/ClassDef/GetClassDefinitionForColumnConfig/GetClassDefinitionForColumnConfigPayloadTest.php b/tests/Unit/Handler/DataObject/ClassDef/GetClassDefinitionForColumnConfig/GetClassDefinitionForColumnConfigPayloadTest.php new file mode 100644 index 00000000..551cd7a4 --- /dev/null +++ b/tests/Unit/Handler/DataObject/ClassDef/GetClassDefinitionForColumnConfig/GetClassDefinitionForColumnConfigPayloadTest.php @@ -0,0 +1,71 @@ + 'news'])->objectId); + } + + public function testEmptyOidYieldsZero(): void + { + self::assertSame(0, self::payloadFor(['id' => 'news', 'oid' => ''])->objectId); + } + + public function testUndefinedStringOidYieldsZero(): void + { + self::assertSame(0, self::payloadFor(['id' => 'news', 'oid' => 'undefined'])->objectId); + } + + public function testNullStringOidYieldsZero(): void + { + self::assertSame(0, self::payloadFor(['id' => 'news', 'oid' => 'null'])->objectId); + } + + public function testNumericOidIsParsed(): void + { + self::assertSame(42, self::payloadFor(['id' => 'news', 'oid' => '42'])->objectId); + } + + public function testZeroOidYieldsZero(): void + { + self::assertSame(0, self::payloadFor(['id' => 'news', 'oid' => '0'])->objectId); + } + + public function testIdIsReadAsString(): void + { + self::assertSame('news', self::payloadFor(['id' => 'news'])->id); + } + + public function testAbsentIdYieldsNull(): void + { + self::assertNull(self::payloadFor([])->id); + } +}