remove 'updates_by' from ConsentDefinition init, change 'updates' to …

…only accept a ConsentDefinition

consent_app/models.py

@@ -17,6 +17,7 @@
     ReviewFieldsMixin,
     VulnerabilityFieldsMixin,
 )
+from edc_consent.managers import ConsentObjectsByCdefManager, CurrentSiteByCdefManager
 from edc_consent.model_mixins import ConsentModelMixin, RequiresConsentFieldsModelMixin
 
 
@@ -70,16 +71,34 @@ class Meta(ConsentModelMixin.Meta):
 
 
 class SubjectConsentV1(SubjectConsent):
+    on_site = CurrentSiteByCdefManager()
+    objects = ConsentObjectsByCdefManager()
+
+    class Meta:
+        proxy = True
+
+
+class SubjectConsentUgV1(SubjectConsent):
+    on_site = CurrentSiteByCdefManager()
+    objects = ConsentObjectsByCdefManager()
+
     class Meta:
         proxy = True
 
 
 class SubjectConsentV2(SubjectConsent):
+
+    on_site = CurrentSiteByCdefManager()
+    objects = ConsentObjectsByCdefManager()
+
     class Meta:
         proxy = True
 
 
 class SubjectConsentV3(SubjectConsent):
+    on_site = CurrentSiteByCdefManager()
+    objects = ConsentObjectsByCdefManager()
+
     class Meta:
         proxy = True
 

edc_consent/consent_definition.py

@@ -18,6 +18,7 @@
     ConsentDefinitionValidityPeriodError,
     NotConsentedError,
 )
+from .managers import ConsentObjectsByCdefManager, CurrentSiteByCdefManager
 
 if TYPE_CHECKING:
     from edc_identifier.model_mixins import NonUniqueSubjectIdentifierModelMixin
@@ -42,8 +43,8 @@ class ConsentDefinition:
     start: datetime = field(default=ResearchProtocolConfig().study_open_datetime, compare=True)
     end: datetime = field(default=ResearchProtocolConfig().study_close_datetime, compare=False)
     version: str = field(default="1", compare=False)
-    updates: tuple[ConsentDefinition, str] = field(default=tuple, compare=False)
-    updated_by: str = field(default=None, compare=False)
+    updates: ConsentDefinition = field(default=None, compare=False)
+    end_extends_on_update: bool = field(default=False, compare=False)
     screening_model: str = field(default=None, compare=False)
     age_min: int = field(default=18, compare=False)
     age_max: int = field(default=110, compare=False)
@@ -53,10 +54,10 @@ class ConsentDefinition:
     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)
-    update_model: str = field(default=None, init=False, compare=False)
-    update_version: str = field(default=None, init=False, compare=False)
+    # set updated_by when the cdef is registered, see site_consents
+    updated_by: ConsentDefinition = field(default=None, compare=False, init=False)
     _model: str = field(init=False, compare=False)
     sort_index: str = field(init=False)
 
@@ -65,12 +66,6 @@ def __post_init__(self):
         self.name = f"{self.proxy_model}-{self.version}"
         self.sort_index = self.name
         self.gender = [MALE, FEMALE] if not self.gender else self.gender
-        try:
-            self.update_cdef, self.update_model = self.updates
-        except (ValueError, TypeError):
-            pass
-        else:
-            self.update_version = self.update_cdef.version
         if not self.screening_model:
             self.screening_model = get_subject_screening_model()
         if MALE not in self.gender and FEMALE not in self.gender:
@@ -88,6 +83,18 @@ def model(self):
             raise ConsentDefinitionError(
                 f"Model class must be a proxy. See {self.name}. Got {model_cls}"
             )
+        elif not isinstance(model_cls.objects, (ConsentObjectsByCdefManager,)):
+            raise ConsentDefinitionError(
+                "Incorrect 'objects' model manager for consent model. "
+                f"Expected {ConsentObjectsByCdefManager}. See {self.name}.  "
+                f"Got {model_cls.objects.__class__}"
+            )
+        elif not isinstance(model_cls.on_site, (CurrentSiteByCdefManager,)):
+            raise ConsentDefinitionError(
+                "Incorrect 'on_site' model manager for consent model. "
+                f"Expected {CurrentSiteByCdefManager}. See {self.name}.  "
+                f"Got {model_cls.objects.__class__}"
+            )
         return self._model
 
     @model.setter
