Skip to content

Commit 270d3de

Browse files
adrianoaru-nhsadrianoaru-nhs
committed
Addressing PR comments
1 parent 5954747 commit 270d3de

File tree

9 files changed

+143
-117
lines changed

9 files changed

+143
-117
lines changed

‎classes/data/data_creation.py‎

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from classes.person import Person
99
from classes.region_type import RegionType
1010
from classes.pi_subject import PISubject
11+
from utils.nhs_number_tools import NHSNumberTools
1112

1213

1314
class DataCreation:
@@ -110,7 +111,7 @@ def generate_random_subject(
110111
f"generateRandomSubject: {random_words_list}, {pi_reference}, {region}"
111112
)
112113
pi_subject = PISubject()
113-
pi_subject.set_nhs_number(self.generate_random_nhs_number())
114+
pi_subject.set_nhs_number(NHSNumberTools.generate_random_nhs_number())
114115
person = self.generate_random_person(random_words_list, GenderType.NOT_KNOWN)
115116
pi_subject.set_family_name(person.get_surname())
116117
pi_subject.set_first_given_names(person.get_forename())
@@ -147,47 +148,6 @@ def generate_random_subject(
147148
logging.info("generateRandomSubject: end")
148149
return pi_subject
149150

150-
def generate_random_nhs_number(self) -> str:
151-
"""
152-
Generates a random NHS number
153-
Returns:
154-
str: The generated NHS number
155-
"""
156-
logging.info("generateRandomNHSNumber: start")
157-
nhs_number_base = 900000000
158-
nhs_number_range = 100000000
159-
while True:
160-
nhs_number = str(
161-
nhs_number_base + self.rand.randint(0, nhs_number_range - 1)
162-
)
163-
if self.is_valid_nhs_number(nhs_number):
164-
break
165-
nhs_number += str(self.calculate_nhs_number_checksum(nhs_number))
166-
logging.info("generateRandomNHSNumber: end")
167-
return nhs_number
168-
169-
def is_valid_nhs_number(self, nhs_number: str) -> bool:
170-
"""
171-
Checks if the NHS number is valid
172-
Retuns:
173-
bool: True if it is valid, False if it is not
174-
"""
175-
return len(nhs_number) == 9 and nhs_number.isdigit()
176-
177-
def calculate_nhs_number_checksum(self, nhs_number: str) -> int:
178-
"""
179-
Calculates the NHS number checksum
180-
Returns:
181-
int: the checksum of the NHS number
182-
"""
183-
digits = [int(d) for d in nhs_number]
184-
total = sum((10 - i) * digits[i] for i in range(9))
185-
remainder = total % 11
186-
checksum = 11 - remainder
187-
if checksum == 11:
188-
checksum = 0
189-
return checksum
190-
191151
def generate_random_registration_code(self) -> str:
192152
"""
193153
Generates a random registraction code
@@ -205,7 +165,7 @@ def generate_random_gp_surgery(self, region: RegionType) -> Organisation | None:
205165
Args:
206166
region (RegionType): The region type (e.g., England)
207167
Returns:
208-
Organisation: The generated gp surgery
168+
Organisation: The generated gp surgery or None if the region type cannot be found
209169
"""
210170
logging.info(f"generateRandomGPSurgery: {region}")
211171
if region == RegionType.ENGLAND:

‎classes/invitation_plan.py‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from typing import Optional
22
from datetime import date, datetime
3+
from classes.invitation_plan_status_type import InvitationPlanStatusType
34

45

56
class InvitationPlan:
@@ -16,20 +17,32 @@ def __init__(
1617
invitations_due: Optional[int] = None,
1718
invitations_per_week: Optional[int] = None,
1819
invitations_per_day: Optional[int] = None,
19-
plan_status: Optional[
20-
str
21-
] = None, # Replace with InvitationPlanStatusType if available
20+
plan_status: Optional[InvitationPlanStatusType] = None,
2221
fip_note: Optional[str] = None,
2322
date_stamp: Optional[datetime] = None,
2423
) -> None:
24+
"""
25+
Initializes an InvitationPlan instance.
26+
Args:
27+
plan_id (Optional[int]): The plan ID.
28+
created_date (Optional[date]): The date the plan was created.
29+
start_date (Optional[date]): The start date of the plan.
30+
end_date (Optional[date]): The end date of the plan.
31+
invitations_due (Optional[int]): The total number of invitations due.
32+
invitations_per_week (Optional[int]): The number of invitations per week.
33+
invitations_per_day (Optional[int]): The number of invitations per day.
34+
plan_status (Optional[InvitationPlanStatusType]): The status of the plan.
35+
fip_note (Optional[str]): Any notes associated with the plan.
36+
date_stamp (Optional[datetime]): The timestamp of the last update.
37+
"""
2538
self.plan_id: Optional[int] = plan_id
2639
self.created_date: Optional[date] = created_date
2740
self.start_date: Optional[date] = start_date
2841
self.end_date: Optional[date] = end_date
2942
self.invitations_due: Optional[int] = invitations_due
3043
self.invitations_per_week: Optional[int] = invitations_per_week
3144
self.invitations_per_day: Optional[int] = invitations_per_day
32-
self.plan_status: Optional[str] = plan_status
45+
self.plan_status: Optional[InvitationPlanStatusType] = plan_status
3346
self.fip_note: Optional[str] = fip_note
3447
self.date_stamp: Optional[datetime] = date_stamp
3548

@@ -140,13 +153,13 @@ def set_invitations_per_day(self, invitations_per_day: int) -> None:
140153
"""
141154
self.invitations_per_day = invitations_per_day
142155

143-
def get_plan_status(self) -> Optional[str]:
156+
def get_plan_status(self) -> Optional[InvitationPlanStatusType]:
144157
"""
145158
Returns the plan status.
146159
"""
147160
return self.plan_status
148161

149-
def set_plan_status(self, plan_status: str) -> None:
162+
def set_plan_status(self, plan_status: InvitationPlanStatusType) -> None:
150163
"""
151164
Sets the plan status.
152165
"""

‎classes/organisation_complex.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
class Organisation:
22
"""
3-
Class representing an organisation with an organisation with id, name, and code.
3+
Class representing an organisation with id, name, and code.
44
"""
55

66
def __init__(self, new_id: int, new_name: str, new_code: str):
@@ -12,22 +12,22 @@ def get_name(self) -> str:
1212
"""Returns the organisation name"""
1313
return self.name
1414

15-
def set_name(self, name: str):
15+
def set_name(self, name: str) -> None:
1616
"""Sets the organisation name"""
1717
self.name = name
1818

1919
def get_id(self) -> int:
2020
"""Returns the organisation id"""
2121
return self.id
2222

23-
def set_id(self, id_: int):
23+
def set_id(self, id_: int) -> None:
2424
"""Sets the organisation id"""
2525
self.id = id_
2626

2727
def get_code(self) -> str:
2828
"""Returns the organisation code"""
2929
return self.code
3030

31-
def set_code(self, code: str):
31+
def set_code(self, code: str) -> None:
3232
"""Sets the organisation code"""
3333
self.code = code

‎classes/pi_subject.py‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,39 @@
1+
from dataclasses import dataclass, field
12
from typing import Optional
23
from datetime import date
34

45

6+
@dataclass
57
class PISubject:
68
"""
79
Represents a PI Subject with all relevant demographic and administrative fields.
810
"""
911

10-
def __init__(self) -> None:
11-
self.screening_subject_id: Optional[int] = None
12-
self.nhs_number: Optional[str] = None
13-
self.family_name: Optional[str] = None
14-
self.first_given_names: Optional[str] = None
15-
self.other_given_names: Optional[str] = None
16-
self.previous_family_name: Optional[str] = None
17-
self.name_prefix: Optional[str] = None
18-
self.birth_date: Optional[date] = None
19-
self.death_date: Optional[date] = None
20-
self.gender_code: Optional[int] = None
21-
self.address_line_1: Optional[str] = None
22-
self.address_line_2: Optional[str] = None
23-
self.address_line_3: Optional[str] = None
24-
self.address_line_4: Optional[str] = None
25-
self.address_line_5: Optional[str] = None
26-
self.postcode: Optional[str] = None
27-
self.gnc_code: Optional[str] = None
28-
self.gp_practice_code: Optional[str] = None
29-
self.nhais_deduction_reason: Optional[str] = None
30-
self.nhais_deduction_date: Optional[date] = None
31-
self.exeter_system: Optional[str] = None
32-
self.removed_to: Optional[str] = None
33-
self.pi_reference: Optional[str] = None
34-
self.superseded_by_nhs_number: Optional[str] = None
35-
self.replaced_nhs_number: Optional[str] = None
12+
screening_subject_id: Optional[int] = None
13+
nhs_number: Optional[str] = None
14+
family_name: Optional[str] = None
15+
first_given_names: Optional[str] = None
16+
other_given_names: Optional[str] = None
17+
previous_family_name: Optional[str] = None
18+
name_prefix: Optional[str] = None
19+
birth_date: Optional[date] = None
20+
death_date: Optional[date] = None
21+
gender_code: Optional[int] = None
22+
address_line_1: Optional[str] = None
23+
address_line_2: Optional[str] = None
24+
address_line_3: Optional[str] = None
25+
address_line_4: Optional[str] = None
26+
address_line_5: Optional[str] = None
27+
postcode: Optional[str] = None
28+
gnc_code: Optional[str] = None
29+
gp_practice_code: Optional[str] = None
30+
nhais_deduction_reason: Optional[str] = None
31+
nhais_deduction_date: Optional[date] = None
32+
exeter_system: Optional[str] = None
33+
removed_to: Optional[str] = None
34+
pi_reference: Optional[str] = None
35+
superseded_by_nhs_number: Optional[str] = None
36+
replaced_nhs_number: Optional[str] = None
3637

3738
def get_screening_subject_id(self) -> Optional[int]:
3839
"""Returns the screening subject ID."""

‎classes/region_type.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ def persistence_unit_name(self) -> Optional[str]:
3434
def get_region(cls, region: str) -> "RegionType":
3535
"""
3636
Factory method to get a RegionType instance by region string.
37+
Returns:
38+
RegionType: The corresponding RegionType instance. By default returns UNDEFINED.
3739
"""
3840
mapping = {
3941
"England": cls(*cls.ENGLAND),

‎classes/repositories/subject_repository.py‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def find_by_nhs_number(self, nhs_number: str) -> Optional[bool]:
2828
return None
2929
return True
3030

31-
def create_pi_subject(self, pio_id: int, pi_subject: "PISubject") -> Optional[int]:
31+
def create_pi_subject(self, pio_id: int, pi_subject: PISubject) -> Optional[int]:
3232
"""
3333
Creates a new screening subject, returning the contact id.
3434
@@ -54,7 +54,7 @@ def create_pi_subject(self, pio_id: int, pi_subject: "PISubject") -> Optional[in
5454

5555
return self.process_pi_subject(pio_id, pi_subject)
5656

57-
def process_pi_subject(self, pio_id: int, pi_subject: "PISubject") -> Optional[int]:
57+
def process_pi_subject(self, pio_id: int, pi_subject: PISubject) -> Optional[int]:
5858
"""
5959
Processes a PI subject using the PKG_SSPI.p_process_pi_subject stored procedure.
6060
@@ -98,7 +98,7 @@ def process_pi_subject(self, pio_id: int, pi_subject: "PISubject") -> Optional[i
9898

9999
return new_contact_id
100100

101-
def update_pi_subject(self, pi_subject: "PISubject") -> None:
101+
def update_pi_subject(self, pi_subject: PISubject) -> None:
102102
"""
103103
Updates an existing screening subject.
104104
@@ -134,14 +134,14 @@ def get_active_gp_practice_in_hub_and_sc(
134134
Returns:
135135
Optional[str]: GP practice org code.
136136
"""
137-
query = (
138-
"SELECT gp.org_code AS gp_code "
139-
"FROM org gp "
140-
"INNER JOIN gp_practice_current_links gpl ON gpl.gp_practice_id = gp.org_id "
141-
"INNER JOIN org hub ON hub.org_id = gpl.hub_id "
142-
"INNER JOIN org sc ON sc.org_id = gpl.sc_id "
143-
"WHERE hub.org_code = :hub_code AND sc.org_code = :sc_code"
144-
)
137+
query = """
138+
SELECT gp.org_code AS gp_code
139+
FROM org gp
140+
INNER JOIN gp_practice_current_links gpl ON gpl.gp_practice_id = gp.org_id
141+
INNER JOIN org hub ON hub.org_id = gpl.hub_id
142+
INNER JOIN org sc ON sc.org_id = gpl.sc_id
143+
WHERE hub.org_code = :hub_code AND sc.org_code = :sc_code
144+
"""
145145
df = self.oracle_db.execute_query(
146146
query, {"hub_code": hub_code, "sc_code": screening_centre_code}
147147
)
@@ -156,12 +156,12 @@ def get_inactive_gp_practice(self) -> Optional[str]:
156156
Returns:
157157
Optional[str]: GP practice org code.
158158
"""
159-
query = (
160-
"SELECT gp.org_code AS gp_code "
161-
"FROM org gp "
162-
"LEFT OUTER JOIN gp_practice_current_links gpl ON gpl.gp_practice_id = gp.org_id "
163-
"WHERE gp.org_type_id = 1009 AND gpl.gp_practice_id IS NULL"
164-
)
159+
query = """
160+
SELECT gp.org_code AS gp_code
161+
FROM org gp
162+
LEFT OUTER JOIN gp_practice_current_links gpl ON gpl.gp_practice_id = gp.org_id
163+
WHERE gp.org_type_id = 1009 AND gpl.gp_practice_id IS NULL
164+
"""
165165
df = self.oracle_db.execute_query(query)
166166
if df.empty:
167167
return None
@@ -177,15 +177,15 @@ def get_latest_gp_practice_for_subject(self, nhs_number: str) -> Optional[str]:
177177
Returns:
178178
Optional[str]: GP practice org code.
179179
"""
180-
query = (
181-
"SELECT gp.org_code AS gp_code "
182-
"FROM sd_contact_t c "
183-
"INNER JOIN subject_in_org sio ON sio.contact_id = c.contact_id "
184-
"INNER JOIN org gp ON gp.org_id = sio.org_id "
185-
"WHERE c.nhs_number = :nhs_number "
186-
"AND sio.org_type_id = 1009 "
187-
"AND sio.sio_id = (SELECT MAX(siox.sio_id) FROM subject_in_org siox WHERE siox.contact_id = c.contact_id AND siox.org_type_id = 1009)"
188-
)
180+
query = """
181+
SELECT gp.org_code AS gp_code
182+
FROM sd_contact_t c
183+
INNER JOIN subject_in_org sio ON sio.contact_id = c.contact_id
184+
INNER JOIN org gp ON gp.org_id = sio.org_id
185+
WHERE c.nhs_number = :nhs_number
186+
AND sio.org_type_id = 1009
187+
AND sio.sio_id = (SELECT MAX(siox.sio_id) FROM subject_in_org siox WHERE siox.contact_id = c.contact_id AND siox.org_type_id = 1009)
188+
"""
189189
df = self.oracle_db.execute_query(query, {"nhs_number": nhs_number})
190190
if df.empty:
191191
return None

‎classes/repositories/word_repository.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ def __init__(self):
1515
db_util: An object that provides the execute_query method as described.
1616
"""
1717
self.db_util = OracleDB()
18+
self.forename_weighting = [80, 95, 100]
19+
self.surname_weighting = [90, 97, 100]
20+
self.names_max_length = 35
1821

1922
def get_random_words_by_weighting(
2023
self, weightings: List[int], max_length: int
@@ -114,10 +117,18 @@ def get_random_subject_details(self) -> Dict[str, str]:
114117
logging.info("START: get_random_subject_details")
115118
details = {}
116119
# Weighted values
117-
details["forename"] = self.get_random_words_by_weighting([80, 95, 100], 35)
118-
details["forename2"] = self.get_random_words_by_weighting([80, 95, 100], 35)
119-
details["surname"] = self.get_random_words_by_weighting([90, 97, 100], 35)
120-
details["surname2"] = self.get_random_words_by_weighting([90, 97, 100], 35)
120+
details["forename"] = self.get_random_words_by_weighting(
121+
self.forename_weighting, self.names_max_length
122+
)
123+
details["forename2"] = self.get_random_words_by_weighting(
124+
self.forename_weighting, self.names_max_length
125+
)
126+
details["surname"] = self.get_random_words_by_weighting(
127+
self.surname_weighting, self.names_max_length
128+
)
129+
details["surname2"] = self.get_random_words_by_weighting(
130+
self.surname_weighting, self.names_max_length
131+
)
121132
# Unweighted values
122133
details["county"] = self.get_random_word()
123134
details["locality"] = self.get_random_word()

0 commit comments

Comments
 (0)