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

fix: retain columns on query change in deployed app (#24245)

Browse Source 
## Description
This PR fixes the behaviour mentioned in the issue. 

This issue was happening due to table widget's column order wasn't
updating. It wasn't updating because we maintained column orders in
widget's DSL as well as in the meta property. Whenever the table data
changes, the widget updates `columnOrder` property in the DSL but this
gets overridden if you have the same property in the widget's meta
property.

To solve this we update the table widget's sticky and column order
property via `super.updateWidgetProperty`. We have also refactor the
hydration logic for sticky columns such that table widget's DSL acts as
a source of truth all the time.

Fixes #23969
#23827

#### Type of change
- Bug fix (non-breaking change which fixes an issue)

## Testing
#### How Has This Been Tested?
> Please describe the tests that you ran to verify your changes. Also
list any relevant details for your test configuration.
> Delete anything that is not relevant
- [x] Manual
- To test that the column freeze functionality works as expected in edit
as well as deployed mode
- To check column freeze functionality works as expected when table
query changes
- To check if the column reordering is working as expected in view mode
- [ ] Jest
- [x] Cypress
- should test that the number of columns needs to be same when table
data changes in deployed app
>
>
#### Test Plan
> Add Testsmith test cases links that relate to this PR
>
>
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Test-plan-implementation#speedbreaker-features-to-consider-for-every-change)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans/_edit#areas-of-interest)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [ ] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
This commit is contained in:
Keyur Paralkar 2023-06-27 10:45:41 +05:30 committed by GitHub
parent 341de5c61f
commit 614b3fe736
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 319 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

212
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/table_data_change_spec.ts Normal file
View File

@ -0,0 +1,212 @@
import {
entityExplorer,
propPane,
agHelper,
draggableWidgets,
deployMode,
table,
locators,
} from "../../../../../support/Objects/ObjectsCore";
const readTableLocalColumnOrder = (columnOrderKey: string) => {
const localColumnOrder = window.localStorage.getItem(columnOrderKey) || "";
if (localColumnOrder) {
const parsedTableConfig = JSON.parse(localColumnOrder);
if (parsedTableConfig) {
const tableWidgetId = Object.keys(parsedTableConfig)[0];
return parsedTableConfig[tableWidgetId];
}
}
};
const freezeColumnFromDropdown = (columnName: string, direction: string) => {
agHelper
.GetElement(`[data-header=${columnName}] .header-menu .bp3-popover2-target`)
.click({ force: true });
agHelper.GetNClickByContains(".bp3-menu", `Freeze column ${direction}`);
};
const checkIfColumnIsFrozenViaCSS = (
columnName: string,
position = "sticky",
) => {
agHelper
.GetElement(table._headerCell(columnName))
.should("have.css", "position", position);
};
const TABLE_DATA_1 = `[
{
"step": "#1",
"task": "Drop a table",
"status": "✅",
"action": ""
},
{
"step": "#2",
"task": "Create a query fetch_users with the Mock DB",
"status": "--",
"action": ""
},
{
"step": "#3",
"task": "Bind the query using => fetch_users.data",
"status": "--",
"action": ""
}
]`;
const TABLE_DATA_2 = `[
{
"id": 1,
"name": "Barty Crouch",
"status": "APPROVED",
"gender": "",
"avatar": "https://robohash.org/sednecessitatibuset.png?size=100x100&set=set1",
"email": "barty.crouch@gmail.com",
"address": "St Petersberg #911 4th main",
"createdAt": "2020-03-16T18:00:05.000Z",
"updatedAt": "2020-08-12T17:29:31.980Z"
},
{
"id": 2,
"name": "Jenelle Kibbys",
"status": "APPROVED",
"gender": "Female",
"avatar": "https://robohash.org/quiaasperiorespariatur.bmp?size=100x100&set=set1",
"email": "jkibby1@hp.com",
"address": "85 Tennessee Plaza",
"createdAt": "2019-10-04T03:22:23.000Z",
"updatedAt": "2019-09-11T20:18:38.000Z"
},
{
"id": 3,
"name": "Demetre",
"status": "APPROVED",
"gender": "Male",
"avatar": "https://robohash.org/iustooptiocum.jpg?size=100x100&set=set1",
"email": "aaaa@bbb.com",
"address": "262 Saint Paul Park",
"createdAt": "2020-05-01T17:30:50.000Z",
"updatedAt": "2019-10-08T14:55:53.000Z"
},
{
"id": 10,
"name": "Tobin Shellibeer",
"status": "APPROVED",
"gender": "Male",
"avatar": "https://robohash.org/odioeumdolores.png?size=100x100&set=set1",
"email": "tshellibeer9@ihg.com",
"address": "4 Ridgeway Lane",
"createdAt": "2019-11-27T06:09:41.000Z",
"updatedAt": "2019-09-07T16:35:48.000Z"
}]`;
describe("Table widget v2: tableData change test", function () {
before(() => {
agHelper.ClearLocalStorageCache();
});
it("1. should test that the number of columns needs to be same when table data changes in depoyed app", function () {
entityExplorer.DragDropWidgetNVerify(draggableWidgets.TABLE, 300, 100);
propPane.EnterJSContext(
"Table data",
`{{appsmith.store.test === '0' ? ${TABLE_DATA_1} : ${TABLE_DATA_2}}}`,
);
entityExplorer.DragDropWidgetNVerify(draggableWidgets.BUTTON, 500, 500);
propPane.UpdatePropertyFieldValue("Label", "Set table data 1");
propPane.SelectPlatformFunction("onClick", "Store value");
agHelper.EnterActionValue("Key", "test");
agHelper.EnterActionValue("Value", "0");
// add a success callback
agHelper.GetNClick(propPane._actionCallbacks);
agHelper.GetNClick(propPane._actionAddCallback("success"));
agHelper.GetNClick(locators._dropDownValue("Show alert"));
agHelper.EnterActionValue("Message", "table data 1 set");
entityExplorer.DragDropWidgetNVerify(draggableWidgets.BUTTON, 500, 600);
propPane.UpdatePropertyFieldValue("Label", "Set table data 2");
propPane.SelectPlatformFunction("onClick", "Store value");
agHelper.EnterActionValue("Key", "test");
agHelper.EnterActionValue("Value", "1");
// add a success callback
agHelper.GetNClick(propPane._actionCallbacks);
agHelper.GetNClick(propPane._actionAddCallback("success"));
agHelper.GetNClick(locators._dropDownValue("Show alert"));
agHelper.EnterActionValue("Message", "table data 2 set");
deployMode.DeployApp();
agHelper.ClickButton("Set table data 1");
agHelper.WaitUntilToastDisappear("table data 1 set");
table.AssertTableHeaderOrder("statussteptaskaction");
let tableLocalColumnOrder = readTableLocalColumnOrder(
"tableWidgetColumnOrder",
);
if (tableLocalColumnOrder)
expect(tableLocalColumnOrder.columnOrder.join("")).equal(
"statussteptaskaction",
);
agHelper.ClickButton("Set table data 2");
agHelper.WaitUntilToastDisappear("table data 2 set");
table.AssertTableHeaderOrder(
"statusidnamegenderavataremailaddresscreatedAtupdatedAt",
);
tableLocalColumnOrder = readTableLocalColumnOrder("tableWidgetColumnOrder");
if (tableLocalColumnOrder)
expect(tableLocalColumnOrder.columnOrder.join("")).equal(
"statusidnamegenderavataremailaddresscreatedAtupdatedAt",
);
/**
* Flow: Check the above flow for frozen columns
* 1. Freeze columns with table data 1.
* 2. Refresh the page.
* 3. Check if the frozen columns are same.
* 4. Similarly do the same thing for table data 2.
*/
agHelper.ClickButton("Set table data 1");
table.AssertTableHeaderOrder("statussteptaskaction");
tableLocalColumnOrder = readTableLocalColumnOrder("tableWidgetColumnOrder");
if (tableLocalColumnOrder)
expect(tableLocalColumnOrder.columnOrder.join("")).equal(
"statussteptaskaction",
);
freezeColumnFromDropdown("status", "left");
freezeColumnFromDropdown("action", "right");
agHelper.RefreshPage(true, "viewPage");
checkIfColumnIsFrozenViaCSS("status");
checkIfColumnIsFrozenViaCSS("action");
agHelper.ClickButton("Set table data 2");
table.AssertTableHeaderOrder(
"statusidnamegenderavataremailaddresscreatedAtupdatedAt",
);
tableLocalColumnOrder = readTableLocalColumnOrder("tableWidgetColumnOrder");
if (tableLocalColumnOrder)
expect(tableLocalColumnOrder.columnOrder.join("")).equal(
"statusidnamegenderavataremailaddresscreatedAtupdatedAt",
);
freezeColumnFromDropdown("id", "left");
freezeColumnFromDropdown("updatedAt", "right");
agHelper.RefreshPage(true, "viewPage");
checkIfColumnIsFrozenViaCSS("id");
checkIfColumnIsFrozenViaCSS("updatedAt");
});
});

140
app/client/src/widgets/TableWidgetV2/widget/index.tsx
View File

@ -27,7 +27,8 @@ import { EventType } from "constants/AppsmithActionConstants/ActionConstants";
import Skeleton from "components/utils/Skeleton";
import { noop, retryPromise } from "utils/AppsmithUtils";
import { SORT_ORDER } from "../component/Constants";
import type { StickyType, ReactTableFilter } from "../component/Constants";
import { StickyType } from "../component/Constants";
import type { ReactTableFilter } from "../component/Constants";
import { AddNewRowActions, DEFAULT_FILTER } from "../component/Constants";
import type {
EditableCell,
@ -388,13 +389,8 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
* based on columnType
*/
getTableColumns = () => {
const {
columnWidthMap,
orderedTableColumns,
primaryColumns,
renderMode,
widgetId,
} = this.props;
const { columnWidthMap, orderedTableColumns, renderMode, widgetId } =
this.props;
const { componentWidth } = this.getPaddingAdjustedDimensions();
const widgetLocalStorageState = getColumnOrderByWidgetIdFromLS(widgetId);
const memoisdGetColumnsWithLocalStorage =
@ -404,9 +400,7 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
columnWidthMap,
orderedTableColumns,
componentWidth,
primaryColumns,
renderMode,
widgetId,
);
};
@ -518,6 +512,7 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
*/
updateColumnProperties = (
tableColumns?: Record<string, ColumnProperties>,
shouldPersistLocalOrderWhenTableDataChanges = false,
) => {
const { columnOrder = [], primaryColumns = {} } = this.props;
const derivedColumns = getDerivedColumns(primaryColumns);
@ -571,6 +566,28 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
newColumnOrder.sort(compareColumns);
propertiesToAdd["columnOrder"] = newColumnOrder;
/**
* As the table data changes in Deployed app, we also update the local storage.
*
* this.updateColumnProperties gets executed on mount and on update of the component.
* On mount we get new tableColumns that may not have any sticky columns.
* This will lead to loss of sticky column that were frozen by the user.
* To avoid this and to maintain user's sticky columns we use shouldPersistLocalOrderWhenTableDataChanges below
* so as to avoid updating the local storage on mount.
**/
if (
this.props.renderMode === RenderModes.PAGE &&
shouldPersistLocalOrderWhenTableDataChanges
) {
const leftOrder = newColumnOrder.filter(
(col: string) => tableColumns[col].sticky === StickyType.LEFT,
);
const rightOrder = newColumnOrder.filter(
(col: string) => tableColumns[col].sticky === StickyType.RIGHT,
);
this.persistColumnOrder(newColumnOrder, leftOrder, rightOrder);
}
}
const propertiesToUpdate: BatchPropertyUpdatePayload = {
@ -614,7 +631,12 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
);
if (localTableColumnOrder) {
const { columnOrder, columnUpdatedAt } = localTableColumnOrder;
const {
columnOrder,
columnUpdatedAt,
leftOrder: localLeftOrder,
rightOrder: localRightOrder,
} = localTableColumnOrder;
if (this.props.columnUpdatedAt !== columnUpdatedAt) {
// Delete and set the column orders defined by the developer
@ -626,7 +648,47 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
rightOrder,
);
} else {
this.props.updateWidgetMetaProperty("columnOrder", columnOrder);
const propertiesToAdd: Record<string, string> = {};
propertiesToAdd["columnOrder"] = columnOrder;
/**
* We reset the sticky values of the columns that were frozen by the developer.
*/
if (Object.keys(this.props.primaryColumns).length > 0) {
columnOrder.forEach((colName: string) => {
if (
this.props.primaryColumns[colName]?.sticky !== StickyType.NONE
) {
propertiesToAdd[`primaryColumns.${colName}.sticky`] =
StickyType.NONE;
}
});
}
/**
* We pickup the left and the right frozen columns from the localstorage
* and update the sticky value of these columns respectively.
*/
if (localLeftOrder.length > 0) {
localLeftOrder.forEach((colName: string) => {
propertiesToAdd[`primaryColumns.${colName}.sticky`] =
StickyType.LEFT;
});
}
if (localRightOrder.length > 0) {
localRightOrder.forEach((colName: string) => {
propertiesToAdd[`primaryColumns.${colName}.sticky`] =
StickyType.RIGHT;
});
}
const propertiesToUpdate = {
modify: propertiesToAdd,
};
super.batchUpdateWidgetProperty(propertiesToUpdate);
}
} else {
// If user deletes local storage or no column orders for the given table widget exists hydrate it with the developer changes.
@ -641,11 +703,6 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
componentDidMount() {
const { canFreezeColumn, renderMode, tableData } = this.props;
if (canFreezeColumn && renderMode === RenderModes.PAGE) {
//dont neet to batch this since single action
this.hydrateStickyColumns();
}
if (_.isArray(tableData) && !!tableData.length) {
const newPrimaryColumns = this.createTablePrimaryColumns();
@ -654,6 +711,11 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
this.updateColumnProperties(newPrimaryColumns);
}
}
if (canFreezeColumn && renderMode === RenderModes.PAGE) {
//dont neet to batch this since single action
this.hydrateStickyColumns();
}
}
componentDidUpdate(prevProps: TableWidgetProps) {
@ -721,7 +783,7 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
const newTableColumns = this.createTablePrimaryColumns();
if (newTableColumns) {
this.updateColumnProperties(newTableColumns);
this.updateColumnProperties(newTableColumns, isTableDataModified);
}
pushBatchMetaUpdates("filters", [DEFAULT_FILTER]);
@ -1157,8 +1219,16 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
updatedOrders.leftOrder,
updatedOrders.rightOrder,
);
// only a single meta property update no need to batch this
this.props.updateWidgetMetaProperty("columnOrder", newColumnOrder);
super.batchUpdateWidgetProperty(
{
modify: {
[`primaryColumns.${columnName}.sticky`]: sticky,
columnOrder: newColumnOrder,
},
},
true,
);
}
}
};
@ -1166,24 +1236,22 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
handleReorderColumn = (columnOrder: string[]) => {
columnOrder = columnOrder.map((alias) => this.getColumnIdByAlias(alias));
if (this.props.renderMode === RenderModes.CANVAS) {
super.updateWidgetProperty("columnOrder", columnOrder);
} else {
if (this.props.canFreezeColumn) {
const localTableColumnOrder = getColumnOrderByWidgetIdFromLS(
this.props.widgetId,
);
if (localTableColumnOrder) {
const { leftOrder, rightOrder } = localTableColumnOrder;
this.persistColumnOrder(columnOrder, leftOrder, rightOrder);
} else {
this.persistColumnOrder(columnOrder, [], []);
}
if (
this.props.canFreezeColumn &&
this.props.renderMode === RenderModes.PAGE
) {
const localTableColumnOrder = getColumnOrderByWidgetIdFromLS(
this.props.widgetId,
);
if (localTableColumnOrder) {
const { leftOrder, rightOrder } = localTableColumnOrder;
this.persistColumnOrder(columnOrder, leftOrder, rightOrder);
} else {
this.persistColumnOrder(columnOrder, [], []);
}
// only a single meta property update no need to batch this
this.props.updateWidgetMetaProperty("columnOrder", columnOrder);
}
super.updateWidgetProperty("columnOrder", columnOrder);
};
handleColumnSorting = (columnAccessor: string, isAsc: boolean) => {

12
app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/getColumnsPureFn.tsx
View File

@ -7,11 +7,7 @@ import {
DEFAULT_COLUMN_WIDTH,
DEFAULT_COLUMN_NAME,
} from "../../constants";
import { fetchSticky } from "../utilities";
import type {
ColumnProperties,
ReactTableColumnProps,
} from "../../component/Constants";
import type { ReactTableColumnProps } from "../../component/Constants";
import memoizeOne from "memoize-one";
export type getColumns = (
@ -19,9 +15,7 @@ export type getColumns = (
columnWidthMap: { [key: string]: number } | undefined,
orderedTableColumns: any,
componentWidth: number,
primaryColumns: Record<string, ColumnProperties>,
renderMode: RenderMode,
widgetId: string,
) => ReactTableColumnProps[];
//TODO: (Vamsi) need to unit test this function
@ -31,9 +25,7 @@ export const getColumnsPureFn: getColumns = (
columnWidthMap = {},
orderedTableColumns = [],
componentWidth,
primaryColumns,
renderMode,
widgetId,
) => {
let columns: ReactTableColumnProps[] = [];
const hiddenColumns: ReactTableColumnProps[] = [];
@ -58,7 +50,7 @@ export const getColumnsPureFn: getColumns = (
isHidden: false,
isAscOrder: column.isAscOrder,
isDerived: column.isDerived,
sticky: fetchSticky(column.id, primaryColumns, renderMode, widgetId),
sticky: column.sticky,
metaProperties: {
isHidden: isHidden,
type: column.columnType,

29
app/client/src/widgets/TableWidgetV2/widget/utilities.ts
View File

@ -1,6 +1,5 @@
import { Colors } from "constants/Colors";
import type { RenderMode } from "constants/WidgetConstants";
import { FontStyleTypes, RenderModes } from "constants/WidgetConstants";
import { FontStyleTypes } from "constants/WidgetConstants";
import _, { filter, isBoolean, isObject, uniq, without } from "lodash";
import tinycolor from "tinycolor2";
import type {
@ -873,32 +872,6 @@ export const deleteLocalTableColumnOrderByWidgetId = (widgetId: string) => {
}
};
export const fetchSticky = (
columnId: string,
primaryColumns: Record<string, ColumnProperties>,
renderMode: RenderMode,
widgetId?: string,
): StickyType | undefined => {
if (renderMode === RenderModes.PAGE && widgetId) {
const localTableColumnOrder = getColumnOrderByWidgetIdFromLS(widgetId);
if (localTableColumnOrder) {
const { leftOrder, rightOrder } = localTableColumnOrder;
if (leftOrder.indexOf(columnId) > -1) {
return StickyType.LEFT;
} else if (rightOrder.indexOf(columnId) > -1) {
return StickyType.RIGHT;
} else {
return StickyType.NONE;
}
} else {
return get(primaryColumns, `${columnId}`).sticky;
}
}
if (renderMode === RenderModes.CANVAS) {
return get(primaryColumns, `${columnId}`).sticky;
}
};
export const updateAndSyncTableLocalColumnOrders = (
columnName: string,
leftOrder: string[],