diff --git a/documentation/docs/developer/database.md b/documentation/docs/developer/database.md index fb5ec270..22e081f6 100644 --- a/documentation/docs/developer/database.md +++ b/documentation/docs/developer/database.md @@ -15,29 +15,27 @@ Database structure has few tables. The models are as follows: - **Patient**: There is one record for each child in the audit - **Visit**: There are multiple records for each patient. In NPDA, children are seen 4 times a year in clinic and have additional contacts. During these visits details regarding care processes and diabetes management are captured. -- **Site**: There is only one active site at any one time, but more than one site might be responsible for the care of a patient over the audit year. ### Reporting tables Tables also track the progress of each child through the audit, as well how they are scoring with regard to their key performance indicators (KPIs). These KPIs are aggregated periodically to feed reports on KPIs at different levels of abstractions (organisational, trust-level or health board, integrated care board, NHS England region and country level) -- **AuditProgress**: Has a one to one relationship with Registration. Stores information on how many fields in each form have been completed. - **KPI**: scores each individual child against the national KPI standards. It stores information on whether a given measure has been passed, failed, has yet to be completed, or whether the child in not eligible to be scored. - **KPIAggregation**: This base model stores results of aggregations of each measure. The base model is subclassed for models representing each geographical level of abstraction. Aggregations are run at scheduled intervals asynchronously and pulled into the dashboards. +- **OrganisationEmployer**: This model serves the middle model between NPDAUser and PaediatricDiabetesUnit. It tracks the number of organisations/PDUs a user is a member of and which is the primary organisation. +- **Submission**: This tracks all csv upload submissions and allocates them to audit years and quarters thereof. Only one active submission at a time can exist. New ones will overwrite the previous ones. +- **Transfer**: This tracks any transfers between paediatric diabetes units. It stores the reason for transferring and the date of transfer. It provide the middle table between Patient and Paediatric Diabetes Unit - **VisitActivity**: Stores user access/visit activity, including number of login attempts and ISP address as well as timestamp -### Link Tables - -There are some many to many relationships. Django normally handles this for you, but the development team chose to implement the link tables in these cases separately to be able to store information about the relationship between the tables. - -- **Site**: The relationships here are complicated since one child may have their diabetes care in different Organisations across a year, though only one centre can be active in the care of a child at any one time. Each Case therefore can have a many to many relationship with the Organisation trust model (since one Organisation can have multiple Cases and one Case can have multiple Organisations). The Site model therefore is a link model between the two. It is used in this way, rather than relying on the Django built-in many-to-many solution, because additional information relating to the organisation can be stored per Case, for example whether the site is actively involved in diabetes care. - ### Lookup Tables -These classes are used as look up tables throughout the NPDA application. They are seeded in the first migrations, either pulling content from the the ```constants``` folder, or from SNOMED CT. Note that the RCPCH NHS Organisation repository maintains the primary source list and these models are kept up to date against this periodically. +The RCPCH NHS Organisation repository maintains the primary source list and these models are kept up to date against this periodically. - **NPDAUser**: The User base model in Django is too basic for the requirements of NPDA and therefore a custom class has been created to describe the different users who either administer or deliver the audit, either on behalf of RCPCH, or the hospital trusts. +### Schema / ERD + +
#### Boundary files and geography extension pack diff --git a/documentation/docs/developer/organisations.md b/documentation/docs/developer/organisations.md index a1574555..e061f20c 100644 --- a/documentation/docs/developer/organisations.md +++ b/documentation/docs/developer/organisations.md @@ -25,10 +25,6 @@ These exist only in Wales and are both equivalent to Trust and ICB in England. O Paediatric Diabetes Units (PSUs) are usually Organisations or Trusts which submit audit data for children. PDUs have unique PZ codes. -### OPENUK Networks - -These are [networks](https://www.rcpch.ac.uk/resources/open-uk-organisation-paediatric-epilepsy-networks-uk) of NHS Health Boards and Trusts that provide care for children with epilepsies, organised regionally and overseen by a UK Working Group. Not all centres are members of an OPEN UK network. There are no boundary shapes to describe each region, but each one has its own identifier, and therefore there is an entity model to hold information on each OPEN UK network referenced by each organisation. - ### NHS England Regions There are 7 of these in England and their model is taken from NHS Digital. Each one has its own boundary code. ICBs fit neatly inside each one. diff --git a/documentation/docs/developer/testing.md b/documentation/docs/developer/testing.md new file mode 100644 index 00000000..eea8c708 --- /dev/null +++ b/documentation/docs/developer/testing.md @@ -0,0 +1,198 @@ +--- +title: Testing +reviewers: Dr Anchit Chandran +--- + +## Getting started + +Run all tests from the root directory: + +```shell +pytest +``` + +## Useful Pytest flags + +Some useful flags to improve the DX of testing. + +### Only re-run failing tests + +```shell +pytest -lf +``` + +### Run a specific test file, e.g. run all tests in only `test_npda_user_model_actions.py` + +```shell +pytest project/npda/tests/permissions_tests/test_npda_user_model_actions.py +``` + +Use `::` notation to run a specific test within a file e.g.: + +```shell +pytest project/npda/tests/permissions_tests/test_npda_user_model_actions.py::test_npda_user_list_view_rcpch_audit_team_can_view_all_users +``` + +### Start a completely clean run, clearing the cache (usually not required) + +```shell +pytest --cache-clear +``` + +### Run tests through keyword expression + +NOTE: this is sometimes slightly slower. + +```shell +pytest -k "MyClass and not method" +``` + +## `pytest.ini` config file + +Explanations and notes on our global `pytest.ini` configuration. + +### Creating new tests + +Test files are found as any `.py` prepended with `test_`: + +```ini +# SET FILENAME FORMATS OF TESTS +python_files = test_*.py +``` + +### Flags set for all `pytest` runs + +```init +# RE USE TEST DB AS DEFAULT +addopts = + --reuse-db + -k "not examples" +``` + +- `--reuse-db` allows a specified starting state testing database to be used between tests. All tests begin with this seeded starting state. The testing db is rolled back to the starting state after each state. + +## Test Database + +Every first test in a file should include the following fixtures to ensure the test database is correctly set up for when a particular test file is run independently: + +```python +@pytest.mark.django_db +def test_npda_user_list_view_users_can_only_see_users_from_their_pdu( + seed_users_fixture, + seed_groups_fixture, + seed_patients_fixture, +): +``` + +### `seed_users_fixture` + +The testing database should include `8 NPDAUsers`. + +At each of the `2` PDUs, GOSH and Alder Hey: + +```py title='seed_users.py' +GOSH_PZ_CODE = "PZ196" +ALDER_HEY_PZ_CODE = "PZ074" +``` + +The following `4` user types are seeded: + +```py title='seed_users.py' +users = [ + test_user_audit_centre_reader_data, + test_user_audit_centre_editor_data, + test_user_audit_centre_coordinator_data, + test_user_rcpch_audit_team_data, +] +``` + +### `seed_groups_fixture` + +Uses the `groups_seeder` to set `Groups` for `NPDAUsers`. + +### `seed_patients_fixture` + +*not yet implemented* + +## Factories + +Factories enable the intuitive and fast creation of testing instances, particularly in cases where multiple related models are required. + +Factories are set up to set sensible defaults wherever possible, which can be overridden through keyword arguments. + +Ideally, in all tests, if a model instance is being created, it should be done through its model Factory. + +### `NPDAUserFactory` + +Example usage below. + +NOTE: we do not need to manually create `OrganisationEmployer`s and `PaediatricsDiabetesUnit` with associations. + +Once an instance of `NPDAUserFactory` is created, the related models will also be created and assigned the relations. These are set using the `organisation_employers` kwarg, with the value being an array of `pz_codes` as strings. + +```py title="seed_users.py" +# GOSH User +new_user_gosh = NPDAUserFactory( + first_name=first_name, + role=user.role, + # Assign flags based on user role + is_active=is_active, + is_staff=is_staff, + is_rcpch_audit_team_member=is_rcpch_audit_team_member, + is_rcpch_staff=is_rcpch_staff, + groups=[user.group_name], + view_preference=( + VIEW_PREFERENCES[2][0] + if user.role == RCPCH_AUDIT_TEAM + else VIEW_PREFERENCES[0][0] + ), + organisation_employers=[GOSH_PZ_CODE], +) + +# Alder hey user +new_user_alder_hey = NPDAUserFactory( + first_name=first_name, + role=user.role, + # Assign flags based on user role + is_active=is_active, + is_staff=is_staff, + is_rcpch_audit_team_member=is_rcpch_audit_team_member, + is_rcpch_staff=is_rcpch_staff, + groups=[user.group_name], + organisation_employers=[ALDER_HEY_PZ_CODE], +) +``` + +### `PatientFactory` + +Once an instance of `PatientFactory` is created, a related `TransferFactory` instance is also generated with the associated `PaediatricsDiabetesUnitFactory` instance. + +### `PaediatricsDiabetesUnitFactory` + +Multiple parent factories require the instantiation of multiple `PaediatricsDiabetesUnitFactory`s. + +As there is a composite unique constraint set on the [`pz_code`, `ods_code`] attributes, we do not want to create multiple instances with duplicate values; instead, we want to mimic the Django ORM's `.get_or_create()` method. + +This is emulated using an override on this factory's `._create()` method: + +```py title="paediatrics_diabetes_unit_factory.py" +@classmethod +def _create(cls, model_class, *args, **kwargs): + """ + Custom create method to handle get_or_create logic for PaediatricsDiabetesUnit. + + Each PDU has a composite unique constraint for pz_code and ods_code. This mimics + a get or create operation every time a new PDUFactory instance is created. + """ + pz_code = kwargs.pop("pz_code", None) + ods_code = kwargs.pop("ods_code", None) + + if pz_code and ods_code: + pdu, created = PaediatricDiabetesUnit.objects.get_or_create( + pz_code=pz_code, + ods_code=ods_code, + ) + return pdu + + return super()._create(model_class, *args, **kwargs) +``` \ No newline at end of file diff --git a/documentation/mkdocs.yml b/documentation/mkdocs.yml index b9b256a9..bac186e1 100644 --- a/documentation/mkdocs.yml +++ b/documentation/mkdocs.yml @@ -146,3 +146,4 @@ nav: - 'developer/manual-setup.md' - 'developer/organisations.md' - 'developer/users.md' + - 'developer/testing.md' diff --git a/project/constants/pz_codes.py b/project/constants/pz_codes.py index 15181ef8..c4ab2ce9 100644 --- a/project/constants/pz_codes.py +++ b/project/constants/pz_codes.py @@ -1,3 +1,16 @@ +from dataclasses import dataclass +from enum import Enum + +@dataclass +class PZCode: + pz_code: str + name: str + +class DummyPZCodes(Enum): + RCPCH = PZCode(pz_code="PZ999", name="RCPCH") + NOT_FOUND = PZCode(pz_code="PZ998", name="NOT_FOUND") + + pzs = [ "RM102", "RXF05", diff --git a/project/npda/admin.py b/project/npda/admin.py index 4a347118..3a1d450d 100644 --- a/project/npda/admin.py +++ b/project/npda/admin.py @@ -1,3 +1,4 @@ +from django.apps import apps from django.contrib import admin from .models import ( @@ -11,6 +12,8 @@ ) from django.contrib.sessions.models import Session +PaediatricDiabetesUnit = apps.get_model("npda", "PaediatricDiabetesUnit") + @admin.register(OrganisationEmployer) class OrganisationEmployerAdmin(admin.ModelAdmin): @@ -27,6 +30,11 @@ class PatientAdmin(admin.ModelAdmin): search_fields = ("nhs_number_icontains", "pk") +@admin.register(PaediatricDiabetesUnit) +class PaediatricDiabetesUnitAdmin(admin.ModelAdmin): + search_fields = ("pk", "ods_code", "pz_code") + + @admin.register(Transfer) class TransferAdmin(admin.ModelAdmin): search_fields = ("paediatric_diabetes_unit", "patient", "pk") diff --git a/project/npda/forms/npda_user_form.py b/project/npda/forms/npda_user_form.py index 449ed8c8..fede9428 100644 --- a/project/npda/forms/npda_user_form.py +++ b/project/npda/forms/npda_user_form.py @@ -2,6 +2,7 @@ import logging # django imports +from django.apps import apps from django.conf import settings from django.contrib.auth.forms import SetPasswordForm, AuthenticationForm from django import forms @@ -15,7 +16,7 @@ # RCPCH imports from ...constants.styles.form_styles import * -from ..models import NPDAUser, OrganisationEmployer +from ..models import NPDAUser from project.npda.general_functions import ( organisations_adapter, ) @@ -27,14 +28,15 @@ class NPDAUserForm(forms.ModelForm): - use_required_attribute = False add_employer = forms.ChoiceField( choices=[], # Initially empty, will be populated dynamically - required=True, + required=False, widget=forms.Select(attrs={"class": SELECT}), label="Add Employer", ) + organisation_choices = [] + class Meta: model = NPDAUser fields = [ @@ -47,6 +49,7 @@ class Meta: "is_rcpch_audit_team_member", "is_rcpch_staff", "role", + "add_employer", ] widgets = { "title": forms.Select(attrs={"class": SELECT}), @@ -60,9 +63,19 @@ class Meta: ), "is_rcpch_staff": forms.CheckboxInput(attrs={"class": "accent-rcpch_pink"}), "role": forms.Select(attrs={"class": SELECT}), + "add_employer": forms.Select( + attrs={ + "class": SELECT, + "required": False, + "name": "add_employer", + "id": "id_add_employer", + } + ), } def __init__(self, *args, **kwargs) -> None: + PaediatricDiabetesUnit = apps.get_model("npda", "PaediatricDiabetesUnit") + OrganisationEmployer = apps.get_model("npda", "OrganisationEmployer") # get the request object from the kwargs self.request = kwargs.pop("request", None) @@ -73,43 +86,64 @@ def __init__(self, *args, **kwargs) -> None: self.fields["surname"].required = True self.fields["email"].required = True self.fields["role"].required = True - self.fields["add_employer"].required = True - - if self.request: - if ( - self.request.user.is_superuser - or self.request.user.is_rcpch_audit_team_member - or self.request.user.is_rcpch_staff - ): - # populate the add_employer field with organisations that the user is not already affiliated with - self.fields["add_employer"].choices = ( - (item[0], item[1]) - for item in organisations_adapter.get_all_nhs_organisations_affiliated_with_paediatric_diabetes_unit() - if item[0] - not in OrganisationEmployer.objects.filter( - npda_user=self.instance - ).values_list("paediatric_diabetes_unit__ods_code", flat=True) - ) - else: - pz_code = self.request.session.get("pz_code") - sibling_organisations = ( - organisations_adapter.get_single_pdu_from_pz_code( - pz_number=pz_code - ).organisations + self.fields["add_employer"].required = False + + # only if the form is bound - this user is being updated + if self.instance.pk is not None: + if self.request.GET: + # populate the add_employer choices with organisations that the user is not already affiliated with, based on user permissions + # this is only necessary if the form is being instantiated to serve to the user, not on form submission + # populating the choices includes an API call and this takes time, so we only want to do it when necessary + self.organisation_choices = ( + organisations_adapter.organisations_to_populate_select_field( + request=self.request, user_instance=self.instance + ) ) - # filter out organisations that the user is already affiliated with - self.fields["add_employer"].choices = [ - (org.ods_code, org.name) - for org in sibling_organisations - if org.ods_code - not in OrganisationEmployer.objects.filter( - npda_user=self.instance - ).values_list("paediatric_diabetes_unit__ods_code", flat=True) - ] - - # set the default value to the current user's organisation - self.fields["add_employer"].initial = self.request.session.get("ods_code") + # this is a bit of a hack but necessary due to htmx. + # The add_employer field is not part of the model, so we need to remove it from the data dictionary + # in a bound form on form submission, so that the form will validate correctly + # the add employer work flow happens via htmx and not form submission + self.data = self.data.copy() + self.data.pop("add_employer", None) + + else: + # this means the form is unbound and this user is being created - therefore need the organisation_choices to be populated with all organisations + # but only on form instantiation, not on form submission + # populating the choices includes an API call and this takes time, so we only want to do it when necessary + if self.request.GET: + self.organisation_choices = ( + organisations_adapter.organisations_to_populate_select_field( + request=self.request, user_instance=None + ) + ) + else: + # this is a POST request - the form is being submitted + # we need to remove the selection from the add_employer field and use this to create the employer relationship + # but only if the form is valid + ods_code = self.data.get("add_employer") + # remove the add_employer field from the data dictionary + # as it is immutable we need to copy it first + self.data = self.data.copy() + self.data.pop("add_employer", None) + + if ods_code and self.is_valid(): + # the form is filled out correctly and the user has selected an employer + # we need to create the employer relationship - since this is a new user, + # this organisation will be the user's primary employer + pdu_object = organisations_adapter.get_single_pdu_from_ods_code( + ods_code + ) + pdu, created = PaediatricDiabetesUnit.objects.update_or_create( + ods_code=ods_code, pz_code=pdu_object["pz_code"] + ) + npda_user = self.instance + npda_user.save() + OrganisationEmployer.objects.update_or_create( + npda_user=npda_user, + paediatric_diabetes_unit=pdu, + is_primary_employer=True, + ) class NPDAUpdatePasswordForm(SetPasswordForm): diff --git a/project/npda/forms/patient_form.py b/project/npda/forms/patient_form.py index f79d8b29..bb8a7c81 100644 --- a/project/npda/forms/patient_form.py +++ b/project/npda/forms/patient_form.py @@ -1,10 +1,18 @@ -from django import forms +# python imports +from datetime import date + +# django imports +from django.apps import apps from django.core.exceptions import ValidationError +from django import forms + +# project imports from ..models import Patient from ...constants.styles.form_styles import * from ..general_functions import ( validate_postcode, - gp_practice_for_postcode + gp_practice_for_postcode, + retrieve_quarter_for_date, ) @@ -14,6 +22,18 @@ class DateInput(forms.DateInput): class PatientForm(forms.ModelForm): + quarter = forms.ChoiceField( + choices=[ + (1, 1), + (2, 2), + (3, 3), + (4, 4), + ], # Initially empty, will be populated dynamically + required=True, + widget=forms.Select(attrs={"class": SELECT}), + label="Audit Year Quarter", + ) + class Meta: model = Patient fields = [ @@ -27,6 +47,7 @@ class Meta: "death_date", "gp_practice_ods_code", "gp_practice_postcode", + "quarter", ] widgets = { "nhs_number": forms.TextInput( @@ -41,8 +62,23 @@ class Meta: "death_date": DateInput(), "gp_practice_ods_code": forms.TextInput(attrs={"class": TEXT_INPUT}), "gp_practice_postcode": forms.TextInput(attrs={"class": TEXT_INPUT}), + "quarter": forms.Select(), } + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + if self.instance.pk: + # this is a bound form, so we need to get the quarter from the submission related to the instance + Submission = apps.get_model("npda", "Submission") + self.initial["quarter"] = Submission.objects.get( + patients=self.instance + ).quarter + else: + # this is an unbound form, so we need to set the quarter to current quarter + self.initial["quarter"] = retrieve_quarter_for_date( + date_instance=date.today() + ) + def clean_postcode(self): if not self.cleaned_data["postcode"]: raise ValidationError("This field is required") @@ -97,27 +133,27 @@ def clean(self): ] } ) - + if gp_practice_ods_code is None and gp_practice_postcode is None: - raise ValidationError({ - "gp_practice_ods_code": [ - "GP Practice ODS code and GP Practice postcode cannot both be empty. At least one must be supplied." - ] - }) - + raise ValidationError( + { + "gp_practice_ods_code": [ + "GP Practice ODS code and GP Practice postcode cannot both be empty. At least one must be supplied." + ] + } + ) + if not gp_practice_ods_code and gp_practice_postcode: try: ods_code = gp_practice_for_postcode(gp_practice_postcode) if not ods_code: - raise ValidationError("Could not find GP practice with that postcode") + raise ValidationError( + "Could not find GP practice with that postcode" + ) else: cleaned_data["gp_practice_ods_code"] = ods_code except Exception as error: - raise ValidationError({ - "gp_practice_postcode": [ - error - ] - }) + raise ValidationError({"gp_practice_postcode": [error]}) return cleaned_data diff --git a/project/npda/general_functions/__init__.py b/project/npda/general_functions/__init__.py index ba3bb35c..7ad8fc45 100644 --- a/project/npda/general_functions/__init__.py +++ b/project/npda/general_functions/__init__.py @@ -4,6 +4,7 @@ from .group_for_group import * from .index_multiple_deprivation import * from .nhs_ods_requests import * +from .organisations_adapter import * from .pdus import * from .rcpch_nhs_organisations import * from .time_elapsed import * @@ -12,4 +13,4 @@ from .visit_categories import * from .model_utils import * from .session import * -from .view_preference import * \ No newline at end of file +from .view_preference import * diff --git a/project/npda/general_functions/model_utils.py b/project/npda/general_functions/model_utils.py index b2f56922..37442c13 100644 --- a/project/npda/general_functions/model_utils.py +++ b/project/npda/general_functions/model_utils.py @@ -30,4 +30,4 @@ def print_instance_field_attrs(instance): except AttributeError: fields_dict[field_name] = "" - logger.info(f"{model.__name__} instance:\n{pformat(fields_dict, indent=2)}") + logger.info(f"%s instance:\n%s", {model.__name__}, pformat(fields_dict, indent=2)) diff --git a/project/npda/general_functions/organisations_adapter.py b/project/npda/general_functions/organisations_adapter.py index 28fc656c..adc8592b 100644 --- a/project/npda/general_functions/organisations_adapter.py +++ b/project/npda/general_functions/organisations_adapter.py @@ -1,4 +1,5 @@ # python imports +from django.apps import apps from .rcpch_nhs_organisations import ( get_nhs_organisation, get_all_nhs_organisations, @@ -18,3 +19,66 @@ # Logging logger = logging.getLogger(__name__) + + +def organisations_to_populate_select_field(request, user_instance=None): + """ + This function is used to populate the add_employer field with organisations that the user_instance is NOT already affiliated with, based on request user permissions. + If no user_instance is provided (as the user form is being created), the function will populate the add_employer field with all organisations that the request user has access to. + """ + + OrganisationEmployer = apps.get_model("npda", "OrganisationEmployer") + + if user_instance: + if ( + request.user.is_superuser + or request.user.is_rcpch_audit_team_member + or request.user.is_rcpch_staff + ): + # populate the add_employer field with organisations that the user is not already affiliated with + organisation_choices = ( + (item[0], item[1]) + for item in get_all_nhs_organisations_affiliated_with_paediatric_diabetes_unit() + if item[0] + not in OrganisationEmployer.objects.filter( + npda_user=user_instance + ).values_list("paediatric_diabetes_unit__ods_code", flat=True) + ) + else: + pz_code = request.session.get("pz_code") + sibling_organisations = get_single_pdu_from_pz_code( + pz_number=pz_code + ).organisations + + # filter out organisations that the user is already affiliated with + organisation_choices = [ + (org.ods_code, org.name) + for org in sibling_organisations + if org.ods_code + not in OrganisationEmployer.objects.filter( + npda_user=user_instance + ).values_list("paediatric_diabetes_unit__ods_code", flat=True) + ] + else: + # this user is being created - therefore need the organisation_choices to be populated with all organisations based on requesting user permissions + if ( + request.user.is_superuser + or request.user.is_rcpch_audit_team_member + or request.user.is_rcpch_staff + ): + # return all organisations that are associated with a paediatric diabetes unit + organisation_choices = ( + (item[0], item[1]) + for item in get_all_nhs_organisations_affiliated_with_paediatric_diabetes_unit() + ) + else: + # return all organisations that are associated with the same paediatric diabetes unit as the request user + pz_code = request.session.get("pz_code") + sibling_organisations = get_single_pdu_from_pz_code( + pz_number=pz_code + ).organisations + + organisation_choices = [ + (org.ods_code, org.name) for org in sibling_organisations + ] + return organisation_choices diff --git a/project/npda/general_functions/pdus.py b/project/npda/general_functions/pdus.py index 270f36de..c1603068 100644 --- a/project/npda/general_functions/pdus.py +++ b/project/npda/general_functions/pdus.py @@ -108,7 +108,6 @@ def get_single_pdu_from_pz_code(pz_number: str) -> Union[PDUWithOrganisations, d """ url = settings.RCPCH_NHS_ORGANISATIONS_API_URL request_url = f"{url}/paediatric_diabetes_units/organisations/?pz_code={pz_number}" - try: response = requests.get(request_url, timeout=10) # times out after 10 seconds response.raise_for_status() @@ -126,13 +125,16 @@ def get_single_pdu_from_pz_code(pz_number: str) -> Union[PDUWithOrganisations, d logger.error(f"An error occurred: {err}") # Return an error value in the same format - return {"error": f"{pz_number=} not found"} + return PDUWithOrganisations( + pz_code="error", + organisations=[OrganisationODSAndName(ods_code="error", name="PDUs not found")], + ) # TODO MRB: this should return dataclasses too def get_single_pdu_from_ods_code( ods_code: str, -) -> Union[PDUWithOrganisations, Dict[str, str]]: +) -> PDUWithOrganisations: """ Fetches a specific Paediatric Diabetes Unit (PDU) with its associated organisations from the RCPCH NHS Organisations API using the ODS code. @@ -140,8 +142,7 @@ def get_single_pdu_from_ods_code( ods_code (str): The ODS code of the Paediatric Diabetes Unit. Returns: - Union[Dict, Dict[str, str]]: A dictionary with the PDU details if the request is successful, - or a dictionary indicating an error. + list[PDUWithOrganisations]: A list of PDUWithOrganisations objects. Error values if PDUs are not found. """ # Ensure the ODS code is uppercase ods_code = ods_code.upper() @@ -153,11 +154,20 @@ def get_single_pdu_from_ods_code( response = requests.get(request_url, timeout=10) # times out after 10 seconds response.raise_for_status() data = response.json()[0] - return data + return PDUWithOrganisations( + pz_code=data["pz_code"], + organisations=[ + OrganisationODSAndName(name=org["name"], ods_code=org["ods_code"]) + for org in data["organisations"] + ], + ) except HTTPError as http_err: logger.error(f"HTTP error occurred: {http_err.response.text}") except Exception as err: - logger.error(f"An error occurred: {err}") + logger.error(f"An error occurred: {err}\n{ods_code=}{response.json()=}") # Return an error value in the same format - return {"error": f"ODS code {ods_code} not found"} + return PDUWithOrganisations( + pz_code="error", + organisations=[OrganisationODSAndName(ods_code="error", name="PDUs not found")], + ) diff --git a/project/npda/general_functions/session.py b/project/npda/general_functions/session.py index acedf9b5..5fe9ee5b 100644 --- a/project/npda/general_functions/session.py +++ b/project/npda/general_functions/session.py @@ -20,8 +20,8 @@ def create_session_object(user): sibling_organisations = get_single_pdu_from_ods_code(ods_code) organisation_choices = [ - (choice["ods_code"], choice["name"]) - for choice in sibling_organisations["organisations"] + (choice.ods_code, choice.name) + for choice in sibling_organisations.organisations ] can_see_all_pdus = user.is_superuser or user.is_rcpch_audit_team_member diff --git a/project/npda/general_functions/view_preference.py b/project/npda/general_functions/view_preference.py index 058520e2..c7e1e028 100644 --- a/project/npda/general_functions/view_preference.py +++ b/project/npda/general_functions/view_preference.py @@ -7,6 +7,7 @@ def get_or_update_view_preference(user, new_view_preference): new_view_preference = int(new_view_preference) if new_view_preference else None + NPDAUser = apps.get_model("npda", "NPDAUser") if new_view_preference == 2 and not user.is_rcpch_audit_team_member: # national logger.warning( @@ -14,7 +15,6 @@ def get_or_update_view_preference(user, new_view_preference): ) raise PermissionDenied() elif new_view_preference: - NPDAUser = apps.get_model("npda", "NPDAUser") user = NPDAUser.objects.get(pk=user.pk) user.view_preference = new_view_preference user.save(update_fields=["view_preference"]) diff --git a/project/npda/migrations/0001_initial.py b/project/npda/migrations/0001_initial.py index 3b70b458..c40df753 100644 --- a/project/npda/migrations/0001_initial.py +++ b/project/npda/migrations/0001_initial.py @@ -421,22 +421,6 @@ class Migration(migrations.Migration): verbose_name="Quarter", ), ), - ( - "pz_code", - models.CharField( - help_text="The PZ code of the Paediatric Diabetes Unit", - max_length=10, - verbose_name="PZ code", - ), - ), - ( - "ods_code", - models.CharField( - help_text="The ODS code of the Organisation", - max_length=10, - verbose_name="PZ code", - ), - ), ( "submission_date", models.DateTimeField( @@ -452,6 +436,14 @@ class Migration(migrations.Migration): verbose_name="Submission active", ), ), + ( + "paediatric_diabetes_unit", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="pdu_submissions", + to="npda.paediatricdiabetesunit", + ), + ), ( "patients", models.ManyToManyField( diff --git a/project/npda/models/organisation_employer.py b/project/npda/models/organisation_employer.py index c0cab298..e0382ad5 100644 --- a/project/npda/models/organisation_employer.py +++ b/project/npda/models/organisation_employer.py @@ -1,4 +1,3 @@ -from typing import Iterable from django.contrib.gis.db.models import Model, BooleanField, CASCADE, ForeignKey from django.utils.translation import gettext_lazy as _ diff --git a/project/npda/models/submission.py b/project/npda/models/submission.py index 273ca206..e0843e98 100644 --- a/project/npda/models/submission.py +++ b/project/npda/models/submission.py @@ -28,22 +28,6 @@ class Submission(models.Model): help_text="The quarter in the audit year of the patient", ) - pz_code = models.CharField( - "PZ code", - max_length=10, - blank=False, - null=False, - help_text="The PZ code of the Paediatric Diabetes Unit", - ) - - ods_code = models.CharField( - "PZ code", - max_length=10, - blank=False, - null=False, - help_text="The ODS code of the Organisation", - ) - submission_date = models.DateTimeField( "Submission date", help_text="Date the submission was created", @@ -62,14 +46,15 @@ class Submission(models.Model): patients = models.ManyToManyField(to="npda.Patient", related_name="submissions") + paediatric_diabetes_unit = models.ForeignKey( + on_delete=models.CASCADE, + to="npda.PaediatricDiabetesUnit", + related_name="pdu_submissions", + ) + def __str__(self) -> str: return f"{self.audit_year} ({self.quarter}), {self.patients.count()} patients" - def save(self, *args, **kwargs) -> None: - self.audit_year = int(self.submission_date.year) - self.quarter = retrieve_quarter_for_date(self.submission_date.date()) - super().save(*args, **kwargs) - class Meta: verbose_name = "Submission" verbose_name_plural = "Submissions" diff --git a/project/npda/templates/npda/npdauser_form.html b/project/npda/templates/npda/npdauser_form.html index 0ab60ca7..7188e225 100644 --- a/project/npda/templates/npda/npdauser_form.html +++ b/project/npda/templates/npda/npdauser_form.html @@ -7,13 +7,17 @@
{% csrf_token %} {% for field in form %} -
{% if field.id_for_label == 'id_add_employer' %} {% if npda_user.organisation_employers %} Employer(s)
- {% include 'partials/employers.html' with organisation_employers=organisation_employers %} + {% include 'partials/employers.html' with organisation_employers=organisation_employers organisation_choices=organisation_choices %} +
+ {% else %} + Employer(s) +
+ {% include 'partials/rcpch_organisation_select.html' with organisation_choices=organisation_choices selected_organisation=field.value hidden_on_load=False %}
{% endif %} {% endif %} @@ -21,19 +25,25 @@
- + {% if field.id_for_label != 'id_add_employer' %} + + + {% endif %}
{% if field.field.widget|is_select %} - {% for choice in field.field.choices %} - {% if not show_rcpch_team and choice.1 == "RCPCH Audit Team" %} - - {% else %} - - {% endif %} + {% if not show_rcpch_team and choice.1 == "RCPCH Audit Team" %} + + {% else %} + + {% endif %} {% endfor %} + {% endif %} {% elif field.field.widget|is_emailfield %} {{ field }} {% elif field.field.widget|is_textinput %} @@ -56,7 +66,7 @@
{% endfor %} - + Back to list {% if form_method == 'update' %} diff --git a/project/npda/templates/npda_users.html b/project/npda/templates/npda_users.html index fca1eed4..3fb55678 100644 --- a/project/npda/templates/npda_users.html +++ b/project/npda/templates/npda_users.html @@ -4,6 +4,7 @@
+

