diff --git a/src/pages/users/BulkUserUpload/UsersToUploadSummary.test.tsx b/src/pages/users/BulkUserUpload/UsersToUploadSummary.test.tsx index 3feb8a0..6500af4 100644 --- a/src/pages/users/BulkUserUpload/UsersToUploadSummary.test.tsx +++ b/src/pages/users/BulkUserUpload/UsersToUploadSummary.test.tsx @@ -6,18 +6,45 @@ import {createMemoryHistory} from "history"; import {Router} from "react-router"; import { ImportUser } from "../../../../Interfaces"; import UsersToUploadSummary from "./UsersToUploadSummary"; -import { getAllRoles } from "../../../utilities/http"; -import { UserRole } from "blaise-api-node-client"; +import { getAllRoles, getAllUsers } from "../../../utilities/http"; +import { User, UserRole } from "blaise-api-node-client"; // set global vars let view:RenderResult; // set mocks type getRolesListResponse = [boolean, UserRole[]]; +type getUsersListResponse = [boolean, User[]]; + jest.mock("../../../utilities/http"); + const getAllRolesMock = getAllRoles as jest.Mock>; +const getAllUsersMock = getAllUsers as jest.Mock>; describe("Upload summary tests", () => { + const importedUsers: ImportUser[] = [ + { + name:"Jamie", + password:"pass", + role:"BDSS", + valid:false, + warnings:[] + }, + { + name:"Rob", + password:"pass2", + role:"DST", + valid:false, + warnings:[] + }, + { + name:"Rich", + password:"pass", + role:"BDSS", + valid:false, + warnings:[] + } + ]; const roles: getRolesListResponse = [ true, @@ -34,42 +61,25 @@ describe("Upload summary tests", () => { } ]]; + const existingUsers: getUsersListResponse = [ + true, + [] + ]; + beforeEach(() => { getAllRolesMock.mockImplementation(() => Promise.resolve(roles)); + getAllUsersMock.mockImplementation(() => Promise.resolve(existingUsers)); }); - it("Upload summary pages for three valid users matches Snapshot", async () => { + it("Upload summary pages for valid imported users matches Snapshot", async () => { //arrange - const userList: ImportUser[] = [ - { - name:"Jamie", - password:"pass", - role:"BDSS", - valid:false, - warnings:[] - }, - { - name:"Rob", - password:"pass2", - role:"DST", - valid:false, - warnings:[] - }, - { - name:"Rich", - password:"pass", - role:"BDSS", - valid:false, - warnings:[] - } - ]; // act await act(async () => { const history = createMemoryHistory(); view = render( - {return;}}/> + {return;}}/> ); }); @@ -81,39 +91,15 @@ describe("Upload summary tests", () => { }); - it("Upload summary pages for three valid users displays correct summary", async () => { + it("Upload summary pages for valid imported users displays correct summary", async () => { //arrange - const userList: ImportUser[] = [ - { - name:"Jamie", - password:"pass", - role:"BDSS", - valid:false, - warnings:[] - }, - { - name:"Rob", - password:"pass2", - role:"DST", - valid:false, - warnings:[] - }, - { - name:"Rich", - password:"pass", - role:"BDSS", - valid:false, - warnings:[] - } - ]; - // act // act await act(async () => { const history = createMemoryHistory(); view = render( - {return;}}/> + {return;}}/> ); }); @@ -140,7 +126,7 @@ describe("Upload summary tests", () => { it("Upload summary pages for two valid and one invalid users matches Snapshot", async () => { //arrange - const userList: ImportUser[] = [ + const invalidImportedUsers: ImportUser[] = [ { name:"Jamie", password:"pass", @@ -169,7 +155,7 @@ describe("Upload summary tests", () => { const history = createMemoryHistory(); view = render( - {return;}}/> + {return;}}/> ); }); @@ -182,7 +168,7 @@ describe("Upload summary tests", () => { it("Upload summary pages for two valid and one invalid users displays correct summary", async () => { //arrange - const userList: ImportUser[] = [ + const invalidImportedUsers: ImportUser[] = [ { name:"Jamie", password:"pass", @@ -211,7 +197,7 @@ describe("Upload summary tests", () => { const history = createMemoryHistory(); view = render( - {return;}}/> + {return;}}/> ); }); @@ -236,38 +222,56 @@ describe("Upload summary tests", () => { expect(user3Summary).toHaveTextContent("Valid User"); }); - it("Upload summary pages for two users with the same name matches Snapshot", async () => { + it("Upload summary pages for an imported users that already exist matches Snapshot", async () => { //arrange - const userList: ImportUser[] = [ + const importedUsersIncludingExisting: ImportUser[] = [ { - name:"Jamie", + name:"Jamie", // user already exists password:"pass", role:"BDSS", valid:false, warnings:[] }, { - name:"Rich", - password:"pass", - role:"BDSS", + name:"Rob", + password:"pass2", + role:"DST", valid:false, warnings:[] }, { - name:"Jamie", - password:"pass", + name:"Rich", // user already exists + password:"pass3", role:"BDSS", valid:false, warnings:[] } ]; + const matchingExistingUsers: getUsersListResponse = [ + true, + [{ + name:"Jamie", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }, + { + name:"Rich", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }] + ]; + + getAllUsersMock.mockImplementation(() => Promise.resolve(matchingExistingUsers)); + // act await act(async () => { const history = createMemoryHistory(); view = render( - {return;}}/> + {return;}}/> ); }); @@ -280,36 +284,54 @@ describe("Upload summary tests", () => { it("Upload summary pages for two users with the same name displays correct summary", async () => { //arrange - const userList: ImportUser[] = [ + const importedUsersIncludingExisting: ImportUser[] = [ { - name:"Jamie", + name:"Jamie", // user already exists password:"pass", role:"BDSS", valid:false, warnings:[] }, { - name:"Rich", - password:"pass", - role:"BDSS", + name:"Rob", + password:"pass2", + role:"DST", valid:false, warnings:[] }, { - name:"Jamie", - password:"pass", + name:"Rich", // user already exists + password:"pass3", role:"BDSS", valid:false, warnings:[] } ]; + const matchingExistingUsers: getUsersListResponse = [ + true, + [{ + name:"Jamie", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }, + { + name:"Rich", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }] + ]; + + getAllUsersMock.mockImplementation(() => Promise.resolve(matchingExistingUsers)); + // act await act(async () => { const history = createMemoryHistory(); view = render( - {return;}}/> + {return;}}/> ); }); @@ -321,16 +343,16 @@ describe("Upload summary tests", () => { const user1Summary = view.getByTestId("user-table-row-0"); expect(user1Summary).toHaveTextContent("Jamie"); expect(user1Summary).toHaveTextContent("BDSS"); - expect(user1Summary).toHaveTextContent("User exists multiple times"); + expect(user1Summary).toHaveTextContent("User already exists"); const user2Summary = view.getByTestId("user-table-row-1"); - expect(user2Summary).toHaveTextContent("Jamie"); + expect(user2Summary).toHaveTextContent("Rich"); expect(user2Summary).toHaveTextContent("BDSS"); - expect(user2Summary).toHaveTextContent("User exists multiple times"); + expect(user2Summary).toHaveTextContent("User already exists"); const user3Summary = view.getByTestId("user-table-row-2"); - expect(user3Summary).toHaveTextContent("Rich"); - expect(user3Summary).toHaveTextContent("BDSS"); + expect(user3Summary).toHaveTextContent("Rob"); + expect(user3Summary).toHaveTextContent("DST"); expect(user3Summary).toHaveTextContent("Valid User"); }); diff --git a/src/pages/users/BulkUserUpload/UsersToUploadSummary.tsx b/src/pages/users/BulkUserUpload/UsersToUploadSummary.tsx index 660bb1d..196b7d4 100644 --- a/src/pages/users/BulkUserUpload/UsersToUploadSummary.tsx +++ b/src/pages/users/BulkUserUpload/UsersToUploadSummary.tsx @@ -3,8 +3,7 @@ import {ErrorBoundary, ONSPanel} from "blaise-design-system-react-components"; import {ImportUser} from "../../../../Interfaces"; import Confirmation from "./Confirmation"; import converter from "number-to-words"; -import { getAllRoles } from "../../../utilities/http"; -import { validateUsers } from "../../../utilities/validation/userValidation"; +import { validateImportedUsers } from "../../../utilities/validation/userValidation"; interface Props { usersToImport: ImportUser[] @@ -20,13 +19,9 @@ function UsersToUploadSummary({usersToImport, uploadUsers}: Props): ReactElement setupUserList().then(() => {return;}); }, []); - async function setupUserList() { + async function setupUserList() { setListError("Loading ..."); - - const [success, roleList] = await getAllRoles(); - if (success) { - validateUsers(usersToImport, roleList); - } + await validateImportedUsers(usersToImport); let noValid = 0; usersToImport.map((user: ImportUser) => { diff --git a/src/pages/users/BulkUserUpload/__snapshots__/UsersToUploadSummary.test.tsx.snap b/src/pages/users/BulkUserUpload/__snapshots__/UsersToUploadSummary.test.tsx.snap index 3e1d9fd..48e2279 100644 --- a/src/pages/users/BulkUserUpload/__snapshots__/UsersToUploadSummary.test.tsx.snap +++ b/src/pages/users/BulkUserUpload/__snapshots__/UsersToUploadSummary.test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Upload summary tests Upload summary pages for three valid users matches Snapshot 1`] = ` +exports[`Upload summary tests Upload summary pages for an imported users that already exist matches Snapshot 1`] = ` Object { "asFragment": [Function], "baseElement": @@ -10,10 +10,9 @@ Object { > Bulk upload - three + one user - s ?

