From 1f68176504e22d4cf176033fad718ef7be9adefb Mon Sep 17 00:00:00 2001 From: erikvw Date: Thu, 7 Mar 2024 12:26:41 -0600 Subject: [PATCH 1/2] allow bypass checks if cdef overlaps by model and site, seperate system checks, filter by version in manager --- README.rst | 132 +++++++++++------- edc_consent/consent_definition.py | 1 + ...consent_definition_form_validator_mixin.py | 5 + .../subject_consent_form_validator.py | 12 +- edc_consent/managers.py | 41 ++++-- .../model_mixins/consent_model_mixin.py | 6 +- edc_consent/site_consents.py | 15 +- edc_consent/system_checks.py | 84 ++++++++++- edc_consent/tests/consent_test_utils.py | 3 + edc_consent/tests/tests/test_consent_model.py | 18 --- 10 files changed, 219 insertions(+), 98 deletions(-) diff --git a/README.rst b/README.rst index f91fea1..15bf2bd 100644 --- a/README.rst +++ b/README.rst @@ -30,17 +30,37 @@ Declare the consent model: subject_identifier_cls = SubjectIdentifier - subject_screening_model = "effect_screening.subjectscreening" + subject_screening_model = "edc_example.subjectscreening" - objects = SubjectConsentManager() + objects = ConsentObjectsManager() on_site = CurrentSiteManager() - consent = ConsentManager() history = HistoricalRecords() class Meta(ConsentModelMixin.Meta, BaseUuidModel.Meta): pass -Declare at least one ``ConsentDefinition`` that references your consent model. + + class SubjectConsentV1(SubjectConsent): + """A proxy model completed by the user that captures version 1 + of the ICF. + """ + objects = ConsentObjectsByCdefManager() + on_site = CurrentSiteByCdefManager() + history = HistoricalRecords() + + class Meta: + proxy = True + verbose_name = "Consent V1" + verbose_name_plural = "Consent V1" + +The next step is to declare and register a ``ConsentDefinition``. A consent definition is a class that represents an +approved Informed Consent. It is linked to a proxy of the consent model, for example ``SubjectConsent`` from above, +using the class attribute +``model``. We use a proxy model since over time each subject may need to submit more than one +version of the consent. Each version of a subject's consent is represented by an instance of the Each version is paird with a proxy model. The approved Informed Consent +also includes a validity period (start=datetime1 to end=datetime2) and a version number +(version=1). There are other attributes of a ``ConsentDefinition`` to consider but lets focus +on the ``start`` date, ``end`` date, ``version`` and ``model`` for now. ``ConsentDefinitions`` are declared in the root of your app in module ``consents.py``. A typical declaration looks something like this: @@ -54,7 +74,7 @@ Declare at least one ``ConsentDefinition`` that references your consent model. from edc_constants.constants import MALE, FEMALE consent_v1 = ConsentDefinition( - 'edc_example.subjectconsent', + 'edc_example.subjectconsentv1', version='1', start=datetime(2013, 10, 15, tzinfo=ZoneInfo("UTC")), end=datetime(2016, 10, 15, 23, 59, 999999, tzinfo=ZoneInfo("UTC")), @@ -78,19 +98,55 @@ add to settings: On bootup ``site_consents`` will ``autodiscover`` the ``consents.py`` and register the ``ConsentDefinition``. -Now create an instance of the ``SubjectConsent`` model, ``subject_consent``. When the instance is saved, the model will find the ``ConsentDefinition`` with a validity period that includes ``subject_consent.consent_datetime`` and update ``subject_consent.version`` with the value of ``consent_definition.version``. -In this case, for ``consent_datetime`` equal to ``datetime(2013, 10, 16, tzinfo=ZoneInfo("UTC"))``, the model will find ``consent_v1``. -If the ``consent_datetime`` is outside of the date boundary, for example datetime(2017, 1, 1, tzinfo=ZoneInfo("UTC")), the model will not find a -``ConsentDefinition`` and an exception will be raised (``ConsentDefinitionNotFound``). +To create an instance of the consent for a subject, find the ``ConsentDefinitions`` and use +``model_cls``. + + +.. code-block:: python + + cdef = site_consents.get_consent_definition( + report_datetime=datetime(2013, 10, 16, tzinfo=ZoneInfo("UTC")) + ) + + assert cdef.version == "1" + assert cdef.model == "edc_example.subjectconsentv1" + + consent_obj = cdef.model_cls.objects.create( + subject_identifier="123456789", + consent_datetime=datetime(2013, 10, 16, tzinfo=ZoneInfo("UTC"), + ...) + + assert consent_obj.consent_version == "1" + assert consent_obj.consent_model == "edc_example.subjectconsentv1" + + Add a second ``ConsentDefinition`` to ``your consents.py`` for version 2: +.. code-block:: python + + class SubjectConsentV2(SubjectConsent): + """A proxy model completed by the user that captures version 2 + of the ICF. + """ + objects = ConsentObjectsByCdefManager() + on_site = CurrentSiteByCdefManager() + history = HistoricalRecords() + + class Meta: + proxy = True + verbose_name = "Consent V2" + verbose_name_plural = "Consent V2" + + + + .. code-block:: python consent_v1 = ConsentDefinition(...) consent_v2 = ConsentDefinition( - 'edc_example.subjectconsent', + 'edc_example.subjectconsentv2', version='2', start=datetime(2016, 10, 16, 0,0,0, tzinfo=ZoneInfo("UTC")), end=datetime(2020, 10, 15, 23, 59, 999999, tzinfo=ZoneInfo("UTC")), @@ -103,8 +159,24 @@ Add a second ``ConsentDefinition`` to ``your consents.py`` for version 2: site_consents.register(consent_v2) -Now resave the instance from above with ``consent_datetime = datetime(2017, 1, 1, tzinfo=ZoneInfo("UTC"))``. The model will find -``consent_v2`` and update ``subject_consent.version = consent_v2.version`` which in this case is "2". + +.. code-block:: python + + cdef = site_consents.get_consent_definition( + report_datetime=datetime(2016, 10, 17, tzinfo=ZoneInfo("UTC")) + ) + + assert cdef.version == "2" + assert cdef.model == "edc_example.subjectconsentv2" + + consent_obj = cdef.model_cls.objects.create( + subject_identifier="123456789", + consent_datetime=datetime(2016, 10, 17, tzinfo=ZoneInfo("UTC"), + ...) + + assert consent_obj.consent_version == "2" + assert consent_obj.consent_model == "edc_example.subjectconsentv2" + ``edc_consent`` is coupled with ``edc_visit_schedule``. In fact, a data collection schedule is declared with one or more ``ConsentDefinitions``. CRFs and Requisitions listed in a schedule may only be submitted if the subject has consented. @@ -120,42 +192,6 @@ Now resave the instance from above with ``consent_datetime = datetime(2017, 1, 1 When a CRF is saved, the CRF model will check the ``schedule`` to find the ``ConsentDefinition`` with a validity period that contains the ``crf.report_datetime``. Using the located ``ConsentDefinitions``, the CRF model will confirm the subject has a saved ``subject_consent`` with this ``consent_definition.version``. -When there is more than one ``ConsentDefinition`` but still just one ``SubjectConsent`` model, declaring proxy models -provides some clarity and allows the ``ModelForm`` and ``ModelAdmin`` classes to be customized. - -.. code-block:: python - - class SubjectConsentV1(SubjectConsent): - - class Meta: - proxy = True - verbose_name = "Consent V1" - verbose_name_plural = "Consent V1" - - - class SubjectConsentV2(SubjectConsent): - - class Meta: - proxy = True - verbose_name = "Consent V2" - verbose_name_plural = "Consent V2" - - -.. code-block:: python - - consent_v1 = ConsentDefinition( - 'edc_example.subjectconsentv1', - version='1', ...) - - consent_v2 = ConsentDefinition( - 'edc_example.subjectconsentv2', - version='2', ...) - - site_consents.register(consent_v1) - site_consents.register(consent_v2) - -Now each model can use a custom ``ModelAdmin`` class. - The ConsentDefinitions above assume that consent version 1 is completed for a subject consenting on or before 2016/10/15 and version 2 for those consenting after 2016/10/15. diff --git a/edc_consent/consent_definition.py b/edc_consent/consent_definition.py index 46226ce..1a4d376 100644 --- a/edc_consent/consent_definition.py +++ b/edc_consent/consent_definition.py @@ -51,6 +51,7 @@ class ConsentDefinition: gender: list[str] | None = field(default_factory=list, compare=False) site_ids: list[int] = field(default_factory=list, compare=False) country: str | None = field(default=None, compare=False) + validate_duration_overlap_by_model: bool | None = field(default=True, compare=False) subject_type: str = field(default="subject", compare=False) name: str = field(init=False, compare=False) update_cdef: ConsentDefinition = field(default=None, init=False, compare=False) diff --git a/edc_consent/form_validators/consent_definition_form_validator_mixin.py b/edc_consent/form_validators/consent_definition_form_validator_mixin.py index 3d6c8a4..2e1baef 100644 --- a/edc_consent/form_validators/consent_definition_form_validator_mixin.py +++ b/edc_consent/form_validators/consent_definition_form_validator_mixin.py @@ -13,6 +13,11 @@ class ConsentDefinitionFormValidatorMixin: + @property + def subject_consent(self): + cdef = self.get_consent_definition() + return cdef.model_cls.objects.get(subject_identifier=self.subject_identifier) + def get_consent_datetime_or_raise( self, report_datetime: datetime = None, fldname: str = None, error_code: str = None ) -> datetime: diff --git a/edc_consent/form_validators/subject_consent_form_validator.py b/edc_consent/form_validators/subject_consent_form_validator.py index 6ca9855..d1cade5 100644 --- a/edc_consent/form_validators/subject_consent_form_validator.py +++ b/edc_consent/form_validators/subject_consent_form_validator.py @@ -25,6 +25,12 @@ def _clean(self) -> None: self.validate_demographics() super()._clean() + def validate_demographics(self) -> None: + self.validate_consent_datetime() + self.validate_age() + self.validate_gender() + self.validate_identity() + @property def gender(self): return self.cleaned_data.get("gender") @@ -48,12 +54,6 @@ def consent_model_cls(self): ) return cdef.model_cls - def validate_demographics(self) -> None: - self.validate_consent_datetime() - self.validate_age() - self.validate_gender() - self.validate_identity() - @property def consent_datetime(self) -> datetime | None: if not self._consent_datetime: diff --git a/edc_consent/managers.py b/edc_consent/managers.py index 39da155..a242ddb 100644 --- a/edc_consent/managers.py +++ b/edc_consent/managers.py @@ -1,23 +1,38 @@ from __future__ import annotations -from typing import TYPE_CHECKING - from django.db import models +from edc_search.model_mixins import SearchSlugManager +from edc_sites.managers import CurrentSiteManager -if TYPE_CHECKING: - from .stubs import ConsentLikeModel +from .site_consents import site_consents -class ObjectConsentManager(models.Manager): +class ConsentObjectsManager(SearchSlugManager, models.Manager): def get_by_natural_key(self, subject_identifier_as_pk): return self.get(subject_identifier_as_pk=subject_identifier_as_pk) -class ConsentManager(models.Manager): - def first_consent(self, subject_identifier=None) -> ConsentLikeModel: - """Returns the first consent by consent_datetime.""" - return ( - self.filter(subject_identifier=subject_identifier) - .order_by("consent_datetime") - .first() - ) +class ConsentObjectsByCdefManager(ConsentObjectsManager): + """An objects model manager to use on consent proxy models + linked to a ConsentDefinition. + + Filters queryset by the proxy model's label_lower + """ + + def get_queryset(self): + qs = super().get_queryset() + cdef = site_consents.get_consent_definition(model=qs.model._meta.label_lower) + return qs.filter(version=cdef.version) + + +class CurrentSiteByCdefManager(CurrentSiteManager): + """A site model manager to use on consent proxy models + linked to a ConsentDefinition. + + Filters queryset by the proxy model's label_lower + """ + + def get_queryset(self): + qs = super().get_queryset() + cdef = site_consents.get_consent_definition(model=qs.model._meta.label_lower) + return qs.filter(version=cdef.version) diff --git a/edc_consent/model_mixins/consent_model_mixin.py b/edc_consent/model_mixins/consent_model_mixin.py index 179f456..c1927a0 100644 --- a/edc_consent/model_mixins/consent_model_mixin.py +++ b/edc_consent/model_mixins/consent_model_mixin.py @@ -11,7 +11,7 @@ from edc_utils import age, formatted_age from ..field_mixins import VerificationFieldsMixin -from ..managers import ConsentManager, ObjectConsentManager +from ..managers import ConsentObjectsManager from .consent_version_model_mixin import ConsentVersionModelMixin @@ -70,9 +70,7 @@ class ConsentModelMixin(ConsentVersionModelMixin, VerificationFieldsMixin, model help_text="A unique identifier for this consent instance", ) - objects = ObjectConsentManager() - - consent = ConsentManager() + objects = ConsentObjectsManager() on_site = CurrentSiteManager() diff --git a/edc_consent/site_consents.py b/edc_consent/site_consents.py index e261e2d..e6aef3f 100644 --- a/edc_consent/site_consents.py +++ b/edc_consent/site_consents.py @@ -40,7 +40,11 @@ def register(self, cdef: ConsentDefinition) -> None: if cdef.name in self.registry: raise AlreadyRegistered(f"Consent definition already registered. Got {cdef.name}.") for registered_cdef in self.registry.values(): - if registered_cdef.model == cdef.model: + if ( + cdef + and cdef.validate_duration_overlap_by_model + and registered_cdef.model == cdef.model + ): if ( registered_cdef.start <= cdef.start <= registered_cdef.end or registered_cdef.start <= cdef.end <= registered_cdef.end @@ -68,15 +72,14 @@ def register(self, cdef: ConsentDefinition) -> None: self.loaded = True def get_registry_display(self): - return "', '".join( - [cdef.display_name for cdef in sorted(list(self.registry.values()))] - ) + cdefs = sorted(list(self.registry.values()), key=lambda x: x.version) + return "', '".join([cdef.display_name for cdef in cdefs]) def get(self, name) -> ConsentDefinition: return self.registry.get(name) def all(self) -> list[ConsentDefinition]: - return sorted(list(self.registry.values())) + return sorted(list(self.registry.values()), key=lambda x: x.version) def get_consent_definition( self, @@ -145,7 +148,7 @@ def get_consent_definitions( for k, v in kwargs.items(): if v is not None: cdefs = [cdef for cdef in cdefs if getattr(cdef, k) == v] - return sorted(cdefs) + return sorted(cdefs, key=lambda x: x.version) @staticmethod def _filter_cdefs_by_model_or_raise( diff --git a/edc_consent/system_checks.py b/edc_consent/system_checks.py index bfcc382..efc0d66 100644 --- a/edc_consent/system_checks.py +++ b/edc_consent/system_checks.py @@ -1,12 +1,90 @@ -from django.core.checks import CheckMessage, Error +from django.core.checks import CheckMessage, Error, Warning -from edc_consent import site_consents +from .consent_definition import ConsentDefinition +from .site_consents import site_consents -def check_site_consents(app_configs, **kwargs) -> list[CheckMessage]: +def check_consents_cdef_registered(app_configs, **kwargs) -> list[CheckMessage]: errors = [] if not site_consents.registry: errors.append( Error("No consent definitions have been registered.", id="edc_consent.E001") ) return errors + + +def check_consents_proxy_models(app_configs, **kwargs) -> list[CheckMessage]: + """Expect proxy models only in ConsentDefinitions""" + errors = [] + for cdef in site_consents.registry.values(): + if not cdef.model_cls._meta.proxy: + errors.append( + Error( + ( + f"Consent definition model is not a proxy model. Got {cdef.model}." + f"See {cdef.name}" + ), + id="edc_consent.E002", + ) + ) + return errors + + +def check_consents_versions(app_configs, **kwargs) -> list[CheckMessage]: + """Expect versions to be unique across `proxy_for` model""" + errors = [] + cdefs1: list[ConsentDefinition] = [cdef for cdef in site_consents.registry.values()] + used = [] + for cdef1 in cdefs1: + if cdef1 in used: + continue + versions = [] + for cdef in site_consents.registry.values(): + if ( + cdef.model_cls._meta.proxy + and cdef.model_cls._meta.proxy_for_model + == cdef1.model_cls._meta.proxy_for_model + ): + versions.append(cdef.version) + used.append(cdef) + if versions and len(set(versions)) != len(versions): + errors.append( + Warning( + "Consent definition version in use already for model. " + f"Got {cdef.version}.", + id="edc_consent.W001", + ) + ) + return errors + + +def check_consents_durations(app_configs, **kwargs) -> list[CheckMessage]: + """Durations may not overlap across `proxy_for` model""" + errors = [] + found = [] + cdefs: list[ConsentDefinition] = [cdef for cdef in site_consents.registry.values()] + for cdef1 in cdefs: + for cdef2 in cdefs: + if cdef1 == cdef2: + continue + if ( + cdef1.model_cls._meta.proxy + and cdef1.model_cls._meta.proxy_for_model + == cdef1.model_cls._meta.proxy_for_model + ): + if ( + cdef1.start <= cdef2.start <= cdef1.end + or cdef1.start <= cdef2.end <= cdef1.end + ): + if sorted([cdef1, cdef2], key=lambda x: x.version) in found: + continue + else: + found.append(sorted([cdef1, cdef2], key=lambda x: x.version)) + errors.append( + Warning( + "Consent definition duration overlap. " + f"Got {cdef1.name} and {cdef2.name}.", + id="edc_consent.W002", + ) + ) + return errors diff --git a/edc_consent/tests/consent_test_utils.py b/edc_consent/tests/consent_test_utils.py index 4565dec..6983685 100644 --- a/edc_consent/tests/consent_test_utils.py +++ b/edc_consent/tests/consent_test_utils.py @@ -48,6 +48,9 @@ def consent_factory(model=None, **kwargs): age_min=kwargs.get("age_min", 16), age_max=kwargs.get("age_max", 64), age_is_adult=kwargs.get("age_is_adult", 18), + validate_duration_overlap_by_model=kwargs.get( + "validate_duration_overlap_by_model", True + ), ) model = kwargs.get("model", model or "consent_app.subjectconsent") consent_definition = ConsentDefinition(model, **options) diff --git a/edc_consent/tests/tests/test_consent_model.py b/edc_consent/tests/tests/test_consent_model.py index 52028dd..c9559ad 100644 --- a/edc_consent/tests/tests/test_consent_model.py +++ b/edc_consent/tests/tests/test_consent_model.py @@ -11,7 +11,6 @@ from faker import Faker from model_bakery import baker -from consent_app.models import SubjectConsent from edc_consent.field_mixins import IdentityFieldsMixinError from edc_consent.site_consents import site_consents @@ -364,23 +363,6 @@ def test_raise_with_incorrect_model_for_cdef(self): dob=get_utcnow() - relativedelta(years=25), ) - def test_manager(self): - for i in range(1, 3): - first_name = fake.first_name() - last_name = fake.last_name() - initials = f"{first_name[0]}{last_name[0]}" - baker.make_recipe( - "consent_app.subjectconsent", - subject_identifier=str(i), - consent_datetime=self.study_open_datetime + relativedelta(days=i), - initials=initials, - first_name=first_name, - last_name=last_name, - ) - - first = SubjectConsent.objects.get(subject_identifier="1") - self.assertEqual(first, SubjectConsent.consent.first_consent(subject_identifier="1")) - def test_model_str_repr_etc(self): obj = baker.make_recipe( "consent_app.subjectconsent", From 917cca09bd3b96f996d3432993b35cdd5215c1ee Mon Sep 17 00:00:00 2001 From: erikvw Date: Thu, 7 Mar 2024 21:41:41 -0600 Subject: [PATCH 2/2] pass site to consent definition in view --- edc_consent/managers.py | 2 ++ edc_consent/view_mixins/consent_view_mixins.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/edc_consent/managers.py b/edc_consent/managers.py index a242ddb..ecfa476 100644 --- a/edc_consent/managers.py +++ b/edc_consent/managers.py @@ -8,6 +8,8 @@ class ConsentObjectsManager(SearchSlugManager, models.Manager): + use_in_migrations = True + def get_by_natural_key(self, subject_identifier_as_pk): return self.get(subject_identifier_as_pk=subject_identifier_as_pk) diff --git a/edc_consent/view_mixins/consent_view_mixins.py b/edc_consent/view_mixins/consent_view_mixins.py index 4e52894..de75744 100644 --- a/edc_consent/view_mixins/consent_view_mixins.py +++ b/edc_consent/view_mixins/consent_view_mixins.py @@ -46,7 +46,8 @@ def consents(self) -> QuerySet[ConsentLikeModel]: """Returns a Queryset of consents for this subject.""" if not self._consents: self._consents = [] - for cdef in site_consents.get_consent_definitions(): + site = site_sites.get(self.request.site.id) + for cdef in site_consents.get_consent_definitions(site=site): try: obj = cdef.get_consent_for(subject_identifier=self.subject_identifier) except NotConsentedError: