Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
414 changes: 414 additions & 0 deletions scripts/seed-course-library-scorecards.ts

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions src/app/golf/(dashboard)/dashboard/courses/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { redirect } from 'next/navigation';
import type { Metadata } from 'next';
import { getGolfSessionProfile } from '@/lib/auth/session';
import { fairwayScope } from '@/lib/redesign/flag';
import { isSuperAdminUserId } from '@/lib/admin/super-admin-shared';
import {
listCoursesStrict,
getCourseTeeCountsStrict,
Expand Down Expand Up @@ -32,13 +33,18 @@ export default async function CoursesPage() {
getTeamSavedCoursesStrict(),
]);

// #913 part 2 — shared library courses (no human creator) may only be
// edited/removed by a super admin; everything else keeps open contribution.
const isSuperAdmin = isSuperAdminUserId(session.userId, process.env.SUPER_ADMIN_USER_IDS);

return (
<div className={fairwayScope('min-h-full bg-canvas')}>
<CourseLibraryClient
courses={courses}
teeCounts={teeCounts}
savedCourses={savedCourses}
canManageTeam={userRole === 'coach'}
isSuperAdmin={isSuperAdmin}
/>
</div>
);
Expand Down
165 changes: 161 additions & 4 deletions src/app/golf/actions/__tests__/course-library.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';

// ── Mocks (declared before importing the action module) ──────────────────────
const revalidatePath = vi.fn();
Expand All @@ -13,7 +13,7 @@ vi.mock('@/lib/golf/resolve-team-server', () => ({
let currentClient: unknown = null;
vi.mock('@/lib/supabase/server', () => ({ createClient: async () => currentClient }));

import { createCourse, listCourses, saveTeamCourse, updateTee, getTeeRoundDefaults, getTeamSavedCourses, updateCourse, restoreCourse, contributeCourseFromRound, listCoursesStrict, getCourseTeeCountsStrict, getTeamSavedCoursesStrict } from '../course-library';
import { createCourse, listCourses, saveTeamCourse, updateTee, getTeeRoundDefaults, getTeamSavedCourses, updateCourse, restoreCourse, softDeleteCourse, getCourseTeeHoles, contributeCourseFromRound, listCoursesStrict, getCourseTeeCountsStrict, getTeamSavedCoursesStrict } from '../course-library';

// ── A scriptable, chainable Supabase query-builder mock ──────────────────────
type Scripted = { maybeSingle?: unknown; single?: unknown; resolve?: unknown };
Expand Down Expand Up @@ -212,7 +212,10 @@ describe('getTeamSavedCourses — soft-delete must not leak into a team library'

describe('Phase 5 — unique normalized_name index: 23505 collision handling', () => {
it('updateCourse surfaces a rename collision as a clear message (not a generic failure)', async () => {
const before = { id: 'c1', name: 'Old Name', normalized_name: 'old name', deleted_at: null };
// created_by_user_id set: a user-owned course, not a library row — keeps
// this test isolated to the 23505 collision path (see the ownership-gate
// describe block below for the library-row-blocks-edit behavior itself).
const before = { id: 'c1', name: 'Old Name', normalized_name: 'old name', deleted_at: null, created_by_user_id: 'u1' };
const courses = tableBuilder({
maybeSingle: { data: before, error: null }, // the "before" fetch
single: { data: null, error: { code: '23505', message: 'duplicate key' } }, // the update collides
Expand All @@ -227,7 +230,12 @@ describe('Phase 5 — unique normalized_name index: 23505 collision handling', (
});

it('restoreCourse refuses to un-delete into an active name collision (23505)', async () => {
const courses = tableBuilder({ resolve: { data: null, error: { code: '23505', message: 'duplicate key' } } });
// maybeSingle: the ownership-gate lookup in setCourseDeleted — created_by_user_id
// set so this stays a user-owned-course scenario, isolated to the 23505 path.
const courses = tableBuilder({
maybeSingle: { data: { id: 'c1', created_by_user_id: 'u1' }, error: null },
resolve: { data: null, error: { code: '23505', message: 'duplicate key' } },
});
currentClient = makeClient({ id: 'u1' }, {
golf_coaches: tableBuilder({ maybeSingle: { data: null, error: null } }),
golf_courses: courses,
Expand Down Expand Up @@ -332,3 +340,152 @@ describe('updateTee — destructive-write guard', () => {
expect(client.from).not.toHaveBeenCalledWith('golf_course_tee_holes');
});
});

describe('#913 part 2 — library-owned courses require a super admin to edit/remove', () => {
afterEach(() => {
vi.unstubAllEnvs();
});

it('updateCourse blocks a non-admin coach from editing a library row (created_by_user_id null)', async () => {
const before = {
id: 'c1', name: 'Pinehurst No. 2', normalized_name: 'pinehurst 2',
deleted_at: null, created_by_user_id: null,
};
const courses = tableBuilder({ maybeSingle: { data: before, error: null } });
currentClient = makeClient({ id: 'u1' }, {
golf_coaches: tableBuilder({ maybeSingle: { data: null, error: null } }),
golf_courses: courses,
});

const res = await updateCourse('c1', { name: 'Renamed' });

expect(res).toEqual({ success: false, error: 'This is a shared library course — only an admin can edit it.' });
expect(courses._calls.update).toBeUndefined(); // never reaches the write
});

it('updateCourse allows a super admin (SUPER_ADMIN_USER_IDS allowlist) to edit a library row', async () => {
vi.stubEnv('SUPER_ADMIN_USER_IDS', 'u1,someone-else');
const before = {
id: 'c1', name: 'Pinehurst No. 2', normalized_name: 'pinehurst 2',
deleted_at: null, created_by_user_id: null,
};
const after = { ...before, name: 'Renamed', normalized_name: 'renamed' };
const courses = tableBuilder({
maybeSingle: { data: before, error: null },
single: { data: after, error: null },
});
currentClient = makeClient({ id: 'u1' }, {
golf_coaches: tableBuilder({ maybeSingle: { data: null, error: null } }),
golf_courses: courses,
golf_course_edit_history: tableBuilder({ resolve: { data: null, error: null } }),
});

const res = await updateCourse('c1', { name: 'Renamed' });
expect(res.success).toBe(true);
});

it('updateCourse leaves a team/user-contributed course editable by any authenticated coach (open-contribution model unchanged)', async () => {
const before = {
id: 'c1', name: 'My Local Club', normalized_name: 'my local club',
deleted_at: null, created_by_user_id: 'some-other-real-user',
};
const after = { ...before, name: 'Renamed', normalized_name: 'renamed' };
const courses = tableBuilder({
maybeSingle: { data: before, error: null },
single: { data: after, error: null },
});
currentClient = makeClient({ id: 'u1' }, {
golf_coaches: tableBuilder({ maybeSingle: { data: null, error: null } }),
golf_courses: courses,
golf_course_edit_history: tableBuilder({ resolve: { data: null, error: null } }),
});

const res = await updateCourse('c1', { name: 'Renamed' });
expect(res.success).toBe(true);
});

it('softDeleteCourse blocks a non-admin coach from removing a library row', async () => {
const courses = tableBuilder({
maybeSingle: { data: { id: 'c1', created_by_user_id: null }, error: null },
});
currentClient = makeClient({ id: 'u1' }, {
golf_coaches: tableBuilder({ maybeSingle: { data: null, error: null } }),
golf_courses: courses,
});

const res = await softDeleteCourse('c1');

expect(res).toEqual({ success: false, error: 'This is a shared library course — only an admin can remove it.' });
expect(courses._calls.update).toBeUndefined();
});

it('softDeleteCourse allows a super admin to remove a library row', async () => {
vi.stubEnv('SUPER_ADMIN_USER_IDS', 'u1');
const courses = tableBuilder({
maybeSingle: { data: { id: 'c1', created_by_user_id: null }, error: null },
resolve: { data: null, error: null },
});
currentClient = makeClient({ id: 'u1' }, {
golf_coaches: tableBuilder({ maybeSingle: { data: null, error: null } }),
golf_courses: courses,
golf_course_edit_history: tableBuilder({ resolve: { data: null, error: null } }),
});

const res = await softDeleteCourse('c1');
expect(res.success).toBe(true);
});

it('softDeleteCourse leaves a team/user-contributed course removable by any authenticated coach (open-contribution model unchanged)', async () => {
const courses = tableBuilder({
maybeSingle: { data: { id: 'c1', created_by_user_id: 'some-other-real-user' }, error: null },
resolve: { data: null, error: null },
});
currentClient = makeClient({ id: 'u1' }, {
golf_coaches: tableBuilder({ maybeSingle: { data: null, error: null } }),
golf_courses: courses,
golf_course_edit_history: tableBuilder({ resolve: { data: null, error: null } }),
});

const res = await softDeleteCourse('c1');
expect(res.success).toBe(true);
});
});

describe('getCourseTeeHoles — #913 part 3 (course detail "Holes" summary)', () => {
it('returns {} for an unauthenticated caller', async () => {
currentClient = makeClient(null);
expect(await getCourseTeeHoles('c1')).toEqual({});
});

it('returns {} when the course has no active tees (skips the holes query)', async () => {
const tees = tableBuilder({ resolve: { data: [], error: null } });
const client = makeClient({ id: 'u1' }, { golf_course_tees: tees });
currentClient = client;

expect(await getCourseTeeHoles('c1')).toEqual({});
expect(client.from).not.toHaveBeenCalledWith('golf_course_tee_holes');
});

it('groups hole rows by tee_id, scoped to only this course’s active tee ids', async () => {
const tees = tableBuilder({ resolve: { data: [{ id: 't1' }, { id: 't2' }], error: null } });
const holes = tableBuilder({
resolve: {
data: [
{ id: 'h1', tee_id: 't1', hole_number: 1, par: 4, yardage: 400, handicap_index: 7 },
{ id: 'h2', tee_id: 't2', hole_number: 1, par: 5, yardage: 520, handicap_index: 3 },
{ id: 'h3', tee_id: 't1', hole_number: 2, par: 3, yardage: 175, handicap_index: 15 },
],
error: null,
},
});
currentClient = makeClient({ id: 'u1' }, { golf_course_tees: tees, golf_course_tee_holes: holes });

const res = await getCourseTeeHoles('c1');

expect(res['t1']).toHaveLength(2);
expect(res['t2']).toHaveLength(1);
expect(res['t1']![0]!.hole_number).toBe(1);
expect(res['t1']![1]!.hole_number).toBe(2);
expect(holes._calls.in).toContainEqual(['tee_id', ['t1', 't2']]);
});
});
99 changes: 99 additions & 0 deletions src/app/golf/actions/course-library.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { resolveCoachTeamIdWithCookie } from '@/lib/golf/resolve-team-server';
import { logServerError } from '@/lib/server-error-logger';
import { fetchAllRowsResult } from '@/lib/supabase/fetch-all-rows';
import { withAdminObserved } from '@/lib/admin/observed-action';
import { isSuperAdminUserId } from '@/lib/admin/super-admin-shared';
import {
normalizeName,
isTeeComplete,
Expand All @@ -43,6 +44,7 @@ import {
import type {
GolfCourse,
GolfCourseTee,
GolfCourseTeeHole,
GolfCourseTeeWithHoles,
GolfTeamSavedCourse,
GolfTeamSavedCourseWithCourse,
Expand Down Expand Up @@ -106,6 +108,30 @@ async function getCoachTeam(
return { userId: user.id, teamId };
}

/**
* True when a golf_courses row has NO human creator — i.e. it's a
* staff-curated shared LIBRARY course (bulk-seeded via a reviewed script,
* e.g. scripts/seed-course-library-scorecards.ts), not a coach/team's own
* contribution. `created_by_user_id` is set on every row created through the
* app (createCourse / contributeCourseFromRound both stamp the actor), so a
* null value can only mean "nobody added this — it shipped with the
* library." (#913 part 2 — ownership gating.)
*
* Library rows stay visible + usable by every team (tees, saves, rounds all
* still work normally), but must not be renamed or removed by an arbitrary
* coach — only a super admin may edit or remove them. Team/user-contributed
* courses keep the pre-existing open-contribution model unchanged.
*/
function isLibraryOwnedCourseRow(row: { created_by_user_id?: unknown }): boolean {
return row.created_by_user_id == null;
}

/** Cheap, DB-round-trip-free super-admin check (env-var allowlist), the same
* pattern src/app/golf/actions/auth.ts already uses outside /admin. */
function isActorSuperAdmin(actor: Actor): boolean {
return isSuperAdminUserId(actor.userId, process.env.SUPER_ADMIN_USER_IDS);
}

// ─────────────────────────────────────────────────────────────────────────────
// READS
// ─────────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -410,6 +436,54 @@ export async function getCourseDetail(
return observedGetCourseDetail(courseId);
}

/**
* Read-only hole rows for EVERY active tee of a course, keyed by tee id
* (each sorted by hole_number). #913 part 3 — powers the course detail
* sheet's compact "Holes" summary (par row + yardage row per tee) so a coach
* can see the scorecard without opening tee-set edit. One extra round trip
* per course-detail open; deliberately a separate action from
* `getCourseDetail` (used by the new-round + tee-picker flows too) so those
* hot paths never pay for hole data they don't render.
*/
async function getCourseTeeHolesImpl(courseId: string): Promise<Record<string, GolfCourseTeeHole[]>> {
const supabase = await createClient();
const { data: { user } } = await supabase.auth.getUser();
if (!user) return {};

const { data: teeRows } = await supabase
.from('golf_course_tees')
.select('id')
.eq('course_id', courseId)
.is('deleted_at', null);
const teeIds = (teeRows ?? []).map((t) => t.id as string);
if (teeIds.length === 0) return {};

const { data: holeRows } = await supabase
.from('golf_course_tee_holes')
.select('*')
.in('tee_id', teeIds)
.order('hole_number');

const byTee: Record<string, GolfCourseTeeHole[]> = {};
for (const row of holeRows ?? []) {
const hole = mapTeeHoleRow(row);
(byTee[hole.tee_id] ??= []).push(hole);
}
return byTee;
}

const observedGetCourseTeeHoles = withAdminObserved(
'getCourseTeeHoles',
{ sport: 'golf', feature: 'course_library' },
getCourseTeeHolesImpl,
);

export async function getCourseTeeHoles(
courseId: string,
): Promise<Record<string, GolfCourseTeeHole[]>> {
return observedGetCourseTeeHoles(courseId);
}

/** A single tee with its hole rows (ordered by hole_number). */
async function getTeeWithHolesImpl(teeId: string): Promise<GolfCourseTeeWithHoles | null> {
const supabase = await createClient();
Expand Down Expand Up @@ -834,6 +908,12 @@ async function updateCourseImpl(
.maybeSingle();
if (!before) return { success: false, error: 'Course not found' };

// #913 part 2 — library-owned rows (no human creator) can only be edited
// by a super admin; team/user-contributed courses keep open contribution.
if (isLibraryOwnedCourseRow(before) && !isActorSuperAdmin(actor)) {
return { success: false, error: 'This is a shared library course — only an admin can edit it.' };
}

const update: CourseUpdate = {
last_edited_by_user_id: actor.userId,
last_edited_by_team_id: actor.teamId,
Expand Down Expand Up @@ -934,6 +1014,25 @@ async function setCourseDeleted(courseId: string, deleted: boolean): Promise<Res
const actor = await getActor(supabase);
if (!actor) return { success: false, error: 'You must be logged in' };

// #913 part 2 — same library-ownership gate as updateCourse. Soft-delete
// and restore are both writes to golf_courses.deleted_at, so they need the
// same guard: an arbitrary coach may not remove (or un-remove) a shared
// library row nobody on their team owns.
const { data: courseRow } = await supabase
.from('golf_courses')
.select('created_by_user_id')
.eq('id', courseId)
.maybeSingle();
if (!courseRow) return { success: false, error: 'Course not found' };
if (isLibraryOwnedCourseRow(courseRow) && !isActorSuperAdmin(actor)) {
return {
success: false,
error: deleted
? 'This is a shared library course — only an admin can remove it.'
: 'This is a shared library course — only an admin can restore it.',
};
}

const { error } = await supabase
.from('golf_courses')
.update({
Expand Down
Loading
Loading