Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(#16584): filterTableData source of truth (#36849)

Browse Source 
## Description
> **TL;DR**: This PR addresses is related to #16584 where editing a
table row with applied filters becomes impossible without clearing the
filters

When a filter is applied to the table data, the current behavior results
in filter updates without validating or saving the updated value of the
row being edited. This leads to a situation where users are unable to
save or discard changes until the filters are cleared. The proposed fix
ensures that the original row is retrieved from the table data and used
in filtering, allowing editing and filtering to work as expected.

Here's a [screen
record](https://drive.google.com/file/d/1JuP_UN_B1vzz_oMeR1ojjPscF_mB3A4x/view?usp=sharing)
for the solution

### Motivation:
The problem arises in scenarios where table rows are editable and
filters are applied. This change ensures that users can continue editing
table rows without being forced to clear filters first.

### Context:
This change is required to improve user experience and fix the broken
editing functionality when filters are active. The change ensures that
editable values in filtered rows are correctly handled and saved.

Fixes https://www.loom.com/share/335d0c61817646a0903d581adf73064e

## Automation

/ok-to-test tags="table-widget,filter,edit"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!CAUTION]  
> If you modify the content in this section, you are likely to disrupt
the CI result for your PR.

<!-- end of auto-generated comment: Cypress test results  -->

## Communication
Should the DevRel and Marketing teams inform users about this change?
- [x] Yes
- [ ] No



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Improved filtering accuracy in the TableWidgetV2 by using original row
data for evaluations.

- **Bug Fixes**
- Enhanced functionality to ensure that filtering conditions are
evaluated correctly with original data.

- **Tests**
- Added new test cases to validate filtering functionality after edits
in the TableWidgetV2.
- Expanded test coverage for checkbox and switch interactions to ensure
accurate filtering behavior, including new interactions with a discard
button.

- **Style**
	- Minor adjustments to comments and formatting for better readability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Anas Khafaga 2024-10-30 15:03:01 +03:00 committed by GitHub
parent c6cf919beb
commit 2cfe0b0fac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 378 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/checkboxCell_spec.js
View File

