Skip to content

NO TICKET: further contact updates #120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jirhiker merged 4 commits into from
Sep 4, 2025
Merged

NO TICKET: further contact updates #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
afdbecc
fix: fix not-null name/organization constraints
jacob-a-brown Sep 4, 2025
7428d07
feat: type hint contact tables with Mapped
jacob-a-brown Sep 4, 2025
aaf5141
feat: enable contact to have editable contact type
jacob-a-brown Sep 4, 2025
8ba1f46
feat: make primary/secondary contact type, not role
jacob-a-brown Sep 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 40 additions & 13 deletions api/contact.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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])

Expand Down Expand Up @@ -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 ===================================================================
Expand Down
6 changes: 3 additions & 3 deletions core/lexicon.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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},
Expand Down
98 changes: 57 additions & 41 deletions db/contact.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment on lines +58 to 65

@ksmuczynski ksmuczynski Sep 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this reference to Author?

@jirhiker jirhiker Sep 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data system needs to store bibliographic data, the author of a paper, map, report, etc.

Since we already had a contact table it made sense to use it for contact info for authors of bureau publication.

A super brittle aspect of the bureau's current data system is its handling of publications. There are multiple systems that need access to this data, e.g., website, bookstore, GIS, publications group, etc Currently, the connections are tenuous at best. Ocotillo should serve as the central authority to manage and share this data.

Ocotillo's current implementation is only an idea of how this could work. there are other better approaches im sure

thing_associations = relationship(
thing_associations: Mapped[List["ThingContactAssociation"]] = relationship(
"ThingContactAssociation",
back_populates="contact",
cascade="all, delete-orphan",
Expand All @@ -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)
Comment thread
jacob-a-brown marked this conversation as resolved.
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",
Expand Down
32 changes: 22 additions & 10 deletions schemas/contact.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 ----------

Expand Down Expand Up @@ -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] = []
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -335,6 +336,7 @@ def second_contact():
release_status="private",
name="Test Second Contact",
role="Owner",
contact_type="Primary",
organization=None,
)
session.add(contact)
Expand Down Expand Up @@ -410,6 +412,7 @@ def third_contact():
release_status="private",
name=None,
role="Owner",
contact_type="Primary",
organization="Third Organization",
)
session.add(contact)
Expand Down
Loading
Loading