Skip to content

Add postcard sending - #8107

Merged
aapeliv merged 20 commits into
developfrom
backend/feature/mail-verification
Apr 4, 2026
Merged

Add postcard sending#8107
aapeliv merged 20 commits into
developfrom
backend/feature/mail-verification

Conversation

@aapeliv

@aapeliv aapeliv commented Mar 15, 2026

Copy link
Copy Markdown
Member

Describe briefly what this PR is doing and why.

Testing

Explain how you tested this PR and give clear steps so the reviewer can replicate.

Backend checklist

  • Added tests for any new code or added a regression test if fixing a bug
  • Run the backend locally and it works
  • Added migrations if there are any database changes, rebased onto develop if necessary for linear migration history

Web frontend checklist

  • There are no console warnings when running the app
  • Added tests where relevant
  • Clicked around my changes running locally and it works
  • Checked Desktop, Mobile and Tablet screen sizes

For maintainers

  • Maintainers can push commits to my branch
  • Maintainers can merge this PR for me

@vercel

vercel Bot commented Mar 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
couchers Ready Ready Preview Apr 4, 2026 4:16am

Request Review

@couchersbot

couchersbot Bot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 2m 19s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0140_add_mypostcard_job_id.py) is SAFE. It adds a nullable VARCHAR column to postal_verification_attempts — a metadata-only operation in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Migration Review

One of our most common issues is migrations that don't work with existing data in production. This is very hard to catch with CI, so we have this bot to check for potential problems.

Overall Risk: SAFE

Files Reviewed

  • 0141_add_mypostcard_job_id.py (New) - Adds nullable INTEGER column mypostcard_job_id and renames country to country_code in postal_verification_attempts

Findings

No issues found.

  • The mypostcard_job_id column is nullable with no default, a metadata-only ADD COLUMN in PostgreSQL, instant and non-blocking.
  • The country to country_code rename via ALTER COLUMN RENAME is also a metadata-only operation, no table rewrite, no data loss.
  • The postal_verification_attempts table is small (low traffic), so lock contention is not a concern.
  • Downgrade correctly reverses both operations in the right order.
  • The previous issue where migration 0118 was modified has been resolved. The rename is now correctly done in this new migration (0141).

Summary

This migration is safe to run against production. Both operations are metadata-only with no data loss risk, correct ordering, and a complete downgrade path.

@couchersbot

couchersbot Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 1m 5s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0140_add_mypostcard_job_id.py) is SAFE. It adds a nullable VARCHAR column to postal_verification_attempts — a metadata-only operation in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 2m 21s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0140_add_mypostcard_job_id.py) is SAFE. It adds a nullable VARCHAR column to postal_verification_attempts — a metadata-only operation in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 44s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0140_add_mypostcard_job_id.py) is SAFE. It adds a nullable VARCHAR column to postal_verification_attempts — a metadata-only operation in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 1m 56s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0140_add_mypostcard_job_id.py) is SAFE. It adds a nullable VARCHAR column to postal_verification_attempts — a metadata-only operation in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 2m 50s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0140_add_mypostcard_job_id.py) is SAFE. It adds a nullable VARCHAR column to postal_verification_attempts — a metadata-only operation in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. Full review posted as a separate comment.

@aapeliv
aapeliv force-pushed the backend/feature/mail-verification branch from f72ce96 to f159b8c Compare March 26, 2026 03:36
@couchersbot

couchersbot Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 2m 34s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0140_add_mypostcard_job_id.py) is SAFE. It adds a nullable VARCHAR column to postal_verification_attempts — a metadata-only operation in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 58s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0141_add_mypostcard_job_id.py) is SAFE. It adds a nullable INTEGER column to postal_verification_attempts — a metadata-only operation in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 2m 57s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: Found 1 critical issue. Migration 0141_add_mypostcard_job_id.py is SAFE (adds a nullable INTEGER column, metadata-only). However, migration 0118_add_postal_verification.py was modified (renaming countrycountry_code) — this is a HIGH RISK change because the migration has already been applied to production with the original country column name. A new migration is needed to rename the column. Full review posted as a separate comment, and an inline comment was added on the problematic line.

sa.Column("state", sa.String(), nullable=True),
sa.Column("postal_code", sa.String(), nullable=True),
sa.Column("country", sa.String(), nullable=False),
sa.Column("country_code", sa.String(), nullable=False),

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.

⚠️ HIGH RISK: Modified existing migration

This migration (0118) has already been applied to production — the postal_verification_attempts table exists with a column named country (confirmed in the current production schema). Changing countrycountry_code in this file has no effect on production since Alembic won't re-run an already-applied migration.

