From 3168ef343fbcfb34d78571c3f81b41c6e40d29ed Mon Sep 17 00:00:00 2001 From: ppettitau Date: Wed, 23 Oct 2024 09:58:06 +1000 Subject: [PATCH 1/3] [ERP-2589] Working Group Types and the Unallocated Working Group: * Hide the Working Group field if the only choice it contains is "Unallocated" * If the working group field is disabled: ** Remove the patient's membership to the Unallocated working group if additional working groups (from types) have been selected ** Allocate the patient to the unallocated working group if they would otherwise have no other working group membership --- rdrf/registry/patients/admin_forms.py | 44 ++++++++++++++++++--------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/rdrf/registry/patients/admin_forms.py b/rdrf/registry/patients/admin_forms.py index d1936bc12..c871dda41 100644 --- a/rdrf/registry/patients/admin_forms.py +++ b/rdrf/registry/patients/admin_forms.py @@ -408,6 +408,16 @@ def clinician_display_str(obj): wgs = ", ".join([wg.name for wg in obj.working_groups.all()]) return f"{title} {full_name} ({wgs})" + def has_displayable_working_groups(choices): + if not choices: + return False + + if len(choices) > 1: + return True + + working_group_id, *_ = choices[0] + return working_group_id != WorkingGroup.objects.get_unallocated(registry=self.registry_model).id + instance = None if "registry_model" in kwargs: @@ -517,7 +527,7 @@ def clinician_display_str(obj): ) self.fields.update(additional_working_group_fields) self.fields["working_groups"].choices = working_groups_choices - if not working_groups_choices: + if not has_displayable_working_groups(working_groups_choices): self.fields["working_groups"].disabled = True self.fields["working_groups"].required = False self.fields[ @@ -739,27 +749,33 @@ def clean_working_groups(self): "disabled" in self.fields["working_groups"].widget.attrs or self.fields["working_groups"].disabled ) - empty_choices = not self.fields["working_groups"].choices + base_working_group_choices =self.fields["working_groups"].choices + + additional_working_group_fields = [ + field_name for field_name in self.data.keys() if field_name.startswith("working_groups_") + ] + additional_working_group_values = [ + value + for field_name in additional_working_group_fields + for value in self.data.getlist(field_name) + ] if is_disabled: - if empty_choices: + if not base_working_group_choices: working_groups = WorkingGroup.objects.none() else: - working_groups = self.instance.working_groups.all() + unallocated_working_group = WorkingGroup.objects.get_unallocated(self.registry_model) + if additional_working_group_values: + working_groups = self.instance.working_groups.exclude(id=unallocated_working_group.id) + elif self.instance.working_groups.exists(): + working_groups = self.instance.working_groups.all() + else: + working_groups = {unallocated_working_group} else: working_groups = self.cleaned_data["working_groups"] - field_names = [ - key for key in self.data.keys() if key.startswith("working_groups_") - ] - field_values = [ - value - for field_name in field_names - for value in self.data.getlist(field_name) - ] - all_selected_working_groups = working_groups.union( - WorkingGroup.objects.filter(id__in=field_values) + WorkingGroup.objects.filter(id__in=additional_working_group_values) ) if not all_selected_working_groups: raise forms.ValidationError( From ab2154f25e4f1076d7ccc69904524198357ec45f Mon Sep 17 00:00:00 2001 From: ppettitau Date: Wed, 23 Oct 2024 10:26:03 +1000 Subject: [PATCH 2/3] Linting --- rdrf/registry/patients/admin_forms.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/rdrf/registry/patients/admin_forms.py b/rdrf/registry/patients/admin_forms.py index c871dda41..329392f38 100644 --- a/rdrf/registry/patients/admin_forms.py +++ b/rdrf/registry/patients/admin_forms.py @@ -416,7 +416,12 @@ def has_displayable_working_groups(choices): return True working_group_id, *_ = choices[0] - return working_group_id != WorkingGroup.objects.get_unallocated(registry=self.registry_model).id + return ( + working_group_id + != WorkingGroup.objects.get_unallocated( + registry=self.registry_model + ).id + ) instance = None @@ -749,10 +754,12 @@ def clean_working_groups(self): "disabled" in self.fields["working_groups"].widget.attrs or self.fields["working_groups"].disabled ) - base_working_group_choices =self.fields["working_groups"].choices + base_working_group_choices = self.fields["working_groups"].choices additional_working_group_fields = [ - field_name for field_name in self.data.keys() if field_name.startswith("working_groups_") + field_name + for field_name in self.data.keys() + if field_name.startswith("working_groups_") ] additional_working_group_values = [ value @@ -764,9 +771,13 @@ def clean_working_groups(self): if not base_working_group_choices: working_groups = WorkingGroup.objects.none() else: - unallocated_working_group = WorkingGroup.objects.get_unallocated(self.registry_model) + unallocated_working_group = ( + WorkingGroup.objects.get_unallocated(self.registry_model) + ) if additional_working_group_values: - working_groups = self.instance.working_groups.exclude(id=unallocated_working_group.id) + working_groups = self.instance.working_groups.exclude( + id=unallocated_working_group.id + ) elif self.instance.working_groups.exists(): working_groups = self.instance.working_groups.all() else: From 4e40e50eefbbd1db417040af6956153bc64560cc Mon Sep 17 00:00:00 2001 From: ppettitau Date: Fri, 25 Oct 2024 09:05:11 +1000 Subject: [PATCH 3/3] * Refactor to ensure working_groups is always a QuerySet * Refactor clean_working_groups in attempt to make it more clearer * Update field config to allow a hidden field to be optional (to prevent save errors where existing data does not exist - and the user has no option to change it) --- rdrf/registry/patients/admin_forms.py | 34 +++++++++++++++++++-------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/rdrf/registry/patients/admin_forms.py b/rdrf/registry/patients/admin_forms.py index 329392f38..75f6316aa 100644 --- a/rdrf/registry/patients/admin_forms.py +++ b/rdrf/registry/patients/admin_forms.py @@ -566,6 +566,7 @@ def apply_field_config(target_field, target_field_config): target_field_config.status == DemographicFields.HIDDEN ): + self.fields[target_field].required = False self.fields[ target_field ].widget = forms.MultipleHiddenInput() @@ -750,43 +751,56 @@ def clean_rdrf_registry(self): return registries def clean_working_groups(self): - is_disabled = ( + is_base_working_groups_disabled = ( "disabled" in self.fields["working_groups"].widget.attrs or self.fields["working_groups"].disabled ) base_working_group_choices = self.fields["working_groups"].choices + selected_working_group_ids = self.data.getlist("working_groups") additional_working_group_fields = [ field_name for field_name in self.data.keys() if field_name.startswith("working_groups_") ] - additional_working_group_values = [ + selected_additional_working_group_ids = [ value for field_name in additional_working_group_fields for value in self.data.getlist(field_name) ] - if is_disabled: + # Determine the base working groups the patient should have + if is_base_working_groups_disabled: if not base_working_group_choices: working_groups = WorkingGroup.objects.none() else: unallocated_working_group = ( WorkingGroup.objects.get_unallocated(self.registry_model) ) - if additional_working_group_values: - working_groups = self.instance.working_groups.exclude( - id=unallocated_working_group.id - ) + if selected_additional_working_group_ids: + # The patient has been assigned to additional working groups, + # and they can't otherwise select from the base working groups, so remove them from "Unallocated" + working_groups = WorkingGroup.objects.filter( + id__in=selected_working_group_ids + ).exclude(id=unallocated_working_group.id) elif self.instance.working_groups.exists(): + # No working groups have been selected, (assume all working groups controls are disabled) + # so keep existing working groups working_groups = self.instance.working_groups.all() else: - working_groups = {unallocated_working_group} + # The patient has no allocated working groups, set to "Unallocated" + working_groups = WorkingGroup.objects.filter( + id=unallocated_working_group.id + ) else: - working_groups = self.cleaned_data["working_groups"] + working_groups = WorkingGroup.objects.filter( + id__in=selected_working_group_ids + ) all_selected_working_groups = working_groups.union( - WorkingGroup.objects.filter(id__in=additional_working_group_values) + WorkingGroup.objects.filter( + id__in=selected_additional_working_group_ids + ) ) if not all_selected_working_groups: raise forms.ValidationError(