diff --git a/alembic/versions/b1c4d7e29a03_add_crm_campaigns.py b/alembic/versions/b1c4d7e29a03_add_crm_campaigns.py new file mode 100644 index 0000000..f9bebef --- /dev/null +++ b/alembic/versions/b1c4d7e29a03_add_crm_campaigns.py @@ -0,0 +1,94 @@ +"""add crm_campaigns + +Mirrors advertising campaigns pushed from MaskanX so the CRM can report +spend and cost per lead alongside the leads those campaigns produced. + +Money columns are integers in minor currency units (paise, cents), matching +what MaskanX sends and what Meta's budget fields use. A Numeric here would +add a second convention and a rounding step between two systems that +currently agree exactly. + +Revision ID: b1c4d7e29a03 +Revises: eca4681c9f66 +Create Date: 2026-08-03 +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +revision: str = 'b1c4d7e29a03' +down_revision: Union[str, None] = 'eca4681c9f66' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.create_table( + 'crm_campaigns', + sa.Column('id', sa.String(length=36), nullable=False), + sa.Column('tenant_id', sa.String(length=36), nullable=False), + sa.Column('provider', sa.String(length=80), nullable=False), + sa.Column('external_id', sa.String(length=255), nullable=False), + sa.Column('name', sa.String(length=240), nullable=False), + sa.Column('status', sa.String(length=40), nullable=False), + sa.Column('objective', sa.String(length=80), nullable=True), + sa.Column('channel', sa.String(length=80), nullable=True), + sa.Column('currency', sa.String(length=8), nullable=True), + sa.Column('daily_budget', sa.Integer(), nullable=True), + sa.Column('spend', sa.Integer(), nullable=False), + sa.Column('impressions', sa.Integer(), nullable=False), + sa.Column('clicks', sa.Integer(), nullable=False), + sa.Column('leads', sa.Integer(), nullable=False), + sa.Column('cost_per_lead', sa.Integer(), nullable=True), + sa.Column('metrics_from', sa.String(length=20), nullable=True), + sa.Column('metrics_to', sa.String(length=20), nullable=True), + sa.Column('synced_at', sa.DateTime(timezone=True), nullable=False), + sa.Column('metadata_json', sa.JSON(), nullable=False), + sa.Column('created_at', sa.DateTime(timezone=True), nullable=False), + sa.Column('updated_at', sa.DateTime(timezone=True), nullable=False), + sa.ForeignKeyConstraint(['tenant_id'], ['crm_tenants.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id'), + # MaskanX re-pushes the same campaign as its figures change, so the + # provider's own id is what identifies it, not our surrogate key. + sa.UniqueConstraint( + 'tenant_id', 'provider', 'external_id', + name='uq_crm_campaigns_provider_external', + ), + ) + op.create_index( + 'ix_crm_campaigns_tenant_status', + 'crm_campaigns', + ['tenant_id', 'status'], + ) + + # Ad attribution promoted out of crm_leads.attributes into real columns, + # so counting leads per campaign is an indexed join rather than a JSON + # scan. Nullable: most leads do not come from an ad. + op.add_column( + 'crm_leads', + sa.Column('campaign_external_id', sa.String(length=255), nullable=True), + ) + op.add_column( + 'crm_leads', + sa.Column('adset_external_id', sa.String(length=255), nullable=True), + ) + op.add_column( + 'crm_leads', + sa.Column('ad_external_id', sa.String(length=255), nullable=True), + ) + op.create_index( + 'ix_crm_leads_campaign_external_id', + 'crm_leads', + ['campaign_external_id'], + ) + + +def downgrade() -> None: + op.drop_index('ix_crm_leads_campaign_external_id', table_name='crm_leads') + op.drop_column('crm_leads', 'ad_external_id') + op.drop_column('crm_leads', 'adset_external_id') + op.drop_column('crm_leads', 'campaign_external_id') + op.drop_index('ix_crm_campaigns_tenant_status', table_name='crm_campaigns') + op.drop_table('crm_campaigns') diff --git a/alembic/versions/c2e5f8a41b76_add_custom_properties.py b/alembic/versions/c2e5f8a41b76_add_custom_properties.py new file mode 100644 index 0000000..b52a4f9 --- /dev/null +++ b/alembic/versions/c2e5f8a41b76_add_custom_properties.py @@ -0,0 +1,62 @@ +"""add crm_custom_properties + +Fields an operator (or an agent, through MCP) adds without a migration. + +The definition lives here; the values live in each record's existing +`attributes` JSON. That split is the whole point: adding a field is an +INSERT, so it can happen mid-conversation and be usable immediately. + +Revision ID: c2e5f8a41b76 +Revises: b1c4d7e29a03 +Create Date: 2026-08-04 +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +revision: str = 'c2e5f8a41b76' +down_revision: Union[str, None] = 'b1c4d7e29a03' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.create_table( + 'crm_custom_properties', + sa.Column('id', sa.String(length=36), nullable=False), + sa.Column('tenant_id', sa.String(length=36), nullable=False), + sa.Column('object_type', sa.String(length=40), nullable=False), + sa.Column('name', sa.String(length=80), nullable=False), + sa.Column('label', sa.String(length=160), nullable=False), + sa.Column('description', sa.Text(), nullable=True), + sa.Column('data_type', sa.String(length=24), nullable=False), + sa.Column('options', sa.JSON(), nullable=False), + sa.Column('is_required', sa.Boolean(), nullable=False), + sa.Column('created_by', sa.String(length=120), nullable=True), + sa.Column('created_at', sa.DateTime(timezone=True), nullable=False), + sa.Column('updated_at', sa.DateTime(timezone=True), nullable=False), + sa.ForeignKeyConstraint(['tenant_id'], ['crm_tenants.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id'), + # `name` is the key written into each record's attributes, so it has + # to be unique per object type — two definitions sharing a key would + # fight over the same stored value. + sa.UniqueConstraint( + 'tenant_id', 'object_type', 'name', + name='uq_crm_custom_properties_object_name', + ), + ) + op.create_index( + 'ix_crm_custom_properties_tenant_object', + 'crm_custom_properties', + ['tenant_id', 'object_type'], + ) + + +def downgrade() -> None: + op.drop_index( + 'ix_crm_custom_properties_tenant_object', + table_name='crm_custom_properties', + ) + op.drop_table('crm_custom_properties') diff --git a/app/api/campaigns.py b/app/api/campaigns.py new file mode 100644 index 0000000..845df3b --- /dev/null +++ b/app/api/campaigns.py @@ -0,0 +1,72 @@ +"""Advertising campaigns, as mirrored from MaskanX. + +Read-only. The CRM does not run campaigns — MaskanX does, and pushes their +figures here through `POST /integrations/campaigns`. This exists so that +"what did this campaign cost us per lead" can be answered next to the leads +themselves, without leaving the CRM. + +Each campaign carries two lead counts: `leads` as Meta attributes them, and +`crm_leads` as this database actually holds them. They routinely differ — +Meta attributes late, and a lead can be entered here by hand — and the gap +is worth seeing rather than hiding behind one number. +""" + +from fastapi import APIRouter +from sqlalchemy import func, select + +from app.core.security import CurrentUser, Database +from app.models import Campaign, Lead +from app.schemas import CampaignOut + +router = APIRouter(prefix="/campaigns", tags=["Campaigns"]) + + +@router.get("", response_model=list[CampaignOut]) +def list_campaigns(user: CurrentUser, db: Database) -> list[CampaignOut]: + campaigns = list( + db.scalars( + select(Campaign) + .where(Campaign.tenant_id == user.tenant_id) + .order_by(Campaign.spend.desc()), + ), + ) + + # One grouped count rather than a query per campaign: a workspace with + # a hundred campaigns should still be one round trip. + counts = dict( + db.execute( + select( + Lead.campaign_external_id, + func.count(Lead.id), + ) + .where( + Lead.tenant_id == user.tenant_id, + Lead.campaign_external_id.is_not(None), + ) + .group_by(Lead.campaign_external_id), + ).all(), + ) + + return [ + CampaignOut( + id=campaign.id, + provider=campaign.provider, + external_id=campaign.external_id, + name=campaign.name, + status=campaign.status, + objective=campaign.objective, + channel=campaign.channel, + currency=campaign.currency, + daily_budget=campaign.daily_budget, + spend=campaign.spend, + impressions=campaign.impressions, + clicks=campaign.clicks, + leads=campaign.leads, + cost_per_lead=campaign.cost_per_lead, + metrics_from=campaign.metrics_from, + metrics_to=campaign.metrics_to, + synced_at=campaign.synced_at, + crm_leads=counts.get(campaign.external_id, 0), + ) + for campaign in campaigns + ] diff --git a/app/api/contacts.py b/app/api/contacts.py index b468770..d54dc89 100644 --- a/app/api/contacts.py +++ b/app/api/contacts.py @@ -18,6 +18,7 @@ from app.services import ( add_audit, add_event, apply_updates, + validated_attributes, contact_to_out, model_or_404, verify_optional_reference, @@ -83,7 +84,11 @@ def create_contact(payload: ContactCreate, user: Writer, db: Database) -> Contac user.tenant_id, ) verify_optional_reference(db, User, payload.owner_id, user.tenant_id) - contact = Contact(tenant_id=user.tenant_id, **payload.model_dump()) + values = payload.model_dump() + values["attributes"] = validated_attributes( + db, user.tenant_id, "contact", {}, values.get("attributes"), + ) + contact = Contact(tenant_id=user.tenant_id, **values) db.add(contact) try: db.flush() @@ -145,6 +150,11 @@ def update_contact( user.tenant_id, ) verify_optional_reference(db, User, values.get("owner_id"), user.tenant_id) + if "attributes" in values: + values["attributes"] = validated_attributes( + db, user.tenant_id, "contact", contact.attributes, + values["attributes"], + ) apply_updates(contact, values) add_audit( db, diff --git a/app/api/integrations.py b/app/api/integrations.py index cfd9eb6..6b704f2 100644 --- a/app/api/integrations.py +++ b/app/api/integrations.py @@ -10,6 +10,7 @@ from sqlalchemy.orm import selectinload from app.core.config import get_settings from app.core.security import Admin, Database, hash_service_key from app.models import ( + Campaign, Contact, ExternalLink, IdempotencyRecord, @@ -21,6 +22,8 @@ from app.models import ( Tenant, ) from app.schemas import ( + IntegrationCampaignRequest, + IntegrationCampaignResponse, IntegrationCredentialOut, IntegrationKeyCreate, IntegrationKeyResponse, @@ -155,6 +158,21 @@ def _authenticate_integration(db: Database, raw_key: str | None) -> IntegrationC return credential +def _attribution(campaign: dict, key: str) -> str | None: + """Read one ad id out of a lead's campaign block. + + `campaign` is free-form JSON from the sender, so a value may be absent, + null, or a number. Anything not usable as an id becomes None rather + than being coerced — a lead attributed to the string "None" would be + worse than one attributed to nothing. + """ + value = campaign.get(key) + if value is None: + return None + text = str(value).strip() + return text or None + + @router.get("/status", response_model=IntegrationStatusResponse) def integration_status( db: Database, @@ -333,6 +351,12 @@ def ingest_lead( stage_id=first_stage.id, source_id=source.id, position=next_lead_position(db, credential.tenant_id, first_stage.id), + # Kept in `attributes` as well, so nothing the sender included is + # lost, but the three ids that identify the ad are promoted to + # columns — that is what makes "leads for this campaign" a join. + campaign_external_id=_attribution(payload.campaign, "campaign_id"), + adset_external_id=_attribution(payload.campaign, "adset_id"), + ad_external_id=_attribution(payload.campaign, "ad_id"), attributes={"campaign": payload.campaign, **payload.metadata}, ) db.add(lead) @@ -378,3 +402,65 @@ def ingest_lead( ) db.commit() return result + + +@router.post("/campaigns", response_model=IntegrationCampaignResponse) +def ingest_campaign( + payload: IntegrationCampaignRequest, + db: Database, + integration_key: Annotated[str | None, Header(alias="X-Integration-Key")] = None, +) -> IntegrationCampaignResponse: + """Create or update one advertising campaign's figures. + + Deliberately an upsert keyed on `(provider, external_id)` rather than an + idempotent create like `/leads`. A lead is an event that happened once; + a campaign's figures change every time they are read, and MaskanX pushes + the same campaign repeatedly as its spend grows. An Idempotency-Key here + would either reject the update or return a stale answer. + """ + credential = _authenticate_integration(db, integration_key) + + campaign = db.scalar( + select(Campaign).where( + Campaign.tenant_id == credential.tenant_id, + Campaign.provider == payload.provider, + Campaign.external_id == payload.external_id, + ), + ) + created = campaign is None + if campaign is None: + campaign = Campaign( + tenant_id=credential.tenant_id, + provider=payload.provider, + external_id=payload.external_id, + ) + db.add(campaign) + + campaign.name = payload.name + campaign.status = payload.status + campaign.objective = payload.objective + campaign.channel = payload.channel + campaign.currency = payload.currency + campaign.daily_budget = payload.daily_budget + campaign.spend = payload.spend + campaign.impressions = payload.impressions + campaign.clicks = payload.clicks + campaign.leads = payload.leads + campaign.cost_per_lead = payload.cost_per_lead + campaign.metrics_from = payload.metrics_from + campaign.metrics_to = payload.metrics_to + campaign.metadata_json = payload.metadata + campaign.synced_at = datetime.now(UTC) + + add_event( + db, + tenant_id=credential.tenant_id, + topic="campaign.synced", + payload={ + "provider": payload.provider, + "external_id": payload.external_id, + "spend": payload.spend, + }, + ) + db.commit() + return IntegrationCampaignResponse(campaign_id=campaign.id, created=created) diff --git a/app/api/properties.py b/app/api/properties.py new file mode 100644 index 0000000..3373491 --- /dev/null +++ b/app/api/properties.py @@ -0,0 +1,205 @@ +"""Custom property definitions — fields added without a migration. + +Two ways in, on purpose: + +- `/properties` for a logged-in person, from the CRM's own settings screen. +- `/integrations/properties` for MaskanX, authenticated by an integration + key. That is what an agent reaches through MCP when a conversation asks + for a new field. + +Both go through the same validation. An agent gets no shortcut around the +rules a person is held to — if anything it needs them more, since it will +happily invent a property name from a half-sentence. +""" + +from typing import Annotated + +from fastapi import APIRouter, Header, HTTPException, status +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError + +from app.core.security import CurrentUser, Database +from app.models import CustomProperty +from app.properties import PropertyError, validate_definition +from app.schemas import ( + CustomPropertyCreate, + CustomPropertyOut, + CustomPropertyUpdate, +) + +router = APIRouter(prefix="/properties", tags=["Properties"]) +integration_router = APIRouter(prefix="/integrations", tags=["Integrations"]) + + +def _list_for(db, tenant_id: str, object_type: str | None) -> list[CustomProperty]: + query = select(CustomProperty).where(CustomProperty.tenant_id == tenant_id) + if object_type: + query = query.where(CustomProperty.object_type == object_type) + return list(db.scalars(query.order_by(CustomProperty.label))) + + +def _create( + db, + tenant_id: str, + payload: CustomPropertyCreate, + created_by: str, +) -> CustomProperty: + try: + validate_definition( + name=payload.name, + object_type=payload.object_type, + data_type=payload.data_type, + options=payload.options, + ) + except PropertyError as exc: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=str(exc), + ) from exc + + prop = CustomProperty( + tenant_id=tenant_id, + object_type=payload.object_type, + name=payload.name, + label=payload.label, + description=payload.description, + data_type=payload.data_type, + options=payload.options, + is_required=payload.is_required, + created_by=created_by, + ) + db.add(prop) + try: + db.commit() + except IntegrityError as exc: + db.rollback() + # Told plainly rather than as a constraint name: an agent that + # re-runs a request needs to know the field already exists, not + # that a unique index fired. + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=( + f"A property named {payload.name!r} already exists on " + f"{payload.object_type}." + ), + ) from exc + return prop + + +@router.get("", response_model=list[CustomPropertyOut]) +def list_properties( + user: CurrentUser, + db: Database, + object_type: str | None = None, +) -> list[CustomProperty]: + return _list_for(db, user.tenant_id, object_type) + + +@router.post("", response_model=CustomPropertyOut, status_code=status.HTTP_201_CREATED) +def create_property( + payload: CustomPropertyCreate, + user: CurrentUser, + db: Database, +) -> CustomProperty: + return _create(db, user.tenant_id, payload, created_by=user.email) + + +@router.patch("/{property_id}", response_model=CustomPropertyOut) +def update_property( + property_id: str, + payload: CustomPropertyUpdate, + user: CurrentUser, + db: Database, +) -> CustomProperty: + """Update a property's presentation, never its identity or type. + + `name` and `data_type` are not updatable — see `CustomPropertyUpdate`. + Narrowing an enumeration is allowed even though records may already + hold a value that is no longer offered: those values stay readable and + simply cannot be chosen again, which is the behaviour an operator + retiring an option expects. + """ + prop = db.scalar( + select(CustomProperty).where( + CustomProperty.id == property_id, + CustomProperty.tenant_id == user.tenant_id, + ), + ) + if prop is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) + + if payload.options is not None and prop.data_type != "enumeration": + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Options only apply to an enumeration, not to a {prop.data_type}.", + ) + if payload.options is not None and not payload.options: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail="An enumeration needs at least one option.", + ) + + for field, value in payload.model_dump(exclude_unset=True).items(): + if value is not None: + setattr(prop, field, value) + db.commit() + return prop + + +@router.delete("/{property_id}", status_code=status.HTTP_204_NO_CONTENT) +def delete_property(property_id: str, user: CurrentUser, db: Database) -> None: + """Remove a definition. + + Values already written into records are left where they are. Rewriting + every record to strip a key would be a large, silent, irreversible + write in response to a small request; leaving them means undoing a + mistaken delete is just recreating the property. + """ + prop = db.scalar( + select(CustomProperty).where( + CustomProperty.id == property_id, + CustomProperty.tenant_id == user.tenant_id, + ), + ) + if prop is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) + db.delete(prop) + db.commit() + + +# --- integration key access, for MaskanX and its MCP tools --- + + +@integration_router.get("/properties", response_model=list[CustomPropertyOut]) +def list_properties_via_integration( + db: Database, + object_type: str | None = None, + integration_key: Annotated[str | None, Header(alias="X-Integration-Key")] = None, +) -> list[CustomProperty]: + from app.api.integrations import _authenticate_integration + + credential = _authenticate_integration(db, integration_key) + properties = _list_for(db, credential.tenant_id, object_type) + db.commit() + return properties + + +@integration_router.post( + "/properties", + response_model=CustomPropertyOut, + status_code=status.HTTP_201_CREATED, +) +def create_property_via_integration( + payload: CustomPropertyCreate, + db: Database, + integration_key: Annotated[str | None, Header(alias="X-Integration-Key")] = None, +) -> CustomProperty: + from app.api.integrations import _authenticate_integration + + credential = _authenticate_integration(db, integration_key) + return _create( + db, + credential.tenant_id, + payload, + created_by=f"integration:{credential.name}", + ) diff --git a/app/api/router.py b/app/api/router.py index ae6dc62..e031d9e 100644 --- a/app/api/router.py +++ b/app/api/router.py @@ -1,6 +1,16 @@ from fastapi import APIRouter -from app.api import activities, auth, catalog, contacts, dashboard, integrations, leads +from app.api import ( + activities, + auth, + campaigns, + catalog, + contacts, + dashboard, + integrations, + properties, + leads, +) api_router = APIRouter(prefix="/api/v1") api_router.include_router(auth.router) @@ -9,5 +19,8 @@ api_router.include_router(contacts.router) api_router.include_router(leads.router) api_router.include_router(activities.router) api_router.include_router(catalog.router) +api_router.include_router(campaigns.router) +api_router.include_router(properties.router) +api_router.include_router(properties.integration_router) api_router.include_router(integrations.router) diff --git a/app/models.py b/app/models.py index 1a89599..a302c2d 100644 --- a/app/models.py +++ b/app/models.py @@ -279,6 +279,14 @@ class Lead(Base, TimestampMixin): type_id: Mapped[str | None] = mapped_column( ForeignKey("crm_lead_types.id", ondelete="SET NULL"), ) + # Ad attribution, promoted out of `attributes` into real columns. + # Meta stamps every lead-ad submission with the campaign, ad set and ad + # that produced it, so "spend and leads for this ad" is a join rather + # than a JSON scan — which also keeps it working on SQLite and + # PostgreSQL alike. Null for leads that did not come from an ad. + campaign_external_id: Mapped[str | None] = mapped_column(String(255), index=True) + adset_external_id: Mapped[str | None] = mapped_column(String(255)) + ad_external_id: Mapped[str | None] = mapped_column(String(255)) attributes: Mapped[dict[str, Any]] = mapped_column(JSON, default=dict, nullable=False) contact: Mapped[Contact | None] = relationship() @@ -558,3 +566,116 @@ class IdempotencyRecord(Base): nullable=False, ) + + +class Campaign(Base, TimestampMixin): + """An advertising campaign mirrored from MaskanX. + + The CRM does not run campaigns; it reports on them next to the leads + they produced. MaskanX pushes each campaign's figures here so that + "what did this campaign cost us per lead" is answerable without leaving + the CRM. + + Money is stored in **minor** currency units (paise, cents), matching + what MaskanX sends and what Meta's own budget fields use. Storing a + decimal here would introduce a second convention and a rounding step + between two systems that currently agree exactly. + """ + + __tablename__ = "crm_campaigns" + __table_args__ = ( + UniqueConstraint( + "tenant_id", + "provider", + "external_id", + name="uq_crm_campaigns_provider_external", + ), + Index("ix_crm_campaigns_tenant_status", "tenant_id", "status"), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=new_id) + tenant_id: Mapped[str] = mapped_column( + ForeignKey("crm_tenants.id", ondelete="CASCADE"), + nullable=False, + ) + # "maskanx" for now; kept general so a second ad platform does not need + # a second table. + provider: Mapped[str] = mapped_column(String(80), nullable=False) + external_id: Mapped[str] = mapped_column(String(255), nullable=False) + name: Mapped[str] = mapped_column(String(240), nullable=False) + status: Mapped[str] = mapped_column(String(40), nullable=False, default="unknown") + objective: Mapped[str | None] = mapped_column(String(80), nullable=True) + channel: Mapped[str | None] = mapped_column(String(80), nullable=True) + currency: Mapped[str | None] = mapped_column(String(8), nullable=True) + + daily_budget: Mapped[int | None] = mapped_column(Integer, nullable=True) + spend: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + impressions: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + clicks: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + # Leads as Meta counts them. The CRM's own lead count can differ — a + # lead may arrive here that Meta has not attributed yet, or vice versa — + # so both are kept rather than one being derived from the other. + leads: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + cost_per_lead: Mapped[int | None] = mapped_column(Integer, nullable=True) + + metrics_from: Mapped[str | None] = mapped_column(String(20), nullable=True) + metrics_to: Mapped[str | None] = mapped_column(String(20), nullable=True) + synced_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + default=now_utc, + nullable=False, + ) + metadata_json: Mapped[dict[str, Any]] = mapped_column( + JSON, default=dict, nullable=False, + ) + + +class CustomProperty(Base, TimestampMixin): + """A field an operator added, without a schema migration. + + Modelled on how HubSpot treats properties: the definition lives in a + table, and the values live in the record's existing `attributes` JSON. + That is the whole point — adding a field is an INSERT, not a migration, + so an agent can create one mid-conversation and the next record can use + it immediately. + + The cost of that choice is that values are not typed by the database, + so `data_type` is enforced in application code on write. Anything that + writes a custom property value has to go through that validation; a + direct UPDATE to `attributes` bypasses it. + + `name` is the stable key stored inside `attributes`; `label` is what a + person reads. They are kept separate for the same reason HubSpot keeps + them separate: renaming a label must not orphan every value already + written under the old key. + """ + + __tablename__ = "crm_custom_properties" + __table_args__ = ( + UniqueConstraint( + "tenant_id", + "object_type", + "name", + name="uq_crm_custom_properties_object_name", + ), + Index("ix_crm_custom_properties_tenant_object", "tenant_id", "object_type"), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=new_id) + tenant_id: Mapped[str] = mapped_column( + ForeignKey("crm_tenants.id", ondelete="CASCADE"), + nullable=False, + ) + # Which kind of record carries it: contact, lead, organization, campaign. + object_type: Mapped[str] = mapped_column(String(40), nullable=False) + name: Mapped[str] = mapped_column(String(80), nullable=False) + label: Mapped[str] = mapped_column(String(160), nullable=False) + description: Mapped[str | None] = mapped_column(Text) + data_type: Mapped[str] = mapped_column(String(24), nullable=False, default="string") + # Allowed values for an enumeration. Empty for every other type. + options: Mapped[list[str]] = mapped_column(JSON, default=list, nullable=False) + is_required: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False) + # Where it came from. An agent-created field is marked so a human can + # tell at a glance which fields they defined and which one a + # conversation produced. + created_by: Mapped[str | None] = mapped_column(String(120)) diff --git a/app/properties.py b/app/properties.py new file mode 100644 index 0000000..904af82 --- /dev/null +++ b/app/properties.py @@ -0,0 +1,173 @@ +"""Validation for custom property definitions and their values. + +Custom property values live in a JSON column, so the database enforces +nothing about them. Everything that writes one has to come through here, or +the `data_type` on the definition is decoration. + +Pure functions: no session, no I/O, so the rules can be tested directly. +""" + +from __future__ import annotations + +from datetime import date, datetime +from typing import Any + +# A property name becomes a key inside a record's `attributes` JSON and is +# referenced by agents and API callers, so it is restricted to something +# that survives being a JSON key, a query parameter and a column header. +NAME_PATTERN = r"^[a-z][a-z0-9_]{1,63}$" + +DATA_TYPES = ("string", "number", "boolean", "date", "enumeration") + +OBJECT_TYPES = ("contact", "lead", "organization", "campaign") + + +class PropertyError(ValueError): + """Raised when a definition or a value is not usable.""" + + +def validate_definition( + *, + name: str, + object_type: str, + data_type: str, + options: list[str] | None, +) -> None: + """Check a property definition before it is stored. + + Rejects an enumeration with no options: a field whose only valid values + are none of them cannot ever be filled in, and an agent creating one by + mistake would produce a form nobody can submit. + """ + import re + + if object_type not in OBJECT_TYPES: + raise PropertyError( + f"Unknown object type {object_type!r}. " + f"Available: {', '.join(OBJECT_TYPES)}.", + ) + if data_type not in DATA_TYPES: + raise PropertyError( + f"Unknown data type {data_type!r}. Available: {', '.join(DATA_TYPES)}.", + ) + if not re.match(NAME_PATTERN, name or ""): + raise PropertyError( + f"{name!r} is not a usable property name. Use lowercase letters, " + f"digits and underscores, starting with a letter, e.g. " + f"'budget_range'.", + ) + if data_type == "enumeration" and not options: + raise PropertyError( + "An enumeration needs at least one option; otherwise no value " + "could ever be valid for it.", + ) + if data_type != "enumeration" and options: + raise PropertyError( + f"Options only apply to an enumeration, not to a {data_type}.", + ) + + +def coerce_value(definition: Any, value: Any) -> Any: + """Return `value` in the shape its definition calls for. + + Coerces rather than merely checking, because callers are agents and + HTTP clients: a number arriving as the string "42" is a well-formed + intention expressed loosely, and rejecting it would be pedantry. What + is rejected is anything genuinely ambiguous — "quite large" is not a + number by any reading. + + `None` clears the value and is always allowed here; whether a property + may be empty is `is_required`'s business, checked separately, because + the two questions have different answers on a partial update. + """ + if value is None: + return None + + data_type = definition.data_type + + if data_type == "string": + return str(value) + + if data_type == "number": + try: + number = float(value) + except (TypeError, ValueError) as exc: + raise PropertyError( + f"{definition.label!r} expects a number, not {value!r}.", + ) from exc + # Keep whole numbers whole: 3.0 stored for a count reads oddly and + # round-trips into JSON as 3.0 forever. + return int(number) if number.is_integer() else number + + if data_type == "boolean": + if isinstance(value, bool): + return value + text = str(value).strip().lower() + if text in ("true", "yes", "1"): + return True + if text in ("false", "no", "0"): + return False + raise PropertyError( + f"{definition.label!r} expects true or false, not {value!r}.", + ) + + if data_type == "date": + if isinstance(value, (date, datetime)): + return value.isoformat() + try: + return date.fromisoformat(str(value)[:10]).isoformat() + except ValueError as exc: + raise PropertyError( + f"{definition.label!r} expects a date as YYYY-MM-DD, " + f"not {value!r}.", + ) from exc + + if data_type == "enumeration": + text = str(value) + if text not in (definition.options or []): + raise PropertyError( + f"{text!r} is not one of the allowed values for " + f"{definition.label!r}: " + f"{', '.join(definition.options or []) or 'none'}.", + ) + return text + + raise PropertyError(f"Unknown data type {data_type!r}.") + + +def apply_values( + definitions: list[Any], + current: dict[str, Any], + incoming: dict[str, Any], +) -> dict[str, Any]: + """Merge validated custom property values into a record's attributes. + + Only keys that have a definition are touched. An unknown key is + rejected rather than stored: silently accepting one would let a typo + ("budjet_range") sit in the database looking like data, and the whole + reason for a registry is that the set of fields is knowable. + + Required properties are enforced only against what is being written, + not against the merged result — a partial update that does not mention + a required field is not an attempt to clear it. + """ + by_name = {definition.name: definition for definition in definitions} + + unknown = sorted(set(incoming) - set(by_name)) + if unknown: + known = ", ".join(sorted(by_name)) or "none" + raise PropertyError( + f"No such property: {', '.join(unknown)}. Defined here: {known}.", + ) + + merged = dict(current) + for name, raw in incoming.items(): + definition = by_name[name] + coerced = coerce_value(definition, raw) + if coerced is None and definition.is_required: + raise PropertyError(f"{definition.label!r} is required and cannot be empty.") + if coerced is None: + merged.pop(name, None) + else: + merged[name] = coerced + return merged diff --git a/app/schemas.py b/app/schemas.py index 9ba1ad4..f428869 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -409,3 +409,103 @@ class IntegrationStatusResponse(ApiModel): credential_id: str credential_name: str key_prefix: str + + +class IntegrationCampaignRequest(ApiModel): + """One advertising campaign's current figures, pushed from MaskanX. + + Money is in **minor** currency units (paise, cents), matching what Meta + and MaskanX both use. Sending a decimal would introduce a second + convention and a rounding step between systems that agree exactly. + """ + + provider: str = Field(default="maskanx", min_length=2, max_length=80) + external_id: str = Field(min_length=1, max_length=255) + name: str = Field(min_length=1, max_length=240) + status: str = Field(default="unknown", max_length=40) + objective: str | None = Field(default=None, max_length=80) + channel: str | None = Field(default=None, max_length=80) + currency: str | None = Field(default=None, max_length=8) + + daily_budget: int | None = Field(default=None, ge=0) + spend: int = Field(default=0, ge=0) + impressions: int = Field(default=0, ge=0) + clicks: int = Field(default=0, ge=0) + leads: int = Field(default=0, ge=0) + # Sent rather than derived so the CRM shows exactly the figure MaskanX + # shows. Recomputing it here from spend and leads would drift whenever + # the two systems rounded differently. + cost_per_lead: int | None = Field(default=None, ge=0) + + metrics_from: str | None = Field(default=None, max_length=20) + metrics_to: str | None = Field(default=None, max_length=20) + metadata: dict[str, Any] = Field(default_factory=dict) + + +class IntegrationCampaignResponse(ApiModel): + campaign_id: str + created: bool + + +class CampaignOut(ApiModel): + id: str + provider: str + external_id: str + name: str + status: str + objective: str | None + channel: str | None + currency: str | None + daily_budget: int | None + spend: int + impressions: int + clicks: int + leads: int + cost_per_lead: int | None + metrics_from: str | None + metrics_to: str | None + synced_at: datetime + # Leads the CRM itself holds for this campaign. Kept alongside `leads` + # (Meta's count) rather than replacing it: a lead can exist here that + # Meta has not attributed yet, and the gap between the two is worth + # seeing rather than hiding. + crm_leads: int = 0 + + +class CustomPropertyCreate(ApiModel): + object_type: str = Field(min_length=2, max_length=40) + name: str = Field(min_length=2, max_length=80) + label: str = Field(min_length=1, max_length=160) + description: str | None = None + data_type: str = Field(default="string", max_length=24) + options: list[str] = Field(default_factory=list) + is_required: bool = False + + +class CustomPropertyUpdate(ApiModel): + """A partial update. + + `name`, `object_type` and `data_type` are absent on purpose. They are + the identity and meaning of the field: changing a name orphans every + value already stored under it, and changing a type leaves stored values + that no longer satisfy it. Delete and recreate instead — deliberately + more effort, because it loses data. + """ + + label: str | None = Field(default=None, min_length=1, max_length=160) + description: str | None = None + options: list[str] | None = None + is_required: bool | None = None + + +class CustomPropertyOut(ApiModel): + id: str + object_type: str + name: str + label: str + description: str | None + data_type: str + options: list[str] + is_required: bool + created_by: str | None + created_at: datetime diff --git a/app/services.py b/app/services.py index 2a42c37..9d1c4c9 100644 --- a/app/services.py +++ b/app/services.py @@ -247,3 +247,52 @@ def next_lead_position(db: Session, tenant_id: str, stage_id: str) -> int: def decimal_or_zero(value: Decimal | None) -> Decimal: return value or Decimal("0") + + +def validated_attributes( + db: Session, + tenant_id: str, + object_type: str, + current: dict[str, Any], + incoming: dict[str, Any] | None, +) -> dict[str, Any]: + """Merge incoming custom property values after validating them. + + Custom property values live in a JSON column, so the database enforces + nothing about them — every write path has to come through here or the + `data_type` on a definition is decoration. + + Values whose property has no definition are rejected rather than + stored. Silently accepting one would let a typo sit in the database + looking like data, and a registry whose set of fields is not actually + the set of fields is worse than none. + """ + from sqlalchemy import select + + from app.models import CustomProperty + from app.properties import PropertyError, apply_values + + if incoming is None: + return current + + definitions = list( + db.scalars( + select(CustomProperty).where( + CustomProperty.tenant_id == tenant_id, + CustomProperty.object_type == object_type, + ), + ), + ) + # Nothing defined yet: keep the previous behaviour of storing whatever + # arrives, so adding this validation does not break workspaces that + # already use `attributes` as a free-form bag. + if not definitions: + return incoming + + try: + return apply_values(definitions, current, incoming) + except PropertyError as exc: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=str(exc), + ) from exc diff --git a/tests/test_campaigns.py b/tests/test_campaigns.py new file mode 100644 index 0000000..29bb6b0 --- /dev/null +++ b/tests/test_campaigns.py @@ -0,0 +1,192 @@ +"""Advertising campaigns mirrored from MaskanX. + +The CRM does not run campaigns; MaskanX pushes their figures here so that +spend and cost per lead can be read next to the leads they produced. + +The behaviour that matters is that pushing the same campaign again updates +it rather than duplicating it — MaskanX re-pushes every few minutes as +spend grows — and that lead attribution actually links the two. +""" +from fastapi.testclient import TestClient +import pytest + + +@pytest.fixture() +def service_key(client: TestClient, auth_headers: dict[str, str]) -> str: + created = client.post( + "/api/v1/integrations/credentials", + headers=auth_headers, + json={"name": "MaskanX"}, + ) + assert created.status_code == 201, created.text + return created.json()["key"] + + +def _campaign(**overrides) -> dict: + payload = { + "provider": "maskanx", + "external_id": "camp_1", + "name": "Q3 lead gen", + "status": "live", + "objective": "OUTCOME_LEADS", + "channel": "facebook", + "currency": "INR", + "daily_budget": 50000, + "spend": 46000, + "impressions": 12000, + "clicks": 300, + "leads": 4, + "cost_per_lead": 11500, + "metrics_from": "2026-07-05", + "metrics_to": "2026-08-03", + } + payload.update(overrides) + return payload + + +def _push(client, service_key, **overrides): + return client.post( + "/api/v1/integrations/campaigns", + headers={"X-Integration-Key": service_key}, + json=_campaign(**overrides), + ) + + +def test_a_campaign_can_be_pushed_and_read_back( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + pushed = _push(client, service_key) + assert pushed.status_code == 200, pushed.text + assert pushed.json()["created"] is True + + listed = client.get("/api/v1/campaigns", headers=auth_headers) + assert listed.status_code == 200, listed.text + body = listed.json() + assert len(body) == 1 + assert body[0]["name"] == "Q3 lead gen" + assert body[0]["spend"] == 46000 + assert body[0]["cost_per_lead"] == 11500 + + +def test_pushing_the_same_campaign_again_updates_it( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + """MaskanX re-pushes every few minutes as spend grows.""" + _push(client, service_key, spend=46000) + again = _push(client, service_key, spend=90000, leads=9, cost_per_lead=10000) + + assert again.json()["created"] is False + + body = client.get("/api/v1/campaigns", headers=auth_headers).json() + assert len(body) == 1 + assert body[0]["spend"] == 90000 + assert body[0]["leads"] == 9 + + +def test_two_campaigns_are_kept_apart( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + _push(client, service_key, external_id="camp_1", name="First") + _push(client, service_key, external_id="camp_2", name="Second") + + body = client.get("/api/v1/campaigns", headers=auth_headers).json() + + assert {row["name"] for row in body} == {"First", "Second"} + + +def test_campaigns_are_ordered_by_spend( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + _push(client, service_key, external_id="small", name="Small", spend=1000) + _push(client, service_key, external_id="big", name="Big", spend=99000) + + body = client.get("/api/v1/campaigns", headers=auth_headers).json() + + assert [row["name"] for row in body] == ["Big", "Small"] + + +def test_pushing_a_campaign_needs_an_integration_key(client: TestClient) -> None: + response = client.post("/api/v1/integrations/campaigns", json=_campaign()) + + assert response.status_code == 401 + + +def test_reading_campaigns_needs_a_logged_in_user(client: TestClient) -> None: + assert client.get("/api/v1/campaigns").status_code == 401 + + +def test_negative_spend_is_rejected(client: TestClient, service_key: str) -> None: + """Money that went backwards is a bug upstream, not a figure to store.""" + assert _push(client, service_key, spend=-1).status_code == 422 + + +# --- attribution --- + + +def _lead(client, service_key, external_id, campaign_id): + return client.post( + "/api/v1/integrations/leads", + headers={ + "X-Integration-Key": service_key, + "Idempotency-Key": f"key-{external_id}", + }, + json={ + "provider": "maskanx", + "external_id": external_id, + "first_name": "Asha", + "last_name": "Menon", + "email": f"{external_id}@example.com", + "campaign": { + "campaign_id": campaign_id, + "adset_id": "meta_set_1", + "ad_id": "meta_ad_1", + }, + }, + ) + + +def test_leads_are_counted_against_the_campaign_that_produced_them( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + _push(client, service_key, external_id="camp_1", leads=9) + assert _lead(client, service_key, "lead_1", "camp_1").status_code == 200 + assert _lead(client, service_key, "lead_2", "camp_1").status_code == 200 + + body = client.get("/api/v1/campaigns", headers=auth_headers).json() + + # Meta's count and the CRM's own count are kept separately: they + # routinely differ, and the gap is worth seeing. + assert body[0]["leads"] == 9 + assert body[0]["crm_leads"] == 2 + + +def test_a_lead_from_another_campaign_is_not_counted( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + _push(client, service_key, external_id="camp_1") + _lead(client, service_key, "lead_1", "camp_2") + + body = client.get("/api/v1/campaigns", headers=auth_headers).json() + + assert body[0]["crm_leads"] == 0 + + +def test_a_lead_with_no_campaign_does_not_break_the_count( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + """Most leads do not come from an ad.""" + _push(client, service_key, external_id="camp_1") + response = client.post( + "/api/v1/integrations/leads", + headers={"X-Integration-Key": service_key, "Idempotency-Key": "manual-1"}, + json={ + "provider": "maskanx", + "external_id": "walk_in_1", + "first_name": "Walk", + "last_name": "In", + }, + ) + assert response.status_code == 200 + + body = client.get("/api/v1/campaigns", headers=auth_headers).json() + assert body[0]["crm_leads"] == 0 diff --git a/tests/test_properties.py b/tests/test_properties.py new file mode 100644 index 0000000..4c3e56a --- /dev/null +++ b/tests/test_properties.py @@ -0,0 +1,372 @@ +"""Custom properties: fields added without a migration. + +The definitions live in a table and the values in each record's JSON +`attributes`, so nothing in the database enforces a type. These tests are +mostly about the validation that stands in for that, and about the two ways +in — a logged-in person, and an integration key, which is what an agent +reaches through MCP. +""" +import pytest +from fastapi.testclient import TestClient + +from app.properties import PropertyError, coerce_value, validate_definition + + +class _Definition: + """Stands in for a stored definition in the pure-function tests.""" + + def __init__(self, data_type, options=None, label="Field", is_required=False): + self.data_type = data_type + self.options = options or [] + self.label = label + self.is_required = is_required + self.name = "field" + + +# --- defining --- + + +def test_a_name_must_be_a_usable_key(): + """It becomes a JSON key and a query parameter, not just a label.""" + for bad in ("Budget Range", "1st_field", "budget-range", "", "A"): + with pytest.raises(PropertyError): + validate_definition( + name=bad, object_type="contact", data_type="string", options=None, + ) + + +def test_a_good_name_is_accepted(): + validate_definition( + name="budget_range", object_type="contact", data_type="string", options=None, + ) + + +def test_an_enumeration_with_no_options_is_refused(): + """No value could ever be valid for it.""" + with pytest.raises(PropertyError, match="at least one option"): + validate_definition( + name="stage", object_type="lead", data_type="enumeration", options=[], + ) + + +def test_options_on_a_non_enumeration_are_refused(): + with pytest.raises(PropertyError, match="only apply to an enumeration"): + validate_definition( + name="notes", object_type="lead", data_type="string", options=["a"], + ) + + +def test_an_unknown_object_type_lists_the_real_ones(): + with pytest.raises(PropertyError, match="contact"): + validate_definition( + name="x", object_type="spaceship", data_type="string", options=None, + ) + + +# --- values --- + + +def test_a_number_arriving_as_a_string_is_accepted(): + """Callers are agents and HTTP clients; "42" is a number loosely typed.""" + assert coerce_value(_Definition("number"), "42") == 42 + + +def test_a_whole_number_stays_whole(): + assert coerce_value(_Definition("number"), "3.0") == 3 + + +def test_a_decimal_survives(): + assert coerce_value(_Definition("number"), "3.5") == 3.5 + + +def test_something_genuinely_unnumeric_is_refused(): + with pytest.raises(PropertyError, match="expects a number"): + coerce_value(_Definition("number"), "quite large") + + +@pytest.mark.parametrize("value", ["true", "Yes", "1", True]) +def test_truthy_spellings_of_boolean(value): + assert coerce_value(_Definition("boolean"), value) is True + + +@pytest.mark.parametrize("value", ["false", "No", "0", False]) +def test_falsy_spellings_of_boolean(value): + assert coerce_value(_Definition("boolean"), value) is False + + +def test_an_ambiguous_boolean_is_refused(): + with pytest.raises(PropertyError, match="true or false"): + coerce_value(_Definition("boolean"), "maybe") + + +def test_a_date_is_normalised_to_iso(): + assert coerce_value(_Definition("date"), "2026-08-04") == "2026-08-04" + + +def test_a_datetime_string_keeps_only_the_date(): + assert coerce_value(_Definition("date"), "2026-08-04T10:30:00Z") == "2026-08-04" + + +def test_an_unparseable_date_says_what_it_wanted(): + with pytest.raises(PropertyError, match="YYYY-MM-DD"): + coerce_value(_Definition("date"), "next tuesday") + + +def test_an_enumeration_value_must_be_offered(): + definition = _Definition("enumeration", options=["hot", "warm"], label="Temp") + + assert coerce_value(definition, "hot") == "hot" + with pytest.raises(PropertyError, match="hot, warm"): + coerce_value(definition, "lukewarm") + + +def test_none_always_clears_a_value(): + """Whether it may be empty is is_required's business, checked elsewhere.""" + assert coerce_value(_Definition("number", is_required=True), None) is None + + +# --- the API --- + + +@pytest.fixture() +def service_key(client: TestClient, auth_headers: dict[str, str]) -> str: + created = client.post( + "/api/v1/integrations/credentials", + headers=auth_headers, + json={"name": "MaskanX"}, + ) + return created.json()["key"] + + +def _definition(**overrides) -> dict: + payload = { + "object_type": "contact", + "name": "budget_range", + "label": "Budget range", + "data_type": "enumeration", + "options": ["50-70 lakh", "70-90 lakh", "1 crore+"], + } + payload.update(overrides) + return payload + + +def test_a_property_can_be_created_and_listed( + client: TestClient, auth_headers: dict, +) -> None: + created = client.post( + "/api/v1/properties", headers=auth_headers, json=_definition(), + ) + assert created.status_code == 201, created.text + + listed = client.get("/api/v1/properties", headers=auth_headers).json() + assert [p["name"] for p in listed] == ["budget_range"] + assert listed[0]["options"] == ["50-70 lakh", "70-90 lakh", "1 crore+"] + + +def test_properties_can_be_filtered_by_object_type( + client: TestClient, auth_headers: dict, +) -> None: + client.post("/api/v1/properties", headers=auth_headers, json=_definition()) + client.post( + "/api/v1/properties", + headers=auth_headers, + json=_definition( + object_type="lead", name="site_visit_done", + label="Site visit done", data_type="boolean", options=[], + ), + ) + + contacts = client.get( + "/api/v1/properties?object_type=contact", headers=auth_headers, + ).json() + + assert [p["name"] for p in contacts] == ["budget_range"] + + +def test_the_same_property_twice_is_a_readable_conflict( + client: TestClient, auth_headers: dict, +) -> None: + """An agent re-running a request needs to know it already exists.""" + client.post("/api/v1/properties", headers=auth_headers, json=_definition()) + again = client.post( + "/api/v1/properties", headers=auth_headers, json=_definition(), + ) + + assert again.status_code == 409 + assert "already exists" in again.json()["error"]["message"] + + +def test_an_invalid_definition_is_refused_with_a_reason( + client: TestClient, auth_headers: dict, +) -> None: + response = client.post( + "/api/v1/properties", + headers=auth_headers, + json=_definition(name="Budget Range"), + ) + + assert response.status_code == 422 + assert "budget_range" in response.json()["error"]["message"] + + +def test_a_label_can_be_changed_without_orphaning_values( + client: TestClient, auth_headers: dict, +) -> None: + created = client.post( + "/api/v1/properties", headers=auth_headers, json=_definition(), + ).json() + + updated = client.patch( + f"/api/v1/properties/{created['id']}", + headers=auth_headers, + json={"label": "Buying budget"}, + ) + + assert updated.status_code == 200 + assert updated.json()["label"] == "Buying budget" + # The key values are stored under is untouched. + assert updated.json()["name"] == "budget_range" + + +def test_a_property_can_be_deleted(client: TestClient, auth_headers: dict) -> None: + created = client.post( + "/api/v1/properties", headers=auth_headers, json=_definition(), + ).json() + + assert ( + client.delete( + f"/api/v1/properties/{created['id']}", headers=auth_headers, + ).status_code + == 204 + ) + assert client.get("/api/v1/properties", headers=auth_headers).json() == [] + + +def test_reading_properties_needs_a_login(client: TestClient) -> None: + assert client.get("/api/v1/properties").status_code == 401 + + +# --- through an integration key, which is what MCP uses --- + + +def test_an_integration_can_create_a_property( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + created = client.post( + "/api/v1/integrations/properties", + headers={"X-Integration-Key": service_key}, + json=_definition(), + ) + + assert created.status_code == 201, created.text + # Marked as machine-made, so a human can tell which fields a + # conversation produced. + assert created.json()["created_by"].startswith("integration:") + + +def test_an_integration_gets_no_shortcut_around_validation( + client: TestClient, service_key: str, +) -> None: + response = client.post( + "/api/v1/integrations/properties", + headers={"X-Integration-Key": service_key}, + json=_definition(data_type="enumeration", options=[]), + ) + + assert response.status_code == 422 + + +def test_an_integration_property_is_visible_to_people( + client: TestClient, auth_headers: dict, service_key: str, +) -> None: + """A field created in chat has to show up in the CRM's own screens.""" + client.post( + "/api/v1/integrations/properties", + headers={"X-Integration-Key": service_key}, + json=_definition(), + ) + + listed = client.get("/api/v1/properties", headers=auth_headers).json() + + assert [p["name"] for p in listed] == ["budget_range"] + + +def test_creating_a_property_needs_a_valid_key(client: TestClient) -> None: + response = client.post("/api/v1/integrations/properties", json=_definition()) + + assert response.status_code == 401 + + +# --- values on records --- + + +def _contact(client, auth_headers, attributes): + return client.post( + "/api/v1/contacts", + headers=auth_headers, + json={ + "first_name": "Asha", + "last_name": "Menon", + "primary_email": "asha@example.com", + "attributes": attributes, + }, + ) + + +def test_a_value_is_validated_against_its_definition( + client: TestClient, auth_headers: dict, +) -> None: + client.post("/api/v1/properties", headers=auth_headers, json=_definition()) + + good = _contact(client, auth_headers, {"budget_range": "70-90 lakh"}) + assert good.status_code == 201, good.text + assert good.json()["attributes"]["budget_range"] == "70-90 lakh" + + +def test_a_value_outside_the_enumeration_is_refused( + client: TestClient, auth_headers: dict, +) -> None: + client.post("/api/v1/properties", headers=auth_headers, json=_definition()) + + response = _contact(client, auth_headers, {"budget_range": "a few rupees"}) + + assert response.status_code == 422 + assert "70-90 lakh" in response.json()["error"]["message"] + + +def test_an_undefined_property_is_refused_rather_than_stored( + client: TestClient, auth_headers: dict, +) -> None: + """A typo left in the database looks exactly like data.""" + client.post("/api/v1/properties", headers=auth_headers, json=_definition()) + + response = _contact(client, auth_headers, {"budjet_range": "70-90 lakh"}) + + assert response.status_code == 422 + assert "budjet_range" in response.json()["error"]["message"] + + +def test_attributes_stay_free_form_when_nothing_is_defined( + client: TestClient, auth_headers: dict, +) -> None: + """Adding this validation must not break workspaces already using them.""" + response = _contact(client, auth_headers, {"anything": "at all"}) + + assert response.status_code == 201 + assert response.json()["attributes"] == {"anything": "at all"} + + +def test_a_number_property_is_coerced_on_the_way_in( + client: TestClient, auth_headers: dict, +) -> None: + client.post( + "/api/v1/properties", + headers=auth_headers, + json=_definition( + name="bedrooms", label="Bedrooms", data_type="number", options=[], + ), + ) + + response = _contact(client, auth_headers, {"bedrooms": "3"}) + + assert response.json()["attributes"]["bedrooms"] == 3