From 9e7d1d85f8ef63d6488bf8dc642e231599b05c96 Mon Sep 17 00:00:00 2001 From: Nilesh Sarupriya Date: Wed, 18 Oct 2023 14:02:09 +0530 Subject: [PATCH] chore: invite unique emails to workspace (#28151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > With the current workflow for inviting users to workspace, we are not filtering out duplicate emails. With this code change, we intend to only invite unique users to workspace. #### PR fixes following issue(s) Fixes https://github.com/appsmithorg/appsmith/issues/28067 #### 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 - Chore (housekeeping or task changes that don't impact user perception) ## 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 - [ ] Manual - [ ] JUnit - [ ] 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 - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] 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 --------- Co-authored-by: Nilesh Sarupriya <20905988+nsarupr@users.noreply.github.com> Co-authored-by: Ankita Kinger --- .../ce/pages/workspace/InviteUsersForm.tsx | 2 +- app/client/src/ce/sagas/userSagas.tsx | 15 ++++++---- .../UserAndAccessManagementServiceCEImpl.java | 5 ++-- .../server/services/WorkspaceServiceTest.java | 28 +++++++++++++++++++ 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/app/client/src/ce/pages/workspace/InviteUsersForm.tsx b/app/client/src/ce/pages/workspace/InviteUsersForm.tsx index b8bf34b7ba..5e7a67d179 100644 --- a/app/client/src/ce/pages/workspace/InviteUsersForm.tsx +++ b/app/client/src/ce/pages/workspace/InviteUsersForm.tsx @@ -388,7 +388,7 @@ function InviteUsersForm(props: any) { const validEmails = usersAsStringsArray.filter((user: string) => isEmail(user), ); - const validEmailsString = validEmails.join(","); + const validEmailsString = [...new Set(validEmails)].join(","); invitedEmails.current = validEmails; AnalyticsUtil.logEvent("INVITE_USER", { diff --git a/app/client/src/ce/sagas/userSagas.tsx b/app/client/src/ce/sagas/userSagas.tsx index 2bde6af6ff..c8b7a91c6a 100644 --- a/app/client/src/ce/sagas/userSagas.tsx +++ b/app/client/src/ce/sagas/userSagas.tsx @@ -351,10 +351,11 @@ export function* inviteUsers( ) { const { data, reject, resolve } = action.payload; try { - const response: ApiResponse = yield callAPI(UserApi.inviteUser, { - usernames: data.usernames, - permissionGroupId: data.permissionGroupId, - }); + const response: ApiResponse<{ id: string; username: string }[]> = + yield callAPI(UserApi.inviteUser, { + usernames: data.usernames, + permissionGroupId: data.permissionGroupId, + }); const isValidResponse: boolean = yield validateResponse(response, false); if (!isValidResponse) { let errorMessage = `${data.usernames}: `; @@ -367,12 +368,14 @@ export function* inviteUsers( workspaceId: data.workspaceId, }, }); + const { data: responseData } = response; yield put({ type: ReduxActionTypes.INVITED_USERS_TO_WORKSPACE, payload: { workspaceId: data.workspaceId, - users: data.usernames.map((name: string) => ({ - username: name, + users: responseData.map((user: { id: string; username: string }) => ({ + userId: user.id, + username: user.username, permissionGroupId: data.permissionGroupId, })), }, diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserAndAccessManagementServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserAndAccessManagementServiceCEImpl.java index 4d4c963f71..67f6a22599 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserAndAccessManagementServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserAndAccessManagementServiceCEImpl.java @@ -25,10 +25,11 @@ import reactor.core.publisher.Mono; import reactor.util.function.Tuple2; import reactor.util.function.Tuples; -import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import static java.lang.Boolean.TRUE; @@ -97,7 +98,7 @@ public class UserAndAccessManagementServiceCEImpl implements UserAndAccessManage return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ROLE)); } - List usernames = new ArrayList<>(); + Set usernames = new HashSet<>(); for (String username : originalUsernames) { usernames.add(username.toLowerCase()); } 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 4c7e148cf4..2aeb50a519 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 @@ -1840,4 +1840,32 @@ public class WorkspaceServiceTest { mongoTemplate.count(queryWorkspaceWithAdditionalField, Workspace.class); assertThat(countWorkspaceWithAdditionalFieldAfterUpdate).isEqualTo(1); } + + @Test + @WithUserDetails(value = "api_user") + void testInviteDuplicateUsers_shouldReturnUniqueUsers() { + String testName = "test"; + List duplicateUsernames = List.of(testName + "user@test.com", testName + "user@test.com"); + Workspace createdWorkspace = workspaceService.create(workspace).block(); + assertThat(createdWorkspace).isNotNull(); + assertThat(createdWorkspace.getDefaultPermissionGroups()).isNotEmpty(); + List defaultWorkspaceRoles = permissionGroupRepository + .findAllById(createdWorkspace.getDefaultPermissionGroups()) + .collectList() + .block(); + assertThat(defaultWorkspaceRoles) + .hasSize(createdWorkspace.getDefaultPermissionGroups().size()); + + InviteUsersDTO inviteUsersDTO = new InviteUsersDTO(); + inviteUsersDTO.setUsernames(duplicateUsernames); + inviteUsersDTO.setPermissionGroupId(defaultWorkspaceRoles.get(0).getId()); + + // Invited users list contains duplicate users. + List userList = userAndAccessManagementService + .inviteUsers(inviteUsersDTO, "test") + .block(); + assertThat(userList).hasSize(1); + User invitedUser = userList.get(0); + assertThat(invitedUser.getUsername()).isEqualTo(testName + "user@test.com"); + } }