Split PhoneNumberMixin into OptionalPhoneNumberMixin

funnel/models/account.py

@@ -1916,7 +1916,6 @@ class AccountPhone(PhoneNumberMixin, BaseMixin, Model):
     """A phone number linked to an account."""
 
     __tablename__ = 'account_phone'
-    __phone_optional__ = False
     __phone_unique__ = True
     __phone_is_exclusive__ = True
     __phone_for__ = 'account'

funnel/models/email_address.py

@@ -187,7 +187,7 @@ class EmailAddress(BaseMixin, Model):
     #: Contains the name of the relationship in the :class:`EmailAddress` model
     __backrefs__: ClassVar[set[str]] = set()
     #: These backrefs claim exclusive use of the email address for their linked owner.
-    #: See :class:`EmailAddressMixin` for implementation detail
+    #: See :class:`OptionalEmailAddressMixin` for implementation detail
     __exclusive_backrefs__: ClassVar[set[str]] = set()
 
     #: The email address, centrepiece of this model. Case preserving.
@@ -894,7 +894,7 @@ def _send_refcount_event_remove(
 
 
 def _send_refcount_event_before_delete(
-    _mapper: Any, _connection: Any, target: EmailAddressMixin
+    _mapper: Any, _connection: Any, target: OptionalEmailAddressMixin
 ) -> None:
     if target.email_address:
         emailaddress_refcount_dropping.send(target.email_address)
@@ -908,7 +908,7 @@ def _setup_refcount_events() -> None:
 
 
 def _email_address_mixin_set_validator(
-    target: EmailAddressMixin,
+    target: OptionalEmailAddressMixin,
     value: EmailAddress | None,
     old_value: EmailAddress | None,
     _initiator: Any,
@@ -921,9 +921,9 @@ def _email_address_mixin_set_validator(
                 raise EmailAddressInUseError("This email address it not available")
 
 
-@event.listens_for(EmailAddressMixin, 'mapper_configured', propagate=True)
+@event.listens_for(OptionalEmailAddressMixin, 'mapper_configured', propagate=True)
 def _email_address_mixin_configure_events(
-    _mapper: Any, cls: type[EmailAddressMixin]
+    _mapper: Any, cls: type[OptionalEmailAddressMixin]
 ) -> None:
     event.listen(cls.email_address, 'set', _email_address_mixin_set_validator)
     event.listen(cls, 'before_delete', _send_refcount_event_before_delete)

funnel/models/notification.py

@@ -241,7 +241,6 @@ class SmsMessage(PhoneNumberMixin, BaseMixin, Model):
     """An outbound SMS message."""
 
     __tablename__ = 'sms_message'
-    __phone_optional__ = False
     __phone_unique__ = False
     __phone_is_exclusive__ = False
     phone_number_reference_is_active: bool = False

funnel/models/phone_number.py

@@ -41,6 +41,7 @@
     'canonical_phone_number',
     'phone_blake2b160_hash',
     'PhoneNumber',
+    'OptionalPhoneNumberMixin',
     'PhoneNumberMixin',
 ]
 
@@ -256,7 +257,7 @@ class PhoneNumber(BaseMixin, Model):
     #: Contains the name of the relationship in the :class:`PhoneNumber` model
     __backrefs__: ClassVar[set[str]] = set()
     #: These backrefs claim exclusive use of the phone number for their linked owner.
-    #: See :class:`PhoneNumberMixin` for implementation detail
+    #: See :class:`OptionalPhoneNumberMixin` for implementation detail
     __exclusive_backrefs__: ClassVar[set[str]] = set()
 
     #: The phone number, centrepiece of this model. Stored normalized in E164 format.
@@ -733,7 +734,7 @@ def get_numbers(cls, prefix: str, remove: bool = True) -> set[str]:
 
 
 @declarative_mixin
-class PhoneNumberMixin:
+class OptionalPhoneNumberMixin:
     """
     Mixin class for models that refer to :class:`PhoneNumber`.
 
@@ -757,7 +758,7 @@ class PhoneNumberMixin:
 
     @declared_attr
     @classmethod
-    def phone_number_id(cls) -> Mapped[int]:
+    def phone_number_id(cls) -> Mapped[int | None]:
         """Foreign key to phone_number table."""
         return sa_orm.mapped_column(
             sa.Integer,
@@ -769,7 +770,7 @@ def phone_number_id(cls) -> Mapped[int]:
 
     @declared_attr
     @classmethod
-    def phone_number(cls) -> Mapped[PhoneNumber]:
+    def phone_number(cls) -> Mapped[PhoneNumber | None]:
         """Instance of :class:`PhoneNumber` as a relationship."""
         backref_name = 'used_in_' + cls.__tablename__
         PhoneNumber.__backrefs__.add(backref_name)
@@ -795,17 +796,17 @@ def phone(self) -> str | None:
         return None
 
     @phone.setter
-    def phone(self, value: str | None) -> None:
+    def phone(self, __value: str | None) -> None:
         if self.__phone_for__:
-            if value is not None:
+            if __value is not None:
                 self.phone_number = PhoneNumber.add_for(
-                    getattr(self, self.__phone_for__), value
+                    getattr(self, self.__phone_for__), __value
                 )
             else:
                 self.phone_number = None
         else:
-            if value is not None:
-                self.phone_number = PhoneNumber.add(value)
+            if __value is not None:
+                self.phone_number = PhoneNumber.add(__value)
             else:
                 self.phone_number = None
 
@@ -829,6 +830,37 @@ def transport_hash(self) -> str | None:
         )
 
 
+@declarative_mixin
+class PhoneNumberMixin(OptionalPhoneNumberMixin):
+    """Non-optional version of :class:`OptionalPhoneNumberMixin`."""
+
+    __phone_optional__: ClassVar[bool] = False
+
+    if TYPE_CHECKING:
+
+        @declared_attr
+        @classmethod
+        def phone_number_id(cls) -> Mapped[int]:  # type: ignore[override]
+            ...
+
+        @declared_attr
+        @classmethod
+        def phone_number(cls) -> Mapped[PhoneNumber]:  # type: ignore[override]
+            ...
+
+        @property  # type: ignore[override]
+        def phone(self) -> str:
+            ...
+
+        @phone.setter
+        def phone(self, __value: str) -> None:
+            ...
+
+        @property
+        def transport_hash(self) -> str:
+            ...
+
+
 def _clear_cached_properties(target: PhoneNumber) -> None:
     """Clear cached properties in :class:`PhoneNumber`."""
     for attr in ('parsed', 'formatted'):
@@ -885,7 +917,7 @@ def _send_refcount_event_remove(
 
 
 def _send_refcount_event_before_delete(
-    _mapper: Any, _connection: Any, target: PhoneNumberMixin
+    _mapper: Any, _connection: Any, target: OptionalPhoneNumberMixin
 ) -> None:
     if target.phone_number:
         phonenumber_refcount_dropping.send(target.phone_number)
@@ -899,7 +931,7 @@ def _setup_refcount_events() -> None:
 
 
 def _phone_number_mixin_set_validator(
-    target: PhoneNumberMixin,
+    target: OptionalPhoneNumberMixin,
     value: PhoneNumber | None,
     old_value: PhoneNumber | None,
     _initiator: Any,
@@ -911,9 +943,9 @@ def _phone_number_mixin_set_validator(
             raise PhoneNumberInUseError("This phone number it not available")
 
 
-@event.listens_for(PhoneNumberMixin, 'mapper_configured', propagate=True)
+@event.listens_for(OptionalPhoneNumberMixin, 'mapper_configured', propagate=True)
 def _phone_number_mixin_configure_events(
-    _mapper: Any, cls: type[PhoneNumberMixin]
+    _mapper: Any, cls: type[OptionalPhoneNumberMixin]
 ) -> None:
     event.listen(cls.phone_number, 'set', _phone_number_mixin_set_validator)
     event.listen(cls, 'before_delete', _send_refcount_event_before_delete)

tests/unit/models/phone_number_test.py

@@ -57,7 +57,6 @@ class PhoneLink(models.PhoneNumberMixin, models.BaseMixin, models.Model):
         """Test model connecting PhoneUser to PhoneNumber."""
 
         __tablename__ = 'test_phone_link'
-        __phone_optional__ = False
         __phone_unique__ = True
         __phone_for__ = 'phoneuser'
         __phone_is_exclusive__ = True
@@ -67,12 +66,16 @@ class PhoneLink(models.PhoneNumberMixin, models.BaseMixin, models.Model):
         )
         phoneuser: Mapped[PhoneUser] = relationship(PhoneUser)
 
-    class PhoneDocument(models.PhoneNumberMixin, models.BaseMixin, models.Model):
+    class PhoneDocument(
+        models.OptionalPhoneNumberMixin, models.BaseMixin, models.Model
+    ):
         """Test model unaffiliated to a user that has a phone number attached."""
 
         __tablename__ = 'test_phone_document'
 
-    class PhoneLinkedDocument(models.PhoneNumberMixin, models.BaseMixin, models.Model):
+    class PhoneLinkedDocument(
+        models.OptionalPhoneNumberMixin, models.BaseMixin, models.Model
+    ):
         """Test model that accepts an optional user and an optional phone."""
 
         __tablename__ = 'test_phone_linked_document'