Skip to content

[#78] Ratings schema + RLS policy#83

Merged
realproject7 merged 4 commits intomainfrom
task/78-ratings-schema-rls
Mar 15, 2026
Merged

[#78] Ratings schema + RLS policy#83
realproject7 merged 4 commits intomainfrom
task/78-ratings-schema-rls

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Added migration 00005_ratings.sql: creates ratings table with score (1-5), comment, unique constraint on (storyline_id, rater_address) for upsert
  • RLS enabled with public read policy (FOR SELECT USING (true)), no public write (writes via service role only)
  • Updated Database type in lib/supabase.ts with ratings Row/Insert/Update types
  • Added Rating convenience type alias

Test plan

  • npm run lint passes
  • npm run typecheck passes
  • Migration creates table with correct columns and constraints
  • RLS allows anonymous SELECT but blocks INSERT/UPDATE/DELETE via anon key

Fixes #78

🤖 Generated with Claude Code

- Migration 00005_ratings.sql: creates ratings table with score 1-5,
  unique constraint on (storyline_id, rater_address) for upsert pattern
- RLS: public read (FOR SELECT USING (true)), no public write
- Database type in lib/supabase.ts updated with ratings Row/Insert/Update
- Added Rating convenience type alias

Fixes #78

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES

Summary

The PR is close, but the new migration does not match the existing schema and will fail when applied. There is also a missing storyline index compared with the issue's requested schema.

Findings

  • [high] The foreign key in ratings.storyline_id points at storylines(id), but the existing schema keys related tables off the on-chain identifier storylines.storyline_id. In this repo, plots and donations both reference storylines(storyline_id), and issue #78 specifies the same. As written, the migration does not match the intended join key and can fail because storylines.id is not declared unique beyond the PK while the rest of the app types and queries are built around storyline_id.
    • File: supabase/migrations/00005_ratings.sql:6
    • Suggestion: change the foreign key to references storylines (storyline_id) so the ratings table lines up with the existing schema and issue spec.
  • [medium] The migration omits the idx_ratings_storyline index that is part of the issue's schema sketch. This table will be read by storyline, so skipping that index is unnecessary regression against the requested shape.
    • File: supabase/migrations/00005_ratings.sql
    • Suggestion: add create index idx_ratings_storyline on ratings (storyline_id);.

Decision

Requesting changes because the foreign key target is incorrect and blocks the migration from matching the existing database model.

- FK now references storylines(storyline_id) matching existing pattern
- Added idx_ratings_storyline index

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES

Summary

The foreign key and index fixes are correct, but the PR still diverges from issue #78 on the ratings column name. The issue schema specifies rating, while this PR creates and types a score column instead.

Findings

  • [medium] The migration and Supabase types use score, but issue #78 explicitly defines the schema as rating smallint not null check (rating between 1 and 5). This is not just naming style: the issue is asking for a concrete table shape, and introducing a different column name creates avoidable mismatch for follow-on work and any code written against the documented schema.
    • File: supabase/migrations/00005_ratings.sql:8
    • Suggestion: rename the column to rating in the migration and update lib/supabase.ts accordingly.
  • [medium] The generated database types mirror the same schema drift by exposing score instead of rating.
    • File: lib/supabase.ts:174
    • Suggestion: keep the TS types aligned with the final SQL column name from the issue spec.

Decision

Requesting one more change so the migration matches the issue's defined schema exactly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES

Summary

The SQL migration now matches issue #78, but the Supabase types still contain one stale field name in the ratings.Update shape.

Findings

  • [medium] ratings.Update still uses score?: number even though the table column is now rating. That leaves the generated types inconsistent with the migration and will mislead any update code written against this interface.
    • File: lib/supabase.ts:192
    • Suggestion: rename score?: number to rating?: number in the Update type so all three shapes (Row/Insert/Update) align with the SQL schema.

Decision

Requesting one final type fix so the database types are fully consistent with the migration.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Verdict: APPROVE

Summary

The migration and Supabase types now align with issue #78 and the existing schema conventions. The ratings table, RLS policy, index, and generated types are all consistent.

Findings

  • No blocking findings.

Decision

Approving because the PR now matches the requested ratings schema and public-read/no-public-write policy.

@realproject7 realproject7 merged commit b864aec into main Mar 15, 2026
1 check passed
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.

[P5-R1] Ratings schema + RLS policy

2 participants