# Plan: 10 deudas técnicas — lote 5

> Fecha: 2026-06-12
> Criterio: ROI = gravedad / coste. ALTA×BAJO (score 3) > MEDIA×BAJO (score 2) > ALTA×MEDIO (score 1.5)
> Cubre: DT-051 a DT-060
> Estado: PENDIENTE

---

## Selección y orden de implementación

| # | ID | Gravedad | Coste | Score | Descripción |
|---|-----|----------|-------|-------|-------------|
| 1 | DT-051 | ALTA | BAJO | 3.0 | `Api\EventoController::votar()` duplica `VotacionService` |
| 2 | DT-052 | ALTA | BAJO | 3.0 | `RankingService::crearSnapshot()` 2 flush fuera de transacción |
| 3 | DT-053 | MEDIA | BAJO | 2.0 | `ColeccionService::agregarJuego()` `findOneBy()` siempre inútil |
| 4 | DT-054 | MEDIA | BAJO | 2.0 | `Api\GrupoController::show()` 3 queries para la misma membresía |
| 5 | DT-055 | MEDIA | BAJO | 2.0 | `VotacionService::puntuarJuego()` hasta 3 flush sin transacción |
| 6 | DT-056 | MEDIA | BAJO | 2.0 | `MailService::$dynamicMailer` cacheado sin invalidación |
| 7 | DT-057 | MEDIA | BAJO | 2.0 | `TelegramService::llamarApi()` URL hardcodeada |
| 8 | DT-058 | BAJA | BAJO | 1.0 | `GrupoController::editar()` bypassa `GrupoPermisoService` |
| 9 | DT-059 | ALTA | MEDIO | 1.5 | `Api\EventoController::serializeEventoDetalle()` N+1 sobre votos |
| 10 | DT-060 | ALTA | MEDIO | 1.5 | `EstadisticasController` agrupa por mes en PHP en lugar de SQL |

---

## DT-051 — `Api\EventoController::votar()` duplica `VotacionService::votarFecha()`

**Por qué importa:** Si `VotacionService::votarFecha()` evoluciona (auditoría, validaciones extra,
cambio de modelo), la ruta API queda silenciosamente desfasada. El mismo patrón ya causó deuda
en DT-031/032 en el controlador web.

### `src/Controller/Api/EventoController.php`

**Paso 1** — añadir `VotacionService` al constructor:

```php
use App\Service\VotacionService;

public function __construct(
    private readonly GrupoRepository $grupoRepository,
    private readonly GrupoMiembroRepository $grupoMiembroRepository,
    private readonly EventoRepository $eventoRepository,
    private readonly FechaPropuestaRepository $fechaPropuestaRepository,
    private readonly VotoFechaRepository $votoFechaRepository,
    private readonly EntityManagerInterface $em,
    private readonly VotacionService $votacionService,  // añadir
) {}
```

**Paso 2** — en `votar()`, reemplazar las líneas ~108–118:

```php
// ANTES (~10 líneas):
$voto = $this->votoFechaRepository->findByFechaAndUsuario($fechaPropuesta, $usuario);
if ($voto === null) {
    $voto = new VotoFecha();
    $voto->setFechaPropuesta($fechaPropuesta);
    $voto->setUsuario($usuario);
    $this->em->persist($voto);
}
$voto->setDisponible($disponibilidad);
$this->em->flush();

// DESPUÉS (1 línea):
$voto = $this->votacionService->votarFecha($fechaPropuesta, $usuario, $disponibilidad);
```

**Paso 3** — limpiar imports no usados:
- `use App\Entity\VotoFecha;` — eliminar si no hay más referencias.
- `private readonly EntityManagerInterface $em` — eliminar del constructor si `votar()` era el único método que lo usaba.

**Riesgo:** Ninguno. El comportamiento es idéntico. La excepción `\RuntimeException` que lanza el servicio si el evento no está abierto ya está cubierta por el check explícito en línea ~85, por lo que nunca llegará al servicio con un evento cerrado.

---

## DT-052 — `RankingService::crearSnapshot()` dos flush fuera de transacción

