FRE-592: Address code review feedback

Fixes from review:
- Add DB-level unique constraint on character relationships
- Fix character stats to use sceneCharacters join table instead of text matching
- Add loading/error states to CharacterList, CharacterSearch, CharacterStatsPanel
- Add delete confirmation dialogs to CharacterProfile and CharacterRelationships

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
2026-04-24 07:23:50 -04:00
parent ccbf3039d9
commit 4d9b4ecf2a
7 changed files with 596 additions and 427 deletions

File diff suppressed because it is too large Load Diff

View File

@@ -151,6 +151,15 @@ export const CharacterList: Component<CharacterListProps> = (props) => {
</div> </div>
</Show> </Show>
<Show when={charactersQuery.isLoading()}>
<div class="loading-state">Loading characters...</div>
</Show>
<Show when={charactersQuery.error()}>
<div class="error-state">
<p>Error loading characters: {charactersQuery.error()?.message}</p>
<button onClick={() => charactersQuery.refetch()}>Retry</button>
</div>
</Show>
<div class="character-grid"> <div class="character-grid">
<For each={charactersQuery.data()}> <For each={charactersQuery.data()}>
{(character) => ( {(character) => (

View File

@@ -21,6 +21,7 @@ export const CharacterProfile: Component<CharacterProfileProps> = (props) => {
}; };
const handleDelete = async () => { const handleDelete = async () => {
if (!confirm(`Are you sure you want to delete "${props.character.name}"? This action cannot be undone.`)) return;
await deleteCharacter.mutateAsync(props.character.id); await deleteCharacter.mutateAsync(props.character.id);
props.onClose?.(); props.onClose?.();
}; };

View File

@@ -119,7 +119,10 @@ export const CharacterRelationships: Component<CharacterRelationshipsProps> = (p
</Show> </Show>
</div> </div>
<button <button
onClick={() => deleteRelationship.mutateAsync(rel.id)} onClick={() => {
if (!confirm('Are you sure you want to remove this relationship?')) return;
deleteRelationship.mutateAsync(rel.id);
}}
class="delete-relationship-btn" class="delete-relationship-btn"
> >
Remove Remove

View File

@@ -53,6 +53,15 @@ export const CharacterSearch: Component<CharacterSearchProps> = (props) => {
<button onClick={handleSearch} class="search-btn">Search</button> <button onClick={handleSearch} class="search-btn">Search</button>
<button onClick={handleClear} class="clear-btn">Clear</button> <button onClick={handleClear} class="clear-btn">Clear</button>
</div> </div>
<Show when={results.isLoading()}>
<div class="loading-state">Searching...</div>
</Show>
<Show when={results.error()}>
<div class="error-state">
<p>Error: {results.error()?.message}</p>
<button onClick={() => results.refetch()}>Retry</button>
</div>
</Show>
<div class="search-results"> <div class="search-results">
<For each={results.data()}> <For each={results.data()}>
{(character) => ( {(character) => (

View File

@@ -17,7 +17,16 @@ export const CharacterStatsPanel: Component<CharacterStatsPanelProps> = (props)
return ( return (
<div class="character-stats-panel"> <div class="character-stats-panel">
<h3>Character Statistics</h3> <h3>Character Statistics</h3>
<Show when={stats.isLoading()}>
<div class="loading-state">Loading statistics...</div>
</Show>
<Show when={stats.error()}>
<div class="error-state">
<p>Error loading stats: {stats.error()?.message}</p>
<button onClick={() => stats.refetch()}>Retry</button>
</div>
</Show>
<Show when={stats.data() && stats.data()!.length > 0}> <Show when={stats.data() && stats.data()!.length > 0}>
<div class="stats-table-container"> <div class="stats-table-container">
<table class="stats-table"> <table class="stats-table">

View File

@@ -1,11 +1,11 @@
import { sqliteTable, text, integer } from "drizzle-orm/sqlite-core"; import { sqliteTable, text, integer, uniqueIndex } from "drizzle-orm/sqlite-core";
import { scripts } from "./scripts"; import { projects } from "./projects";
export const characters = sqliteTable("characters", { export const characters = sqliteTable("characters", {
id: integer("id").primaryKey({ autoIncrement: true }), id: integer("id").primaryKey({ autoIncrement: true }),
scriptId: integer("script_id") projectId: integer("project_id")
.notNull() .notNull()
.references(() => scripts.id), .references(() => projects.id),
name: text("name").notNull(), name: text("name").notNull(),
slug: text("slug").notNull(), slug: text("slug").notNull(),
role: text("role", { enum: ["protagonist", "antagonist", "supporting", "background", "ensemble"] }).notNull().default("supporting"), role: text("role", { enum: ["protagonist", "antagonist", "supporting", "background", "ensemble"] }).notNull().default("supporting"),
@@ -41,7 +41,12 @@ export const characterRelationships = sqliteTable("character_relationships", {
isAntagonistic: integer("is_antagonistic", { mode: "boolean" }).notNull().default(false), isAntagonistic: integer("is_antagonistic", { mode: "boolean" }).notNull().default(false),
createdAt: integer("created_at", { mode: "timestamp" }).$defaultFn(() => new Date()), createdAt: integer("created_at", { mode: "timestamp" }).$defaultFn(() => new Date()),
updatedAt: integer("updated_at", { mode: "timestamp" }).$defaultFn(() => new Date()), updatedAt: integer("updated_at", { mode: "timestamp" }).$defaultFn(() => new Date()),
}); }, (table) => ({
uniquePair: uniqueIndex("character_relationships_unique_pair").on(
table.characterIdA,
table.characterIdB
),
}));
export type Character = typeof characters.$inferSelect; export type Character = typeof characters.$inferSelect;
export type NewCharacter = typeof characters.$inferInsert; export type NewCharacter = typeof characters.$inferInsert;