From cfa91a5c09f339427715ceefdcc6aeca5f61ebe0 Mon Sep 17 00:00:00 2001 From: Lizandro Guarnizo <77708265+lizandrogd@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:26:44 -0500 Subject: [PATCH] fix(portal): el avance del dashboard mostraba siempre el valor anterior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auditoría de /portal/dashboard. Cuatro problemas, todos en el mismo bucle: 1. El avance se veía un render atrasado. ActualizarProgresoProyecto corría DESPUÉS de cargar los proyectos: escribía el valor nuevo en la base pero los structs ya cargados seguían con el viejo, que es lo que se renderiza. El usuario veía el cálculo de la visita anterior. 2. N+1 con escrituras en un GET: dos COUNT y un UPDATE por proyecto. Con 10 proyectos, 30 consultas y 10 escrituras por cada carga del dashboard — incluyendo las de cualquier bot que pase. Ahora es UNA consulta agrupada y ninguna escritura; los caminos que tocan una fase ya mantienen la columna al día, así que recalcular en el GET no aportaba nada. 3. Orden aleatorio de los grupos: se recorría un map de Go, así que un partner veía sus clientes en distinto orden en cada recarga. 4. isPartner se decidía con u.Rol mientras el alcance se decidía con Role.EsPortalPartner. Desincronizados, un usuario veía proyectos de varios clientes sin agrupar, o la vista agrupada vacía. Ahora hay una sola definición (PortalUser.EsPartner) con test de los dos sentidos. Además, el error de carga se descartaba con `_` y el usuario terminaba viendo "no tenés proyectos", indistinguible de una caída de la base. Sin hallazgos de seguridad: el chequeo de acceso por cliente está en todas las rutas, la sesión revalida Activo en cada request, y las plantillas no usan x-html ni template.HTML, así que Go escapa todo. Co-Authored-By: Claude Sonnet 5 --- pkg/models/portal_user.go | 37 ++++++++++----- pkg/models/portal_user_partner_test.go | 62 ++++++++++++++++++++++++++ pkg/models/proyecto.go | 50 +++++++++++++++++++++ resources/views/portal/dashboard.html | 8 ++++ rest/controllers/portal_controller.go | 38 ++++++++++++---- 5 files changed, 175 insertions(+), 20 deletions(-) create mode 100644 pkg/models/portal_user_partner_test.go diff --git a/pkg/models/portal_user.go b/pkg/models/portal_user.go index 90f3892..0789ccb 100644 --- a/pkg/models/portal_user.go +++ b/pkg/models/portal_user.go @@ -181,12 +181,27 @@ func RemovePortalAcceso(portalUserID, clienteID uint) error { } // GetClienteIDsForPortalUser devuelve todos los clienteIDs accesibles para un portal user. -func GetClienteIDsForPortalUser(u *PortalUser) []uint { - isPartner := u.Rol == "partner" - if u.Role != nil { - isPartner = u.Role.EsPortalPartner +// EsPartner es la ÚNICA definición de "este usuario es partner". Existe +// porque el rol se guarda en dos lados (la columna Rol y el flag +// EsPortalPartner del Role) y consultarlos por separado se contradice: el +// dashboard decidía el layout con u.Rol y los proyectos con Role, así que un +// usuario con los dos desincronizados veía proyectos de varios clientes sin +// agrupar, o la vista agrupada vacía. +func (u *PortalUser) EsPartner() bool { + if u == nil { + return false } - if isPartner { + if u.Role != nil { + return u.Role.EsPortalPartner + } + return u.Rol == "partner" +} + +func GetClienteIDsForPortalUser(u *PortalUser) []uint { + if u == nil { + return []uint{} + } + if u.EsPartner() { ids := make([]uint, 0, len(u.PortalAccesos)) for _, a := range u.PortalAccesos { ids = append(ids, a.ClienteID) @@ -202,12 +217,12 @@ func GetClienteIDsForPortalUser(u *PortalUser) []uint { // ─── PortalPasswordResetToken ──────────────────────────────────────────────── type PortalPasswordResetToken struct { - ID uint `gorm:"primaryKey;autoIncrement"` - PortalUserID uint `gorm:"column:portal_user_id;index;not null"` - Token string `gorm:"column:token;uniqueIndex;not null"` - ExpiresAt time.Time `gorm:"column:expires_at;not null"` - Used bool `gorm:"column:used;default:false"` - CreatedAt time.Time + ID uint `gorm:"primaryKey;autoIncrement"` + PortalUserID uint `gorm:"column:portal_user_id;index;not null"` + Token string `gorm:"column:token;uniqueIndex;not null"` + ExpiresAt time.Time `gorm:"column:expires_at;not null"` + Used bool `gorm:"column:used;default:false"` + CreatedAt time.Time } func (PortalPasswordResetToken) TableName() string { return "portal_password_reset_tokens" } diff --git a/pkg/models/portal_user_partner_test.go b/pkg/models/portal_user_partner_test.go new file mode 100644 index 0000000..ab75a7f --- /dev/null +++ b/pkg/models/portal_user_partner_test.go @@ -0,0 +1,62 @@ +package models + +import "testing" + +// El rol vive en dos lados (la columna Rol y el flag EsPortalPartner del +// Role). Consultarlos por separado se contradecía: el dashboard elegía el +// layout con uno y los proyectos con el otro. EsPartner es ahora la única +// definición, y estos casos fijan cuál gana. +func TestEsPartner(t *testing.T) { + casos := []struct { + nombre string + user *PortalUser + esperado bool + }{ + {"nil no es partner", nil, false}, + {"sin Role, rol partner", &PortalUser{Rol: "partner"}, true}, + {"sin Role, rol cliente", &PortalUser{Rol: "cliente"}, false}, + {"con Role, el flag manda aunque el rol diga cliente", + &PortalUser{Rol: "cliente", Role: &Roles{EsPortalPartner: true}}, true}, + {"con Role, el flag manda aunque el rol diga partner", + &PortalUser{Rol: "partner", Role: &Roles{EsPortalPartner: false}}, false}, + } + for _, cas := range casos { + t.Run(cas.nombre, func(t *testing.T) { + if got := cas.user.EsPartner(); got != cas.esperado { + t.Errorf("EsPartner() = %v, esperaba %v", got, cas.esperado) + } + }) + } +} + +// El alcance del portal se deriva de EsPartner: un desajuste acá es que un +// usuario vea proyectos de un cliente que no le corresponde. +func TestGetClienteIDsForPortalUser(t *testing.T) { + cid := uint(7) + casos := []struct { + nombre string + user *PortalUser + esperado []uint + }{ + {"nil no ve nada", nil, []uint{}}, + {"cliente ve el suyo", &PortalUser{Rol: "cliente", ClienteID: &cid}, []uint{7}}, + {"cliente sin ClienteID no ve nada", &PortalUser{Rol: "cliente"}, []uint{}}, + {"partner ve los de sus accesos", + &PortalUser{Rol: "partner", PortalAccesos: []PortalAcceso{{ClienteID: 3}, {ClienteID: 9}}}, + []uint{3, 9}}, + {"partner sin accesos no ve nada", &PortalUser{Rol: "partner"}, []uint{}}, + } + for _, cas := range casos { + t.Run(cas.nombre, func(t *testing.T) { + got := GetClienteIDsForPortalUser(cas.user) + if len(got) != len(cas.esperado) { + t.Fatalf("= %v, esperaba %v", got, cas.esperado) + } + for i := range got { + if got[i] != cas.esperado[i] { + t.Errorf("= %v, esperaba %v", got, cas.esperado) + } + } + }) + } +} diff --git a/pkg/models/proyecto.go b/pkg/models/proyecto.go index 1fda52b..eae1ba6 100644 --- a/pkg/models/proyecto.go +++ b/pkg/models/proyecto.go @@ -327,3 +327,53 @@ func GetProyectoDocumentoByID(id uint) (*ProyectoDocumento, error) { func DeleteProyectoDocumento(id uint) error { return app.Http.Database.DB.Delete(&ProyectoDocumento{}, id).Error } + +// ProgresoPorProyecto calcula el avance de varios proyectos en UNA consulta +// agrupada. +// +// El dashboard hacía esto con ActualizarProgresoProyecto en un bucle: dos +// COUNT y un UPDATE por proyecto (3N consultas, N de ellas escrituras en un +// GET). Y no servía para lo que se estaba renderizando: los proyectos ya +// estaban cargados en memoria, así que el UPDATE iba a la base pero la +// pantalla seguía mostrando el valor anterior — el avance se veía siempre un +// render atrasado. +// +// Las escrituras no hacen falta: todos los caminos que tocan una fase ya +// llaman a ActualizarProgresoProyecto (ver proyecto_controller y +// proyecto_service). Acá solo se lee. +func ProgresoPorProyecto(proyectoIDs []uint) map[uint]int { + out := make(map[uint]int, len(proyectoIDs)) + if len(proyectoIDs) == 0 { + return out + } + var filas []struct { + ProyectoID uint + Total int64 + Completados int64 + } + app.Http.Database.DB.Model(&ProyectoFase{}). + Select("proyecto_id, COUNT(*) AS total, SUM(CASE WHEN estado = 'completado' THEN 1 ELSE 0 END) AS completados"). + Where("proyecto_id IN ? AND deleted_at IS NULL", proyectoIDs). + Group("proyecto_id").Scan(&filas) + + for _, f := range filas { + if f.Total > 0 { + out[f.ProyectoID] = int((f.Completados * 100) / f.Total) + } + } + return out +} + +// AplicarProgreso pone el avance recién calculado en los structs que se van a +// renderizar. Un proyecto sin fases queda en 0, que es lo correcto: no hay +// nada planificado todavía. +func AplicarProgreso(proyectos []Proyecto) { + ids := make([]uint, len(proyectos)) + for i, p := range proyectos { + ids[i] = p.ID + } + progresos := ProgresoPorProyecto(ids) + for i := range proyectos { + proyectos[i].Progreso = progresos[proyectos[i].ID] + } +} diff --git a/resources/views/portal/dashboard.html b/resources/views/portal/dashboard.html index b9169b6..931640b 100644 --- a/resources/views/portal/dashboard.html +++ b/resources/views/portal/dashboard.html @@ -15,6 +15,14 @@