**Por qué importa:** Si el proceso muere entre el primer y el segundo flush, el ranking histórico
queda permanentemente vacío. `crearSnapshot()` se ejecuta desde un comando periódico — en hosting
compartido podría ser interrumpido por el límite de tiempo del cron.

### `src/Service/RankingService.php`

Reemplazar el método completo `crearSnapshot()` (~L91–112):

```php
public function crearSnapshot(): void
{
    $ranking = $this->puntuacionRepository->getRankingGlobalPonderado(50);
    $fecha   = new \DateTimeImmutable('today');

    $this->em->wrapInTransaction(function () use ($ranking, $fecha): void {
        $existentes = $this->snapshotRepository->findBy(['fecha' => $fecha]);
        foreach ($existentes as $e) {
            $this->em->remove($e);
        }

        foreach ($ranking as $idx => $item) {
            $snapshot = new RankingSnapshot();
            $snapshot->setJuego($item['juego']);
            $snapshot->setPosicion($idx + 1);
            $snapshot->setFecha($fecha);
            $this->em->persist($snapshot);
        }

        $this->em->flush();
    });
}
```

**Riesgo:** Ninguno. El comportamiento es idéntico pero ahora es atómico — si cualquier operación
falla, se hace rollback automático y los snapshots anteriores quedan intactos.

---

## DT-053 — `ColeccionService::agregarJuego()` `findOneBy()` siempre inútil

**Por qué importa:** En una sincronización de 200 juegos se ejecutan 200 queries `SELECT` que
siempre devuelven `null` (la colección fue borrada por `deleteByUsuario()` justo antes). Latencia
añadida sin ningún beneficio.

### `src/Service/ColeccionService.php`

Reemplazar el método privado `agregarJuego()` (~L105–120):

```php
// ANTES (con findOneBy inútil):
private function agregarJuego(Juego $juego, Usuario $usuario): Coleccion
{
    $existente = $this->coleccionRepository->findOneBy([
        'usuario' => $usuario,
        'juego'   => $juego,
    ]);
    if ($existente !== null) {
        return $existente;
    }
    $coleccion = new Coleccion();
    $coleccion->setUsuario($usuario);
    $coleccion->setJuego($juego);
    $this->em->persist($coleccion);
    return $coleccion;
}

// DESPUÉS:
private function agregarJuego(Juego $juego, Usuario $usuario): Coleccion
{
    $coleccion = new Coleccion();
    $coleccion->setUsuario($usuario);
    $coleccion->setJuego($juego);
    $this->em->persist($coleccion);
    return $coleccion;
}
```

**Nota:** Mantener `$this->coleccionRepository` en el constructor — `deleteByUsuario()` lo sigue
necesitando.

**Riesgo:** Ninguno. `deleteByUsuario()` es llamado antes de cualquier llamada a `agregarJuego()`,
garantizando que nunca existirá un registro previo del mismo par (usuario, juego).

---

## DT-054 — `Api\GrupoController::show()` 3 queries para la misma membresía

**Por qué importa:** Cada `GET /api/grupos/{id}` ejecuta `findOneBy(['grupo', 'usuario'])` tres
veces: una en `show()` para verificar acceso, y dos más en `serializeGrupoResumen()` y
`serializeGrupoDetalle()`. Tres queries idénticas a `grupo_miembros` por request.

### `src/Controller/Api/GrupoController.php`

**Paso 1** — en `show()`, pasar el miembro ya cargado a la serialización:

```php
public function show(#[CurrentUser] Usuario $usuario, int $id): JsonResponse
{
    $grupo = $this->grupoRepository->find($id);
    if ($grupo === null) {
        return $this->json(['error' => 'Grupo no encontrado'], 404);
    }

    $miembro = $this->grupoMiembroRepository->findOneBy(['grupo' => $grupo, 'usuario' => $usuario]);
    if ($miembro === null) {
        return $this->json(['error' => 'Acceso denegado'], 403);
    }

    return $this->json($this->serializeGrupoDetalle($grupo, $miembro));
}
```

**Paso 2** — cambiar la firma de `serializeGrupoDetalle()` para recibir el miembro:

```php
private function serializeGrupoDetalle(Grupo $grupo, GrupoMiembro $miembroActual): array
{
    // eliminar la línea: $miembroActual = $this->grupoMiembroRepository->findOneBy(...)
    // usar $miembroActual directamente
    ...
}
```

