docs(sprint-7): cierre formal — TECH_DEBT guards+auth, OBSERVATIONS reset Sprint 8, decisión org→areas
Some checks failed
/ Deploy to Cloudflare Pages (push) Has been cancelled
Some checks failed
/ Deploy to Cloudflare Pages (push) Has been cancelled
This commit is contained in:
110
OBSERVATIONS.md
110
OBSERVATIONS.md
@@ -1,9 +1,9 @@
|
||||
# OBSERVATIONS — Sprint 7
|
||||
# OBSERVATIONS — Sprint 8
|
||||
|
||||
> Hallazgos descubiertos durante el sprint actual.
|
||||
> Este archivo se vacía al cerrar el sprint: los hallazgos se promocionan a TECH_DEBT.md o BACKLOG.md, o quedan resueltos.
|
||||
>
|
||||
> **Sprint 6 cerrado el 2026-06-24.** Observaciones archivadas — ítems migrados a BACKLOG.md y TECH_DEBT.md.
|
||||
> **Sprint 7 cerrado el 2026-07-07.** Ver reflexiones completas en `STRATEGIC_DIRECTION.md`.
|
||||
|
||||
---
|
||||
|
||||
@@ -24,75 +24,13 @@
|
||||
|
||||
## Hallazgos resueltos durante el sprint
|
||||
|
||||
### [RESOLVED] Etapa 1 — Migraciones DB + SECURITY DEFINER helpers
|
||||
|
||||
**Policy `activity_resource_assignments`:** el BRIEF asumía `"all_activity_resource_assignments"` como nombre de la policy. La migration 007 la creó como `"all_assignments"`. Claude Code auditó las migraciones 001-013 antes de escribir los DROP POLICY y corrigió el nombre. Sin la corrección, el DROP habría pasado silenciosamente (IF EXISTS) sin eliminar la policy permisiva original, manteniendo acceso sin restricción de rol.
|
||||
|
||||
**Tabla `resources` sin `process_id`:** el brief del ETAPA_1_PROMPT incluía una policy `resources_select` con condición `resources.process_id IS NULL`. La columna no existe desde Sprint 3 (catálogo global). Claude Code detectó la discrepancia y adaptó la policy a `auth.uid() IS NOT NULL`, equivalente funcional para catálogo sin filtro de proceso.
|
||||
|
||||
**Auto-migración `client` → `org_id`:** en el entorno de dev, los 158 procesos existentes no tienen valor en el campo `client` → 0 procesos vinculados a org. Esto es esperado: el campo `client` es opcional y en dev no está poblado con clientes reales. En producción, si hay procesos con `client` no vacío, la auto-migración los vinculará automáticamente al primer deploy.
|
||||
|
||||
**OK formal del director:** 2026-07-06.
|
||||
|
||||
---
|
||||
|
||||
### [RESOLVED] Etapa 2 — Edge Functions invite-user y revoke-user
|
||||
|
||||
Funciones creadas en `supabase/functions/`. El diseño simplificó el scope del BRIEF (que pedía `delete-user` + `suspend-user` + `unsuspend-user` separadas) a `invite-user` + `revoke-user` (soft suspension a 'guest'). Las limitaciones quedaron documentadas en `UsersPage.tsx`:
|
||||
- "Eliminar" llama `revoke-user` (baja a guest) — eliminación real requiere función futura
|
||||
- "Reactivar" restaura siempre a 'client_editor' — rol previo no se guarda
|
||||
**OK formal implícito del director:** 2026-07-06.
|
||||
|
||||
---
|
||||
|
||||
### [RESOLVED] Etapa 3 — Auth layer: fix rol desde DB + email/password + reset-password
|
||||
|
||||
Fix crítico: `syncProfileFromDB` ahora lee `platform_role` y `org_id` desde DB en lugar de inferir el rol del dominio del email. `AuthUser` tiene `orgId?: string`. `signInWithEmailPassword` implementado. `TOKEN_REFRESHED` no dispara loading. `LoginPage` con form email/password. `ResetPasswordPage` en `/reset-password`. 558 tests (+ 6 nuevos).
|
||||
|
||||
**Deuda identificada:** `PASSWORD_RECOVERY` maneja vía segunda suscripción a `onAuthStateChange` (evitar importación circular). No es bug, pero el patrón de doble suscripción es frágil. Ver ítem en TECH_DEBT.md.
|
||||
**OK formal del director:** 2026-07-06.
|
||||
|
||||
---
|
||||
|
||||
### [RESOLVED] Etapa 4 — Panel de administración
|
||||
|
||||
`/admin/organizations`, `/admin/organizations/$orgId`, `/admin/users` con guard de `platform_admin`. Invitación de usuarios via Edge Function `invite-user`. Suspensión/reactivación via `revoke-user`. Skeletons durante carga. Badge "Tú" en fila propia. Acciones deshabilitadas para self-action. RoleBadge con colores por rol.
|
||||
|
||||
**Deuda identificada:** guard implementado en `AdminLayout` como componente (no en `beforeLoad` de TanStack Router) — puede haber flash breve del contenido antes del redirect. Ver ítem recurrente en TECH_DEBT.md.
|
||||
**OK formal del director:** 2026-07-06 (validación visual con DOM).
|
||||
|
||||
---
|
||||
|
||||
### [RESOLVED] Etapa 5 — UX de roles de cliente y routing
|
||||
|
||||
Guards de rol en `WorkspacePage` e `ImportPage` vía `useEffect`. `client_viewer` redirigido a `/report/$processId` al entrar a workspace. `client_editor` bloqueado de `/import`. Botón "Importar BPMN" oculto para roles de cliente. Botón "Nuevo proceso" oculto para roles de cliente. Panel de grupos oculto para roles de cliente (lista plana). `ReportPage` con `defaultTab = 'roi'` para `client_viewer`. 578 tests (+ 20 nuevos).
|
||||
|
||||
**Bug encontrado en validación:** `group_label = "un Cliente"` guardado como dato corrupto en Supabase → `"2 un clientes"` en UI. Causa: el campo era editable desde Settings y en algún momento se guardó con el artículo. Fix: `normalizeGroupLabel()` en `settings-repo.ts` que limpia artículos iniciales (`un`, `una`, `el`, `la`) al leer y al guardar. Solución transparente sin migración de datos.
|
||||
|
||||
**Deuda identificada (BACKLOG):** Settings page (ícono de tuerca) accesible para `client_editor`. La página es cosmética (solo label de grupos), pero no debería ser visible para roles de cliente. Ítem agregado en BACKLOG.md.
|
||||
|
||||
**Patrón recurrente identificado:** guards de rol implementados tres veces via `useEffect` en lugar de `beforeLoad` de TanStack Router (AdminLayout, WorkspacePage, ImportPage). Funcional pero subóptimo — posible flash antes del redirect. Candidato a TECH_DEBT sprint siguiente.
|
||||
|
||||
**OK formal del director:** 2026-07-06 (validación visual con cambio temporal de rol en Supabase Studio).
|
||||
*(vacío — sprint recién abierto)*
|
||||
|
||||
---
|
||||
|
||||
## Hallazgos diferidos (promocionar al cerrar sprint)
|
||||
|
||||
### [DEFERRED-TECH-DEBT] Guards de rol via `useEffect` en lugar de `beforeLoad` (3 componentes)
|
||||
|
||||
`AdminLayout`, `WorkspacePage`, `ImportPage` — los tres usan `useEffect` para detectar el rol y redirigir. Con `beforeLoad` de TanStack Router, la ruta nunca monta para usuarios sin acceso. Con `useEffect`, el componente monta brevemente antes del redirect (posible flash). No es bug de seguridad (RLS protege los datos), pero es anti-patrón UX y arquitectónico para TanStack Router.
|
||||
**Migrar a:** TECH_DEBT.md al cerrar Sprint 7.
|
||||
|
||||
### [DEFERRED-TECH-DEBT] Doble suscripción `onAuthStateChange` en AuthContext
|
||||
|
||||
AuthContext tiene dos listeners de `onAuthStateChange`: uno principal (SIGNED_IN, SIGNED_OUT, TOKEN_REFRESHED) y uno secundario para `PASSWORD_RECOVERY`. Fue implementado para evitar una importación circular. El patrón correcto es un único listener con switch/case completo. Refactorizar cuando se resuelva la dependencia circular.
|
||||
**Migrar a:** TECH_DEBT.md al cerrar Sprint 7.
|
||||
|
||||
### [DEFERRED-BACKLOG] Settings page accesible para client_editor/client_viewer
|
||||
|
||||
El ícono de configuración (tuerca) en LibraryPage y la ruta `/settings` (si existe) son accesibles para todos los roles. La página solo configura el label de grupos (preferencia cosmética), pero un usuario cliente no debería verla. Ocultar el ícono de configuración y agregar guard de rol en la ruta.
|
||||
**Migrar a:** BACKLOG.md al cerrar Sprint 7 — ya agregado por Claude Code.
|
||||
*(vacío)*
|
||||
|
||||
---
|
||||
|
||||
@@ -107,9 +45,43 @@ El ícono de configuración (tuerca) en LibraryPage y la ruta `/settings` (si ex
|
||||
## Cierre del sprint (checklist)
|
||||
|
||||
```
|
||||
[x] Verificar que todos los items [CURRENT-SPRINT] están RESOLVED
|
||||
[x] Mover items [DEFERRED-TECH-DEBT] a TECH_DEBT.md (pendiente commit)
|
||||
[x] Mover items [DEFERRED-BACKLOG] a BACKLOG.md (ya agregado por Claude Code en Etapa 5)
|
||||
[ ] Verificar que todos los items [CURRENT-SPRINT] están RESOLVED
|
||||
[ ] Mover items [DEFERRED-TECH-DEBT] a TECH_DEBT.md
|
||||
[ ] Mover items [DEFERRED-BACKLOG] a BACKLOG.md
|
||||
[ ] Archivar o purgar este archivo
|
||||
[ ] Crear OBSERVATIONS.md vacío para el sprint siguiente
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Aprendizajes de Sprint 7 relevantes para Sprint 8
|
||||
|
||||
> Cosecha de método — decisiones y patterns emergentes de Sprint 7 que informan cómo trabajar en Sprint 8.
|
||||
|
||||
### Race condition en roles: `isProfileLoaded` como guard
|
||||
|
||||
`buildAuthUser()` asigna `platformRole: 'guest'` heurísticamente (por dominio de email) antes de que `syncProfileFromDB` complete. El rol correcto llega ~200ms después. Cualquier UI condicionada por rol que renderice antes de ese sync muestra el estado incorrecto.
|
||||
|
||||
**Patrón validado:** `isProfileLoaded: boolean` en AuthContext — se pone en `true` solo después de que `syncProfileFromDB` completa. Componentes que dependen del rol deben esperar `isProfileLoaded` antes de renderizar contenido rol-dependiente.
|
||||
|
||||
**Aprendizaje para Sprint 8:** cualquier componente nuevo que condicione su contenido por `platformRole` debe incluir `isProfileLoaded` como prerequisito en su spec de implementación.
|
||||
|
||||
### JOIN vs fetch separado: orgName en getById
|
||||
|
||||
Fetch separado en `useEffect` para obtener el nombre de la org en `WorkspacePage` fue frágil: race conditions entre `currentProcess` cargando y el efecto, resets de estado al desmontar, y respuesta en array cuando `.maybeSingle()` esperaba objeto. El síntoma (botón mostrando "Biblioteca" en lugar de "Solar Banco") requirió ~8 iteraciones de diagnóstico.
|
||||
|
||||
**Fix definitivo:** JOIN en `getById()` con `select('*, org:organizations!org_id(name)')`. El dato llega embebido en el proceso — sin async adicional, sin estado local, sin race condition.
|
||||
|
||||
**Aprendizaje para Sprint 8:** cuando un componente necesita datos relacionados de otra tabla para display, preferir JOIN en el repositorio sobre fetch separado en el componente. Los fetches separados en `useEffect` con datos del store como dependencia son estructuralmente frágiles.
|
||||
|
||||
### Dos sistemas paralelos de "cliente" → consolidar en Sprint 8
|
||||
|
||||
`process_groups`, `clientName` (texto libre) y `org_id` intentaban modelar el mismo concepto ("a qué cliente pertenece el proceso") desde tres ángulos distintos. La confusión generó bugs de UX (back button, agrupación en biblioteca) y deuda conceptual.
|
||||
|
||||
**Decisión tomada:** Sprint 8 consolida en `organizations → areas → processes`. Ver `STRATEGIC_DIRECTION.md` → "Consolidación del modelo de cliente".
|
||||
|
||||
### Diagnóstico por DOM observable > teorización de librerías
|
||||
|
||||
El bug del freeze de perfil (Sprint 5, confirmado resuelto en Sprint 6) y el bug del back button (Sprint 7) compartieron el mismo patrón: múltiples iteraciones de diagnóstico teórico sin resolver → un dato observable del runtime (HTML del DOM, Network tab, respuesta de request) colapsó el espacio de hipótesis en 1 paso.
|
||||
|
||||
**Aprendizaje consolidado:** ante bug de UI que persiste 2+ fixes teóricos, pedir el dato observable antes de seguir. Específicamente: HTML del elemento, Network tab con el request real, o `console.log` del valor en runtime.
|
||||
|
||||
19
TECH_DEBT.md
19
TECH_DEBT.md
@@ -67,6 +67,25 @@ documentación.
|
||||
reload completo de página el valor guardado se relee correctamente desde la DB.
|
||||
**Sprint cerrado en:** Sprint 6 Etapa 3.
|
||||
|
||||
### [BACKLOG] Guards de rol via `useEffect` en lugar de `beforeLoad` (3 componentes)
|
||||
|
||||
**Estado actual:** `AdminLayout`, `WorkspacePage`, e `ImportPage` usan `useEffect` para detectar el rol y redirigir. El componente monta brevemente antes del redirect — posible flash de contenido.
|
||||
**Causa raíz:** el pattern surgió de prompts que describieron el guard en términos de comportamiento de componente ("si el rol no es X, redirigir") en lugar de especificar la sección de router.
|
||||
**Impacto:** no es bug de seguridad (RLS protege los datos en Supabase), pero es anti-patrón para TanStack Router y puede causar flash visual breve en conexiones lentas.
|
||||
**Solución propuesta:** migrar a `beforeLoad` de TanStack Router. Ejemplo: `beforeLoad: ({ context }) => { if (context.auth.user?.platformRole !== 'platform_admin') throw redirect({ to: '/library' }) }`.
|
||||
**Archivos afectados:** `src/features/admin/AdminLayout.tsx`, `src/features/workspace/WorkspacePage.tsx`, `src/features/import/ImportPage.tsx` (aprox. línea 65-70 en WorkspacePage).
|
||||
**Aprendizaje de método:** en prompts futuros que requieran guards de rol, especificar siempre el código exacto de `beforeLoad` en la sección de router — no solo el comportamiento esperado del componente.
|
||||
**Origen:** Sprint 7 Etapa 5 — patrón recurrente detectado en 3 componentes.
|
||||
|
||||
### [BACKLOG] Doble suscripción `onAuthStateChange` en AuthContext
|
||||
|
||||
**Estado actual:** `AuthContext.tsx` tiene dos listeners de `onAuthStateChange`: uno principal (SIGNED_IN, SIGNED_OUT, TOKEN_REFRESHED, INITIAL_SESSION) y uno secundario para `PASSWORD_RECOVERY`. El secundario fue implementado para evitar importación circular entre `AuthContext` y `SupabaseAuthService`.
|
||||
**Causa raíz:** `PASSWORD_RECOVERY` necesita llamar a `supabase.auth.updateUser()` directamente desde el contexto — si se delega a `SupabaseAuthService`, este importa desde `AuthContext` creando un ciclo.
|
||||
**Impacto:** ambos listeners se registran en cada mount del `AuthProvider`. En la mayoría de los casos no genera bugs, pero el patrón es frágil y puede causar doble-dispatch de eventos en casos edge de remount.
|
||||
**Solución propuesta:** refactorizar para un único switch/case en el listener principal. Resolver la importación circular moviendo `supabase` client a un módulo compartido sin dependencia de AuthContext.
|
||||
**Archivos afectados:** `src/auth/AuthContext.tsx`.
|
||||
**Origen:** Sprint 7 Etapa 3 — documentado en OBSERVATIONS.md.
|
||||
|
||||
### [BACKLOG] `getCurrentUser()` en SupabaseAuthService — sin callers activos
|
||||
**Estado actual:** `SupabaseAuthService.getCurrentUser()` usa `getSession()` directamente (sin timeout, sin processLock workaround). Es la única excepción documentada a la regla — justificada porque necesita el objeto sesión completo, no solo una barrera. Sin callers activos hoy.
|
||||
**Impacto si no se resuelve:** ninguno mientras no tenga callers. Si en el futuro algún flujo llama `getCurrentUser()`, podría colgar el processLock si el servidor no responde.
|
||||
|
||||
Reference in New Issue
Block a user