Merge pull request #120 from DataIntegrationGroup/jab-contact-updates

NO TICKET: further contact updates

Author: jirhiker

api/contact.py

@@ -46,6 +46,7 @@
 from services.contact_helper import (
     add_contact,
 )
+from services.lexicon_helper import get_terms_by_category
 from services.query_helper import (
     simple_get_by_id,
     paginated_all_getter,
@@ -107,6 +108,18 @@ def database_error_handler(
             "type": "value_error",
             "input": {"contact_id": payload.contact_id},
         }
+    elif (
+        error_message
+        == 'insert or update on table "contact" violates foreign key constraint "contact_contact_type_fkey"'
+    ):
+        valid_terms = get_terms_by_category("contact_type")
+        valid_contact_types_for_msg = " | ".join(valid_terms)
+        detail = {
+            "loc": ["body", "contact_type"],
+            "msg": f"Invalid contact_type. Valid terms are: {valid_contact_types_for_msg}",
+            "type": "value_error",
+            "input": {"contact_type": payload.contact_type},
+        }
 
     raise PydanticStyleException(status_code=status.HTTP_409_CONFLICT, detail=[detail])
 
@@ -309,40 +322,54 @@ async def update_contact(
     """
     contact = simple_get_by_id(session, Contact, contact_id)
 
+    """
+    if new name is set to None, new organization is unset, and existing organization is already None raise an error
+    if new organization is set to None, new name is unset, and existing name is already None raise an error
+
+    both new name and new organization cannot be set to None - this is a schema restriction
+    """
+    # exclude unsets so only intentional Nones are evaluated
+    payload_excluding_unsets = contact_data.model_dump(exclude_unset=True)
+
     if (
-        contact.name is None
-        and contact_data.name is None
-        and contact_data.organization is None
+        contact.organization is None
+        and payload_excluding_unsets.get("name", "unset") is None
+        and payload_excluding_unsets.get("organization", "unset") == "unset"
     ):
         raise PydanticStyleException(
             status_code=status.HTTP_409_CONFLICT,
             detail=[
                 {
-                    "loc": ["body", "organization"],
-                    "msg": "organization cannot be None if name is None.",
+                    "loc": ["body", "name"],
+                    "msg": "name cannot be set to None because organization is already None.",
                     "type": "value_error",
-                    "input": {"organization": contact_data.organization},
+                    "input": {"name": payload_excluding_unsets.get("name")},
                 }
             ],
         )
     elif (
-        contact.organization is None
-        and contact_data.organization is None
-        and contact_data.name is None
+        contact.name is None
+        and payload_excluding_unsets.get("organization", "unset") is None
+        and payload_excluding_unsets.get("name", "unset") == "unset"
     ):
         raise PydanticStyleException(
             status_code=status.HTTP_409_CONFLICT,
             detail=[
                 {
-                    "loc": ["body", "name"],
-                    "msg": "name cannot be None if organization is None.",
+                    "loc": ["body", "organization"],
+                    "msg": "organization cannot be set to None because name is already None.",
                     "type": "value_error",
-                    "input": {"name": contact_data.name},
+                    "input": {
+                        "organization": payload_excluding_unsets.get("organization")
+                    },
                 }
             ],
         )
 
-    return model_patcher(session, Contact, contact_id, contact_data, user=user)
+    try:
+        return model_patcher(session, Contact, contact_id, contact_data, user=user)
+    except ProgrammingError as e:
+        database_error_handler(contact_data, e)
 
 
 # ====== GET ===================================================================

core/lexicon.json

@@ -105,8 +105,6 @@
   {"categories": [{"name": "county", "description": null}], "term": "Valencia", "definition": "Valencia"},
 
   {"categories": [{"name": "role", "description": null}], "term": "Owner", "definition": "Owner"},
-  {"categories": [{"name": "role", "description": null}], "term": "Primary", "definition": "Primary"},
-  {"categories": [{"name": "role", "description": null}], "term": "Secondary", "definition": "Secondary"},
   {"categories": [{"name": "role", "description": null}], "term": "Manager", "definition": "Manager"},
   {"categories": [{"name": "role", "description": null}], "term": "Operator", "definition": "Operator"},
   {"categories": [{"name": "role", "description": null}], "term": "Driller", "definition": "Driller"},
@@ -119,7 +117,9 @@
 
   {"categories": [{"name": "email_type", "description": null},
                   {"name": "phone_type", "description": null},
-                  {"name": "address_type", "description": null}], "term": "Primary", "definition": "primary"},
+                  {"name": "address_type", "description": null},
+                  {"name": "contact_type", "description": null}], "term": "Primary", "definition": "primary"},
+  {"categories": [{"name": "contact_type", "description": null}], "term": "Secondary", "definition": "secondary"},     
 
   {"categories": [{"name": "email_type", "description": null},
                   {"name": "phone_type", "description": null},

db/contact.py

@@ -13,47 +13,57 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 # ===============================================================================
-from sqlalchemy import Column, Integer, ForeignKey, String
+from sqlalchemy import Integer, ForeignKey, String
 from sqlalchemy.ext.associationproxy import association_proxy
-from sqlalchemy.orm import relationship
+from sqlalchemy.orm import relationship, Mapped, mapped_column
 from sqlalchemy_utils import TSVectorType
+from typing import List
 
 from db.base import Base, AutoBaseMixin, ReleaseMixin, lexicon_term
 
 
 class ThingContactAssociation(Base, AutoBaseMixin):
-    thing_id = Column(
+    thing_id: Mapped[int] = mapped_column(
         Integer, ForeignKey("thing.id", ondelete="CASCADE"), nullable=False
     )
-    contact_id = Column(
+    contact_id: Mapped[int] = mapped_column(
         Integer, ForeignKey("contact.id", ondelete="CASCADE"), nullable=False
     )
 
-    contact = relationship("Contact")
-    thing = relationship("Thing")
+    contact: Mapped[List["Contact"]] = relationship("Contact")
+    thing: Mapped[List["Thing"]] = relationship("Thing")  # noqa: F821
 
 
 class Contact(Base, AutoBaseMixin, ReleaseMixin):
-    name = Column(String(100), nullable=True)
-    role = lexicon_term(nullable=False)
-    organization = Column(String(100), nullable=True)
-    nma_pk_owners = Column(String(100), nullable=True)
-
-    phones = relationship("Phone", back_populates="contact", passive_deletes=True)
-    emails = relationship("Email", back_populates="contact", passive_deletes=True)
-    addresses = relationship("Address", back_populates="contact", passive_deletes=True)
+    name: Mapped[str | None] = mapped_column(String(100))
+    organization: Mapped[str | None] = mapped_column(String(100))
+    role: Mapped[str] = lexicon_term()
+    contact_type: Mapped[str] = lexicon_term()
+    nma_pk_owners: Mapped[str | None] = mapped_column(String(100))
+
+    phones: Mapped[List["Phone"]] = relationship(
+        "Phone", back_populates="contact", passive_deletes=True
+    )
+    emails: Mapped[List["Email"]] = relationship(
+        "Email", back_populates="contact", passive_deletes=True
+    )
+    addresses: Mapped[List["Address"]] = relationship(
+        "Address", back_populates="contact", passive_deletes=True
+    )
 
-    search_vector = Column(
+    search_vector: Mapped[TSVectorType] = mapped_column(
         TSVectorType("name", "role", "organization", "nma_pk_owners")
     )
 
-    author_associations = relationship(
-        "AuthorContactAssociation",
-        back_populates="contact",
-        cascade="all, delete-orphan",
+    author_associations: Mapped[List["AuthorContactAssociation"]] = (  # noqa: F821
+        relationship(
+            "AuthorContactAssociation",
+            back_populates="contact",
+            cascade="all, delete-orphan",
+        )
     )
     authors = association_proxy("author_associations", "author")
-    thing_associations = relationship(
+    thing_associations: Mapped[List["ThingContactAssociation"]] = relationship(
         "ThingContactAssociation",
         back_populates="contact",
         cascade="all, delete-orphan",
@@ -63,42 +73,48 @@ class Contact(Base, AutoBaseMixin, ReleaseMixin):
 
 
 class Phone(Base, AutoBaseMixin, ReleaseMixin):
-    contact_id = Column(
+    contact_id: Mapped[int] = mapped_column(
         Integer, ForeignKey("contact.id", ondelete="CASCADE"), nullable=False
     )
-    phone_number = Column(String(20), nullable=False)
-    phone_type = lexicon_term(nullable=False)
+    phone_number: Mapped[str] = mapped_column(String(20), nullable=False)
+    phone_type: Mapped[str] = lexicon_term(nullable=False)
 
-    contact = relationship("Contact", back_populates="phones", passive_deletes=True)
-    search_vector = Column(TSVectorType("phone_number"))
+    contact: Mapped["Contact"] = relationship(
+        "Contact", back_populates="phones", passive_deletes=True
+    )
+    search_vector: Mapped[TSVectorType] = mapped_column(TSVectorType("phone_number"))
 
 
 class Email(Base, AutoBaseMixin, ReleaseMixin):
-    contact_id = Column(
+    contact_id: Mapped[int] = mapped_column(
         Integer, ForeignKey("contact.id", ondelete="CASCADE"), nullable=False
     )
-    email = Column(String(100), nullable=False)
-    email_type = lexicon_term(nullable=False)
+    email: Mapped[str] = mapped_column(String(100), nullable=False)
+    email_type: Mapped[str] = lexicon_term(nullable=False)
 
-    contact = relationship("Contact", back_populates="emails", passive_deletes=True)
+    contact: Mapped["Contact"] = relationship(
+        "Contact", back_populates="emails", passive_deletes=True
+    )
 
-    search_vector = Column(TSVectorType("email"))
+    search_vector: Mapped[TSVectorType] = mapped_column(TSVectorType("email"))
 
 
 class Address(Base, AutoBaseMixin, ReleaseMixin):
-    contact_id = Column(
+    contact_id: Mapped[int] = mapped_column(
         Integer, ForeignKey("contact.id", ondelete="CASCADE"), nullable=False
     )
-    address_line_1 = Column(String(255), nullable=False)
-    address_line_2 = Column(String(255), nullable=True)
-    city = Column(String(100), nullable=False)
-    state = Column(String(50), nullable=False)
-    postal_code = Column(String(20), nullable=False)
-    country = lexicon_term(nullable=False, default="United States")
-    address_type = lexicon_term(nullable=False)
-
-    contact = relationship("Contact", back_populates="addresses", passive_deletes=True)
-    search_vector = Column(
+    address_line_1: Mapped[str] = mapped_column(String(255), nullable=False)
+    address_line_2: Mapped[str | None] = mapped_column(String(255), nullable=True)
+    city: Mapped[str] = mapped_column(String(100), nullable=False)
+    state: Mapped[str] = mapped_column(String(50), nullable=False)
+    postal_code: Mapped[str] = mapped_column(String(20), nullable=False)
+    country: Mapped[str] = lexicon_term(nullable=False, default="United States")
+    address_type: Mapped[str] = lexicon_term(nullable=False)
+
+    contact: Mapped["Contact"] = relationship(
+        "Contact", back_populates="addresses", passive_deletes=True
+    )
+    search_vector: Mapped[TSVectorType] = mapped_column(
         TSVectorType(
             "address_line_1",
             "address_line_2",

schemas/contact.py

@@ -27,6 +27,20 @@
 # -------- VALIDATORS ----------
 
 
+class ValidateContact(BaseModel):
+    name: str | None = None
+    organization: str | None = None
+
+    @model_validator(mode="before")
+    def check_empty(data: dict) -> dict:
+        if (
+            data.get("name", "unset") is None
+            and data.get("organization", "unset") is None
+        ):
+            raise ValueError("Either name or organization must be provided.")
+        return data
+
+
 class ValidateEmail(BaseModel):
 
     email: str | None = None
@@ -111,15 +125,16 @@ class CreateAddress(BaseCreateModel):
 #     thing_id: int
 
 
-class CreateContact(BaseCreateModel):
+class CreateContact(BaseCreateModel, ValidateContact):
     """
     Schema for creating a contact.
     """
 
     thing_id: int
     name: str | None = None
-    role: str
     organization: str | None = None
+    role: str
+    contact_type: str = "Primary"
     # description: str | None = None
     # email: str | None = None
     # phone: str | None = None
@@ -128,12 +143,6 @@ class CreateContact(BaseCreateModel):
     phones: list[CreatePhone] | None = None
     addresses: list[CreateAddress] | None = None
 
-    @model_validator(mode="before")
-    def check_empty(data: dict) -> dict:
-        if data.get("name", None) is None and data.get("organization", None) is None:
-            raise ValueError("Either name or organization must be provided.")
-        return data
-
 
 # -------- RESPONSE ----------
 
@@ -179,8 +188,10 @@ class ContactResponse(BaseResponseModel):
     Response schema for contact details.
     """
 
-    name: str
+    name: str | None
+    organization: str | None
     role: str
+    contact_type: str
     emails: List[EmailResponse] = []
     phones: List[PhoneResponse] = []
     addresses: List[AddressResponse] = []
@@ -198,13 +209,14 @@ class ContactResponse(BaseResponseModel):
 
 
 # -------- UPDATE ----------
-class UpdateContact(BaseUpdateModel):
+class UpdateContact(BaseUpdateModel, ValidateContact):
     """
     Schema for updating contact information.
     """
 
     name: str | None = None
     role: str | None = None
+    contact_type: str | None = None
     thing_id: int | None = None
     organization: str | None = None
     # email: str | None = None

tests/conftest.py

@@ -262,6 +262,7 @@ def contact(water_well_thing):
             release_status="private",
             name="Test Contact",
             role="Owner",
+            contact_type="Primary",
             organization="Test Organization",
         )
         session.add(contact)
@@ -335,6 +336,7 @@ def second_contact():
             release_status="private",
             name="Test Second Contact",
             role="Owner",
+            contact_type="Primary",
             organization=None,
         )
         session.add(contact)
@@ -410,6 +412,7 @@ def third_contact():
             release_status="private",
             name=None,
             role="Owner",
+            contact_type="Primary",
             organization="Third Organization",
         )
         session.add(contact)