NPDA Users at {{chosen_pdu}}

{% url 'npda_users' as hx_post %}
{% include 'partials/view_preference.html' with view_preference=request.user.view_preference hx_post=hx_post organisation_choices=organisation_choices ods_code_select_name="npdauser_ods_code_select_name" pz_code_select_name="npdauser_pz_code_select_name" ods_code=ods_code pdu_choices=pdu_choices chosen_pdu=chosen_pdu hx_target="#npdauser_view_preference" %} diff --git a/project/npda/templates/partials/employers.html b/project/npda/templates/partials/employers.html index 5a5d1a7d..67760e05 100644 --- a/project/npda/templates/partials/employers.html +++ b/project/npda/templates/partials/employers.html @@ -3,9 +3,33 @@
{{ organisation_employer.paediatric_diabetes_unit }} Primary Employer
{% else %}
- {{ organisation_employer.paediatric_diabetes_unit }} + {{ organisation_employer.paediatric_diabetes_unit }}
{% endif %} -{% endfor %} \ No newline at end of file +{% endfor %} +
+
+ +
+ +
+ {% include 'partials/rcpch_organisation_select.html' with organisation_choices=organisation_choices selected_organisation=field.value hidden_on_load=True %} + + +
+
\ No newline at end of file diff --git a/project/npda/templates/partials/npda_user_table.html b/project/npda/templates/partials/npda_user_table.html index 0b2a3e08..5fd62303 100644 --- a/project/npda/templates/partials/npda_user_table.html +++ b/project/npda/templates/partials/npda_user_table.html @@ -1,6 +1,6 @@ {% load npda_tags %} {% if npdauser_list %} - +
diff --git a/project/npda/templates/partials/patient_table.html b/project/npda/templates/partials/patient_table.html index 9a86b1cf..4df16c36 100644 --- a/project/npda/templates/partials/patient_table.html +++ b/project/npda/templates/partials/patient_table.html @@ -1,19 +1,19 @@ {% load npda_tags %} {% if patient_list %} -
NPDA UserID
- +
+ - - - - - - - - - - - + + + + + + + + + + + diff --git a/project/npda/templates/partials/rcpch_organisation_select.html b/project/npda/templates/partials/rcpch_organisation_select.html new file mode 100644 index 00000000..f1288445 --- /dev/null +++ b/project/npda/templates/partials/rcpch_organisation_select.html @@ -0,0 +1,5 @@ + \ No newline at end of file diff --git a/project/npda/templates/patients.html b/project/npda/templates/patients.html index fc210a14..0b70bff7 100644 --- a/project/npda/templates/patients.html +++ b/project/npda/templates/patients.html @@ -20,9 +20,5 @@

