Refactored code based on code review comments

Andyg79 · Andyg79 · commit bd9c1d08e646 · 2025-03-20T14:09:25.000Z
diff --git a/pages/cognito_login_page.py b/pages/cognito_login_page.py
@@ -1,30 +1,25 @@
 import os
 
 from playwright.sync_api import Page
-from utils.user_tools import UserTools
-from dotenv import load_dotenv
 
 
-class BcssLoginPage:
+class CognitoLoginPage:
 
     def __init__(self, page: Page):
         self.page = page
-        self.page.goto("/")
         self.username = page.get_by_role("textbox", name="Username")
         self.password = page.get_by_role("textbox", name="Password")
         self.submit_button = page.get_by_role("button", name="submit")
-        load_dotenv()  # Take environment variables from .env
 
-    def login_as_user(self, username: str) -> None:
+    def login_as_user(self, username: str, password: str) -> None:
         """Logs in to bcss with specified user credentials
         Args:
             username (str) enter a username that exists in users.json
+            password (str) the password for the user provided
         """
         # Retrieve and enter username from users.json
-        user_details = UserTools.retrieve_user(username)
-        self.username.fill(user_details["username"])
+        self.username.fill(username)
         # Retrieve and enter password from .env file
-        password = os.getenv("BCSS_PASS")
         self.password.fill(password)
         # Click Submit
         self.submit_button.click()
diff --git a/tests/test_bowel_scope_page.py b/tests/test_bowel_scope_page.py
@@ -1,17 +1,16 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the bowel scope page
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the bowel scope page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to bowel scope page
     MainMenu(page).go_to_bowel_scope_page()
diff --git a/tests/test_call_and_recall_page.py b/tests/test_call_and_recall_page.py
@@ -1,17 +1,16 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the call and recall page
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the call and recall page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to call and recall page
     MainMenu(page).go_to_call_and_recall_page()
diff --git a/tests/test_communications_production_page.py b/tests/test_communications_production_page.py
@@ -1,17 +1,17 @@
 import pytest
 from playwright.sync_api import Page, expect
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 from pages.bcss_home_page import MainMenu
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the communications
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the communications
     production page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to communications production page
     MainMenu(page).go_to_communications_production_page()
diff --git a/tests/test_contacts_list_page.py b/tests/test_contacts_list_page.py
@@ -1,17 +1,16 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the contacts list page
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the contacts list page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to contacts list page
     MainMenu(page).go_to_contacts_list_page()
diff --git a/tests/test_download_page.py b/tests/test_download_page.py
@@ -1,17 +1,16 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the download page
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the download page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to download page
     MainMenu(page).go_to_download_page()
diff --git a/tests/test_fit_test_kits_page.py b/tests/test_fit_test_kits_page.py
@@ -1,18 +1,17 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the
     fit test kits page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to fit test kits page
     MainMenu(page).go_to_fit_test_kits_page()
diff --git a/tests/test_gfobt_test_kits_page.py b/tests/test_gfobt_test_kits_page.py
@@ -1,18 +1,17 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the
     gfob test kits page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to gFOBT test kits page
     MainMenu(page).go_to_gfob_test_kits_page()
diff --git a/tests/test_home_page_links.py b/tests/test_home_page_links.py
@@ -1,18 +1,18 @@
 import pytest
 from playwright.sync_api import Page, expect
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 from pages.bcss_home_page import BcssHomePage
 from utils.date_time_utils import DateTimeUtils
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and results in the home page
+    Before every test is executed, this fixture logs in to BCSS as the specified user and results in the home page
     being displayed
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
 
 @pytest.mark.smoke
diff --git a/tests/test_login_to_bcss.py b/tests/test_login_to_bcss.py
@@ -1,13 +1,13 @@
 from playwright.sync_api import Page, expect
-
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 def test_successful_login_to_bcss(page: Page) -> None:
     """
     Confirms that a user with valid credentials can log in to bcss
     """
     # Enter a valid username and password and click 'sign in' button
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
+
     # Confirm user has successfully signed in and is viewing the bcss homepage
     expect(page.locator("#ntshAppTitle")).to_contain_text("Bowel Cancer Screening System")
