Enforce one cover photo per restaurant in the schema
- Dominant language
- TypeScript
- Stars
- 0
- Forks
- 0
- Avg merge
- 2h 19m
- Merged PRs (30d)
- 30
Description
Raised by @greptile during review of #54, and flagged in that PR's description
as a deliberate follow-up. **Depends on #54 merging** — the helper named below
arrives with it.
## The problem
"At most one `preview=True` row per restaurant" is enforced only in
application code. `app/api/utils.py:clear_other_previews` reads the current
previews, demotes them, and the caller commits alongside the new cover. #54
added `SELECT ... FOR UPDATE` on the restaurant row, which serialises the two
write paths against each other on Postgres, but:
- the rule still has no backing in the schema, so anything that writes
`preview` without going through that helper can break it (a seeder, a shell,
a future route);
- **rows already in production may violate it**, written by the pre-#54 code
that set `preview` from the request body without clearing the old cover.
When two rows do have `preview=True`, `GET /api/restaurants` picks whichever
iterates last (`restaurant_routes.py`, the preview loop), so a restaurant's
card can change cover between requests for no visible reason.
## Suggested fix
A partial unique index — supported by both Postgres and SQLite, so the test
suite is covered too:
```sql
CREATE UNIQUE INDEX uq_restaurant_images_one_preview
ON restaurant_images (restaurant_id)
WHERE preview;
```
The migration has to clean up before it can add the index, or it will fail on
any restaurant that already has duplicates:
1. For each `restaurant_id` with more than one `preview=True` row, keep one
(lowest `id` is the stable choice, and matches the promotion order #54 uses
when a cover is deleted) and set the rest to `false`.
2. Then create the index.
3. `downgrade()` drops the index; the demoted rows stay demoted, which is
correct — they were never meant to be covers.
Alembic head is currently `b4d7f1a6e93c` (`zipcode_as_text`). Production uses
a schema prefix (`add_prefix_for_prod`), so the migration needs the same
treatment the existing ones use.
## Why it is worth doing
With the index in place a racing or buggy write fails loudly with an
`IntegrityError` instead of silently leaving two covers, and the listing's
choice of cover becomes deterministic by construction rather than by
convention.
Contributor guide
No contributing guide indexed for this repository
Research direction
After #54 merges, read app/api/utils.py:clear_other_previews and the preview loop in restaurant_routes.py, then inspect the Alembic migrations after head b4d7f1a6e93c and their add_prefix_for_prod handling. The migration is done when duplicate previews are cleaned deterministically, the partial unique index is added and dropped by downgrade(), and production schema prefixes are supported.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, sql, sqlite
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100