Repository navigation
Conversation
…min/tags con service, controller, routes (Swagger) - Endpoints POST/GET/PATCH/DELETE /api/admin/tags - Tests unitarios (37 tests) - Registro en admin/index.js y app.js
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSe implementa una API CRUD administrativa para etiquetas en ChangesAdministración de Etiquetas de Producto
Sequence DiagramsequenceDiagram
participant Client
participant Express
participant Controller
participant TagService
participant Prisma
Client->>Express: POST /api/admin/tags {name}
Express->>Express: authenticate, requireRole(ROLES.ADMIN)
Express->>Controller: createAdminTag(req)
Controller->>TagService: createAdminTagService(name)
TagService->>Prisma: findFirst (duplicado case-insensitive)
Prisma-->>TagService: resultado
TagService->>Prisma: create productTags
Prisma-->>TagService: tag creada
TagService-->>Controller: DTO {id,name,status,createdAt}
Controller-->>Client: 201 JSON
Client->>Express: GET /api/admin/tags
Express->>Express: authenticate, requireRole(ROLES.ADMIN)
Express->>Controller: getAdminTags(req)
Controller->>TagService: getAllAdminTagsService()
TagService->>Prisma: findMany (status=true, orderBy name, select _count)
Prisma-->>TagService: tags con _count
TagService-->>Controller: array mapeado con productCount
Controller-->>Client: 200 JSON
Client->>Express: PATCH /api/admin/tags/:id {name}
Express->>Express: authenticate, requireRole(ROLES.ADMIN)
Express->>Controller: updateAdminTag(req)
Controller->>TagService: updateAdminTagService(id,name)
TagService->>Prisma: findUnique (validar existe)
Prisma-->>TagService: tag
TagService->>Prisma: findFirst (duplicado excluyendo id)
Prisma-->>TagService: resultado
TagService->>Prisma: update productTags
Prisma-->>TagService: tag actualizado
TagService-->>Controller: DTO actualizado
Controller-->>Client: 200 JSON
Client->>Express: DELETE /api/admin/tags/:id
Express->>Express: authenticate, requireRole(ROLES.ADMIN)
Express->>Controller: deleteAdminTag(req)
Controller->>TagService: deleteAdminTagService(id)
TagService->>Prisma: $transaction
Prisma->>Prisma: deleteMany productTagRelations
Prisma->>Prisma: update productTags (status=false)
Prisma-->>TagService: completado
TagService-->>Controller: -
Controller-->>Client: 204 No Content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/admin/admin-tags.test.js (1)
212-233: 💤 Low valueTest redundante con el caso 201 previo.
El test
"trim del nombre al crear"(212-233) verifica esencialmente lo mismo que"devuelve 201 y crea etiqueta con status=true"(179-210), que ya asserta el trim del nombre enprisma.productTags.create. Podés consolidarlos o aprovechar este test para cubrir un caso aún no contemplado (p. ej. duplicado de nombre soft-deleted). Solo una sugerencia menor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/admin/admin-tags.test.js` around lines 212 - 233, El test "trim del nombre al crear" duplica la verificación ya cubierta por "devuelve 201 y crea etiqueta con status=true"; elimina este test redundante OR repurposalo para cubrir un caso no probado: simular que prisma.productTags.findFirst devuelve un registro con el mismo name y status: false (soft-deleted) and then assert el comportamiento esperado (por ejemplo, que la API re-cree/active la etiqueta o responda con conflicto) usando los mismos helpers (asRole, request(app).post("/api/admin/tags")) y las mismas mocks (prisma.productTags.findFirst / prisma.productTags.create o prisma.productTags.update) para reflejar la lógica que quieras validar.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/admin/tags/admin-tag.service.js`:
- Around line 22-48: The current TOCTOU in admin-tag.service.js
(prisma.productTags.findFirst + prisma.productTags.create using normalizedName)
can create duplicate active tags because the DB lacks a unique constraint;
update the Prisma schema for model ProductTags to add a DB-level unique
constraint/index enforcing uniqueness for active names (e.g. a
partial/functional unique index on lower(name) where status = true or an
equivalent DB constraint) and run the migration, and update the service to catch
Prisma unique-constraint errors (P2002) thrown by prisma.productTags.create and
translate them into a consistent 409 response or ValidationError indicating the
tag already exists; reference prisma.productTags.findFirst,
prisma.productTags.create, normalizedName, and the ProductTags model when
locating code to change.
---
Nitpick comments:
In `@tests/unit/admin/admin-tags.test.js`:
- Around line 212-233: El test "trim del nombre al crear" duplica la
verificación ya cubierta por "devuelve 201 y crea etiqueta con status=true";
elimina este test redundante OR repurposalo para cubrir un caso no probado:
simular que prisma.productTags.findFirst devuelve un registro con el mismo name
y status: false (soft-deleted) and then assert el comportamiento esperado (por
ejemplo, que la API re-cree/active la etiqueta o responda con conflicto) usando
los mismos helpers (asRole, request(app).post("/api/admin/tags")) y las mismas
mocks (prisma.productTags.findFirst / prisma.productTags.create o
prisma.productTags.update) para reflejar la lógica que quieras validar.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fdaa3af1-4d9b-402d-8f6d-075f09a6ce76
📒 Files selected for processing (6)
src/app.jssrc/modules/admin/index.jssrc/modules/admin/tags/admin-tag.controller.jssrc/modules/admin/tags/admin-tag.routes.jssrc/modules/admin/tags/admin-tag.service.jstests/unit/admin/admin-tags.test.js
Comment on lines
+22
to
+48
| const existingTag = await prisma.productTags.findFirst({ | ||
| where: { | ||
| name: { | ||
| equals: normalizedName, | ||
| mode: "insensitive" | ||
| }, | ||
| status: true | ||
| }, | ||
| select: { id_product_tag: true } | ||
| }); | ||
|
|
||
| if (existingTag) { | ||
| throw new ValidationError("Ya existe una etiqueta con ese nombre"); | ||
| } | ||
|
|
||
| const createdTag = await prisma.productTags.create({ | ||
| data: { | ||
| name: normalizedName, | ||
| status: true | ||
| }, | ||
| select: { | ||
| id_product_tag: true, | ||
| name: true, | ||
| status: true, | ||
| created_at: true | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Buscar el modelo de etiquetas y restricciones en el schema de Prisma
fd -e prisma | xargs -I {} sh -c 'echo "== {} =="; cat "{}"' | rg -n -i -A20 'model\s+ProductTags|product_tag|@@unique|`@unique`'Repository: CrisNAC/BackendMarketplace
Length of output: 14068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Localizar schemas Prisma
SCHEMAS=$(fd -e prisma . | tr '\n' ' ')
echo "Schemas:"; echo "$SCHEMAS"
# Mostrar el bloque completo del modelo ProductTags
for f in $(fd -e prisma .); do
echo "===== $f (ProductTags) ====="
rg -n -S "model\s+ProductTags\s*\{" "$f" -n || true
# Capturar ~60 líneas desde la definición para incluir @@unique/@@index si existiera
rg -n -S "model\s+ProductTags\s*\{" "$f" -n | awk -F: '{print $1 ":" $2}' >/dev/null || true
# Mejor: usar un rango fijo desde la primera aparición
line=$(rg -n -S "model\s+ProductTags\s*\{" "$f" | head -n1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line))
end=$((line+80))
sed -n "${start},${end}p" "$f"
fi
done | head -n 200Repository: CrisNAC/BackendMarketplace
Length of output: 3761
🏁 Script executed:
#!/bin/bash
set -euo pipefail
SCHEMA_FILES=$(fd -e prisma .)
echo "Prisma files:"
echo "$SCHEMA_FILES"
# 1) Mostrar el bloque completo de ProductTags (por rango de líneas)
echo "----- ProductTags block (context) -----"
for f in $SCHEMA_FILES; do
line=$(rg -n -S "model\s+ProductTags\s*\{" "$f" | head -n1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-5))
end=$((line+60))
echo "==== $f lines $start-$end ===="
sed -n "${start},${end}p" "$f"
fi
done
# 2) Buscar restricciones únicas o índices relacionados con "ProductTags" o "name"
echo "----- Search unique/index patterns -----"
rg -n -S "ProductTags|@@unique|`@unique`|@@index|`@index`" $SCHEMA_FILES | rg -n -S "ProductTags|name|productTags|product_tag" || true
# 3) Buscar explícitamente si "name" tiene `@unique` dentro de ProductTags
echo "----- Search `@unique` with name -----"
rg -n -S "`@unique`" $SCHEMA_FILES | rg -n -S "name|ProductTags" || trueRepository: CrisNAC/BackendMarketplace
Length of output: 3038
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Buscar migraciones que toquen product_tags / producttags / name con unique
if [ -d prisma/migrations ]; then
echo "== scanning prisma/migrations =="
rg -n -i --hidden \
"(product[_-]tags|producttags|ProductTags)|unique|constraint|index|lower\\(.*name\\)|citext|collate" \
prisma/migrations \
|| true
else
echo "No existe prisma/migrations en este repo."
fiRepository: CrisNAC/BackendMarketplace
Length of output: 24810
Posible duplicación por falta de restricción única (y condición de carrera) en ProductTags
En el esquema actual (model ProductTags) no hay @unique/@@unique sobre name, así que hoy no hay un índice único que dispare P2002; igual, el patrón findFirst(...status: true) + create(...) tiene una ventana TOCTOU y permite crear dos etiquetas activas con el mismo normalizedName ante requests concurrentes. También puede terminar habiendo varias filas con el mismo name si existe una etiqueta soft-deleted (status=false) y llegan dos altas en paralelo.
Conviene imponer unicidad a nivel DB para el caso “activo” (p.ej. índice único parcial por lower(name) donde status=true) y, si se aplica esa restricción, capturar el error de conflicto para devolver un 409/400 consistente.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/admin/tags/admin-tag.service.js` around lines 22 - 48, The
current TOCTOU in admin-tag.service.js (prisma.productTags.findFirst +
prisma.productTags.create using normalizedName) can create duplicate active tags
because the DB lacks a unique constraint; update the Prisma schema for model
ProductTags to add a DB-level unique constraint/index enforcing uniqueness for
active names (e.g. a partial/functional unique index on lower(name) where status
= true or an equivalent DB constraint) and run the migration, and update the
service to catch Prisma unique-constraint errors (P2002) thrown by
prisma.productTags.create and translate them into a consistent 409 response or
ValidationError indicating the tag already exists; reference
prisma.productTags.findFirst, prisma.productTags.create, normalizedName, and the
ProductTags model when locating code to change.
Collaborator
|
revisar sugerencias del conejo |
…ive tags and catching P2002 error
|
leoAchu16
approved these changes
May 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…min/tags con service, controller, routes (Swagger) - Endpoints POST/GET/PATCH/DELETE /api/admin/tags - Tests unitarios (37 tests) - Registro en admin/index.js y app.js
Summary by CodeRabbit
New Features
Validaciones
Tests