feat(artist): editable display name + rename surface; drop name-uniqueness (#130 step 1)
First step of decoupling artist identity/storage/display. migration 0077 drops uq_artist_name so the display name is free text (two genuinely different creators can share a name); the slug stays the immutable, unique storage/identity key (the on-disk path component — untouched, so nothing moves). ArtistService.rename + PATCH /api/artists/<id> change the name ONLY. Frontend: inline pencil-edit on the artist header (mirrors TagCard), slug/route unaffected so no navigation. Fixes the operator's 'no surface to rename an artist' + the name-collision fragility. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDgx8bQS5YrGRK76v8HUnM
This commit is contained in:
@@ -0,0 +1,32 @@
|
||||
"""drop uq_artist_name — decouple display name from identity/storage
|
||||
|
||||
Revision ID: 0077
|
||||
Revises: 0076
|
||||
Create Date: 2026-07-04
|
||||
|
||||
Artist model fragility fix (milestone #130). One `slug` column was doing
|
||||
identity + storage-path + display, and BOTH `name` and `slug` were UNIQUE, so
|
||||
the display name couldn't be edited freely and two genuinely different creators
|
||||
collided. Decouple: `slug` stays the immutable, unique storage/identity key (the
|
||||
on-disk path component — untouched here); `name` becomes freely editable, NON-
|
||||
unique display text. This migration only drops the `uq_artist_name` constraint;
|
||||
no data moves and no path changes.
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
|
||||
revision: str = "0077"
|
||||
down_revision: Union[str, None] = "0076"
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.drop_constraint("uq_artist_name", "artist", type_="unique")
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
# Re-adding the UNIQUE would fail if duplicate names now exist; callers that
|
||||
# need to reverse this must dedupe names first.
|
||||
op.create_unique_constraint("uq_artist_name", "artist", ["name"])
|
||||
@@ -31,6 +31,24 @@ async def create_or_get():
|
||||
}), 201
|
||||
|
||||
|
||||
@artists_bp.route("/<int:artist_id>", methods=["PATCH"])
|
||||
async def rename(artist_id: int):
|
||||
"""Rename an artist's DISPLAY NAME (#130). Name only — the slug and every
|
||||
on-disk path stay put, so this is instant and safe. Name is non-unique."""
|
||||
body = await request.get_json()
|
||||
if not isinstance(body, dict) or not isinstance(body.get("name"), str):
|
||||
return jsonify({"error": "invalid_body"}), 400
|
||||
async with get_session() as session:
|
||||
svc = ArtistService(session)
|
||||
try:
|
||||
artist = await svc.rename(artist_id, body["name"])
|
||||
except ValueError as exc:
|
||||
return jsonify({"error": "empty_name", "detail": str(exc)}), 400
|
||||
if artist is None:
|
||||
return jsonify({"error": "not_found"}), 404
|
||||
return jsonify({"id": artist.id, "name": artist.name, "slug": artist.slug})
|
||||
|
||||
|
||||
@artists_bp.route("/autocomplete", methods=["GET"])
|
||||
async def autocomplete():
|
||||
q = request.args.get("q") or ""
|
||||
|
||||
@@ -15,7 +15,14 @@ class Artist(Base):
|
||||
__tablename__ = "artist"
|
||||
|
||||
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
||||
name: Mapped[str] = mapped_column(String(255), nullable=False, unique=True)
|
||||
# Display name: freely editable, NON-unique (two real creators can share a
|
||||
# name). Decoupled from identity/storage in migration 0077 (#130) — renaming
|
||||
# touches ONLY this. Was unique until then.
|
||||
name: Mapped[str] = mapped_column(String(255), nullable=False)
|
||||
# Storage/identity key: IMMUTABLE + unique. This is the on-disk path
|
||||
# component (download_service artist_slug = artist.slug → images_root/<slug>/
|
||||
# <platform>/…), so it is set once at creation (collision-suffixed) and NEVER
|
||||
# changes — a rename must not move files. Existing artists keep their slug.
|
||||
slug: Mapped[str] = mapped_column(String(255), nullable=False, unique=True)
|
||||
notes: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||
|
||||
|
||||
@@ -283,6 +283,23 @@ class ArtistService:
|
||||
await self.session.commit()
|
||||
return artist, created
|
||||
|
||||
async def rename(self, artist_id: int, name: str) -> Artist | None:
|
||||
"""Change the display NAME only (#130). The slug — and every on-disk path
|
||||
keyed off it — is IMMUTABLE, so a rename never moves files or risks a path
|
||||
collision. Name is free text and NON-unique (migration 0077). Returns the
|
||||
updated Artist, None if not found; raises ValueError on empty name."""
|
||||
cleaned = (name or "").strip()
|
||||
if not cleaned:
|
||||
raise ValueError("artist name must not be empty")
|
||||
artist = (await self.session.execute(
|
||||
select(Artist).where(Artist.id == artist_id)
|
||||
)).scalar_one_or_none()
|
||||
if artist is None:
|
||||
return None
|
||||
artist.name = cleaned
|
||||
await self.session.commit()
|
||||
return artist
|
||||
|
||||
async def autocomplete(self, prefix: str, limit: int = 20) -> list[Artist]:
|
||||
cleaned = (prefix or "").strip()
|
||||
if not cleaned:
|
||||
|
||||
@@ -1,8 +1,21 @@
|
||||
<template>
|
||||
<header class="fc-artist-header">
|
||||
<div class="fc-artist-header__left">
|
||||
<h1 class="fc-artist-header__name">{{ name }}</h1>
|
||||
<span v-if="stats" class="fc-artist-header__stats">{{ stats }}</span>
|
||||
<template v-if="!editing">
|
||||
<h1 class="fc-artist-header__name">{{ name }}</h1>
|
||||
<v-icon
|
||||
class="fc-artist-header__edit" size="16" icon="mdi-pencil"
|
||||
title="Rename artist (display name only)" @click="startEdit"
|
||||
/>
|
||||
</template>
|
||||
<v-text-field
|
||||
v-else
|
||||
v-model="draft"
|
||||
density="compact" variant="outlined" hide-details autofocus
|
||||
class="fc-artist-header__edit-field"
|
||||
@keydown.enter="submit" @keydown.esc="cancel" @blur="cancel"
|
||||
/>
|
||||
<span v-if="stats && !editing" class="fc-artist-header__stats">{{ stats }}</span>
|
||||
</div>
|
||||
<v-tabs
|
||||
:model-value="modelValue"
|
||||
@@ -34,7 +47,7 @@
|
||||
</template>
|
||||
|
||||
<script setup>
|
||||
import { computed } from 'vue'
|
||||
import { computed, ref } from 'vue'
|
||||
import { formatLocalDate } from '../../utils/date.js'
|
||||
|
||||
const props = defineProps({
|
||||
@@ -45,7 +58,23 @@ const props = defineProps({
|
||||
modelValue: { type: String, required: true },
|
||||
})
|
||||
|
||||
defineEmits(['update:modelValue'])
|
||||
const emit = defineEmits(['update:modelValue', 'rename'])
|
||||
|
||||
// Inline rename (display name only — #130). Mirrors TagCard's pencil-edit.
|
||||
const editing = ref(false)
|
||||
const draft = ref('')
|
||||
function startEdit () {
|
||||
draft.value = props.name
|
||||
editing.value = true
|
||||
}
|
||||
function cancel () {
|
||||
editing.value = false
|
||||
}
|
||||
function submit () {
|
||||
const next = draft.value.trim()
|
||||
editing.value = false
|
||||
if (next && next !== props.name) emit('rename', next)
|
||||
}
|
||||
|
||||
const stats = computed(() => {
|
||||
const parts = []
|
||||
@@ -111,6 +140,20 @@ const stats = computed(() => {
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
|
||||
.fc-artist-header__edit {
|
||||
flex: 0 0 auto;
|
||||
opacity: 0;
|
||||
cursor: pointer;
|
||||
color: rgb(var(--v-theme-on-surface-variant));
|
||||
transition: opacity .15s ease;
|
||||
}
|
||||
.fc-artist-header__left:hover .fc-artist-header__edit { opacity: .7; }
|
||||
.fc-artist-header__edit:hover { opacity: 1; }
|
||||
.fc-artist-header__edit-field {
|
||||
max-width: 340px;
|
||||
font-family: 'Fraunces', Georgia, serif;
|
||||
}
|
||||
|
||||
.fc-artist-header__tabs {
|
||||
flex: 0 0 auto;
|
||||
}
|
||||
|
||||
@@ -73,6 +73,16 @@ export const useArtistStore = defineStore('artist', () => {
|
||||
}
|
||||
}
|
||||
|
||||
// Rename the DISPLAY name only (#130). The slug is immutable, so the route
|
||||
// (slug-based) and on-disk paths are unaffected — just patch the shown name.
|
||||
async function rename (name) {
|
||||
if (!overview.value) return
|
||||
const id = overview.value.id
|
||||
const body = await api.patch(`/api/artists/${id}`, { body: { name } })
|
||||
if (overview.value && overview.value.id === id) overview.value.name = body.name
|
||||
return body
|
||||
}
|
||||
|
||||
const hasMoreImages = computed(() => !started || nextCursor.value !== null)
|
||||
const postCount = computed(() => overview.value?.post_count ?? null)
|
||||
const imageCount = computed(() => overview.value?.image_count ?? null)
|
||||
@@ -81,6 +91,6 @@ export const useArtistStore = defineStore('artist', () => {
|
||||
return {
|
||||
overview, images, loading, imagesLoading, error, notFound,
|
||||
hasMoreImages, postCount, imageCount, lastAdded,
|
||||
load, loadMoreImages,
|
||||
load, loadMoreImages, rename,
|
||||
}
|
||||
})
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
:image-count="store.imageCount"
|
||||
:post-count="store.postCount"
|
||||
:last-added="store.lastAdded"
|
||||
@rename="onRename"
|
||||
/>
|
||||
<v-container fluid class="pt-2 pb-4">
|
||||
<!-- "N new since last visit" banner. Visible only on the initial
|
||||
@@ -59,6 +60,7 @@ import { computed, ref, watch } from 'vue'
|
||||
import { useRoute } from 'vue-router'
|
||||
|
||||
import { useArtistStore } from '../stores/artist.js'
|
||||
import { toast } from '../utils/toast.js'
|
||||
import ArtistHeader from '../components/artist/ArtistHeader.vue'
|
||||
import ArtistPostsTab from '../components/artist/ArtistPostsTab.vue'
|
||||
import ArtistGalleryTab from '../components/artist/ArtistGalleryTab.vue'
|
||||
@@ -76,6 +78,17 @@ const { tab, resolve } = useTabQuery(
|
||||
() => ((store.postCount ?? 0) > 0 ? 'posts' : 'gallery'),
|
||||
)
|
||||
|
||||
// Rename the artist's display name (#130). Slug/route unchanged, so no
|
||||
// navigation — the header just reflects the new name; also refresh the tab title.
|
||||
async function onRename (name) {
|
||||
try {
|
||||
await store.rename(name)
|
||||
document.title = `${store.overview.name} — FabledCurator`
|
||||
} catch (e) {
|
||||
toast({ text: `Rename failed: ${e.message}`, type: 'error' })
|
||||
}
|
||||
}
|
||||
|
||||
// One-shot banner — reset on each new artist-slug load so it re-appears
|
||||
// when navigating between artists that each have unseen content.
|
||||
const unseenBanner = ref(false)
|
||||
|
||||
@@ -47,3 +47,28 @@ async def test_autocomplete_empty_query(client):
|
||||
resp = await client.get("/api/artists/autocomplete?q=")
|
||||
assert resp.status_code == 200
|
||||
assert await resp.get_json() == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_patch_renames_display_name_only(client):
|
||||
# #130: PATCH renames the display name; slug unchanged (paths safe).
|
||||
created = await (await client.post("/api/artists", json={"name": "12345678"})).get_json()
|
||||
resp = await client.patch(
|
||||
f"/api/artists/{created['id']}", json={"name": "Kurotsuchi Machi"}
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
body = await resp.get_json()
|
||||
assert body["name"] == "Kurotsuchi Machi"
|
||||
assert body["slug"] == created["slug"] # slug frozen
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_patch_rename_validation(client):
|
||||
created = await (await client.post("/api/artists", json={"name": "Nn"})).get_json()
|
||||
assert (await client.patch(
|
||||
f"/api/artists/{created['id']}", json={"name": " "}
|
||||
)).status_code == 400
|
||||
assert (await client.patch(
|
||||
f"/api/artists/{created['id']}", json={})).status_code == 400
|
||||
assert (await client.patch(
|
||||
"/api/artists/999999", json={"name": "Ghost"})).status_code == 404
|
||||
|
||||
@@ -54,3 +54,40 @@ async def test_autocomplete_ranks_exact_prefix_substring(db):
|
||||
async def test_autocomplete_empty_query_returns_empty(db):
|
||||
svc = ArtistService(db)
|
||||
assert await svc.autocomplete("") == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rename_changes_name_not_slug(db):
|
||||
# #130: rename touches the display name ONLY — the slug (and every on-disk
|
||||
# path keyed off it) is immutable.
|
||||
svc = ArtistService(db)
|
||||
artist, _ = await svc.find_or_create("12345678")
|
||||
orig_slug = artist.slug
|
||||
renamed = await svc.rename(artist.id, "Kurotsuchi Machi")
|
||||
assert renamed.name == "Kurotsuchi Machi"
|
||||
assert renamed.slug == orig_slug # slug frozen
|
||||
fresh = await db.get(Artist, artist.id)
|
||||
assert fresh.name == "Kurotsuchi Machi"
|
||||
assert fresh.slug == orig_slug
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rename_rejects_empty_and_missing(db):
|
||||
svc = ArtistService(db)
|
||||
artist, _ = await svc.find_or_create("Someone")
|
||||
with pytest.raises(ValueError):
|
||||
await svc.rename(artist.id, " ")
|
||||
assert await svc.rename(999999, "Ghost") is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_two_artists_can_share_a_display_name(db):
|
||||
# #130: name is non-unique now (two genuinely different creators can share a
|
||||
# display name); the immutable slug keeps them distinct.
|
||||
a = Artist(name="Same Name", slug="same-name")
|
||||
b = Artist(name="Same Name", slug="same-name-2")
|
||||
db.add_all([a, b])
|
||||
await db.flush()
|
||||
assert a.id != b.id
|
||||
assert a.name == b.name
|
||||
assert a.slug != b.slug
|
||||
|
||||
Reference in New Issue
Block a user