diff --git a/tests/test_lynch_surveillance_page.py b/tests/test_lynch_surveillance_page.py
@@ -1,18 +1,17 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the
     lynch surveillance page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to Lynch Surveillance page
     MainMenu(page).go_to_lynch_surveillance_page()
diff --git a/tests/test_organisations_page.py b/tests/test_organisations_page.py
@@ -1,18 +1,17 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the
     organisations page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to organisations page
     MainMenu(page).go_to_organisations_page()
diff --git a/tests/test_reports_page.py b/tests/test_reports_page.py
@@ -1,19 +1,18 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages import reports_page
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the
     reports page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Open reports page
     MainMenu(page).go_to_reports_page()
diff --git a/tests/test_screening_practitioner_appointments_page.py b/tests/test_screening_practitioner_appointments_page.py
@@ -1,18 +1,17 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the
     screening_practitioner_appointments page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to screening practitioner appointments page
     MainMenu(page).go_to_screening_practitioner_appointments_page()
diff --git a/tests/test_screening_subject_search_page.py b/tests/test_screening_subject_search_page.py
@@ -1,20 +1,19 @@
 import pytest
 from playwright.sync_api import Page, expect
-
 from pages.bcss_home_page import MainMenu
-from pages.login_page import BcssLoginPage
 from pages.screening_subject_search_page import ScreeningStatusSearchOptions, LatestEpisodeStatusSearchOptions, \
     SearchAreaSearchOptions
+from utils.user_tools import UserTools
 
 
 @pytest.fixture(scope="function", autouse=True)
 def before_each(page: Page):
     """
-    Before every test is executed, this fixture logs in to BCSS as a test user and navigates to the
+    Before every test is executed, this fixture logs in to BCSS as the specified user and navigates to the
     screening_subject_search page
     """
     # Log in to BCSS
-    BcssLoginPage(page).login_as_user("BCSS401")
+    UserTools.user_login(page, "Hub Manager State Registered")
 
     # Go to screening subject search page
     MainMenu(page).go_to_screening_subject_search_page()
diff --git a/users.json b/users.json
@@ -1,13 +1,13 @@
 {
   "_comment": "This file can be used to store data on users for your application, and then pulled through using the utils.user_tools UserTools utility. The documentation for this utility explains how this file is read.",
 
-  "BCSS401": {
+  "Hub Manager State Registered": {
     "username": "BCSS401",
     "roles": [
       "Hub Manager State Registered, Midlands and North West"
     ]
   },
-  "BCSS118": {
+  "Screening Centre Manager": {
     "username": "BCSS118",
     "roles": [
       "Screening Centre Manager for Wolverhampton, Midlands and North West"
diff --git a/utils/user_tools.py b/utils/user_tools.py
@@ -1,7 +1,11 @@
 import json
 import logging
+import os
 from pathlib import Path
+from flask.cli import load_dotenv
+from playwright.sync_api import Page
 
+from pages.cognito_login_page import CognitoLoginPage
 
 logger = logging.getLogger(__name__)
 USERS_FILE = Path(__file__).parent.parent / "users.json"
@@ -12,6 +16,24 @@ class UserTools:
     A utility class for retrieving and doing common actions with users.
     """
 
+    @staticmethod
+    def user_login(page: Page, username: str) -> None:
+        """
+        Logs into the BCSS application as a specified user.
+
+        Args:
+            page (playwright.sync_api.Page): The Playwright page object to interact with.
+            username (str): Enter a username that exists in the users.json file.
+        """
+        # Load dotenv to enable retrieval of a password from .env file
+        load_dotenv()
+        # Go to base url
+        page.goto("/")
+        # Retrieve username from users.json
+        user_details = UserTools.retrieve_user(username)
+        # Login to bcss using retrieved username and a password stored in the .env file
+        CognitoLoginPage(page).login_as_user(user_details["username"], os.getenv("BCSS_PASS"))
+
     @staticmethod
     def retrieve_user(user: str) -> dict:
         """