Patients {% endif %} -
-
Patient Upload Summary
-

This will contain a table summary of uploaded cases, maybe with ticks against those deemed valid, and crosses against those that failed with a list of invalid fields

-
{% endblock %} \ No newline at end of file diff --git a/project/npda/templates/submissions_list.html b/project/npda/templates/submissions_list.html index 9363622c..33457d69 100644 --- a/project/npda/templates/submissions_list.html +++ b/project/npda/templates/submissions_list.html @@ -1,12 +1,12 @@ {% extends "base.html" %} {% load static %} {% block content %} -
+
- All Submissions for {{pz_code}} - {% include 'partials/submission_history.html' with submissions=submissions %} +

All Submissions for {{pz_code}}

+ {% include 'partials/submission_history.html' with submissions=object_list %}
diff --git a/project/npda/tests/conftest.py b/project/npda/tests/conftest.py index 1ff2c4b4..460a7290 100644 --- a/project/npda/tests/conftest.py +++ b/project/npda/tests/conftest.py @@ -17,8 +17,10 @@ PatientFactory, PatientVisitFactory, NPDAUserFactory, + OrganisationEmployerFactory, + PaediatricsDiabetesUnitFactory, + TransferFactory, ) -from project.npda.models import Patient # register factories to be used across test directory @@ -27,3 +29,6 @@ register(PatientFactory) # => patient_factory register(PatientVisitFactory) # => patient_visit_factory register(NPDAUserFactory) # => npdauser_factory +register(OrganisationEmployerFactory) # => npdauser_factory +register(PaediatricsDiabetesUnitFactory) # => npdauser_factory +register(TransferFactory) # => npdauser_factory diff --git a/project/npda/tests/factories/NPDAUserFactory.py b/project/npda/tests/factories/NPDAUserFactory.py deleted file mode 100644 index 95af1a17..00000000 --- a/project/npda/tests/factories/NPDAUserFactory.py +++ /dev/null @@ -1,59 +0,0 @@ -"""Factory function to create new NPDAUser.""" - -# Standard imports -import factory - -# Project imports -from project.npda.models import NPDAUser, OrganisationEmployer -from project.npda.general_functions.rcpch_nhs_organisations import get_nhs_organisation - - -class NPDAUserFactory(factory.django.DjangoModelFactory): - """Factory for creating a minimum viable NPDAUser.""" - - class Meta: - model = NPDAUser - skip_postgeneration_save = True - - email = factory.Sequence(lambda n: f"npda_test_user_{n}@nhs.net") - first_name = "Mandel" - surname = "Brot" - is_active = True - is_superuser = False - email_confirmed = True - - @factory.post_generation - def groups(self, create, extracted, **kwargs): - if not create: - return - - # Set a default password - self.set_password("pw") - - # Add the extracted groups if provided - if extracted: - for group in extracted: - self.groups.add(group) - - self.save() - - - @factory.post_generation - def organisation_employers(self, create, extracted, **kwargs): - if not create: - return - - # Add the extracted org ods_codes if provided - if extracted: - orgs = [] - for ods_code in extracted: - org_obj = get_nhs_organisation(ods_code=ods_code) - org, created = OrganisationEmployer.objects.get_or_create( - name=org_obj.name, - ods_code=org_obj.ods_code, - pz_code=org_obj.paediatric_diabetes_unit.pz_code, - ) - orgs.append(org) - self.organisation_employers.set(orgs) - - self.save() diff --git a/project/npda/tests/factories/PatientFactory.py b/project/npda/tests/factories/PatientFactory.py index bd6a539c..a9961e24 100644 --- a/project/npda/tests/factories/PatientFactory.py +++ b/project/npda/tests/factories/PatientFactory.py @@ -10,8 +10,8 @@ import nhs_number # rcpch imports -from project.npda.models import Patient, Site -from .SiteFactory import SiteFactory +from project.npda.models import Patient +from .TransferFactory import TransferFactory from project.constants import ( ETHNICITIES, DIABETES_TYPES, @@ -34,6 +34,7 @@ class PatientFactory(factory.django.DjangoModelFactory): class Meta: model = Patient + skip_postgeneration_save=True @factory.lazy_attribute def nhs_number(self): @@ -65,6 +66,8 @@ def nhs_number(self): gp_practice_ods_code = "RP401" - site = factory.SubFactory( - SiteFactory, + # Once a Patient is created, we must create a Transfer object + transfer = factory.RelatedFactory( + TransferFactory, + factory_related_name='patient' ) diff --git a/project/npda/tests/factories/SiteFactory.py b/project/npda/tests/factories/SiteFactory.py deleted file mode 100644 index 3cc09c66..00000000 --- a/project/npda/tests/factories/SiteFactory.py +++ /dev/null @@ -1,25 +0,0 @@ -"""Factory fn to create new Visit, related to a Patient. -""" - -# standard imports - -# third-party imports -import factory - -# rcpch imports -from project.npda.models import Site - - -class SiteFactory(factory.django.DjangoModelFactory): - """Dependency factory for creating a minimum viable NPDAManagement. - - This Factory is generated AFTER a Patient has been generated. - """ - - class Meta: - model = Site - - - - paediatric_diabetes_unit_pz_code = 'PZ130' - organisation_ods_code = 'RP426' diff --git a/project/npda/tests/factories/TransferFactory.py b/project/npda/tests/factories/TransferFactory.py new file mode 100644 index 00000000..f0c1c5f7 --- /dev/null +++ b/project/npda/tests/factories/TransferFactory.py @@ -0,0 +1,28 @@ +"""Factory fn to create new Visit, related to a Patient. +""" + +# standard imports +from datetime import date +from django.apps import apps + +# third-party imports +import factory + +# rcpch imports +from project.npda.models import Transfer +from project.npda.tests.factories import PaediatricsDiabetesUnitFactory + + +class TransferFactory(factory.django.DjangoModelFactory): + """Dependency factory for creating a minimum viable NPDA Patient. + + This Factory is generated AFTER a Patient has been generated. + """ + + class Meta: + model = Transfer + + # Relationships + paediatric_diabetes_unit = factory.SubFactory(PaediatricsDiabetesUnitFactory) + # Once a PatientFactory instance is created, it will attach here + patient = None diff --git a/project/npda/tests/factories/__init__.py b/project/npda/tests/factories/__init__.py index a57500cc..f4e53d68 100644 --- a/project/npda/tests/factories/__init__.py +++ b/project/npda/tests/factories/__init__.py @@ -1,7 +1,9 @@ from .seed_groups_permissions import * from .seed_patients import * from .seed_users import * -from .NPDAUserFactory import * +from .npda_user_factory import * from .PatientFactory import * from .PatientVisitFactory import * -from .SiteFactory import * +from .TransferFactory import * +from .paediatrics_diabetes_unit_factory import * +from .organisation_employer_factory import * \ No newline at end of file diff --git a/project/npda/tests/factories/npda_user_factory.py b/project/npda/tests/factories/npda_user_factory.py new file mode 100644 index 00000000..6b14b9ff --- /dev/null +++ b/project/npda/tests/factories/npda_user_factory.py @@ -0,0 +1,94 @@ +"""Factory function to create new NPDAUser.""" + +# Standard imports +import factory +import logging + +# Project imports +from project.npda.general_functions.pdus import ( + PDUWithOrganisations, + get_single_pdu_from_pz_code, +) +from project.npda.models import NPDAUser +from project.npda.tests.factories.organisation_employer_factory import ( + OrganisationEmployerFactory, +) +from project.npda.tests.factories.paediatrics_diabetes_unit_factory import ( + PaediatricsDiabetesUnitFactory, +) + +# Logging +logger = logging.getLogger(__name__) + + +class NPDAUserFactory(factory.django.DjangoModelFactory): + """Factory for creating a minimum viable NPDAUser. + + Additionally, takes in a `organisation_employers` list of pz_codes to associate with the user. + Associated PDU's are created if they do not exist in the db. + """ + + class Meta: + model = NPDAUser + skip_postgeneration_save = True + + email = factory.Sequence(lambda n: f"npda_test_user_{n}@nhs.net") + first_name = "Mandel" + surname = "Brot" + is_active = True + is_superuser = False + email_confirmed = True + + @factory.post_generation + def groups(self, create, extracted, **kwargs): + if not create: + return + + # Set a default password + self.set_password("pw") + + # Add the extracted groups if provided + if extracted: + for group in extracted: + self.groups.add(group) + + self.save() + + @factory.post_generation + def organisation_employers(self, create, extracted, **kwargs): + if not create: + logger.info("Not creating OrganisationEmployer instances.") + return + + # Initialize the list to hold PaediatricDiabetesUnit instances + pdus = [] + + # If no pz_codes are provided, create a default PaediatricsDiabetesUnit and OrganisationEmployer + if not extracted: + default_pdu = PaediatricsDiabetesUnitFactory() + OrganisationEmployerFactory.create( + npda_user=self, paediatric_diabetes_unit=default_pdu + ) + pdus.append(default_pdu) + else: + # If pz_codes are provided, create OrganisationEmployer for each pz_code + for pz_code in extracted: + pdu_data = get_single_pdu_from_pz_code(pz_number=pz_code) + + # Raise error if not a PDU object + if not isinstance(pdu_data, PDUWithOrganisations): + raise ValueError(f"Invalid PDU object {pdu_data=}") + + pdu = PaediatricsDiabetesUnitFactory( + pz_code=pz_code, + ods_code=pdu_data.organisations[0].ods_code, + ) + + OrganisationEmployerFactory.create( + npda_user=self, paediatric_diabetes_unit=pdu + ) + pdus.append(pdu) + + # Set the organisation_employers field with the created PaediatricsDiabetesUnit instances + self.organisation_employers.set(pdus) + self.save() diff --git a/project/npda/tests/factories/organisation_employer_factory.py b/project/npda/tests/factories/organisation_employer_factory.py new file mode 100644 index 00000000..95b26022 --- /dev/null +++ b/project/npda/tests/factories/organisation_employer_factory.py @@ -0,0 +1,30 @@ +"""Factory function to create new OrganisationEmployerFactory.""" + +# Standard imports +import factory +import logging + +# Project imports +from project.npda.models import OrganisationEmployer + +# Logging +logger = logging.getLogger(__name__) + +class OrganisationEmployerFactory(factory.django.DjangoModelFactory): + """Dependency for creating a minimum viable NPDAUser. + + Organisation Employer will to PaediatricsDiabetesUnitFactory Default. + """ + + class Meta: + model = OrganisationEmployer + skip_postgeneration_save = True + + # Use a local import to avoid circular dependency issues + paediatric_diabetes_unit = factory.SubFactory( + "project.npda.tests.factories.PaediatricsDiabetesUnitFactory" + ) + + # Once an NPDAUser is created, it will attach to this attribute + npda_user = None + diff --git a/project/npda/tests/factories/paediatrics_diabetes_unit_factory.py b/project/npda/tests/factories/paediatrics_diabetes_unit_factory.py new file mode 100644 index 00000000..6c38b0a8 --- /dev/null +++ b/project/npda/tests/factories/paediatrics_diabetes_unit_factory.py @@ -0,0 +1,45 @@ +"""Factory fn to create new Visit, related to a Patient. +""" + +# standard imports +from django.apps import apps + +# third-party imports +import factory + +# rcpch imports +from project.npda.models.paediatric_diabetes_unit import PaediatricDiabetesUnit + + +class PaediatricsDiabetesUnitFactory(factory.django.DjangoModelFactory): + """PDU Factory. + + Defaults to Chelsea Westminster Hospital. + """ + + class Meta: + model = PaediatricDiabetesUnit + + # Chelsea Westminster Hospital default + pz_code = "PZ130" + ods_code = "RQM01" + + @classmethod + def _create(cls, model_class, *args, **kwargs): + """ + Custom create method to handle get_or_create logic for PaediatricsDiabetesUnit. + + Each PDU has a composite unique constraint for pz_code and ods_code. This mimics + a get or create operation every time a new PDUFactory instance is created. + """ + pz_code = kwargs.pop("pz_code", None) + ods_code = kwargs.pop("ods_code", None) + + if pz_code and ods_code: + pdu, created = PaediatricDiabetesUnit.objects.get_or_create( + pz_code=pz_code, + ods_code=ods_code, + ) + return pdu + + return super()._create(model_class, *args, **kwargs) diff --git a/project/npda/tests/factories/seed_patients.py b/project/npda/tests/factories/seed_patients.py index d47014f6..9dfb590a 100644 --- a/project/npda/tests/factories/seed_patients.py +++ b/project/npda/tests/factories/seed_patients.py @@ -9,7 +9,6 @@ # NPDA Imports from project.npda.models import Patient -from .PatientFactory import PatientFactory @pytest.fixture(scope="session") diff --git a/project/npda/tests/factories/seed_users.py b/project/npda/tests/factories/seed_users.py index dcd93539..ad18bb7a 100644 --- a/project/npda/tests/factories/seed_users.py +++ b/project/npda/tests/factories/seed_users.py @@ -3,9 +3,10 @@ """ # Standard imports -from project.npda.general_functions.rcpch_nhs_organisations import get_nhs_organisation import pytest +from django.apps import apps + # NPDA Imports from project.npda.tests.UserDataClasses import ( @@ -14,11 +15,8 @@ test_user_audit_centre_reader_data, test_user_rcpch_audit_team_data, ) -from project.npda.models import ( - NPDAUser, - OrganisationEmployer, -) -from .NPDAUserFactory import NPDAUserFactory +from project.npda.models import NPDAUser +from project.npda.tests.factories.npda_user_factory import NPDAUserFactory from project.constants.user import RCPCH_AUDIT_TEAM from project.constants import VIEW_PREFERENCES import logging @@ -26,9 +24,12 @@ logger = logging.getLogger(__name__) + + @pytest.fixture(scope="session") def seed_users_fixture(django_db_setup, django_db_blocker): + # Define user data to seed users = [ test_user_audit_centre_reader_data, @@ -39,10 +40,9 @@ def seed_users_fixture(django_db_setup, django_db_blocker): with django_db_blocker.unblock(): - # Don't repeat seed if users already exist. if NPDAUser.objects.exists(): - logger.info("Test users already seeded. Skipping") - return + logger.info("Test users already seeded. Deleting all users.") + NPDAUser.objects.all().delete() # Otherwise, seed the users is_active = True @@ -50,10 +50,10 @@ def seed_users_fixture(django_db_setup, django_db_blocker): is_rcpch_audit_team_member = False is_rcpch_staff = False - GOSH_ODS_CODE = "RP401" - ALDER_HEY_ODS_CODE = "RBS25" + GOSH_PZ_CODE = "PZ196" + ALDER_HEY_PZ_CODE = "PZ074" - logger.info(f"Seeding test users at {GOSH_ODS_CODE=}.") + logger.info(f"Seeding test users at {GOSH_PZ_CODE=} and {ALDER_HEY_PZ_CODE=}.") # Seed a user of each type at GOSH for user in users: first_name = user.role_str @@ -75,14 +75,12 @@ def seed_users_fixture(django_db_setup, django_db_blocker): is_rcpch_audit_team_member=is_rcpch_audit_team_member, is_rcpch_staff=is_rcpch_staff, groups=[user.group_name], - organisation_employers=[ - GOSH_ODS_CODE - ], # Factory handles creating and assigning OrganisationEmployer view_preference=( VIEW_PREFERENCES[2][0] if user.role == RCPCH_AUDIT_TEAM else VIEW_PREFERENCES[0][0] ), + organisation_employers=[GOSH_PZ_CODE], ) # Alder hey user @@ -95,10 +93,9 @@ def seed_users_fixture(django_db_setup, django_db_blocker): is_rcpch_audit_team_member=is_rcpch_audit_team_member, is_rcpch_staff=is_rcpch_staff, groups=[user.group_name], - organisation_employers=[ - ALDER_HEY_ODS_CODE - ], # Factory handles creating and assigning OrganisationEmployer + organisation_employers=[ALDER_HEY_PZ_CODE], ) - logger.info(f"Seeded {new_user_gosh=} and {new_user_alder_hey=}") - logger.info(f"All test users sucessfully seeded: {NPDAUser.objects.all()=}") + logger.info(f"Seeded users: \n{new_user_gosh=} and \n{new_user_alder_hey=}") + + assert NPDAUser.objects.count() == len(users) * 2 diff --git a/project/npda/tests/misc_py_shell_code.py b/project/npda/tests/misc_py_shell_code.py index e2675a9b..bb92ced5 100644 --- a/project/npda/tests/misc_py_shell_code.py +++ b/project/npda/tests/misc_py_shell_code.py @@ -11,7 +11,7 @@ ) from project.npda.general_functions.rcpch_nhs_organisations import get_nhs_organisation from project.npda.models import OrganisationEmployer -from project.npda.tests.factories.NPDAUserFactory import NPDAUserFactory +from project.npda.tests.factories.npda_user_factory import NPDAUserFactory from project.constants.user import RCPCH_AUDIT_TEAM users = [ diff --git a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py index f00be1fb..22e4c020 100644 --- a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py +++ b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py @@ -77,6 +77,7 @@ def test_npda_user_list_view_rcpch_audit_team_can_view_all_users( organisation_employers__pz_code=ALDER_HEY_PZ_CODE, role=RCPCH_AUDIT_TEAM ).first() + client = login_and_verify_user(client, ah_audit_team_user) diff --git a/project/npda/tests/test_setup_db_for_tests.py b/project/npda/tests/test_setup_db_for_tests.py index 4bbbd1bf..2817df6f 100644 --- a/project/npda/tests/test_setup_db_for_tests.py +++ b/project/npda/tests/test_setup_db_for_tests.py @@ -4,9 +4,21 @@ """ import pytest +import logging from django.contrib.auth.models import Group + from project.npda.models import NPDAUser, OrganisationEmployer +from project.npda.models.paediatric_diabetes_unit import PaediatricDiabetesUnit +from project.npda.tests.factories.npda_user_factory import NPDAUserFactory +from project.npda.tests.UserDataClasses import ( + test_user_audit_centre_reader_data, +) +from project.constants import VIEW_PREFERENCES + +# logging +logger = logging.getLogger(__name__) + @pytest.mark.django_db def test__seed_test_db( @@ -17,3 +29,30 @@ def test__seed_test_db( assert Group.objects.all().exists() assert OrganisationEmployer.objects.all().exists() assert NPDAUser.objects.all().exists() + + +@pytest.mark.django_db +def test__multiple_PaediatricsDiabetesUnitFactory_instances_not_created( + seed_groups_fixture, + seed_users_fixture, + seed_patients_fixture, +): + # GOSH User + user_data = test_user_audit_centre_reader_data + GOSH_PZ_CODE = "PZ196" + + for _ in range(5): + new_user_gosh = NPDAUserFactory( + first_name="test", + role=user_data.role, + # Assign flags based on user role + is_active=user_data.is_active, + is_staff=user_data.is_staff, + is_rcpch_audit_team_member=user_data.is_rcpch_audit_team_member, + is_rcpch_staff=user_data.is_rcpch_staff, + groups=[user_data.group_name], + view_preference=(VIEW_PREFERENCES[0][0]), + organisation_employers=[GOSH_PZ_CODE], + ) + + assert PaediatricDiabetesUnit.objects.filter(pz_code=GOSH_PZ_CODE).count() == 1 diff --git a/project/npda/views/mixins.py b/project/npda/views/mixins.py index af946498..0354cd1a 100644 --- a/project/npda/views/mixins.py +++ b/project/npda/views/mixins.py @@ -2,6 +2,7 @@ import logging +from django.apps import apps from django.conf import settings from django.core.exceptions import PermissionDenied from django.contrib.auth.mixins import AccessMixin @@ -121,6 +122,8 @@ def dispatch(self, request, *args, **kwargs): model = self.get_model().__name__ + Transfer = apps.get_model("npda", "Transfer") + # get PDU assigned to user who is trying to access a view user_pdu = request.user.organisation_employers.first().pz_code @@ -133,7 +136,8 @@ def dispatch(self, request, *args, **kwargs): elif model == "Patient": requested_patient = Patient.objects.get(pk=self.kwargs["pk"]) - requested_pdu = requested_patient.transfer.paediatric_diabetes_unit.pz_code + transfer = Transfer.objects.get(patient=requested_patient) + requested_pdu = transfer.paediatric_diabetes_unit.pz_code elif model == "Visit": requested_patient = Patient.objects.get(pk=self.kwargs["patient_id"]) diff --git a/project/npda/views/npda_users.py b/project/npda/views/npda_users.py index 46cb92f9..8db73603 100644 --- a/project/npda/views/npda_users.py +++ b/project/npda/views/npda_users.py @@ -2,6 +2,7 @@ import logging from django.apps import apps +from django.forms import BaseModelForm from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.utils import timezone from django.shortcuts import redirect, render @@ -188,6 +189,16 @@ def get_context_data(self, **kwargs): context["title"] = "Add New NPDA User" context["button_title"] = "Add NPDA User" context["form_method"] = "create" + # populate the add_employer field with organisations that the user is affilitated with unless the user is a superuser or RCPCH audit team member + # in which case all organisations are shown + context["organisation_choices"] = ( + organisations_adapter.get_all_nhs_organisations() + if ( + self.request.user.is_superuser + or self.request.user.is_rcpch_audit_team_member + ) + else organisations_adapter.get_all_nhs_organisations_affiliated_with_paediatric_diabetes_unit() + ) return context def form_valid(self, form): @@ -294,7 +305,6 @@ def form_valid(self, form): def get_success_url(self) -> str: return reverse( "npda_users", - # organisation_id=organisation_id, ) @@ -341,61 +351,12 @@ def get_context_data(self, **kwargs): .all() .order_by("-is_primary_employer") ) - return context - - def form_valid(self, form): - PaediatricDiabetesUnit = apps.get_model("npda", "PaediatricDiabetesUnit") - instance = form.save() - - new_employer_ods_code = form.cleaned_data["add_employer"] - - if new_employer_ods_code: - # a new employer has been added - # fetch the organisation object from the API using the ODS code - organisation = organisations_adapter.get_single_pdu_from_ods_code( - new_employer_ods_code + context["organisation_choices"] = ( + organisations_adapter.organisations_to_populate_select_field( + request=self.request, user_instance=context["npda_user"] ) - - if "error" in organisation: - messages.error( - self.request, - f"Error: {organisation['error']}. Organisation not added. Please contact the NPDA team if this issue persists.", - ) - return HttpResponseRedirect(self.get_success_url()) - - # Get the name of the organistion from the API response - matching_organisation = next( - ( - org - for org in organisation["organisations"] - if org["ods_code"] == new_employer_ods_code - ), - None, - ) - - if matching_organisation: - # creat or update the OrganisationEmployer object - paediatric_diabetes_unit, created = ( - PaediatricDiabetesUnit.objects.update_or_create( - ods_code=new_employer_ods_code, - pz_code=organisation["pz_code"], - ) - ) - # set all employers to False - OrganisationEmployer.objects.filter(npda_user=instance).update( - is_primary_employer=False - ) - # createt the new employer as the primary employer - OrganisationEmployer.objects.create( - paediatric_diabetes_unit=paediatric_diabetes_unit, - npda_user=instance, - is_primary_employer=True, - ) - instance.refresh_from_db() - - return HttpResponseRedirect(self.get_success_url()) - - return super().form_valid(form) + ) + return context def post(self, request: HttpRequest, *args: str, **kwargs) -> HttpResponse: """ @@ -403,24 +364,17 @@ def post(self, request: HttpRequest, *args: str, **kwargs) -> HttpResponse: TODO: Only Superusers or Coordinators can do this """ if request.htmx: + # these are HTMX post requests from the edit user form + # the return value is a partial view of the employers list, with the select, delete and set primary employer buttons npda_user = NPDAUser.objects.get(pk=self.kwargs["pk"]) if request.POST.get("update") == "delete": + # delete the selected employer + # cannot delete the primary employer but can set another employer as primary first and then delete the employer OrganisationEmployer.objects.filter( pk=request.POST.get("organisation_employer_id") ).delete() - return render( - request=request, - template_name="partials/employers.html", - context={ - "npda_user": npda_user, - "organisation_employers": OrganisationEmployer.objects.filter( - npda_user=npda_user - ) - .all() - .order_by("-is_primary_employer"), - }, - ) elif request.POST.get("update") == "update": + # set the selected employer as the primary employer. Reset all other employers to False before setting the selected employer to True since only one employer can be primary # set all employers to False OrganisationEmployer.objects.filter(npda_user=npda_user).update( is_primary_employer=False @@ -430,6 +384,76 @@ def post(self, request: HttpRequest, *args: str, **kwargs) -> HttpResponse: pk=request.POST.get("organisation_employer_id") ).update(is_primary_employer=True) + elif request.POST.get("add_employer"): + # add to new employer to the employer list + + # add the user to the appropriate organisation + new_employer_ods_code = request.POST.get("add_employer") + if new_employer_ods_code: + # a new employer has been added + # fetch the organisation object from the API using the ODS code + organisation = organisations_adapter.get_single_pdu_from_ods_code( + new_employer_ods_code + ) + + if "error" in organisation: + messages.error( + self.request, + f"Error: {organisation['error']}. Organisation not added. Please contact the NPDA team if this issue persists.", + ) + return HttpResponseRedirect(self.get_success_url()) + + # Get the name of the organistion from the API response + matching_organisation = next( + ( + org + for org in organisation["organisations"] + if org["ods_code"] == new_employer_ods_code + ), + None, + ) + + if matching_organisation: + # create or update the OrganisationEmployer object + PaediatricDiabetesUnit = apps.get_model( + "npda", "PaediatricDiabetesUnit" + ) + paediatric_diabetes_unit, created = ( + PaediatricDiabetesUnit.objects.update_or_create( + ods_code=new_employer_ods_code, + pz_code=organisation["pz_code"], + ) + ) + if npda_user.organisation_employers.count() == 0: + # if the user has no employers, set the new employer as the primary employer + OrganisationEmployer.objects.update_or_create( + paediatric_diabetes_unit=paediatric_diabetes_unit, + npda_user=npda_user, + is_primary_employer=True, + ) + npda_user.refresh_from_db() + else: + # add the new employer to the user's employer list + OrganisationEmployer.objects.update_or_create( + paediatric_diabetes_unit=paediatric_diabetes_unit, + npda_user=npda_user, + is_primary_employer=False, + ) + npda_user.refresh_from_db() + + # return the partial view of the employers list + # if the a new employer has been added to the user, the new employer needs to be removed from the add_employer select list + # the add_employer select list is repopulated with the remaining organisations - this happens by calling the get_form method + + # get the user being edited + user_instance = self.get_object() + + organisation_choices = ( + organisations_adapter.organisations_to_populate_select_field( + request=self.request, user_instance=user_instance + ) + ) + return render( request=request, template_name="partials/employers.html", @@ -440,6 +464,7 @@ def post(self, request: HttpRequest, *args: str, **kwargs) -> HttpResponse: ) .all() .order_by("-is_primary_employer"), + "organisation_choices": organisation_choices, }, ) if "resend_email" in request.POST: diff --git a/project/npda/views/patient.py b/project/npda/views/patient.py index 8856d24a..53d34bc9 100644 --- a/project/npda/views/patient.py +++ b/project/npda/views/patient.py @@ -4,6 +4,7 @@ # Django imports from django.apps import apps +from django.utils import timezone from django.contrib.messages.views import SuccessMessageMixin from django.contrib.auth.mixins import PermissionRequiredMixin from django.db.models import Count, Case, When, Max, Q, F @@ -23,6 +24,7 @@ get_new_session_fields, get_or_update_view_preference, ) +from project.npda.general_functions.quarter_for_date import retrieve_quarter_for_date from project.npda.models import NPDAUser, Submission # RCPCH imports @@ -45,7 +47,7 @@ def get_queryset(self): """ Return all patients with the number of errors in their visits Order by valid patients first, then by number of errors in visits, then by primary key - Scope to patient only in the same organisation as the user + Scope to patient only in the same organisation as the user and current audit year """ pz_code = self.request.session.get("pz_code") ods_code = self.request.session.get("ods_code") @@ -53,12 +55,13 @@ def get_queryset(self): # filter patients to the view preference of the user if self.request.user.view_preference == 0: # organisation view - filtered_patients = Q( - paediatric_diabetes_units__ods_code=ods_code, - ) + # this has been deprecated + pass elif self.request.user.view_preference == 1: # PDU view - filtered_patients = Q(paediatric_diabetes_units__pz_code=pz_code) + filtered_patients = Q( + submissions__paediatric_diabetes_unit__pz_code=pz_code, + ) elif self.request.user.view_preference == 2: # National view - no filter pass @@ -169,7 +172,7 @@ class PatientCreateView( LoginAndOTPRequiredMixin, PermissionRequiredMixin, SuccessMessageMixin, CreateView ): """ - Handle creation of new patient in audit + Handle creation of new patient in audit - should link the patient to the current audit year and quarter, and the logged in user's PDU """ permission_required = "npda.add_patient" @@ -196,11 +199,12 @@ def form_valid(self, form: BaseForm) -> HttpResponse: # add the PDU to the patient record # get or create the paediatric diabetes unit object - paediatric_diabetes_unit = apps.get_model( - "npda", "PaediatricDiabetesUnit" - ).objects.get_or_create( - pz_code=self.request.session.get("pz_code"), - ods_code=self.request.session.get("ods_code"), + PaediatricDiabetesUnit = apps.get_model("npda", "PaediatricDiabetesUnit") + paediatric_diabetes_unit, created = ( + PaediatricDiabetesUnit.objects.get_or_create( + pz_code=self.request.session.get("pz_code"), + ods_code=self.request.session.get("ods_code"), + ) ) Transfer = apps.get_model("npda", "Transfer") @@ -224,10 +228,24 @@ def form_valid(self, form: BaseForm) -> HttpResponse: reason_leaving_service=None, ) - # add patient to the latest audit cohort - if Submission.objects.count() > 0: - new_first = Submission.objects.order_by("-submission_date").first() - new_first.patients.add(patient) + # add patient to the latest audit year, and user selected quarter, and the logged in user's PDU + # the form is initialised with the current audit year and quarter + quarter = form.cleaned_data["quarter"] + Submission = apps.get_model("npda", "Submission") + submission, created = Submission.objects.update_or_create( + audit_year=date.today().year, + quarter=quarter, + paediatric_diabetes_unit=paediatric_diabetes_unit, + submission_active=True, + defaults={ + "submission_by": NPDAUser.objects.get(pk=self.request.user.pk), + "submission_by": NPDAUser.objects.get(pk=self.request.user.pk), + "submission_date": timezone.now(), + }, + ) + submission.patients.add(patient) + submission.save() + return super().form_valid(form) @@ -248,13 +266,18 @@ class PatientUpdateView( form_class = PatientForm success_message = "New child record updated successfully" success_url = reverse_lazy("patients") + Submission = apps.get_model("npda", "Submission") def get_context_data(self, **kwargs): + Transfer = apps.get_model("npda", "Transfer") patient = Patient.objects.get(pk=self.kwargs["pk"]) - pz_code = patient.transfer.pz_code - ods_code = patient.transfer.ods_code + + transfer = Transfer.objects.get(patient=patient) + context = super().get_context_data(**kwargs) - context["title"] = f"Edit Child Details in {ods_code}({pz_code})" + context["title"] = ( + f"Edit Child Details in {transfer.paediatric_diabetes_unit.ods_code}({transfer.paediatric_diabetes_unit.pz_code})" + ) context["button_title"] = "Edit Child Details" context["form_method"] = "update" context["patient_id"] = self.kwargs["pk"] @@ -262,8 +285,13 @@ def get_context_data(self, **kwargs): def form_valid(self, form: BaseForm) -> HttpResponse: patient = form.save(commit=False) + quarter = form.cleaned_data["quarter"] patient.is_valid = True patient.save() + # update the quarter for the patient in the submission + Submission.objects.filter(patients=patient, submission_active=True).update( + quarter=quarter + ) return super().form_valid(form) diff --git a/project/npda/views/submissions.py b/project/npda/views/submissions.py index 452e34c5..3f00631a 100644 --- a/project/npda/views/submissions.py +++ b/project/npda/views/submissions.py @@ -32,12 +32,16 @@ def get_queryset(self) -> Iterable[Any]: :return: The queryset for the view """ + PaediatricDiabetesUnit = apps.get_model( + app_label="npda", model_name="PaediatricDiabetesUnit" + ) + pdu, created = PaediatricDiabetesUnit.objects.get_or_create( + pz_code=self.request.session.get("pz_code"), + ods_code=self.request.session.get("ods_code"), + ) queryset = ( - self.model.objects.filter( - pz_code=self.request.session.get("pz_code"), - ods_code=self.request.session.get("ods_code"), - ) - .values("submission_date", "pz_code", "ods_code", "quarter", "audit_year") + self.model.objects.filter(paediatric_diabetes_unit=pdu) + .values("submission_date", "quarter", "audit_year") .annotate( patient_count=Count("patients"), submission_active=F("submission_active"), @@ -48,8 +52,6 @@ def get_queryset(self) -> Iterable[Any]: ) .order_by( "-submission_date", - "pz_code", - "ods_code", "audit_year", "quarter", "submission_active", @@ -68,17 +70,6 @@ def get_context_data(self, **kwargs: Any) -> dict: context["pz_code"] = self.request.session.get("pz_code") return context - def get(self, request, *args, **kwargs): - """ - Handle the GET request. - - :param request: The request - :param args: The arguments - :param kwargs: The keyword arguments - :return: The response - """ - return super().get(request, *args, **kwargs) - def post(self, request, *args, **kwargs): """ Handle the POST request. diff --git a/static/tailwind.css b/static/tailwind.css index 26e26be6..f60bd11a 100644 --- a/static/tailwind.css +++ b/static/tailwind.css @@ -46,4 +46,8 @@ .fade-in { opacity: 1; transition: opacity 500ms ease-out; +} + +.hidden { + display: none; } \ No newline at end of file

NPDA IDNHS NumberSexDate of BirthPostcodeEthnicityDiabetes TypeDiagnosis DateDate UploadedAudit Year (Quarterly Cohort)NPDA IDNHS NumberSexDate of BirthPostcodeEthnicityDiabetes TypeDiagnosis DateDate UploadedAudit Year (Quarterly Cohort)