Make song ratings final after submission

src/lib/components/Valoracion.svelte

@@ -16,14 +16,13 @@
 	import { page } from '$app/state';
 
 	interface Props {
-		cancionSlug: string;
 		/** Media de 1 a 5, o `null` si todavía no ha votado nadie. */
 		media: number | null;
 		/** Lo que puso quien mira, si entró y votó. */
 		mia?: number;
 	}
 
-	let { cancionSlug, media, mia }: Props = $props();
+	let { media, mia }: Props = $props();
 
 	const ESTRELLAS = [1, 2, 3, 4, 5];
 
@@ -32,32 +31,25 @@
 
 	/** Se envía mientras vuela la respuesta, para que la elección no parpadee. */
 	let enviando = $state<number | undefined>(undefined);
-	/**
+	/** La acción confirmada, ligada a su ruta para no arrastrarla a otro tema. */
-	 * Último voto que la acción confirmó en esta visita.
+	let confirmada = $state<{ ruta: string; puntuacion: number } | undefined>(undefined);
-	 *
+	const propia = $derived(confirmada?.ruta === volverA ? confirmada.puntuacion : mia);
-	 * No se depende solo de que el `load` vuelva a traer `mia`: entre la
+	const elegida = $derived(enviando ?? propia);
-	 * respuesta de la acción y esa recarga el radio regresaba al valor anterior,
-	 * y daba la impresión —además de dejar el control en un estado incoherente—
-	 * de que un voto ya emitido no se podía modificar.
-	 */
-	let confirmada = $state<{ cancionSlug: string; puntuacion: number } | undefined>(undefined);
-	const elegida = $derived(
-		enviando ?? (confirmada?.cancionSlug === cancionSlug ? confirmada.puntuacion : undefined) ?? mia
-	);
 </script>
 
 <section class="valoracion" aria-label="Valoración de la canción">
