From 0dc54668bced4ffb66cd6622547c657516a8832f Mon Sep 17 00:00:00 2001 From: balajisoundar Date: Tue, 13 Feb 2024 11:54:49 +0530 Subject: [PATCH] fix: [Custom widget] update default dynamic height value (#31078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Add migration to set default value of custom widget dynamic height. #### PR fixes following issue(s) Fixes https://github.com/appsmithorg/appsmith/issues/31077 > if no issue exists, please create an issue and ask the maintainers about this first > > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change > Please delete options that are not relevant. - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Breaking change (fix or feature that would cause existing functionality to not work as expected) - Chore (housekeeping or task changes that don't impact user perception) - This change requires a documentation update > > > ## 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 - [ ] JUnit - [x] Jest - [ ] Cypress > > #### 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 - [x] 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/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#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 ## Summary by CodeRabbit - **New Features** - Added a migration feature for custom widgets to support dynamic height adjustments, ensuring widgets can now have a fixed height setting by default. - **Tests** - Introduced test suites for validating the migration of custom widget dynamic height settings. --- app/client/packages/dsl/src/migrate/index.ts | 8 ++- ...88-migrate-custom-widget-dynamic-height.ts | 10 ++++ .../src/migrate/tests/CustomWidget.test.ts | 53 +++++++++++++++++++ .../src/migrate/tests/DSLMigration.test.ts | 10 ++++ 4 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 app/client/packages/dsl/src/migrate/migrations/088-migrate-custom-widget-dynamic-height.ts create mode 100644 app/client/packages/dsl/src/migrate/tests/CustomWidget.test.ts diff --git a/app/client/packages/dsl/src/migrate/index.ts b/app/client/packages/dsl/src/migrate/index.ts index 434a375f84..cdb810ee7b 100644 --- a/app/client/packages/dsl/src/migrate/index.ts +++ b/app/client/packages/dsl/src/migrate/index.ts @@ -89,9 +89,10 @@ import { migrateSelectWidgetAddSourceDataPropertyPathList } from "./migrations/0 import { migrateDefaultValuesForCustomEChart } from "./migrations/085-migrate-default-values-for-custom-echart"; import { migrateTableServerSideFiltering } from "./migrations/086-migrate-table-server-side-filtering"; import { migrateChartwidgetCustomEchartConfig } from "./migrations/087-migrate-chart-widget-customechartdata"; +import { migrateCustomWidgetDynamicHeight } from "./migrations/088-migrate-custom-widget-dynamic-height"; import type { DSLWidget } from "./types"; -export const LATEST_DSL_VERSION = 88; +export const LATEST_DSL_VERSION = 89; export const calculateDynamicHeight = () => { const DEFAULT_GRID_ROW_HEIGHT = 10; @@ -586,6 +587,11 @@ const migrateVersionedDSL = (currentDSL: DSLWidget, newPage = false) => { if (currentDSL.version === 87) { currentDSL = migrateChartwidgetCustomEchartConfig(currentDSL); + currentDSL.version = 88; + } + + if (currentDSL.version === 88) { + currentDSL = migrateCustomWidgetDynamicHeight(currentDSL); currentDSL.version = LATEST_DSL_VERSION; } diff --git a/app/client/packages/dsl/src/migrate/migrations/088-migrate-custom-widget-dynamic-height.ts b/app/client/packages/dsl/src/migrate/migrations/088-migrate-custom-widget-dynamic-height.ts new file mode 100644 index 0000000000..1a7c5ea775 --- /dev/null +++ b/app/client/packages/dsl/src/migrate/migrations/088-migrate-custom-widget-dynamic-height.ts @@ -0,0 +1,10 @@ +import type { DSLWidget, WidgetProps } from "../types"; +import { traverseDSLAndMigrate } from "../utils"; + +export const migrateCustomWidgetDynamicHeight = (currentDSL: DSLWidget) => { + return traverseDSLAndMigrate(currentDSL, (widget: WidgetProps) => { + if (widget.type === "CUSTOM_WIDGET" && !widget.dynamicHeight) { + widget.dynamicHeight = "FIXED"; + } + }); +}; diff --git a/app/client/packages/dsl/src/migrate/tests/CustomWidget.test.ts b/app/client/packages/dsl/src/migrate/tests/CustomWidget.test.ts new file mode 100644 index 0000000000..adcef19bb3 --- /dev/null +++ b/app/client/packages/dsl/src/migrate/tests/CustomWidget.test.ts @@ -0,0 +1,53 @@ +import { migrateCustomWidgetDynamicHeight } from "../migrations/088-migrate-custom-widget-dynamic-height"; +import type { DSLWidget, WidgetProps } from "../types"; + +const inputDSL: DSLWidget = { + widgetId: "", + widgetName: "canvas widget", + type: "CANVAS_WIDGET", + renderMode: "CANVAS", + version: 1, + parentColumnSpace: 1, + parentRowSpace: 1, + isLoading: false, + topRow: 0, + bottomRow: 0, + leftColumn: 0, + rightColumn: 0, + children: [ + { + widgetId: "", + widgetName: "chart widget", + type: "CUSTOM_WIDGET", + renderMode: "CANVAS", + version: 1, + parentColumnSpace: 1, + parentRowSpace: 1, + isLoading: false, + topRow: 0, + bottomRow: 0, + leftColumn: 0, + rightColumn: 0, + labelOrientation: "stagger", + allowScroll: true, + children: [], + }, + ], +}; + +describe("Migrate Custom widget dynamic height", () => { + it("test that dynamic height default value is set", () => { + let outputDSL = migrateCustomWidgetDynamicHeight(inputDSL); + let outputChartWidgetDSL = (outputDSL.children && + outputDSL.children[0]) as WidgetProps; + expect(outputChartWidgetDSL.dynamicHeight).toEqual("FIXED"); + + outputChartWidgetDSL.dynamicHeight = "autoheight"; + + outputDSL = migrateCustomWidgetDynamicHeight(inputDSL); + outputChartWidgetDSL = (outputDSL.children && + outputDSL.children[0]) as WidgetProps; + + expect(outputChartWidgetDSL.dynamicHeight).toEqual("autoheight"); + }); +}); diff --git a/app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts b/app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts index dcb790fcfc..9a925b68a7 100644 --- a/app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts +++ b/app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts @@ -88,6 +88,7 @@ import * as m84 from "../migrations/084-migrate-select-widget-add-source-data-pr import * as m85 from "../migrations/085-migrate-default-values-for-custom-echart"; import * as m86 from "../migrations/086-migrate-table-server-side-filtering"; import * as m87 from "../migrations/087-migrate-chart-widget-customechartdata"; +import * as m88 from "../migrations/088-migrate-custom-widget-dynamic-height"; interface Migration { functionLookup: { @@ -910,6 +911,15 @@ const migrations: Migration[] = [ ], version: 87, }, + { + functionLookup: [ + { + moduleObj: m88, + functionName: "migrateCustomWidgetDynamicHeight", + }, + ], + version: 88, + }, ]; const mockFnObj: Record = {};