Skip to content

Commit 91046fd

Browse files
Feature/bcss 22223 refactor investigation dataset util (#164)
<!-- markdownlint-disable-next-line first-line-heading --> ## Description <!-- Describe your changes in detail. --> Changing the logic around populating the investigation dataset form to be more inline with selenium and be easier to manage. Refactoring FOBT + Surveillance regression tests Refactoring Investigation dataset UI app to accommodate new changes Refactoring Subject assertion to give better error messages Fixing inconsistent KIT logging ## Context <!-- Why is this change required? What problem does it solve? --> Changing the logic around populating the investigation dataset form to be more inline with selenium and be easier to manage. Refactoring FOBT + Surveillance regression tests Refactoring Investigation dataset UI app to accommodate new changes Refactoring Subject assertion to give better error messages Fixing inconsistent KIT logging ## Type of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply. --> - [x] Refactoring (non-breaking change) - [x] 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.
1 parent 2a2a265 commit 91046fd

23 files changed

+2281
-1320
lines changed

classes/repositories/kit_service_management_repository.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import logging
2-
from typing import Optional
2+
from typing import Optional, Any
33
from utils.oracle.oracle import OracleDB
44
from classes.kits.kit_service_management_record import KitServiceManagementRecord
55
from classes.entities.kit_service_management_entity import KitServiceManagementEntity
6+
import numpy as np
7+
from decimal import Decimal
68

79

810
class KitServiceManagementRepository:
@@ -284,6 +286,22 @@ def update_kit_service_management_entity(
284286
if entity.put_attempts is not None:
285287
params["put_attempts"] = entity.put_attempts
286288

289+
params = {k: _sanitize_param(v) for k, v in params.items()}
287290
self.oracle_db.update_or_insert_data_to_table(sql_query, params)
288291
except Exception as ex:
289292
raise RuntimeError(f"Error updating KIT_QUEUE record: {ex}")
293+
294+
295+
def _sanitize_param(val: Any) -> Any:
296+
"""
297+
Sanitizes a parameter value for database operations.
298+
Args:
299+
val: The parameter value to sanitize.
300+
Returns:
301+
The sanitized parameter value.
302+
"""
303+
if isinstance(val, np.generic):
304+
return val.item()
305+
if isinstance(val, Decimal):
306+
return float(val)
307+
return val

docs/InvestigationDatasetBuilderApplication.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ Examples include `"yes_no"` and `"therapeutic_diagnostic"`, which are handled as
329329

330330
1. Choose a unique string for `"type"` (e.g., `"yes_no"`, `"therapeutic_diagnostic"`).
331331
2. In your JSON field definition, set `"type"` to this string.
332-
3. Ensure your `render_field` function in `investigation_dataset_ui.py` has a case for your custom type, rendering the appropriate widget (usually a dropdown/selectbox).
332+
3. Ensure your `render_field` function in `investigation_dataset_ui.py` has a case for your custom type, rendering the appropriate widget (dropdown, radio, etc.).
333333
- For `"yes_no"`, the UI will show a dropdown with "yes" and "no".
334334
- For `"therapeutic_diagnostic"`, the UI will show a dropdown with "therapeutic" and "diagnostic".
335335
4. You can add more custom types by extending the `render_field` function with new cases in the `match-case` or `if` dispatch.

investigation_dataset_ui.py

Lines changed: 105 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,23 @@ def show_section_with_imports(section_name: str) -> None:
546546
)
547547
else:
548548
import_block = ""
549-
st.code(f"{import_block}{section_name} = {pretty_dict(result)}", language="python")
549+
# Map section to correct fill method
550+
fill_methods = {
551+
"general_information": "fill_out_general_information",
552+
"drug_information": "fill_out_drug_information",
553+
"endoscopy_information": "fill_endoscopy_information",
554+
"completion_information": "fill_out_completion_information",
555+
"failure_information": "fill_out_failure_information",
556+
"radiology_information": "fill_out_radiology_information",
557+
"suspected_findings": "fill_out_suspected_findings",
558+
}
559+
method = fill_methods.get(section_name, f"fill_{section_name}")
560+
st.code(
561+
f"InvestigationDatasetCompletion(page).{method}({pretty_dict(result)})",
562+
language="python",
563+
)
564+
if import_block:
565+
st.code(import_block, language="python")
550566

551567

552568
def show_drug_group_section_with_imports(section_name: str) -> None:
@@ -569,14 +585,28 @@ def show_drug_group_section_with_imports(section_name: str) -> None:
569585
all_fields.extend(group["fields"])
570586
_render_drug_group(section_name, group, result)
571587

588+
# No special merging; each section is handled independently
589+
572590
enums = get_enums_used(all_fields)
573591
if enums:
574592
import_block = (
575593
enum_import_string + new_indented_line_string.join(sorted(enums)) + "\n)\n"
576594
)
577595
else:
578596
import_block = ""
579-
st.code(f"{import_block}{section_name} = {pretty_dict(result)}", language="python")
597+
# Map section to correct fill method
598+
fill_methods = {
599+
"drug_information": "fill_out_drug_information",
600+
"contrast_tagging_and_drug": "fill_out_contrast_tagging_and_drug_information",
601+
"tagging_agent_given_drug_information": "fill_out_tagging_agent_given_drug_information",
602+
}
603+
method = fill_methods.get(section_name, f"fill_{section_name}")
604+
st.code(
605+
f"InvestigationDatasetCompletion(page).{method}({pretty_dict(result)})",
606+
language="python",
607+
)
608+
if import_block:
609+
st.code(import_block, language="python")
580610

581611

582612
def _render_single_entry_fields(fields: list, result: dict) -> None:
@@ -638,6 +668,51 @@ def _render_drug_entry(fields: list, index: int, result: dict) -> None:
638668
result[dose_field["key"]] = ddose
639669

640670

671+
def _polyp_output_block(
672+
pi: int, info_dict: dict, intervention: object, hist_dict: dict
673+
) -> str:
674+
"""
675+
Generate the code block for a polyp's information, intervention(s), and histology.
676+
Args:
677+
pi (int): The polyp index (1-based).
678+
info_dict (dict): The polyp information dictionary.
679+
intervention (object): The polyp intervention(s), can be dict or list of dicts.
680+
hist_dict (dict): The polyp histology dictionary.
681+
Returns:
682+
str: The generated code block.
683+
"""
684+
code_lines = []
685+
code_lines.append(
686+
f"InvestigationDatasetCompletion(page).fill_polyp_x_information({pretty_dict(info_dict)}, {pi})"
687+
)
688+
# Intervention
689+
if isinstance(intervention, list):
690+
if len(intervention) > 1:
691+
code_lines.append(
692+
f"InvestigationDatasetCompletion(page).fill_polyp_x_multiple_interventions({pretty_list(intervention)}, {pi})"
693+
)
694+
elif len(intervention) == 1 and isinstance(intervention[0], dict):
695+
code_lines.append(
696+
f"InvestigationDatasetCompletion(page).fill_polyp_x_intervention({pretty_dict(intervention[0])}, {pi})"
697+
)
698+
else:
699+
code_lines.append(f"# No intervention for polyp {pi}")
700+
elif isinstance(intervention, dict):
701+
code_lines.append(
702+
f"InvestigationDatasetCompletion(page).fill_polyp_x_intervention({pretty_dict(intervention)}, {pi})"
703+
)
704+
else:
705+
code_lines.append(f"# No intervention for polyp {pi}")
706+
# Histology
707+
if not hist_dict:
708+
code_lines.append(f"# No histology for polyp {pi}")
709+
else:
710+
code_lines.append(
711+
f"InvestigationDatasetCompletion(page).fill_polyp_x_histology({pretty_dict(hist_dict)}, {pi})"
712+
)
713+
return "\n".join(code_lines)
714+
715+
641716
def show_polyp_information_and_intervention_and_histology() -> None:
642717
"""
643718
Show the Polyp Information, Intervention & Histology section, allowing multiple polyps and interventions.
@@ -652,39 +727,39 @@ def show_polyp_information_and_intervention_and_histology() -> None:
652727
# Collect all fields for import analysis
653728
all_fields = polyp_info_fields + polyp_intervention_fields + polyp_histology_fields
654729
enums = get_enums_used(all_fields)
655-
if enums:
656-
import_block = (
657-
enum_import_string + new_indented_line_string.join(sorted(enums)) + "\n)\n"
658-
)
659-
else:
660-
import_block = ""
730+
import_block = (
731+
enum_import_string + new_indented_line_string.join(sorted(enums)) + "\n)\n"
732+
if enums
733+
else ""
734+
)
661735

662736
num_polyps = st.number_input(
663737
"Number of polyps", min_value=0, max_value=20, value=1, step=1
664738
)
665-
polyp_information = []
666-
polyp_intervention = []
667-
polyp_histology = []
739+
polyp_info_dicts = {}
740+
polyp_histology_dicts = {}
741+
polyp_interventions_dicts = {}
668742

669743
for pi in range(1, num_polyps + 1):
670744
st.markdown(f"### Polyp {pi}")
671-
polyp_information.append(_render_polyp_info(polyp_info_fields, pi))
672-
polyp_intervention.append(_render_interventions(polyp_intervention_fields, pi))
673-
polyp_histology.append(_render_histology(polyp_histology_fields, pi))
745+
polyp_info_dicts[pi] = _render_polyp_info(polyp_info_fields, pi)
746+
interventions = _render_interventions(polyp_intervention_fields, pi)
747+
polyp_interventions_dicts[pi] = (
748+
interventions[0]
749+
if isinstance(interventions, list) and len(interventions) == 1
750+
else interventions
751+
)
752+
polyp_histology_dicts[pi] = _render_histology(polyp_histology_fields, pi)
674753

675754
st.markdown("#### Output")
676-
st.code(
677-
f"{import_block}polyp_information = {pretty_list(polyp_information)}",
678-
language="python",
679-
)
680-
st.code(
681-
f"polyp_intervention = {pretty_list(polyp_intervention)}",
682-
language="python",
683-
)
684-
st.code(
685-
f"polyp_histology = {pretty_list(polyp_histology)}",
686-
language="python",
687-
)
755+
for pi in range(1, num_polyps + 1):
756+
info_dict = polyp_info_dicts[pi]
757+
hist_dict = polyp_histology_dicts[pi]
758+
intervention = polyp_interventions_dicts[pi]
759+
code_block = _polyp_output_block(pi, info_dict, intervention, hist_dict)
760+
st.code(code_block, language="python")
761+
if import_block:
762+
st.code(import_block, language="python")
688763

689764

690765
def _render_polyp_info(fields: list, pi: int) -> dict:
@@ -718,7 +793,7 @@ def _render_interventions(fields: list, pi: int) -> list:
718793
f"Add interventions for polyp {pi}?", key=f"add_interventions_{pi}"
719794
)
720795
if not add_interventions:
721-
return interventions
796+
return []
722797
num_int = st.number_input(
723798
f"Number of interventions for polyp {pi}",
724799
min_value=0,
@@ -735,6 +810,8 @@ def _render_interventions(fields: list, pi: int) -> list:
735810
if val is not None:
736811
int_dict[field["key"]] = val
737812
interventions.append(int_dict)
813+
if len(interventions) == 1:
814+
return interventions[0]
738815
return interventions
739816

740817

@@ -799,7 +876,7 @@ def _render_histology(fields: list, pi: int) -> dict:
799876
"endoscopy_information": show_section_with_imports,
800877
"completion_information": show_section_with_imports,
801878
"failure_information": show_section_with_imports,
802-
"polyp_information_and_intervention_and_histology": lambda _: show_polyp_information_and_intervention_and_histology(), # If you want imports here, update similarly
879+
"polyp_information_and_intervention_and_histology": lambda _: show_polyp_information_and_intervention_and_histology(),
803880
"contrast_tagging_and_drug": show_drug_group_section_with_imports,
804881
"tagging_agent_given_drug_information": show_drug_group_section_with_imports,
805882
"radiology_information": show_section_with_imports,

pages/contacts_list/maintain_contacts_page.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,14 @@ def click_person_link_from_forename(self, forename: str) -> None:
6565
forename (str): The forename of the subject
6666
"""
6767
self.click(self.page.get_by_role("link", name=forename).last)
68+
69+
def select_person_by_id(self, person_id: int) -> None:
70+
"""
71+
Selects a person by their unique person ID by clicking the corresponding link.
72+
Requires a search to have been conducted and returned at least one result.
73+
Args:
74+
person_id (int): The unique ID of the person to select
75+
"""
76+
href_selector = f"a[href*='{person_id}']"
77+
person_link = self.page.locator(href_selector).first
78+
self.click(person_link)

pages/datasets/investigation_dataset_page.py

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ def __init__(self, page: Page):
9696
self.diagnostic_test_result = self.page.locator(
9797
"#datasetContent > div:nth-child(1) > div:nth-child(7) > span.userInput"
9898
)
99+
self.show_details_links = self.page.locator('a:has-text("Show details")')
99100

100101
# Repeat strings:
101102
self.bowel_preparation_administered_string = "Bowel Preparation Administered"
@@ -361,10 +362,13 @@ def select_diagnostic_procedure_type(self) -> None:
361362

362363
def click_show_completion_proof_information(self) -> None:
363364
"""
364-
This method is designed to click on the show completion proof information link.
365-
It clicks on the show completion proof information link.
365+
Clicks on the show completion proof information link if it contains the text "show" (case-insensitive).
366366
"""
367-
self.click(self.show_completion_proof_information_details)
367+
if (
368+
"show"
369+
in self.show_completion_proof_information_details.inner_text().lower()
370+
):
371+
self.click(self.show_completion_proof_information_details)
368372

369373
def click_show_failure_information(self) -> None:
370374
"""
@@ -415,6 +419,17 @@ def click_save_dataset_button(self) -> None:
415419
It clicks on the save dataset button.
416420
"""
417421
self.safe_accept_dialog(self.save_dataset_button)
422+
self.page.wait_for_timeout(3000) # 3 second timeout to allow page to update
423+
424+
def click_save_dataset_button_assert_dialog(self, expected_text: str) -> None:
425+
"""
426+
Clicks on the save dataset button and performs an assertion of the resulting dialog text.
427+
Once done it dismisses the dialog.
428+
Args:
429+
expected_text (str): The expected text in the resultant dialog
430+
"""
431+
self.assert_dialog_text(expected_text)
432+
self.click(self.save_dataset_button)
418433

419434
def expect_text_to_be_visible(self, text: str) -> None:
420435
"""
@@ -1193,6 +1208,24 @@ def assert_test_result(self, expected_text: str) -> None:
11931208
actual_text.lower() == expected_text.lower()
11941209
), f"Expected '{expected_text}', but found '{actual_text}'"
11951210

1211+
def open_all_details_tabs(self) -> None:
1212+
"""
1213+
Clicks all visible "Show details" links on the page to open all details tabs.
1214+
"""
1215+
count = self.show_details_links.count()
1216+
for i in range(count):
1217+
link = self.show_details_links.nth(i)
1218+
if link.is_visible():
1219+
self.click(link)
1220+
1221+
def open_all_minimized_sections(self) -> None:
1222+
"""
1223+
Opens all the minimized sections in the investigation dataset form.
1224+
"""
1225+
self.open_all_details_tabs()
1226+
# Then do it again to get the internal sections
1227+
self.open_all_details_tabs()
1228+
11961229

11971230
def normalize_label(text: str) -> str:
11981231
"""

0 commit comments

Comments
 (0)