Skip to content

Commit d549045

Browse files
committed
[FIX] xConfig: Use property-type specific comparison for removal
When removing entries from xConfig properties, filter conditions require a different comparison than sort/group conditions: - filterConditions: Multiple conditions can exist for the same key, so we compare key AND condition content to find the correct entry - sortConditions/groupConditions: Key is unique, so key-only comparison is sufficient The previous fix (commit df947a9) used deepEqual on the entire entry, which broke sort/group reset because the payload index (restore position) differs from the stored index (original position). This fix addresses both issues by using targeted comparison logic based on the property type. Fixes: #4277 SNOW: DINC0728376 SNOW: DINC0490163 Change-Id: I6b040d5aa8c5f62b55425acb19532a40cd933a0f
1 parent 05cca48 commit d549045

File tree

2 files changed

+142
-1
lines changed

2 files changed

+142
-1
lines changed

src/sap.m/src/sap/m/p13n/modules/xConfigAPI.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
*/
44
sap.ui.define([
55
"sap/base/util/merge",
6+
"sap/base/util/deepEqual",
67
"sap/ui/core/util/reflection/JsControlTreeModifier"
7-
], (merge, JsControlTreeModifier) => {
8+
], (merge, deepEqual, JsControlTreeModifier) => {
89
"use strict";
910

1011
/**
@@ -449,6 +450,16 @@ sap.ui.define([
449450
const sOperation = oModificationPayload.operation;
450451

451452
const oItem = oConfig.properties[sAffectedProperty].find((oEntry) => {
453+
// DINC0728376 / DINC0490163: Property-type specific comparison
454+
// For filterConditions: multiple conditions can exist for the same key,
455+
// so we need to compare the condition content to find the correct one.
456+
// For sortConditions/groupConditions: key is unique, so key comparison is sufficient.
457+
// We cannot use deepEqual on the entire entry because the index in the payload
458+
// (restore position) differs from the stored index (original position).
459+
if (sAffectedProperty === "filterConditions") {
460+
return oEntry.key === oModificationPayload.key &&
461+
deepEqual(oEntry.condition, oModificationPayload.value.condition);
462+
}
452463
return oEntry.key === oModificationPayload.key;
453464
});
454465

src/sap.m/test/sap/m/qunit/p13n/xConfigAPI.qunit.js

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,4 +520,134 @@ sap.ui.define([
520520
});
521521
});
522522

523+
// DINC0728376: Test that sort conditions can be removed even when payload index differs from stored index
524+
// This test verifies that sort removal works correctly - it should pass with key-only comparison
525+
// but would fail if we used deepEqual (because index differs)
526+
QUnit.test("Check that sort condition is removed correctly when payload index differs from stored index (DINC0728376)", function(assert) {
527+
// Step 1: Add a sort condition at index 0
528+
const oAddPayload = {
529+
key: "sort_field_1",
530+
property: "sortConditions",
531+
operation: "add",
532+
value: {
533+
key: "sort_field_1",
534+
descending: false,
535+
index: 0
536+
}
537+
};
538+
539+
return xConfigAPI.enhanceConfig(this.oControl, oAddPayload)
540+
.then(() => {
541+
const oCustomData = this.oControl.getCustomData()[0];
542+
const oConfig = JSON.parse(oCustomData.getValue().replace(/\\/g, ''));
543+
544+
assert.deepEqual(oConfig, {
545+
"properties": {
546+
"sortConditions": [
547+
{
548+
"key": "sort_field_1",
549+
"descending": false,
550+
"index": 0
551+
}
552+
]
553+
}
554+
}, "Sort condition was added correctly");
555+
556+
// Step 2: Remove the sort condition with a DIFFERENT index in payload
557+
// This simulates what happens during revert - the payload index is the "restore to" position,
558+
// not the current position in the array
559+
const oRemovePayload = {
560+
key: "sort_field_1",
561+
property: "sortConditions",
562+
operation: "remove",
563+
value: {
564+
key: "sort_field_1",
565+
descending: false,
566+
index: 5 // Different index than stored (0) - this is the revert scenario
567+
}
568+
};
569+
570+
return xConfigAPI.enhanceConfig(this.oControl, oRemovePayload);
571+
})
572+
.then(() => {
573+
const oCustomData = this.oControl.getCustomData()[0];
574+
const oConfig = JSON.parse(oCustomData.getValue().replace(/\\/g, ''));
575+
576+
// The sort condition should be removed, leaving an empty array
577+
assert.deepEqual(oConfig, {
578+
"properties": {
579+
"sortConditions": []
580+
}
581+
}, "Sort condition was removed correctly even though payload index differed from stored index");
582+
});
583+
});
584+
585+
// DINC0728376: Test that the correct filter condition is removed when multiple exist for same key
586+
QUnit.test("Check that correct filter condition is removed when multiple conditions exist for same key (DINC0490163)", function(assert) {
587+
// Step 1: Add first filter condition
588+
const oAddPayload1 = {
589+
key: "filter_field",
590+
property: "filterConditions",
591+
operation: "add",
592+
value: {
593+
key: "filter_field",
594+
condition: {
595+
operator: "EQ",
596+
values: ["Maria"]
597+
}
598+
}
599+
};
600+
601+
return xConfigAPI.enhanceConfig(this.oControl, oAddPayload1)
602+
.then(() => {
603+
// Step 2: Add second filter condition for same key
604+
const oAddPayload2 = {
605+
key: "filter_field",
606+
property: "filterConditions",
607+
operation: "add",
608+
value: {
609+
key: "filter_field",
610+
condition: {
611+
operator: "EQ",
612+
values: ["John"]
613+
}
614+
}
615+
};
616+
return xConfigAPI.enhanceConfig(this.oControl, oAddPayload2);
617+
})
618+
.then(() => {
619+
const oCustomData = this.oControl.getCustomData()[0];
620+
const oConfig = JSON.parse(oCustomData.getValue().replace(/\\/g, ''));
621+
622+
assert.equal(oConfig.properties.filterConditions.length, 2, "Two filter conditions exist");
623+
624+
// Step 3: Remove the "Maria" condition specifically
625+
const oRemovePayload = {
626+
key: "filter_field",
627+
property: "filterConditions",
628+
operation: "remove",
629+
value: {
630+
key: "filter_field",
631+
condition: {
632+
operator: "EQ",
633+
values: ["Maria"]
634+
}
635+
}
636+
};
637+
638+
return xConfigAPI.enhanceConfig(this.oControl, oRemovePayload);
639+
})
640+
.then(() => {
641+
const oCustomData = this.oControl.getCustomData()[0];
642+
const oConfig = JSON.parse(oCustomData.getValue().replace(/\\/g, ''));
643+
644+
// Only "John" should remain
645+
assert.equal(oConfig.properties.filterConditions.length, 1, "One filter condition remains");
646+
assert.deepEqual(oConfig.properties.filterConditions[0].condition, {
647+
operator: "EQ",
648+
values: ["John"]
649+
}, "The correct filter condition (John) remains - Maria was removed");
650+
});
651+
});
652+
523653
});

0 commit comments

Comments
 (0)