Refactoring reports_page and test_reports_page (2nd Attempt) (#19)

<!-- markdownlint-disable-next-line first-line-heading -->
## Description
refactor reports pages and tests


<!-- Describe your changes in detail. -->
## Context
The reports pages have a lot of different locators that perform the same
function. I have removed locators and test data from the tests and added
them to the page object, and created a function for clicking on NHS
number links.


<!-- Why is this change required? What problem does it solve? -->
## Type of changes
The tests are generally cleaner and more readable. All of the tests can
now call the same function to click on an NHS number link.


<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply. -->

- [x] Refactoring (non-breaking change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would change existing
functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->

- [x] I am familiar with the [contributing
guidelines](https://github.com/nhs-england-tools/playwright-python-blueprint/blob/main/CONTRIBUTING.md)
- [x] I have followed the code style of the project
- [x] I have added tests to cover my changes (where appropriate)
- [x] I have updated the documentation accordingly
- [ ] This PR is a result of pair or mob programming

---

## Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others
privacy, we kindly ask you to NOT including [PII (Personal Identifiable
Information) / PID (Personal Identifiable
Data)](https://digital.nhs.uk/data-and-information/keeping-data-safe-and-benefitting-the-public)
or any other sensitive data in this PR (Pull Request) and the codebase
changes. We will remove any PR that do contain any sensitive
information. We really appreciate your cooperation in this matter.

- [x] I confirm that neither PII/PID nor sensitive data are included in
this PR and the codebase changes.

---------

Co-authored-by: adrianoaru-nhs <[email protected]>
Co-authored-by: victor-soares-ibm <[email protected]>
Co-authored-by: Adriano Aru <[email protected]>

Author: 3 people

pages/reports/reports_page.py

@@ -6,92 +6,131 @@ class ReportsPage(BasePage):
     def __init__(self, page):
         super().__init__(page)
         self.page = page
+
         # Reports page main menu links
-        self.bureau_reports_page = self.page.get_by_role("link", name="Bureau Reports")
-        self.failsafe_reports_page = self.page.get_by_role(
+        self.bureau_reports_link = self.page.get_by_text("Bureau Reports")
+        self.failsafe_reports_link = self.page.get_by_role(
             "link", name="Failsafe Reports"
         )
-        self.operational_reports_page = self.page.get_by_role(
+        self.operational_reports_link = self.page.get_by_role(
             "link", name="Operational Reports"
         )
-        self.strategic_reports_page = self.page.get_by_role(
+        self.strategic_reports_link = self.page.get_by_role(
             "link", name="Strategic Reports"
         )
-        self.cancer_waiting_times_reports_page = self.page.get_by_role(
+        self.cancer_waiting_times_reports_link = self.page.get_by_role(
             "link", name="Cancer Waiting Times Reports"
         )
-        self.dashboard = self.page.get_by_role("link", name="Dashboard")
-        self.qa_report_dataset_completion_page = self.page.get_by_role(
+        self.dashboard_link = self.page.get_by_role("link", name="Dashboard")
+        self.qa_report_dataset_completion_link = self.page.get_by_role(
             "link", name="QA Report : Dataset Completion"
         )
-        # Reports pages shared buttons & links
+
+        # Reports pages shared buttons, locators & links
         self.refresh_page_button = self.page.get_by_role("button", name="Refresh")
+        self.reports_update_button = self.page.get_by_role("button", name="Update")
+        self.report_start_date_field = self.page.get_by_role(
+            "textbox", name="Report Start Date"
+        )
+        self.qa_report_dataset_completion_link = self.page.get_by_text(
+            "QA Report : Dataset Completion"
+        )
+
+        # Generate Report button locators
         self.generate_report_button = self.page.get_by_role(
             "button", name="Generate Report"
         )
-        self.reports_update_button = self.page.get_by_role("button", name="Update")
-        self.report_timestamp_element = self.page.locator("b")
+        self.operational_reports_sp_appointments_generate_report_button = (
+            self.page.locator("#submitThisForm")
+        )
+
+        # Set patients screening centre dropdown locators
         self.set_patients_screening_centre_dropdown = self.page.locator(
             "#cboScreeningCentre"
         )
+        self.six_weeks_availability_not_set_up_set_patients_screening_centre_dropdown = self.page.get_by_label(
+            "Screening Centre"
+        )
+        self.practitioner_appointments_set_patients_screening_centre_dropdown = (
+            page.get_by_label("Screening Centre")
+        )
+        self.attendance_not_updated_set_patients_screening_centre_dropdown = (
+            page.get_by_label("Screening Centre")
+        )
+
+        # Select screening practitioner dropdown locators
+        self.screening_practitioner_dropdown = self.page.locator("#A_C_NURSE")
+
+        # Report Timestamp locators
+        self.common_report_timestamp_element = self.page.locator("b")
+        self.subject_ceased_report_timestamp_element = self.page.locator(
+            "#displayGenerateDate > tbody > tr > td > b"
+        )
+        self.fobt_logged_not_read_report_timestamp_element = self.page.locator(
+            "#report-generated"
+        )
+        self.six_weeks_availability_not_set_up_report_timestamp_element = (
+            self.page.locator("#displayGenerateDate")
+        )
 
         # Failsafe Reports menu links
-        self.date_report_last_requested_page = self.page.get_by_role(
+        self.date_report_last_requested_link = self.page.get_by_role(
             "link", name="Date Report Last Requested"
         )
-        self.screening_subjects_with_inactive_open_episode_link_page = (
-            self.page.get_by_role("link", name="Screening Subjects With")
+        self.screening_subjects_with_inactive_open_episode_link = self.page.get_by_role(
+            "link", name="Screening Subjects With"
         )
-        self.subjects_ceased_due_to_date_of_birth_changes_page = self.page.get_by_role(
+        self.subjects_ceased_due_to_date_of_birth_changes_link = self.page.get_by_role(
             "link", name="Subjects Ceased Due to Date"
         )
-        self.allocate_sc_for_patient_movements_within_hub_boundaries_page = (
+        self.allocate_sc_for_patient_movements_within_hub_boundaries_link = (
             self.page.get_by_role(
                 "link", name="Allocate SC for Patient Movements within Hub Boundaries"
             )
         )
-        self.allocate_sc_for_patient_movements_into_your_hub_page = (
+        self.allocate_sc_for_patient_movements_into_your_hub_link = (
             self.page.get_by_role(
                 "link", name="Allocate SC for Patient Movements into your Hub"
             )
         )
-        self.identify_and_link_new_gp_page = self.page.get_by_role(
+        self.identify_and_link_new_gp_link = self.page.get_by_role(
             "link", name="Identify and link new GP"
         )
+
         # Operational Reports menu links
-        self.appointment_attendance_not_updated_page = self.page.get_by_role(
+        self.appointment_attendance_not_updated_link = self.page.get_by_role(
             "link", name="Appointment Attendance Not"
         )
-        self.fobt_kits_logged_but_not_read_page = self.page.get_by_role(
+        self.fobt_kits_logged_but_not_read_link = self.page.get_by_role(
             "link", name="FOBT Kits Logged but Not Read"
         )
-        self.demographic_update_inconsistent_with_manual_update_page = (
+        self.demographic_update_inconsistent_with_manual_update_link = (
             self.page.get_by_role("link", name="Demographic Update")
         )
-        self.screening_practitioner_6_weeks_availability_not_set_up_report_page = (
+        self.screening_practitioner_6_weeks_availability_not_set_up_report_link = (
             page.get_by_role("link", name="Screening Practitioner 6")
         )
-        self.screening_practitioner_appointments_page = self.page.get_by_role(
+        self.screening_practitioner_appointments_link = self.page.get_by_role(
             "link", name="Screening Practitioner Appointments"
         )
 
-    # Reports page main menu links
+    # Reports page main menu navigation
     def go_to_failsafe_reports_page(self) -> None:
-        self.click(self.failsafe_reports_page)
+        self.click(self.failsafe_reports_link)
 
     def go_to_operational_reports_page(self) -> None:
-        self.click(self.operational_reports_page)
+        self.click(self.operational_reports_link)
 
     def go_to_strategic_reports_page(self) -> None:
-        self.click(self.strategic_reports_page)
+        self.click(self.strategic_reports_link)
 
     def go_to_cancer_waiting_times_reports_page(self) -> None:
-        self.click(self.cancer_waiting_times_reports_page)
+        self.click(self.cancer_waiting_times_reports_link)
 
     def go_to_dashboard(self) -> None:
-        self.click(self.dashboard)
+        self.click(self.dashboard_link)
 
-    # Reports pages shared buttons actions
+    # Reports pages shared buttons and actions
     def click_refresh_button(self) -> None:
         self.click(self.refresh_page_button)
 
@@ -103,41 +142,69 @@ def click_reports_pages_update_button(self) -> None:
 
     # Failsafe Reports menu links
     def go_to_date_report_last_requested_page(self) -> None:
-        self.click(self.date_report_last_requested_page)
+        self.click(self.date_report_last_requested_link)
 
     def go_to_screening_subjects_with_inactive_open_episode_link_page(self) -> None:
-        self.click(self.screening_subjects_with_inactive_open_episode_link_page)
+        self.click(self.screening_subjects_with_inactive_open_episode_link)
 
     def go_to_subjects_ceased_due_to_date_of_birth_changes_page(self) -> None:
-        self.click(self.subjects_ceased_due_to_date_of_birth_changes_page)
+        self.click(self.subjects_ceased_due_to_date_of_birth_changes_link)
 
     def go_to_allocate_sc_for_patient_movements_within_hub_boundaries_page(
         self,
     ) -> None:
-        self.click(self.allocate_sc_for_patient_movements_within_hub_boundaries_page)
+        self.click(self.allocate_sc_for_patient_movements_within_hub_boundaries_link)
 
     def go_to_allocate_sc_for_patient_movements_into_your_hub_page(self) -> None:
-        self.click(self.allocate_sc_for_patient_movements_into_your_hub_page)
+        self.click(self.allocate_sc_for_patient_movements_into_your_hub_link)
 
     def go_to_identify_and_link_new_gp_page(self) -> None:
-        self.click(self.identify_and_link_new_gp_page)
+        self.click(self.identify_and_link_new_gp_link)
 
     # Operational Reports menu links
     def go_to_appointment_attendance_not_updated_page(self) -> None:
-        self.click(self.appointment_attendance_not_updated_page)
+        self.click(self.appointment_attendance_not_updated_link)
 
     def go_to_fobt_kits_logged_but_not_read_page(self) -> None:
-        self.click(self.fobt_kits_logged_but_not_read_page)
+        self.click(self.fobt_kits_logged_but_not_read_link)
 
     def go_to_demographic_update_inconsistent_with_manual_update_page(self) -> None:
-        self.click(self.demographic_update_inconsistent_with_manual_update_page)
+        self.click(self.demographic_update_inconsistent_with_manual_update_link)
 
     def go_to_screening_practitioner_6_weeks_availability_not_set_up_report_page(
         self,
     ) -> None:
         self.click(
-            self.screening_practitioner_6_weeks_availability_not_set_up_report_page
+            self.screening_practitioner_6_weeks_availability_not_set_up_report_link
         )
 
     def go_to_screening_practitioner_appointments_page(self) -> None:
-        self.click(self.screening_practitioner_appointments_page)
+        self.click(self.screening_practitioner_appointments_link)
+
+    def click_nhs_number_link(self, page: Page) -> None:
+        """
+        Clicks the first NHS number link present on the screen if any are found.
+        """
+        # List of locators to check for NHS number links.
+        # This implementation is a workaround for the fact that the NHS number
+        # links are not using the same locators accross bcss
+        # This is a temporary solution until
+        # we have a table utility that will allow us to interact with tables across bcss.
+        locators = [
+            "#listReportDataTable > tbody > tr:nth-child(3) > td:nth-child(1) > a",
+            "//*[@id='listReportDataTable']/tbody/tr[3]/td[1]",
+            "//*[@id='listReportDataTable']/tbody/tr[3]/td[2]",
+            "#listReportDataTable > tbody > tr:nth-child(3) > td:nth-child(1) > a",
+            "#subjInactiveOpenEpisodes > tbody > tr:nth-child(1) > td.NHS_NUMBER.dt-type-numeric > a",
+        ]
+
+        for locator_string in locators:
+            try:
+                # Use page.locator to get a locator object
+                locator = page.locator(locator_string)
+                # Check if the locator is visible
+                if locator.is_visible():
+                    # Click the locator
+                    locator.click()
+            except Exception:
+                print("No NHS number links found on the page")