**Paso 3** — en `index()`, `serializeGrupoResumen()` se llama por cada grupo del usuario. Para
ese caso, cargar el miembro en la query principal con un JOIN sería el fix ideal (DT-008 pattern),
pero por ROI es suficiente con corregir `show()`. Dejar `serializeGrupoResumen()` como está
por ahora y registrar la mejora en `index()` si la lista de grupos escala.

**Riesgo:** Bajo. Solo cambia la firma de `serializeGrupoDetalle()` (método privado).

---

## DT-055 — `VotacionService::puntuarJuego()` hasta 3 flush separados sin transacción

**Por qué importa:** Puede ejecutar: 1) `flush()` para borrar puntuación duplicada, 2) `flush()`
para actualizar existente, o 3) `flush()` para insertar nueva. Si el proceso muere entre dos
flush, la BD queda en estado inconsistente (una puntuación borrada sin que se haya guardado la nueva).

### `src/Service/VotacionService.php`

Reemplazar el método `puntuarJuego()` completo (~L47–85):

```php
public function puntuarJuego(Grupo $grupo, Juego $juego, Usuario $usuario, int $puntuacion): PuntuacionJuego
{
    if ($puntuacion < 1 || $puntuacion > 5) {
        throw new \InvalidArgumentException('La puntuación debe estar entre 1 y 5.');
    }

    return $this->em->wrapInTransaction(function () use ($grupo, $juego, $usuario, $puntuacion): PuntuacionJuego {
        $existente = $this->puntuacionRepository->findOneBy([
            'grupo'   => $grupo,
            'usuario' => $usuario,
            'juego'   => $juego,
        ]);

        $yaUsado = $this->puntuacionRepository->findOneBy([
            'grupo'      => $grupo,
            'usuario'    => $usuario,
            'puntuacion' => $puntuacion,
        ]);

        if ($yaUsado !== null && $yaUsado !== $existente) {
            $this->em->remove($yaUsado);
        }

        if ($existente !== null) {
            $existente->setPuntuacion($puntuacion);
            $this->em->flush();
            return $existente;
        }

        $puntuacionJuego = new PuntuacionJuego();
        $puntuacionJuego->setGrupo($grupo);
        $puntuacionJuego->setJuego($juego);
        $puntuacionJuego->setUsuario($usuario);
        $puntuacionJuego->setPuntuacion($puntuacion);
        $this->em->persist($puntuacionJuego);
        $this->em->flush();

        return $puntuacionJuego;
    });
}
```

**Riesgo:** Bajo. El comportamiento es idéntico pero ahora es atómico.

---

## DT-056 — `MailService::$dynamicMailer` cacheado sin invalidación

**Por qué importa:** En hosting compartido, los procesos PHP-FPM viven horas. Si el admin
actualiza la configuración SMTP en el panel a las 10:00, los emails enviados antes del próximo
reciclado del proceso siguen usando la config antigua. Silencioso e impredecible.

### `src/Service/MailService.php`

**Eliminar** la propiedad cacheada y simplificar `sendEmail()`:

```php
// ANTES:
private ?MailerInterface $dynamicMailer = null;

public function sendEmail(..., bool $forceReload = false): void
{
    ...
    if ($this->dynamicMailer === null || $forceReload) {
        $this->dynamicMailer = $this->createDynamicMailer($config);
    }
    $this->dynamicMailer->send($email);
}

// DESPUÉS:
// (eliminar $dynamicMailer)

public function sendEmail(string $to, string $subject, string $htmlBody, ?string $textBody = null): void
{
    ...
    $mailer = $this->createDynamicMailer($config);
    $mailer->send($email);
}
```

**Verificar antes:** buscar `sendEmail` con `grep -rn "sendEmail"` para confirmar que ningún
llamador pasa `$forceReload = true`. Si lo hay, adaptar ese llamador antes de eliminar el parámetro.

**Riesgo:** Mínimo. `Transport::fromDsn()` y `new Mailer()` son baratos (no abren conexión TCP;
la conexión se abre en `send()`). El overhead es negligible.

---

## DT-057 — `TelegramService::llamarApi()` URL base hardcodeada