@@ -138,10 +145,10 @@ def get_consent_for(
         try:
             consent_obj = self.model_cls.objects.get(**opts)
         except ObjectDoesNotExist:
-            if self.update_cdef:
-                opts.update(version=self.update_cdef.version)
+            if self.updates:
+                opts.update(version=self.updates.version)
                 try:
-                    consent_obj = self.update_cdef.model_cls.objects.get(**opts)
+                    consent_obj = self.updates.model_cls.objects.get(**opts)
                 except ObjectDoesNotExist:
                     pass
         if not consent_obj and raise_if_not_consented:

edc_consent/model_mixins/consent_version_model_mixin.py

@@ -62,11 +62,17 @@ def consent_definition(self):
             report_datetime=self.consent_datetime,
             site=site_sites.get(site.id),
         )
-        if self._meta.label_lower not in [cdef.model, cdef.update_model]:
+        if self._meta.label_lower != cdef.model:
             raise ConsentDefinitionModelError(
                 f"Incorrect model for consent_definition. This model cannot be used "
-                f"to create or update consent version '{cdef.version}'. Expected "
-                f"'{cdef.model}' or '{cdef.update_model}'. Got '{self._meta.label_lower}'."
+                f"to 'create' consent version '{cdef.version}'. Expected "
+                f"'{cdef.model}'. Got '{self._meta.label_lower}'."
+            )
+        elif cdef.updates and self._meta.label_lower != cdef.updates.updated_by.model:
+            raise ConsentDefinitionModelError(
+                f"Incorrect model to update a consent. This model cannot be used "
+                f"to 'update' consent version '{cdef.version}'. Expected "
+                f"'{cdef.updates.updated_by.model}'. Got '{self._meta.label_lower}'."
             )
         return cdef
 

edc_consent/site_consents.py

@@ -36,14 +36,55 @@ def __init__(self):
         self.registry = {}
         self.loaded = False
 
-    def register(self, cdef: ConsentDefinition) -> None:
+    def register(
+        self, cdef: ConsentDefinition, updated_by: ConsentDefinition | None = None
+    ) -> None:
+        cdef.updated_by = updated_by
         if cdef.name in self.registry:
             raise AlreadyRegistered(f"Consent definition already registered. Got {cdef.name}.")
+        self.validate_period_overlap_or_raise(cdef)
+        self.validate_updates_or_raise(cdef)
+        self.registry.update({cdef.name: cdef})
+        self.loaded = True
+
+    def get_registry_display(self):
+        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()), key=lambda x: x.version)
+
+    def validate_updates_or_raise(self, cdef: ConsentDefinition) -> None:
+        if cdef.updates:
+            if cdef.updates not in self.registry.values():
+                raise ConsentDefinitionError(
+                    f"Updates unregistered consent definition. See {cdef.name}. "
+                    f"Got {cdef.updates.name}"
+                )
+            elif cdef.updates and cdef.updates.updated_by is None:
+                raise ConsentDefinitionError(
+                    f"Cdef mismatch with consent definition configured to update another. "
+                    f"'{cdef.name}' is configured to update "
+                    f"'{cdef.updates.name}' but '{cdef.updates.name}' "
+                    f"updated_by is None. "
+                )
+            elif cdef.updates and cdef.updates.updated_by != cdef:
+                raise ConsentDefinitionError(
+                    f"Cdef mismatch with consent definition configured to update another. "
+                    f"'{cdef.name}' is configured to update "
+                    f"'{cdef.updates.name}' but '{cdef.updates.name}' "
+                    f"updated_by='{cdef.updates.updated_by.name}' not '{cdef.name}'. "
+                )
+
+    def validate_period_overlap_or_raise(self, cdef: ConsentDefinition):
         for registered_cdef in self.registry.values():
             if (
                 cdef
                 and cdef.validate_duration_overlap_by_model
-                and registered_cdef.model == cdef.model
+                and registered_cdef.proxy_model == cdef.proxy_model
             ):
                 if (
                     registered_cdef.start <= cdef.start <= registered_cdef.end
@@ -54,32 +95,6 @@ def register(self, cdef: ConsentDefinition) -> None:
                         f"definition. See already registered consent {registered_cdef.name}. "
                         f"Got {cdef.name}."
                     )
