Skip to content

feat(users): add endpoint to update users' marketing updates by email#526

Open
dspachos wants to merge 9 commits into
devfrom
feature/AI-user-receive-marketing-updates
Open

feat(users): add endpoint to update users' marketing updates by email#526
dspachos wants to merge 9 commits into
devfrom
feature/AI-user-receive-marketing-updates

Conversation

@dspachos
Copy link
Copy Markdown
Contributor

@dspachos dspachos commented May 20, 2026

Greptile Summary

This PR adds a receive_marketing_updates boolean field to the user model (with a matching migration), a new PUT /users/by-email/marketing-updates endpoint to bulk-update that flag by normalised email, and a new HubSpotService that syncs the marketable status to HubSpot's CRM on every change.

  • The DB schema, migration, and Pydantic schemas are all consistent and correctly aligned.
  • Both the new bulk endpoint and the modified update_user path commit the database before calling HubSpot; a HubSpot failure returns 502 while leaving the database already updated — and in update_user the retry-skip guard (!= previous_marketing_updates) prevents a subsequent retry from ever reaching HubSpot again.
  • The bulk endpoint calls HubSpot with each user's raw aliased email (e.g. user+tag@example.com) rather than the normalised canonical address used to look them up, which will create duplicate HubSpot contacts instead of updating the existing one.

Confidence Score: 3/5

The core user-update and bulk-by-email paths both commit to the database before calling HubSpot; a transient HubSpot error leaves the two systems permanently out of sync with no recovery path.

Two distinct defects exist in the changed code: the update_user retry-skip guard means a failed HubSpot call is unrecoverable without a manual DB revert, and the bulk endpoint calls HubSpot with aliased addresses that will silently create duplicate contacts. Both affect the core purpose of this feature.

app/api/users.py — specifically the HubSpot call placement in update_user (after db.commit) and the email passed to upsert_contact_marketable_status in update_users_marketing_updates_by_email.

Important Files Changed

Filename Overview
app/api/users.py Adds new PUT /by-email/marketing-updates endpoint and HubSpot sync to update_user; both paths commit the DB before calling HubSpot, creating silent inconsistency on failure. The by-email endpoint also calls HubSpot with aliased emails instead of the canonical address.
app/services/hubspot.py New HubSpotService that wraps the CRM batch upsert endpoint; raises HTTPException(502) on API failure. Logic looks correct; the hs_marketable_status string encoding matches HubSpot's expected enum format.
app/migrations/versions/20260520_160000_add_receive_marketing_updates_to_users.py Migration correctly adds receive_marketing_updates boolean column with server_default=false and matching downgrade; aligned with the model definition.
app/db/models.py Adds receive_marketing_updates Boolean column with default=False, nullable=False, and server_default consistent with the migration.
app/schemas/models.py Adds receive_marketing_updates to UserCreate (Optional), UserUpdate (Optional), User response (default False), and new UserMarketingUpdatesByEmailUpdate schema; all fields look correct.
app/core/config.py Adds HUBSPOT_TOKEN setting with empty string default; the empty default causes a 500 error if the token is not configured before the endpoint is used.
tests/test_users.py Adds tests for the new marketing-updates endpoint and updated user creation; HubSpot calls are mocked correctly.

Sequence Diagram

sequenceDiagram
    participant Client
    participant API as PUT /by-email/marketing-updates
    participant DB as Database
    participant HS as HubSpotService

    Client->>API: "PUT {email, receive_marketing_updates}"
    API->>DB: Query users by normalised email
    DB-->>API: "[user1 (foo+a@ex.com), user2 (foo+b@ex.com)]"
    API->>DB: UPDATE receive_marketing_updates for all
    API->>DB: db.commit() DB updated
    API->>DB: db.refresh(users)
    loop Each user (raw aliased email)
        API->>HS: "upsert_contact(email=foo+a@ex.com)"
        Note over HS: Aliased email may create new HubSpot contact
        HS-->>API: 200 OK or 502 error
        Note over API: If 502 DB committed but client receives error
    end
    API-->>Client: 200 [users] or 502 (DB already committed)
Loading

Reviews (1): Last reviewed commit: "feat(users): add receive_marketing_updat..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

@dspachos dspachos marked this pull request as ready for review May 20, 2026 18:03
Copilot AI review requested due to automatic review settings May 20, 2026 18:03
Comment thread app/api/users.py Outdated
Comment thread app/api/users.py
Comment thread app/api/users.py
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds marketing-consent tracking to users and introduces admin APIs that update the flag and sync it to HubSpot.

Changes:

  • Add receive_marketing_updates column + schema fields and default behavior (false).
  • Add PUT /users/by-email/marketing-updates (system-admin) and HubSpot sync on updates (by-id and by-email).
  • Minor formatting/refactors in tests and scripts.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_users.py Adds assertions/tests for the new marketing-updates behavior and HubSpot sync.
tests/test_trial_access.py Updates DBUser mocks to include the new attribute.
tests/test_public_models_missing.py Formatting-only updates to with (...) blocks and long assertions.
scripts/check_missing_models.py Formatting-only updates for line wrapping.
app/services/hubspot.py Introduces HubSpot client/service for upserting contact marketable status.
app/schemas/models.py Adds receive_marketing_updates to user schemas + new request payload schema.
app/migrations/versions/20260520_160000_add_receive_marketing_updates_to_users.py Alembic migration adding the new non-null boolean column with default.
app/main.py Formatting-only change in OpenAPI customization.
app/db/models.py Adds receive_marketing_updates column to DBUser.
app/core/config.py Adds HUBSPOT_TOKEN setting.
app/api/users.py Adds by-email marketing-updates endpoint and triggers HubSpot sync when the field changes.
app/api/public.py Minor formatting-only tweaks.
Comments suppressed due to low confidence (1)

app/api/users.py:963

  • update_user commits and refreshes the DB user before calling HubSpot. If the HubSpot call fails, the client receives an error response even though the user record was already updated in the DB. Align the transactional semantics by either making HubSpot sync best-effort after commit, or performing HubSpot sync before commit and rolling back on failure.
    if (
        user_update.receive_marketing_updates is not None
        and user_update.receive_marketing_updates != previous_marketing_updates
    ):
        hubspot = HubSpotService()
        await hubspot.upsert_contact_marketable_status(
            email=db_user.email, enabled=db_user.receive_marketing_updates
        )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/api/users.py Outdated
Comment thread app/api/users.py Outdated
Comment thread app/services/hubspot.py Outdated
Comment thread app/services/hubspot.py
Agent-Logs-Url: https://github.com/amazeeio/amazee.ai/sessions/bd59c38c-40db-4097-ba17-ec52137a3905

Co-authored-by: dspachos <6309422+dspachos@users.noreply.github.com>
@dspachos dspachos requested a review from pmelab May 22, 2026 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants