diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/DisplayWidgets/Table_EmptyRow_Color_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/DisplayWidgets/Table_EmptyRow_Color_spec.js new file mode 100644 index 0000000000..fa8a8f8652 --- /dev/null +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/DisplayWidgets/Table_EmptyRow_Color_spec.js @@ -0,0 +1,52 @@ +const widgetsPage = require("../../../../locators/Widgets.json"); +const dsl = require("../../../../fixtures/tableNewDsl.json"); +const commonlocators = require("../../../../locators/commonlocators.json"); + +describe("Table Widget empty row color validation", function() { + before(() => { + cy.addDsl(dsl); + }); + + it("1. Validate cell background of columns", function() { + // Open property pane + cy.openPropertyPane("tablewidget"); + // give general color to all table row + cy.selectColor("cellbackgroundcolor", -17); + + cy.editColumn("id"); + // Click on cell background color + cy.selectColor("cellbackground", -27); + cy.wait("@updateLayout"); + cy.get(commonlocators.editPropBackButton).click({ force: true }); + + cy.editColumn("email"); + cy.selectColor("cellbackground", -33); + cy.wait("@updateLayout"); + cy.get(commonlocators.editPropBackButton).click({ force: true }); + + // Verify the cell background color of first column + cy.readTabledataValidateCSS( + "1", + "0", + "background-color", + "rgb(99, 102, 241)", + ); + // Verify the cell background color of second column + cy.readTabledataValidateCSS( + "1", + "1", + "background-color", + "rgb(30, 58, 138)", + ); + }); + it("2. Validate empty row background", function() { + // first cell of first row should be transparent + cy.get( + ".t--widget-tablewidget .tbody div[data-cy='empty-row-0-cell-0']", + ).should("have.css", "background-color", "rgb(99, 102, 241)"); + // second cell of first row should be transparent + cy.get( + ".t--widget-tablewidget .tbody div[data-cy='empty-row-0-cell-1']", + ).should("have.css", "background-color", "rgb(30, 58, 138)"); + }); +}); diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/GitDiscardChange/DiscardChanges_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/GitDiscardChange/DiscardChanges_spec.js new file mode 100644 index 0000000000..b5cae1240c --- /dev/null +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/GitDiscardChange/DiscardChanges_spec.js @@ -0,0 +1,228 @@ +const datasource = require("../../../../locators/DatasourcesEditor.json"); +const queryLocators = require("../../../../locators/QueryEditor.json"); +const dynamicInputLocators = require("../../../../locators/DynamicInput.json"); +const explorer = require("../../../../locators/explorerlocators.json"); + +describe("Git discard changes:", function() { + let datasourceName; + let repoName; + const query1 = "get_users"; + const query2 = "get_allusers"; + const jsObject = "JSObject1"; + const page2 = "Page_2"; + const page3 = "Page_3"; + + it("1. Create an app with Query1 and JSObject1, connect it to git", () => { + // Create new postgres datasource + cy.NavigateToDatasourceEditor(); + cy.get(datasource.PostgreSQL).click(); + + cy.getPluginFormsAndCreateDatasource(); + + cy.fillPostgresDatasourceForm(); + + cy.testSaveDatasource(); + + cy.get("@createDatasource").then((httpResponse) => { + datasourceName = httpResponse.response.body.data.name; + + cy.get(datasource.datasourceCard) + .contains(datasourceName) + .scrollIntoView() + .should("be.visible") + .closest(datasource.datasourceCard) + .within(() => { + cy.get(datasource.createQuerty).click(); + }); + }); + // Create new postgres query + cy.get(queryLocators.queryNameField).type(`${query1}`); + cy.get(queryLocators.switch) + .last() + .click({ force: true }); + cy.get(queryLocators.templateMenu).click(); + cy.get(queryLocators.query).click({ force: true }); + cy.get(".CodeMirror textarea") + .first() + .focus() + .type("SELECT * FROM users ORDER BY id LIMIT 10;", { + force: true, + parseSpecialCharSequences: false, + }); + cy.WaitAutoSave(); + cy.runQuery(); + + cy.CheckAndUnfoldEntityItem("PAGES"); + cy.wait(1000); + cy.get(".t--entity-item:contains(Page1)") + .first() + .click(); + cy.wait("@getPage"); + // bind input widget to postgres query on page1 + cy.get(explorer.addWidget).click(); + cy.dragAndDropToCanvas("inputwidgetv2", { x: 300, y: 300 }); + cy.get(".t--widget-inputwidgetv2").should("exist"); + cy.get(dynamicInputLocators.input) + .eq(1) + .click({ force: true }) + .type(`{{${query1}.data[0].name}}`, { + parseSpecialCharSequences: false, + }); + cy.wait(2000); + cy.CheckAndUnfoldEntityItem("PAGES"); + cy.Createpage(page2); + cy.wait(1000); + cy.get(`.t--entity-item:contains(${page2})`) + .first() + .click(); + cy.wait("@getPage"); + cy.createJSObject('return "Success";'); + cy.get(explorer.addWidget).click(); + // bind input widget to JSObject response on page2 + cy.dragAndDropToCanvas("inputwidgetv2", { x: 300, y: 300 }); + cy.get(".t--widget-inputwidgetv2").should("exist"); + cy.get(dynamicInputLocators.input) + .eq(1) + .click({ force: true }) + .type("{{JSObject1.myFun1()}}", { parseSpecialCharSequences: false }); + cy.get("#switcher--explorer").click({ force: true }); + // connect app to git + cy.generateUUID().then((uid) => { + repoName = uid; + + cy.createTestGithubRepo(repoName); + cy.connectToGitRepo(repoName); + }); + }); + + it("2. Add new datasource query, discard changes, verify query is deleted", () => { + cy.get(`.t--entity-item:contains("Page1")`) + .first() + .click(); + cy.wait("@getPage"); + // create new postgres query + cy.NavigateToQueryEditor(); + cy.NavigateToActiveTab(); + cy.get(datasource.datasourceCard) + .contains(datasourceName) + .scrollIntoView() + .should("be.visible") + .closest(datasource.datasourceCard) + .within(() => { + cy.get(datasource.createQuerty).click(); + }); + cy.get(queryLocators.queryNameField).type(`${query2}`); + cy.get(queryLocators.switch) + .last() + .click({ force: true }); + cy.get(queryLocators.templateMenu).click(); + cy.get(queryLocators.query).click({ force: true }); + cy.get(".CodeMirror textarea") + .first() + .focus() + .type("SELECT * FROM users;", { + force: true, + parseSpecialCharSequences: false, + }); + cy.WaitAutoSave(); + cy.runQuery(); + // navoigate to Page1 + cy.get(`.t--entity-item:contains(Page1)`) + .first() + .click(); + cy.wait("@getPage"); + // discard changes + cy.gitDiscardChanges(); + cy.CheckAndUnfoldEntityItem("QUERIES/JS"); + // verify query2 is not present + cy.get(`.t--entity-name:contains(${query2})`).should("not.exist"); + }); + + it("3. Add new JSObject , discard changes verify JSObject is deleted", () => { + cy.createJSObject('return "Success";'); + cy.CheckAndUnfoldEntityItem("QUERIES/JS"); + // verify jsObject is not duplicated + cy.get(`.t--entity-name:contains(${jsObject})`).should("have.length", 1); + cy.gitDiscardChanges(); + cy.CheckAndUnfoldEntityItem("QUERIES/JS"); + // verify jsObject2 is deleted after discarding changes + cy.get(`.t--entity-name:contains(${jsObject})`).should("not.exist"); + }); + + it("4. Delete page2 and trigger discard flow, page2 should be available again", () => { + cy.Deletepage(page2); + // verify page is deleted + cy.CheckAndUnfoldEntityItem("PAGES"); + cy.get(`.t--entity-name:contains(${page2})`).should("not.exist"); + cy.gitDiscardChanges(); + // verify page2 is recovered back + cy.get(`.t--entity-name:contains(${page2})`).should("be.visible"); + cy.get(`.t--entity-item:contains(${page2})`) + .first() + .click(); + cy.wait("@getPage"); + // verify data binding on page2 + cy.get(".bp3-input").should("have.value", "Success"); + }); + + it("5. Delete Query1 and trigger discard flow, Query1 will be recovered", () => { + // navigate to Page1 + cy.CheckAndUnfoldEntityItem("PAGES"); + cy.get(`.t--entity-item:contains("Page1")`) + .first() + .click(); + cy.wait("@getPage"); + // delete query1 + cy.deleteQueryOrJS(query1); + // verify Query1 is deleted + cy.get(`.t--entity-name:contains(${query1})`).should("not.exist"); + // discard changes + cy.gitDiscardChanges(); + //verify query1 is recovered + cy.get(`.t--entity-name:contains(${query1})`).should("be.visible"); + + cy.get(".bp3-input").should("have.value", "Test user 7"); + }); + + it("6. Delete JSObject1 and trigger discard flow, JSObject1 should be active again", () => { + // navigate to page2 + cy.CheckAndUnfoldEntityItem("PAGES"); + cy.get(`.t--entity-item:contains(${page2})`) + .first() + .click(); + cy.wait("@getPage"); + // delete jsObject1 + cy.CheckAndUnfoldEntityItem("QUERIES/JS"); + cy.get(`.t--entity-item:contains(${jsObject})`).within(() => { + cy.get(".t--context-menu").click({ force: true }); + }); + cy.selectAction("Delete"); + cy.selectAction("Are you sure?"); + cy.get(`.t--entity-name:contains(${jsObject})`).should("not.exist"); + // discard changes + cy.gitDiscardChanges(); + //verify JSObject is recovered + cy.get(`.t--entity-name:contains(${jsObject})`).should("be.visible"); + cy.get(".bp3-input").should("have.value", "Success"); + }); + + it("7. Add new page i.e page3, go to page2 & discard changes, verify page3 is removed", () => { + // create new page page3 and move to page1 + cy.Createpage(page3); + cy.get(`.t--entity-item:contains(${page2})`) + .first() + .click(); + // discard changes + cy.gitDiscardChanges(); + // verify page3 is removed + cy.CheckAndUnfoldEntityItem("PAGES"); + cy.get(`.t--entity-name:contains("${page3}")`).should("not.exist"); + }); + + it("8. Add new page i.e page3, discard changes should give error resource not found", () => { + cy.Createpage(page3); + cy.gitDiscardChanges(false); + cy.go("back"); + cy.reload(); + }); +}); diff --git a/app/client/cypress/locators/gitSyncLocators.js b/app/client/cypress/locators/gitSyncLocators.js index 9f0b480c63..91964ee618 100644 --- a/app/client/cypress/locators/gitSyncLocators.js +++ b/app/client/cypress/locators/gitSyncLocators.js @@ -51,4 +51,5 @@ export default { gitPullCount: ".t--bottom-bar-pull .count", gitConnectionContainer: "[data-test=t--git-connection-container]", gitRemoteURLContainer: "[data-test=t--remote-url-container]", + discardChanges: ".t--discard-button", }; diff --git a/app/client/cypress/support/gitSync.js b/app/client/cypress/support/gitSync.js index 95f605264f..c2772250fa 100644 --- a/app/client/cypress/support/gitSync.js +++ b/app/client/cypress/support/gitSync.js @@ -363,3 +363,37 @@ Cypress.Commands.add( }); }, ); + +Cypress.Commands.add("gitDiscardChanges", (assertResourceFound = true) => { + cy.get(gitSyncLocators.bottomBarCommitButton).click(); + //cy.intercept("GET", "/api/v1/git/status/*").as("gitStatus"); + // cy.wait("@gitStatus").should( + // "have.nested.property", + // "response.body.responseMeta.status", + // 200, + // ); + cy.get(gitSyncLocators.discardChanges) + .children() + .should("have.text", "Discard changes"); + + cy.get(gitSyncLocators.discardChanges).click(); + cy.contains(Cypress.env("MESSAGES").DISCARD_CHANGES_WARNING()); + + cy.get(gitSyncLocators.discardChanges) + .children() + .should("have.text", "Are you sure?"); + cy.get(gitSyncLocators.discardChanges).click(); + cy.contains(Cypress.env("MESSAGES").DISCARDING_AND_PULLING_CHANGES()); + if (assertResourceFound) { + cy.wait("@applications").should( + "have.nested.property", + "response.body.responseMeta.status", + 200, + ); + cy.validateToastMessage("Discarded changes successfully."); + } else { + cy.get(".bold-text").should(($x) => { + expect($x).contain("Page not found"); + }); + } +}); diff --git a/app/client/cypress/support/widgetCommands.js b/app/client/cypress/support/widgetCommands.js index 532c0a3565..8b322c0d84 100644 --- a/app/client/cypress/support/widgetCommands.js +++ b/app/client/cypress/support/widgetCommands.js @@ -424,7 +424,7 @@ Cypress.Commands.add("updateCodeInput", ($selector, value) => { }); }); -Cypress.Commands.add("selectColor", (GivenProperty) => { +Cypress.Commands.add("selectColor", (GivenProperty, colorOffset = -15) => { // Property pane of the widget is opened, and click given property. cy.get( ".t--property-control-" + GivenProperty + " .bp3-input-group input", @@ -433,7 +433,7 @@ Cypress.Commands.add("selectColor", (GivenProperty) => { }); cy.get(widgetsPage.colorPickerV2Color) - .eq(-15) + .eq(colorOffset) .then(($elem) => { cy.get($elem).click({ force: true }); }); @@ -1109,7 +1109,16 @@ Cypress.Commands.add("clearPropertyValue", (value) => { // eslint-disable-next-line cypress/no-unnecessary-waiting cy.wait(1000); }); - +Cypress.Commands.add("deleteQueryOrJS", (Action) => { + cy.CheckAndUnfoldEntityItem("QUERIES/JS"); + cy.get(`.t--entity-item:contains(${Action})`).within(() => { + cy.get(".t--context-menu").click({ force: true }); + }); + cy.selectAction("Delete"); + cy.selectAction("Are you sure?"); + cy.wait("@deleteAction"); + cy.get("@deleteAction").should("have.property", "status", 200); +}); Cypress.Commands.add( "validateNSelectDropdown", (ddTitle, currentValue, newValue) => { diff --git a/app/client/package.json b/app/client/package.json index fcbf51b9e7..c2a34709c5 100644 --- a/app/client/package.json +++ b/app/client/package.json @@ -83,6 +83,7 @@ "mammoth": "^1.4.19", "marked": "^2.0.0", "memoize-one": "^5.2.1", + "micro-memoize": "^4.0.10", "moment": "^2.24.0", "moment-timezone": "^0.5.27", "nanoid": "^2.0.4", diff --git a/app/client/src/ce/pages/AdminSettings/LeftPane.tsx b/app/client/src/ce/pages/AdminSettings/LeftPane.tsx index dae1d94640..9446fea0d2 100644 --- a/app/client/src/ce/pages/AdminSettings/LeftPane.tsx +++ b/app/client/src/ce/pages/AdminSettings/LeftPane.tsx @@ -55,7 +55,7 @@ export const StyledLink = styled(Link)<{ $active: boolean }>` } `; -export function useSettingsCategory() { +export function getSettingsCategory() { return Array.from(AdminConfig.categories); } @@ -109,7 +109,7 @@ export function Categories({ } export default function LeftPane() { - const categories = useSettingsCategory(); + const categories = getSettingsCategory(); const { category, subCategory } = useParams() as any; return ( diff --git a/app/client/src/ce/pages/AdminSettings/Main.tsx b/app/client/src/ce/pages/AdminSettings/Main.tsx index 2f3e59207a..b5591b667b 100644 --- a/app/client/src/ce/pages/AdminSettings/Main.tsx +++ b/app/client/src/ce/pages/AdminSettings/Main.tsx @@ -11,7 +11,7 @@ const Main = () => { const wrapperCategory = AdminConfig.wrapperCategories[subCategory ?? category]; - if (!!wrapperCategory && !!wrapperCategory.component) { + if (!!wrapperCategory?.component) { const { component: WrapperCategoryComponent } = wrapperCategory; return ; } else if ( diff --git a/app/client/src/ce/pages/AdminSettings/config/authentication/index.tsx b/app/client/src/ce/pages/AdminSettings/config/authentication/index.tsx index 6549643031..cab5a2c906 100644 --- a/app/client/src/ce/pages/AdminSettings/config/authentication/index.tsx +++ b/app/client/src/ce/pages/AdminSettings/config/authentication/index.tsx @@ -25,7 +25,7 @@ const { enableGoogleOAuth, } = getAppsmithConfigs(); -const Form_Auth: AdminConfigType = { +const FormAuth: AdminConfigType = { type: SettingCategories.FORM_AUTH, controlType: SettingTypes.GROUP, title: "Form Login", @@ -65,7 +65,7 @@ const Form_Auth: AdminConfigType = { ], }; -const Google_Auth: AdminConfigType = { +const GoogleAuth: AdminConfigType = { type: SettingCategories.GOOGLE_AUTH, controlType: SettingTypes.GROUP, title: "Google Authentication", @@ -111,7 +111,7 @@ const Google_Auth: AdminConfigType = { ], }; -const Github_Auth: AdminConfigType = { +const GithubAuth: AdminConfigType = { type: SettingCategories.GITHUB_AUTH, controlType: SettingTypes.GROUP, title: "Github Authentication", @@ -149,7 +149,7 @@ const Github_Auth: AdminConfigType = { ], }; -export const Form_Auth_Callout: AuthMethodType = { +export const FormAuthCallout: AuthMethodType = { id: "APPSMITH_FORM_LOGIN_AUTH", category: SettingCategories.FORM_AUTH, label: "Form Login", @@ -159,7 +159,7 @@ export const Form_Auth_Callout: AuthMethodType = { isConnected: !disableLoginForm, }; -export const Google_Auth_Callout: AuthMethodType = { +export const GoogleAuthCallout: AuthMethodType = { id: "APPSMITH_GOOGLE_AUTH", category: SettingCategories.GOOGLE_AUTH, label: "Google", @@ -170,7 +170,7 @@ export const Google_Auth_Callout: AuthMethodType = { isConnected: enableGoogleOAuth, }; -export const Github_Auth_Callout: AuthMethodType = { +export const GithubAuthCallout: AuthMethodType = { id: "APPSMITH_GITHUB_AUTH", category: SettingCategories.GITHUB_AUTH, label: "Github", @@ -181,7 +181,7 @@ export const Github_Auth_Callout: AuthMethodType = { isConnected: enableGithubOAuth, }; -export const Saml_Auth_Callout: AuthMethodType = { +export const SamlAuthCallout: AuthMethodType = { id: "APPSMITH_SAML_AUTH", category: "saml", label: "SAML 2.0", @@ -191,7 +191,7 @@ export const Saml_Auth_Callout: AuthMethodType = { type: "OTHER", }; -export const Oidc_Auth_Callout: AuthMethodType = { +export const OidcAuthCallout: AuthMethodType = { id: "APPSMITH_OIDC_AUTH", category: "oidc", label: "OIDC", @@ -202,11 +202,11 @@ export const Oidc_Auth_Callout: AuthMethodType = { }; const AuthMethods = [ - Oidc_Auth_Callout, - Saml_Auth_Callout, - Google_Auth_Callout, - Github_Auth_Callout, - Form_Auth_Callout, + OidcAuthCallout, + SamlAuthCallout, + GoogleAuthCallout, + GithubAuthCallout, + FormAuthCallout, ]; function AuthMain() { @@ -218,6 +218,6 @@ export const config: AdminConfigType = { controlType: SettingTypes.PAGE, title: "Authentication", canSave: false, - children: [Form_Auth, Google_Auth, Github_Auth], + children: [FormAuth, GoogleAuth, GithubAuth], component: AuthMain, }; diff --git a/app/client/src/components/editorComponents/form/fields/DropdownWrapper.tsx b/app/client/src/components/editorComponents/form/fields/DropdownWrapper.tsx index 704db3bfdc..2ff5d34a24 100644 --- a/app/client/src/components/editorComponents/form/fields/DropdownWrapper.tsx +++ b/app/client/src/components/editorComponents/form/fields/DropdownWrapper.tsx @@ -7,7 +7,8 @@ type DropdownWrapperProps = { value?: string; onChange?: (value?: string) => void; }; - options: Array<{ id: string; value: string; label: string }>; + options: Array<{ id: string; value: string; label?: string }>; + fillOptions?: boolean; }; function DropdownWrapper(props: DropdownWrapperProps) { @@ -28,6 +29,7 @@ function DropdownWrapper(props: DropdownWrapperProps) { return ( ; + options: Array<{ id: string; value: string; label?: string }>; size?: "large" | "small"; outline?: boolean; + fillOptions?: boolean; }; export function SelectField(props: SelectFieldProps) { return ( { const editorConfig = editorConfigs[action.config.pluginId]; const dependencyConfig = pluginDependencyConfig[action.config.pluginId]; @@ -173,15 +177,24 @@ export class DataTreeFactory { dependencyConfig, ); }); + const endActions = performance.now(); + + const startJsActions = performance.now(); + jsActions.forEach((js) => { dataTree[js.config.name] = generateDataTreeJSAction(js); }); + const endJsActions = performance.now(); + + const startWidgets = performance.now(); + Object.values(widgets).forEach((widget) => { dataTree[widget.widgetName] = generateDataTreeWidget( widget, widgetsMeta[widget.widgetId], ); }); + const endWidgets = performance.now(); dataTree.pageList = pageList; dataTree.appsmith = { @@ -192,6 +205,17 @@ export class DataTreeFactory { theme, } as DataTreeAppsmith; (dataTree.appsmith as DataTreeAppsmith).ENTITY_TYPE = ENTITY_TYPE.APPSMITH; + const end = performance.now(); + + const out = { + total: end - start, + widgets: endWidgets - startWidgets, + actions: endActions - startActions, + jsActions: endJsActions - startJsActions, + }; + + log.debug("### Create unevalTree timing", out); + return dataTree; } } diff --git a/app/client/src/entities/DataTree/dataTreeWidget.test.ts b/app/client/src/entities/DataTree/dataTreeWidget.test.ts index a4fb85e407..f935ebadc4 100644 --- a/app/client/src/entities/DataTree/dataTreeWidget.test.ts +++ b/app/client/src/entities/DataTree/dataTreeWidget.test.ts @@ -289,7 +289,6 @@ describe("generateDataTreeWidget", () => { privateWidgets: {}, deepObj: { level1: { - value: 10, metaValue: 10, }, }, diff --git a/app/client/src/entities/DataTree/dataTreeWidget.ts b/app/client/src/entities/DataTree/dataTreeWidget.ts index 93e24c86dc..4e0b31f20c 100644 --- a/app/client/src/entities/DataTree/dataTreeWidget.ts +++ b/app/client/src/entities/DataTree/dataTreeWidget.ts @@ -1,22 +1,28 @@ -import WidgetFactory from "utils/WidgetFactory"; import { getAllPathsFromPropertyConfig } from "entities/Widget/utils"; -import { getEntityDynamicBindingPathList } from "utils/DynamicBindingUtils"; import _ from "lodash"; - +import memoize from "micro-memoize"; import { FlattenedWidgetProps } from "reducers/entityReducers/canvasWidgetsReducer"; -import { setOverridingProperty } from "./utils"; +import { getEntityDynamicBindingPathList } from "utils/DynamicBindingUtils"; +import WidgetFactory from "utils/WidgetFactory"; import { - OverridingPropertyPaths, - PropertyOverrideDependency, - OverridingPropertyType, DataTreeWidget, ENTITY_TYPE, + OverridingPropertyPaths, + OverridingPropertyType, + PropertyOverrideDependency, } from "./dataTreeFactory"; +import { setOverridingProperty } from "./utils"; -export const generateDataTreeWidget = ( +// We are splitting generateDataTreeWidget into two parts to memoize better as the widget doesn't change very often. +// Widget changes only when dynamicBindingPathList changes. +// Only meta properties change very often, for example typing in an input or selecting a table row. +const generateDataTreeWidgetWithoutMeta = ( widget: FlattenedWidgetProps, - widgetMetaProps: Record = {}, -): DataTreeWidget => { +): { + dataTreeWidgetWithoutMetaProps: DataTreeWidget; + overridingMetaPropsMap: Record; + defaultMetaProps: Record; +} => { const derivedProps: any = {}; const blockedDerivedProps: Record = {}; const unInitializedDefaultProps: Record = {}; @@ -94,16 +100,6 @@ export const generateDataTreeWidget = ( }, ); - const overridingMetaProps: Record = {}; - - // overridingMetaProps has all meta property value either from metaReducer or default set by widget whose dependent property also has default property. - Object.entries(defaultMetaProps).forEach(([key, value]) => { - if (overridingMetaPropsMap[key]) { - overridingMetaProps[key] = - key in widgetMetaProps ? widgetMetaProps[key] : value; - } - }); - const { bindingPaths, reactivePaths, @@ -134,12 +130,12 @@ export const generateDataTreeWidget = ( * * Therefore spread is replaced with "merge" which merges objects recursively. */ - return _.merge( + const dataTreeWidgetWithoutMetaProps = _.merge( {}, widget, unInitializedDefaultProps, defaultMetaProps, - widgetMetaProps, + // widgetMetaProps, derivedProps, { defaultProps, @@ -149,7 +145,7 @@ export const generateDataTreeWidget = ( ...widget.logBlackList, ...blockedDerivedProps, }, - meta: _.merge(overridingMetaProps, widgetMetaProps), + // meta: _.merge(overridingMetaProps, widgetMetaProps), propertyOverrideDependency, overridingPropertyPaths, bindingPaths, @@ -162,4 +158,59 @@ export const generateDataTreeWidget = ( }, }, ); + return { + dataTreeWidgetWithoutMetaProps, + overridingMetaPropsMap, + defaultMetaProps, + }; +}; + +// @todo set the max size dynamically based on number of widgets. (widgets.length) +// Remove the debug statements in July 2022 +const generateDataTreeWidgetWithoutMetaMemoized = memoize( + generateDataTreeWidgetWithoutMeta, + { + maxSize: 1000, + // onCacheHit: (cache, options) => { + // console.log("####### cache was hit: ", cache.keys.length); + // }, + // onCacheAdd: (cache, options) => { + // console.log( + // "####### cache was missed ", + // cache.keys.length, + // cache.keys[0][0].widgetName, + // ); + // }, + }, +); + +export const generateDataTreeWidget = ( + widget: FlattenedWidgetProps, + widgetMetaProps: Record = {}, +) => { + const { + dataTreeWidgetWithoutMetaProps: dataTreeWidget, + defaultMetaProps, + overridingMetaPropsMap, + } = generateDataTreeWidgetWithoutMetaMemoized(widget); + const overridingMetaProps: Record = {}; + + // overridingMetaProps has all meta property value either from metaReducer or default set by widget whose dependent property also has default property. + Object.entries(defaultMetaProps).forEach(([key, value]) => { + if (overridingMetaPropsMap[key]) { + overridingMetaProps[key] = + key in widgetMetaProps ? widgetMetaProps[key] : value; + } + }); + + const meta = _.merge(overridingMetaProps, widgetMetaProps); + + Object.entries(widgetMetaProps).forEach(([key, value]) => { + // Since meta properties are always updated as a whole, we are replacing instead of merging. + // Merging mutates the memoized value, avoid merging meta values + dataTreeWidget[key] = value; + }); + + dataTreeWidget["meta"] = meta; + return dataTreeWidget; }; diff --git a/app/client/src/entities/Widget/utils.ts b/app/client/src/entities/Widget/utils.ts index f7696aaf32..b5b924341a 100644 --- a/app/client/src/entities/Widget/utils.ts +++ b/app/client/src/entities/Widget/utils.ts @@ -1,11 +1,12 @@ -import { WidgetProps } from "widgets/BaseWidget"; import { PropertyPaneConfig, ValidationConfig, } from "constants/PropertyControlConstants"; -import { get, isObject, isUndefined, omitBy } from "lodash"; -import { FlattenedWidgetProps } from "reducers/entityReducers/canvasWidgetsReducer"; import { EvaluationSubstitutionType } from "entities/DataTree/dataTreeFactory"; +import { get, isObject, isUndefined, omitBy } from "lodash"; +import memoize from "micro-memoize"; +import { FlattenedWidgetProps } from "reducers/entityReducers/canvasWidgetsReducer"; +import { WidgetProps } from "widgets/BaseWidget"; /** * @typedef {Object} Paths @@ -163,7 +164,7 @@ const childHasPanelConfig = ( return { reactivePaths, triggerPaths, validationPaths, bindingPaths }; }; -export const getAllPathsFromPropertyConfig = ( +const getAllPathsFromPropertyConfigWithoutMemo = ( widget: WidgetProps, widgetConfig: readonly PropertyPaneConfig[], defaultProperties: Record, @@ -276,6 +277,11 @@ export const getAllPathsFromPropertyConfig = ( return { reactivePaths, triggerPaths, validationPaths, bindingPaths }; }; +export const getAllPathsFromPropertyConfig = memoize( + getAllPathsFromPropertyConfigWithoutMemo, + { maxSize: 1000 }, +); + /** * this function gets the next available row for pasting widgets * NOTE: this function excludes modal widget when calculating next available row diff --git a/app/client/src/pages/Applications/index.tsx b/app/client/src/pages/Applications/index.tsx index 2cd13f0bb3..ad36911bae 100644 --- a/app/client/src/pages/Applications/index.tsx +++ b/app/client/src/pages/Applications/index.tsx @@ -357,9 +357,9 @@ function OrgMenuItem({ isFetchingApplications, org, selected }: any) { isFetchingApplications ? BlueprintClasses.SKELETON : "" } ellipsize={20} - href={`${window.location.pathname}#${org.organization.slug}`} + href={`${window.location.pathname}#${org.organization.id}`} icon="workspace" - key={org.organization.slug} + key={org.organization.id} ref={menuRef} selected={selected} text={org.organization.name} @@ -419,9 +419,9 @@ function LeftPane() { userOrgs.map((org: any) => ( ))} @@ -652,7 +652,7 @@ function ApplicationsSection(props: any) { {(currentUser || isFetchingApplications) && OrgMenuTarget({ orgName: organization.name, - orgSlug: organization.slug, + orgSlug: organization.id, })} {hasManageOrgPermissions && ( { setOrgToOpenMenu(null); }} @@ -753,7 +753,7 @@ function ApplicationsSection(props: any) { className="t--options-icon" name="context-menu" onClick={() => { - setOrgToOpenMenu(organization.slug); + setOrgToOpenMenu(organization.id); }} size={IconSize.XXXL} /> diff --git a/app/client/src/pages/Editor/Explorer/Entity/AddButton.tsx b/app/client/src/pages/Editor/Explorer/Entity/AddButton.tsx index 09ec10883d..c39cc5d633 100644 --- a/app/client/src/pages/Editor/Explorer/Entity/AddButton.tsx +++ b/app/client/src/pages/Editor/Explorer/Entity/AddButton.tsx @@ -15,6 +15,14 @@ const Wrapper = styled(EntityTogglesWrapper)` } } } + &.selected { + background: ${Colors.SHARK2} !important; + svg { + path { + fill: ${Colors.WHITE} !important; + } + } + } `; const PlusIcon = ControlIcons.INCREASE_CONTROL_V2; diff --git a/app/client/src/pages/Editor/Explorer/Files/Submenu.tsx b/app/client/src/pages/Editor/Explorer/Files/Submenu.tsx index 74abce51d3..e699705fc6 100644 --- a/app/client/src/pages/Editor/Explorer/Files/Submenu.tsx +++ b/app/client/src/pages/Editor/Explorer/Files/Submenu.tsx @@ -236,7 +236,10 @@ export default function ExplorerSubMenu({ hoverOpenDelay={TOOLTIP_HOVER_ON_DELAY} position={Position.RIGHT} > - setShow(true)} /> + setShow(true)} + /> ); diff --git a/app/client/src/pages/Settings/FormGroup/Dropdown.tsx b/app/client/src/pages/Settings/FormGroup/Dropdown.tsx index d05293ec7c..de9558fc3c 100644 --- a/app/client/src/pages/Settings/FormGroup/Dropdown.tsx +++ b/app/client/src/pages/Settings/FormGroup/Dropdown.tsx @@ -1,6 +1,6 @@ import React from "react"; import { FormGroup, SettingComponentProps } from "./Common"; -import SelectField from "components/ads/formFields/SelectField"; +import SelectField from "components/editorComponents/form/fields/SelectField"; export default function DropDown( props: { diff --git a/app/client/src/pages/Settings/SettingsForm.tsx b/app/client/src/pages/Settings/SettingsForm.tsx index 10452195bf..598dc45e97 100644 --- a/app/client/src/pages/Settings/SettingsForm.tsx +++ b/app/client/src/pages/Settings/SettingsForm.tsx @@ -59,7 +59,7 @@ function getSettingDetail(category: string, subCategory: string) { return AdminConfig.getCategoryDetails(category, subCategory); } -function useSettings(category: string, subCategory?: string) { +function getSettingsConfig(category: string, subCategory?: string) { return AdminConfig.get(subCategory ?? category); } @@ -68,7 +68,7 @@ export function SettingsForm( ) { const params = useParams() as any; const { category, subCategory } = params; - const settingsDetails = useSettings(category, subCategory); + const settingsDetails = getSettingsConfig(category, subCategory); const { settings, settingsConfig } = props; const details = getSettingDetail(category, subCategory); const dispatch = useDispatch(); diff --git a/app/client/src/pages/Settings/components.ts b/app/client/src/pages/Settings/components.ts index 1b0fdc1185..31dc49891c 100644 --- a/app/client/src/pages/Settings/components.ts +++ b/app/client/src/pages/Settings/components.ts @@ -8,8 +8,8 @@ export const Wrapper = styled.div` overflow: auto; `; -export const HeaderWrapper = styled.div` - margin-bottom: 16px; +export const HeaderWrapper = styled.div<{ margin?: string }>` + margin-bottom: ${(props) => props.margin ?? `16px`}; `; export const SettingsHeader = styled.h2` diff --git a/app/client/src/sagas/OrgSagas.ts b/app/client/src/sagas/OrgSagas.ts index 3d3cb068d8..d0c5276140 100644 --- a/app/client/src/sagas/OrgSagas.ts +++ b/app/client/src/sagas/OrgSagas.ts @@ -258,8 +258,8 @@ export function* createOrgSaga( } // get created org in focus - const slug = response.data.slug; - history.push(`${window.location.pathname}#${slug}`); + const orgId = response.data.id; + history.push(`${window.location.pathname}#${orgId}`); } catch (error) { yield call(reject, { _error: error.message }); yield put({ diff --git a/app/client/src/widgets/MultiSelectWidget/component/index.tsx b/app/client/src/widgets/MultiSelectWidget/component/index.tsx index 1f0bd17707..9f8548b4dd 100644 --- a/app/client/src/widgets/MultiSelectWidget/component/index.tsx +++ b/app/client/src/widgets/MultiSelectWidget/component/index.tsx @@ -164,7 +164,6 @@ function MultiSelectComponent({ }, []); const id = _.uniqueId(); - console.log("dropDownWidth", dropDownWidth); return ( { const cellProps = cell.getCellProps(); - if (columns[0]?.columnProperties?.cellBackground) { - cellProps.style.background = - columns[0].columnProperties.cellBackground; - } - return
; + set( + cellProps, + "style.backgroundColor", + get(cell, "column.columnProperties.cellBackground"), + ); + return ( +
+ ); })}
); @@ -568,6 +576,10 @@ export const renderEmptyRows = ( width: column.width + "px", boxSizing: "border-box", flex: `${column.width} 0 auto`, + backgroundColor: get( + column, + "columnProperties.cellBackground", + ), }} /> ); diff --git a/app/client/yarn.lock b/app/client/yarn.lock index a8438e1b73..ff8516ae11 100644 --- a/app/client/yarn.lock +++ b/app/client/yarn.lock @@ -11464,6 +11464,11 @@ methods@~1.1.2: version "1.1.2" resolved "https://registry.npmjs.org/methods/-/methods-1.1.2.tgz" +micro-memoize@^4.0.10: + version "4.0.10" + resolved "https://registry.yarnpkg.com/micro-memoize/-/micro-memoize-4.0.10.tgz#cedf7682df990cd2290700af4537afa6dba7d4e9" + integrity sha512-rk0OlvEQkShjbr2EvGn1+GdCsgLDgABQyM9ZV6VoHNU7hiNM+eSOkjGWhiNabU/XWiEalWbjNQrNO+zcqd+pEA== + microevent.ts@~0.1.1: version "0.1.1" resolved "https://registry.yarnpkg.com/microevent.ts/-/microevent.ts-0.1.1.tgz#70b09b83f43df5172d0205a63025bce0f7357fa0" diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java index 32290d23dd..bea945c252 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java @@ -43,14 +43,6 @@ public class Organization extends BaseDomain { @JsonIgnore private String logoAssetId; - public String makeSlug() { - return toSlug(name); - } - - public static String toSlug(String text) { - return text == null ? null : text.replaceAll("[^\\w\\d]+", "-").toLowerCase(); - } - public String getLogoUrl() { return Url.ASSET_URL + "/" + logoAssetId; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index 7587ecf810..4915756f7e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -69,7 +69,6 @@ import com.appsmith.server.dtos.WorkspacePluginStatus; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.helpers.GitDeployKeyGenerator; import com.appsmith.server.helpers.TextUtils; -import com.appsmith.server.services.WorkspaceService; import com.fasterxml.jackson.databind.ObjectMapper; import com.github.cloudyrock.mongock.ChangeLog; import com.github.cloudyrock.mongock.ChangeSet; @@ -311,23 +310,11 @@ public class DatabaseChangelog { dropIndexIfExists(mongoTemplate, Organization.class, "name"); } - @ChangeSet(order = "003", id = "add-org-slugs", author = "") - public void addOrgSlugs(MongockTemplate mongoTemplate, WorkspaceService workspaceService) { - // For all existing organizations, add a slug field, which should be unique. - // We are blocking here for adding a slug to each existing organization. This is bad and slow. Do NOT copy this - // code fragment into the services' control flow. This is a single migration code and is expected to run once in - // lifetime of a deployment. - for (Organization organization : mongoTemplate.findAll(Organization.class)) { - if (organization.getSlug() == null) { - workspaceService.getNextUniqueSlug(organization.makeSlug()) - .doOnSuccess(slug -> { - organization.setSlug(slug); - mongoTemplate.save(organization); - }) - .block(); - } - } - } + /* + * @ChangeSet(order = "003", id = "add-org-slugs", author = "") + * This migration has been removed as it's no more required. A newer version of this migration has been added + * in the @ChangeLog(order = "008") with id="update-organization-slugs" + */ /** * We are creating indexes manually because Spring's index resolver creates indexes on fields as well. @@ -368,11 +355,6 @@ public class DatabaseChangelog { makeIndex("email").unique() ); - ensureIndexes(mongoTemplate, Organization.class, - createdAtIndex, - makeIndex("slug").unique() - ); - ensureIndexes(mongoTemplate, Page.class, createdAtIndex, makeIndex("applicationId", "name").unique().named("application_page_compound_index") diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java index 2609e92494..f19f179313 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java @@ -15,12 +15,14 @@ import com.appsmith.server.domains.Plugin; import com.appsmith.server.domains.QApplication; import com.appsmith.server.domains.QNewAction; import com.appsmith.server.domains.QNewPage; +import com.appsmith.server.domains.QOrganization; import com.appsmith.server.domains.QPlugin; import com.appsmith.server.domains.Sequence; import com.appsmith.server.domains.Workspace; import com.appsmith.server.dtos.ActionDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.helpers.TextUtils; import com.github.cloudyrock.mongock.ChangeLog; import com.github.cloudyrock.mongock.ChangeSet; import com.github.cloudyrock.mongock.driver.mongodb.springdata.v3.decorator.impl.MongockTemplate; @@ -751,7 +753,32 @@ public class DatabaseChangelog2 { ); } - @ChangeSet(order = "008", id = "copy-organization-to-workspaces", author = "") + /** + * We'll remove the uniqe index on organization slugs. We'll also regenerate the slugs for all organizations as + * most of them are outdated + * @param mongockTemplate MongockTemplate instance + */ + @ChangeSet(order = "008", id = "update-organization-slugs", author = "") + public void updateOrganizationSlugs(MongockTemplate mongockTemplate) { + dropIndexIfExists(mongockTemplate, Organization.class, "slug"); + + // update organizations + final Query getAllOrganizationsQuery = query(where("deletedAt").is(null)); + getAllOrganizationsQuery.fields() + .include(fieldName(QOrganization.organization.name)); + + List organizations = mongockTemplate.find(getAllOrganizationsQuery, Organization.class); + + for (Organization organization : organizations) { + mongockTemplate.updateFirst( + query(where(fieldName(QOrganization.organization.id)).is(organization.getId())), + new Update().set(fieldName(QOrganization.organization.slug), TextUtils.makeSlug(organization.getName())), + Organization.class + ); + } + } + + @ChangeSet(order = "009", id = "copy-organization-to-workspaces", author = "") public void copyOrganizationToWorkspaces(MongockTemplate mongockTemplate) { Gson gson = new Gson(); for (Organization organization : mongockTemplate.findAll(Organization.class)) { @@ -760,21 +787,19 @@ public class DatabaseChangelog2 { } } - /** * We are creating indexes manually because Spring's index resolver creates indexes on fields as well. * See https://stackoverflow.com/questions/60867491/ for an explanation of the problem. We have that problem with * the `Action.datasource` field. */ - @ChangeSet(order = "009", id = "add-workspace-indexes", author = "") + @ChangeSet(order = "010", id = "add-workspace-indexes", author = "") public void addWorkspaceIndexes(MongockTemplate mongockTemplate) { ensureIndexes(mongockTemplate, Workspace.class, - makeIndex("createdAt"), - makeIndex("slug").unique() + makeIndex("createdAt") ); } - @ChangeSet(order = "010", id = "update-sequence-names-from-organization-to-workspace", author = "") + @ChangeSet(order = "011", id = "update-sequence-names-from-organization-to-workspace", author = "") public void updateSequenceNamesFromOrganizationToWorkspace(MongockTemplate mongockTemplate) { for (Sequence sequence : mongockTemplate.findAll(Sequence.class)) { String oldName = sequence.getName(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomWorkspaceRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomWorkspaceRepositoryCE.java index c13b1aeb9e..1df97bd69c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomWorkspaceRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomWorkspaceRepositoryCE.java @@ -15,8 +15,6 @@ public interface CustomWorkspaceRepositoryCE extends AppsmithRepository findByIdsIn(Set orgIds, AclPermission aclPermission, Sort sort); - Mono nextSlugNumber(String slugPrefix); - Mono updateUserRoleNames(String userId, String userName); Flux findAllWorkspaces(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomWorkspaceRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomWorkspaceRepositoryCEImpl.java index f4d58698b4..1ffd7ee119 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomWorkspaceRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomWorkspaceRepositoryCEImpl.java @@ -16,10 +16,8 @@ import reactor.core.publisher.Mono; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; import static org.springframework.data.mongodb.core.query.Criteria.where; -import static org.springframework.data.mongodb.core.query.Query.query; @Slf4j public class CustomWorkspaceRepositoryCEImpl extends BaseAppsmithRepositoryImpl @@ -43,29 +41,6 @@ public class CustomWorkspaceRepositoryCEImpl extends BaseAppsmithRepositoryImpl< return queryAll(List.of(orgIdsCriteria), aclPermission, sort); } - @Override - public Mono nextSlugNumber(String slugPrefix) { - final String slugField = fieldName(QWorkspace.workspace.slug); - final Query slugPrefixQuery = query(where(slugField).regex("^" + slugPrefix + "\\d*$")); - slugPrefixQuery.fields().include(slugField); - return mongoOperations - .find(slugPrefixQuery, Workspace.class) - .map(Workspace::getSlug) - .collect(Collectors.toSet()) - .map(slugs -> { - if (slugs.isEmpty() || !slugs.contains(slugPrefix)) { - return 0L; - } - - long number = 1L; - while (slugs.contains(slugPrefix + number)) { - ++number; - } - - return number; - }); - } - @Override public Mono updateUserRoleNames(String userId, String userName) { return mongoOperations diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java index a181bb53b4..952c14f70e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java @@ -909,7 +909,7 @@ public class UserServiceCEImpl extends BaseService if (isNewUser) { params.put("inviteUrl", inviteUrl); } else { - params.put("inviteUrl", inviteUrl + "/applications#" + workspace.getSlug()); + params.put("inviteUrl", inviteUrl + "/applications#" + workspace.getId()); } return params; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCE.java index c1edd02c1f..9fd6b9e052 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCE.java @@ -17,10 +17,6 @@ public interface WorkspaceServiceCE extends CrudService { Mono create(Workspace workspace); - Mono getBySlug(String slug); - - Mono getNextUniqueSlug(String initialSlug); - Mono createDefault(Workspace workspace, User user); Mono create(Workspace workspace, User user); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java index 9fc1ad2f7d..7bbad479cc 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java @@ -14,6 +14,7 @@ import com.appsmith.server.domains.UserRole; import com.appsmith.server.dtos.WorkspacePluginStatus; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.helpers.TextUtils; import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.AssetRepository; import com.appsmith.server.repositories.WorkspaceRepository; @@ -103,17 +104,6 @@ public class WorkspaceServiceCEImpl extends BaseService getBySlug(String slug) { - return repository.findBySlug(slug); - } - - @Override - public Mono getNextUniqueSlug(String initialSlug) { - return repository.nextSlugNumber(initialSlug) - .map(number -> initialSlug + (number == 0 ? "" : number)); - } - /** * Creates the given workspace as a default workspace for the given user. That is, the workspace's name * is changed to "[username]'s apps" and then created. The current value of the workspace name @@ -160,19 +150,9 @@ public class WorkspaceServiceCEImpl extends BaseService setSlugMono; - if (workspace.getName() == null) { - setSlugMono = Mono.just(workspace); - } else { - setSlugMono = getNextUniqueSlug(workspace.makeSlug()) - .map(slug -> { - workspace.setSlug(slug); - return workspace; - }); - } + workspace.setSlug(TextUtils.makeSlug(workspace.getName())); - return setSlugMono - .flatMap(this::validateObject) + return validateObject(workspace) // Install all the default plugins when the org is created /* TODO: This is a hack. We should ideally use the pluginService.installPlugin() function. Not using it right now because of circular dependency b/w workspaceService and pluginService @@ -226,6 +206,10 @@ public class WorkspaceServiceCEImpl extends BaseService { AppsmithBeanUtils.copyNewFieldValuesIntoOldObject(resource, existingWorkspace); @@ -248,12 +232,15 @@ public class WorkspaceServiceCEImpl extends BaseService save(Workspace workspace) { + if(StringUtils.hasLength(workspace.getName())) { + workspace.setSlug(TextUtils.makeSlug(workspace.getName())); + } return repository.save(workspace); } @Override - public Mono findByIdAndPluginsPluginId(String organizationId, String pluginId) { - return repository.findByIdAndPluginsPluginId(organizationId, pluginId); + public Mono findByIdAndPluginsPluginId(String workspaceId, String pluginId) { + return repository.findByIdAndPluginsPluginId(workspaceId, pluginId); } @Override diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java index 654d130b16..f3f35f959a 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java @@ -51,6 +51,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; import static com.appsmith.server.acl.AclPermission.MANAGE_APPLICATIONS; import static com.appsmith.server.acl.AclPermission.MANAGE_USERS; @@ -94,15 +95,12 @@ public class UserServiceTest { Mono userMono; - Mono workspaceMono; - @Autowired UserSignup userSignup; @Before public void setup() { userMono = userService.findByEmail("usertest@usertest.com"); - workspaceMono = workspaceService.getBySlug("spring-test-workspace"); } //Test if email params are updating correctly @@ -110,13 +108,13 @@ public class UserServiceTest { public void checkEmailParamsForExistingUser() { Workspace workspace = new Workspace(); workspace.setName("UserServiceTest Update Org"); - workspace.setSlug("userservicetest-update-org"); + workspace.setId(UUID.randomUUID().toString()); User inviter = new User(); inviter.setName("inviterUserToApplication"); String inviteUrl = "http://localhost:8080"; - String expectedUrl = inviteUrl + "/applications#userservicetest-update-org"; + String expectedUrl = inviteUrl + "/applications#" + workspace.getId(); Map params = userService.getEmailParams(workspace, inviter, inviteUrl, false); assertEquals(expectedUrl, params.get("inviteUrl")); @@ -127,8 +125,8 @@ public class UserServiceTest { @Test public void checkEmailParamsForNewUser() { Workspace workspace = new Workspace(); + workspace.setId(UUID.randomUUID().toString()); workspace.setName("UserServiceTest Update Org"); - workspace.setSlug("userservicetest-update-org"); User inviter = new User(); inviter.setName("inviterUserToApplication"); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java index 98bf376eba..af38c8eed4 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java @@ -2,24 +2,20 @@ package com.appsmith.server.services; import com.appsmith.external.models.Policy; import com.appsmith.server.acl.AppsmithRole; -import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.configurations.WithMockAppsmithUser; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.InviteUser; import com.appsmith.server.domains.LoginSource; -import com.appsmith.server.domains.Workspace; import com.appsmith.server.domains.User; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.repositories.UserRepository; -import com.appsmith.server.solutions.UserSignup; import lombok.extern.slf4j.Slf4j; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.test.context.support.WithUserDetails; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringRunner; @@ -42,32 +38,17 @@ public class UserServiceWithDisabledSignupTest { @Autowired UserService userService; - @Autowired - WorkspaceService workspaceService; - @Autowired ApplicationService applicationService; @Autowired UserRepository userRepository; - @Autowired - PasswordEncoder passwordEncoder; - - @Autowired - CommonConfig commonConfig; - Mono userMono; - Mono workspaceMono; - - @Autowired - UserSignup userSignup; - @Before public void setup() { userMono = userService.findByEmail("usertest@usertest.com"); - workspaceMono = workspaceService.getBySlug("spring-test-workspace"); } @Test @@ -232,8 +213,8 @@ public class UserServiceWithDisabledSignupTest { StepVerifier.create(userMono) .assertNext(user -> { - assertThat(user.getEmail().equals(newUser.getEmail())); - assertThat(user.getSource().equals(LoginSource.FORM)); + assertThat(user.getEmail()).isEqualTo(newUser.getEmail()); + assertThat(user.getSource()).isEqualTo(LoginSource.FORM); assertThat(user.getIsEnabled()).isTrue(); }) .verifyComplete(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java index ad482912a2..89ec866454 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java @@ -16,6 +16,7 @@ import com.appsmith.server.domains.UserRole; import com.appsmith.server.dtos.InviteUsersDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.helpers.TextUtils; import com.appsmith.server.repositories.AssetRepository; import com.appsmith.server.repositories.DatasourceRepository; import com.appsmith.server.repositories.WorkspaceRepository; @@ -49,6 +50,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.stream.Collectors; import static com.appsmith.server.acl.AclPermission.EXECUTE_DATASOURCES; @@ -196,7 +198,7 @@ public class WorkspaceServiceTest { assertThat(workspace1.getName()).isEqualTo("Test Name"); assertThat(workspace1.getPolicies()).isNotEmpty(); assertThat(workspace1.getPolicies()).containsAll(Set.of(manageOrgAppPolicy, manageOrgPolicy)); - assertThat(workspace1.getSlug() != null); + assertThat(workspace1.getSlug()).isEqualTo(TextUtils.makeSlug(workspace.getName())); assertThat(workspace1.getEmail()).isEqualTo("api_user"); assertThat(workspace1.getIsAutoGeneratedOrganization()).isNull(); }) @@ -313,24 +315,6 @@ public class WorkspaceServiceTest { .verify(); } - @Test - @WithUserDetails(value = "api_user") - public void uniqueSlugs() { - Workspace workspace = new Workspace(); - workspace.setName("Slug org"); - workspace.setDomain("example.com"); - workspace.setWebsite("https://example.com"); - - Mono uniqueSlug = workspaceService.create(workspace) - .flatMap(org -> workspaceService.getNextUniqueSlug(org.getSlug())); - - StepVerifier.create(uniqueSlug) - .assertNext(slug -> { - assertThat(slug).isEqualTo("slug-org1"); - }) - .verifyComplete(); - } - @Test @WithUserDetails(value = "api_user") public void createDuplicateNameWorkspace() { @@ -349,8 +333,11 @@ public class WorkspaceServiceTest { StepVerifier.create(Mono.zip(firstOrgCreation, secondOrgCreation)) .assertNext(orgsTuple -> { + assertThat(orgsTuple.getT1().getName()).isEqualTo("Really good org"); assertThat(orgsTuple.getT1().getSlug()).isEqualTo("really-good-org"); - assertThat(orgsTuple.getT2().getSlug()).isEqualTo("really-good-org1"); + + assertThat(orgsTuple.getT2().getName()).isEqualTo("Really good org"); + assertThat(orgsTuple.getT2().getSlug()).isEqualTo("really-good-org"); }) .verifyComplete(); } @@ -1183,4 +1170,49 @@ public class WorkspaceServiceTest { .create(deleteOrgMono) .verifyComplete(); } + + @Test + @WithUserDetails(value = "api_user") + public void save_WhenNameIsPresent_SlugGenerated() { + String uniqueString = UUID.randomUUID().toString(); // to make sure name is not conflicted with other tests + Workspace workspace = new Workspace(); + workspace.setName("My Organization " + uniqueString); + + String finalName = "Renamed Organization " + uniqueString; + + Mono workspaceMono = workspaceService.create(workspace) + .flatMap(savedWorkspace -> { + savedWorkspace.setName(finalName); + return workspaceService.save(savedWorkspace); + }); + + StepVerifier.create(workspaceMono) + .assertNext(savedWorkspace -> { + assertThat(savedWorkspace.getSlug()).isEqualTo(TextUtils.makeSlug(finalName)); + }) + .verifyComplete(); + } + + @Test + @WithUserDetails(value = "api_user") + public void update_WhenNameIsNotPresent_SlugIsNotGenerated() { + String uniqueString = UUID.randomUUID().toString(); // to make sure name is not conflicted with other tests + String initialName = "My Organization " + uniqueString; + Workspace workspace = new Workspace(); + workspace.setName(initialName); + + Mono workspaceMono = workspaceService.create(workspace) + .flatMap(savedWorkspace -> { + Workspace workspaceDto = new Workspace(); + workspaceDto.setWebsite("https://appsmith.com"); + return workspaceService.update(savedWorkspace.getId(), workspaceDto); + }); + + StepVerifier.create(workspaceMono) + .assertNext(savedWorkspace -> { + // slug should be unchanged + assertThat(savedWorkspace.getSlug()).isEqualTo(TextUtils.makeSlug(initialName)); + }) + .verifyComplete(); + } }