-        if cdef.update_cdef:
-            if cdef.update_cdef not in self.registry.values():
-                raise ConsentDefinitionError(
-                    f"Updates unregistered consent definition. See {cdef.name}. "
-                    f"Got {cdef.update_cdef.name}"
-                )
-            elif cdef.update_cdef.updated_by and cdef.update_cdef.updated_by != cdef.version:
-                raise ConsentDefinitionError(
-                    f"Version mismatch with consent definition configured to update another. "
-                    f"'{cdef.name}' is configured to update "
-                    f"'{cdef.update_cdef.name}' but '{cdef.update_cdef.name}' "
-                    f"updated_by='{cdef.update_cdef.version}' not '{cdef.version}'. "
-                )
-
-        self.registry.update({cdef.name: cdef})
-        self.loaded = True
-
-    def get_registry_display(self):
-        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()), key=lambda x: x.version)
 
     def get_consent_definition(
         self,

edc_consent/system_checks.py

@@ -33,58 +33,80 @@ def check_consents_proxy_models(app_configs, **kwargs) -> list[CheckMessage]:
 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:
+    for cdef in [cdef for cdef in site_consents.registry.values()]:
+        if cdef 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",
-                )
-            )
+        err, used = inspect_others_using_same_proxy_for_model_with_duplicate_versions(
+            cdef, used
+        )
+        if err:
+            errors.append(err)
     return errors
 
 
 def check_consents_durations(app_configs, **kwargs) -> list[CheckMessage]:
-    """Durations may not overlap across `proxy_for` model"""
+    """Durations may not overlap across `proxy_for` model
+
+    This check needs models to be ready otherwise we would add it
+    to site_consents.register.
+    """
     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",
-                            )
-                        )
+            err, found = inspect_possible_overlap_in_validity_period(cdef1, cdef2, found)
+            if err:
+                errors.append(err)
     return errors
+
+
+def inspect_possible_overlap_in_validity_period(
+    cdef1, cdef2, found
+) -> tuple[Warning | None, list]:
+    """Durations between cdef1 and cdef2 may not overlap
+    if they are using proxies of the same model -- `proxy_for` model.
+
+    This is just a warning as there may be valid cases to allow this.
+    For example, where consent definitions are customized by site.
+    """
+    err = None
+    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:
+                pass
+            else:
+                found.append(sorted([cdef1, cdef2], key=lambda x: x.version))
+                err = Warning(
+                    "Consent definition duration overlap found for same proxy_for_model. "
+                    f"Got {cdef1.name} and {cdef2.name}.",
+                    id="edc_consent.W002",
+                )
+
+    return err, found
+
+
+def inspect_others_using_same_proxy_for_model_with_duplicate_versions(
+    cdef1: ConsentDefinition, used: list[ConsentDefinition]
+) -> tuple[Warning | None, list[ConsentDefinition]]:
+    err = None
+    versions = []
+    opts1 = cdef1.model_cls._meta
+    for cdef2 in site_consents.registry.values():
+        opts2 = cdef2.model_cls._meta
+        if opts2.proxy and opts2.proxy_for_model == opts1.proxy_for_model:
+            versions.append(cdef2.version)
+            used.append(cdef2)
+    if duplicates := [v for v in versions if versions.count(v) > 1]:
+        err = Warning(
+            f"Duplicate consent definition 'version' found for same proxy_for_model. "
+            f"Got '{opts1.proxy_for_model._meta.label_lower}' versions {set(duplicates)}.",
+            id="edc_consent.W001",
+        )
+    return err, used