@ -145,6 +145,7 @@ describe(
.contains("This is a test message")
.should("be.visible");
});
_.agHelper.ClickButton("Discard", { index: 0 }); // discard changes
});
it("4. Verify filter condition", () => {
@ -159,7 +160,17 @@ describe(
// filter and verify checked rows
cy.getTableV2DataSelector("0", "4").then((selector) => {
cy.get(selector + checkboxSelector).should("be.checked");
_.agHelper.AssertExistingCheckedState(
selector + checkboxSelector,
"true",
);
});
cy.getTableV2DataSelector("1", "4").then((selector) => {
_.agHelper.AssertExistingCheckedState(
selector + checkboxSelector,
"true",
);
});
// Filter and verify unchecked rows
@ -170,10 +181,10 @@ describe(
cy.get(publishPage.applyFiltersBtn).click();
cy.getTableV2DataSelector("0", "4").then((selector) => {
cy.get(selector + checkboxSelector).should("not.be.checked");
});
cy.getTableV2DataSelector("1", "4").then((selector) => {
cy.get(selector + checkboxSelector).should("not.be.checked");
_.agHelper.AssertExistingCheckedState(
selector + checkboxSelector,
"false",
);
});
});

12
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js
View File

@ -137,6 +137,7 @@ describe(
cy.wait(100);
agHelper.ValidateToastMessage("This is a test message");
});
agHelper.ClickButton("Discard", { index: 0 }); // discard changes
});
it("4. Verify filter condition", () => {
@ -151,7 +152,11 @@ describe(
// filter and verify checked rows
cy.getTableV2DataSelector("0", "4").then((selector) => {
cy.get(selector + switchSelector).should("be.checked");
agHelper.AssertExistingCheckedState(selector + switchSelector, "true");
});
cy.getTableV2DataSelector("1", "4").then((selector) => {
agHelper.AssertExistingCheckedState(selector + switchSelector, "true");
});
// Filter and verify unchecked rows
@ -162,10 +167,7 @@ describe(
cy.get(publishPage.applyFiltersBtn).click();
cy.getTableV2DataSelector("0", "4").then((selector) => {
cy.get(selector + switchSelector).should("not.be.checked");
});
cy.getTableV2DataSelector("1", "4").then((selector) => {
cy.get(selector + switchSelector).should("not.be.checked");
agHelper.AssertExistingCheckedState(selector + switchSelector, "false");
});
});

56
app/client/src/widgets/TableWidgetV2/widget/derived.js
View File

@ -446,25 +446,42 @@ export default {
sortedTableData = transformedTableDataForSorting.sort((a, b) => {
if (_.isPlainObject(a) && _.isPlainObject(b)) {
let [processedA, processedB] = [a, b];
if (!selectColumnKeysWithSortByLabel.length) {
const originalA = (props.tableData ??
transformedTableDataForSorting)[a.__originalIndex__];
const originalB = (props.tableData ??
transformedTableDataForSorting)[b.__originalIndex__];
[processedA, processedB] = [
{ ...a, ...originalA },
{ ...b, ...originalB },
];
}
if (
isEmptyOrNil(a[sortByColumnOriginalId]) ||
isEmptyOrNil(b[sortByColumnOriginalId])
isEmptyOrNil(processedA[sortByColumnOriginalId]) ||
isEmptyOrNil(processedB[sortByColumnOriginalId])
) {
/* push null, undefined and "" values to the bottom. */
return isEmptyOrNil(a[sortByColumnOriginalId]) ? 1 : -1;
return isEmptyOrNil(processedA[sortByColumnOriginalId]) ? 1 : -1;
} else {
switch (columnType) {
case "number":
case "currency":
return sortByOrder(
Number(a[sortByColumnOriginalId]) >
Number(b[sortByColumnOriginalId]),
Number(processedA[sortByColumnOriginalId]) >
Number(processedB[sortByColumnOriginalId]),
);
case "date":
try {
return sortByOrder(
moment(a[sortByColumnOriginalId], inputFormat).isAfter(
moment(b[sortByColumnOriginalId], inputFormat),
moment(
processedA[sortByColumnOriginalId],
inputFormat,
).isAfter(
moment(processedB[sortByColumnOriginalId], inputFormat),
),
);
} catch (e) {
@ -489,8 +506,8 @@ export default {
}
default:
return sortByOrder(
a[sortByColumnOriginalId].toString().toLowerCase() >
b[sortByColumnOriginalId].toString().toLowerCase(),
processedA[sortByColumnOriginalId].toString().toLowerCase() >
processedB[sortByColumnOriginalId].toString().toLowerCase(),
);
}
}
@ -676,6 +693,9 @@ export default {
const finalTableData = sortedTableData.filter((row) => {
let isSearchKeyFound = true;
const originalRow = (props.tableData ?? sortedTableData)[
row.__originalIndex__
];
const columnWithDisplayText = Object.values(props.primaryColumns).filter(
(column) => column.columnType === "url" && column.displayText,
);
@ -780,7 +800,10 @@ export default {
};
if (searchKey) {
isSearchKeyFound = Object.values(_.omit(displayedRow, hiddenColumns))
isSearchKeyFound = [
...Object.values(_.omit(displayedRow, hiddenColumns)),
...Object.values(_.omit(originalRow, hiddenColumns)),
]
.join(", ")
.toLowerCase()
.includes(searchKey);
@ -811,10 +834,15 @@ export default {
ConditionFunctions[props.filters[i].condition];
if (conditionFunction) {
filterResult = conditionFunction(
displayedRow[props.filters[i].column],
props.filters[i].value,
);
filterResult =
conditionFunction(
originalRow[props.filters[i].column],
props.filters[i].value,
) ||
conditionFunction(
displayedRow[props.filters[i].column],
props.filters[i].value,
);
}
} catch (e) {
filterResult = false;

313
app/client/src/widgets/TableWidgetV2/widget/derived.test.js
View File

@ -580,6 +580,93 @@ describe("Validates getFilteredTableData Properties", () => {
expect(result).toStrictEqual(expected);
});
it("validates generated filtered edited table data to be sorted correctly based on column type", () => {
const { getFilteredTableData } = derivedProperty;
const input = {
processedTableData: [
{ id: 123, name: "BAC", __originalIndex__: 0 },
{ id: 1234, name: "ABC", __originalIndex__: 1 },
{ id: 234, name: "CAB", __originalIndex__: 2 },
],
tableData: [
{ id: 123, name: "BAC" },
{ id: 1234, name: "ABC" },
{ id: 234, name: "CAB" },
],
sortOrder: { column: "name", order: "asc" },
columnOrder: ["name", "id"],
primaryColumns: {
id: {
index: 1,
width: 150,
id: "id",
alias: "id",
originalId: "id",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "number",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "id",
isAscOrder: false,
},
name: {
index: 0,
width: 150,
id: "name",
alias: "name",
originalId: "name",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "text",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "awesome",
isAscOrder: undefined,
computedValue: ["BAC", "ABC", "AAB"],
},
},
};
input.orderedTableColumns = Object.values(input.primaryColumns).sort(
(a, b) => {
return input.columnOrder[a.id] < input.columnOrder[b.id];
},
);
const expected = [
{
id: 1234,
name: "ABC",
__originalIndex__: 1,
},
{
id: 123,
name: "BAC",
__originalIndex__: 0,
},
{
id: 234,
name: "AAB",
__originalIndex__: 2,
},
];
let result = getFilteredTableData(input, moment, _);
expect(result).toStrictEqual(expected);
});
it("validates generated filtered table data with null values to be sorted correctly", () => {
const { getFilteredTableData } = derivedProperty;
const input = {
@ -1181,6 +1268,122 @@ describe("Validates getFilteredTableData Properties", () => {
expect(result).toStrictEqual(expected);
});
it("should filter correctly after editing a value with an applied filter", () => {
const { getFilteredTableData } = derivedProperty;
const input = {
tableData: [
{ id: 1234, name: "Jim Doe" },
{ id: 123, name: "Hamza Khafaga" },
{ id: 234, name: "Khadija Khafaga" },
],
processedTableData: [
{ id: 1234, name: "Jim Doe", __originalIndex__: 0 },
{ id: 123, name: "Hamza Anas", __originalIndex__: 1 },
{ id: 234, name: "Khadija Khafaga", __originalIndex__: 2 },
],
filters: [
{
condition: "contains",
column: "name",
value: "Khafaga",
},
],
sortOrder: { column: "id", order: "desc" },
columnOrder: ["name", "id"],
primaryColumns: {
id: {
index: 1,
width: 150,
id: "id",
alias: "id",
originalId: "id",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "number",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "id",
isAscOrder: false,
},
name: {
index: 0,
width: 150,
id: "name",
alias: "name",
originalId: "name",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "text",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "awesome",
isAscOrder: undefined,
},
},
tableColumns: [
{
index: 0,
width: 150,
id: "name",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "text",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "awesome",
isAscOrder: undefined,
},
{
index: 1,
width: 150,
id: "id",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "number",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "id",
isAscOrder: false,
},
],
};
input.orderedTableColumns = Object.values(input.primaryColumns).sort(
(a, b) => {
return input.columnOrder[a.id] < input.columnOrder[b.id];
},
);
const expected = [
{ id: 234, name: "Khadija Khafaga", __originalIndex__: 2 },
{ id: 123, name: "Hamza Anas", __originalIndex__: 1 },
];
let result = getFilteredTableData(input, moment, _);
expect(result).toStrictEqual(expected);
});
it("validates generated sanitized table data with valid property keys", () => {
const { getProcessedTableData } = derivedProperty;
@ -1333,6 +1536,116 @@ describe("Validates getFilteredTableData Properties", () => {
expect(result).toStrictEqual(expected);
});
it("filters correctly after editing a value with an applied search key", () => {
const { getFilteredTableData } = derivedProperty;
const input = {
tableData: [
{ id: 1234, name: "Jim Doe" },
{ id: 123, name: "Hamza Khafaga" },
{ id: 234, name: "Khadija Khafaga" },
],
processedTableData: [
{ id: 1234, name: "Jim Doe", __originalIndex__: 0 },
{ id: 123, name: "Hamza Anas", __originalIndex__: 1 },
{ id: 234, name: "Khadija Khafaga", __originalIndex__: 2 },
],
searchText: "Khafaga",
sortOrder: { column: "id", order: "desc" },
columnOrder: ["name", "id"],
primaryColumns: {
id: {
index: 1,
width: 150,
id: "id",
alias: "id",
originalId: "id",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "number",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "id",
isAscOrder: false,
},
name: {
index: 0,
width: 150,
id: "name",
alias: "name",
originalId: "name",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "text",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "awesome",
isAscOrder: undefined,
},
},
tableColumns: [
{
index: 0,
width: 150,
id: "name",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "text",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "awesome",
isAscOrder: undefined,
},
{
index: 1,
width: 150,
id: "id",
horizontalAlignment: "LEFT",
verticalAlignment: "CENTER",
columnType: "number",
textColor: "#231F20",
textSize: "PARAGRAPH",
fontStyle: "REGULAR",
enableFilter: true,
enableSort: true,
isVisible: true,
isDerived: false,
label: "id",
isAscOrder: false,
},
],
};
input.orderedTableColumns = Object.values(input.primaryColumns).sort(
(a, b) => {
return input.columnOrder[a.id] < input.columnOrder[b.id];
},
);
const expected = [
{ id: 234, name: "Khadija Khafaga", __originalIndex__: 2 },
{ id: 123, name: "Hamza Anas", __originalIndex__: 1 },
];
let result = getFilteredTableData(input, moment, _);
expect(result).toStrictEqual(expected);
});
});
describe("Validate getSelectedRow function", () => {