From 78abc4ae90cdb800bba43054d64ca8ded862eb10 Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sat, 1 Aug 2026 21:35:50 +0300 Subject: [PATCH 01/10] feat: add profile education schema foundation Refine the profile and education schema with explicit column grants, RLS, section revision triggers, review and primary-selection RPCs, and forward-only legacy compatibility. --- docs/DATA_MODEL.md | 18 +- docs/RLS_POLICY_MATRIX.md | 7 +- docs/SECURITY_AND_PRIVACY.md | 7 + docs/TESTING_STRATEGY.md | 6 + .../20260801213000_profile_core_education.sql | 449 ++++++++++++++++++ .../tests/database/000_structure.test.sql | 58 ++- .../tests/database/010_profiles_rls.test.sql | 21 +- .../020_education_entries_rls.test.sql | 31 +- .../030_profile_section_state.test.sql | 74 +++ 9 files changed, 630 insertions(+), 41 deletions(-) create mode 100644 supabase/migrations/20260801213000_profile_core_education.sql create mode 100644 supabase/tests/database/030_profile_section_state.test.sql diff --git a/docs/DATA_MODEL.md b/docs/DATA_MODEL.md index c489727..9d7c9ab 100644 --- a/docs/DATA_MODEL.md +++ b/docs/DATA_MODEL.md @@ -81,9 +81,21 @@ erDiagram ## Profile domain (Phase 1A proposal; user-owned unless marked shared) -**Implementation status**: only the deliberately minimal Phase 0 `profiles` and -`education_entries` migration exists. Everything in this section is a Phase 1A logical target, -not implemented SQL. Phase 1B resume and suggestion records are expressly excluded. +**Implementation status**: the first Phase 1A vertical slice is implemented: refined +`profiles`, refined `education_entries`, and section freshness/review rows for `basic_profile` +and `education`. The remaining profile-domain tables below remain logical targets, not +implemented SQL. Phase 1B resume and suggestion records are expressly excluded. + +### Implemented profile-core migration note + +The Phase 0 `profiles.degree_program`, `profiles.degree_year`, and +`profiles.profile_complete` columns are retained temporarily as deprecated compatibility fields. +They are not authoritative and the browser does not write them. A forward migration copies a +legacy degree year only when its profile has exactly one existing education row and that target +field is null; it never chooses a primary row, infers a status, creates an institution, or maps +the ambiguous legacy degree-program text into `degree` or `field`. Existing profiles begin with +`basic_profile` and `education` revisions at zero and no reviews. New profile creation initializes +those rows transactionally. **Ownership rule (binding, see [AGENTS.md](../AGENTS.md))**: every user-owned table has direct `user_id`, including association tables. The root `profiles.user_id` references `auth.users(id)`; diff --git a/docs/RLS_POLICY_MATRIX.md b/docs/RLS_POLICY_MATRIX.md index 0a5ab94..471dfb6 100644 --- a/docs/RLS_POLICY_MATRIX.md +++ b/docs/RLS_POLICY_MATRIX.md @@ -57,8 +57,9 @@ both this file and its corresponding test. ## Profile domain (user-owned) -**Status**: `profiles` and `education_entries` are implemented and pgTAP-tested as of Phase 0. -All other rows in this profile-domain section are proposed Phase 1A policy, not schema. Phase 1B +**Status**: `profiles`, `education_entries`, `profile_section_revisions`, and +`profile_section_reviews` are implemented and pgTAP-tested in the first Phase 1A slice. All other +rows in this profile-domain section remain proposed Phase 1A policy, not schema. Phase 1B resume/suggestion tables are intentionally not designed here. Every table's owner column is literally named `user_id` (see [AGENTS.md](../AGENTS.md)) — child @@ -71,7 +72,7 @@ once per statement rather than once per row — the currently-recommended, non-d |---|---|---|---|---|---|---|---|---|---|---| | `profiles` | User-owned root row; direct `user_id` references Auth user | Own only | Own only | Own only | **No direct browser DELETE** | Trusted service/administrative code may have broader maintenance, export, repair, and coordinated-account-deletion access. Root-profile deletion is permitted only in an explicit trusted workflow that coordinates the Auth user, database records, Storage objects, confirmation, and any export requirement — never as a normal browser or routine service-role path. | `(select auth.uid()) = user_id` for SELECT/INSERT/UPDATE; no browser DELETE grant/policy | No | Owner SELECT/INSERT/UPDATE; DELETE fails even for owner; cross-user and forged-owner denial; service deletion is exercised only through the trusted workflow | High | | `education_entries`, `work_experience`, `projects`, `profile_links`, `user_skills`, `user_languages`, `preferences`, `target_companies` | User-owned; direct `user_id` references `profiles(user_id)` | Own only | Own only | Own only | Own only | Full for maintenance/export/account deletion | `(select auth.uid()) = user_id` for `USING` and `WITH CHECK` | No | Owner CRUD; cross-user SELECT/UPDATE/DELETE denial; forged `user_id` insert; ownership-rewrite denial | High (`profile_links`/targets Medium) | -| `profile_section_reviews` | User-owned deliberate review; composite key `(user_id, section_key)` | Own only | Own only | Own only | Own only | Full | Direct owner predicate; controlled section key and current revision must be recorded | No | Owner CRUD; forged-owner and cross-user denial; cannot write a review for an unknown key or stale/future revision | High | +| `profile_section_reviews` | User-owned deliberate review; composite key `(user_id, section_key)` | Own only | **No direct browser write**; restricted RPC only | **No direct browser write**; restricted RPC only | **No direct browser write** | Full | Direct owner SELECT; `review_profile_section` derives `auth.uid()` and locks the current revision | No | User cannot forge an owner/key/revision; RPC writes only the locked current revision | High | | `profile_section_revisions` | User-owned, database-maintained freshness metadata | Own only | No direct browser write | No direct browser write | No direct browser write | Trusted migration/mutation mechanism only | Direct owner SELECT; no browser mutation grant | No | User can read only own revision; browser cannot forge/increment/rewrite revision; relevant insert/update/delete increments it atomically | Medium | | `preference_locations`, `user_target_engineering_areas`, `user_target_role_types` | User-owned relation; direct `user_id` | Own only | Own only | Own only where applicable | Own only | Full | Direct owner predicate plus composite parent `(user_id, parent_id)` validation | No | Owner CRUD; all cross-user denials; forged owner; cannot attach a valid user A row to B's preferences/profile | Medium | | `project_skills` | User-owned relation; direct `user_id` | Own only | Own only | N/A or own only | Own only | Full | Direct owner predicate plus `(user_id, project_id)` composite FK | No | User A cannot attach B's project to A's skill or vice versa, even when UUID is known; owner CRUD | High | diff --git a/docs/SECURITY_AND_PRIVACY.md b/docs/SECURITY_AND_PRIVACY.md index d084f1a..fe21500 100644 --- a/docs/SECURITY_AND_PRIVACY.md +++ b/docs/SECURITY_AND_PRIVACY.md @@ -46,6 +46,13 @@ SELECT/INSERT/UPDATE/DELETE permissions, the expected RLS predicate, anonymous-a the isolation test each row requires. This section states the *principle*; that document is the *checklist* a migration must satisfy. +**Implemented Phase 1A profile-core hardening**: section revisions and reviews are browser +read-only. The only review mutation is a narrowly granted `SECURITY DEFINER` RPC that derives the +caller from `auth.uid()`, locks that user's current revision, and records that exact value. Primary +education selection likewise uses a narrowly granted ownership-checking RPC; browser clients do +not have column privilege to set `is_primary` directly. Every privileged function fixes its search +path, revokes default `PUBLIC` execution, and grants only the browser RPCs to `authenticated`. + **Row-level data isolation for future public users**: because the schema is already `user_id`- scoped and RLS-enforced, enabling additional user accounts later is a matter of allowing sign-up — the isolation mechanism doesn't change. This is the concrete mechanism behind the "private-first, diff --git a/docs/TESTING_STRATEGY.md b/docs/TESTING_STRATEGY.md index 6892956..24090ec 100644 --- a/docs/TESTING_STRATEGY.md +++ b/docs/TESTING_STRATEGY.md @@ -72,6 +72,12 @@ each rule gets explicit positive and negative test cases. ## Phase 1A profile implementation tests (future) +**Implemented in the first profile-core/education slice**: pgTAP now verifies refined profile and +education schema/grants, browser denial of root-profile deletion and direct revision/review/primary +writes, revision freshness after profile and education mutations, locked review RPC behavior, +primary-selection idempotence, and cascade safety. The real Auth/JWT/PostgREST suite is extended +in the following commit; later Phase 1A domains remain future work. + - **Migration structure tests**: exact tables, direct `user_id` ownership, required foreign keys, ownership-safe composite parent references, primary-education partial unique constraint, typed-skill-evidence exactly-one-source check, bounded note, section-review/revision constraints, diff --git a/supabase/migrations/20260801213000_profile_core_education.sql b/supabase/migrations/20260801213000_profile_core_education.sql new file mode 100644 index 0000000..461a96d --- /dev/null +++ b/supabase/migrations/20260801213000_profile_core_education.sql @@ -0,0 +1,449 @@ +-- Phase 1A, first product slice: profile core and primary education. +-- +-- This is intentionally forward-only. The Phase 0 profile fields remain in +-- place as deprecated compatibility data until a later, audited cleanup +-- migration can prove they have been manually resolved. + +-- --------------------------------------------------------------------------- +-- Schema evolution and safe legacy backfill +-- --------------------------------------------------------------------------- + +alter table public.profiles + add column preferred_name text, + add column created_via text not null default 'manual', + add column updated_at timestamptz not null default now(), + add column last_confirmed_at timestamptz not null default now(), + add constraint profiles_created_via_check check (created_via in ('manual', 'migration')); + +alter table public.education_entries + add column degree_year smallint, + add column expected_graduation_month smallint, + add column expected_graduation_year smallint, + add column education_status text not null default 'unknown', + add column is_primary boolean not null default false, + add column created_via text not null default 'manual', + add column updated_at timestamptz not null default now(), + add column last_confirmed_at timestamptz not null default now(), + add constraint education_entries_degree_year_check check (degree_year is null or degree_year between 1 and 10), + add constraint education_entries_graduation_month_check check (expected_graduation_month is null or expected_graduation_month between 1 and 12), + add constraint education_entries_graduation_year_check check (expected_graduation_year is null or expected_graduation_year between 2000 and 2100), + add constraint education_entries_graduation_pair_check check ( + (expected_graduation_month is null) = (expected_graduation_year is null) + ), + add constraint education_entries_status_check check (education_status in ('current', 'completed', 'paused', 'withdrawn', 'unknown')), + add constraint education_entries_current_end_date_check check (education_status <> 'current' or end_date is null), + add constraint education_entries_completed_end_date_check check (education_status <> 'completed' or end_date is not null), + add constraint education_entries_graduation_current_check check ( + expected_graduation_year is null or education_status = 'current' + ), + add constraint education_entries_graduation_after_start_check check ( + start_date is null + or expected_graduation_year is null + or make_date(expected_graduation_year, expected_graduation_month, 1) >= date_trunc('month', start_date)::date + ), + add constraint education_entries_primary_current_check check (not is_primary or education_status = 'current'), + add constraint education_entries_created_via_check check (created_via in ('manual', 'migration')); + +-- The Phase 0 migration allowed blank/overlong text. Do not use NOT VALID +-- text CHECKs: PostgreSQL would then reject unrelated edits to a preserved +-- legacy row. The validation triggers below instead validate every new value +-- and every changed text value, while leaving unchanged legacy data editable. +update public.profiles +set + created_via = 'migration', + updated_at = created_at, + last_confirmed_at = null; + +update public.education_entries +set + created_via = 'migration', + updated_at = created_at, + last_confirmed_at = null; + +-- Only the degree-year mapping has identical semantics in the target model. +-- A lone legacy education row is not made primary and is not assigned a status. +with profiles_with_one_education as ( + select user_id, (array_agg(id order by id))[1] as education_id + from public.education_entries + group by user_id + having count(*) = 1 +) +update public.education_entries as education +set degree_year = profile.degree_year +from public.profiles as profile +join profiles_with_one_education as one_education on one_education.user_id = profile.user_id +where education.id = one_education.education_id + and profile.degree_year is not null + and education.degree_year is null; + +comment on column public.profiles.degree_program is + 'Deprecated Phase 0 compatibility field. Not authoritative; do not write from the browser or use for completeness.'; +comment on column public.profiles.degree_year is + 'Deprecated Phase 0 compatibility field. Safely copied only to a lone existing education row; not authoritative.'; +comment on column public.profiles.profile_complete is + 'Deprecated Phase 0 compatibility field. Completeness is derived; this value is not authoritative.'; +comment on column public.profiles.created_via is + 'How this current-format profile row entered the system: manual or migration.'; +comment on column public.profiles.last_confirmed_at is + 'Last explicit manual profile-content creation or edit; separate from section review.'; +comment on column public.education_entries.last_confirmed_at is + 'Last explicit manual education-content creation or edit; separate from section review.'; + +create unique index education_entries_one_primary_per_user_idx + on public.education_entries (user_id) + where is_primary; + +-- --------------------------------------------------------------------------- +-- Section state. Existing data starts at a neutral baseline with no review. +-- --------------------------------------------------------------------------- + +create table public.profile_section_revisions ( + user_id uuid not null references public.profiles (user_id) on delete cascade, + section_key text not null check (section_key in ('basic_profile', 'education')), + content_revision bigint not null default 0 check (content_revision >= 0), + updated_at timestamptz not null default now(), + primary key (user_id, section_key) +); + +create table public.profile_section_reviews ( + user_id uuid not null references public.profiles (user_id) on delete cascade, + section_key text not null check (section_key in ('basic_profile', 'education')), + reviewed_content_revision bigint not null check (reviewed_content_revision >= 0), + reviewed_at timestamptz not null default now(), + created_at timestamptz not null default now(), + updated_at timestamptz not null default now(), + primary key (user_id, section_key), + foreign key (user_id, section_key) + references public.profile_section_revisions (user_id, section_key) + on delete cascade +); + +insert into public.profile_section_revisions (user_id, section_key, content_revision) +select profiles.user_id, section_keys.section_key, 0 +from public.profiles +cross join (values ('basic_profile'::text), ('education'::text)) as section_keys(section_key); + +comment on table public.profile_section_revisions is + 'Database-maintained, deletion-safe section content mutation counters.'; +comment on table public.profile_section_reviews is + 'Deliberate review recorded against an exact profile-section content revision.'; + +-- --------------------------------------------------------------------------- +-- Hardened database functions and triggers +-- --------------------------------------------------------------------------- + +create function public.raise_profile_content_validation( + value text, + field_name text, + maximum_length integer +) +returns void +language plpgsql +security definer +set search_path = '' +as $$ +begin + if value is not null and (btrim(value) = '' or char_length(value) > maximum_length) then + raise exception using + errcode = '23514', + message = 'invalid profile content', + detail = field_name, + constraint = 'profile_content_text_check'; + end if; +end; +$$; + +create function public.profiles_before_write() +returns trigger +language plpgsql +security definer +set search_path = '' +as $$ +begin + if tg_op = 'INSERT' then + perform public.raise_profile_content_validation(new.preferred_name, 'preferred_name', 100); + perform public.raise_profile_content_validation(new.headline, 'headline', 160); + return new; + end if; + + if new.preferred_name is distinct from old.preferred_name then + perform public.raise_profile_content_validation(new.preferred_name, 'preferred_name', 100); + end if; + if new.headline is distinct from old.headline then + perform public.raise_profile_content_validation(new.headline, 'headline', 160); + end if; + + if new.preferred_name is distinct from old.preferred_name + or new.headline is distinct from old.headline then + new.updated_at := now(); + new.last_confirmed_at := now(); + else + new.created_at := old.created_at; + new.created_via := old.created_via; + new.updated_at := old.updated_at; + new.last_confirmed_at := old.last_confirmed_at; + end if; + return new; +end; +$$; + +create function public.education_entries_before_write() +returns trigger +language plpgsql +security definer +set search_path = '' +as $$ +declare + content_changed boolean; +begin + if tg_op = 'INSERT' then + perform public.raise_profile_content_validation(new.institution, 'institution', 200); + perform public.raise_profile_content_validation(new.degree, 'degree', 160); + perform public.raise_profile_content_validation(new.field, 'field', 160); + return new; + end if; + + if new.institution is distinct from old.institution then + perform public.raise_profile_content_validation(new.institution, 'institution', 200); + end if; + if new.degree is distinct from old.degree then + perform public.raise_profile_content_validation(new.degree, 'degree', 160); + end if; + if new.field is distinct from old.field then + perform public.raise_profile_content_validation(new.field, 'field', 160); + end if; + + content_changed := new.institution is distinct from old.institution + or new.degree is distinct from old.degree + or new.field is distinct from old.field + or new.degree_year is distinct from old.degree_year + or new.expected_graduation_month is distinct from old.expected_graduation_month + or new.expected_graduation_year is distinct from old.expected_graduation_year + or new.education_status is distinct from old.education_status + or new.is_primary is distinct from old.is_primary + or new.start_date is distinct from old.start_date + or new.end_date is distinct from old.end_date; + + if content_changed then + new.updated_at := now(); + new.last_confirmed_at := now(); + else + new.user_id := old.user_id; + new.created_at := old.created_at; + new.created_via := old.created_via; + new.updated_at := old.updated_at; + new.last_confirmed_at := old.last_confirmed_at; + end if; + return new; +end; +$$; + +create function public.increment_profile_section_revision(target_user_id uuid, target_section_key text) +returns void +language plpgsql +security definer +set search_path = '' +as $$ +begin + -- The profile-existence guard makes a cascading account deletion harmless: + -- after the parent disappears there is no state left to recreate. + insert into public.profile_section_revisions (user_id, section_key, content_revision, updated_at) + select target_user_id, target_section_key, 1, now() + where exists (select 1 from public.profiles where user_id = target_user_id) + on conflict (user_id, section_key) do update + set content_revision = public.profile_section_revisions.content_revision + 1, + updated_at = excluded.updated_at; +end; +$$; + +create function public.profiles_after_content_change() +returns trigger +language plpgsql +security definer +set search_path = '' +as $$ +begin + if tg_op = 'INSERT' then + insert into public.profile_section_revisions (user_id, section_key, content_revision) + values (new.user_id, 'basic_profile', 0), (new.user_id, 'education', 0) + on conflict (user_id, section_key) do nothing; + perform public.increment_profile_section_revision(new.user_id, 'basic_profile'); + elsif new.preferred_name is distinct from old.preferred_name + or new.headline is distinct from old.headline then + perform public.increment_profile_section_revision(new.user_id, 'basic_profile'); + end if; + return null; +end; +$$; + +create function public.education_entries_after_content_change() +returns trigger +language plpgsql +security definer +set search_path = '' +as $$ +declare + owner_id uuid; +begin + owner_id := case when tg_op = 'DELETE' then old.user_id else new.user_id end; + if tg_op <> 'UPDATE' + or new.institution is distinct from old.institution + or new.degree is distinct from old.degree + or new.field is distinct from old.field + or new.degree_year is distinct from old.degree_year + or new.expected_graduation_month is distinct from old.expected_graduation_month + or new.expected_graduation_year is distinct from old.expected_graduation_year + or new.education_status is distinct from old.education_status + or new.is_primary is distinct from old.is_primary + or new.start_date is distinct from old.start_date + or new.end_date is distinct from old.end_date then + perform public.increment_profile_section_revision(owner_id, 'education'); + end if; + return null; +end; +$$; + +create trigger profiles_before_write +before insert or update on public.profiles +for each row execute function public.profiles_before_write(); + +create trigger profiles_after_content_change +after insert or update on public.profiles +for each row execute function public.profiles_after_content_change(); + +create trigger education_entries_before_write +before insert or update on public.education_entries +for each row execute function public.education_entries_before_write(); + +create trigger education_entries_after_content_change +after insert or update or delete on public.education_entries +for each row execute function public.education_entries_after_content_change(); + +create function public.review_profile_section(requested_section_key text) +returns bigint +language plpgsql +security definer +set search_path = '' +as $$ +declare + caller_id uuid := auth.uid(); + current_revision bigint; +begin + if caller_id is null then + raise exception using errcode = '28000', message = 'authentication required'; + end if; + if requested_section_key not in ('basic_profile', 'education') then + raise exception using errcode = '22023', message = 'unknown profile section'; + end if; + + select content_revision into current_revision + from public.profile_section_revisions + where user_id = caller_id and section_key = requested_section_key + for update; + if not found then + raise exception using errcode = 'P0002', message = 'profile section does not exist'; + end if; + + insert into public.profile_section_reviews ( + user_id, section_key, reviewed_content_revision, reviewed_at, created_at, updated_at + ) values (caller_id, requested_section_key, current_revision, now(), now(), now()) + on conflict (user_id, section_key) do update + set reviewed_content_revision = excluded.reviewed_content_revision, + reviewed_at = excluded.reviewed_at, + updated_at = excluded.updated_at; + return current_revision; +end; +$$; + +create function public.set_primary_education(education_id uuid) +returns void +language plpgsql +security definer +set search_path = '' +as $$ +declare + caller_id uuid := auth.uid(); + selected_status text; +begin + if caller_id is null then + raise exception using errcode = '28000', message = 'authentication required'; + end if; + + -- Deterministic locking serializes simultaneous primary-selection requests. + perform 1 + from public.education_entries + where user_id = caller_id + order by id + for update; + + select education_status into selected_status + from public.education_entries + where id = education_id and user_id = caller_id; + if not found then + raise exception using errcode = 'P0002', message = 'education entry not found'; + end if; + if selected_status <> 'current' then + raise exception using errcode = '23514', message = 'primary education must be current'; + end if; + + update public.education_entries + set is_primary = false + where user_id = caller_id and is_primary and id <> education_id; + + update public.education_entries + set is_primary = true + where user_id = caller_id and id = education_id and not is_primary; +end; +$$; + +-- No function starts browser-callable merely because it is created in public. +revoke all on function public.raise_profile_content_validation(text, text, integer) from public, anon, authenticated; +revoke all on function public.profiles_before_write() from public, anon, authenticated; +revoke all on function public.education_entries_before_write() from public, anon, authenticated; +revoke all on function public.increment_profile_section_revision(uuid, text) from public, anon, authenticated; +revoke all on function public.profiles_after_content_change() from public, anon, authenticated; +revoke all on function public.education_entries_after_content_change() from public, anon, authenticated; +revoke all on function public.review_profile_section(text) from public, anon; +revoke all on function public.set_primary_education(uuid) from public, anon; +grant execute on function public.review_profile_section(text) to authenticated; +grant execute on function public.set_primary_education(uuid) to authenticated; + +-- --------------------------------------------------------------------------- +-- Explicit grants and RLS +-- --------------------------------------------------------------------------- + +drop policy if exists profiles_delete_own on public.profiles; +revoke all on public.profiles from anon, authenticated, service_role; +grant select on public.profiles to authenticated; +grant insert (user_id, preferred_name, headline) on public.profiles to authenticated; +grant update (preferred_name, headline) on public.profiles to authenticated; +grant select, insert, update, delete on public.profiles to service_role; + +revoke all on public.education_entries from anon, authenticated, service_role; +grant select on public.education_entries to authenticated; +grant insert ( + user_id, institution, degree, field, degree_year, + expected_graduation_month, expected_graduation_year, education_status, + start_date, end_date +) on public.education_entries to authenticated; +grant update ( + institution, degree, field, degree_year, + expected_graduation_month, expected_graduation_year, education_status, + start_date, end_date +) on public.education_entries to authenticated; +grant delete on public.education_entries to authenticated; +grant select, insert, update, delete on public.education_entries to service_role; + +alter table public.profile_section_revisions enable row level security; +alter table public.profile_section_reviews enable row level security; +revoke all on public.profile_section_revisions from anon, authenticated, service_role; +revoke all on public.profile_section_reviews from anon, authenticated, service_role; +grant select on public.profile_section_revisions to authenticated; +grant select on public.profile_section_reviews to authenticated; +grant select, insert, update, delete on public.profile_section_revisions to service_role; +grant select, insert, update, delete on public.profile_section_reviews to service_role; + +create policy profile_section_revisions_select_own on public.profile_section_revisions + for select to authenticated using ((select auth.uid()) = user_id); +create policy profile_section_reviews_select_own on public.profile_section_reviews + for select to authenticated using ((select auth.uid()) = user_id); diff --git a/supabase/tests/database/000_structure.test.sql b/supabase/tests/database/000_structure.test.sql index 2b79c71..b357470 100644 --- a/supabase/tests/database/000_structure.test.sql +++ b/supabase/tests/database/000_structure.test.sql @@ -2,17 +2,21 @@ -- policy existence, and grant existence (including the absence of any -- grant to anon). No fixture data needed; this only inspects catalogs. begin; -select plan(29); +select plan(58); -- profiles ------------------------------------------------------------------ select has_table('public', 'profiles', 'profiles table exists'); select has_column('public', 'profiles', 'user_id', 'profiles.user_id exists'); select has_column('public', 'profiles', 'headline', 'profiles.headline exists'); +select has_column('public', 'profiles', 'preferred_name', 'profiles.preferred_name exists'); select has_column('public', 'profiles', 'degree_program', 'profiles.degree_program exists'); select has_column('public', 'profiles', 'degree_year', 'profiles.degree_year exists'); select has_column('public', 'profiles', 'profile_complete', 'profiles.profile_complete exists'); select has_column('public', 'profiles', 'created_at', 'profiles.created_at exists'); +select has_column('public', 'profiles', 'created_via', 'profiles.created_via exists'); +select has_column('public', 'profiles', 'updated_at', 'profiles.updated_at exists'); +select has_column('public', 'profiles', 'last_confirmed_at', 'profiles.last_confirmed_at exists'); select col_is_pk('public', 'profiles', 'user_id', 'profiles.user_id is the primary key'); select fk_ok('public', 'profiles', 'user_id', 'auth', 'users', 'id', 'profiles.user_id references auth.users.id'); select has_check('public', 'profiles', 'profiles has a check constraint (degree_year range)'); @@ -24,15 +28,20 @@ select is( ); select policies_are( 'public', 'profiles', - array['profiles_select_own', 'profiles_insert_own', 'profiles_update_own', 'profiles_delete_own'], - 'profiles has exactly the four expected policies' + array['profiles_select_own', 'profiles_insert_own', 'profiles_update_own'], + 'profiles has exactly the three expected policies' ); select policy_cmd_is('public', 'profiles', 'profiles_select_own', 'SELECT', 'profiles_select_own applies to SELECT'); select policy_cmd_is('public', 'profiles', 'profiles_insert_own', 'INSERT', 'profiles_insert_own applies to INSERT'); select policy_cmd_is('public', 'profiles', 'profiles_update_own', 'UPDATE', 'profiles_update_own applies to UPDATE'); -select policy_cmd_is('public', 'profiles', 'profiles_delete_own', 'DELETE', 'profiles_delete_own applies to DELETE'); - -select table_privs_are('public', 'profiles', 'authenticated', array['SELECT', 'INSERT', 'UPDATE', 'DELETE'], 'authenticated has exactly SELECT/INSERT/UPDATE/DELETE on profiles'); +select is( + has_table_privilege('authenticated', 'public.profiles', 'SELECT') + and has_column_privilege('authenticated', 'public.profiles', 'user_id', 'INSERT') + and has_column_privilege('authenticated', 'public.profiles', 'headline', 'UPDATE') + and not has_table_privilege('authenticated', 'public.profiles', 'DELETE'), + true, + 'authenticated has the intended profiles table and column privileges' +); select table_privs_are('public', 'profiles', 'service_role', array['SELECT', 'INSERT', 'UPDATE', 'DELETE'], 'service_role has exactly SELECT/INSERT/UPDATE/DELETE on profiles'); select table_privs_are('public', 'profiles', 'anon', array[]::text[], 'anon has no grants at all on profiles'); @@ -40,6 +49,14 @@ select table_privs_are('public', 'profiles', 'anon', array[]::text[], 'anon has select has_table('public', 'education_entries', 'education_entries table exists'); select has_column('public', 'education_entries', 'user_id', 'education_entries.user_id exists (not profile_id)'); +select has_column('public', 'education_entries', 'degree_year', 'education_entries.degree_year exists'); +select has_column('public', 'education_entries', 'expected_graduation_month', 'education_entries.expected_graduation_month exists'); +select has_column('public', 'education_entries', 'expected_graduation_year', 'education_entries.expected_graduation_year exists'); +select has_column('public', 'education_entries', 'education_status', 'education_entries.education_status exists'); +select has_column('public', 'education_entries', 'is_primary', 'education_entries.is_primary exists'); +select has_column('public', 'education_entries', 'created_via', 'education_entries.created_via exists'); +select has_column('public', 'education_entries', 'updated_at', 'education_entries.updated_at exists'); +select has_column('public', 'education_entries', 'last_confirmed_at', 'education_entries.last_confirmed_at exists'); select col_is_pk('public', 'education_entries', 'id', 'education_entries.id is the primary key'); select fk_ok('public', 'education_entries', 'user_id', 'public', 'profiles', 'user_id', 'education_entries.user_id references profiles.user_id'); select has_index('public', 'education_entries', 'education_entries_user_id_idx', 'index on education_entries.user_id exists'); @@ -56,8 +73,35 @@ select policies_are( 'education_entries has exactly the four expected policies' ); -select table_privs_are('public', 'education_entries', 'authenticated', array['SELECT', 'INSERT', 'UPDATE', 'DELETE'], 'authenticated has exactly SELECT/INSERT/UPDATE/DELETE on education_entries'); +select is( + has_table_privilege('authenticated', 'public.education_entries', 'SELECT, DELETE') + and has_column_privilege('authenticated', 'public.education_entries', 'institution', 'INSERT, UPDATE') + and not has_column_privilege('authenticated', 'public.education_entries', 'is_primary', 'UPDATE'), + true, + 'authenticated has intended editable education columns but cannot change is_primary directly' +); select table_privs_are('public', 'education_entries', 'anon', array[]::text[], 'anon has no grants at all on education_entries'); +-- section freshness state ---------------------------------------------------- + +select has_table('public', 'profile_section_revisions', 'profile_section_revisions table exists'); +select has_table('public', 'profile_section_reviews', 'profile_section_reviews table exists'); +select has_column('public', 'profile_section_revisions', 'user_id', 'revisions own directly via user_id'); +select has_column('public', 'profile_section_revisions', 'content_revision', 'revisions counter exists'); +select has_column('public', 'profile_section_reviews', 'reviewed_content_revision', 'reviews record observed revision'); +select col_is_pk('public', 'profile_section_revisions', array['user_id', 'section_key'], 'revisions have composite identity'); +select col_is_pk('public', 'profile_section_reviews', array['user_id', 'section_key'], 'reviews have composite identity'); +select is((select relrowsecurity from pg_class where oid = 'public.profile_section_revisions'::regclass), true, 'RLS is enabled on revisions'); +select is((select relrowsecurity from pg_class where oid = 'public.profile_section_reviews'::regclass), true, 'RLS is enabled on reviews'); +select policies_are('public', 'profile_section_revisions', array['profile_section_revisions_select_own'], 'revisions have only the select-own policy'); +select policies_are('public', 'profile_section_reviews', array['profile_section_reviews_select_own'], 'reviews have only the select-own policy'); +select table_privs_are('public', 'profile_section_revisions', 'authenticated', array['SELECT'], 'authenticated can only select revisions'); +select table_privs_are('public', 'profile_section_reviews', 'authenticated', array['SELECT'], 'authenticated can only select reviews'); +select table_privs_are('public', 'profile_section_revisions', 'anon', array[]::text[], 'anon has no revisions grants'); +select table_privs_are('public', 'profile_section_reviews', 'anon', array[]::text[], 'anon has no reviews grants'); +select has_index('public', 'education_entries', 'education_entries_one_primary_per_user_idx', 'partial primary education index exists'); +select has_function('public', 'review_profile_section', array['text'], 'review RPC exists'); +select has_function('public', 'set_primary_education', array['uuid'], 'primary selection RPC exists'); + select * from finish(); rollback; diff --git a/supabase/tests/database/010_profiles_rls.test.sql b/supabase/tests/database/010_profiles_rls.test.sql index e769f5f..970148c 100644 --- a/supabase/tests/database/010_profiles_rls.test.sql +++ b/supabase/tests/database/010_profiles_rls.test.sql @@ -69,14 +69,11 @@ select is( 'user A cannot update user B''s profile (0 rows affected)' ); -with del as ( - delete from public.profiles where user_id = '22222222-2222-2222-2222-222222222222' - returning 1 -) -select is( - (select count(*)::int from del), - 0, - 'user A cannot delete user B''s profile (0 rows affected)' +select throws_ok( + $$ delete from public.profiles where user_id = '22222222-2222-2222-2222-222222222222' $$, + '42501'::char(5), + null, + 'user A has no profile DELETE privilege' ); select throws_ok( @@ -114,14 +111,16 @@ select is( 'service_role can read both users'' profiles (deliberate RLS bypass, not a leak)' ); --- ---- Back to user A: owner delete (run last; mutates state other tests rely on) -- +-- ---- Back to user A: root profile delete is deliberately unavailable ------- reset role; select set_config('request.jwt.claims', json_build_object('sub', '11111111-1111-1111-1111-111111111111', 'role', 'authenticated')::text, true); set role authenticated; -select lives_ok( +select throws_ok( $$ delete from public.profiles where user_id = '11111111-1111-1111-1111-111111111111' $$, - 'user A can delete their own profile' + '42501'::char(5), + null, + 'user A cannot delete their own root profile' ); reset role; diff --git a/supabase/tests/database/020_education_entries_rls.test.sql b/supabase/tests/database/020_education_entries_rls.test.sql index 936119b..f585a64 100644 --- a/supabase/tests/database/020_education_entries_rls.test.sql +++ b/supabase/tests/database/020_education_entries_rls.test.sql @@ -23,26 +23,26 @@ select set_config('request.jwt.claims', json_build_object('sub', '11111111-1111- set role authenticated; select lives_ok( - $$ insert into public.education_entries (id, user_id, institution) values ('aaaaaaaa-0000-0000-0000-000000000001', '11111111-1111-1111-1111-111111111111', 'University of A') $$, + $$ insert into public.education_entries (user_id, institution) values ('11111111-1111-1111-1111-111111111111', 'University of A') $$, 'user A can insert their own education entry' ); select lives_ok( - $$ insert into public.education_entries (id, user_id, institution) values ('aaaaaaaa-0000-0000-0000-000000000002', '11111111-1111-1111-1111-111111111111', 'Second entry, deleted later') $$, + $$ insert into public.education_entries (user_id, institution) values ('11111111-1111-1111-1111-111111111111', 'Second entry, deleted later') $$, 'user A can insert a second education entry' ); select is( - (select institution from public.education_entries where id = 'aaaaaaaa-0000-0000-0000-000000000001'), + (select institution from public.education_entries where institution = 'University of A'), 'University of A', 'user A can select their own education entry' ); select lives_ok( - $$ update public.education_entries set institution = 'University of A (renamed)' where id = 'aaaaaaaa-0000-0000-0000-000000000001' $$, + $$ update public.education_entries set institution = 'University of A (renamed)' where institution = 'University of A' $$, 'user A can update their own education entry' ); select is( - (select institution from public.education_entries where id = 'aaaaaaaa-0000-0000-0000-000000000001'), + (select institution from public.education_entries where institution = 'University of A (renamed)'), 'University of A (renamed)', 'user A''s update to their own education entry was applied' ); @@ -53,7 +53,7 @@ select set_config('request.jwt.claims', json_build_object('sub', '22222222-2222- set role authenticated; select lives_ok( - $$ insert into public.education_entries (id, user_id, institution) values ('bbbbbbbb-0000-0000-0000-000000000001', '22222222-2222-2222-2222-222222222222', 'University of B') $$, + $$ insert into public.education_entries (user_id, institution) values ('22222222-2222-2222-2222-222222222222', 'University of B') $$, 'user B can insert their own education entry' ); @@ -91,7 +91,7 @@ select is( -- names B as the owner, even though the FK target (profiles.user_id = B) -- legitimately exists. select throws_ok( - $$ insert into public.education_entries (id, user_id, institution) values ('cccccccc-0000-0000-0000-000000000001', '22222222-2222-2222-2222-222222222222', 'forged') $$, + $$ insert into public.education_entries (user_id, institution) values ('22222222-2222-2222-2222-222222222222', 'forged') $$, '42501'::char(5), null, 'user A cannot insert an education entry with user_id = user B (foreign-key-bypass attempt)' @@ -99,14 +99,11 @@ select throws_ok( -- The ownership-rewrite test: A must not be able to "claim" B's existing -- entry by rewriting its user_id to A's own id. -with upd as ( - update public.education_entries set user_id = '11111111-1111-1111-1111-111111111111' where id = 'bbbbbbbb-0000-0000-0000-000000000001' - returning 1 -) -select is( - (select count(*)::int from upd), - 0, - 'user A cannot take ownership of user B''s education entry by rewriting its user_id' +select throws_ok( + $$ update public.education_entries set user_id = '11111111-1111-1111-1111-111111111111' where institution = 'University of B' $$, + '42501'::char(5), + null, + 'user A cannot take ownership of an education entry by rewriting user_id' ); -- ---- Anonymous: insufficient privilege, not merely empty rows ------------- @@ -121,7 +118,7 @@ select throws_ok( 'anonymous SELECT on education_entries is rejected with insufficient privilege' ); select throws_ok( - $$ insert into public.education_entries (id, user_id, institution) values ('dddddddd-0000-0000-0000-000000000001', '11111111-1111-1111-1111-111111111111', 'anon') $$, + $$ insert into public.education_entries (user_id, institution) values ('11111111-1111-1111-1111-111111111111', 'anon') $$, '42501'::char(5), null, 'anonymous INSERT on education_entries is rejected with insufficient privilege' @@ -143,7 +140,7 @@ select set_config('request.jwt.claims', json_build_object('sub', '11111111-1111- set role authenticated; select lives_ok( - $$ delete from public.education_entries where id = 'aaaaaaaa-0000-0000-0000-000000000002' $$, + $$ delete from public.education_entries where institution = 'Second entry, deleted later' $$, 'user A can delete their own education entry' ); diff --git a/supabase/tests/database/030_profile_section_state.test.sql b/supabase/tests/database/030_profile_section_state.test.sql new file mode 100644 index 0000000..e2ad019 --- /dev/null +++ b/supabase/tests/database/030_profile_section_state.test.sql @@ -0,0 +1,74 @@ +-- Revision counters, privileged RPCs, direct-write denial, and cascade safety. +begin; +select plan(21); + +insert into auth.users (id, aud, role, email) +values + ('33333333-3333-3333-3333-333333333333', 'authenticated', 'authenticated', 'section-a@example.test'), + ('44444444-4444-4444-4444-444444444444', 'authenticated', 'authenticated', 'section-b@example.test'); + +reset role; +select set_config('request.jwt.claims', json_build_object('sub', '33333333-3333-3333-3333-333333333333', 'role', 'authenticated')::text, true); +set role authenticated; + +select lives_ok( + $$ insert into public.profiles (user_id, headline) values ('33333333-3333-3333-3333-333333333333', 'Profile A') $$, + 'profile insert succeeds and initializes section state' +); +select is((select content_revision from public.profile_section_revisions where section_key = 'basic_profile'), 1::bigint, 'profile creation advances basic-profile revision'); +select is((select content_revision from public.profile_section_revisions where section_key = 'education'), 0::bigint, 'education begins at revision zero'); + +select lives_ok($$ update public.profiles set headline = 'Profile A updated' $$, 'meaningful profile update succeeds'); +select is((select content_revision from public.profile_section_revisions where section_key = 'basic_profile'), 2::bigint, 'meaningful profile update advances its revision'); + +select lives_ok( + $$ insert into public.education_entries (user_id, institution, education_status) values ('33333333-3333-3333-3333-333333333333', 'Synthetic University', 'current') $$, + 'owner can create current education' +); +select is((select content_revision from public.profile_section_revisions where section_key = 'education'), 1::bigint, 'education insert advances its revision'); + +select lives_ok($$ select public.review_profile_section('education') $$, 'owner can record a current education review through RPC'); +select is( + (select reviewed_content_revision from public.profile_section_reviews where section_key = 'education'), + 1::bigint, + 'review stores the locked current revision' +); +select lives_ok($$ update public.education_entries set degree = 'BSc' where institution = 'Synthetic University' $$, 'education update succeeds'); +select is((select content_revision from public.profile_section_revisions where section_key = 'education'), 2::bigint, 'education update advances its revision'); +select isnt( + (select reviewed_content_revision from public.profile_section_reviews where section_key = 'education'), + (select content_revision from public.profile_section_revisions where section_key = 'education'), + 'content mutation makes the prior review stale' +); + +select throws_ok( + $$ insert into public.profile_section_revisions (user_id, section_key, content_revision) values ('33333333-3333-3333-3333-333333333333', 'education', 99) $$, + '42501'::char(5), null, 'browser cannot directly forge revisions' +); +select throws_ok( + $$ update public.education_entries set is_primary = true where institution = 'Synthetic University' $$, + '42501'::char(5), null, 'browser cannot directly set primary education' +); +select lives_ok( + $$ select public.set_primary_education((select id from public.education_entries where institution = 'Synthetic University')) $$, + 'primary selection RPC succeeds for the owner' +); +select is((select is_primary from public.education_entries where institution = 'Synthetic University'), true, 'selected row is primary'); +select is((select content_revision from public.profile_section_revisions where section_key = 'education'), 3::bigint, 'primary selection advances revision'); +select lives_ok( + $$ select public.set_primary_education((select id from public.education_entries where institution = 'Synthetic University')) $$, + 'reselecting the current primary is idempotent' +); +select is((select content_revision from public.profile_section_revisions where section_key = 'education'), 3::bigint, 'idempotent selection does not change revision'); +select throws_ok($$ select public.review_profile_section('projects') $$, '22023'::char(5), null, 'unknown review key is rejected'); + +reset role; +set role service_role; +select lives_ok( + $$ delete from public.profiles where user_id = '33333333-3333-3333-3333-333333333333' $$, + 'trusted profile deletion with education rows does not break revision triggers' +); + +reset role; +select * from finish(); +rollback; From 314bd24cecf546c7d8d6a67cc18485414580c10f Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sat, 1 Aug 2026 21:37:04 +0300 Subject: [PATCH 02/10] test: extend profile education API coverage Exercise refined profile permissions, education CRUD, constrained primary and review RPCs, review freshness, and cross-user denial through real local Auth and PostgREST. --- docs/TESTING_STRATEGY.md | 5 +- supabase/scripts/api-integration-test.mjs | 69 ++++++++++++++++++++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/docs/TESTING_STRATEGY.md b/docs/TESTING_STRATEGY.md index 24090ec..4a30397 100644 --- a/docs/TESTING_STRATEGY.md +++ b/docs/TESTING_STRATEGY.md @@ -75,8 +75,9 @@ each rule gets explicit positive and negative test cases. **Implemented in the first profile-core/education slice**: pgTAP now verifies refined profile and education schema/grants, browser denial of root-profile deletion and direct revision/review/primary writes, revision freshness after profile and education mutations, locked review RPC behavior, -primary-selection idempotence, and cascade safety. The real Auth/JWT/PostgREST suite is extended -in the following commit; later Phase 1A domains remain future work. +primary-selection idempotence, and cascade safety. The real Auth/JWT/PostgREST suite covers +column restrictions, education CRUD, both RPCs, review staleness, and cross-user denial; later +Phase 1A domains remain future work. - **Migration structure tests**: exact tables, direct `user_id` ownership, required foreign keys, ownership-safe composite parent references, primary-education partial unique constraint, diff --git a/supabase/scripts/api-integration-test.mjs b/supabase/scripts/api-integration-test.mjs index 0c5e982..fae1ac5 100644 --- a/supabase/scripts/api-integration-test.mjs +++ b/supabase/scripts/api-integration-test.mjs @@ -178,6 +178,58 @@ async function main() { }); check('user A can update their own profile', updateOwnA.ok && updateOwnA.data[0].headline === 'Updated by A'); + const forgeProfileMetadata = await rest(tokenA, `/profiles?user_id=eq.${userA.id}`, { + method: 'PATCH', + body: { created_via: 'migration' }, + prefer: 'return=representation', + }); + check('user A cannot forge profile provenance through the REST API', !forgeProfileMetadata.ok && forgeProfileMetadata.status === 403); + + // ---- education CRUD, primary RPC, and review freshness ----------------- + const createEducationA = await rest(tokenA, '/education_entries', { + method: 'POST', + body: { + user_id: userA.id, + institution: 'Synthetic Integration University', + degree: 'BSc', + degree_year: 2, + education_status: 'current', + expected_graduation_month: 7, + expected_graduation_year: 2028, + }, + prefer: 'return=representation', + }); + const educationA = createEducationA.data?.[0]; + check('user A can create current education via the REST API', createEducationA.ok && educationA?.institution === 'Synthetic Integration University'); + + const directPrimary = await rest(tokenA, `/education_entries?id=eq.${educationA.id}`, { + method: 'PATCH', body: { is_primary: true }, prefer: 'return=representation', + }); + check('user A cannot directly set primary education', !directPrimary.ok && directPrimary.status === 403); + + const selectPrimary = await rest(tokenA, '/rpc/set_primary_education', { + method: 'POST', body: { education_id: educationA.id }, + }); + check('user A can select their current education through the primary RPC', selectPrimary.ok); + + const revisionsBeforeReview = await rest(tokenA, '/profile_section_revisions?select=section_key,content_revision'); + check('user A can read their own section revisions', revisionsBeforeReview.ok && revisionsBeforeReview.data.length === 2); + const reviewEducation = await rest(tokenA, '/rpc/review_profile_section', { + method: 'POST', body: { requested_section_key: 'education' }, + }); + const reviewBasic = await rest(tokenA, '/rpc/review_profile_section', { + method: 'POST', body: { requested_section_key: 'basic_profile' }, + }); + check('user A can review both implemented sections through constrained RPCs', reviewEducation.ok && reviewBasic.ok); + + const updateEducationA = await rest(tokenA, `/education_entries?id=eq.${educationA.id}`, { + method: 'PATCH', body: { field: 'Electrical Engineering' }, prefer: 'return=representation', + }); + check('user A can update their education via the REST API', updateEducationA.ok && updateEducationA.data[0].field === 'Electrical Engineering'); + const staleReview = await rest(tokenA, '/profile_section_reviews?select=section_key,reviewed_content_revision'); + const afterMutationRevision = await rest(tokenA, '/profile_section_revisions?section_key=eq.education&select=content_revision'); + check('an education mutation makes its recorded review stale', staleReview.ok && afterMutationRevision.ok && staleReview.data.find((row) => row.section_key === 'education').reviewed_content_revision !== afterMutationRevision.data[0].content_revision); + // ---- isolation: A cannot read or update B's data ---------------------- const readBAsA = await rest(tokenA, `/profiles?user_id=eq.${userB.id}`, {}); check('user A cannot read user B\'s profile (empty result, not an error)', readBAsA.ok && readBAsA.data.length === 0); @@ -196,6 +248,21 @@ async function main() { }); check('user A cannot insert an education entry owned by user B', !forgedInsert.ok && forgedInsert.status === 403); + const createEducationB = await rest(tokenB, '/education_entries', { + method: 'POST', + body: { user_id: userB.id, institution: 'Synthetic B University', education_status: 'current' }, + prefer: 'return=representation', + }); + const foreignPrimary = await rest(tokenA, '/rpc/set_primary_education', { + method: 'POST', body: { education_id: createEducationB.data?.[0]?.id }, + }); + check('user A cannot select user B\'s education as primary', !foreignPrimary.ok && foreignPrimary.status >= 400); + + const directRevisionWrite = await rest(tokenA, '/profile_section_revisions', { + method: 'POST', body: { user_id: userA.id, section_key: 'education', content_revision: 99 }, + }); + check('user A cannot directly forge section revisions', !directRevisionWrite.ok && directRevisionWrite.status === 403); + // ---- isolation: B cannot access A's data ------------------------------ const readAAsB = await rest(tokenB, `/profiles?user_id=eq.${userA.id}`, {}); check('user B cannot read user A\'s profile', readAAsB.ok && readAAsB.data.length === 0); @@ -204,7 +271,7 @@ async function main() { method: 'DELETE', prefer: 'return=representation', }); - check('user B cannot delete user A\'s profile (0 rows affected)', deleteAAsB.ok && deleteAAsB.data.length === 0); + check('user B cannot delete user A\'s profile (browser DELETE is not granted)', !deleteAAsB.ok && deleteAAsB.status === 403); console.log(`\nAll ${checkCount} checks passed.`); } finally { From 3a66f2e6862ad6324723389fc9797172b8a2998b Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sat, 1 Aug 2026 21:39:16 +0300 Subject: [PATCH 03/10] feat: add routed profile workspace Install React Router 8.3.0 and introduce authenticated redirects, a protected profile layout, bookmarkable profile routes, safe route errors, and in-memory router coverage. --- README.md | 2 +- app/package-lock.json | 30 ++++++++- app/package.json | 3 +- app/src/App.tsx | 105 ++++++++++++++++++++++++++------ app/src/AppRouter.test.tsx | 17 ++++++ app/src/pages/ProfileLayout.tsx | 38 ++++++++++++ docs/ARCHITECTURE.md | 5 +- 7 files changed, 174 insertions(+), 26 deletions(-) create mode 100644 app/src/AppRouter.test.tsx create mode 100644 app/src/pages/ProfileLayout.tsx diff --git a/README.md b/README.md index e0d029e..6960b2e 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ hackathons, scholarships, and other resume-building opportunities; compares them detailed saved profile; explains where the student is competitive and where they are not; and tracks the full lifecycle from "found it" to "applied" to "interviewed." -Phase 0 (foundation) is implemented: a React/TypeScript/Vite frontend, a local Supabase stack +Phase 0 is implemented, and the first Phase 1A profile-core/education slice is in progress: a React/TypeScript/Vite frontend, a local Supabase stack (Postgres/Auth/PostgREST), the initial `profiles`/`education_entries` schema with Row Level Security, and CI. See [Local development setup](#local-development-setup) below to run it. diff --git a/app/package-lock.json b/app/package-lock.json index 640529e..bef495c 100644 --- a/app/package-lock.json +++ b/app/package-lock.json @@ -10,7 +10,8 @@ "dependencies": { "@supabase/supabase-js": "^2.110.8", "react": "^19.2.7", - "react-dom": "^19.2.7" + "react-dom": "^19.2.7", + "react-router": "^8.3.0" }, "devDependencies": { "@eslint/js": "^10.0.1", @@ -34,7 +35,7 @@ "vitest": "^4.1.10" }, "engines": { - "node": ">=22 <23" + "node": "22.23.1" } }, "node_modules/@adobe/css-tools": { @@ -1944,6 +1945,11 @@ "dev": true, "license": "MIT" }, + "node_modules/cookie-es": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/cookie-es/-/cookie-es-3.1.1.tgz", + "integrity": "sha512-UaXxwISYJPTr9hwQxMFYZ7kNhSXboMXP+Z3TRX6f1/NyaGPfuNUZOWP1pUEb75B2HjfklIYLVRfWiFZJyC6Npg==" + }, "node_modules/cross-spawn": { "version": "7.0.6", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", @@ -3326,6 +3332,26 @@ "license": "MIT", "peer": true }, + "node_modules/react-router": { + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/react-router/-/react-router-8.3.0.tgz", + "integrity": "sha512-qyPMvW83jGIct3yiieisxdk9M745anqhpIMKN5m1t6yBMfgVPpt77aHOqs5fUlEJRMCGffg9BaQLH9oPVOL7xQ==", + "dependencies": { + "cookie-es": "^3.1.1" + }, + "engines": { + "node": ">=22.22.0" + }, + "peerDependencies": { + "react": ">=19.2.7", + "react-dom": ">=19.2.7" + }, + "peerDependenciesMeta": { + "react-dom": { + "optional": true + } + } + }, "node_modules/redent": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/redent/-/redent-3.0.0.tgz", diff --git a/app/package.json b/app/package.json index 9942fc1..ae44711 100644 --- a/app/package.json +++ b/app/package.json @@ -20,7 +20,8 @@ "dependencies": { "@supabase/supabase-js": "^2.110.8", "react": "^19.2.7", - "react-dom": "^19.2.7" + "react-dom": "^19.2.7", + "react-router": "^8.3.0" }, "devDependencies": { "@eslint/js": "^10.0.1", diff --git a/app/src/App.tsx b/app/src/App.tsx index 5496a3d..ec4257e 100644 --- a/app/src/App.tsx +++ b/app/src/App.tsx @@ -1,42 +1,107 @@ +import { + createBrowserRouter, + Navigate, + Outlet, + useRouteError, +} from 'react-router' +import { RouterProvider } from 'react-router/dom' import { AuthProvider, useAuth } from './contexts/AuthContext' -import { SignInPage } from './pages/SignInPage' +import { ProfileLayout } from './pages/ProfileLayout' import { ProfilePage } from './pages/ProfilePage' +import { SignInPage } from './pages/SignInPage' -// This shell decides which screen to render based on whether a session -// exists. That is a user-experience convenience only -- it runs entirely in -// the browser and does not authorize anything. The actual authorization -// boundary is Postgres Row Level Security, enforced server-side on every -// request regardless of what this component renders (see -// docs/RLS_POLICY_MATRIX.md and AGENTS.md). -function AppShell() { - const { session, loading, initError, retryInit } = useAuth() - +function AuthPending() { + const { initError, loading, retryInit } = useAuth() + if (loading) + return ( +
+