+ + {{ if .error }} +
+ {{ .error }} +
+ {{ end }} + {{ if .isPartner }} diff --git a/rest/controllers/portal_controller.go b/rest/controllers/portal_controller.go index e411dba..ecbb3d4 100644 --- a/rest/controllers/portal_controller.go +++ b/rest/controllers/portal_controller.go @@ -9,6 +9,7 @@ import ( "net/url" "os" "path/filepath" + "sort" "strconv" "strings" "time" @@ -152,10 +153,18 @@ func PortalDashboard(c *fiber.Ctx) error { } clienteIDs := models.GetClienteIDsForPortalUser(fullUser) - proyectos, _ := models.GetProyectosByClienteIDs(clienteIDs) - for i := range proyectos { - _ = models.ActualizarProgresoProyecto(proyectos[i].ID) + proyectos, err := models.GetProyectosByClienteIDs(clienteIDs) + if err != nil { + // Antes este error se descartaba y el usuario veía "no tenés + // proyectos": indistinguible de una caída de la base. + log.Printf("[PORTAL] dashboard: no se pudieron cargar los proyectos del usuario %d: %v", fullUser.ID, err) + return c.Status(500).Render("portal/dashboard", fiber.Map{ + "portalUser": fullUser, + "error": "No pudimos cargar tus proyectos en este momento. Volvé a intentar en un minuto.", + "isPartner": fullUser.EsPartner(), + }, "layouts/portal") } + models.AplicarProgreso(proyectos) // Si partner: agrupar proyectos por cliente type ClienteProyectos struct { @@ -173,12 +182,15 @@ func PortalDashboard(c *fiber.Ctx) error { for _, g := range clienteMap { grupos = append(grupos, *g) } + // El recorrido de un map en Go es aleatorio: sin ordenar, un partner veía + // sus clientes en distinto orden en cada recarga. + sort.Slice(grupos, func(i, j int) bool { return grupos[i].Cliente.Nombre < grupos[j].Cliente.Nombre }) return c.Render("portal/dashboard", fiber.Map{ "portalUser": fullUser, "proyectos": proyectos, "grupos": grupos, - "isPartner": fullUser.Rol == "partner", + "isPartner": fullUser.EsPartner(), }, "layouts/portal") } @@ -214,7 +226,7 @@ func PortalProyecto(c *fiber.Ctx) error { avances, _ := models.GetAvancesByProyecto(proy.ID, true) entregables, _ := models.GetEntregablesByProyecto(proy.ID, true) tickets, _ := models.GetTicketsByProyecto(proy.ID) - _ = models.ActualizarProgresoProyecto(proy.ID) + proy.Progreso = models.ProgresoPorProyecto([]uint{proy.ID})[proy.ID] return c.Render("portal/proyecto", fiber.Map{ "portalUser": fullUser, @@ -236,10 +248,18 @@ func PortalGetProyectos(c *fiber.Ctx) error { } fullUser, _ := models.GetPortalUserByID(u.ID) clienteIDs := models.GetClienteIDsForPortalUser(fullUser) - proyectos, _ := models.GetProyectosByClienteIDs(clienteIDs) - for i := range proyectos { - _ = models.ActualizarProgresoProyecto(proyectos[i].ID) + proyectos, err := models.GetProyectosByClienteIDs(clienteIDs) + if err != nil { + // Antes este error se descartaba y el usuario veía "no tenés + // proyectos": indistinguible de una caída de la base. + log.Printf("[PORTAL] dashboard: no se pudieron cargar los proyectos del usuario %d: %v", fullUser.ID, err) + return c.Status(500).Render("portal/dashboard", fiber.Map{ + "portalUser": fullUser, + "error": "No pudimos cargar tus proyectos en este momento. Volvé a intentar en un minuto.", + "isPartner": fullUser.EsPartner(), + }, "layouts/portal") } + models.AplicarProgreso(proyectos) return c.JSON(proyectos) } @@ -275,7 +295,7 @@ func PortalGetProyectoData(c *fiber.Ctx) error { facturas, _ := models.GetFacturasByCliente(proy.ClienteID, true) facturasNoLeidas := models.CountUnreadFacturaNotifs(u.ID) documentos, _ := models.GetDocumentosByProyecto(proy.ID) - _ = models.ActualizarProgresoProyecto(proy.ID) + proy.Progreso = models.ProgresoPorProyecto([]uint{proy.ID})[proy.ID] return c.JSON(fiber.Map{ "proyecto": proy,