This means:

  • Production will still have the column named country
  • The new code referencing country_code will break against the production database
  • The model and migration are now out of sync with the actual production schema

Fix: Create a new migration (e.g., 0142) that renames the column:

op.alter_column("postal_verification_attempts", "country", new_column_name="country_code")

And revert this change to 0118 so it matches what was actually applied.

@couchersbot

couchersbot Bot commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 2m 49s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0141_add_mypostcard_job_id.py) is SAFE. It adds a nullable INTEGER column to postal_verification_attempts and renames country to country_code — both metadata-only operations in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. The previous issue with modifying migration 0118 has been resolved. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 3m 12s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0141_add_mypostcard_job_id.py) is SAFE. It adds a nullable INTEGER column to postal_verification_attempts and renames country to country_code — both metadata-only operations in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. The previous issue with modifying migration 0118 has been resolved. Full review posted as a separate comment.

@couchersbot

couchersbot Bot commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 1m 51s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0141_add_mypostcard_job_id.py) is SAFE. It adds a nullable INTEGER column to postal_verification_attempts and renames country to country_code — both metadata-only operations in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. The previous issue with modifying migration 0118 has been resolved. Full review posted as a separate comment.

aapeliv and others added 20 commits April 4, 2026 00:08
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add types-qrcode stubs, fix type annotations for font fallbacks,
return types, and QR image handling.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace programmatic image generation with a static postcard-front.png,
remove pillow/qrcode deps, inline API URLs, and let exceptions propagate
instead of catching them.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix mypy: Image.NEAREST → Image.Resampling.NEAREST
- Fix postal verification tests: mock send_postcard to return job_id string
- Rewrite postcard service tests: remove classes, use context managers,
  test through public API instead of internal functions

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…line dates

- Add couchers/sentry.py with report_error/report_message wrappers
- Change mypostcard_job_id from String to Integer throughout
- Move get_orders inline import to top-level, accept date objects
- Filter known_job_ids to last 2 weeks in orphan check
- Remove stale if True block in handler
- Report orphaned jobs to Sentry instead of logger.error

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The field stores an ISO 3166-1 alpha-2 code, not a country name.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The column rename should be in the latest migration on this PR branch,
not in the original table creation migration.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Test generates sample front/back postcard images to test_artifacts/
- CI collects and uploads them to the preview S3 bucket
- Clean up QR code rendering: use named bounds, extend outward, no white bg
- Fix postal verification URL query param

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ache resources

- Compute QR box coordinates inline (227+312=539, 419+312=731)
- Hardcode font size to 58 instead of iterating from 100
- Move QR URL generation into _generate_back_left_side
- Remove qr_code_url parameter from send_postcard and _generate_back_left_side
- Add @functools.cache to postcard resource functions
- Remove unused get_postcard_logo
- Inline front_page, back_left_side, auth_token variables

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ser instead of LiteUser

- Rename PostalAddress.country to country_code in proto definition
- Update all Python references to use the new field name
- Switch ListPostcards query from LiteUser to User join

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@aapeliv
aapeliv force-pushed the backend/feature/mail-verification branch from bdc5959 to 1abcc8d Compare April 4, 2026 04:10
@couchersbot

couchersbot Bot commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aapeliv's task in 1m 39s —— View job


Migration Safety Review

  • Get PR diff and identify migration files
  • Review migration files for production safety
  • Check current schema and models
  • Post review summary

Result: The single migration (0142_add_mypostcard_job_id.py) is SAFE. It adds a nullable INTEGER column to postal_verification_attempts and renames country to country_code — both metadata-only operations in PostgreSQL with no data loss risk, no backfill needed, and a correct downgrade path. The previous issue with modifying migration 0118 has been resolved. Full review posted as a separate comment.

@aapeliv
aapeliv merged commit 0661987 into develop Apr 4, 2026
7 checks passed
@aapeliv
aapeliv deleted the backend/feature/mail-verification branch April 4, 2026 13:39
@github-actions github-actions Bot added the release notes: pending Add to stuff that should be included in release notes label Apr 4, 2026
@github-actions

github-actions Bot commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

📝 Release Notes

This PR should be included in the release notes.

Suggested release note:

Implemented postcard sending for postal verification so members can verify their address with a mailed verification code
🤖 Bot Debug Information

Model: couchers.openai.gpt-5.4
Decision: include
Reasoning: This PR implements actual postcard sending for postal verification, which is a significant user-facing safety and verification feature. Users pursuing postal verification will directly experience receiving a real postcard with a verification code, making this notable for release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: pending Add to stuff that should be included in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants