From 6b296715d42ce8eb2eeee7a8d156fb9dc1631523 Mon Sep 17 00:00:00 2001 From: Georgy Butaev <41178744+g-but@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:02:38 +0200 Subject: [PATCH] fix(timeline): likes, dislikes, replies, deleting and quoting were all dead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported as separate complaints; nearly all of them were one cause. Nineteen plpgsql functions write to a column, table or type that does not exist. plpgsql only plans a statement when it RUNS, so each raised 42703 the moment a user triggered it and at no other time. Every one looked healthy: defined, listed by \df, routed by PostgREST, called happily by the app. Nine of them are the timeline. like/unlike_timeline_event, dislike/undislike_timeline_event end with `UPDATE timeline_events SET like_count = ...` under a comment reading "if column exists". timeline_events has no count columns and never has; the counts live in timeline_event_stats, which the line above already updates correctly. Since a function body is one transaction, the raise rolled back the INSERT three lines earlier — so the like was written, then unwritten. The UI applied its optimistic update, saw success:false and rolled back, which is why the heart filled for ~150ms and emptied again. add_timeline_comment, delete_timeline_comment same line, same column, so replies never saved either. soft_delete_timeline_event checked `SELECT user_id FROM profiles WHERE role = 'admin'`. profiles has neither column. And even fixed it would not have worked: it set only deleted_at while every read path filters is_deleted. Production confirms nothing ever took this path — 102 events have both flags, zero have deleted_at alone. Both are set now, the reason goes in the deletion_reason COLUMN that exists, and the phantom admin branch is gone rather than repaired: it could never have run, so nothing real is lost, and a moderator capability should be built deliberately against a table that exists. create_quote_reply failed three separate ways, each hidden behind the last: assigned text into a jsonb column, incremented a quote_count that does not exist, named four columns notifications does not have, used a `type` its CHECK constraint forbids, and never set the NOT NULL subject_type — so it could not have succeeded on any input. It also never wrote `description`, which is the field PostContent renders, so a quote reply would have shown as an empty post. Verified by clicking Like on orangecat.ch/timeline and reading the response: HTTP 400, 42703. Each function was then rehearsed against production inside a transaction and rolled back — like returns 1 and lands in timeline_event_stats, delete returns true and the row is genuinely hidden, a quote reply creates with a non-empty body and notifies its author. THE GATE, because nineteen is not a run of bad luck. plpgsql_check reads every function body against the live schema and reports exactly this class. Exposed as count_broken_plpgsql_functions() over PostgREST, in the same shape as count_email_derived_usernames, and ratcheted in check-data-invariants.mjs at the eleven that remain. Its limits are stated where it is defined: it is static, so a NOT NULL left unset or a CHECK violation still only fails at runtime — both of which create_quote_reply also did. Nothing else could have caught these. Unit tests mock the database; check-rpc-exists proves a function is DEFINED, which all nineteen were; migration replay proves the SQL applies, and creating a function never validates its body. Ctrl+Enter was a different bug with the same shape — it worked, and looked like it did not. The post was created; the composer kept the text, because the effect syncing content into the contentEditable bails whenever the editor is the active element, which it always is right after a keystroke. So the obvious response was to press again, and the server rejected that as a duplicate. Clicking the button never showed it because a click moves focus to the button first. A reset to empty is now exempt from that guard: there is nothing to clobber. UI, following X rather than inventing: The action row was five near-identical JSX blocks that had already drifted — one had -ml-2, some had titles, the reply count was computed by an inline three-way IIFE. It is one config-driven component now. Counts render into a fixed-width tabular slot instead of collapsing to '' at zero, so liking something no longer reflows the row and slides every later icon under the cursor that just clicked it. Edit post: Save moves from the header to the bottom right, beside the character count that decides whether it is allowed, instead of diagonally opposite it. Repost: the quoted post now sits directly under what you are writing, with the buttons last. The action row used to separate the composer from the thing it refers to. The textarea loses its border — the dialog is already the container. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012dpTLxh5GJWeWTF1UEvcD5 --- .../hooks/useContentEditableEditor.test.tsx | 83 ++++ scripts/check-data-invariants.mjs | 43 ++ src/components/timeline/EditPostModal.tsx | 57 +-- src/components/timeline/PostActions.tsx | 214 ++++++--- src/components/timeline/RepostModal.tsx | 77 ++-- src/hooks/useContentEditableEditor.ts | 31 +- ...ons_write_a_column_that_does_not_exist.sql | 436 ++++++++++++++++++ ...etect_functions_that_reference_nothing.sql | 81 ++++ 8 files changed, 884 insertions(+), 138 deletions(-) create mode 100644 __tests__/unit/hooks/useContentEditableEditor.test.tsx create mode 100644 supabase/migrations/20260828110000_reactions_write_a_column_that_does_not_exist.sql create mode 100644 supabase/migrations/20260828120000_detect_functions_that_reference_nothing.sql diff --git a/__tests__/unit/hooks/useContentEditableEditor.test.tsx b/__tests__/unit/hooks/useContentEditableEditor.test.tsx new file mode 100644 index 000000000..8d139dfbc --- /dev/null +++ b/__tests__/unit/hooks/useContentEditableEditor.test.tsx @@ -0,0 +1,83 @@ +/** + * The composer must empty itself after a post — including when it still has + * focus. + * + * This is the bug that made Ctrl+Enter look broken in production. The shortcut + * worked and the post was created; the composer simply kept showing the text, + * because the effect that syncs `content` into the contentEditable bailed out + * whenever the editor was the active element — which it always is right after + * you press a key in it. So the natural response was to press again, and the + * server rejected that as a duplicate ("You just posted this"), which looked + * like a second failure. + * + * Clicking the button never showed it: a click moves focus to the button first, + * so the guard did not apply. A shortcut and a button that disagree about + * whether the composer clears is the shape worth pinning here. + */ + +import { renderHook, act } from '@testing-library/react'; +import { useContentEditableEditor } from '@/hooks/useContentEditableEditor'; + +jest.mock('@/utils/markdownEditor', () => ({ + markdownToHtml: (md: string) => md, + htmlToMarkdown: (html: string) => html, + getSelectionRange: () => ({ start: 0, end: 0 }), + setSelectionRange: jest.fn(), +})); + +/** + * Mount the hook and give it a real element to own. `tabIndex` is what makes a + * div focusable in jsdom — without it `focus()` is a no-op and the guard under + * test would never engage, so the test would pass for the wrong reason. + */ +function mount(initial: string) { + const editor = document.createElement('div'); + editor.contentEditable = 'true'; + editor.tabIndex = 0; + document.body.appendChild(editor); + + const view = renderHook( + ({ content }: { content: string }) => + useContentEditableEditor({ content, onContentChange: jest.fn() }), + { initialProps: { content: initial } } + ); + + act(() => { + view.result.current.editorRef.current = editor; + }); + + return { editor, rerender: view.rerender }; +} + +describe('useContentEditableEditor content sync', () => { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('clears the editor when content is reset while it still has focus', () => { + const { editor, rerender } = mount('a draft'); + editor.innerHTML = 'a draft'; + editor.focus(); + // Guard the guard: if focus did not take, this test proves nothing. + expect(document.activeElement).toBe(editor); + + // What a successful post does: content state goes back to empty. + act(() => rerender({ content: '' })); + + expect(editor.textContent?.trim()).toBe(''); + }); + + it('still refuses to overwrite text being typed while focused', () => { + // The relaxed guard exists for a real reason, and only the empty case is + // exempt — an external change to non-empty text must not clobber a + // half-written post. + const { editor, rerender } = mount(''); + editor.innerHTML = 'what I am typing'; + editor.focus(); + expect(document.activeElement).toBe(editor); + + act(() => rerender({ content: 'something the app decided' })); + + expect(editor.textContent).toBe('what I am typing'); + }); +}); diff --git a/scripts/check-data-invariants.mjs b/scripts/check-data-invariants.mjs index 01bd67a14..a0a6a2e4e 100644 --- a/scripts/check-data-invariants.mjs +++ b/scripts/check-data-invariants.mjs @@ -383,6 +383,48 @@ async function checkCatHandle() { notes.push(`cat: @${CAT_HANDLE} resolves to the Cat`); } +/** + * Functions that reference a column, table or type that does not exist. + * + * Nineteen of them on 2026-08-28, silently, for months: likes, dislikes, + * replies, deleting a post and quote replies were all dead in production, along + * with four AI-withdrawal functions and both nearby searches. Every one looked + * healthy — defined, routable, called by the app — because plpgsql only plans a + * statement when it runs, so a write to a missing column raises 42703 at call + * time and never before. + * + * Nothing else in the stack sees this. Unit tests mock the database; + * check-rpc-exists proves a function is DEFINED, which all of these were; + * migration replay proves the SQL applies, and creating a function never + * validates its body. + * + * A ratchet rather than a demand for zero: eleven remain after the timeline + * ones were repaired, and each of those needs a decision rather than a + * mechanical edit (does `ai_creator_withdrawals` want a `completed_at` column, + * or should the write go?). Demanding zero tomorrow would make this red about + * work that is queued, which is how a gate teaches people to ignore it. + * `SELECT * FROM list_broken_plpgsql_functions()` names them. + */ +const BROKEN_FUNCTION_BASELINE = 11; + +async function checkBrokenFunctions() { + const count = Number(await rpc('count_broken_plpgsql_functions')); + + if (count > BROKEN_FUNCTION_BASELINE) { + violation( + 'functions.reference_missing_objects', + `${count} plpgsql function(s) reference something that does not exist, up from ` + + `${BROKEN_FUNCTION_BASELINE}. A new one will fail only when a user triggers it, with ` + + `42703 and no other symptom — run list_broken_plpgsql_functions() to see which`, + [] + ); + } else { + notes.push( + `functions: ${count} reference a missing object (baseline ${BROKEN_FUNCTION_BASELINE}, never rises)` + ); + } +} + async function checkOrphanedProfiles() { const count = Number(await rpc('count_orphaned_profiles')); @@ -516,6 +558,7 @@ async function main() { checkOrphanedProfiles, checkEmailDerivedUsernames, checkCatHandle, + checkBrokenFunctions, checkOrphanedCatConversations, checkOrphanedActors, ]; diff --git a/src/components/timeline/EditPostModal.tsx b/src/components/timeline/EditPostModal.tsx index 985fab1a3..f1ee93774 100644 --- a/src/components/timeline/EditPostModal.tsx +++ b/src/components/timeline/EditPostModal.tsx @@ -118,29 +118,15 @@ export function EditPostModal({ {/* Modal */}
- {/* Header */} -
+ {/* Header — dismissal only. The action that commits the edit lives at + the end of the form, next to the character count that says whether + it is allowed, rather than diagonally opposite it. */} +

Edit post

- -
{/* Content */} @@ -194,14 +180,33 @@ export function EditPostModal({
- {/* Character count */} -
- {charCount.toLocaleString()} / {maxChars.toLocaleString()} + {/* Character count and the commit action, in reading order: what + you have written, then whether you may save it, then Save. */} +
+
+ {charCount.toLocaleString()} / {maxChars.toLocaleString()} +
+ +
diff --git a/src/components/timeline/PostActions.tsx b/src/components/timeline/PostActions.tsx index 4158d2275..53c0abc72 100644 --- a/src/components/timeline/PostActions.tsx +++ b/src/components/timeline/PostActions.tsx @@ -1,18 +1,31 @@ 'use client'; import React from 'react'; -import { Heart, MessageCircle, Share2, ThumbsDown, Repeat2 } from 'lucide-react'; +import { Heart, MessageCircle, Share2, ThumbsDown, Repeat2, type LucideIcon } from 'lucide-react'; import { ShareModal } from '@/components/timeline/ShareModal'; import { TimelineDisplayEvent } from '@/types/timeline'; import { usePostInteractions } from '@/hooks/usePostInteractions'; import { cn } from '@/lib/utils'; -import { TIMELINE_SURFACE } from '@/config/timeline'; /** - * PostActions Component + * The row of things you can do to a post. * - * Renders interaction buttons (like, dislike, comment, repost, share) for a post. - * All business logic is delegated to the usePostInteractions hook for DRY compliance. + * Written as data rather than as five near-identical JSX blocks, because that + * is what it is: the same button with a different icon, count, colour and + * handler. The duplicated version had already drifted — the reply button + * carried a `-ml-2` none of the others did, only some had a `title`, and the + * reply count was computed inline by a three-way fallback IIFE while the rest + * read a single field. Every one of those is invisible until you look at two + * of them side by side. + * + * Layout follows X, for the reason X does it: the count sits in a fixed-width + * slot that is reserved whether or not there is a number in it. Rendering `''` + * for zero — which is what this did — means the row re-flows the moment you + * like something, and every icon after it slides sideways under the cursor + * that just clicked. Reserving the space costs nothing and the row never moves. + * + * All business logic stays in usePostInteractions. This file decides how the + * row looks and nothing else. */ interface PostActionsProps { @@ -24,8 +37,59 @@ interface PostActionsProps { isReposting?: boolean; } -function actionClassName(active: boolean, activeClassName: string) { - return cn(TIMELINE_SURFACE.iconButton, 'gap-1 px-2 text-sm', active && activeClassName); +interface ActionSpec { + key: string; + label: string; + icon: LucideIcon; + count?: number; + active: boolean; + /** Colour once active, and on hover — one accent per action, as X does. */ + accent: string; + onClick?: () => void; + disabled?: boolean; + /** Only where the meaning is not obvious from the icon. */ + title?: string; + /** The heart and repeat fill when active; share never does. */ + fillWhenActive?: boolean; +} + +function ActionButton({ spec }: { spec: ActionSpec }) { + const { icon: Icon, count, active, accent, fillWhenActive = true } = spec; + + return ( + + ); } export function PostActions({ @@ -36,7 +100,6 @@ export function PostActions({ onRepostClick, isReposting = false, }: PostActionsProps) { - // Delegate all interaction logic to the hook const { isLiking, handleLike, @@ -49,76 +112,79 @@ export function PostActions({ handleShareConfirm, } = usePostInteractions({ event, onUpdate, onAddEvent }); - return ( - <> -
- {/* Reply */} - - - {/* Repost */} - + // One place decides what a reply count is. It used to be an inline IIFE with + // three fallbacks in this file only, so any other reader of the same number + // could disagree with the row rendering it. + const replyCount = + event.replyCount ?? + (Array.isArray(event.replies) ? event.replies.length : undefined) ?? + event.commentsCount ?? + 0; - {/* Like */} - + const actions: ActionSpec[] = [ + { + key: 'reply', + label: 'Reply', + icon: MessageCircle, + count: replyCount, + active: false, + accent: 'text-fg-primary', + onClick: onToggleComments, + }, + { + key: 'repost', + label: 'Repost', + icon: Repeat2, + count: event.repostsCount || 0, + active: !!event.userReposted, + accent: 'text-status-positive', + onClick: onRepostClick, + disabled: isReposting, + }, + { + key: 'like', + label: 'Like', + icon: Heart, + count: event.likesCount || 0, + active: !!event.userLiked, + accent: 'text-status-negative', + onClick: handleLike, + disabled: isLiking, + }, + { + key: 'dislike', + label: 'Dislike', + icon: ThumbsDown, + count: event.dislikesCount || 0, + active: !!event.userDisliked, + accent: 'text-status-warning', + onClick: handleDislike, + disabled: isDisliking, + title: 'Dislike this post — helps the crowd flag scams', + }, + { + key: 'share', + label: 'Share', + icon: Share2, + active: !!event.userShared, + accent: 'text-fg-primary', + onClick: handleShareOpen, + disabled: isSharing, + fillWhenActive: false, + }, + ]; - {/* Dislike */} - - - {/* Share */} - + return ( + <> + {/* `-ml-1.5` pulls the first icon's round target back so the ICON aligns + with the text above it, not the padding around it. Applied once, to + the row, instead of to one button and not the others. */} +
+ {actions.map(spec => ( + + ))}
- {/* Share Modal */}
-
-
- - {remainingCharacters} - - characters left -
-
- - -
-
- - {/* Original Post Preview (X-style) */} + {/* The post being quoted sits directly under what you are writing, + as it does on X and as it will in the timeline once posted. It + used to come AFTER the buttons, which put the action row between + the composer and the thing it refers to and broke the reading + order — write, see what you are quoting, then act. */}
+ +
+ {/* Only counts down once it matters. A number sitting there from + the first keystroke reads as a limit you are approaching. */} + + {remainingCharacters <= 20 ? `${remainingCharacters} left` : ''} + +
+ + +
+
diff --git a/src/hooks/useContentEditableEditor.ts b/src/hooks/useContentEditableEditor.ts index 0a68c3797..d9955171d 100644 --- a/src/hooks/useContentEditableEditor.ts +++ b/src/hooks/useContentEditableEditor.ts @@ -44,16 +44,41 @@ export function useContentEditableEditor({ useEffect(() => { const editor = editorRef.current; - if (!editor || isComposing || document.activeElement === editor) { + if (!editor || isComposing) { + return; + } + + // Normally an external content change must not overwrite what someone is + // actively typing, so a focused editor is left alone. Being RESET to empty + // is the exception, and it has to be: that is what a successful post does, + // and after Ctrl+Enter the editor still has focus. + // + // Without this exception the composer kept the text of a post that had just + // been created. It looked like the shortcut had done nothing, so the + // natural response was to press it again — which the server rejected as a + // duplicate ("You just posted this"), making it look broken twice over. + // Clicking the button never showed this because clicking moves focus to the + // button first. + // + // Safe as an exception because there is nothing to clobber: the text being + // removed is text the app has already decided to remove. + const isReset = content.trim().length === 0; + if (!isReset && document.activeElement === editor) { return; } const currentHtml = editor.innerHTML.replace(/\s+/g, ' ').trim(); const expectedHtml = markdownToHtml(content).replace(/\s+/g, ' ').trim(); - if (currentHtml !== expectedHtml && expectedHtml !== '
') { + // `expectedHtml !== '
'` stops an empty render from wiping the editor + // mid-keystroke, but it would also block a deliberate reset, so a reset is + // allowed past it for the same reason as above. + if (currentHtml !== expectedHtml && (isReset || expectedHtml !== '
')) { const selection = getSelectionRange(editor); - const wasFocused = document.activeElement === editor; + // On a reset there is no position left to restore — the old offsets point + // into text that no longer exists — so the caret simply goes to the start + // of the now-empty composer, which is where someone would begin typing. + const wasFocused = !isReset && document.activeElement === editor; editor.innerHTML = sanitize(expectedHtml || '
'); diff --git a/supabase/migrations/20260828110000_reactions_write_a_column_that_does_not_exist.sql b/supabase/migrations/20260828110000_reactions_write_a_column_that_does_not_exist.sql new file mode 100644 index 000000000..d66bcc273 --- /dev/null +++ b/supabase/migrations/20260828110000_reactions_write_a_column_that_does_not_exist.sql @@ -0,0 +1,436 @@ +-- Likes, dislikes and replies have never worked. This is why. +-- +-- Six functions end with a line writing a column that does not exist: +-- +-- -- Update timeline_events.like_count if column exists +-- UPDATE timeline_events SET like_count = v_count WHERE id = p_event_id; +-- +-- `timeline_events` has no count columns at all — never has. The comment says +-- "if column exists", but plpgsql has no such conditional: the statement is +-- planned when the function runs and raises 42703, `column "like_count" of +-- relation "timeline_events" does not exist`. +-- +-- Because a function body is one transaction, the exception rolls back the +-- INSERT that ran three lines earlier. So the like is written, then unwritten, +-- and the caller gets an error. The UI does the rest of the damage invisibly: +-- it applies an optimistic update, sees `success: false`, and rolls back — so +-- the heart fills for ~150ms and empties again. Nothing is logged where anyone +-- would look, and the button never appears broken, only unresponsive. +-- +-- Verified in production 2026-08-28 by clicking Like on orangecat.ch/timeline +-- and reading the response: HTTP 400, code 42703. +-- +-- Affected, all reachable from the app: +-- like_timeline_event / unlike_timeline_event → likes +-- dislike_timeline_event / undislike_timeline_event → dislikes +-- add_timeline_comment → replies +-- delete_timeline_comment → deleting a reply +-- +-- THE FIX is to delete that line, and nothing else. Every one of these already +-- upserts `timeline_event_stats`, which is the real home for counts +-- (like_count, dislike_count, comment_count, share_count, view_count) and what +-- the read path already reads. The bad line was a second, non-existent copy of +-- a number that was being stored correctly one line above — so removing it +-- loses nothing and restores the single source of truth. +-- +-- Bodies below are the LIVE definitions, taken from pg_get_functiondef, with +-- only that line (and its comment) removed. Reproducing them from the baseline +-- migration instead would risk reverting whatever else has been replaced since. + +CREATE OR REPLACE FUNCTION public.add_timeline_comment(p_event_id uuid, p_user_id uuid, p_content text, p_parent_comment_id uuid DEFAULT NULL::uuid) + RETURNS TABLE(comment_id uuid, comment_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_comment_id UUID; + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Insert the comment + INSERT INTO timeline_comments (event_id, user_id, content, parent_comment_id) + VALUES (p_event_id, p_user_id, p_content, p_parent_comment_id) + RETURNING id INTO v_comment_id; + + -- Get the updated count (only top-level comments) + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_comments + WHERE event_id = p_event_id AND is_deleted = false AND parent_comment_id IS NULL; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, comment_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET comment_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_comment_id, v_count; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.delete_timeline_comment(p_comment_id uuid, p_user_id uuid) + RETURNS boolean + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_event_id UUID; + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Get event_id before deleting + SELECT event_id INTO v_event_id + FROM timeline_comments WHERE id = p_comment_id AND user_id = p_user_id; + + IF v_event_id IS NULL THEN + RETURN FALSE; + END IF; + + -- Soft delete the comment + UPDATE timeline_comments + SET is_deleted = true, deleted_at = NOW() + WHERE id = p_comment_id AND user_id = p_user_id; + + -- Update count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_comments + WHERE event_id = v_event_id AND is_deleted = false AND parent_comment_id IS NULL; + + -- Update cached stats + UPDATE timeline_event_stats SET comment_count = v_count, updated_at = NOW() + WHERE event_id = v_event_id; + + + RETURN TRUE; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.dislike_timeline_event(p_event_id uuid, p_user_id uuid) + RETURNS TABLE(dislike_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Insert the dislike (ignore if already exists) + INSERT INTO timeline_dislikes (event_id, user_id) + VALUES (p_event_id, p_user_id) + ON CONFLICT (event_id, user_id) DO NOTHING; + + -- Remove any existing like + DELETE FROM timeline_likes + WHERE event_id = p_event_id AND user_id = p_user_id; + + -- Get the updated count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_dislikes WHERE event_id = p_event_id; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, dislike_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET dislike_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_count; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.like_timeline_event(p_event_id uuid, p_user_id uuid) + RETURNS TABLE(like_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Insert the like (ignore if already exists) + INSERT INTO timeline_likes (event_id, user_id) + VALUES (p_event_id, p_user_id) + ON CONFLICT (event_id, user_id) DO NOTHING; + + -- Remove any existing dislike + DELETE FROM timeline_dislikes + WHERE event_id = p_event_id AND user_id = p_user_id; + + -- Get the updated count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_likes WHERE event_id = p_event_id; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, like_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET like_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_count; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.undislike_timeline_event(p_event_id uuid, p_user_id uuid) + RETURNS TABLE(dislike_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Remove the dislike + DELETE FROM timeline_dislikes + WHERE event_id = p_event_id AND user_id = p_user_id; + + -- Get the updated count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_dislikes WHERE event_id = p_event_id; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, dislike_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET dislike_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_count; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.unlike_timeline_event(p_event_id uuid, p_user_id uuid) + RETURNS TABLE(like_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Remove the like + DELETE FROM timeline_likes + WHERE event_id = p_event_id AND user_id = p_user_id; + + -- Get the updated count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_likes WHERE event_id = p_event_id; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, like_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET like_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_count; +END; +$function$; + + +-- --------------------------------------------------------------------------- +-- Deleting a post has never worked either, for two independent reasons. +-- +-- One: the ownership check reads `SELECT user_id FROM profiles WHERE role = +-- 'admin'`. `profiles` has neither column — it keys on `id` and has no `role` +-- at all — so the IF raised 42703 on every call and no post was ever deleted. +-- The admin escape hatch it was reaching for has therefore never existed; +-- removing it takes away nothing that ever worked, and leaves the honest rule: +-- you can delete your own posts. A real moderator capability should be built +-- deliberately, against a table that exists, not inherited from a line that +-- always threw. +-- +-- Two, and this one would have bitten even after fixing the first: the function +-- set only `deleted_at`, while every read path filters on `is_deleted = false` +-- (services/timeline/queries/*). A post "deleted" this way would have stayed +-- fully visible. Confirmed against production: 102 events have both flags set, +-- and ZERO have `deleted_at` without `is_deleted` — nothing has ever taken this +-- path. Both flags are now set together, and the reason goes in the +-- `deletion_reason` COLUMN that already exists rather than being stuffed into +-- metadata beside it. +-- +-- The trailing "decrement the parent's reply_count" block is dropped: that +-- column does not exist either, so it never ran. +CREATE OR REPLACE FUNCTION public.soft_delete_timeline_event(event_id uuid, reason text DEFAULT NULL::text) + RETURNS boolean + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_actor_id UUID; + v_rows INTEGER := 0; +BEGIN + v_actor_id := auth.uid(); + + IF v_actor_id IS NULL THEN + RAISE EXCEPTION 'Authentication required'; + END IF; + + UPDATE timeline_events + SET is_deleted = TRUE, + deleted_at = NOW(), + deletion_reason = reason, + updated_at = NOW() + WHERE id = event_id + AND actor_id = v_actor_id + AND is_deleted IS NOT TRUE; + + GET DIAGNOSTICS v_rows = ROW_COUNT; + + IF v_rows = 0 THEN + -- Deleting something already deleted is the user's intent already + -- satisfied, not a failure to report. Only a post that is not theirs (or + -- does not exist) is an error. + IF EXISTS ( + SELECT 1 FROM timeline_events + WHERE id = event_id AND actor_id = v_actor_id + ) THEN + RETURN TRUE; + END IF; + RAISE EXCEPTION 'Event not found or access denied'; + END IF; + + RETURN TRUE; +END; +$function$; + +-- --------------------------------------------------------------------------- +-- A quote reply could not be created, and would have rendered blank if it had. +-- +-- `timeline_events.content` is jsonb and the function assigned the text +-- parameter straight into it, which plpgsql_check reports as "column content is +-- of type jsonb but expression is of type text". It also incremented +-- `quote_count`, another column that does not exist. +-- +-- The rendering half matters just as much: PostContent reads +-- `event.description`, and this function never wrote one. So a quote reply that +-- somehow succeeded would have appeared as an empty post — which is the shape +-- of "reposts look broken" rather than "reposts error". Both are written now, +-- `description` for the reader and `content` in the {text: …} shape the rest of +-- the timeline uses, so the two cannot disagree about what the post says. +CREATE OR REPLACE FUNCTION public.create_quote_reply(p_parent_event_id uuid, p_actor_id uuid, p_content text, p_quoted_content text, p_visibility text DEFAULT 'public'::text) + RETURNS uuid + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_new_event_id UUID; + v_parent_actor_id UUID; +BEGIN + IF auth.uid() != p_actor_id THEN + RAISE EXCEPTION 'Actor mismatch'; + END IF; + + SELECT actor_id INTO v_parent_actor_id + FROM timeline_events + WHERE id = p_parent_event_id + AND is_deleted IS NOT TRUE; + + IF v_parent_actor_id IS NULL THEN + RAISE EXCEPTION 'Parent event not found'; + END IF; + + -- `subject_type` is NOT NULL and was never supplied, so this INSERT could not + -- have succeeded on any input — a third failure, hiding behind the first two + -- and invisible to plpgsql_check, which does not evaluate constraints. Every + -- one of the 1,406 existing rows sets subject_type and subject_id; a post + -- authored by a person is ('profile', that person), which is what the + -- formatter already assumes when it defaults subjectType to 'profile'. + INSERT INTO timeline_events ( + event_type, + actor_id, + subject_type, + subject_id, + title, + description, + content, + parent_event_id, + visibility, + metadata, + created_at, + updated_at + ) VALUES ( + 'quote_reply', + p_actor_id, + 'profile', + p_actor_id, + 'Quoted a post', + p_content, + jsonb_build_object('text', p_content), + p_parent_event_id, + p_visibility, + jsonb_build_object( + 'quoted_content', p_quoted_content, + 'quoted_actor_id', v_parent_actor_id + ), + NOW(), + NOW() + ) + RETURNING id INTO v_new_event_id; + + -- The notification insert named four columns `notifications` does not have + -- (actor_id, title, data) and used a `type` its CHECK constraint forbids + -- ('quote_reply'). Only the first of those is visible to plpgsql_check — a + -- constraint violation is a runtime failure — so this had two more ways to + -- fail after the obvious one was fixed. + -- + -- The real shape is (user_id, type, message, metadata, is_read, created_at, + -- read_at, action_url). `comment` is the allowed type that means what this + -- means: somebody responded to your post. The quote-ness is kept in metadata + -- rather than invented as a new type, because adding a type means widening + -- the constraint AND teaching the notification UI to render it — a deliberate + -- change, not a side effect of repairing a broken function. + IF v_parent_actor_id != p_actor_id THEN + INSERT INTO notifications ( + user_id, + type, + message, + metadata, + action_url, + created_at + ) VALUES ( + v_parent_actor_id, + 'comment', + LEFT(p_content, 100), + jsonb_build_object( + 'kind', 'quote_reply', + 'event_id', v_new_event_id, + 'parent_event_id', p_parent_event_id, + 'actor_id', p_actor_id + ), + '/post/' || v_new_event_id::text, + NOW() + ) + ON CONFLICT DO NOTHING; + END IF; + + RETURN v_new_event_id; +END; +$function$; diff --git a/supabase/migrations/20260828120000_detect_functions_that_reference_nothing.sql b/supabase/migrations/20260828120000_detect_functions_that_reference_nothing.sql new file mode 100644 index 000000000..7691d2f16 --- /dev/null +++ b/supabase/migrations/20260828120000_detect_functions_that_reference_nothing.sql @@ -0,0 +1,81 @@ +-- Make "this function writes a column that does not exist" a thing we can see. +-- +-- Nineteen functions were in that state on 2026-08-28, and had been for as long +-- as anyone can tell. Likes, dislikes, replies, deleting a post and quote +-- replies were all dead in production; four AI-withdrawal functions and both +-- nearby-search functions are dead too. Every one of them looked fine: the +-- function exists, `\df` lists it, PostgREST routes to it, and the app calls it +-- happily. plpgsql only plans a statement when it RUNS, so a reference to a +-- column that was never added raises 42703 at call time and at no other moment. +-- +-- Nothing in the stack could have caught it. Unit tests mock the database. The +-- RPC-existence gate (scripts/check-rpc-exists.mjs) proves a function is +-- DEFINED, which every one of these was. Migration replay proves the SQL +-- applies, which it does — creating a function never validates its body. +-- Existence and correctness came apart, and only correctness matters. +-- +-- plpgsql_check reads every function body against the live schema and reports +-- exactly this. Exposed here as a COUNT over PostgREST, matching +-- count_email_derived_usernames, so the nightly data-invariant gate can ratchet +-- it down and never up. +-- +-- LIMITS, stated because a gate that is trusted beyond its reach is worse than +-- none. This is a STATIC check: it sees undefined columns, tables and type +-- mismatches. It does NOT evaluate constraints, so a NOT NULL column left +-- unset or a value outside a CHECK still fails only at runtime — both of which +-- create_quote_reply also did, hiding behind the static error until it was +-- fixed. Trigger functions are skipped because plpgsql_check needs a relation +-- to check them against. + +CREATE EXTENSION IF NOT EXISTS plpgsql_check; + +CREATE OR REPLACE FUNCTION public.count_broken_plpgsql_functions() +RETURNS bigint +LANGUAGE sql +SECURITY DEFINER +SET search_path TO 'public' +AS $$ + SELECT count(DISTINCT p.oid)::bigint + FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + JOIN pg_language l ON l.oid = p.prolang + CROSS JOIN LATERAL plpgsql_check_function_tb(p.oid) cf + WHERE n.nspname = 'public' + AND l.lanname = 'plpgsql' + AND p.prorettype <> 'trigger'::regtype + AND cf.level = 'error'; +$$; + +-- Same lock-down as the other invariant helpers: the list of broken functions +-- is a map of what is currently exploitable-by-accident, so it returns a number +-- and only service_role may ask. +REVOKE ALL ON FUNCTION public.count_broken_plpgsql_functions() FROM PUBLIC; +REVOKE ALL ON FUNCTION public.count_broken_plpgsql_functions() FROM anon, authenticated; +GRANT EXECUTE ON FUNCTION public.count_broken_plpgsql_functions() TO service_role; + +COMMENT ON FUNCTION public.count_broken_plpgsql_functions() IS + 'How many plpgsql functions reference a column, table or type that does not exist. Static only — constraints still fail at runtime. A ratchet for check-data-invariants.mjs: it may fall or hold, never rise.'; + +-- The names, for a human fixing them. Kept separate from the count so the gate +-- can stay a number while a person can still ask "which ones?". +CREATE OR REPLACE FUNCTION public.list_broken_plpgsql_functions() +RETURNS TABLE(function_name text, problem text) +LANGUAGE sql +SECURITY DEFINER +SET search_path TO 'public' +AS $$ + SELECT p.proname::text, cf.message::text + FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + JOIN pg_language l ON l.oid = p.prolang + CROSS JOIN LATERAL plpgsql_check_function_tb(p.oid) cf + WHERE n.nspname = 'public' + AND l.lanname = 'plpgsql' + AND p.prorettype <> 'trigger'::regtype + AND cf.level = 'error' + ORDER BY 1; +$$; + +REVOKE ALL ON FUNCTION public.list_broken_plpgsql_functions() FROM PUBLIC; +REVOKE ALL ON FUNCTION public.list_broken_plpgsql_functions() FROM anon, authenticated; +GRANT EXECUTE ON FUNCTION public.list_broken_plpgsql_functions() TO service_role;