- 3 + 1 of 3 users are valid and will be uploaded. @@ -66,9 +65,8 @@ Object { for="confirm-upload" > Yes, upload - three + one valid user - s

@@ -185,9 +183,9 @@ Object { class="ons-table__cell " > - Valid User + User already exists. @@ -198,20 +196,20 @@ Object { - Rob + Rich - DST + BDSS - Valid User + User already exists. @@ -222,12 +220,12 @@ Object { - Rich + Rob - BDSS + DST Bulk upload - three + one user - s ?

- 3 + 1 of 3 users are valid and will be uploaded. @@ -305,9 +302,8 @@ Object { for="confirm-upload" > Yes, upload - three + one valid user - s

@@ -424,9 +420,9 @@ Object { class="ons-table__cell " > - Valid User + User already exists. @@ -437,20 +433,20 @@ Object { - Rob + Rich - DST + BDSS - Valid User + User already exists. @@ -461,12 +457,12 @@ Object { - Rich + Rob - BDSS + DST @@ -545,9 +541,10 @@ Object { > Bulk upload - one + two user + s ?

- 1 + 2 of 3 users are valid and will be uploaded. @@ -600,8 +597,9 @@ Object { for="confirm-upload" > Yes, upload - one + two valid user + s

@@ -707,12 +705,12 @@ Object { - Jamie + Rob - BDSS + BOB - User exists multiple times. + Not a valid role. @@ -742,9 +740,9 @@ Object { class="ons-table__cell " > - User exists multiple times. + Valid User @@ -782,9 +780,10 @@ Object { > Bulk upload - one + two user + s ?

- 1 + 2 of 3 users are valid and will be uploaded. @@ -837,8 +836,9 @@ Object { for="confirm-upload" > Yes, upload - one + two valid user + s

@@ -944,12 +944,12 @@ Object { - Jamie + Rob - BDSS + BOB - User exists multiple times. + Not a valid role. @@ -979,9 +979,9 @@ Object { class="ons-table__cell " > - User exists multiple times. + Valid User @@ -1066,7 +1066,7 @@ Object { } `; -exports[`Upload summary tests Upload summary pages for two valid and one invalid users matches Snapshot 1`] = ` +exports[`Upload summary tests Upload summary pages for valid imported users matches Snapshot 1`] = ` Object { "asFragment": [Function], "baseElement": @@ -1076,7 +1076,7 @@ Object { > Bulk upload - two + three user s @@ -1090,7 +1090,7 @@ Object { class="ons-panel__body " >

- 2 + 3 of 3 users are valid and will be uploaded. @@ -1132,7 +1132,7 @@ Object { for="confirm-upload" > Yes, upload - two + three valid user s @@ -1240,20 +1240,20 @@ Object { - Rob + Jamie - BOB + BDSS - Not a valid role. + Valid User @@ -1264,12 +1264,12 @@ Object { - Jamie + Rob - BDSS + DST Bulk upload - two + three user s @@ -1329,7 +1329,7 @@ Object { class="ons-panel__body " >

- 2 + 3 of 3 users are valid and will be uploaded. @@ -1371,7 +1371,7 @@ Object { for="confirm-upload" > Yes, upload - two + three valid user s @@ -1479,20 +1479,20 @@ Object { - Rob + Jamie - BOB + BDSS - Not a valid role. + Valid User @@ -1503,12 +1503,12 @@ Object { - Jamie + Rob - BDSS + DST { +// set mocks +type getRolesListResponse = [boolean, UserRole[]]; +type getUsersListResponse = [boolean, User[]]; - const validRoles: UserRole[] = [ - { - name: "BDSS", - description: "", - permissions: [] - }, - { - name: "DST", - description: "", - permissions: [] - } - ]; +jest.mock( "../http"); - const userList: ImportUser[] = [ +const getAllRolesMock = getAllRoles as jest.Mock>; +const getAllUsersMock = getAllUsers as jest.Mock>; + +describe("Function validateImportedUsers", () => { + + const importedUsers: ImportUser[] = [ { name:"Jamie", password:"pass", role:"BDSS", - valid:true, + valid:false, warnings:[] }, { name:"Rob", password:"pass2", role:"DST", - valid:true, + valid:false, warnings:[] }, { - name:"Jamie", + name:"Rich", password:"pass", role:"BDSS", - valid:true, + valid:false, warnings:[] } ]; - it("It should mark the valid status to false if a user is in multiple times", async () => { + const roles: getRolesListResponse = [ + true, + [ + { + name: "BDSS", + description: "", + permissions: [] + }, + { + name: "DST", + description: "", + permissions: [] + } + ]]; + + const existingUsers: getUsersListResponse = [ + true, + [] + ]; + + beforeEach(() => { + getAllRolesMock.mockImplementation(() => Promise.resolve(roles)); + getAllUsersMock.mockImplementation(() => Promise.resolve(existingUsers)); + }); + + it("should mark the valid status to true if users are valid", async () => { // act - validateUsers(userList, validRoles); - console.debug(userList); + await validateImportedUsers(importedUsers); + // assert - expect(userList[0].valid).toBeFalsy(); - expect(userList[1].valid).toBeTruthy(); - expect(userList[2].valid).toBeFalsy(); + expect(importedUsers[0].valid).toBeTruthy(); + expect(importedUsers[1].valid).toBeTruthy(); + expect(importedUsers[2].valid).toBeTruthy(); }); - it("It should set an appropriate warning message if a user is in multiple times", async () => { + it("should not populate the warnings list if users are valid", async () => { // act - validateUsers(userList, validRoles); + await validateImportedUsers(importedUsers); // assert - expect(userList[0].warnings).toEqual(["User exists multiple times"]); - expect(userList[1].warnings).toEqual([]); - expect(userList[2].warnings).toEqual(["User exists multiple times"]); + expect(importedUsers[0].warnings).toEqual([]); + expect(importedUsers[1].warnings).toEqual([]); + expect(importedUsers[2].warnings).toEqual([]); + }); + + it("should mark the valid status to false if a user already exists", async () => { + // arrange + const matchingExistingUsers: getUsersListResponse = [ + true, + [{ + name:"Jamie", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }, + { + name:"Rich", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }] + ]; + + getAllUsersMock.mockImplementation(() => Promise.resolve(matchingExistingUsers)); + + // act + await validateImportedUsers(importedUsers); + + // assert + expect(importedUsers[0].valid).toBeFalsy(); + expect(importedUsers[1].valid).toBeTruthy(); + expect(importedUsers[2].valid).toBeFalsy(); + }); + + it("should set an appropriate warning message if a user is in multiple times", async () => { + // arrange + const matchingExistingUsers: getUsersListResponse = [ + true, + [{ + name:"Jamie", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }, + { + name:"Rich", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }] + ]; + + getAllUsersMock.mockImplementation(() => Promise.resolve(matchingExistingUsers)); + + // act + await validateImportedUsers(importedUsers); + + // assert + expect(importedUsers[0].warnings).toEqual(["User already exists"]); + expect(importedUsers[1].warnings).toEqual([]); + expect(importedUsers[2].warnings).toEqual(["User already exists"]); + }); + + afterAll(() => { + jest.clearAllMocks(); }); }); @@ -90,12 +174,85 @@ describe("Function validateUser", () => { }; // act - validateUser(validUser, validRoles); + validateUser(validUser, validRoles, []); // assert expect(validUser.valid).toBeTruthy(); }); + it("No warnings should be set if the user is valid", async () => { + // arrange + + const validUser: ImportUser = + { + name:"Jamie", + password:"pass", + role:"BDSS", + valid:false, // set to opposite of we want to ensure test intregrity + warnings:[] + }; + + // act + validateUser(validUser, validRoles, []); + + // assert + expect(validUser.warnings).toEqual([]); + }); + + it("The valid property should be set to false if the user already exists", async () => { + // arrange + + const invalidMatchingUser: ImportUser = + { + name:"Jamie", + password:"pass", + role:"BDSS", + valid:true, // set to opposite of we want to ensure test intregrity + warnings:[] + }; + + const matchingExistingUser: User[] = + [{ + name:"Jamie", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }]; + + // act + validateUser(invalidMatchingUser, validRoles, matchingExistingUser); + + // assert + expect(invalidMatchingUser.valid).toBeFalsy(); + }); + + it("The waring list should be set if the user already exists", async () => { + // arrange + + const invalidMatchingUser: ImportUser = + { + name:"Jamie", + password:"pass", + role:"BDSS", + valid:true, // set to opposite of we want to ensure test intregrity + warnings:[] + }; + + const matchingExistingUser: User[] = + [{ + name:"Jamie", + role:"BDSS", + serverParks:[], + defaultServerPark:"" + }]; + + // act + validateUser(invalidMatchingUser, validRoles, matchingExistingUser); + + // assert + expect(invalidMatchingUser.warnings).toEqual(["User already exists"]); + }); + it("The valid property should be set to false if the password is not supplied", async () => { // arrange @@ -109,9 +266,28 @@ describe("Function validateUser", () => { }; // act - validateUser(validUser, validRoles); + validateUser(validUser, validRoles, []); // assert expect(validUser.valid).toBeFalsy(); }); + + it("The waring list should be set if the password is not supplied", async () => { + // arrange + + const validUser: ImportUser = + { + name:"Jamie", + password:undefined, + role:"BDSS", + valid:true, // set to opposite of we want to ensure test intregrity + warnings:[] + }; + + // act + validateUser(validUser, validRoles, []); + + // assert + expect(validUser.warnings).toEqual(["Invalid password"]); + }); }); \ No newline at end of file diff --git a/src/utilities/validation/userValidation.ts b/src/utilities/validation/userValidation.ts index 17ded48..6943e2c 100644 --- a/src/utilities/validation/userValidation.ts +++ b/src/utilities/validation/userValidation.ts @@ -1,20 +1,44 @@ -import { UserRole } from "blaise-api-node-client"; +import { User, UserRole } from "blaise-api-node-client"; import { ImportUser } from "../../../Interfaces"; +import { getAllRoles, getAllUsers } from "../http"; + +async function getRoles(): Promise { + const [success, roles] = await getAllRoles(); + if (success) { + return roles; + } + + return []; +} + +async function getExistingUsers(): Promise { + const [success, users] = await getAllUsers(); + if (success) { + return users; + } + + return []; +} + +async function validateImportedUsers(users: ImportUser[]): Promise { + const validRoles = await getRoles(); + const existingUsers = await getExistingUsers(); -function validateUsers(users: ImportUser[], validRoles: UserRole[]): void { users.map((user) => { - validateUser(user, validRoles); - if (users.filter((u) => u.name===user.name).length > 1) { - user.valid = false; - user.warnings.push("User exists multiple times"); - } + validateUser(user, validRoles, existingUsers); + console.debug(user); }); } -function validateUser(user: ImportUser, validRoles: UserRole[]): void { +function validateUser(user: ImportUser, validRoles: UserRole[], existingUsers: User[]): void { user.valid = true; user.warnings = []; + if(existingUsers.find(existingUser => existingUser.name===user.name)) { + user.valid = false; + user.warnings.push("User already exists"); + } + if (user.name === undefined || user.name === null) { user.valid = false; user.warnings.push("Invalid name"); @@ -40,4 +64,4 @@ function validateUser(user: ImportUser, validRoles: UserRole[]): void { } } -export { validateUsers, validateUser }; \ No newline at end of file +export { validateImportedUsers, validateUser }; \ No newline at end of file