From 7a501ecd9c4766b0d029bcb63e335541c03951f6 Mon Sep 17 00:00:00 2001 From: rahulramesha <71900764+rahulramesha@users.noreply.github.com> Date: Fri, 6 May 2022 19:41:09 +0530 Subject: [PATCH] fix: stopping list widget from copying onto another list widget (#13621) * fix for stopping list widget from copying onto itself * remove duplicate comment --- .../WidgetCopyPaste/WidgetCopyPaste_spec.js | 24 ++++ app/client/src/sagas/WidgetOperationSagas.tsx | 8 +- .../src/sagas/WidgetOperationUtils.test.ts | 109 +++++++++++++++++- app/client/src/sagas/WidgetOperationUtils.ts | 32 +++-- 4 files changed, 162 insertions(+), 11 deletions(-) diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/WidgetCopyPaste/WidgetCopyPaste_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/WidgetCopyPaste/WidgetCopyPaste_spec.js index 5234504354..9c125ada39 100644 --- a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/WidgetCopyPaste/WidgetCopyPaste_spec.js +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/WidgetCopyPaste/WidgetCopyPaste_spec.js @@ -105,4 +105,28 @@ describe("Widget Copy paste", function() { .find(widgetsPage.chartWidget) .should("have.length", 2); }); + + it("should not be able to paste list widget inside another list widget", function() { + //clean up + cy.get(`#div-selection-0`).click({ + force: true, + }); + cy.get("body").type(`{${modifierKey}}{a}`); + cy.get("body").type("{del}"); + + //add list widget + cy.dragAndDropToCanvas("listwidget", { x: 300, y: 700 }); + cy.get(`div[data-testid='t--selected']`).should("have.length", 1); + + //copy + cy.get("body").type(`{${modifierKey}}{c}`); + + //paste + cy.get("body").type(`{${modifierKey}}{v}`); + cy.get(widgetsPage.listWidget).should("have.length", 2); + cy.get(widgetsPage.listWidget) + .eq(0) + .find(widgetsPage.listWidget) + .should("have.length", 0); + }); }); diff --git a/app/client/src/sagas/WidgetOperationSagas.tsx b/app/client/src/sagas/WidgetOperationSagas.tsx index e906b5bbab..7d3cac59fa 100644 --- a/app/client/src/sagas/WidgetOperationSagas.tsx +++ b/app/client/src/sagas/WidgetOperationSagas.tsx @@ -112,6 +112,7 @@ import { getWidgetsFromIds, getDefaultCanvas, isDropTarget, + checkIfPastingListWidgetOntoItself, getValueFromTree, } from "./WidgetOperationUtils"; import { getSelectedWidgets } from "selectors/ui"; @@ -875,7 +876,12 @@ const getNewPositions = function*( if ( !( selectedWidgets.length === 1 && - isDropTarget(selectedWidgets[0].type, true) + isDropTarget(selectedWidgets[0].type, true) && + !checkIfPastingListWidgetOntoItself( + canvasWidgets, + selectedWidgets[0], + copiedWidgetGroups, + ) ) && selectedWidgets.length > 0 ) { diff --git a/app/client/src/sagas/WidgetOperationUtils.test.ts b/app/client/src/sagas/WidgetOperationUtils.test.ts index fff2de7936..996302887d 100644 --- a/app/client/src/sagas/WidgetOperationUtils.test.ts +++ b/app/client/src/sagas/WidgetOperationUtils.test.ts @@ -1,4 +1,5 @@ import { OccupiedSpace } from "constants/CanvasEditorConstants"; +import { RenderModes } from "constants/WidgetConstants"; import { get } from "lodash"; import { WidgetProps } from "widgets/BaseWidget"; import { FlattenedWidgetProps } from "widgets/constants"; @@ -6,7 +7,8 @@ import { handleIfParentIsListWidgetWhilePasting, handleSpecificCasesWhilePasting, doesTriggerPathsContainPropertyPath, - checkIfPastingIntoListWidget, + getSelectedWidgetIfPastingIntoListWidget, + checkIfPastingListWidgetOntoItself, updateListWidgetPropertiesOnChildDelete, purgeOrphanedDynamicPaths, getBoundariesFromSelectedWidgets, @@ -401,8 +403,8 @@ describe("WidgetOperationSaga", () => { ); }); - it("should returns widgets after executing checkIfPastingIntoListWidget", async () => { - const result = checkIfPastingIntoListWidget( + it("should returns widgets after executing getSelectedWidgetIfPastingIntoListWidget", async () => { + const result = getSelectedWidgetIfPastingIntoListWidget( { list2: { widgetId: "list2", @@ -979,6 +981,107 @@ describe("WidgetOperationSaga", () => { }, ]); }); + it("should test checkIfPastingListWidgetOntoItself", () => { + const canvasWidgets = { + list2: { + widgetId: "list2", + type: "LIST_WIDGET", + widgetName: "List2", + parentId: "0", + renderMode: RenderModes.CANVAS, + parentColumnSpace: 2, + parentRowSpace: 3, + leftColumn: 2, + rightColumn: 3, + topRow: 1, + bottomRow: 3, + isLoading: false, + listData: [], + version: 16, + disablePropertyPane: false, + template: {}, + children: [], + }, + }; + const selectedWidget = { + widgetId: "list2", + type: "LIST_WIDGET", + widgetName: "List2", + parentId: "0", + renderMode: RenderModes.CANVAS, + parentColumnSpace: 2, + parentRowSpace: 3, + leftColumn: 2, + rightColumn: 3, + topRow: 1, + bottomRow: 3, + isLoading: false, + listData: [], + version: 16, + disablePropertyPane: false, + template: {}, + children: [], + }; + //if copying list widget onto list widget + expect( + checkIfPastingListWidgetOntoItself(canvasWidgets, selectedWidget, [ + { + widgetId: "list2", + parentId: "0", + list: [ + { + widgetId: "list2", + type: "LIST_WIDGET", + widgetName: "List2", + parentId: "0", + renderMode: "CANVAS", + parentColumnSpace: 2, + parentRowSpace: 3, + leftColumn: 2, + rightColumn: 3, + topRow: 1, + bottomRow: 3, + isLoading: false, + listData: [], + version: 16, + disablePropertyPane: false, + template: {}, + }, + ], + }, + ]), + ).toBe(true); + + //if copying container widget onto list widget + expect( + checkIfPastingListWidgetOntoItself(canvasWidgets, selectedWidget, [ + { + widgetId: "container", + parentId: "0", + list: [ + { + widgetId: "container", + type: "CONTAINER_WIDGET", + widgetName: "container", + parentId: "0", + renderMode: "CANVAS", + parentColumnSpace: 2, + parentRowSpace: 3, + leftColumn: 2, + rightColumn: 3, + topRow: 1, + bottomRow: 3, + isLoading: false, + listData: [], + version: 16, + disablePropertyPane: false, + template: {}, + }, + ], + }, + ]), + ).toBe(false); + }); }); describe("getValueFromTree - ", () => { diff --git a/app/client/src/sagas/WidgetOperationUtils.ts b/app/client/src/sagas/WidgetOperationUtils.ts index 44fbcebac5..182103dcbb 100644 --- a/app/client/src/sagas/WidgetOperationUtils.ts +++ b/app/client/src/sagas/WidgetOperationUtils.ts @@ -350,14 +350,10 @@ export const isCopiedModalWidget = function( return false; }; -export const checkIfPastingIntoListWidget = function( +export const getSelectedWidgetIfPastingIntoListWidget = function( canvasWidgets: CanvasWidgetsReduxState, selectedWidget: FlattenedWidgetProps | undefined, - copiedWidgets: { - widgetId: string; - parentId: string; - list: WidgetProps[]; - }[], + copiedWidgets: CopiedWidgetGroup[], ) { // when list widget is selected, if the user is pasting, we want it to be pasted in the template // which is first children of list widget @@ -387,6 +383,28 @@ export const checkIfPastingIntoListWidget = function( return selectedWidget; }; +/** + * checks if the selectedWidget is a list widget and copiedWidgetsGroups contains list widget + * + * @param canvasWidgets + * @param selectedWidget + * @param copiedWidgetsGroups + * @returns + */ +export const checkIfPastingListWidgetOntoItself = function( + canvasWidgets: CanvasWidgetsReduxState, + selectedWidget: FlattenedWidgetProps | undefined, + copiedWidgetsGroups: CopiedWidgetGroup[], +) { + return ( + getSelectedWidgetIfPastingIntoListWidget( + canvasWidgets, + selectedWidget, + copiedWidgetsGroups, + )?.type === "LIST_WIDGET" + ); +}; + /** * get top, left, right, bottom most widgets and totalWidth from copied groups when pasting * @@ -474,7 +492,7 @@ export const getSelectedWidgetWhenPasting = function*() { getFocusedWidget, ); - selectedWidget = checkIfPastingIntoListWidget( + selectedWidget = getSelectedWidgetIfPastingIntoListWidget( canvasWidgets, selectedWidget || focusedWidget, copiedWidgetGroups,