From fefeb9b0ac3b4de15adebab194568a9513e64a4e Mon Sep 17 00:00:00 2001 From: Keyur Paralkar Date: Fri, 14 Oct 2022 18:19:58 +0530 Subject: [PATCH] fix: updated logic to retain the files on page change (#17389) * fix: updated logic to retain the files on page change * fix: update test case title * fix: addressed review comments * fix: cypress failure related to filepicker spec Co-authored-by: Ashit Rath --- .../FilePickerV2_Widget_reset_spec.js | 9 ++- .../Widgets/Filepicker/FilePickerV2_spec.js | 20 ++++- .../FilePickerWidgetV2/widget/index.tsx | 74 +++++++++++++++---- 3 files changed, 81 insertions(+), 22 deletions(-) diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Filepicker/FilePickerV2_Widget_reset_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Filepicker/FilePickerV2_Widget_reset_spec.js index a83a8d5f29..fa82cdd7e9 100644 --- a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Filepicker/FilePickerV2_Widget_reset_spec.js +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Filepicker/FilePickerV2_Widget_reset_spec.js @@ -3,12 +3,12 @@ const dsl = require("../../../../../fixtures/filePickerV2_reset_check_dsl.json") const Layoutpage = require("../../../../../locators/Layout.json"); const widgetsPage = require("../../../../../locators/Widgets.json"); -describe("Checkbox Widget Functionality", function() { +describe("File Picker Widget V2 Functionality", function() { before(() => { cy.addDsl(dsl); }); - it("Check if the uploaded data reset when tab switch in the TabsWidget", () => { + it("Check if the uploaded data does not reset when tab switch in the TabsWidget", () => { cy.get(widgetsPage.filepickerwidgetv2).should("contain", "Select Files"); cy.get(widgetsPage.filepickerwidgetv2).click(); cy.get(commonlocators.filePickerInput) @@ -31,6 +31,9 @@ describe("Checkbox Widget Functionality", function() { cy.get(Layoutpage.tabWidget) .contains("Tab 1") .should("have.class", "is-selected"); - cy.get(widgetsPage.filepickerwidgetv2).should("contain", "Select Files"); + cy.get(widgetsPage.filepickerwidgetv2).should( + "contain", + "1 files selected", + ); }); }); diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Filepicker/FilePickerV2_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Filepicker/FilePickerV2_spec.js index 0d0a8ef063..4fdd39457b 100644 --- a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Filepicker/FilePickerV2_spec.js +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Filepicker/FilePickerV2_spec.js @@ -29,18 +29,30 @@ describe("File picker widget v2", () => { cy.get(".t--widget-textwidget").should("contain", "true"); }); - it("Check if the uploaded data reset when back from query page", () => { + it("Check if the uploaded data does not reset when back from query page", () => { cy.openPropertyPane("textwidget"); cy.updateCodeInput( ".t--property-control-text", `{{FilePicker1.files[0].name}}`, ); - cy.createAndFillApi("{{FilePicker1.files[0]}}", ""); + cy.createAndFillApi("https://mock-api.appsmith.com/users", ""); + cy.updateCodeInput( + "[class*='t--actionConfiguration']", + "{{FilePicker1.files}}", + ); + cy.wait(1000); + cy.validateEvaluatedValue("testFile.mov"); + cy.get(".t--more-action-menu") .first() .click({ force: true }); + + // Go back to widgets page cy.get(explorer.widgetSwitchId).click(); - cy.get(widgetsPage.filepickerwidgetv2).should("contain", "Select Files"); - cy.get(`.t--widget-textwidget`).should("contain", ""); + cy.get(widgetsPage.filepickerwidgetv2).should( + "contain", + "1 files selected", + ); + cy.get(".t--widget-textwidget").should("contain", "testFile.mov"); }); }); diff --git a/app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx b/app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx index b1af48b382..73455534b4 100644 --- a/app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx +++ b/app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx @@ -22,6 +22,8 @@ import UpIcon from "assets/icons/ads/up-arrow.svg"; import CloseIcon from "assets/icons/ads/cross.svg"; import { Colors } from "constants/Colors"; import Papa from "papaparse"; +import { klona } from "klona"; +import { UppyFile } from "@uppy/utils"; const CSV_ARRAY_LABEL = "Array (CSVs only)"; const CSV_FILE_TYPE_REGEX = /.+(\/csv)$/; @@ -611,22 +613,36 @@ class FilePickerWidget extends BaseWidget< * Uppy provides an argument called reason. It helps us to distinguish on which event the file-removed event was called. * Refer to the following issue to know about reason prop: https://github.com/transloadit/uppy/pull/2323 */ - let updatedFiles = []; if (reason === "removed-by-user") { - updatedFiles = this.props.selectedFiles - ? this.props.selectedFiles.filter((dslFile) => { - return file.id !== dslFile.id; - }) - : []; - } else if (reason === "cancel-all") { - updatedFiles = []; + const fileCount = this.props.selectedFiles?.length || 0; + + /** + * Once the file is removed we update the selectedFiles + * with the current files present in the uppy's internal state + */ + const updatedFiles = this.state.uppy + .getFiles() + .map((currentFile: UppyFile, index: number) => ({ + type: currentFile.type, + id: currentFile.id, + data: currentFile.data, + name: currentFile.meta + ? currentFile.meta.name + : `File-${index + fileCount}`, + size: currentFile.size, + dataFormat: this.props.fileDataType, + })); + this.props.updateWidgetMetaProperty( + "selectedFiles", + updatedFiles ?? [], + ); } - this.props.updateWidgetMetaProperty("selectedFiles", updatedFiles); }); this.state.uppy.on("files-added", (files: any[]) => { - const dslFiles = this.props.selectedFiles - ? [...this.props.selectedFiles] + // Deep cloning the selectedFiles + const selectedFiles = this.props.selectedFiles + ? klona(this.props.selectedFiles) : []; const fileCount = this.props.selectedFiles?.length || 0; @@ -650,6 +666,7 @@ class FilePickerWidget extends BaseWidget< file.type, this.props.fileDataType, ), + meta: file.meta, name: file.meta ? file.meta.name : `File-${index + fileCount}`, size: file.size, dataFormat: this.props.fileDataType, @@ -662,6 +679,7 @@ class FilePickerWidget extends BaseWidget< type: file.type, id: file.id, data: data, + meta: file.meta, name: file.meta ? file.meta.name : `File-${index + fileCount}`, size: file.size, dataFormat: this.props.fileDataType, @@ -676,10 +694,17 @@ class FilePickerWidget extends BaseWidget< this.props.updateWidgetMetaProperty("isDirty", true); } - this.props.updateWidgetMetaProperty( - "selectedFiles", - dslFiles.concat(files), - ); + if (selectedFiles.length !== 0) { + files.forEach((fileItem: any) => { + if (!fileItem?.meta?.isInitializing) { + selectedFiles.push(fileItem); + } + }); + this.props.updateWidgetMetaProperty("selectedFiles", selectedFiles); + } else { + // update with newly added files when the selectedFiles is empty. + this.props.updateWidgetMetaProperty("selectedFiles", [...files]); + } }); }); @@ -741,11 +766,30 @@ class FilePickerWidget extends BaseWidget< }); } + initializeSelectedFiles() { + /** + * Since on unMount the uppy instance closes and it's internal state is lost along with the files present in it. + * Below we add the files again to the uppy instance so that the files are retained. + */ + this.props.selectedFiles?.forEach((fileItem: any) => { + this.state.uppy.addFile({ + name: fileItem.name, + type: fileItem.type, + data: new Blob([fileItem.data]), + meta: { + // Adding this flag to distinguish a file in the files-added event + isInitializing: true, + }, + }); + }); + } + componentDidMount() { super.componentDidMount(); try { this.initializeUppyEventListeners(); + this.initializeSelectedFiles(); } catch (e) { log.debug("Error in initializing uppy"); }