-	{#if usuario}
+	{#if usuario && propia === undefined}
 		<form
 			method="POST"
 			action="?/valorar"
 			use:enhance={({ formData }) => {
-				const puntuacion = Number(formData.get('puntuacion'));
+				enviando = Number(formData.get('puntuacion'));
-				enviando = puntuacion;
 				return async ({ result, update }) => {
 					try {
 						await update({ reset: false });
-						if (result.type === 'success') confirmada = { cancionSlug, puntuacion };
+						if (result.type === 'success') {
+							confirmada = { ruta: volverA, puntuacion: Number(formData.get('puntuacion')) };
+						}
 					} finally {
 						enviando = undefined;
 					}
@@ -83,6 +75,12 @@
 				</div>
 			</fieldset>
 		</form>
+	{:else if usuario && propia !== undefined}
+		<div class="estrellas estrellas-lectura" role="img" aria-label="Tu valoración: {propia} de 5">
+			{#each ESTRELLAS as estrella (estrella)}
+				<span class:llena={estrella <= propia} aria-hidden="true">★</span>
+			{/each}
+		</div>
 	{:else}
 		<a
 			class="estrellas estrellas-lectura"

src/lib/components/Valoracion.svelte.spec.ts

@@ -18,12 +18,6 @@ type AlEnviar = (entrada: {
 	update: (opciones: { reset: boolean }) => Promise<void>;
 }) => Promise<void>;
 
-/*
- * El componente solo necesita aquí el contrato de mejora progresiva: capturar
- * el formulario, ejecutar su callback y confirmar una respuesta correcta. La
- * persistencia se comprueba en el módulo del servidor; esta prueba protege el
- * estado que ve y puede volver a pulsar la persona.
- */
 vi.mock('$app/forms', () => ({
 	enhance(formulario: HTMLFormElement, alEnviar: AlEnviar) {
 		const enviar = async (evento: SubmitEvent) => {
@@ -46,23 +40,20 @@ vi.mock('$app/forms', () => ({
 }));
 
 describe('Valoracion', () => {
-	it('permite reemplazar una valoración ya emitida', async () => {
+	it('convierte el primer voto confirmado en una valoración de solo lectura', async () => {
-		const vista = render(Valoracion, { cancionSlug: 'prometiste', media: 2, mia: 2 });
+		render(Valoracion, { media: null });
 
-		const dos = page.getByRole('radio', { name: '2 de 5' });
 		const cuatro = page.getByRole('radio', { name: '4 de 5' });
-		await expect.element(dos).toBeChecked();
-
 		await cuatro.click();
 
-		await expect.element(cuatro).toBeChecked();
+		await expect.element(page.getByRole('radio')).not.toBeInTheDocument();
-		await expect.element(dos).not.toBeChecked();
+		await expect.element(page.getByRole('img', { name: 'Tu valoración: 4 de 5' })).toBeVisible();
-		await expect.element(cuatro).toBeEnabled();
+	});
+
+	it('no ofrece controles para modificar una valoración existente', async () => {
+		render(Valoracion, { media: 3.5, mia: 2 });
 
-		// La confirmación local pertenece a ese tema, no al componente reutilizado
+		await expect.element(page.getByRole('radio')).not.toBeInTheDocument();
-		// al navegar a otra ficha de canción.
+		await expect.element(page.getByRole('img', { name: 'Tu valoración: 2 de 5' })).toBeVisible();
-		await vista.rerender({ cancionSlug: 'otra-cancion', media: 1, mia: 1 });
-		await expect.element(page.getByRole('radio', { name: '1 de 5' })).toBeChecked();
-		await expect.element(cuatro).not.toBeChecked();
 	});
 });

src/lib/server/db/schema/interaccion.ts

@@ -120,8 +120,8 @@ export const accesoCancionUsuario = pgTable(
  * Lo que una cuenta le pone a un tema, de 1 a 5.
  *
  * La clave primaria es el par cuenta-tema, asi que nadie puede votar dos veces
- * lo mismo: volver a votar reemplaza el voto anterior. Eso lo garantiza la base
+ * lo mismo. La inserción ignora el conflicto en vez de actualizarlo: una
- * de datos, no el codigo de la accion, que es donde tiene que estar.
+ * valoración emitida es definitiva.
  *
  * El indice por `cancion_id` es para la media: se calcula por tema, y sin el
  * habria que recorrer la tabla entera en cada ficha.

src/lib/server/valoraciones-persistencia.spec.ts

@@ -0,0 +1,50 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const dobles = vi.hoisted(() => {
+	const returning = vi.fn();
+	const onConflictDoNothing = vi.fn(() => ({ returning }));
+	const values = vi.fn(() => ({ onConflictDoNothing }));
+	const insert = vi.fn(() => ({ values }));
+	const idDeCancion = vi.fn();
+	return { returning, onConflictDoNothing, values, insert, idDeCancion };
+});
+
+vi.mock('./db', () => ({ db: { insert: dobles.insert } }));
+vi.mock('./canciones', () => ({ idDeCancion: dobles.idDeCancion }));
+vi.mock('./db/schema', () => ({
+	cancion: { id: 'cancion.id', slug: 'cancion.slug' },
+	valoracion: {
+		usuarioId: 'valoracion.usuarioId',
+		cancionId: 'valoracion.cancionId',
+		puntuacion: 'valoracion.puntuacion'
+	}
+}));
+
+import { valorar } from './valoraciones';
+
+describe('persistencia de valoraciones', () => {
+	beforeEach(() => {
+		vi.clearAllMocks();
+		dobles.idDeCancion.mockResolvedValue('cancion-1');
+	});
+
+	it('inserta el primer voto pero no actualiza uno existente', async () => {
+		dobles.returning.mockResolvedValueOnce([{ puntuacion: 4 }]).mockResolvedValueOnce([]);
+
+		await expect(valorar('usuario-1', 'prometiste', 4)).resolves.toBe('guardada');
+		await expect(valorar('usuario-1', 'prometiste', 2)).resolves.toBe('ya-valorada');
+		expect(dobles.onConflictDoNothing).toHaveBeenCalledTimes(2);
+		expect(dobles.values).toHaveBeenNthCalledWith(2, {
+			usuarioId: 'usuario-1',
+			cancionId: 'cancion-1',
+			puntuacion: 2
+		});
+	});
+
+	it('no intenta guardar el voto de una canción inexistente', async () => {
+		dobles.idDeCancion.mockResolvedValue(undefined);
+
+		await expect(valorar('usuario-1', 'no-existe', 4)).resolves.toBe('cancion-inexistente');
+		expect(dobles.insert).not.toHaveBeenCalled();
+	});
+});

src/lib/server/valoraciones.ts

@@ -1,10 +1,9 @@
 /**
  * Valoraciones de un tema.
  *
- * Solo valora quien tiene cuenta, y una sola vez por tema: volver a votar
+ * Solo valora quien tiene cuenta, y una sola vez por tema. El voto es
- * reemplaza el voto anterior en lugar de sumar otro. Eso lo impone la clave
+ * definitivo: la clave primaria compuesta impide duplicarlo y la inserción no
- * primaria compuesta de la tabla, no este módulo; aquí solo se escribe de
+ * actualiza la fila cuando ya existe.
- * forma que la base de datos pueda hacer su trabajo.
  *
  * No hace falta haber comprado el tema para valorarlo: la web permite
  * escucharlo completo a calidad reducida. Exigir la compra convertiría la
@@ -85,40 +84,29 @@ export async function valoracionDe(
 }
 
 /**
- * Guarda o reemplaza el voto de una cuenta.
+ * Guarda el voto definitivo de una cuenta.
  *
- * Es un `upsert` sobre la clave primaria: si ya había voto, se pisa y se anota
+ * El conflicto no actualiza: así esta regla no depende de que la interfaz
- * cuándo. La fecha de creación no se toca, para poder distinguir un voto viejo
+ * esconda los radios y también se cumple ante dos peticiones simultáneas o un
- * corregido de uno nuevo.
+ * POST construido a mano.
  */
 export async function valorar(
 	usuarioId: string,
 	cancionSlug: string,
 	puntuacion: number
-): Promise<boolean> {
+): Promise<'guardada' | 'ya-valorada' | 'cancion-inexistente'> {
 	if (!esPuntuacionValida(puntuacion)) {
 		throw new Error(`Puntuación fuera de rango: ${puntuacion}`);
 	}
 
 	const cancionId = await idDeCancion(cancionSlug);
-	if (!cancionId) return false;
+	if (!cancionId) return 'cancion-inexistente';
 
-	await db
+	const [insertada] = await db
 		.insert(valoracion)
 		.values({ usuarioId, cancionId, puntuacion })
-		.onConflictDoUpdate({
+		.onConflictDoNothing({ target: [valoracion.usuarioId, valoracion.cancionId] })
-			target: [valoracion.usuarioId, valoracion.cancionId],
+		.returning({ puntuacion: valoracion.puntuacion });
-			set: { puntuacion, actualizadaEn: new Date() }
-		});
-	return true;
-}
-
-/** Retira el voto de una cuenta. Sin voto, el tema vuelve a su media sin él. */
-export async function retirarValoracion(usuarioId: string, cancionSlug: string): Promise<void> {
-	const cancionId = await idDeCancion(cancionSlug);
-	if (!cancionId) return;
 
-	await db
+	return insertada ? 'guardada' : 'ya-valorada';
-		.delete(valoracion)
-		.where(and(eq(valoracion.usuarioId, usuarioId), eq(valoracion.cancionId, cancionId)));
 }

src/routes/(sitio)/canciones/[slug]/+page.server.ts

@@ -8,13 +8,7 @@ import {
 	portadaDeCancion
 } from '$lib/server/catalogo';
 import { componer } from '$lib/server/markdown';
-import {
+import { aPuntuacion, resumenDeCancion, valoracionDe, valorar } from '$lib/server/valoraciones';
-	aPuntuacion,
-	resumenDeCancion,
-	retirarValoracion,
-	valoracionDe,
-	valorar
-} from '$lib/server/valoraciones';
 import {
 	alternarCancionEnLista,
 	crearListaConCancion,
@@ -74,29 +68,19 @@ export const actions = {
 		if (!locals.usuario) {
 			return fail(401, { valoracion: { error: 'Entra con tu cuenta para valorar.' } });
 		}
-		// Un slug que no existe no se valora. Desde la fase 3 la tabla tiene
-		// clave foránea contra `cancion`, así que esto es lo que convierte un
-		// error de base de datos en un 404 con su mensaje.
-		if (!(await obtenerCancion(params.slug))) {
-			return fail(404, { valoracion: { error: 'Ese tema no existe.' } });
-		}
-
 		const puntuacion = aPuntuacion((await request.formData()).get('puntuacion'));
 		if (puntuacion === undefined) {
 			return fail(400, { valoracion: { error: 'La valoración va de 1 a 5.' } });
 		}
 
-		await valorar(locals.usuario.id, params.slug, puntuacion);
+		const resultado = await valorar(locals.usuario.id, params.slug, puntuacion);
-		return { valoracion: { puntuacion } };
+		if (resultado === 'cancion-inexistente') {
-	},
+			return fail(404, { valoracion: { error: 'Ese tema no existe.' } });
-
-	retirar: async ({ params, locals }) => {
-		if (!locals.usuario) {
-			return fail(401, { valoracion: { error: 'Entra con tu cuenta para valorar.' } });
 		}
-
+		if (resultado === 'ya-valorada') {
-		await retirarValoracion(locals.usuario.id, params.slug);
+			return fail(409, { valoracion: { error: 'Ya has valorado este tema.' } });
-		return { valoracion: { puntuacion: undefined } };
+		}
+		return { valoracion: { puntuacion } };
 	},
 
 	alternarLista: async ({ params, request, locals }) => {

src/routes/(sitio)/canciones/[slug]/+page.svelte

@@ -159,11 +159,7 @@
 			{/if}
 
 			<div class="valoracion-superior">
-				<Valoracion
+				<Valoracion media={data.valoracion.media} mia={data.miValoracion} />
-					cancionSlug={cancion.slug}
-					media={data.valoracion.media}
-					mia={data.miValoracion}
-				/>
 			</div>
 
 			<!--