Loading…

+
+ ) if (initError) { return (
-

{initError}

+

Could not check your sign-in status.

) } + return null +} - if (loading) { - return ( -
-

Loading…

-
- ) - } +function RootRedirect() { + const { session, loading, initError } = useAuth() + if (loading || initError) return + return +} + +function RequireAuth() { + const { session, loading, initError } = useAuth() + if (loading || initError) return + return session ? : +} - return session ? : +function SignInRoute() { + const { session, loading, initError } = useAuth() + if (loading || initError) return + return session ? : } +function NotFoundPage() { + return ( +
+

Page not found

+

The page you requested is not available.

+
+ ) +} + +function RouteErrorPage() { + useRouteError() + return ( +
+

Something went wrong

+

+ The page could not be displayed. Please return to your profile and try + again. +

+
+ ) +} + +const router = createBrowserRouter([ + { path: '/', element: , errorElement: }, + { + path: '/sign-in', + element: , + errorElement: , + }, + { + element: , + errorElement: , + children: [ + { + path: '/profile', + element: , + children: [ + { index: true, element: }, + { path: 'basic', element: }, + { path: 'education', element: }, + { path: 'education/new', element: }, + { path: 'education/:educationId/edit', element: }, + ], + }, + ], + }, + { path: '*', element: }, +]) + function App() { return ( - + ) } diff --git a/app/src/AppRouter.test.tsx b/app/src/AppRouter.test.tsx new file mode 100644 index 0000000..9c422ac --- /dev/null +++ b/app/src/AppRouter.test.tsx @@ -0,0 +1,17 @@ +import { render, screen } from '@testing-library/react' +import { createMemoryRouter } from 'react-router' +import { RouterProvider } from 'react-router/dom' +import { describe, expect, it } from 'vitest' + +describe('router foundation', () => { + it('uses an in-memory router to honor a bookmarkable route', async () => { + const memoryRouter = createMemoryRouter( + [{ path: '/profile/education', element:

Education

}], + { initialEntries: ['/profile/education'] }, + ) + render() + expect( + await screen.findByRole('heading', { name: 'Education' }), + ).toBeInTheDocument() + }) +}) diff --git a/app/src/pages/ProfileLayout.tsx b/app/src/pages/ProfileLayout.tsx new file mode 100644 index 0000000..a56795f --- /dev/null +++ b/app/src/pages/ProfileLayout.tsx @@ -0,0 +1,38 @@ +import { NavLink, Outlet } from 'react-router' +import { useState } from 'react' +import { useAuth } from '../contexts/AuthContext' + +export function ProfileLayout() { + const { signOut } = useAuth() + const [error, setError] = useState(null) + + async function handleSignOut() { + setError(null) + const result = await signOut() + if (result.error) setError(result.error) + } + + return ( +
+
+

CareerOS

+ +
+ {error &&

{error}

} + + +
+ ) +} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6109850..7b7e44e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -93,8 +93,9 @@ revision makes that review stale after a relevant create, update, or delete. Sec empty, loading, success, validation-error, server-error, and deletion-confirmation states. **React Router is now justified** because these are real bookmarkable pages with browser-history -behavior, not conditional panels. This is a recorded decision, not an installed dependency in this -planning PR. **TanStack Query is not added yet**: simple typed repositories and explicit reloads +behavior, not conditional panels. The first Phase 1A slice installs the current stable +`react-router` package and uses `createBrowserRouter`/`RouterProvider`; tests use +`createMemoryRouter`. **TanStack Query is not added yet**: simple typed repositories and explicit reloads remain enough until shared cached queries across routes, repetitive mutation invalidation, background refresh, optimistic updates, or more complex server-state coordination provides a concrete need. From 547ffe3f52651e3228c70818f579f6a19d07fb4c Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sat, 1 Aug 2026 21:43:14 +0300 Subject: [PATCH 04/10] feat: add profile and education workflows Add typed profile, education, and review repositories with protected basic-profile and education CRUD workflows, explicit primary selection, validation, review controls, and safe user feedback. --- app/src/App.test.tsx | 16 ++ app/src/App.tsx | 14 +- app/src/lib/educationRepository.ts | 56 +++++ app/src/lib/profileRepository.ts | 29 +++ app/src/lib/profileReviewRepository.ts | 35 ++++ app/src/lib/profileTypes.ts | 59 ++++++ app/src/lib/profileValidation.ts | 71 +++++++ app/src/pages/BasicProfilePage.tsx | 98 +++++++++ app/src/pages/EducationEditorPage.tsx | 140 +++++++++++++ app/src/pages/EducationListPage.tsx | 128 ++++++++++++ app/src/pages/ProfilePage.test.tsx | 202 +++++------------- app/src/pages/ProfilePage.tsx | 271 +++++++++++-------------- docs/USER_WORKFLOWS.md | 5 + 13 files changed, 821 insertions(+), 303 deletions(-) create mode 100644 app/src/lib/educationRepository.ts create mode 100644 app/src/lib/profileRepository.ts create mode 100644 app/src/lib/profileReviewRepository.ts create mode 100644 app/src/lib/profileTypes.ts create mode 100644 app/src/lib/profileValidation.ts create mode 100644 app/src/pages/BasicProfilePage.tsx create mode 100644 app/src/pages/EducationEditorPage.tsx create mode 100644 app/src/pages/EducationListPage.tsx diff --git a/app/src/App.test.tsx b/app/src/App.test.tsx index ad373c3..61d0dee 100644 --- a/app/src/App.test.tsx +++ b/app/src/App.test.tsx @@ -24,6 +24,22 @@ vi.mock('./lib/supabaseClient', () => ({ }, })) +vi.mock('./lib/profileRepository', () => ({ + profileRepository: { + get: () => Promise.resolve({ data: null, error: null }), + }, +})) +vi.mock('./lib/educationRepository', () => ({ + educationRepository: { + list: () => Promise.resolve({ data: [], error: null }), + }, +})) +vi.mock('./lib/profileReviewRepository', () => ({ + profileReviewRepository: { + state: () => Promise.resolve({ data: [], error: null }), + }, +})) + describe('App', () => { beforeEach(() => { mockGetSession.mockReset() diff --git a/app/src/App.tsx b/app/src/App.tsx index ec4257e..6603508 100644 --- a/app/src/App.tsx +++ b/app/src/App.tsx @@ -7,6 +7,9 @@ import { import { RouterProvider } from 'react-router/dom' import { AuthProvider, useAuth } from './contexts/AuthContext' import { ProfileLayout } from './pages/ProfileLayout' +import { BasicProfilePage } from './pages/BasicProfilePage' +import { EducationEditorPage } from './pages/EducationEditorPage' +import { EducationListPage } from './pages/EducationListPage' import { ProfilePage } from './pages/ProfilePage' import { SignInPage } from './pages/SignInPage' @@ -87,10 +90,13 @@ const router = createBrowserRouter([ element: , children: [ { index: true, element: }, - { path: 'basic', element: }, - { path: 'education', element: }, - { path: 'education/new', element: }, - { path: 'education/:educationId/edit', element: }, + { path: 'basic', element: }, + { path: 'education', element: }, + { path: 'education/new', element: }, + { + path: 'education/:educationId/edit', + element: , + }, ], }, ], diff --git a/app/src/lib/educationRepository.ts b/app/src/lib/educationRepository.ts new file mode 100644 index 0000000..59dc613 --- /dev/null +++ b/app/src/lib/educationRepository.ts @@ -0,0 +1,56 @@ +import { supabase } from './supabaseClient' +import type { EducationEntry, EducationStatus } from './profileTypes' + +export type EducationInput = { + institution: string + degree: string | null + field: string | null + degree_year: number | null + expected_graduation_month: number | null + expected_graduation_year: number | null + education_status: EducationStatus + start_date: string | null + end_date: string | null +} + +const columns = + 'id, user_id, institution, degree, field, degree_year, expected_graduation_month, expected_graduation_year, education_status, is_primary, start_date, end_date' + +export const educationRepository = { + async list(userId: string) { + return supabase + .from('education_entries') + .select(columns) + .eq('user_id', userId) + .order('created_at', { ascending: true }) + .returns() + }, + async get(id: string) { + return supabase + .from('education_entries') + .select(columns) + .eq('id', id) + .maybeSingle() + }, + async create(userId: string, input: EducationInput) { + return supabase + .from('education_entries') + .insert({ user_id: userId, ...input }) + .select(columns) + .maybeSingle() + }, + async update(id: string, input: EducationInput) { + return supabase + .from('education_entries') + .update(input) + .eq('id', id) + .select(columns) + .maybeSingle() + }, + async remove(id: string) { + return supabase.from('education_entries').delete().eq('id', id).select('id') + }, + async setPrimary(id: string) { + return supabase.rpc('set_primary_education', { education_id: id }) + }, +} diff --git a/app/src/lib/profileRepository.ts b/app/src/lib/profileRepository.ts new file mode 100644 index 0000000..35e8206 --- /dev/null +++ b/app/src/lib/profileRepository.ts @@ -0,0 +1,29 @@ +import { supabase } from './supabaseClient' +import type { ProfileRow } from './profileTypes' + +export const profileRepository = { + async get(userId: string) { + return supabase + .from('profiles') + .select('user_id, preferred_name, headline, degree_program') + .eq('user_id', userId) + .maybeSingle() + }, + async create(userId: string) { + return supabase + .from('profiles') + .insert({ user_id: userId }) + .select('user_id') + .maybeSingle() + }, + async update( + userId: string, + input: { preferred_name: string | null; headline: string | null }, + ) { + return supabase + .from('profiles') + .update(input) + .eq('user_id', userId) + .select('user_id') + }, +} diff --git a/app/src/lib/profileReviewRepository.ts b/app/src/lib/profileReviewRepository.ts new file mode 100644 index 0000000..22526cc --- /dev/null +++ b/app/src/lib/profileReviewRepository.ts @@ -0,0 +1,35 @@ +import { supabase } from './supabaseClient' +import type { SectionState } from './profileTypes' + +export const profileReviewRepository = { + async state(userId: string) { + const [revisions, reviews] = await Promise.all([ + supabase + .from('profile_section_revisions') + .select('section_key, content_revision') + .eq('user_id', userId), + supabase + .from('profile_section_reviews') + .select('section_key, reviewed_content_revision') + .eq('user_id', userId), + ]) + if (revisions.error) return { data: null, error: revisions.error } + if (reviews.error) return { data: null, error: reviews.error } + return { + data: (revisions.data ?? []).map((revision) => ({ + section_key: revision.section_key as SectionState['section_key'], + content_revision: revision.content_revision, + reviewed_content_revision: + reviews.data?.find( + (review) => review.section_key === revision.section_key, + )?.reviewed_content_revision ?? null, + })), + error: null, + } + }, + async review(sectionKey: SectionState['section_key']) { + return supabase.rpc('review_profile_section', { + requested_section_key: sectionKey, + }) + }, +} diff --git a/app/src/lib/profileTypes.ts b/app/src/lib/profileTypes.ts new file mode 100644 index 0000000..eeaa4ca --- /dev/null +++ b/app/src/lib/profileTypes.ts @@ -0,0 +1,59 @@ +export type EducationStatus = + 'current' | 'completed' | 'paused' | 'withdrawn' | 'unknown' + +export interface ProfileRow { + user_id: string + preferred_name: string | null + headline: string | null + degree_program: string | null +} + +export interface EducationEntry { + id: string + user_id: string + institution: string + degree: string | null + field: string | null + degree_year: number | null + expected_graduation_month: number | null + expected_graduation_year: number | null + education_status: EducationStatus + is_primary: boolean + start_date: string | null + end_date: string | null +} + +export interface SectionState { + section_key: 'basic_profile' | 'education' + content_revision: number + reviewed_content_revision: number | null +} + +export type SafeError = + 'network' | 'permission' | 'validation' | 'missing' | 'unknown' + +export function safeError( + error: { code?: string; message?: string } | null, +): SafeError { + if (!error) return 'unknown' + if (error.code === '42501') return 'permission' + if ( + error.code === '23514' || + error.code === '23505' || + error.code === '22023' + ) + return 'validation' + if (error.code === 'PGRST116') return 'missing' + if (!error.code) return 'network' + return 'unknown' +} + +export function errorMessage(kind: SafeError): string { + return { + network: 'We could not reach the service. Please try again.', + permission: 'You do not have permission to make that change.', + validation: 'Check the highlighted values and try again.', + missing: 'This item no longer exists. The page was refreshed.', + unknown: 'The change could not be saved. Please try again.', + }[kind] +} diff --git a/app/src/lib/profileValidation.ts b/app/src/lib/profileValidation.ts new file mode 100644 index 0000000..e3d2732 --- /dev/null +++ b/app/src/lib/profileValidation.ts @@ -0,0 +1,71 @@ +import type { EducationInput } from './educationRepository' + +export type FieldErrors = Record +const trimmed = (value: string) => value.trim() +const optional = (value: string) => (trimmed(value) ? trimmed(value) : null) + +export function normalizeProfile(preferredName: string, headline: string) { + return { + preferred_name: optional(preferredName), + headline: optional(headline), + } +} + +export function validateProfile( + preferredName: string, + headline: string, +): FieldErrors { + const errors: FieldErrors = {} + if (trimmed(preferredName).length > 100) + errors.preferred_name = 'Preferred name must be 100 characters or fewer.' + if (trimmed(headline).length > 160) + errors.headline = 'Headline must be 160 characters or fewer.' + return errors +} + +export function normalizeEducation( + values: Record, +): EducationInput { + const number = (value: string) => (value === '' ? null : Number(value)) + return { + institution: trimmed(values.institution), + degree: optional(values.degree), + field: optional(values.field), + degree_year: number(values.degree_year), + expected_graduation_month: number(values.expected_graduation_month), + expected_graduation_year: number(values.expected_graduation_year), + education_status: + values.education_status as EducationInput['education_status'], + start_date: values.start_date || null, + end_date: values.end_date || null, + } +} + +export function validateEducation(values: Record): FieldErrors { + const errors: FieldErrors = {} + if (!trimmed(values.institution)) + errors.institution = 'Institution is required.' + if (trimmed(values.institution).length > 200) + errors.institution = 'Institution must be 200 characters or fewer.' + if (trimmed(values.degree).length > 160) + errors.degree = 'Degree must be 160 characters or fewer.' + if (trimmed(values.field).length > 160) + errors.field = 'Field must be 160 characters or fewer.' + const month = values.expected_graduation_month + const year = values.expected_graduation_year + if ((month === '') !== (year === '')) + errors.graduation = 'Enter both graduation month and year, or neither.' + if (values.education_status !== 'current' && (month || year)) + errors.graduation = 'Expected graduation is only for current education.' + if (values.education_status === 'current' && values.end_date) + errors.end_date = 'Current education cannot have an end date.' + if (values.education_status === 'completed' && !values.end_date) + errors.end_date = 'Completed education needs an end date.' + if ( + values.start_date && + values.end_date && + values.end_date < values.start_date + ) + errors.end_date = 'End date cannot precede start date.' + return errors +} diff --git a/app/src/pages/BasicProfilePage.tsx b/app/src/pages/BasicProfilePage.tsx new file mode 100644 index 0000000..28b244a --- /dev/null +++ b/app/src/pages/BasicProfilePage.tsx @@ -0,0 +1,98 @@ +import { useCallback, useEffect, useState, type FormEvent } from 'react' +import { useAuth } from '../contexts/AuthContext' +import { profileRepository } from '../lib/profileRepository' +import { profileReviewRepository } from '../lib/profileReviewRepository' +import { normalizeProfile, validateProfile } from '../lib/profileValidation' + +export function BasicProfilePage() { + const { session } = useAuth() + const userId = session?.user.id + const [name, setName] = useState('') + const [headline, setHeadline] = useState('') + const [legacy, setLegacy] = useState(null) + const [message, setMessage] = useState(null) + const [errors, setErrors] = useState>({}) + const load = useCallback(async () => { + if (!userId) return + const { data } = await profileRepository.get(userId) + if (data) { + setName(data.preferred_name ?? '') + setHeadline(data.headline ?? '') + setLegacy(data.degree_program) + } + }, [userId]) + useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect -- asynchronous initial load updates state after the repository resolves + void load() + }, [load]) + async function save(event: FormEvent) { + event.preventDefault() + const next = validateProfile(name, headline) + setErrors(next) + if (Object.keys(next).length || !userId) return + const result = await profileRepository.update( + userId, + normalizeProfile(name, headline), + ) + setMessage(result.error ? 'Profile could not be saved.' : 'Saved.') + } + async function review() { + const result = await profileReviewRepository.review('basic_profile') + setMessage( + result.error ? 'Review could not be saved.' : 'Basic profile reviewed.', + ) + } + return ( +
+

Basic profile

+ {legacy && ( +

+ Previous degree-program information is available for reference. + Confirm it by editing an education entry. +

+ )} +
+ {Object.keys(errors).length > 0 && ( +
+

Please correct the fields below.

+
+ )} +
+ + setName(event.target.value)} + aria-describedby={ + errors.preferred_name ? 'preferred-name-error' : undefined + } + /> + {errors.preferred_name && ( + + )} +
+
+ + setHeadline(event.target.value)} + aria-describedby={errors.headline ? 'headline-error' : undefined} + /> + {errors.headline && ( + + )} +
+ +
+ + {message &&

{message}

} +
+ ) +} diff --git a/app/src/pages/EducationEditorPage.tsx b/app/src/pages/EducationEditorPage.tsx new file mode 100644 index 0000000..4e29b48 --- /dev/null +++ b/app/src/pages/EducationEditorPage.tsx @@ -0,0 +1,140 @@ +import { Link, useNavigate, useParams } from 'react-router' +import { useCallback, useEffect, useState, type FormEvent } from 'react' +import { useAuth } from '../contexts/AuthContext' +import { educationRepository } from '../lib/educationRepository' +import { normalizeEducation, validateEducation } from '../lib/profileValidation' + +const empty = { + institution: '', + degree: '', + field: '', + degree_year: '', + expected_graduation_month: '', + expected_graduation_year: '', + education_status: 'unknown', + start_date: '', + end_date: '', +} +export function EducationEditorPage() { + const { educationId } = useParams() + const navigate = useNavigate() + const { session } = useAuth() + const [values, setValues] = useState>(empty) + const [errors, setErrors] = useState>({}) + const [message, setMessage] = useState(null) + const load = useCallback(async () => { + if (!educationId) return + const result = await educationRepository.get(educationId) + if (result.error || !result.data) { + setMessage('This education entry no longer exists.') + return + } + const entry = result.data + setValues({ + institution: entry.institution, + degree: entry.degree ?? '', + field: entry.field ?? '', + degree_year: entry.degree_year?.toString() ?? '', + expected_graduation_month: + entry.expected_graduation_month?.toString() ?? '', + expected_graduation_year: + entry.expected_graduation_year?.toString() ?? '', + education_status: entry.education_status, + start_date: entry.start_date ?? '', + end_date: entry.end_date ?? '', + }) + }, [educationId]) + useEffect(() => { + void load() + }, [load]) + function field(name: string, value: string) { + setValues((current) => ({ ...current, [name]: value })) + } + async function save(event: FormEvent) { + event.preventDefault() + const next = validateEducation(values) + setErrors(next) + if (Object.keys(next).length) return + const input = normalizeEducation(values) + const result = educationId + ? await educationRepository.update(educationId, input) + : await educationRepository.create(session!.user.id, input) + if (result.error || !result.data) { + setMessage( + 'Education could not be saved. Check the values and try again.', + ) + return + } + navigate('/profile/education', { replace: true }) + } + const input = (name: string, label: string, type = 'text') => ( +
+ + field(name, event.target.value)} + aria-describedby={errors[name] ? `${name}-error` : undefined} + /> + {errors[name] && ( + + )} +
+ ) + return ( +
+

{educationId ? 'Edit education' : 'Add education'}

+
+ {Object.keys(errors).length > 0 && ( +
Please correct the fields below.
+ )} + {input('institution', 'Institution')} + {input('degree', 'Degree')} + {input('field', 'Field')} + {input('degree_year', 'Degree year', 'number')} +
+ Education dates and status +
+ + +
+ {input('start_date', 'Start date', 'date')} + {input('end_date', 'End date', 'date')} + {input( + 'expected_graduation_month', + 'Expected graduation month', + 'number', + )} + {input( + 'expected_graduation_year', + 'Expected graduation year', + 'number', + )} + {errors.graduation &&

{errors.graduation}

} +
+ +
+ {message &&

{message}

} +

+ Cancel +

+
+ ) +} diff --git a/app/src/pages/EducationListPage.tsx b/app/src/pages/EducationListPage.tsx new file mode 100644 index 0000000..66cb37e --- /dev/null +++ b/app/src/pages/EducationListPage.tsx @@ -0,0 +1,128 @@ +import { Link } from 'react-router' +import { useCallback, useEffect, useState } from 'react' +import { useAuth } from '../contexts/AuthContext' +import { educationRepository } from '../lib/educationRepository' +import { profileReviewRepository } from '../lib/profileReviewRepository' +import type { EducationEntry } from '../lib/profileTypes' + +export function EducationListPage() { + const { session } = useAuth() + const userId = session?.user.id + const [entries, setEntries] = useState([]) + const [status, setStatus] = useState<'loading' | 'ready' | 'error'>('loading') + const [deleting, setDeleting] = useState(null) + const [message, setMessage] = useState(null) + const load = useCallback(async () => { + if (!userId) return + setStatus('loading') + const result = await educationRepository.list(userId) + if (result.error) return setStatus('error') + setEntries(result.data) + setStatus('ready') + }, [userId]) + useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect -- asynchronous initial load updates state after the repository resolves + void load() + }, [load]) + async function primary(id: string) { + const result = await educationRepository.setPrimary(id) + setMessage( + result.error + ? 'Only a current education you own can be primary.' + : 'Primary education updated.', + ) + await load() + } + async function remove(id: string) { + const result = await educationRepository.remove(id) + if (result.error || !result.data?.length) + setMessage('This entry could not be deleted. It may already be gone.') + else setMessage('Education entry deleted.') + setDeleting(null) + await load() + } + async function review() { + const result = await profileReviewRepository.review('education') + setMessage( + result.error + ? 'Education review could not be saved.' + : 'Education reviewed.', + ) + } + if (status === 'loading') + return ( +
+

Education

+

Loading…

+
+ ) + if (status === 'error') + return ( +
+

Education

+

Education could not be loaded.

+ +
+ ) + return ( +
+

Education

+

+ Add education +

+ {entries.length === 0 ? ( +

No education entries yet.

+ ) : ( +
    + {entries.map((entry) => ( +
  • + {entry.institution} + {entry.is_primary && ' — Primary'} +
    + {entry.education_status} +
    + Edit + {entry.education_status === 'current' && !entry.is_primary && ( + + )} + + {deleting === entry.id && ( +
    +

    Delete this education entry?

    + + +
    + )} +
    • + ))} +
+ )} + + {message &&

{message}

} +
+ ) +} diff --git a/app/src/pages/ProfilePage.test.tsx b/app/src/pages/ProfilePage.test.tsx index c169a9b..2761145 100644 --- a/app/src/pages/ProfilePage.test.tsx +++ b/app/src/pages/ProfilePage.test.tsx @@ -1,165 +1,71 @@ import { render, screen } from '@testing-library/react' -import userEvent from '@testing-library/user-event' -import { describe, expect, it, vi, beforeEach } from 'vitest' +import { describe, expect, it, vi } from 'vitest' +import { MemoryRouter } from 'react-router' import { ProfilePage } from './ProfilePage' -const mockSignOut = vi.fn() -const mockMaybeSingle = vi.fn() -const mockInsert = vi.fn() -const mockUpdateEq = vi.fn() -const mockUpdateSelect = vi.fn() +const getProfile = vi.fn() +const listEducation = vi.fn() +const sectionState = vi.fn() vi.mock('../contexts/AuthContext', () => ({ - useAuth: () => ({ - session: { user: { id: 'user-1' } }, - loading: false, - signOut: mockSignOut, - }), + useAuth: () => ({ session: { user: { id: 'user-1' } } }), })) - -vi.mock('../lib/supabaseClient', () => ({ - supabase: { - from: () => ({ - select: () => ({ - eq: () => ({ - maybeSingle: () => mockMaybeSingle(), - }), - }), - insert: (...args: unknown[]) => mockInsert(...args), - update: () => ({ - eq: (...args: unknown[]) => { - mockUpdateEq(...args) - return { select: (...args2: unknown[]) => mockUpdateSelect(...args2) } - }, - }), - }), +vi.mock('../lib/profileRepository', () => ({ + profileRepository: { + get: (...args: unknown[]) => getProfile(...args), + create: vi.fn(), + update: vi.fn(), + }, +})) +vi.mock('../lib/educationRepository', () => ({ + educationRepository: { list: (...args: unknown[]) => listEducation(...args) }, +})) +vi.mock('../lib/profileReviewRepository', () => ({ + profileReviewRepository: { + state: (...args: unknown[]) => sectionState(...args), }, })) describe('ProfilePage', () => { - beforeEach(() => { - mockMaybeSingle.mockReset() - mockInsert.mockReset() - mockUpdateEq.mockReset() - mockUpdateSelect.mockReset() - mockSignOut.mockReset() - mockSignOut.mockResolvedValue({ error: null }) - }) - - it('shows a loading state before the profile query resolves', () => { - mockMaybeSingle.mockReturnValue(new Promise(() => {})) // never resolves - - render() - + it('shows a loading state before profile data resolves', () => { + getProfile.mockReturnValue(new Promise(() => {})) + listEducation.mockResolvedValue({ data: [], error: null }) + sectionState.mockResolvedValue({ data: [], error: null }) + render( + + + , + ) expect(screen.getByText(/loading/i)).toBeInTheDocument() }) - - it('shows an error state when the profile query fails', async () => { - mockMaybeSingle.mockResolvedValue({ - data: null, - error: { message: 'network error' }, - }) - - render() - - expect(await screen.findByRole('alert')).toHaveTextContent('network error') - }) - - it('shows an empty state and lets the user create a profile when none exists yet', async () => { - mockMaybeSingle.mockResolvedValueOnce({ data: null, error: null }) - mockInsert.mockResolvedValue({ error: null }) - mockMaybeSingle.mockResolvedValueOnce({ - data: { user_id: 'user-1', headline: '' }, - error: null, - }) - const user = userEvent.setup() - - render() - - expect(await screen.findByText(/no profile yet/i)).toBeInTheDocument() - - await user.click(screen.getByRole('button', { name: /create profile/i })) - - expect(mockInsert).toHaveBeenCalledWith({ user_id: 'user-1', headline: '' }) - }) - - it('loads the existing headline and saves an edit (authenticated state)', async () => { - mockMaybeSingle.mockResolvedValue({ - data: { user_id: 'user-1', headline: 'Original headline' }, - error: null, - }) - mockUpdateSelect.mockResolvedValue({ - data: [{ user_id: 'user-1' }], - error: null, - }) - const user = userEvent.setup() - - render() - - const headlineInput = await screen.findByLabelText(/headline/i) - expect(headlineInput).toHaveValue('Original headline') - - await user.clear(headlineInput) - await user.type(headlineInput, 'New headline') - await user.click(screen.getByRole('button', { name: /^save$/i })) - - expect(mockUpdateEq).toHaveBeenCalledWith('user_id', 'user-1') - expect(await screen.findByText(/saved/i)).toBeInTheDocument() - }) - - it('does not show Saved when the update affects zero rows, and reloads into the missing-profile state', async () => { - mockMaybeSingle - .mockResolvedValueOnce({ - data: { user_id: 'user-1', headline: 'Original headline' }, - error: null, - }) - .mockResolvedValueOnce({ data: null, error: null }) - mockUpdateSelect.mockResolvedValue({ data: [], error: null }) - const user = userEvent.setup() - - render() - - const headlineInput = await screen.findByLabelText(/headline/i) - await user.clear(headlineInput) - await user.type(headlineInput, 'New headline') - await user.click(screen.getByRole('button', { name: /^save$/i })) - - expect(await screen.findByText(/no profile yet/i)).toBeInTheDocument() - expect(screen.queryByText(/saved/i)).not.toBeInTheDocument() - }) - - it('calls signOut when the sign-out button is clicked', async () => { - mockMaybeSingle.mockResolvedValue({ - data: { user_id: 'user-1', headline: 'x' }, - error: null, - }) - const user = userEvent.setup() - - render() - await screen.findByLabelText(/headline/i) - - await user.click(screen.getByRole('button', { name: /sign out/i })) - - expect(mockSignOut).toHaveBeenCalled() - }) - - it('shows feedback when sign-out fails', async () => { - mockMaybeSingle.mockResolvedValue({ - data: { user_id: 'user-1', headline: 'x' }, + it('shows actionable partial completeness without a percentage', async () => { + getProfile.mockResolvedValue({ data: { user_id: 'user-1' }, error: null }) + listEducation.mockResolvedValue({ data: [], error: null }) + sectionState.mockResolvedValue({ + data: [ + { + section_key: 'basic_profile', + content_revision: 1, + reviewed_content_revision: 1, + }, + { + section_key: 'education', + content_revision: 0, + reviewed_content_revision: null, + }, + ], error: null, }) - mockSignOut.mockResolvedValue({ - error: 'Sign-out failed. Please try again.', - }) - const user = userEvent.setup() - - render() - await screen.findByLabelText(/headline/i) - - await user.click(screen.getByRole('button', { name: /sign out/i })) - - expect(await screen.findByRole('alert')).toHaveTextContent( - /sign-out failed/i, + render( + + + , ) + expect( + await screen.findByText(/add or select primary education/i), + ).toBeInTheDocument() + expect( + screen.getByRole('heading', { name: /not yet available/i }), + ).toBeInTheDocument() }) }) diff --git a/app/src/pages/ProfilePage.tsx b/app/src/pages/ProfilePage.tsx index 9f20ce4..926442b 100644 --- a/app/src/pages/ProfilePage.tsx +++ b/app/src/pages/ProfilePage.tsx @@ -1,168 +1,137 @@ -import { useCallback, useEffect, useState, type FormEvent } from 'react' -import { supabase } from '../lib/supabaseClient' +import { Link } from 'react-router' +import { useCallback, useEffect, useState } from 'react' import { useAuth } from '../contexts/AuthContext' - -interface ProfileRow { - user_id: string - headline: string | null +import { educationRepository } from '../lib/educationRepository' +import { profileRepository } from '../lib/profileRepository' +import { profileReviewRepository } from '../lib/profileReviewRepository' +import type { SectionState } from '../lib/profileTypes' + +function reviewed( + state: SectionState[] | null, + section: SectionState['section_key'], +) { + const item = state?.find((entry) => entry.section_key === section) + return item?.reviewed_content_revision === item?.content_revision } -type Status = 'loading' | 'loaded' | 'empty' | 'error' - export function ProfilePage() { - const { session, signOut } = useAuth() + const { session } = useAuth() const userId = session?.user.id + const [status, setStatus] = useState<'loading' | 'empty' | 'ready' | 'error'>( + 'loading', + ) + const [state, setState] = useState(null) + const [hasPrimary, setHasPrimary] = useState(false) + const [degreeYear, setDegreeYear] = useState(false) + const [graduation, setGraduation] = useState(false) - const [status, setStatus] = useState('loading') - const [headline, setHeadline] = useState('') - const [errorMessage, setErrorMessage] = useState(null) - const [saving, setSaving] = useState(false) - const [saveMessage, setSaveMessage] = useState(null) - const [signOutError, setSignOutError] = useState(null) - - const loadProfile = useCallback(async () => { + const load = useCallback(async () => { if (!userId) return setStatus('loading') - setErrorMessage(null) - - // RLS scopes this to the signed-in user's own row; a missing profile - // shows an "empty" state here, never another user's data -- there is no - // code path in this query that could return a row belonging to anyone - // else, regardless of what this component does with the result. - const { data, error } = await supabase - .from('profiles') - .select('user_id, headline') - .eq('user_id', userId) - .maybeSingle() - - if (error) { - setStatus('error') - setErrorMessage(error.message) - return - } - if (!data) { - setStatus('empty') - return - } - setHeadline(data.headline ?? '') - setStatus('loaded') + const [profile, education, sections] = await Promise.all([ + profileRepository.get(userId), + educationRepository.list(userId), + profileReviewRepository.state(userId), + ]) + if (profile.error || education.error || sections.error) + return setStatus('error') + if (!profile.data) return setStatus('empty') + const primary = education.data.find((entry) => entry.is_primary) + setHasPrimary(Boolean(primary)) + setDegreeYear(Boolean(primary?.degree_year)) + setGraduation( + Boolean( + primary?.expected_graduation_month && primary?.expected_graduation_year, + ), + ) + setState(sections.data) + setStatus('ready') }, [userId]) useEffect(() => { - // eslint-disable-next-line react-hooks/set-state-in-effect -- fetch-on-mount pattern; state updates land asynchronously once the request resolves - void loadProfile() - }, [loadProfile]) - - async function handleCreateProfile() { - if (!userId) return - setSaving(true) - setErrorMessage(null) - - const { error } = await supabase - .from('profiles') - .insert({ user_id: userId, headline: '' }) - - setSaving(false) - if (error) { - setStatus('error') - setErrorMessage(error.message) - return - } - await loadProfile() - } - - async function handleSave(event: FormEvent) { - event.preventDefault() - if (!userId) return - setSaving(true) - setSaveMessage(null) - setErrorMessage(null) - - // .select() makes the affected rows come back in `data` so a save that - // matches zero rows (RLS-filtered, or the row was deleted concurrently) - // can be told apart from an actual save -- PostgREST returns 200 with no - // error in both cases, so `error` alone is not enough to confirm success. - const { data, error } = await supabase - .from('profiles') - .update({ headline }) - .eq('user_id', userId) - .select('user_id') - - setSaving(false) - if (error) { - setErrorMessage(error.message) - return - } - if (!data || data.length === 0) { - await loadProfile() - return - } - setSaveMessage('Saved.') - } - - async function handleSignOut() { - setSignOutError(null) - const { error } = await signOut() - if (error) { - setSignOutError(error) - } - } - - return ( -
-
-

Your profile

+ // eslint-disable-next-line react-hooks/set-state-in-effect -- asynchronous initial load updates state after the repository resolves + void load() + }, [load]) + if (status === 'loading') + return ( +
+

Your profile

+

Loading…

+
+ ) + if (status === 'error') + return ( +
+

Your profile

+

Your profile could not be loaded.

+ +
+ ) + if (status === 'empty') + return ( +
+

Your profile

+

No profile yet.

-
- - {signOutError &&

{signOutError}

} - - {status === 'loading' &&

Loading…

} - - {status === 'error' &&

{errorMessage}

} - - {status === 'empty' && ( -
-

No profile yet.

- -
- )} - - {status === 'loaded' && ( -
-
- - setHeadline(event.target.value)} - /> -
- - {saveMessage &&

{saveMessage}

} -
- )} -
+ + ) + const checks = [ + [ + 'Basic profile review', + reviewed(state, 'basic_profile'), + '/profile/basic', + 'Review basic profile', + ], + [ + 'Primary education', + hasPrimary, + '/profile/education', + 'Add or select primary education', + ], + ['Degree year', degreeYear, '/profile/education', 'Add degree year'], + [ + 'Expected graduation', + graduation, + '/profile/education', + 'Add expected graduation timing', + ], + [ + 'Education review', + reviewed(state, 'education'), + '/profile/education', + 'Review education', + ], + ] + return ( +
+

Your profile

+

Complete the implemented profile details below.

+
    + {checks.map(([label, ok, href, action]) => ( +
  • + {label as string}:{' '} + {ok ? ( + 'Present' + ) : ( + {action as string} + )} +
    • + ))} +
+

Not yet available

+

+ Experience, skills, languages, preferences, targets, and work + eligibility are not yet available in CareerOS. +

+
) } diff --git a/docs/USER_WORKFLOWS.md b/docs/USER_WORKFLOWS.md index 04c705f..a3c5715 100644 --- a/docs/USER_WORKFLOWS.md +++ b/docs/USER_WORKFLOWS.md @@ -5,6 +5,11 @@ architecture and data-model decisions grounded in real usage rather than abstrac ## Workflow 1 — Phase 1A manual profile setup +**Implemented first slice**: the routed profile workspace currently supports basic-profile editing +and education drafts/CRUD. A user explicitly selects one current education as primary; no other +entry is promoted automatically. The overview shows only the implemented review/education checks +and labels all other Phase 1A sections as not yet available. + 1. User signs in and sees a profile overview with named completeness checks and concrete next actions, never just a mysterious percentage. 2. User edits education, experience/research, projects/links, skills/evidence, languages, From 6211dd3b1f7be6eb720d8a4df656c1f4f017b5df Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sat, 1 Aug 2026 21:44:42 +0300 Subject: [PATCH 05/10] feat: add education completeness slice Add a pure versioned education-slice completeness evaluator with public-safe tests and document its named checks, neutral deferred availability state, and migration decisions. --- README.md | 3 +- app/src/lib/profileCompleteness.test.ts | 35 +++++++++++++ app/src/lib/profileCompleteness.ts | 65 +++++++++++++++++++++++++ docs/DEVELOPMENT_ROADMAP.md | 9 ++-- docs/OPEN_QUESTIONS.md | 8 +-- docs/PROFILE_COMPLETENESS_SPEC.md | 8 +-- 6 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 app/src/lib/profileCompleteness.test.ts create mode 100644 app/src/lib/profileCompleteness.ts diff --git a/README.md b/README.md index 6960b2e..1c22617 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,8 @@ hackathons, scholarships, and other resume-building opportunities; compares them detailed saved profile; explains where the student is competitive and where they are not; and tracks the full lifecycle from "found it" to "applied" to "interviewed." -Phase 0 is implemented, and the first Phase 1A profile-core/education slice is in progress: a React/TypeScript/Vite frontend, a local Supabase stack +Phase 0 is implemented, and the first Phase 1A profile-core/education slice adds routed manual +profile and primary-education workflows: a React/TypeScript/Vite frontend, a local Supabase stack (Postgres/Auth/PostgREST), the initial `profiles`/`education_entries` schema with Row Level Security, and CI. See [Local development setup](#local-development-setup) below to run it. diff --git a/app/src/lib/profileCompleteness.test.ts b/app/src/lib/profileCompleteness.test.ts new file mode 100644 index 0000000..822d783 --- /dev/null +++ b/app/src/lib/profileCompleteness.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest' +import { + completenessVersion, + evaluateEducationSlice, +} from './profileCompleteness' + +describe('education completeness slice', () => { + it('is versioned and exposes named missing actions without a percentage', () => { + const checks = evaluateEducationSlice( + [], + [ + { + section_key: 'basic_profile', + content_revision: 0, + reviewed_content_revision: 0, + }, + { + section_key: 'education', + content_revision: 0, + reviewed_content_revision: null, + }, + ], + ) + expect(completenessVersion).toBe('profile-completeness/v2-slice-education') + expect( + checks.find((check) => check.id === 'primary_education'), + ).toMatchObject({ + outcome: 'missing', + action: 'Add or select primary education', + }) + expect( + checks.find((check) => check.id === 'education_review'), + ).toMatchObject({ outcome: 'unconfirmed' }) + }) +}) diff --git a/app/src/lib/profileCompleteness.ts b/app/src/lib/profileCompleteness.ts new file mode 100644 index 0000000..6727b01 --- /dev/null +++ b/app/src/lib/profileCompleteness.ts @@ -0,0 +1,65 @@ +import type { EducationEntry, SectionState } from './profileTypes' + +export const completenessVersion = + 'profile-completeness/v2-slice-education' as const +export type CompletenessOutcome = 'present' | 'missing' | 'unconfirmed' +export type CompletenessCheckId = + | 'basic_profile_review' + | 'primary_education' + | 'degree_year' + | 'graduation_timing' + | 'education_review' + +export interface CompletenessCheck { + id: CompletenessCheckId + outcome: CompletenessOutcome + action: string + href: string +} + +function reviewed(state: SectionState[], section: SectionState['section_key']) { + const item = state.find((entry) => entry.section_key === section) + return item?.reviewed_content_revision === item?.content_revision +} + +export function evaluateEducationSlice( + entries: EducationEntry[], + state: SectionState[], +): CompletenessCheck[] { + const primary = entries.find((entry) => entry.is_primary) + return [ + { + id: 'basic_profile_review', + outcome: reviewed(state, 'basic_profile') ? 'present' : 'unconfirmed', + action: 'Review basic profile', + href: '/profile/basic', + }, + { + id: 'primary_education', + outcome: primary ? 'present' : 'missing', + action: 'Add or select primary education', + href: '/profile/education', + }, + { + id: 'degree_year', + outcome: primary?.degree_year ? 'present' : 'missing', + action: 'Add degree year', + href: '/profile/education', + }, + { + id: 'graduation_timing', + outcome: + primary?.expected_graduation_month && primary?.expected_graduation_year + ? 'present' + : 'missing', + action: 'Add expected graduation timing', + href: '/profile/education', + }, + { + id: 'education_review', + outcome: reviewed(state, 'education') ? 'present' : 'unconfirmed', + action: 'Review education', + href: '/profile/education', + }, + ] +} diff --git a/docs/DEVELOPMENT_ROADMAP.md b/docs/DEVELOPMENT_ROADMAP.md index 217d1a9..2608cdc 100644 --- a/docs/DEVELOPMENT_ROADMAP.md +++ b/docs/DEVELOPMENT_ROADMAP.md @@ -25,16 +25,17 @@ process. save it (RLS-permitted), sign out — verified both by automated tests and by hand in a real browser against the real local stack. No opportunity data yet. -## Next implementation PR — Profile core and primary education vertical slice +## First Phase 1A PR — Profile core and primary education vertical slice — **implemented** -This remains deliberately small. It will implement only the forward migration of existing Phase 0 +This deliberately small slice implements only the forward migration of existing Phase 0 education values, refined `profiles` and `education_entries`, `profile_section_reviews` for `basic_profile` and `education`, exact grants/RLS/pgTAP/API tests, React Router foundation, profile overview, education CRUD, and completeness checks limited to those implemented inputs. It must not claim full completeness while other Phase 1A domains do not exist. -The forward migration must preserve `profiles.degree_program` and `profiles.degree_year` by copying -them into the selected or created primary education row before they stop being authoritative. +The migration retains the legacy fields as deprecated compatibility data. It safely copies a legacy +degree year only to one existing education row when unambiguous; it does not select a primary, +infer a status, or map the ambiguous degree-program text. ## Phase 1A — Structured manual career profile diff --git a/docs/OPEN_QUESTIONS.md b/docs/OPEN_QUESTIONS.md index 138987c..5cc30fa 100644 --- a/docs/OPEN_QUESTIONS.md +++ b/docs/OPEN_QUESTIONS.md @@ -36,10 +36,10 @@ phase that needs it. where rules clearly fail is the likely direction, but this needs a dedicated design pass before Phase 1 of [DEVELOPMENT_ROADMAP.md](DEVELOPMENT_ROADMAP.md), including a cost check (LLM resume parsing would be the first real LLM cost, ahead of the Phase 7 explanation feature). -- **Primary-education migration selection**: the next implementation PR must define the - deterministic fallback when Phase 0 values exist but no education entry is an obvious primary - candidate (for example, create a clearly marked incomplete primary row for user review rather - than silently selecting among multiple entries). +- ~~**Primary-education migration selection**~~ — resolved in the first profile-core slice: no + legacy row is selected as primary or inferred current. A degree year is copied only to exactly + one existing row when empty; ambiguous program text remains a deprecated read-only notice for + manual review. ## Compliance diff --git a/docs/PROFILE_COMPLETENESS_SPEC.md b/docs/PROFILE_COMPLETENESS_SPEC.md index 2048b75..b3f33ea 100644 --- a/docs/PROFILE_COMPLETENESS_SPEC.md +++ b/docs/PROFILE_COMPLETENESS_SPEC.md @@ -71,6 +71,8 @@ the section revision; “stale” means it does not. | `all-technical-skills-evidenced` | Two technical claims, each with one valid typed evidence row. | `technical_skill` and `skill_evidence` are `present`; lacking count 0. | | `eligibility-stale` | Work-eligibility row exists, but its confirmation or section review predates a change. | `eligibility_reviewed = unconfirmed`. | -The next implementation vertical slice evaluates only checks whose inputs exist in that slice; all -other checks are reported as **not yet implemented by the product**, not as user omissions. The -full v2 evaluation begins only after its remaining Phase 1A tables exist. +The implemented first vertical slice exposes `profile-completeness/v2-slice-education`. It evaluates +only `basic_profile_review`, `primary_education`, `degree_year`, `graduation_timing`, and +`education_review`, using `present`, `missing`, and `unconfirmed`. All remaining v2 inputs are a +UI-only `not_implemented` availability state, never a user omission or a percentage. The full v2 +evaluation begins only after its remaining Phase 1A tables exist. From 99e4c82767a061f614641bc13425b7bdf0462450 Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sun, 2 Aug 2026 00:13:18 +0300 Subject: [PATCH 06/10] fix: make profile migration legacy-safe Make migrated confirmation timestamps nullable, add an automated Phase 0-to-Phase 1A compatibility path, and pin React Router exactly. --- .github/workflows/ci.yml | 6 ++++++ README.md | 3 +++ app/package-lock.json | 3 ++- app/package.json | 2 +- docs/DATA_MODEL.md | 4 ++++ docs/TESTING_STRATEGY.md | 6 ++++++ .../migration/assert_phase1_migration.sql | 14 +++++++++++++ .../fixtures/migration/phase0_fixture.sql | 21 +++++++++++++++++++ .../20260801213000_profile_core_education.sql | 8 +++---- .../scripts/migration-compatibility-test.sh | 10 +++++++++ 10 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 supabase/fixtures/migration/assert_phase1_migration.sql create mode 100644 supabase/fixtures/migration/phase0_fixture.sql create mode 100755 supabase/scripts/migration-compatibility-test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64acb48..bc0ff2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,6 +62,12 @@ jobs: - name: Reset database from empty (applies every migration) run: supabase db reset + - name: Verify Phase 0-to-Phase 1A migration compatibility + run: ./supabase/scripts/migration-compatibility-test.sh + + - name: Reset database from empty for test suite + run: supabase db reset + - name: Run pgTAP database/RLS tests run: supabase test db diff --git a/README.md b/README.md index 1c22617..1119734 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,9 @@ cd app && npm ci && npm run lint && npm run typecheck && npm run test && npm run # Database: pgTAP RLS tests, run against the real local Postgres instance supabase test db +# Migration compatibility: Phase 0 schema/data -> Phase 1A migration +./supabase/scripts/migration-compatibility-test.sh + # API-path integration test: real Auth -> JWT -> PostgREST -> RLS -> Postgres, # using temporary users (each with their own randomly generated per-run # password) that are always cleaned up afterward diff --git a/app/package-lock.json b/app/package-lock.json index bef495c..76b47a8 100644 --- a/app/package-lock.json +++ b/app/package-lock.json @@ -11,7 +11,7 @@ "@supabase/supabase-js": "^2.110.8", "react": "^19.2.7", "react-dom": "^19.2.7", - "react-router": "^8.3.0" + "react-router": "8.3.0" }, "devDependencies": { "@eslint/js": "^10.0.1", @@ -3336,6 +3336,7 @@ "version": "8.3.0", "resolved": "https://registry.npmjs.org/react-router/-/react-router-8.3.0.tgz", "integrity": "sha512-qyPMvW83jGIct3yiieisxdk9M745anqhpIMKN5m1t6yBMfgVPpt77aHOqs5fUlEJRMCGffg9BaQLH9oPVOL7xQ==", + "license": "MIT", "dependencies": { "cookie-es": "^3.1.1" }, diff --git a/app/package.json b/app/package.json index ae44711..2b7e0a4 100644 --- a/app/package.json +++ b/app/package.json @@ -21,7 +21,7 @@ "@supabase/supabase-js": "^2.110.8", "react": "^19.2.7", "react-dom": "^19.2.7", - "react-router": "^8.3.0" + "react-router": "8.3.0" }, "devDependencies": { "@eslint/js": "^10.0.1", diff --git a/docs/DATA_MODEL.md b/docs/DATA_MODEL.md index 9d7c9ab..5d24847 100644 --- a/docs/DATA_MODEL.md +++ b/docs/DATA_MODEL.md @@ -97,6 +97,10 @@ the ambiguous legacy degree-program text into `degree` or `field`. Existing prof `basic_profile` and `education` revisions at zero and no reviews. New profile creation initializes those rows transactionally. +`last_confirmed_at` is nullable for migrated profile and education rows: `NULL` means the legacy +row has not yet been manually confirmed in the current profile model. New manual creation defaults +to the current timestamp, and later manual content edits refresh it. + **Ownership rule (binding, see [AGENTS.md](../AGENTS.md))**: every user-owned table has direct `user_id`, including association tables. The root `profiles.user_id` references `auth.users(id)`; child user-owned rows generally reference `profiles(user_id)` through that same direct `user_id`. diff --git a/docs/TESTING_STRATEGY.md b/docs/TESTING_STRATEGY.md index 4a30397..1b903b9 100644 --- a/docs/TESTING_STRATEGY.md +++ b/docs/TESTING_STRATEGY.md @@ -79,6 +79,12 @@ primary-selection idempotence, and cascade safety. The real Auth/JWT/PostgREST s column restrictions, education CRUD, both RPCs, review staleness, and cross-user denial; later Phase 1A domains remain future work. +The CI database job also runs `supabase/scripts/migration-compatibility-test.sh`. It uses the +installed CLI's `supabase db reset --version` and `supabase migration up --local` commands to +reset through Phase 0, insert synthetic legacy records, apply the Phase 1A migration, and verify +preservation, safe degree-year backfill, neutral revisions, no reviews, migration provenance, and +nullable migrated confirmation timestamps. + - **Migration structure tests**: exact tables, direct `user_id` ownership, required foreign keys, ownership-safe composite parent references, primary-education partial unique constraint, typed-skill-evidence exactly-one-source check, bounded note, section-review/revision constraints, diff --git a/supabase/fixtures/migration/assert_phase1_migration.sql b/supabase/fixtures/migration/assert_phase1_migration.sql new file mode 100644 index 0000000..d9fe114 --- /dev/null +++ b/supabase/fixtures/migration/assert_phase1_migration.sql @@ -0,0 +1,14 @@ +do $$ +begin + if (select headline from public.profiles where user_id = '50000000-0000-0000-0000-000000000001') <> 'Preserved headline' then raise exception 'headline was not preserved'; end if; + if (select count(*) from public.education_entries where user_id = '50000000-0000-0000-0000-000000000001') <> 0 then raise exception 'migration invented education'; end if; + if (select degree_year from public.education_entries where id = '60000000-0000-0000-0000-000000000001') <> 3 then raise exception 'lone degree year was not safely copied'; end if; + if exists (select 1 from public.education_entries where user_id = '50000000-0000-0000-0000-000000000003' and degree_year is not null) then raise exception 'multiple education rows received a legacy degree year'; end if; + if exists (select 1 from public.education_entries where is_primary) then raise exception 'migration selected a primary'; end if; + if exists (select 1 from public.education_entries where education_status <> 'unknown') then raise exception 'migration inferred a status'; end if; + if exists (select 1 from public.profiles where created_via <> 'migration' or last_confirmed_at is not null) then raise exception 'migrated profiles lack expected provenance or confirmation state'; end if; + if exists (select 1 from public.education_entries where created_via <> 'migration' or last_confirmed_at is not null) then raise exception 'migrated education lacks expected provenance or confirmation state'; end if; + if exists (select 1 from public.profile_section_reviews) then raise exception 'migration created reviews'; end if; + if exists (select 1 from public.profile_section_revisions where content_revision <> 0) then raise exception 'migration revisions are not neutral'; end if; +end; +$$; diff --git a/supabase/fixtures/migration/phase0_fixture.sql b/supabase/fixtures/migration/phase0_fixture.sql new file mode 100644 index 0000000..e06b755 --- /dev/null +++ b/supabase/fixtures/migration/phase0_fixture.sql @@ -0,0 +1,21 @@ +-- Synthetic Phase 0 data applied after resetting through the Phase 0 schema. +do $$ +begin +insert into auth.users (id, aud, role, email) values + ('50000000-0000-0000-0000-000000000001', 'authenticated', 'authenticated', 'legacy-none@example.test'), + ('50000000-0000-0000-0000-000000000002', 'authenticated', 'authenticated', 'legacy-one@example.test'), + ('50000000-0000-0000-0000-000000000003', 'authenticated', 'authenticated', 'legacy-many@example.test'), + ('50000000-0000-0000-0000-000000000004', 'authenticated', 'authenticated', 'legacy-null@example.test'); + +insert into public.profiles (user_id, headline, degree_program, degree_year) values + ('50000000-0000-0000-0000-000000000001', 'Preserved headline', ' ', 2), + ('50000000-0000-0000-0000-000000000002', repeat('x', 200), 'Unmapped legacy program', 3), + ('50000000-0000-0000-0000-000000000003', 'Multiple education rows', 'Multiple program', 4), + ('50000000-0000-0000-0000-000000000004', null, null, null); + +insert into public.education_entries (id, user_id, institution, degree, field) values + ('60000000-0000-0000-0000-000000000001', '50000000-0000-0000-0000-000000000002', 'Existing Institution', 'Existing degree', 'Existing field'), + ('60000000-0000-0000-0000-000000000002', '50000000-0000-0000-0000-000000000003', 'First Institution', 'First degree', 'First field'), + ('60000000-0000-0000-0000-000000000003', '50000000-0000-0000-0000-000000000003', 'Second Institution', null, null); +end; +$$; diff --git a/supabase/migrations/20260801213000_profile_core_education.sql b/supabase/migrations/20260801213000_profile_core_education.sql index 461a96d..12468ed 100644 --- a/supabase/migrations/20260801213000_profile_core_education.sql +++ b/supabase/migrations/20260801213000_profile_core_education.sql @@ -12,7 +12,7 @@ alter table public.profiles add column preferred_name text, add column created_via text not null default 'manual', add column updated_at timestamptz not null default now(), - add column last_confirmed_at timestamptz not null default now(), + add column last_confirmed_at timestamptz default now(), add constraint profiles_created_via_check check (created_via in ('manual', 'migration')); alter table public.education_entries @@ -23,7 +23,7 @@ alter table public.education_entries add column is_primary boolean not null default false, add column created_via text not null default 'manual', add column updated_at timestamptz not null default now(), - add column last_confirmed_at timestamptz not null default now(), + add column last_confirmed_at timestamptz default now(), add constraint education_entries_degree_year_check check (degree_year is null or degree_year between 1 and 10), add constraint education_entries_graduation_month_check check (expected_graduation_month is null or expected_graduation_month between 1 and 12), add constraint education_entries_graduation_year_check check (expected_graduation_year is null or expected_graduation_year between 2000 and 2100), @@ -85,9 +85,9 @@ comment on column public.profiles.profile_complete is comment on column public.profiles.created_via is 'How this current-format profile row entered the system: manual or migration.'; comment on column public.profiles.last_confirmed_at is - 'Last explicit manual profile-content creation or edit; separate from section review.'; + 'Last explicit manual profile-content creation or edit; NULL means a migrated row has not yet been manually confirmed in the current profile model.'; comment on column public.education_entries.last_confirmed_at is - 'Last explicit manual education-content creation or edit; separate from section review.'; + 'Last explicit manual education-content creation or edit; NULL means a migrated row has not yet been manually confirmed in the current profile model.'; create unique index education_entries_one_primary_per_user_idx on public.education_entries (user_id) diff --git a/supabase/scripts/migration-compatibility-test.sh b/supabase/scripts/migration-compatibility-test.sh new file mode 100755 index 0000000..b7b11e8 --- /dev/null +++ b/supabase/scripts/migration-compatibility-test.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Exercise the actual supported CLI path: reset through Phase 0, insert +# synthetic Phase 0 data, then apply pending local migrations. +supabase db reset --local --version 20260726145144 --no-seed +supabase db query --local --file supabase/fixtures/migration/phase0_fixture.sql +supabase migration up --local +supabase db query --local --file supabase/fixtures/migration/assert_phase1_migration.sql +echo 'Phase 0-to-Phase 1A migration compatibility test passed.' From 63c1604a013414f088c044491ce0c992da774574 Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sun, 2 Aug 2026 00:15:34 +0300 Subject: [PATCH 07/10] fix: align profile education workflow state Use the completeness evaluator as the overview source, distinguish missing review state, refresh visible review status, protect zero-row saves, validate education inputs, and gate edit forms on loaded records. --- app/src/lib/profileCompleteness.test.ts | 32 +++++++++++ app/src/lib/profileCompleteness.ts | 6 +- app/src/lib/profileReviewRepository.ts | 11 ++++ app/src/lib/profileValidation.ts | 30 ++++++++++ app/src/pages/BasicProfilePage.tsx | 28 ++++++++- app/src/pages/EducationEditorPage.tsx | 35 +++++++++++- app/src/pages/EducationListPage.tsx | 19 ++++++- app/src/pages/ProfilePage.tsx | 75 ++++++++----------------- 8 files changed, 177 insertions(+), 59 deletions(-) diff --git a/app/src/lib/profileCompleteness.test.ts b/app/src/lib/profileCompleteness.test.ts index 822d783..fa16342 100644 --- a/app/src/lib/profileCompleteness.test.ts +++ b/app/src/lib/profileCompleteness.test.ts @@ -32,4 +32,36 @@ describe('education completeness slice', () => { checks.find((check) => check.id === 'education_review'), ).toMatchObject({ outcome: 'unconfirmed' }) }) + + it('never treats missing, null, or stale review state as current', () => { + expect( + evaluateEducationSlice([], []).find( + (check) => check.id === 'basic_profile_review', + )?.outcome, + ).toBe('unconfirmed') + expect( + evaluateEducationSlice( + [], + [ + { + section_key: 'basic_profile', + content_revision: 2, + reviewed_content_revision: null, + }, + ], + ).find((check) => check.id === 'basic_profile_review')?.outcome, + ).toBe('unconfirmed') + expect( + evaluateEducationSlice( + [], + [ + { + section_key: 'basic_profile', + content_revision: 2, + reviewed_content_revision: 1, + }, + ], + ).find((check) => check.id === 'basic_profile_review')?.outcome, + ).toBe('unconfirmed') + }) }) diff --git a/app/src/lib/profileCompleteness.ts b/app/src/lib/profileCompleteness.ts index 6727b01..da6895e 100644 --- a/app/src/lib/profileCompleteness.ts +++ b/app/src/lib/profileCompleteness.ts @@ -19,7 +19,11 @@ export interface CompletenessCheck { function reviewed(state: SectionState[], section: SectionState['section_key']) { const item = state.find((entry) => entry.section_key === section) - return item?.reviewed_content_revision === item?.content_revision + return Boolean( + item && + item.reviewed_content_revision !== null && + item.reviewed_content_revision === item.content_revision, + ) } export function evaluateEducationSlice( diff --git a/app/src/lib/profileReviewRepository.ts b/app/src/lib/profileReviewRepository.ts index 22526cc..a0a0cc4 100644 --- a/app/src/lib/profileReviewRepository.ts +++ b/app/src/lib/profileReviewRepository.ts @@ -1,6 +1,17 @@ import { supabase } from './supabaseClient' import type { SectionState } from './profileTypes' +export function reviewStatus( + state: SectionState[] | null, + section: SectionState['section_key'], +): 'current' | 'stale' | 'not_reviewed' { + const item = state?.find((entry) => entry.section_key === section) + if (!item || item.reviewed_content_revision === null) return 'not_reviewed' + return item.reviewed_content_revision === item.content_revision + ? 'current' + : 'stale' +} + export const profileReviewRepository = { async state(userId: string) { const [revisions, reviews] = await Promise.all([ diff --git a/app/src/lib/profileValidation.ts b/app/src/lib/profileValidation.ts index e3d2732..d6b26f9 100644 --- a/app/src/lib/profileValidation.ts +++ b/app/src/lib/profileValidation.ts @@ -51,10 +51,32 @@ export function validateEducation(values: Record): FieldErrors { errors.degree = 'Degree must be 160 characters or fewer.' if (trimmed(values.field).length > 160) errors.field = 'Field must be 160 characters or fewer.' + const degreeYear = Number(values.degree_year) + if ( + values.degree_year && + (!Number.isInteger(degreeYear) || degreeYear < 1 || degreeYear > 10) + ) + errors.degree_year = 'Degree year must be between 1 and 10.' const month = values.expected_graduation_month const year = values.expected_graduation_year if ((month === '') !== (year === '')) errors.graduation = 'Enter both graduation month and year, or neither.' + if ( + month && + (!Number.isInteger(Number(month)) || + Number(month) < 1 || + Number(month) > 12) + ) + errors.expected_graduation_month = + 'Graduation month must be between 1 and 12.' + if ( + year && + (!Number.isInteger(Number(year)) || + Number(year) < 2000 || + Number(year) > 2100) + ) + errors.expected_graduation_year = + 'Graduation year must be between 2000 and 2100.' if (values.education_status !== 'current' && (month || year)) errors.graduation = 'Expected graduation is only for current education.' if (values.education_status === 'current' && values.end_date) @@ -67,5 +89,13 @@ export function validateEducation(values: Record): FieldErrors { values.end_date < values.start_date ) errors.end_date = 'End date cannot precede start date.' + if ( + values.start_date && + month && + year && + `${year}-${month.padStart(2, '0')}-01` < + `${values.start_date.slice(0, 7)}-01` + ) + errors.graduation = 'Expected graduation cannot precede the start month.' return errors } diff --git a/app/src/pages/BasicProfilePage.tsx b/app/src/pages/BasicProfilePage.tsx index 28b244a..8d735db 100644 --- a/app/src/pages/BasicProfilePage.tsx +++ b/app/src/pages/BasicProfilePage.tsx @@ -1,7 +1,10 @@ import { useCallback, useEffect, useState, type FormEvent } from 'react' import { useAuth } from '../contexts/AuthContext' import { profileRepository } from '../lib/profileRepository' -import { profileReviewRepository } from '../lib/profileReviewRepository' +import { + profileReviewRepository, + reviewStatus, +} from '../lib/profileReviewRepository' import { normalizeProfile, validateProfile } from '../lib/profileValidation' export function BasicProfilePage() { @@ -12,6 +15,9 @@ export function BasicProfilePage() { const [legacy, setLegacy] = useState(null) const [message, setMessage] = useState(null) const [errors, setErrors] = useState>({}) + const [sectionState, setSectionState] = useState< + 'current' | 'stale' | 'not_reviewed' + >('not_reviewed') const load = useCallback(async () => { if (!userId) return const { data } = await profileRepository.get(userId) @@ -20,6 +26,9 @@ export function BasicProfilePage() { setHeadline(data.headline ?? '') setLegacy(data.degree_program) } + const sections = await profileReviewRepository.state(userId) + if (!sections.error) + setSectionState(reviewStatus(sections.data, 'basic_profile')) }, [userId]) useEffect(() => { // eslint-disable-next-line react-hooks/set-state-in-effect -- asynchronous initial load updates state after the repository resolves @@ -34,17 +43,32 @@ export function BasicProfilePage() { userId, normalizeProfile(name, headline), ) - setMessage(result.error ? 'Profile could not be saved.' : 'Saved.') + if (result.error) return setMessage('Profile could not be saved.') + if (!result.data?.length) + return setMessage( + 'Your profile no longer exists. Return to the overview and create it again.', + ) + setMessage('Saved.') + await load() } async function review() { const result = await profileReviewRepository.review('basic_profile') setMessage( result.error ? 'Review could not be saved.' : 'Basic profile reviewed.', ) + await load() } return (

Basic profile

+

+ Review status:{' '} + {sectionState === 'current' + ? 'Reviewed and current' + : sectionState === 'stale' + ? 'Stale — review again' + : 'Not reviewed'} +

{legacy && (

Previous degree-program information is available for reference. diff --git a/app/src/pages/EducationEditorPage.tsx b/app/src/pages/EducationEditorPage.tsx index 4e29b48..523f1d1 100644 --- a/app/src/pages/EducationEditorPage.tsx +++ b/app/src/pages/EducationEditorPage.tsx @@ -22,10 +22,23 @@ export function EducationEditorPage() { const [values, setValues] = useState>(empty) const [errors, setErrors] = useState>({}) const [message, setMessage] = useState(null) + const [loadState, setLoadState] = useState< + 'loading' | 'loaded' | 'missing' | 'error' + >(educationId ? 'loading' : 'loaded') const load = useCallback(async () => { if (!educationId) return const result = await educationRepository.get(educationId) - if (result.error || !result.data) { + if (result.error) { + setLoadState(result.error.code === 'PGRST116' ? 'missing' : 'error') + setMessage( + result.error.code === 'PGRST116' + ? 'This education entry no longer exists.' + : 'This education entry could not be loaded.', + ) + return + } + if (!result.data) { + setLoadState('missing') setMessage('This education entry no longer exists.') return } @@ -43,6 +56,7 @@ export function EducationEditorPage() { start_date: entry.start_date ?? '', end_date: entry.end_date ?? '', }) + setLoadState('loaded') }, [educationId]) useEffect(() => { void load() @@ -54,7 +68,7 @@ export function EducationEditorPage() { event.preventDefault() const next = validateEducation(values) setErrors(next) - if (Object.keys(next).length) return + if (Object.keys(next).length || loadState !== 'loaded') return const input = normalizeEducation(values) const result = educationId ? await educationRepository.update(educationId, input) @@ -84,6 +98,23 @@ export function EducationEditorPage() { )} ) + if (loadState === 'loading') + return ( +

+

Edit education

+

Loading…

+
+ ) + if (loadState === 'missing' || loadState === 'error') + return ( +
+

Edit education

+

{message}

+

+ Return to education +

+
+ ) return (

{educationId ? 'Edit education' : 'Add education'}

diff --git a/app/src/pages/EducationListPage.tsx b/app/src/pages/EducationListPage.tsx index 66cb37e..21a11a1 100644 --- a/app/src/pages/EducationListPage.tsx +++ b/app/src/pages/EducationListPage.tsx @@ -2,7 +2,10 @@ import { Link } from 'react-router' import { useCallback, useEffect, useState } from 'react' import { useAuth } from '../contexts/AuthContext' import { educationRepository } from '../lib/educationRepository' -import { profileReviewRepository } from '../lib/profileReviewRepository' +import { + profileReviewRepository, + reviewStatus, +} from '../lib/profileReviewRepository' import type { EducationEntry } from '../lib/profileTypes' export function EducationListPage() { @@ -12,12 +15,18 @@ export function EducationListPage() { const [status, setStatus] = useState<'loading' | 'ready' | 'error'>('loading') const [deleting, setDeleting] = useState(null) const [message, setMessage] = useState(null) + const [sectionState, setSectionState] = useState< + 'current' | 'stale' | 'not_reviewed' + >('not_reviewed') const load = useCallback(async () => { if (!userId) return setStatus('loading') const result = await educationRepository.list(userId) if (result.error) return setStatus('error') setEntries(result.data) + const sections = await profileReviewRepository.state(userId) + if (!sections.error) + setSectionState(reviewStatus(sections.data, 'education')) setStatus('ready') }, [userId]) useEffect(() => { @@ -69,6 +78,14 @@ export function EducationListPage() { return (

Education

+

+ Review status:{' '} + {sectionState === 'current' + ? 'Reviewed and current' + : sectionState === 'stale' + ? 'Stale — review again' + : 'Not reviewed'} +

Add education

diff --git a/app/src/pages/ProfilePage.tsx b/app/src/pages/ProfilePage.tsx index 926442b..226ed32 100644 --- a/app/src/pages/ProfilePage.tsx +++ b/app/src/pages/ProfilePage.tsx @@ -4,15 +4,8 @@ import { useAuth } from '../contexts/AuthContext' import { educationRepository } from '../lib/educationRepository' import { profileRepository } from '../lib/profileRepository' import { profileReviewRepository } from '../lib/profileReviewRepository' -import type { SectionState } from '../lib/profileTypes' - -function reviewed( - state: SectionState[] | null, - section: SectionState['section_key'], -) { - const item = state?.find((entry) => entry.section_key === section) - return item?.reviewed_content_revision === item?.content_revision -} +import type { EducationEntry, SectionState } from '../lib/profileTypes' +import { evaluateEducationSlice } from '../lib/profileCompleteness' export function ProfilePage() { const { session } = useAuth() @@ -21,9 +14,8 @@ export function ProfilePage() { 'loading', ) const [state, setState] = useState(null) - const [hasPrimary, setHasPrimary] = useState(false) - const [degreeYear, setDegreeYear] = useState(false) - const [graduation, setGraduation] = useState(false) + const [entries, setEntries] = useState([]) + const [createError, setCreateError] = useState(null) const load = useCallback(async () => { if (!userId) return @@ -36,14 +28,7 @@ export function ProfilePage() { if (profile.error || education.error || sections.error) return setStatus('error') if (!profile.data) return setStatus('empty') - const primary = education.data.find((entry) => entry.is_primary) - setHasPrimary(Boolean(primary)) - setDegreeYear(Boolean(primary?.degree_year)) - setGraduation( - Boolean( - primary?.expected_graduation_month && primary?.expected_graduation_year, - ), - ) + setEntries(education.data) setState(sections.data) setStatus('ready') }, [userId]) @@ -74,55 +59,39 @@ export function ProfilePage() {

Your profile

No profile yet.

+ {createError &&

{createError}

}
) - const checks = [ - [ - 'Basic profile review', - reviewed(state, 'basic_profile'), - '/profile/basic', - 'Review basic profile', - ], - [ - 'Primary education', - hasPrimary, - '/profile/education', - 'Add or select primary education', - ], - ['Degree year', degreeYear, '/profile/education', 'Add degree year'], - [ - 'Expected graduation', - graduation, - '/profile/education', - 'Add expected graduation timing', - ], - [ - 'Education review', - reviewed(state, 'education'), - '/profile/education', - 'Review education', - ], - ] + const checks = evaluateEducationSlice(entries, state ?? []) return (

Your profile

Complete the implemented profile details below.

    - {checks.map(([label, ok, href, action]) => ( -
  • - {label as string}:{' '} - {ok ? ( + {checks.map((check) => ( +
  • + {check.id.replaceAll('_', ' ')}:{' '} + {check.outcome === 'present' ? ( 'Present' ) : ( - {action as string} + {check.action} )}
    • ))} From 499556b7eb89d804bfae9fe0cf72dae8924a9fef Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sun, 2 Aug 2026 00:17:22 +0300 Subject: [PATCH 08/10] test: close profile education coverage gaps Add browser root-profile deletion and direct review-write integration coverage, and reconcile the documented API security checks. --- docs/TESTING_STRATEGY.md | 4 ++++ supabase/scripts/api-integration-test.mjs | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/docs/TESTING_STRATEGY.md b/docs/TESTING_STRATEGY.md index 1b903b9..45774ca 100644 --- a/docs/TESTING_STRATEGY.md +++ b/docs/TESTING_STRATEGY.md @@ -79,6 +79,10 @@ primary-selection idempotence, and cascade safety. The real Auth/JWT/PostgREST s column restrictions, education CRUD, both RPCs, review staleness, and cross-user denial; later Phase 1A domains remain future work. +The corrective API coverage also explicitly verifies that neither an owner nor another browser +user can delete a root profile, and that browser clients cannot directly insert revision or review +metadata. + The CI database job also runs `supabase/scripts/migration-compatibility-test.sh`. It uses the installed CLI's `supabase db reset --version` and `supabase migration up --local` commands to reset through Phase 0, insert synthetic legacy records, apply the Phase 1A migration, and verify diff --git a/supabase/scripts/api-integration-test.mjs b/supabase/scripts/api-integration-test.mjs index fae1ac5..4820254 100644 --- a/supabase/scripts/api-integration-test.mjs +++ b/supabase/scripts/api-integration-test.mjs @@ -185,6 +185,11 @@ async function main() { }); check('user A cannot forge profile provenance through the REST API', !forgeProfileMetadata.ok && forgeProfileMetadata.status === 403); + const deleteOwnProfile = await rest(tokenA, `/profiles?user_id=eq.${userA.id}`, { + method: 'DELETE', prefer: 'return=representation', + }); + check('user A cannot delete their own root profile through the browser role', !deleteOwnProfile.ok && deleteOwnProfile.status === 403); + // ---- education CRUD, primary RPC, and review freshness ----------------- const createEducationA = await rest(tokenA, '/education_entries', { method: 'POST', @@ -263,6 +268,11 @@ async function main() { }); check('user A cannot directly forge section revisions', !directRevisionWrite.ok && directRevisionWrite.status === 403); + const directReviewWrite = await rest(tokenA, '/profile_section_reviews', { + method: 'POST', body: { user_id: userA.id, section_key: 'education', reviewed_content_revision: 999 }, + }); + check('user A cannot directly forge section reviews', !directReviewWrite.ok && directReviewWrite.status === 403); + // ---- isolation: B cannot access A's data ------------------------------ const readAAsB = await rest(tokenB, `/profiles?user_id=eq.${userA.id}`, {}); check('user B cannot read user A\'s profile', readAAsB.ok && readAAsB.data.length === 0); From 4b4130199b349020be397111bec83175cf376781 Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sun, 2 Aug 2026 16:37:38 +0300 Subject: [PATCH 09/10] fix: harden profile education page states Add explicit basic-profile loading, ready, missing, and error states; refresh education review state; guard duplicate mutations; and cover observable page workflows. --- app/src/pages/BasicProfilePage.test.tsx | 162 ++++++++++++++++++ app/src/pages/BasicProfilePage.tsx | 105 +++++++++--- app/src/pages/EducationEditorPage.test.tsx | 111 +++++++++++++ app/src/pages/EducationEditorPage.tsx | 33 +++- app/src/pages/EducationListPage.test.tsx | 183 +++++++++++++++++++++ app/src/pages/EducationListPage.tsx | 59 +++++-- 6 files changed, 611 insertions(+), 42 deletions(-) create mode 100644 app/src/pages/BasicProfilePage.test.tsx create mode 100644 app/src/pages/EducationEditorPage.test.tsx create mode 100644 app/src/pages/EducationListPage.test.tsx diff --git a/app/src/pages/BasicProfilePage.test.tsx b/app/src/pages/BasicProfilePage.test.tsx new file mode 100644 index 0000000..c861cd2 --- /dev/null +++ b/app/src/pages/BasicProfilePage.test.tsx @@ -0,0 +1,162 @@ +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter } from 'react-router' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { BasicProfilePage } from './BasicProfilePage' + +const getProfile = vi.fn() +const updateProfile = vi.fn() +const sectionState = vi.fn() +const review = vi.fn() + +vi.mock('../contexts/AuthContext', () => ({ + useAuth: () => ({ session: { user: { id: 'user-1' } } }), +})) +vi.mock('../lib/profileRepository', () => ({ + profileRepository: { + get: (...args: unknown[]) => getProfile(...args), + update: (...args: unknown[]) => updateProfile(...args), + }, +})) +vi.mock('../lib/profileReviewRepository', () => ({ + profileReviewRepository: { + state: (...args: unknown[]) => sectionState(...args), + review: (...args: unknown[]) => review(...args), + }, + reviewStatus: ( + state: Array<{ + reviewed_content_revision: number | null + content_revision: number + }>, + ) => + state[0]?.reviewed_content_revision === null + ? 'not_reviewed' + : state[0]?.reviewed_content_revision === state[0]?.content_revision + ? 'current' + : 'stale', +})) + +const profile = { + user_id: 'user-1', + preferred_name: 'Ada', + headline: 'Engineer', + degree_program: null, +} +const current = { + data: [ + { + section_key: 'basic_profile', + content_revision: 1, + reviewed_content_revision: 1, + }, + ], + error: null, +} + +function renderPage() { + return render( + + + , + ) +} + +afterEach(() => vi.resetAllMocks()) + +describe('BasicProfilePage', () => { + it('hides the editable form while the profile is loading', () => { + getProfile.mockReturnValue(new Promise(() => {})) + renderPage() + expect(screen.getByText('Loading…')).toBeInTheDocument() + expect(screen.queryByRole('textbox')).not.toBeInTheDocument() + }) + + it('populates the existing profile before enabling the form', async () => { + getProfile.mockResolvedValue({ data: profile, error: null }) + sectionState.mockResolvedValue(current) + renderPage() + expect(await screen.findByDisplayValue('Ada')).toBeInTheDocument() + expect(screen.getByDisplayValue('Engineer')).toBeInTheDocument() + expect(screen.getByText(/Reviewed and current/)).toBeInTheDocument() + }) + + it('offers retry after a load failure instead of an empty form', async () => { + getProfile.mockResolvedValue({ + data: null, + error: { code: '', message: '' }, + }) + renderPage() + expect(await screen.findByRole('alert')).toHaveTextContent(/try again/i) + expect(screen.getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect(screen.queryByRole('textbox')).not.toBeInTheDocument() + }) + + it('routes missing profiles back to the overview without rendering a form', async () => { + getProfile.mockResolvedValue({ data: null, error: null }) + renderPage() + expect( + await screen.findByText(/root profile does not exist/i), + ).toBeInTheDocument() + expect( + screen.getByRole('link', { name: /return to profile overview/i }), + ).toHaveAttribute('href', '/profile') + expect(screen.queryByRole('textbox')).not.toBeInTheDocument() + }) + + it('does not report a zero-row save as success', async () => { + getProfile.mockResolvedValue({ data: profile, error: null }) + sectionState.mockResolvedValue(current) + updateProfile.mockResolvedValue({ data: [], error: null }) + const user = userEvent.setup() + renderPage() + await screen.findByDisplayValue('Ada') + await user.click(screen.getByRole('button', { name: 'Save' })) + expect( + await screen.findByText(/profile no longer exists/i), + ).toBeInTheDocument() + expect(screen.queryByText('Saved.')).not.toBeInTheDocument() + }) + + it('refreshes review state after a successful review', async () => { + getProfile.mockResolvedValue({ data: profile, error: null }) + sectionState + .mockResolvedValueOnce({ + data: [ + { + section_key: 'basic_profile', + content_revision: 1, + reviewed_content_revision: null, + }, + ], + error: null, + }) + .mockResolvedValueOnce(current) + review.mockResolvedValue({ data: null, error: null }) + const user = userEvent.setup() + renderPage() + expect(await screen.findByText(/Not reviewed/)).toBeInTheDocument() + await user.click( + screen.getByRole('button', { name: /mark basic profile reviewed/i }), + ) + expect(await screen.findByText(/Reviewed and current/)).toBeInTheDocument() + }) + + it('keeps the prior status visible after a failed review', async () => { + getProfile.mockResolvedValue({ data: profile, error: null }) + sectionState.mockResolvedValue(current) + review.mockResolvedValue({ + data: null, + error: { code: '42501', message: '' }, + }) + const user = userEvent.setup() + renderPage() + expect(await screen.findByText(/Reviewed and current/)).toBeInTheDocument() + await user.click( + screen.getByRole('button', { name: /mark basic profile reviewed/i }), + ) + expect( + await screen.findByText(/do not have permission/i), + ).toBeInTheDocument() + expect(screen.getByText(/Reviewed and current/)).toBeInTheDocument() + }) +}) diff --git a/app/src/pages/BasicProfilePage.tsx b/app/src/pages/BasicProfilePage.tsx index 8d735db..d2d3fc9 100644 --- a/app/src/pages/BasicProfilePage.tsx +++ b/app/src/pages/BasicProfilePage.tsx @@ -1,3 +1,4 @@ +import { Link } from 'react-router' import { useCallback, useEffect, useState, type FormEvent } from 'react' import { useAuth } from '../contexts/AuthContext' import { profileRepository } from '../lib/profileRepository' @@ -6,6 +7,7 @@ import { reviewStatus, } from '../lib/profileReviewRepository' import { normalizeProfile, validateProfile } from '../lib/profileValidation' +import { errorMessage, safeError } from '../lib/profileTypes' export function BasicProfilePage() { const { session } = useAuth() @@ -15,20 +17,39 @@ export function BasicProfilePage() { const [legacy, setLegacy] = useState(null) const [message, setMessage] = useState(null) const [errors, setErrors] = useState>({}) + const [pageState, setPageState] = useState< + 'loading' | 'ready' | 'missing' | 'error' + >('loading') + const [saving, setSaving] = useState(false) + const [reviewing, setReviewing] = useState(false) const [sectionState, setSectionState] = useState< 'current' | 'stale' | 'not_reviewed' >('not_reviewed') const load = useCallback(async () => { if (!userId) return - const { data } = await profileRepository.get(userId) - if (data) { - setName(data.preferred_name ?? '') - setHeadline(data.headline ?? '') - setLegacy(data.degree_program) + setPageState('loading') + const profile = await profileRepository.get(userId) + if (profile.error) { + setMessage(errorMessage(safeError(profile.error))) + setPageState('error') + return } + if (!profile.data) { + setMessage('Your root profile does not exist yet.') + setPageState('missing') + return + } + setName(profile.data.preferred_name ?? '') + setHeadline(profile.data.headline ?? '') + setLegacy(profile.data.degree_program) const sections = await profileReviewRepository.state(userId) - if (!sections.error) - setSectionState(reviewStatus(sections.data, 'basic_profile')) + if (sections.error) { + setMessage(errorMessage(safeError(sections.error))) + setPageState('error') + return + } + setSectionState(reviewStatus(sections.data, 'basic_profile')) + setPageState('ready') }, [userId]) useEffect(() => { // eslint-disable-next-line react-hooks/set-state-in-effect -- asynchronous initial load updates state after the repository resolves @@ -38,26 +59,65 @@ export function BasicProfilePage() { event.preventDefault() const next = validateProfile(name, headline) setErrors(next) - if (Object.keys(next).length || !userId) return + if (Object.keys(next).length || !userId || saving) return + setSaving(true) const result = await profileRepository.update( userId, normalizeProfile(name, headline), ) - if (result.error) return setMessage('Profile could not be saved.') - if (!result.data?.length) - return setMessage( + if (result.error) { + setMessage(errorMessage(safeError(result.error))) + setSaving(false) + return + } + if (!result.data?.length) { + setMessage( 'Your profile no longer exists. Return to the overview and create it again.', ) - setMessage('Saved.') + setPageState('missing') + setSaving(false) + return + } + setMessage('Saved. Review status was refreshed.') await load() + setSaving(false) } async function review() { + if (reviewing) return + setReviewing(true) const result = await profileReviewRepository.review('basic_profile') - setMessage( - result.error ? 'Review could not be saved.' : 'Basic profile reviewed.', - ) - await load() + if (result.error) setMessage(errorMessage(safeError(result.error))) + else { + setMessage('Basic profile reviewed. Review status was refreshed.') + await load() + } + setReviewing(false) } + if (pageState === 'loading') + return ( +
    +

    Basic profile

    +

    Loading…

    +
    + ) + if (pageState === 'missing') + return ( +
    +

    Basic profile

    +

    {message}

    + Return to profile overview +
    + ) + if (pageState === 'error') + return ( +
    +

    Basic profile

    +

    {message ?? 'Profile could not be loaded.'}

    + +
    + ) return (

    Basic profile

    @@ -111,10 +171,17 @@ export function BasicProfilePage() {

    )} - + - {message &&

    {message}

    }
    diff --git a/app/src/pages/EducationEditorPage.test.tsx b/app/src/pages/EducationEditorPage.test.tsx new file mode 100644 index 0000000..80d3e3f --- /dev/null +++ b/app/src/pages/EducationEditorPage.test.tsx @@ -0,0 +1,111 @@ +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter, Route, Routes } from 'react-router' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { EducationEditorPage } from './EducationEditorPage' + +const get = vi.fn() +const update = vi.fn() +const create = vi.fn() + +vi.mock('../contexts/AuthContext', () => ({ + useAuth: () => ({ session: { user: { id: 'user-1' } } }), +})) +vi.mock('../lib/educationRepository', () => ({ + educationRepository: { + get: (...args: unknown[]) => get(...args), + update: (...args: unknown[]) => update(...args), + create: (...args: unknown[]) => create(...args), + }, +})) + +const entry = { + id: 'edu-1', + user_id: 'user-1', + institution: 'Example University', + degree: 'BSc', + field: 'Computing', + degree_year: 3, + expected_graduation_month: 6, + expected_graduation_year: 2028, + education_status: 'current', + is_primary: false, + start_date: '2024-09-01', + end_date: null, +} + +function renderEdit() { + return render( + + + } + /> + Education list

    } /> +     +     , + ) +} + +afterEach(() => vi.resetAllMocks()) + +describe('EducationEditorPage', () => { + it('shows loading rather than an empty editable form', () => { + get.mockReturnValue(new Promise(() => {})) + renderEdit() + expect(screen.getByText('Loading…')).toBeInTheDocument() + expect(screen.queryByRole('textbox')).not.toBeInTheDocument() + }) + + it('populates an existing entry once it loads', async () => { + get.mockResolvedValue({ data: entry, error: null }) + renderEdit() + expect( + await screen.findByDisplayValue('Example University'), + ).toBeInTheDocument() + expect(screen.getByDisplayValue('BSc')).toBeInTheDocument() + }) + + it('does not render a submit-capable form for a missing entry', async () => { + get.mockResolvedValue({ data: null, error: null }) + renderEdit() + expect(await screen.findByText(/no longer exists/i)).toBeInTheDocument() + expect( + screen.queryByRole('button', { name: /save education/i }), + ).not.toBeInTheDocument() + }) + + it('offers a retry for a safe load failure', async () => { + get.mockResolvedValue({ data: null, error: { code: '', message: '' } }) + renderEdit() + expect(await screen.findByRole('alert')).toHaveTextContent(/try again/i) + expect(screen.getByRole('button', { name: 'Retry' })).toBeInTheDocument() + }) + + it('keeps entered values after a recoverable save failure', async () => { + get.mockResolvedValue({ data: entry, error: null }) + update.mockResolvedValue({ data: null, error: { code: '', message: '' } }) + const user = userEvent.setup() + renderEdit() + const institution = await screen.findByLabelText('Institution') + await user.clear(institution) + await user.type(institution, 'Changed University') + await user.click(screen.getByRole('button', { name: 'Save education' })) + expect(await screen.findByRole('alert')).toHaveTextContent(/try again/i) + expect(screen.getByDisplayValue('Changed University')).toBeInTheDocument() + }) + + it('treats a zero-row update as a missing entry', async () => { + get.mockResolvedValue({ data: entry, error: null }) + update.mockResolvedValue({ data: null, error: null }) + const user = userEvent.setup() + renderEdit() + await screen.findByDisplayValue('Example University') + await user.click(screen.getByRole('button', { name: 'Save education' })) + expect(await screen.findByText(/no longer exists/i)).toBeInTheDocument() + expect( + screen.queryByRole('button', { name: /save education/i }), + ).not.toBeInTheDocument() + }) +}) diff --git a/app/src/pages/EducationEditorPage.tsx b/app/src/pages/EducationEditorPage.tsx index 523f1d1..b84d2c1 100644 --- a/app/src/pages/EducationEditorPage.tsx +++ b/app/src/pages/EducationEditorPage.tsx @@ -3,6 +3,7 @@ import { useCallback, useEffect, useState, type FormEvent } from 'react' import { useAuth } from '../contexts/AuthContext' import { educationRepository } from '../lib/educationRepository' import { normalizeEducation, validateEducation } from '../lib/profileValidation' +import { errorMessage, safeError } from '../lib/profileTypes' const empty = { institution: '', @@ -22,6 +23,7 @@ export function EducationEditorPage() { const [values, setValues] = useState>(empty) const [errors, setErrors] = useState>({}) const [message, setMessage] = useState(null) + const [saving, setSaving] = useState(false) const [loadState, setLoadState] = useState< 'loading' | 'loaded' | 'missing' | 'error' >(educationId ? 'loading' : 'loaded') @@ -29,11 +31,11 @@ export function EducationEditorPage() { if (!educationId) return const result = await educationRepository.get(educationId) if (result.error) { - setLoadState(result.error.code === 'PGRST116' ? 'missing' : 'error') + setLoadState(safeError(result.error) === 'missing' ? 'missing' : 'error') setMessage( - result.error.code === 'PGRST116' + safeError(result.error) === 'missing' ? 'This education entry no longer exists.' - : 'This education entry could not be loaded.', + : errorMessage(safeError(result.error)), ) return } @@ -68,15 +70,21 @@ export function EducationEditorPage() { event.preventDefault() const next = validateEducation(values) setErrors(next) - if (Object.keys(next).length || loadState !== 'loaded') return + if (Object.keys(next).length || loadState !== 'loaded' || saving) return + setSaving(true) const input = normalizeEducation(values) const result = educationId ? await educationRepository.update(educationId, input) : await educationRepository.create(session!.user.id, input) - if (result.error || !result.data) { - setMessage( - 'Education could not be saved. Check the values and try again.', - ) + if (result.error) { + setMessage(errorMessage(safeError(result.error))) + setSaving(false) + return + } + if (!result.data) { + setMessage('This education entry no longer exists. Return to education.') + setLoadState('missing') + setSaving(false) return } navigate('/profile/education', { replace: true }) @@ -113,6 +121,11 @@ export function EducationEditorPage() {

    Return to education

    + {loadState === 'error' && ( + + )}
) return ( @@ -160,7 +173,9 @@ export function EducationEditorPage() { )} {errors.graduation &&

{errors.graduation}

} - + {message &&

{message}

}

diff --git a/app/src/pages/EducationListPage.test.tsx b/app/src/pages/EducationListPage.test.tsx new file mode 100644 index 0000000..195a2fc --- /dev/null +++ b/app/src/pages/EducationListPage.test.tsx @@ -0,0 +1,183 @@ +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter } from 'react-router' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { EducationListPage } from './EducationListPage' + +const list = vi.fn() +const remove = vi.fn() +const setPrimary = vi.fn() +const sectionState = vi.fn() +const review = vi.fn() + +vi.mock('../contexts/AuthContext', () => ({ + useAuth: () => ({ session: { user: { id: 'user-1' } } }), +})) +vi.mock('../lib/educationRepository', () => ({ + educationRepository: { + list: (...args: unknown[]) => list(...args), + remove: (...args: unknown[]) => remove(...args), + setPrimary: (...args: unknown[]) => setPrimary(...args), + }, +})) +vi.mock('../lib/profileReviewRepository', () => ({ + profileReviewRepository: { + state: (...args: unknown[]) => sectionState(...args), + review: (...args: unknown[]) => review(...args), + }, + reviewStatus: ( + state: Array<{ + reviewed_content_revision: number | null + content_revision: number + }>, + ) => + state[0]?.reviewed_content_revision === null + ? 'not_reviewed' + : state[0]?.reviewed_content_revision === state[0]?.content_revision + ? 'current' + : 'stale', +})) + +const entry = { + id: 'edu-1', + user_id: 'user-1', + institution: 'Example University', + degree: null, + field: null, + degree_year: null, + expected_graduation_month: null, + expected_graduation_year: null, + education_status: 'current' as const, + is_primary: false, + start_date: null, + end_date: null, +} +const notReviewed = { + data: [ + { + section_key: 'education', + content_revision: 1, + reviewed_content_revision: null, + }, + ], + error: null, +} +const current = { + data: [ + { + section_key: 'education', + content_revision: 1, + reviewed_content_revision: 1, + }, + ], + error: null, +} + +function renderPage() { + return render( + + + , + ) +} +afterEach(() => vi.resetAllMocks()) + +describe('EducationListPage', () => { + it('shows the empty state', async () => { + list.mockResolvedValue({ data: [], error: null }) + sectionState.mockResolvedValue(notReviewed) + renderPage() + expect( + await screen.findByText(/No education entries yet/), + ).toBeInTheDocument() + }) + + it('shows entries and their primary marker', async () => { + list.mockResolvedValue({ + data: [{ ...entry, is_primary: true }], + error: null, + }) + sectionState.mockResolvedValue(current) + renderPage() + expect(await screen.findByText('Example University')).toBeInTheDocument() + expect(screen.getByText(/Primary/)).toBeInTheDocument() + }) + + it('refreshes visible state after a successful review', async () => { + list.mockResolvedValue({ data: [], error: null }) + sectionState + .mockResolvedValueOnce(notReviewed) + .mockResolvedValueOnce(current) + review.mockResolvedValue({ data: null, error: null }) + const user = userEvent.setup() + renderPage() + await screen.findByText(/Not reviewed/) + await user.click( + screen.getByRole('button', { name: /mark education reviewed/i }), + ) + expect(await screen.findByText(/Reviewed and current/)).toBeInTheDocument() + }) + + it('keeps review state after a failed review', async () => { + list.mockResolvedValue({ data: [], error: null }) + sectionState.mockResolvedValue(current) + review.mockResolvedValue({ + data: null, + error: { code: '42501', message: '' }, + }) + const user = userEvent.setup() + renderPage() + await screen.findByText(/Reviewed and current/) + await user.click( + screen.getByRole('button', { name: /mark education reviewed/i }), + ) + expect( + await screen.findByText(/do not have permission/i), + ).toBeInTheDocument() + expect(screen.getByText(/Reviewed and current/)).toBeInTheDocument() + }) + + it('confirms then reloads after a successful delete', async () => { + list + .mockResolvedValueOnce({ data: [entry], error: null }) + .mockResolvedValueOnce({ data: [], error: null }) + sectionState + .mockResolvedValueOnce(current) + .mockResolvedValueOnce({ + data: [ + { + section_key: 'education', + content_revision: 2, + reviewed_content_revision: 1, + }, + ], + error: null, + }) + remove.mockResolvedValue({ data: [{ id: 'edu-1' }], error: null }) + const user = userEvent.setup() + renderPage() + await screen.findByText('Example University') + await user.click(screen.getByRole('button', { name: 'Delete' })) + expect(screen.getByRole('dialog')).toBeInTheDocument() + await user.click(screen.getByRole('button', { name: 'Confirm delete' })) + expect( + await screen.findByText(/No education entries yet/), + ).toBeInTheDocument() + expect(screen.getByText(/Stale/)).toBeInTheDocument() + }) + + it('does not claim success for a zero-row delete', async () => { + list.mockResolvedValue({ data: [entry], error: null }) + sectionState.mockResolvedValue(current) + remove.mockResolvedValue({ data: [], error: null }) + const user = userEvent.setup() + renderPage() + await screen.findByText('Example University') + await user.click(screen.getByRole('button', { name: 'Delete' })) + await user.click(screen.getByRole('button', { name: 'Confirm delete' })) + expect(await screen.findByText(/may already be gone/i)).toBeInTheDocument() + expect( + screen.queryByText('Education entry deleted.'), + ).not.toBeInTheDocument() + }) +}) diff --git a/app/src/pages/EducationListPage.tsx b/app/src/pages/EducationListPage.tsx index 21a11a1..796b80a 100644 --- a/app/src/pages/EducationListPage.tsx +++ b/app/src/pages/EducationListPage.tsx @@ -7,6 +7,7 @@ import { reviewStatus, } from '../lib/profileReviewRepository' import type { EducationEntry } from '../lib/profileTypes' +import { errorMessage, safeError } from '../lib/profileTypes' export function EducationListPage() { const { session } = useAuth() @@ -14,6 +15,8 @@ export function EducationListPage() { const [entries, setEntries] = useState([]) const [status, setStatus] = useState<'loading' | 'ready' | 'error'>('loading') const [deleting, setDeleting] = useState(null) + const [pendingOperation, setPendingOperation] = useState(null) + const [reviewing, setReviewing] = useState(false) const [message, setMessage] = useState(null) const [sectionState, setSectionState] = useState< 'current' | 'stale' | 'not_reviewed' @@ -25,8 +28,8 @@ export function EducationListPage() { if (result.error) return setStatus('error') setEntries(result.data) const sections = await profileReviewRepository.state(userId) - if (!sections.error) - setSectionState(reviewStatus(sections.data, 'education')) + if (sections.error) return setStatus('error') + setSectionState(reviewStatus(sections.data, 'education')) setStatus('ready') }, [userId]) useEffect(() => { @@ -34,29 +37,42 @@ export function EducationListPage() { void load() }, [load]) async function primary(id: string) { + if (pendingOperation) return + setPendingOperation(`primary:${id}`) const result = await educationRepository.setPrimary(id) setMessage( result.error - ? 'Only a current education you own can be primary.' + ? errorMessage(safeError(result.error)) : 'Primary education updated.', ) - await load() + if (!result.error) await load() + setPendingOperation(null) } async function remove(id: string) { + if (pendingOperation) return + setPendingOperation(`delete:${id}`) const result = await educationRepository.remove(id) - if (result.error || !result.data?.length) + if (result.error) setMessage(errorMessage(safeError(result.error))) + else if (!result.data?.length) setMessage('This entry could not be deleted. It may already be gone.') - else setMessage('Education entry deleted.') + else { + setMessage('Education entry deleted.') + await load() + } setDeleting(null) - await load() + setPendingOperation(null) } async function review() { + if (reviewing) return + setReviewing(true) const result = await profileReviewRepository.review('education') setMessage( result.error - ? 'Education review could not be saved.' - : 'Education reviewed.', + ? errorMessage(safeError(result.error)) + : 'Education reviewed. Review status was refreshed.', ) + if (!result.error) await load() + setReviewing(false) } if (status === 'loading') return ( @@ -105,14 +121,18 @@ export function EducationListPage() { )} {message &&

{message}

}
From c190ecc52e5ef9c22ac430e084e73e2f7184eca8 Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Sun, 2 Aug 2026 16:44:30 +0300 Subject: [PATCH 10/10] test: verify profile education security behavior Cover privileged function hardening, primary and review behavior, API concurrency and isolation, and stronger legacy migration assertions. --- app/src/pages/EducationListPage.test.tsx | 22 +++-- docs/TESTING_STRATEGY.md | 22 +++-- .../migration/assert_phase1_migration.sql | 7 ++ supabase/scripts/api-integration-test.mjs | 59 ++++++++++++ .../040_privileged_function_security.test.sql | 90 +++++++++++++++++++ .../050_primary_review_behavior.test.sql | 47 ++++++++++ 6 files changed, 228 insertions(+), 19 deletions(-) create mode 100644 supabase/tests/database/040_privileged_function_security.test.sql create mode 100644 supabase/tests/database/050_primary_review_behavior.test.sql diff --git a/app/src/pages/EducationListPage.test.tsx b/app/src/pages/EducationListPage.test.tsx index 195a2fc..5c9b12c 100644 --- a/app/src/pages/EducationListPage.test.tsx +++ b/app/src/pages/EducationListPage.test.tsx @@ -141,18 +141,16 @@ describe('EducationListPage', () => { list .mockResolvedValueOnce({ data: [entry], error: null }) .mockResolvedValueOnce({ data: [], error: null }) - sectionState - .mockResolvedValueOnce(current) - .mockResolvedValueOnce({ - data: [ - { - section_key: 'education', - content_revision: 2, - reviewed_content_revision: 1, - }, - ], - error: null, - }) + sectionState.mockResolvedValueOnce(current).mockResolvedValueOnce({ + data: [ + { + section_key: 'education', + content_revision: 2, + reviewed_content_revision: 1, + }, + ], + error: null, + }) remove.mockResolvedValue({ data: [{ id: 'edu-1' }], error: null }) const user = userEvent.setup() renderPage() diff --git a/docs/TESTING_STRATEGY.md b/docs/TESTING_STRATEGY.md index 45774ca..7a65d32 100644 --- a/docs/TESTING_STRATEGY.md +++ b/docs/TESTING_STRATEGY.md @@ -72,12 +72,14 @@ each rule gets explicit positive and negative test cases. ## Phase 1A profile implementation tests (future) -**Implemented in the first profile-core/education slice**: pgTAP now verifies refined profile and +**Implemented in the first profile-core/education slice**: pgTAP verifies refined profile and education schema/grants, browser denial of root-profile deletion and direct revision/review/primary -writes, revision freshness after profile and education mutations, locked review RPC behavior, -primary-selection idempotence, and cascade safety. The real Auth/JWT/PostgREST suite covers -column restrictions, education CRUD, both RPCs, review staleness, and cross-user denial; later -Phase 1A domains remain future work. +writes, review freshness, primary switching/idempotence, and cascade safety. It also catalog-tests +all eight Phase 1A privileged functions for `SECURITY DEFINER`, an empty hardened search path, and +the exact callable surface: browser clients may execute only the constrained review and +primary-selection RPCs. The real Auth/JWT/PostgREST suite covers column restrictions, education +CRUD, primary switching, review staleness, cross-user denial, and concurrent mutation revision +advancement; later Phase 1A domains remain future work. The corrective API coverage also explicitly verifies that neither an owner nor another browser user can delete a root profile, and that browser clients cannot directly insert revision or review @@ -86,8 +88,14 @@ metadata. The CI database job also runs `supabase/scripts/migration-compatibility-test.sh`. It uses the installed CLI's `supabase db reset --version` and `supabase migration up --local` commands to reset through Phase 0, insert synthetic legacy records, apply the Phase 1A migration, and verify -preservation, safe degree-year backfill, neutral revisions, no reviews, migration provenance, and -nullable migrated confirmation timestamps. +row counts, long/blank legacy-text preservation, safe degree-year backfill, no inferred primary or +status, neutral revisions, no reviews, migration provenance, and nullable migrated confirmation +timestamps. + +The frontend component suite covers the user-observable loading, ready, missing, error, retry, +zero-row, review-refresh, and pending-operation states for the basic-profile and education pages. +Those pages never substitute a failed section-state request with “Not reviewed”; their visible +status always comes from the returned revision/review state. - **Migration structure tests**: exact tables, direct `user_id` ownership, required foreign keys, ownership-safe composite parent references, primary-education partial unique constraint, diff --git a/supabase/fixtures/migration/assert_phase1_migration.sql b/supabase/fixtures/migration/assert_phase1_migration.sql index d9fe114..8642df0 100644 --- a/supabase/fixtures/migration/assert_phase1_migration.sql +++ b/supabase/fixtures/migration/assert_phase1_migration.sql @@ -1,8 +1,15 @@ do $$ begin if (select headline from public.profiles where user_id = '50000000-0000-0000-0000-000000000001') <> 'Preserved headline' then raise exception 'headline was not preserved'; end if; + if (select headline from public.profiles where user_id = '50000000-0000-0000-0000-000000000002') <> repeat('x', 200) then raise exception 'overlength legacy headline was not preserved'; end if; + if (select degree_program from public.profiles where user_id = '50000000-0000-0000-0000-000000000001') <> ' ' then raise exception 'blank legacy degree-program text was normalized'; end if; + if (select count(*) from public.profiles) <> 4 then raise exception 'profile row count changed'; end if; + if (select count(*) from public.education_entries) <> 3 then raise exception 'education row count changed'; end if; if (select count(*) from public.education_entries where user_id = '50000000-0000-0000-0000-000000000001') <> 0 then raise exception 'migration invented education'; end if; if (select degree_year from public.education_entries where id = '60000000-0000-0000-0000-000000000001') <> 3 then raise exception 'lone degree year was not safely copied'; end if; + if (select degree_year from public.education_entries where id = '60000000-0000-0000-0000-000000000002') is not null then raise exception 'null legacy degree year changed'; end if; + if (select institution from public.education_entries where id = '60000000-0000-0000-0000-000000000003') <> 'Second Institution' then raise exception 'legacy institution changed'; end if; + if (select field from public.education_entries where id = '60000000-0000-0000-0000-000000000002') <> 'First field' then raise exception 'legacy field changed'; end if; if exists (select 1 from public.education_entries where user_id = '50000000-0000-0000-0000-000000000003' and degree_year is not null) then raise exception 'multiple education rows received a legacy degree year'; end if; if exists (select 1 from public.education_entries where is_primary) then raise exception 'migration selected a primary'; end if; if exists (select 1 from public.education_entries where education_status <> 'unknown') then raise exception 'migration inferred a status'; end if; diff --git a/supabase/scripts/api-integration-test.mjs b/supabase/scripts/api-integration-test.mjs index 4820254..9cbde0b 100644 --- a/supabase/scripts/api-integration-test.mjs +++ b/supabase/scripts/api-integration-test.mjs @@ -80,6 +80,14 @@ async function adminGetProfile(userId) { return res.json(); } +async function adminGetRows(table, userId) { + const res = await fetch(`${SUPABASE_URL}/rest/v1/${table}?user_id=eq.${userId}`, { + headers: { apikey: SECRET_KEY, Authorization: `Bearer ${SECRET_KEY}` }, + }); + if (!res.ok) throw new Error(`admin ${table} lookup failed: HTTP ${res.status}`); + return res.json(); +} + async function verifyUserFullyRemoved(id) { const problems = []; const userRes = await adminGetUser(id); @@ -90,6 +98,10 @@ async function verifyUserFullyRemoved(id) { if (profileRows.length !== 0) { problems.push(`profile row for ${id} still present after user deletion (expected cascade delete)`); } + for (const table of ['education_entries', 'profile_section_revisions', 'profile_section_reviews']) { + const rows = await adminGetRows(table, id); + if (rows.length !== 0) problems.push(`${table} rows for ${id} still present after user deletion (expected cascade delete)`); + } return problems; } @@ -227,6 +239,13 @@ async function main() { }); check('user A can review both implemented sections through constrained RPCs', reviewEducation.ok && reviewBasic.ok); + const mutateProfileAfterReview = await rest(tokenA, `/profiles?user_id=eq.${userA.id}`, { + method: 'PATCH', body: { headline: 'Changed after review' }, prefer: 'return=representation', + }); + const basicStateAfterMutation = await rest(tokenA, '/profile_section_revisions?section_key=eq.basic_profile&select=content_revision'); + const basicReviewAfterMutation = await rest(tokenA, '/profile_section_reviews?section_key=eq.basic_profile&select=reviewed_content_revision'); + check('a profile mutation makes the basic-profile review stale', mutateProfileAfterReview.ok && basicStateAfterMutation.ok && basicReviewAfterMutation.ok && basicStateAfterMutation.data[0].content_revision !== basicReviewAfterMutation.data[0].reviewed_content_revision); + const updateEducationA = await rest(tokenA, `/education_entries?id=eq.${educationA.id}`, { method: 'PATCH', body: { field: 'Electrical Engineering' }, prefer: 'return=representation', }); @@ -235,6 +254,39 @@ async function main() { const afterMutationRevision = await rest(tokenA, '/profile_section_revisions?section_key=eq.education&select=content_revision'); check('an education mutation makes its recorded review stale', staleReview.ok && afterMutationRevision.ok && staleReview.data.find((row) => row.section_key === 'education').reviewed_content_revision !== afterMutationRevision.data[0].content_revision); + const createSecondEducationA = await rest(tokenA, '/education_entries', { + method: 'POST', body: { user_id: userA.id, institution: 'Second Current University', education_status: 'current' }, prefer: 'return=representation', + }); + const secondEducationA = createSecondEducationA.data?.[0]; + check('user A can create a second current education entry', createSecondEducationA.ok && secondEducationA?.education_status === 'current'); + const switchPrimary = await rest(tokenA, '/rpc/set_primary_education', { + method: 'POST', body: { education_id: secondEducationA.id }, + }); + const primaryRows = await rest(tokenA, '/education_entries?select=id,is_primary'); + check('switching primary leaves exactly the selected second entry primary', switchPrimary.ok && primaryRows.ok && primaryRows.data.filter((row) => row.is_primary).length === 1 && primaryRows.data.find((row) => row.is_primary)?.id === secondEducationA.id); + const revisionBeforeReselect = await rest(tokenA, '/profile_section_revisions?section_key=eq.education&select=content_revision'); + const reselectPrimary = await rest(tokenA, '/rpc/set_primary_education', { method: 'POST', body: { education_id: secondEducationA.id } }); + const revisionAfterReselect = await rest(tokenA, '/profile_section_revisions?section_key=eq.education&select=content_revision'); + check('reselecting the current primary is idempotent', reselectPrimary.ok && revisionBeforeReselect.data[0].content_revision === revisionAfterReselect.data[0].content_revision); + const createCompletedEducationA = await rest(tokenA, '/education_entries', { + method: 'POST', body: { user_id: userA.id, institution: 'Completed University', education_status: 'completed', end_date: '2025-01-01' }, prefer: 'return=representation', + }); + const nonCurrentPrimary = await rest(tokenA, '/rpc/set_primary_education', { method: 'POST', body: { education_id: createCompletedEducationA.data?.[0]?.id } }); + check('a non-current education entry cannot become primary', !nonCurrentPrimary.ok && nonCurrentPrimary.status >= 400); + const deletePrimary = await rest(tokenA, `/education_entries?id=eq.${secondEducationA.id}`, { method: 'DELETE', prefer: 'return=representation' }); + const primaryAfterDelete = await rest(tokenA, '/education_entries?select=id,is_primary'); + check('deleting the selected primary does not automatically promote another entry', deletePrimary.ok && primaryAfterDelete.ok && primaryAfterDelete.data.filter((row) => row.is_primary).length === 0); + const reviewEducationAgain = await rest(tokenA, '/rpc/review_profile_section', { method: 'POST', body: { requested_section_key: 'education' } }); + const revisionBeforeConcurrentUpdates = await rest(tokenA, '/profile_section_revisions?section_key=eq.education&select=content_revision'); + const concurrentUpdates = await Promise.all([ + rest(tokenA, `/education_entries?id=eq.${educationA.id}`, { method: 'PATCH', body: { degree: 'BSc revised once' }, prefer: 'return=representation' }), + rest(tokenA, `/education_entries?id=eq.${educationA.id}`, { method: 'PATCH', body: { field: 'Electrical Engineering revised' }, prefer: 'return=representation' }), + ]); + const revisionAfterConcurrentUpdates = await rest(tokenA, '/profile_section_revisions?section_key=eq.education&select=content_revision'); + const educationReviewAfterConcurrentUpdates = await rest(tokenA, '/profile_section_reviews?section_key=eq.education&select=reviewed_content_revision'); + check('concurrent education mutations advance revision monotonically without losing increments', reviewEducationAgain.ok && concurrentUpdates.every((result) => result.ok) && revisionAfterConcurrentUpdates.data[0].content_revision >= revisionBeforeConcurrentUpdates.data[0].content_revision + 2); + check('education review is stale after a later education mutation', educationReviewAfterConcurrentUpdates.ok && educationReviewAfterConcurrentUpdates.data[0].reviewed_content_revision !== revisionAfterConcurrentUpdates.data[0].content_revision); + // ---- isolation: A cannot read or update B's data ---------------------- const readBAsA = await rest(tokenA, `/profiles?user_id=eq.${userB.id}`, {}); check('user A cannot read user B\'s profile (empty result, not an error)', readBAsA.ok && readBAsA.data.length === 0); @@ -263,6 +315,13 @@ async function main() { }); check('user A cannot select user B\'s education as primary', !foreignPrimary.ok && foreignPrimary.status >= 400); + const revisionsAAsB = await rest(tokenB, `/profile_section_revisions?user_id=eq.${userA.id}`); + const reviewsAAsB = await rest(tokenB, `/profile_section_reviews?user_id=eq.${userA.id}`); + check('user B cannot read user A\'s revisions or reviews', revisionsAAsB.ok && reviewsAAsB.ok && revisionsAAsB.data.length === 0 && reviewsAAsB.data.length === 0); + const reviewB = await rest(tokenB, '/rpc/review_profile_section', { method: 'POST', body: { requested_section_key: 'basic_profile' } }); + const reviewsAAfterBReview = await rest(tokenA, '/profile_section_reviews?section_key=eq.basic_profile&select=reviewed_content_revision'); + check('user B\'s review RPC changes only user B\'s state', reviewB.ok && reviewsAAfterBReview.ok && reviewsAAfterBReview.data[0].reviewed_content_revision !== null); + const directRevisionWrite = await rest(tokenA, '/profile_section_revisions', { method: 'POST', body: { user_id: userA.id, section_key: 'education', content_revision: 99 }, }); diff --git a/supabase/tests/database/040_privileged_function_security.test.sql b/supabase/tests/database/040_privileged_function_security.test.sql new file mode 100644 index 0000000..3e71ae5 --- /dev/null +++ b/supabase/tests/database/040_privileged_function_security.test.sql @@ -0,0 +1,90 @@ +-- Every Phase 1A function is deliberately SECURITY DEFINER with an empty +-- search_path. Browser roles may execute only the two constrained RPCs. +begin; +select plan(40); + +with functions(signature, label, browser_callable) as ( + values + ('public.raise_profile_content_validation(text,text,integer)', 'raise_profile_content_validation', false), + ('public.profiles_before_write()', 'profiles_before_write', false), + ('public.education_entries_before_write()', 'education_entries_before_write', false), + ('public.increment_profile_section_revision(uuid,text)', 'increment_profile_section_revision', false), + ('public.profiles_after_content_change()', 'profiles_after_content_change', false), + ('public.education_entries_after_content_change()', 'education_entries_after_content_change', false), + ('public.review_profile_section(text)', 'review_profile_section', true), + ('public.set_primary_education(uuid)', 'set_primary_education', true) +) +select ok(to_regprocedure(signature) is not null, label || ' exists') from functions; + +with functions(signature, label) as ( + values + ('public.raise_profile_content_validation(text,text,integer)', 'raise_profile_content_validation'), + ('public.profiles_before_write()', 'profiles_before_write'), + ('public.education_entries_before_write()', 'education_entries_before_write'), + ('public.increment_profile_section_revision(uuid,text)', 'increment_profile_section_revision'), + ('public.profiles_after_content_change()', 'profiles_after_content_change'), + ('public.education_entries_after_content_change()', 'education_entries_after_content_change'), + ('public.review_profile_section(text)', 'review_profile_section'), + ('public.set_primary_education(uuid)', 'set_primary_education') +) +select ok( + (select prosecdef from pg_proc where oid = to_regprocedure(signature)), + label || ' is SECURITY DEFINER' +) from functions; + +with functions(signature, label) as ( + values + ('public.raise_profile_content_validation(text,text,integer)', 'raise_profile_content_validation'), + ('public.profiles_before_write()', 'profiles_before_write'), + ('public.education_entries_before_write()', 'education_entries_before_write'), + ('public.increment_profile_section_revision(uuid,text)', 'increment_profile_section_revision'), + ('public.profiles_after_content_change()', 'profiles_after_content_change'), + ('public.education_entries_after_content_change()', 'education_entries_after_content_change'), + ('public.review_profile_section(text)', 'review_profile_section'), + ('public.set_primary_education(uuid)', 'set_primary_education') +) +select ok( + exists ( + select 1 from pg_proc + where oid = to_regprocedure(signature) + and coalesce(proconfig, array[]::text[]) @> array['search_path=""'] + ), + label || ' has an empty hardened search_path' +) from functions; + +with functions(signature, label) as ( + values + ('public.raise_profile_content_validation(text,text,integer)', 'raise_profile_content_validation'), + ('public.profiles_before_write()', 'profiles_before_write'), + ('public.education_entries_before_write()', 'education_entries_before_write'), + ('public.increment_profile_section_revision(uuid,text)', 'increment_profile_section_revision'), + ('public.profiles_after_content_change()', 'profiles_after_content_change'), + ('public.education_entries_after_content_change()', 'education_entries_after_content_change'), + ('public.review_profile_section(text)', 'review_profile_section'), + ('public.set_primary_education(uuid)', 'set_primary_education') +) +select ok( + not has_function_privilege('public', signature, 'EXECUTE') + and not has_function_privilege('anon', signature, 'EXECUTE'), + label || ' is not executable by PUBLIC or anon' +) from functions; + +with functions(signature, label, browser_callable) as ( + values + ('public.raise_profile_content_validation(text,text,integer)', 'raise_profile_content_validation', false), + ('public.profiles_before_write()', 'profiles_before_write', false), + ('public.education_entries_before_write()', 'education_entries_before_write', false), + ('public.increment_profile_section_revision(uuid,text)', 'increment_profile_section_revision', false), + ('public.profiles_after_content_change()', 'profiles_after_content_change', false), + ('public.education_entries_after_content_change()', 'education_entries_after_content_change', false), + ('public.review_profile_section(text)', 'review_profile_section', true), + ('public.set_primary_education(uuid)', 'set_primary_education', true) +) +select ok( + has_function_privilege('authenticated', signature, 'EXECUTE') = browser_callable + and not has_function_privilege('service_role', signature, 'EXECUTE'), + label || ' has the documented authenticated and service-role execute privileges' +) from functions; + +select * from finish(); +rollback; diff --git a/supabase/tests/database/050_primary_review_behavior.test.sql b/supabase/tests/database/050_primary_review_behavior.test.sql new file mode 100644 index 0000000..36977bd --- /dev/null +++ b/supabase/tests/database/050_primary_review_behavior.test.sql @@ -0,0 +1,47 @@ +-- Focused browser-role behavior for review freshness, primary selection, and +-- cascade safety. All fixture identities are synthetic and rolled back. +begin; +select plan(20); + +insert into auth.users (id, aud, role, email) +values + ('55555555-5555-5555-5555-555555555555', 'authenticated', 'authenticated', 'behavior-a@example.test'), + ('66666666-6666-6666-6666-666666666666', 'authenticated', 'authenticated', 'behavior-b@example.test'); +insert into public.profiles (user_id, headline) +values + ('55555555-5555-5555-5555-555555555555', 'A'), + ('66666666-6666-6666-6666-666666666666', 'B'); + +reset role; +set role anon; +select throws_ok($$ select public.review_profile_section('education') $$, '42501'::char(5), null, 'anonymous callers cannot execute the review RPC'); +select throws_ok($$ select public.set_primary_education('00000000-0000-0000-0000-000000000000') $$, '42501'::char(5), null, 'anonymous callers cannot execute the primary RPC'); + +reset role; +select set_config('request.jwt.claims', json_build_object('sub', '55555555-5555-5555-5555-555555555555', 'role', 'authenticated')::text, true); +set role authenticated; +select lives_ok($$ insert into public.education_entries (user_id, institution, education_status, end_date) values ('55555555-5555-5555-5555-555555555555', 'First', 'current', null), ('55555555-5555-5555-5555-555555555555', 'Second', 'current', null), ('55555555-5555-5555-5555-555555555555', 'Finished', 'completed', '2025-01-01') $$, 'owner can create two current and one completed entry'); +select lives_ok($$ select public.set_primary_education((select id from public.education_entries where institution = 'First')) $$, 'owner can select the first current entry'); +select is((select count(*)::int from public.education_entries where is_primary), 1, 'exactly one entry is primary after first selection'); +select is((select institution from public.education_entries where is_primary), 'First', 'the first selected row is primary'); +select lives_ok($$ select public.set_primary_education((select id from public.education_entries where institution = 'Second')) $$, 'owner can switch to the second current entry'); +select is((select count(*)::int from public.education_entries where is_primary), 1, 'switching still leaves exactly one primary'); +select is((select institution from public.education_entries where is_primary), 'Second', 'the second selected row becomes primary'); +select lives_ok($$ select public.set_primary_education((select id from public.education_entries where institution = 'Second')) $$, 'reselecting the current primary is idempotent'); +select throws_ok($$ select public.set_primary_education((select id from public.education_entries where institution = 'Finished')) $$, '23514'::char(5), null, 'a non-current entry cannot become primary'); +select lives_ok($$ select public.review_profile_section('education') $$, 'owner can review education'); +select lives_ok($$ delete from public.education_entries where institution = 'Second' $$, 'owner can delete the selected primary'); +select is((select count(*)::int from public.education_entries where is_primary), 0, 'deleting a primary does not promote another row'); +select isnt((select reviewed_content_revision from public.profile_section_reviews where section_key = 'education'), (select content_revision from public.profile_section_revisions where section_key = 'education'), 'education review becomes stale after deletion'); +select throws_ok($$ insert into public.profile_section_reviews (user_id, section_key, reviewed_content_revision) values ('55555555-5555-5555-5555-555555555555', 'education', 99) $$, '42501'::char(5), null, 'browser cannot directly write review rows'); +select throws_ok($$ delete from public.profiles where user_id = '55555555-5555-5555-5555-555555555555' $$, '42501'::char(5), null, 'profile owner DELETE remains denied'); + +reset role; +set role service_role; +select lives_ok($$ delete from public.profiles where user_id = '55555555-5555-5555-5555-555555555555' $$, 'trusted deletion cascades safely'); +select is((select count(*)::int from public.profile_section_revisions where user_id = '55555555-5555-5555-5555-555555555555'), 0, 'cascade deletion removes revision rows'); +select is((select count(*)::int from public.profile_section_reviews where user_id = '55555555-5555-5555-5555-555555555555'), 0, 'cascade deletion removes review rows'); + +reset role; +select * from finish(); +rollback;