**Por qué importa:** Los tests de integración que quieran verificar llamadas reales a Telegram
no pueden redirigir el cliente a un mock server sin parchear toda la clase. Con una constante
se documenta el endpoint y se facilita un futuro test.

### `src/Service/TelegramService.php`

Añadir constante privada al inicio de la clase:

```php
private const API_BASE = 'https://api.telegram.org/bot';
```

En `llamarApi()` (~L168), reemplazar:

```php
// ANTES:
$apiBase = "https://api.telegram.org/bot{$token}";

// DESPUÉS:
$apiBase = self::API_BASE . $token;
```

**Extensión futura (no implementar ahora):** si se quiere inyectar para tests, cambiar a
`#[Autowire('%telegram.api_base%')]` y definir el parámetro en `services.yaml`.

**Riesgo:** Ninguno.

---

## DT-058 — `GrupoController::editar()` bypassa `GrupoPermisoService`

**Por qué importa:** `editar()` inyecta `GrupoMiembroRepository` directamente y llama `isAdmin()`
en la entidad, inconsistente con el resto del controlador. Si en el futuro se añade un permiso
`EditarGrupo`, este check no lo cubrirá.

### `src/Controller/GrupoController.php`

**Paso 1** — sustituir `GrupoMiembroRepository` por `GrupoPermisoService` en la firma del método:

```php
public function editar(
    Grupo $grupo,
    Request $request,
    GrupoPermisoService $grupoPermisoService,  // en lugar de GrupoMiembroRepository
    EntityManagerInterface $em,
    ImageService $imageService,
    #[Autowire('%kernel.project_dir%/public/uploads/grupos')] string $gruposDir,
): Response {
```

**Paso 2** — el check de permisos queda igual semánticamente:

```php
// ANTES:
$membership = $grupoMiembroRepository->findMembership($grupo, $usuario);
if ($membership === null || !$membership->isAdmin()) {
    throw $this->createAccessDeniedException('Solo los administradores pueden editar el grupo.');
}

// DESPUÉS:
$membership = $grupoPermisoService->getMembership($grupo, $usuario);
if ($membership === null || !$membership->isAdmin()) {
    throw $this->createAccessDeniedException('Solo los administradores pueden editar el grupo.');
}
```

**Paso 3** — verificar con `grep -n "GrupoMiembroRepository" GrupoController.php` que no hay
otros usos en el archivo. Si no los hay, eliminar el import.

**Riesgo:** Ninguno. `getMembership()` hace exactamente la misma query que `findMembership()`.

---

## DT-059 — `Api\EventoController::serializeEventoDetalle()` carga votos dos veces (N+1)

**Por qué importa:** Con 5 fechas y 10 miembros: la query de `getMatrizDisponibilidad()` es
eficiente, pero a continuación `$fp->getVotos()` genera 5 lazy-loads de colecciones, y cada
`$voto->getUsuario()` genera hasta 50 lazy-loads más. En total: 55+ queries para una sola
llamada `GET /api/eventos/{id}`.

### Plan en dos partes:

**Parte A — `src/Repository/FechaPropuestaRepository.php`**

Modificar `findByEventoOrderedByFecha()` para precargar votos y usuarios:

```php
/** @return FechaPropuesta[] */
public function findByEventoOrderedByFecha(Evento $evento): array
{
    return $this->createQueryBuilder('fp')
        ->leftJoin('fp.votos', 'v')
        ->addSelect('v')
        ->leftJoin('v.usuario', 'u')
        ->addSelect('u')
        ->where('fp.evento = :evento')
        ->setParameter('evento', $evento)
        ->orderBy('fp.fecha', 'ASC')
        ->getQuery()
        ->getResult();
}
```

**Parte B — `src/Controller/Api/EventoController.php`**

El loop sobre `$fp->getVotos()` ya no dispara queries (la colección está precargada en la
identidad map de Doctrine). No hay cambio de lógica.

**Verificar impacto lateral:** buscar con `grep -rn "findByEventoOrderedByFecha"` todos los
llamadores. Si alguno está en un contexto donde el JOIN extra es indeseable (ej. admin con
paginación pesada), crear un método alternativo `findByEventoOrdered()` sin el JOIN y usar
el original para la API.

