From 80895b876b04b9975636cd7275e06799a6648829 Mon Sep 17 00:00:00 2001 From: Pawan Kumar Date: Thu, 18 Mar 2021 18:40:57 +0530 Subject: [PATCH 1/5] Enhancement: Virtualized Query Editor Table (#3496) * add react-window on react-table * remove .vscode launch.json * add cellwrapper * fix height issue in virtualized table * useBlockLayout in Table * add border-right on table row Co-authored-by: Pawan Kumar --- .../src/pages/Editor/QueryEditor/Table.tsx | 79 ++++++++++--------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/app/client/src/pages/Editor/QueryEditor/Table.tsx b/app/client/src/pages/Editor/QueryEditor/Table.tsx index 76aa4cb059..1839afe8b4 100644 --- a/app/client/src/pages/Editor/QueryEditor/Table.tsx +++ b/app/client/src/pages/Editor/QueryEditor/Table.tsx @@ -1,7 +1,7 @@ import React from "react"; import styled from "styled-components"; import { FixedSizeList } from "react-window"; -import { useTable, useFlexLayout } from "react-table"; +import { useTable, useBlockLayout } from "react-table"; import { Colors } from "constants/Colors"; import { scrollbarWidth } from "utils/helpers"; @@ -57,6 +57,7 @@ export const TableWrapper = styled.div` } .tr { overflow: hidden; + border-right: 1px solid ${Colors.GEYSER_LIGHT}; :nth-child(even) { background: ${Colors.ATHENS_GRAY_DARKER}; } @@ -200,6 +201,13 @@ const Table = (props: TableProps) => { return []; }, [data]); + const defaultColumn = React.useMemo( + () => ({ + width: 170, + }), + [], + ); + const { getTableProps, getTableBodyProps, @@ -212,8 +220,9 @@ const Table = (props: TableProps) => { columns, data, manualPagination: true, + defaultColumn, }, - useFlexLayout, + useBlockLayout, ); const scrollBarSize = React.useMemo(() => scrollbarWidth(), []); @@ -232,16 +241,8 @@ const Table = (props: TableProps) => { > {row.cells.map((cell: any, cellIndex: number) => { return ( -
- - {cell.render("Cell")} - +
+ {cell.render("Cell")}
); })} @@ -258,33 +259,39 @@ const Table = (props: TableProps) => {
- {headerGroups.map((headerGroup: any, index: number) => ( -
- {headerGroup.headers.map((column: any, columnIndex: number) => ( -
-
- {column.render("Header")} -
-
- ))} -
- ))} +
+ {headerGroups.map((headerGroup: any, index: number) => ( +
+ {headerGroup.headers.map( + (column: any, columnIndex: number) => ( +
+
+ {column.render("Header")} +
+
+ ), + )} +
+ ))} +
Date: Thu, 18 Mar 2021 19:15:57 +0530 Subject: [PATCH 2/5] Remove page save event from frontend --- app/client/src/sagas/PageSagas.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/app/client/src/sagas/PageSagas.tsx b/app/client/src/sagas/PageSagas.tsx index 26578af83f..fa37f695cd 100644 --- a/app/client/src/sagas/PageSagas.tsx +++ b/app/client/src/sagas/PageSagas.tsx @@ -306,7 +306,6 @@ function* savePageSaga(action: ReduxAction<{ isRetry?: boolean }>) { pageId: savePageRequest.pageId, }, ); - AnalyticsUtil.logEvent("PAGE_SAVE", JSON.stringify(savePageRequest)); try { // Store the updated DSL in the pageDSLs reducer yield put({ From 55c17a66aa45f36f98353d7247fd7b3a4685fc5c Mon Sep 17 00:00:00 2001 From: Kaushik Varanasi Date: Thu, 18 Mar 2021 19:20:33 +0530 Subject: [PATCH 3/5] Fix success message for single user invite and multiple user invite (#3591) * Fixed error message for single user invite * updated success message for single user invite and created tests * use state to detect and update the number of users invited * removing unnecessary lines * typo --- .../OrganisationTests/CreateOrgTests_spec.js | 8 ++++++++ app/client/cypress/support/commands.js | 5 ----- app/client/src/constants/messages.ts | 2 ++ .../src/pages/organization/OrgInviteUsersForm.tsx | 12 +++++++++++- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/client/cypress/integration/Smoke_TestSuite/ServerSideTests/OrganisationTests/CreateOrgTests_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ServerSideTests/OrganisationTests/CreateOrgTests_spec.js index b0f0552a65..d3a56e8673 100644 --- a/app/client/cypress/integration/Smoke_TestSuite/ServerSideTests/OrganisationTests/CreateOrgTests_spec.js +++ b/app/client/cypress/integration/Smoke_TestSuite/ServerSideTests/OrganisationTests/CreateOrgTests_spec.js @@ -19,6 +19,14 @@ describe("Create new org and share with a user", function() { Cypress.env("TESTUSERNAME1"), homePage.viewerRole, ); + // check that the success message is correct + const successMessage = "The user has been invited successfully"; + cy.contains(successMessage); + cy.get(homePage.manageUsers).click({ force: true }); + cy.xpath(homePage.appHome) + .first() + .should("be.visible") + .click(); cy.CheckShareIcon(orgid, 2); cy.CreateAppForOrg(orgid, appid); }); diff --git a/app/client/cypress/support/commands.js b/app/client/cypress/support/commands.js index ffa0d16b98..b411a810e7 100644 --- a/app/client/cypress/support/commands.js +++ b/app/client/cypress/support/commands.js @@ -85,11 +85,6 @@ Cypress.Commands.add("inviteUserForOrg", (orgName, email, role) => { 200, ); cy.contains(email); - cy.get(homePage.manageUsers).click({ force: true }); - cy.xpath(homePage.appHome) - .first() - .should("be.visible") - .click(); }); Cypress.Commands.add("CheckShareIcon", (orgName, count) => { diff --git a/app/client/src/constants/messages.ts b/app/client/src/constants/messages.ts index c2c8831c26..72b168888e 100644 --- a/app/client/src/constants/messages.ts +++ b/app/client/src/constants/messages.ts @@ -129,6 +129,8 @@ export const INVITE_USERS_SUBMIT_ERROR = () => `We were unable to invite the users, please try again later`; export const INVITE_USERS_SUBMIT_SUCCESS = () => `The users have been invited successfully`; +export const INVITE_USER_SUBMIT_SUCCESS = () => + `The user has been invited successfully`; export const INVITE_USERS_VALIDATION_EMAILS_EMPTY = () => `Please enter the user emails`; diff --git a/app/client/src/pages/organization/OrgInviteUsersForm.tsx b/app/client/src/pages/organization/OrgInviteUsersForm.tsx index de198a02fc..a7c7b16014 100644 --- a/app/client/src/pages/organization/OrgInviteUsersForm.tsx +++ b/app/client/src/pages/organization/OrgInviteUsersForm.tsx @@ -18,6 +18,7 @@ import { INVITE_USERS_TO_ORG_FORM } from "constants/forms"; import { createMessage, INVITE_USERS_SUBMIT_SUCCESS, + INVITE_USER_SUBMIT_SUCCESS, INVITE_USERS_VALIDATION_EMAILS_EMPTY, INVITE_USERS_VALIDATION_EMAIL_LIST, INVITE_USERS_VALIDATION_ROLE_EMPTY, @@ -220,6 +221,8 @@ const OrgInviteUsersForm = (props: any) => { isLoading, } = props; + // set state for checking number of users invited + const [numberOfUsersInvited, updateNumberOfUsersInvited] = useState(0); const currentOrg = useSelector(getCurrentAppOrg); const userOrgPermissions = currentOrg?.userPermissions ?? []; @@ -275,6 +278,9 @@ const OrgInviteUsersForm = (props: any) => { onSubmit={handleSubmit((values: any, dispatch: any) => { validateFormValues(values); AnalyticsUtil.logEvent("INVITE_USER", values); + const usersAsStringsArray = values.users.split(","); + // update state to show success message correctly + updateNumberOfUsersInvited(usersAsStringsArray.length); return inviteUsersToOrg({ ...values, orgId: props.orgId }, dispatch); })} > @@ -362,7 +368,11 @@ const OrgInviteUsersForm = (props: any) => { 1 + ? INVITE_USERS_SUBMIT_SUCCESS() + : INVITE_USER_SUBMIT_SUCCESS() + } /> )} {((submitFailed && error) || emailError) && ( From bb1d0059d3f8cc4b34614e34bad683f2d37f6809 Mon Sep 17 00:00:00 2001 From: Shri Date: Thu, 18 Mar 2021 21:08:56 +0530 Subject: [PATCH 4/5] Clear OAuth tokens for forked datasources (#3609) * Clear OAuth tokens for forked datasources * Fix datasource duplicate finder in light of oAuth tokens * Fix potential NPE --- .../models/AuthenticationResponse.java | 5 ++++- .../solutions/ExamplesOrganizationCloner.java | 21 +++++++++++++++++-- .../ExamplesOrganizationClonerTests.java | 12 +++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/AuthenticationResponse.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/AuthenticationResponse.java index d5582a981c..a35d0392bb 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/AuthenticationResponse.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/AuthenticationResponse.java @@ -1,13 +1,16 @@ package com.appsmith.external.models; -import com.fasterxml.jackson.annotation.JsonIgnore; +import lombok.AllArgsConstructor; import lombok.Getter; +import lombok.NoArgsConstructor; import lombok.Setter; import java.time.Instant; @Getter @Setter +@NoArgsConstructor +@AllArgsConstructor public class AuthenticationResponse { String token; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ExamplesOrganizationCloner.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ExamplesOrganizationCloner.java index c41a34fdd3..5eac88d0fe 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ExamplesOrganizationCloner.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ExamplesOrganizationCloner.java @@ -1,5 +1,6 @@ package com.appsmith.server.solutions; +import com.appsmith.external.models.AuthenticationDTO; import com.appsmith.external.models.BaseDomain; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Application; @@ -361,7 +362,23 @@ public class ExamplesOrganizationCloner { final Datasource templateDatasource = tuple.getT1(); final List existingDatasources = tuple.getT2(); + final AuthenticationDTO authentication = templateDatasource.getDatasourceConfiguration() == null + ? null : templateDatasource.getDatasourceConfiguration().getAuthentication(); + if (authentication != null) { + authentication.setIsAuthorized(null); + authentication.setAuthenticationResponse(null); + } + return Flux.fromIterable(existingDatasources) + .map(ds -> { + final AuthenticationDTO auth = ds.getDatasourceConfiguration() == null + ? null : ds.getDatasourceConfiguration().getAuthentication(); + if (auth != null) { + auth.setIsAuthorized(null); + auth.setAuthenticationResponse(null); + } + return ds; + }) .filter(templateDatasource::softEquals) .next() // Get the first matching datasource, we don't need more than one here. .switchIfEmpty(Mono.defer(() -> { @@ -369,8 +386,8 @@ public class ExamplesOrganizationCloner { makePristine(templateDatasource); templateDatasource.setOrganizationId(toOrganizationId); - if (templateDatasource.getDatasourceConfiguration() != null) { - datasourceContextService.decryptSensitiveFields(templateDatasource.getDatasourceConfiguration().getAuthentication()); + if (authentication != null) { + datasourceContextService.decryptSensitiveFields(authentication); } return createSuffixedDatasource(templateDatasource); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ExamplesOrganizationClonerTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ExamplesOrganizationClonerTests.java index ebbe67750e..16f9ef7d2e 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ExamplesOrganizationClonerTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ExamplesOrganizationClonerTests.java @@ -1,6 +1,7 @@ package com.appsmith.server.solutions; import com.appsmith.external.models.ActionConfiguration; +import com.appsmith.external.models.AuthenticationResponse; import com.appsmith.external.models.Connection; import com.appsmith.external.models.DBAuth; import com.appsmith.external.models.DatasourceConfiguration; @@ -59,6 +60,7 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -762,6 +764,8 @@ public class ExamplesOrganizationClonerTests { new Property("custom auth param 1", "custom auth param value 1"), new Property("custom auth param 2", "custom auth param value 2") )); + auth.setIsAuthorized(true); + auth.setAuthenticationResponse(new AuthenticationResponse("token", "refreshToken", Instant.now(), Instant.now(), null)); dc.setAuthentication(auth); final Datasource ds2 = new Datasource(); @@ -898,6 +902,14 @@ public class ExamplesOrganizationClonerTests { "datasource 2" ); + final Datasource ds1 = data.datasources.stream().filter(ds -> ds.getName().equals("datasource 1")).findFirst().get(); + assertThat(ds1.getDatasourceConfiguration().getAuthentication().getIsAuthorized()).isNull(); + assertThat(ds1.getDatasourceConfiguration().getAuthentication().getAuthenticationResponse()).isNull(); + + final Datasource ds2 = data.datasources.stream().filter(ds -> ds.getName().equals("datasource 2")).findFirst().get(); + assertThat(ds2.getDatasourceConfiguration().getAuthentication().getIsAuthorized()).isNull(); + assertThat(ds2.getDatasourceConfiguration().getAuthentication().getAuthenticationResponse()).isNull(); + assertThat(getUnpublishedActionName(data.actions)).containsExactlyInAnyOrder( "action1", "action2", From 93d5a061e2f445a3b34df15817c186761452f357 Mon Sep 17 00:00:00 2001 From: Nidhi Date: Thu, 18 Mar 2021 21:48:13 +0530 Subject: [PATCH 5/5] Added logs and analytics (#3622) * Added logs and analytics * Fix in error string Co-authored-by: Shri * Review fixes :) Co-authored-by: Shri Co-authored-by: Shri --- .../server/constants/AnalyticsEvents.java | 1 + .../server/exceptions/AppsmithError.java | 3 +- .../services/LayoutActionServiceImpl.java | 63 +++++++++++++++---- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/AnalyticsEvents.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/AnalyticsEvents.java index 62454b4ceb..a22833b88d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/AnalyticsEvents.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/AnalyticsEvents.java @@ -8,6 +8,7 @@ public enum AnalyticsEvents { DELETE, FIRST_LOGIN, EXECUTE_ACTION("execute_ACTION_TRIGGERED"), + UPDATE_LAYOUT, ; private final String eventName; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java index 2a2e6c8e0c..5e46dc1d5f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java @@ -37,7 +37,8 @@ public enum AppsmithError { " \"widgetName\" : \"{1}\"," + " \"widgetId\" : \"{2}\"," + " \"pageId\" : \"{4}\"," + - " \"layoutId\" : \"{5}\"", + " \"layoutId\" : \"{5}\"," + + " \"dynamicBinding\" : \"{6}\"", AppsmithErrorAction.LOG_EXTERNALLY), USER_ALREADY_EXISTS_IN_ORGANIZATION(400, 4021, "The user {0} has already been added to the organization with role {1}. To change the role, please navigate to `Manage Users` page.", AppsmithErrorAction.DEFAULT), UNAUTHORIZED_DOMAIN(401, 4019, "Invalid email domain {0} used for sign in/sign up. Please contact the administrator to configure this domain if this is unexpected.", AppsmithErrorAction.DEFAULT), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java index 5a64884605..f01231fc4a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java @@ -2,9 +2,12 @@ package com.appsmith.server.services; import com.appsmith.external.helpers.MustacheHelper; import com.appsmith.external.models.ActionConfiguration; +import com.appsmith.server.constants.AnalyticsEvents; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.ActionDependencyEdge; import com.appsmith.server.domains.Layout; +import com.appsmith.server.domains.NewPage; +import com.appsmith.server.domains.User; import com.appsmith.server.dtos.ActionDTO; import com.appsmith.server.dtos.ActionMoveDTO; import com.appsmith.server.dtos.DslActionDTO; @@ -31,6 +34,7 @@ import reactor.core.publisher.Mono; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -53,6 +57,8 @@ public class LayoutActionServiceImpl implements LayoutActionService { private final NewPageService newPageService; private final NewActionService newActionService; private final PageLoadActionsUtil pageLoadActionsUtil; + private final SessionUserService sessionUserService; + /* * To replace fetchUsers in `{{JSON.stringify(fetchUsers)}}` with getUsers, the following regex is required : @@ -65,12 +71,15 @@ public class LayoutActionServiceImpl implements LayoutActionService { public LayoutActionServiceImpl(ObjectMapper objectMapper, AnalyticsService analyticsService, NewPageService newPageService, - NewActionService newActionService, PageLoadActionsUtil pageLoadActionsUtil) { + NewActionService newActionService, + PageLoadActionsUtil pageLoadActionsUtil, + SessionUserService sessionUserService) { this.objectMapper = objectMapper; this.analyticsService = analyticsService; this.newPageService = newPageService; this.newActionService = newActionService; this.pageLoadActionsUtil = pageLoadActionsUtil; + this.sessionUserService = sessionUserService; } @Override @@ -310,7 +319,7 @@ public class LayoutActionServiceImpl implements LayoutActionService { // For nested fields, the parent dsl to search in would shift by one level every iteration Object parent = dsl; Iterator fieldsIterator = Arrays.stream(fields).filter(fieldToken -> !fieldToken.isBlank()).iterator(); - Boolean isLeafNode = false; + boolean isLeafNode = false; // This loop will end at either a leaf node, or the last identified JSON field (by throwing an exception) // Valid forms of the fieldPath for this search could be: // root.field.list[index].childField.anotherList.indexWithDotOperator.multidimensionalList[index1][index2] @@ -327,30 +336,30 @@ public class LayoutActionServiceImpl implements LayoutActionService { } catch (IndexOutOfBoundsException e) { // The index being referred does not exist. Hence the path would not exist. throw new AppsmithException(AppsmithError.INVALID_DYNAMIC_BINDING_REFERENCE, widgetType, - widgetName, widgetId, fieldPath, pageId, layoutId); + widgetName, widgetId, fieldPath, pageId, layoutId, null); } } else { throw new AppsmithException(AppsmithError.INVALID_DYNAMIC_BINDING_REFERENCE, widgetType, - widgetName, widgetId, fieldPath, pageId, layoutId); + widgetName, widgetId, fieldPath, pageId, layoutId, null); } } // After updating the parent, check for the types if (parent == null) { throw new AppsmithException(AppsmithError.INVALID_DYNAMIC_BINDING_REFERENCE, widgetType, - widgetName, widgetId, fieldPath, pageId, layoutId); + widgetName, widgetId, fieldPath, pageId, layoutId, null); } else if (parent instanceof String) { // If we get String value, then this is a leaf node isLeafNode = true; } } // Only extract mustache keys from leaf nodes - if (parent != null && isLeafNode) { + if (isLeafNode) { Set mustacheKeysFromFields = MustacheHelper.extractMustacheKeysFromFields(parent); // We found the path. But if the path does not have any mustache bindings, throw the error if (mustacheKeysFromFields.isEmpty()) { throw new AppsmithException(AppsmithError.INVALID_DYNAMIC_BINDING_REFERENCE, widgetType, - widgetName, widgetId, fieldPath, pageId, layoutId); + widgetName, widgetId, fieldPath, pageId, layoutId, parent); } dynamicBindings.addAll(mustacheKeysFromFields); @@ -499,9 +508,38 @@ public class LayoutActionServiceImpl implements LayoutActionService { .then(Mono.just(pageId)); } + private Mono sendUpdateLayoutAnalyticsEvent(String pageId, String layoutId, JSONObject dsl, boolean isSuccess, Throwable error) { + return Mono.zip( + sessionUserService.getCurrentUser(), + newPageService.getById(pageId) + ) + .flatMap(tuple -> { + User t1 = tuple.getT1(); + NewPage t2 = tuple.getT2(); + + final Map data = Map.of( + "username", t1.getUsername(), + "appId", t2.getApplicationId(), + "pageId", pageId, + "layoutId", layoutId, + "dsl", dsl.toJSONString(), + "isSuccessfulExecution", isSuccess, + "error", error == null ? "" : error.getMessage() + ); + + analyticsService.sendEvent(AnalyticsEvents.UPDATE_LAYOUT.getEventName(), t1.getUsername(), data); + return Mono.just(isSuccess); + }) + .onErrorResume(e -> { + log.warn("Error sending action execution data point", e); + return Mono.just(isSuccess); + }); + + } + @Override public Mono updateLayout(String pageId, String layoutId, Layout layout) { - JSONObject dsl = layout.getDsl(); + final JSONObject dsl = layout.getDsl(); if (dsl == null) { // There is no DSL here. No need to process anything. Return as is. return Mono.just(generateResponseDTO(layout)); @@ -512,7 +550,8 @@ public class LayoutActionServiceImpl implements LayoutActionService { try { extractAllWidgetNamesAndDynamicBindingsFromDSL(dsl, widgetNames, jsSnippetsInDynamicBindings, pageId, layoutId); } catch (Throwable t) { - return Mono.error(t); + return sendUpdateLayoutAnalyticsEvent(pageId, layoutId, dsl, false, t) + .then(Mono.error(t)); } layout.setWidgetNames(widgetNames); @@ -581,11 +620,13 @@ public class LayoutActionServiceImpl implements LayoutActionService { } return Mono.empty(); }) - .map(savedLayout -> { + .flatMap(savedLayout -> { LayoutDTO layoutDTO = generateResponseDTO(savedLayout); layoutDTO.setActionUpdates(actionUpdates); layoutDTO.setMessages(messages); - return layoutDTO; + + return sendUpdateLayoutAnalyticsEvent(pageId, layoutId, dsl, true, null) + .thenReturn(layoutDTO); }); }