Appt 491 mya user managent tests #456
Conversation
…/NHSDigital/nbs-appointments-management-service into APPT-491-MYA-User-Managent-Tests
| ] | ||
| } |
There was a problem hiding this comment.
Can I recommend you configure VS Code to auto-format files using Prettier so we don't get misaligned files like this?
If you open the workspace it's declared as a recommended extension so VS Code should prompt you to install it. Failing that you can install it manually through the extensions tab then set it as your default formatter (again if you open the workspace it'll set this for you). It'll then automatically format files whenever you save them so you never have to worry about this again :)
There was a problem hiding this comment.
We don't actually need this file, I think we created it by accident while resolving the merge. It already exists in this folder.
Confusingly, Dave has called this admin_test_user.json rather than zzz_test_uer_7.json (but it's still using test user 7's username/password in the playwright config). You can just delete this file.
Optionally if you want to rename admin_test_user.json to zzz_test_uer_7.json feel free
There was a problem hiding this comment.
Deleted zzz_test_uer_7.json extra file and renamed admin_test_user.json to zzz_test_uer_7.json
src/client/testing/fixtures.ts
Outdated
| @@ -13,3 +13,10 @@ export const test = baseTest.extend<object, { newUserName: string }>({ | |||
| { scope: 'worker' }, | |||
| ], | |||
| }); | |||
|
|
|||
| export const nonNhsEmailId = `int-test-user-${Math.floor(Math.random() * 10000)}@gmail.com`; | |||
There was a problem hiding this comment.
You don't need to create this yourself, we already have it as a method on the base test class.
You can use it in a test like so: test('test name', async ({ newUserName }) => {...
There was a problem hiding this comment.
I have define this one to generate non NHS email Id.
There was a problem hiding this comment.
Fair enough, but I still think we should be consistent and define it the same as the nhs username has been. The benefits of this are:
- We don't need to implement a random number generator ourselves
- We can rely on it being the same on each reference, but different between each test (the const will only be different each time it's imported)
- It's consistent with the way we're already doing this, rather than introducing a new pattern
| name: 'Assign Staff Roles', | ||
| }); | ||
| this.removeUserSuccessBanner = page.getByText( | ||
| 'You have successfully removed [email protected] from the current site.', |
There was a problem hiding this comment.
We should probably avoid hardcoding this in the page object as ideally methods in here should be re-usable.
We could make it a function which takes the email as a parameter, but to be honest I think this would be better off just asserted in the test itself
There was a problem hiding this comment.
Removed that locator as we already defined another function where we handling email dynamically.
eg: You have successfully removed ${userName} from the current site.,
| async signInWithRequiredUser(userName: string, password: string) { | ||
| await this.page.getByLabel('Username').fill(userName); | ||
| await this.page.getByLabel('Password').fill(password); | ||
| await this.page.getByLabel('Password').press('Enter'); | ||
| } |
There was a problem hiding this comment.
We already have this method in this file just above this one
There was a problem hiding this comment.
Designed second function to sign in with any user, earlier function was hardcoded with one user which is Test user1 going forward will make changes in code to use second function and will remove first one.
There was a problem hiding this comment.
That's incorrect, the existing function lets you sign in with any user you want. ser: { username: string; password: string } = env.TEST_USERS.testUser1 means that the parameters username and password are optional. If not provided, they will use test user 1 by default, but if they are provided they'll use the provided values instead.
There is an example of this in user-management-permissions.spec.ts => await oAuthPage.signIn(TEST_USERS.testUser2);
| // async veriyTileNotViisible(){ | ||
| // // await expect( | ||
| // // sitePage.viewAvailabilityAndManageAppointmentsCard, | ||
| // // ).toBeVisible(); | ||
| // // await expect(sitePage.createAvailabilityCard).not.toBeVisible(); | ||
| // // await expect(sitePage.userManagementCard).not.toBeVisible(); | ||
| // // await expect(sitePage.siteManagementCard).toBeVisible(); | ||
| // }; |
There was a problem hiding this comment.
Removed the commented code
…/NHSDigital/nbs-appointments-management-service into APPT-491-MYA-User-Managent-Tests
…/NHSDigital/nbs-appointments-management-service into APPT-491-MYA-User-Managent-Tests
azure-pipelines-bulk-data.yml
Outdated
There was a problem hiding this comment.
Did you intend to make these changes? Looks like this might have been a rebase mistake?
There was a problem hiding this comment.
I haven't changed anything there
There was a problem hiding this comment.
I haven't changed anything there
| @@ -182,7 +182,7 @@ public async Task RecalculateAppointmentStatuses(string site, DateOnly day) | |||
| continue; | |||
| } | |||
|
|
|||
| if (booking.Status is AppointmentStatus.Booked or AppointmentStatus.Provisional) | |||
| if (booking.Status is AppointmentStatus.Booked) | |||
There was a problem hiding this comment.
Is this change intentional or a rebase error?
There was a problem hiding this comment.
Its a rebase error
| @@ -518,7 +518,7 @@ public async Task RecalculateAppointmentStatuses_OrphansLiveAppointmentsIfTheyCa | |||
| From = new DateTime(2025, 01, 01, 9, 10, 0), | |||
| Reference = "2", | |||
| AttendeeDetails = new AttendeeDetails { FirstName = "Alexander", LastName = "Cooper" }, | |||
| Status = AppointmentStatus.Provisional, | |||
| Status = AppointmentStatus.Booked, | |||
There was a problem hiding this comment.
Is this change intentional or rebase error? surely this PR shouldn't be changing any Booking files?
There was a problem hiding this comment.
Its a rebase error
|
There was a problem hiding this comment.
Looks like we still have some rebase errors (this file for example). Can you go through the files changed and let me know when all the changes are ones you intend to make please?
There was a problem hiding this comment.
Below are files which got updated after Rebase
data/CosmosDbSeeder/items/dev/core_data/notification_config.json
src/api/Nhs.Appointments.Api/Notifications/template-bookingcancelled-rsv-email.txt
src/api/Nhs.Appointments.Api/Notifications/template-bookingcancelled-rsv-sms.txt
src/api/Nhs.Appointments.Api/Notifications/template-userroleschanged.txt
src/api/Nhs.Appointments.Core/BookingsService.cs
tests/Nhs.Appointments.Core.UnitTests/BookingsServiceTests.cs
worked on below files:
data/CosmosDbSeeder/items/local/core_data/zzz_test_user_10.json
data/CosmosDbSeeder/items/local/core_data/zzz_test_user_11.json
data/CosmosDbSeeder/items/local/core_data/zzz_test_user_8.json
data/CosmosDbSeeder/items/local/core_data/zzz_test_user_9.json
mock-oidc/users.json
src/client/testing/create-and-remove-user.spec.ts
src/client/testing/fixtures.ts
src/client/testing/page-objects/change-site-details-pages/site-details.ts
src/client/testing/page-objects/confirm-remove-user.ts
src/client/testing/page-objects/create-availability.ts
t/testing/page-objects/user-management.ts → ...-objects/manage-users/create-user-page.ts
src/client/testing/page-objects/manage-users/edit-manage-user-roles-page.ts
src/client/testing/page-objects/manage-users/remove-user-page.ts
src/client/testing/page-objects/manage-users/users-page.ts
src/client/testing/page-objects/root.ts
src/client/testing/page-objects/site.ts
src/client/testing/page-objects/users.ts
.../testing/page-objects/view-availability-appointment-pages/month-view-availability-page.ts
src/client/testing/playwright.env
src/client/testing/testEnvironment.ts
...lient/testing/create-availability.spec.ts → ...ability-tests/create-availability.spec.ts
src/client/testing/tests/manage-user-tests/create-user.spec.ts
src/client/testing/tests/manage-user-tests/edit-user.spec.ts
src/client/testing/tests/manage-user-tests/remove-user.spec.ts
src/client/testing/tests/manage-user-tests/user-management-permissions.spec.ts
There was a problem hiding this comment.
Rebase error
There was a problem hiding this comment.
Rebase error
There was a problem hiding this comment.
Rebase error
There was a problem hiding this comment.
Rebase error
There was a problem hiding this comment.
Rebase error
Added user management auto tests