**Riesgo:** Bajo. Doctrine gestiona correctamente la hidratación de colecciones con `addSelect`;
no genera duplicados de objetos. Los resultados son idénticos, solo más eficientes.

---

## DT-060 — `EstadisticasController` agrupa por mes en PHP en lugar de en SQL

**Por qué importa:** `findFechasRegistroDesde()` y `findFechasCreacionDesde()` cargan TODAS las
filas del período en memoria y el controlador las agrupa en PHP. Con 1.000+ usuarios/eventos,
esto carga todos los objetos DateTimeInterface en memoria para descartarlos inmediatamente.

### `src/Repository/UsuarioRepository.php`

Añadir:

```php
/**
 * @return array<string, int>  claves 'YYYY-MM' => count
 */
public function countRegistrosByMonth(\DateTimeImmutable $desde): array
{
    $rows = $this->createQueryBuilder('u')
        ->select("SUBSTRING(u.fechaRegistro, 1, 7) AS mes, COUNT(u.id) AS total")
        ->where('u.fechaRegistro >= :desde')
        ->setParameter('desde', $desde)
        ->groupBy('mes')
        ->orderBy('mes', 'ASC')
        ->getQuery()
        ->getArrayResult();

    return array_column($rows, 'total', 'mes');
}
```

### `src/Repository/EventoRepository.php`

Añadir:

```php
/**
 * @return array<string, int>  claves 'YYYY-MM' => count
 */
public function countCreacionesByMonth(\DateTimeImmutable $desde): array
{
    $rows = $this->createQueryBuilder('e')
        ->select("SUBSTRING(e.fechaCreacion, 1, 7) AS mes, COUNT(e.id) AS total")
        ->where('e.fechaCreacion >= :desde')
        ->setParameter('desde', $desde)
        ->groupBy('mes')
        ->orderBy('mes', 'ASC')
        ->getQuery()
        ->getArrayResult();

    return array_column($rows, 'total', 'mes');
}
```

### `src/Controller/Admin/EstadisticasController.php`

Reemplazar los dos bloques de agrupación en PHP (~L42–53):

```php
// ANTES:
$regMap = [];
foreach ($usuarioRepo->findFechasRegistroDesde($desde12m) as $fila) {
    $mes = $fila['fechaRegistro']->format('Y-m');
    $regMap[$mes] = ($regMap[$mes] ?? 0) + 1;
}
$registrosMeses = array_map(fn(string $m): int => $regMap[$m] ?? 0, $meses);

$evMap = [];
foreach ($eventoRepo->findFechasCreacionDesde($desde12m) as $fila) {
    $mes = $fila['fechaCreacion']->format('Y-m');
    $evMap[$mes] = ($evMap[$mes] ?? 0) + 1;
}
$eventosMeses = array_map(fn(string $m): int => $evMap[$m] ?? 0, $meses);

// DESPUÉS:
$regMap         = $usuarioRepo->countRegistrosByMonth($desde12m);
$registrosMeses = array_map(fn(string $m): int => $regMap[$m] ?? 0, $meses);

$evMap        = $eventoRepo->countCreacionesByMonth($desde12m);
$eventosMeses = array_map(fn(string $m): int => $evMap[$m] ?? 0, $meses);
```

**Limpieza:** los métodos antiguos `findFechasRegistroDesde()` y `findFechasCreacionDesde()`
quedan sin llamadores → eliminarlos (verificar con `grep -rn` que no hay tests que los usen).

**Nota técnica:** `SUBSTRING(campo, 1, 7)` es SQL estándar y funciona en MySQL y SQLite,
lo que mantiene la compatibilidad con tests en memoria si los hubiera. `DATE_FORMAT` solo
funciona en MySQL. `SUBSTRING` es la opción más portable.

**Riesgo:** Bajo. La query devuelve escalares (no entidades); el mapping manual con
`array_column` es robusto.

---

## Validación tras implementar

```bash
php bin/console cache:clear
php bin/console doctrine:schema:validate
php bin/phpunit tests/
```

Comprobar en el profiler de Symfony (o con `EXPLAIN` en MySQL) que las queries N+1 de
DT-059 han desaparecido antes y después de aplicar el fix.