diff --git a/api/contact.py b/api/contact.py index ba597c881..e7ea8ed4e 100644 --- a/api/contact.py +++ b/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 =================================================================== diff --git a/core/lexicon.json b/core/lexicon.json index 29be34926..0a0031747 100644 --- a/core/lexicon.json +++ b/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}, diff --git a/db/contact.py b/db/contact.py index 5a54a67ae..9a1d06f2e 100644 --- a/db/contact.py +++ b/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", diff --git a/schemas/contact.py b/schemas/contact.py index f9de822cf..22d835240 100644 --- a/schemas/contact.py +++ b/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 diff --git a/tests/conftest.py b/tests/conftest.py index 5be97022d..5e9283d6d 100644 --- a/tests/conftest.py +++ b/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) diff --git a/tests/test_contact.py b/tests/test_contact.py index 12e7705a9..b505ff4cb 100644 --- a/tests/test_contact.py +++ b/tests/test_contact.py @@ -6,7 +6,7 @@ from db import Contact, Address, Email, Phone from main import app from tests import client, cleanup_post_test, cleanup_patch_test, override_authentication -from schemas.contact import ValidateEmail, ValidatePhone, CreateContact +from schemas.contact import ValidateEmail, ValidatePhone, ValidateContact import pytest from pydantic import ValidationError @@ -36,7 +36,7 @@ def test_check_empty_fields(): with pytest.raises( ValueError, match="Either name or organization must be provided." ): - CreateContact(name=None, organization=None) + ValidateContact(name=None, organization=None) def test_validate_phone(): @@ -71,6 +71,7 @@ def test_add_contact(spring_thing): "release_status": "private", "name": "Test Contact 2", "role": "Owner", + "contact_type": "Primary", "organization": "Well Owner LLC", "thing_id": spring_thing.id, "emails": [ @@ -109,6 +110,8 @@ def test_add_contact(spring_thing): assert data["release_status"] == payload["release_status"] assert data["name"] == payload["name"] assert data["role"] == payload["role"] + assert data["contact_type"] == payload["contact_type"] + assert data["organization"] == payload["organization"] assert len(data["emails"]) == 1 assert "id" in data["emails"][0] @@ -156,6 +159,7 @@ def test_add_contact_409_bad_thing_id(): "release_status": "private", "name": "Test Contact 3", "role": "Owner", + "contact_type": "Primary", "organization": "Well Owner LLC", "thing_id": bad_thing_id, "emails": [ @@ -366,7 +370,9 @@ def test_get_contacts(contact, email, address, phone): ) assert data["items"][0]["name"] == contact.name assert data["items"][0]["role"] == contact.role + assert data["items"][0]["contact_type"] == contact.contact_type assert data["items"][0]["release_status"] == contact.release_status + assert data["items"][0]["organization"] == contact.organization assert len(data["items"][0]["emails"]) == 1 assert data["items"][0]["emails"][0]["id"] == email.id @@ -412,7 +418,9 @@ def test_get_contact_by_id(contact, email, address, phone): assert data["created_at"] == contact.created_at.isoformat().replace("+00:00", "Z") assert data["name"] == contact.name assert data["role"] == contact.role + assert data["contact_type"] == contact.contact_type assert data["release_status"] == contact.release_status + assert data["organization"] == contact.organization assert len(data["emails"]) == 1 assert data["emails"][0]["id"] == email.id @@ -735,9 +743,12 @@ def test_patch_contact_409_null_name(second_contact): assert response.status_code == 409 data = response.json() assert data["detail"][0]["loc"] == ["body", "name"] - assert data["detail"][0]["msg"] == "name cannot be None if organization is None." + assert ( + data["detail"][0]["msg"] + == "name cannot be set to None because organization is already None." + ) assert data["detail"][0]["type"] == "value_error" - assert data["detail"][0]["input"] == {"name": None} + assert data["detail"][0]["input"] == {"name": payload["name"]} def test_patch_contact_409_null_organization(third_contact): @@ -750,9 +761,26 @@ def test_patch_contact_409_null_organization(third_contact): assert response.status_code == 409 data = response.json() assert data["detail"][0]["loc"] == ["body", "organization"] - assert data["detail"][0]["msg"] == "organization cannot be None if name is None." + assert ( + data["detail"][0]["msg"] + == "organization cannot be set to None because name is already None." + ) + assert data["detail"][0]["type"] == "value_error" + assert data["detail"][0]["input"] == {"organization": payload["organization"]} + + +def test_patch_contact_409_bad_contact_type(third_contact): + payload = {"contact_type": "Tertiary"} + response = client.patch(f"/contact/{third_contact.id}", json=payload) + assert response.status_code == 409 + data = response.json() + assert data["detail"][0]["loc"] == ["body", "contact_type"] + assert ( + data["detail"][0]["msg"] + == "Invalid contact_type. Valid terms are: Primary | Secondary" + ) assert data["detail"][0]["type"] == "value_error" - assert data["detail"][0]["input"] == {"organization": None} + assert data["detail"][0]["input"] == {"contact_type": payload["contact_type"]} def test_patch_email(email): diff --git a/transfers/contact_transfer.py b/transfers/contact_transfer.py index 4df88ec68..5e6eff8e7 100644 --- a/transfers/contact_transfer.py +++ b/transfers/contact_transfer.py @@ -29,7 +29,7 @@ def extract_owner_role(comment): # if "Director" in comment: # return "Director" - return "Primary" + return "Owner" def transfer_contacts(session): @@ -46,7 +46,7 @@ def transfer_contacts(session): # TODO: extract role from OwnerComment # role = extract_owner_role(row.OwnerComment) - role = "Primary" + role = "Owner" # TODO: put in guards for null values # name OR organization must be defined, otherwise skip @@ -59,6 +59,7 @@ def transfer_contacts(session): contact1 = Contact( name=f"{row.FirstName} {row.LastName}", role=role, + contact_type="Primary", organization=row.Company, # assumes organization applies to both contacts nma_pk_owners=row.OwnerKey, ) @@ -109,7 +110,8 @@ def transfer_contacts(session): print(f"Transferring second contact for PointID {row.PointID}") contact2 = Contact( name=f"{row.SecondFirstName} {row.SecondLastName}", - role="Secondary", + role="Owner", + contact_type="Secondary", organization=row.Company, # Assumes organization applies to both contacts nma_pk_owners=row.OwnerKey, )