Conversation
- ampliar prompts para discovery/planificación del MVP - reestructurar readme con objetivos, arquitectura, modelo de datos, HU y tickets - añadir especificación OpenAPI 3.1 y carpeta de documentación por HU (BE/FE/DB/testing)
- ampliar prompts para discovery/planificación del MVP - reestructurar readme con objetivos, arquitectura, modelo de datos, HU y tickets - añadir especificación OpenAPI 3.1 y carpeta de documentación por HU (BE/FE/DB/testing)
📝 WalkthroughWalkthroughThis pull request introduces comprehensive documentation for the CitaYa medical appointment platform MVP, including a complete OpenAPI 3.1.0 specification, architectural reference, and ten structured user stories (HU1–HU10) with backend, database, frontend, and testing specifications for all core features: patient/doctor registration, doctor search, appointment booking/rescheduling, profile management, document verification, scheduling, reviews, and admin dashboard. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes This PR introduces 10,000+ lines of structured documentation for a complete MVP feature set. While repetitive in organization, each user story (HU1–HU10) contains distinct technical specifications across backend, database, frontend, and testing domains. Review requires verification of architectural consistency, database schema alignment across migrations, API specification completeness, and validation of acceptance criteria across multiple feature flows. The scope and heterogeneity of feature specifications demand systematic cross-referencing. Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
prompts.md (1)
71-116: Conflicting rule about active appointments.
Line 73 says there is no maximum of active appointments per patient, but Line 115 says the patient/doctor can have only one active appointment at a time. Please reconcile or annotate the final decision to avoid spec drift.
🤖 Fix all issues with AI agents
In `@documentation/HU3/frontend/HU3-FE-001-Busqueda-Medicos.md`:
- Around line 231-241: The JSX uses router.push inside the component when
rendering doctor cards but never defines router; import and initialize Next's
client router by adding an import for useRouter from "next/navigation" and
calling const router = useRouter() at the top of the component (the same
component rendering the doctor list), then leave the existing
router.push(`/doctors/${doctor.id}`) call unchanged so navigation works; ensure
the useRouter import and const router = useRouter() are placed in the component
scope that contains the map over data?.doctors.
In `@documentation/HU4/frontend/HU4-FE-001-Reserva-Cita.md`:
- Around line 111-114: The onSuccess callback uses undefined symbols: `router`
and `data`. Import and initialize the router inside the component (e.g., via
useRouter) so `router.push` is available, and accept the mutation result as the
onSuccess parameter (e.g., onSuccess: (result) => { ... }) then use that
parameter to read the created appointment id (e.g., result.id or result.data.id
depending on your mutation shape) when calling router.push and invalidating
queries; update the `onSuccess` signature and add the router initialization
accordingly.
🟠 Major comments (25)
documentation/HU10/backend/HU10-BE-001-Dashboard-Admin.md-35-37 (1)
35-37: Cache invalidation strategy needs more specificity.Line 37 mentions "invalidate métricas si se ejecuta batch o acciones críticas" but doesn't specify which "acciones críticas" trigger invalidation.
Document specific cache invalidation triggers:
- Which admin actions invalidate which cache keys?
- Does approving a doctor invalidate
admin:metrics?- Does a new appointment invalidate
admin:reservations:{filters}?Clear invalidation rules prevent stale data and help developers maintain consistency.
documentation/HU5/database/HU5-DB-001-Migracion-Update-APPOINTMENTS.md-31-34 (1)
31-34: Inconsistency: Entity definition mixes TEXT type with length constraint.Similar to the migration issue, TypeORM's
@Columndecorator withtype: 'text'andlength: 500is inconsistent. TEXT types don't honor length constraints at the database level.Use
type: 'varchar'withlength: 500for proper enforcement.🔧 Proposed fix
-@Column({ type: 'text', nullable: true, length: 500 }) +@Column({ type: 'varchar', length: 500, nullable: true }) cancellationReason?: string;documentation/HU5/database/HU5-DB-001-Migracion-Update-APPOINTMENTS.md-20-22 (1)
20-22: Inconsistency: TEXT type doesn't support length constraints.The migration code uses
type: 'text'which doesn't support thelengthproperty in most SQL databases. TEXT columns have their own maximum sizes defined by the database engine, and you cannot specify a custom length.For a 500-character limit, use
VARCHAR(500)instead.🔧 Proposed fix
await queryRunner.addColumns('APPOINTMENTS', [ - new TableColumn({ name: 'cancellation_reason', type: 'text', isNullable: true }), + new TableColumn({ name: 'cancellation_reason', type: 'varchar', length: '500', isNullable: true }), ]);documentation/HU1/database/HU1-DB-002-Migracion-Tabla-Audit-Logs.md-54-60 (1)
54-60: Potential conflict: UUID generation strategy.Lines 58-59 specify both
generationStrategy: 'uuid'anddefault: '(UUID())'. This creates a potential conflict:
generationStrategy: 'uuid'tells TypeORM to generate UUIDs at the application leveldefault: '(UUID())'is MySQL-specific database-level generationRecommendation: Choose one approach:
- Option 1 (Recommended): Use TypeORM's
generationStrategy: 'uuid'without database default (portable across databases)- Option 2: Use database-level default and remove
generationStrategy(database-specific but works with raw SQL inserts)🔧 Proposed fix (Option 1 - Portable)
{ name: 'id', type: 'varchar', length: '36', isPrimary: true, - generationStrategy: 'uuid', - default: '(UUID())', + isGenerated: true, + generationStrategy: 'uuid', },documentation/HU3/frontend/HU3-FE-001-Busqueda-Medicos.md-125-158 (1)
125-158: Pagination controls don’t affect the query.
page/limitare hardcoded to"1"and"20"in the query params, but the UI updatespageviasetValue. This will make pagination UI non-functional. Please wirepageandlimitfrom form state intoURLSearchParamsand include them in thequeryKey.✅ Suggested fix
- const { data, isLoading, error } = useQuery({ - queryKey: ['doctors', { specialty, lat, lng, postalCode, radius, date }], + const page = watch('page'); + const limit = watch('limit'); + const { data, isLoading, error } = useQuery({ + queryKey: ['doctors', { specialty, lat, lng, postalCode, radius, date, page, limit }], queryFn: async () => { const params = new URLSearchParams({ specialty: specialty || '', radius: radius.toString(), - page: '1', - limit: '20', + page: page.toString(), + limit: limit.toString(), });Also applies to: 246-264
documentation/HU3/frontend/HU3-FE-001-Busqueda-Medicos.md-16-19 (1)
16-19: Auth requirement not reflected in the flow.CA1 requires redirecting unauthenticated users, but the sample only adds an
Authorizationheader fromlocalStorage. Consider documenting explicit redirect/guard behavior (or mention that middleware guards the page) to align with CA1.Also applies to: 146-149
documentation/HU3/backend/HU3-BE-001-Busqueda-Medicos.md-98-116 (1)
98-116: Pagination cache key is incomplete.
generateCacheKeyomitspage/limit, so cached results will be reused across pages and limits. Includepageandlimitin the key (and in filters defaulting) to avoid serving wrong pages.✅ Suggested fix
private generateCacheKey(filters: SearchFilters): string { const parts = [ 'doctors', filters.specialty || 'all', filters.lat || '0', filters.lng || '0', filters.postalCode || '0', filters.radius || '5', filters.date || 'all', + filters.page || '1', + filters.limit || '20', ]; return parts.join(':'); }Also applies to: 139-171, 176-186
documentation/HU2/frontend/HU2-FE-001-Registro-Medico.md-39-41 (1)
39-41: Spec says store refreshToken, but sample only stores accessToken.Either update the sample to store
refreshTokenor adjust CA6 to match actual storage behavior.✅ Suggested fix
- localStorage.setItem('accessToken', result.accessToken); + localStorage.setItem('accessToken', result.accessToken); + if (result.refreshToken) { + localStorage.setItem('refreshToken', result.refreshToken); + }Also applies to: 154-169
documentation/HU5/testing/HU5-TEST-001-Testing-Reprogramacion-Cancelacion.md-19-23 (1)
19-23: Missing audit-log and idempotency checks in tests.Backend spec requires audit log entries for cancel/reschedule. Please add test cases validating audit_log records and behavior under client retries (idempotent cancel/reschedule or clear error).
Also applies to: 29-33
api-specification.yaml-61-123 (1)
61-123: Doctor registration payload misses required address/postalCode fields.
HU2 requires address + postalCode at registration (for geocoding), but/auth/registeronly accepts basic user fields. Align by extending the schema (e.g., patient/doctoroneOf) or update HU2 to collect these after registration to avoid inconsistent implementation.documentation/HU1/database/HU1-DB-001-Migracion-Tabla-USERS.md-247-253 (1)
247-253: Security concern: example uses plaintext-like password values.The verification SQL example inserts passwords as simple strings like
'hash123'and'hash456'. While this is documented as an example, it could mislead developers. Ensure the documentation clearly emphasizes that actual implementations must use proper password hashing (bcrypt with appropriate cost factor).📝 Improve example to show realistic password hash format
-- Verificar constraint de email único INSERT INTO USERS (email, password, firstName, lastName, role) -VALUES ('test@example.com', 'hash123', 'Test', 'User', 'patient'); +VALUES ('test@example.com', '$2b$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy', 'Test', 'User', 'patient'); -- Intentar insertar email duplicado (debe fallar) INSERT INTO USERS (email, password, firstName, lastName, role) -VALUES ('test@example.com', 'hash456', 'Test2', 'User2', 'patient'); +VALUES ('test@example.com', '$2b$10$DifferentHashValueForDifferentPassword12345678', 'Test2', 'User2', 'patient'); -- Error esperado: Duplicate entry 'test@example.com' for key 'IDX_USERS_EMAIL'Add a note emphasizing this is a bcrypt hash example and never store plaintext passwords.
documentation/HU8/database/HU8-DB-001-Tablas-DOCTOR_SCHEDULES-y-SLOTS.md-12-12 (1)
12-12: Document the dayOfWeek convention.The
dayOfWeekfield uses TINYINT (0-6), but the documentation doesn't specify which day corresponds to 0. Common conventions include Sunday=0 (JavaScript) or Monday=0 (ISO 8601). Explicitly documenting this prevents confusion and implementation errors.📝 Add clarification to the documentation
## Estructura DOCTOR_SCHEDULES - `id` UUID PK - `doctor_id` UUID FK -> DOCTORS (CASCADE) -- `dayOfWeek` TINYINT (0-6) NOT NULL +- `dayOfWeek` TINYINT (0-6) NOT NULL -- 0=Domingo, 1=Lunes, ..., 6=Sábado (convención JavaScript Date.getDay()) - `startTime` TIME NOT NULLdocumentation/HU9/frontend/HU9-FE-001-Crear-Resena.md-12-12 (1)
12-12: Clarify text sanitization requirements.The documentation mentions "sanitizar texto antes de enviar" but doesn't specify what sanitization entails. For a review system, clarify:
- XSS Prevention: Strip or escape HTML tags?
- Profanity Filtering: Is this handled client-side or server-side?
- Character Normalization: Trim whitespace, normalize Unicode?
Typically, input sanitization for user-generated content should be primarily server-side, with client-side validation for UX only.
📝 Add clarification to the documentation
## CA cubiertos - Auth patient y dueño de la cita completada. - Formulario: rating 1-5 (estrellas), comentario 10-1000 chars, contador de caracteres. -- Validaciones en tiempo real; sanitizar texto antes de enviar. +- Validaciones en tiempo real; sanitizar texto antes de enviar (trim whitespace, escape HTML). +- **Nota**: El backend realizará sanitización adicional y moderación de contenido. - Mensajes: éxito "pendiente de moderación"; errores 403/404/400/409.documentation/HU6/database/HU6-DB-001-Ajustes-DOCTORS.md-11-11 (1)
11-11: Enforce bio TEXT max length constraint.The documentation specifies
bioas "TEXT, nullable, max 1000", but the TEXT type in MySQL doesn't inherently enforce a maximum length of 1000 characters—TEXT can hold up to 65,535 characters. You must enforce this limit at the application level (backend validation) or use VARCHAR(1000) instead.♻️ Consider using VARCHAR(1000) or ensure application-level validation
Option 1: Use VARCHAR for database-level enforcement:
-- Asegurar columnas: `bio` (TEXT, nullable, max 1000), `address` (VARCHAR 500), `postalCode` (VARCHAR 20), `latitude` DECIMAL(10,8), `longitude` DECIMAL(11,8). +- Asegurar columnas: `bio` (VARCHAR(1000), nullable), `address` (VARCHAR 500), `postalCode` (VARCHAR 20), `latitude` DECIMAL(10,8), `longitude` DECIMAL(11,8).Option 2: Keep TEXT but explicitly document the application-level validation requirement:
-- Asegurar columnas: `bio` (TEXT, nullable, max 1000), `address` (VARCHAR 500), `postalCode` (VARCHAR 20), `latitude` DECIMAL(10,8), `longitude` DECIMAL(11,8). +- Asegurar columnas: `bio` (TEXT, nullable, validar max 1000 en backend), `address` (VARCHAR 500), `postalCode` (VARCHAR 20), `latitude` DECIMAL(10,8), `longitude` DECIMAL(11,8).And ensure the backend validation is documented in HU6-BE-001.
documentation/TICKETS_INDEX.md-310-321 (1)
310-321: Correct estimation totals—they contain multiple arithmetic errors.The summary section claims ~426 hours (~70 story points) and 11 weeks, but the actual totals are:
- Frontend: 117 hours (not ~127)
- Backend: 152 hours (not ~162)
- Database: 31 hours (not ~26)
- Testing: 101 hours (not ~111)
- Grand Total: 401 hours (not ~426)
This represents a 25-hour discrepancy. At 40 hours/week, the project is approximately 10 weeks, not 11.
documentation/HU10/database/HU10-DB-001-Soporte-Dashboard.md-17-24 (1)
17-24: Specify Redis cache strategy details.While Redis caching is mentioned as central to the dashboard approach, this document lacks critical cache strategy details such as:
- TTL for cached metrics
- Cache invalidation triggers (e.g., when appointments/reviews are modified)
- Cache warming strategy on cold start
- Cache key naming conventions
If these are defined in HU10-BE-001, add a cross-reference. Otherwise, consider documenting them here to ensure the database and backend implementations align.
documentation/HU1/backend/HU1-BE-001-Registro-Paciente.md-265-276 (1)
265-276: Avoid returning refresh tokens in the response body.If the refresh token is in an httpOnly cookie, also returning it in JSON weakens XSS defenses. Prefer cookie‑only delivery (or document the risk explicitly if required).
documentation/HU1/backend/HU1-BE-001-Registro-Paciente.md-91-92 (1)
91-92: Restrictroleto patient for this endpoint.CA1 states role must be
"patient", but the DTO allows"doctor", enabling unintended self‑registration as doctor via the patient flow. Align the DTO with CA1.documentation/HU6/backend/HU6-BE-001-Gestion-Perfil-Medico.md-13-14 (1)
13-14: Clarify geocoding failure handling and user feedback.Line 13 states that if geocoding fails, the system should show a warning but save the data anyway. However, the specification doesn't clarify:
- What should happen to existing coordinates if geocoding fails for an update?
- Should the user be able to proceed despite the warning, or should they be required to acknowledge it?
- How will this warning be surfaced in the API response?
Consider specifying:
- Response format when geocoding fails (e.g.,
{ success: true, warning: 'Geocoding failed...', profile: {...} })- Whether to preserve old coordinates or set them to null when geocoding fails
- Whether a partial update (data saved but coordinates not updated) is acceptable for the business logic
💡 Proposed specification clarification
Add to the CA section:
### Geocoding Failure Handling - Si geocoding falla durante actualización de dirección/CP: - Guardar firstName, lastName, phone, bio, address, postalCode normalmente - Preservar coordenadas existentes (NO actualizarlas ni establecerlas a null) - Retornar HTTP 200 con estructura: ```json { "success": true, "warning": "Dirección guardada pero no se pudo geocodificar. Las coordenadas en el mapa pueden no ser precisas.", "profile": { ... } } ``` - Registrar warning en logs para análisis posteriordocumentation/HU4/frontend/HU4-FE-001-Reserva-Cita.md-71-84 (1)
71-84: Missing error handling for slot fetch.The
queryFnat Lines 73-82 doesn't handle non-ok responses or network errors properly. If the API returns an error status,response.json()might fail or return an error object that gets treated as valid data.🔧 Add proper error handling
queryFn: async () => { const response = await fetch( `/api/v1/doctors/${doctorId}/slots?date=${format(selectedDate, 'yyyy-MM-dd')}`, { headers: { Authorization: `Bearer ${localStorage.getItem('accessToken')}`, }, }, ); + + if (!response.ok) { + throw new Error(`Failed to fetch slots: ${response.status}`); + } + return response.json(); },documentation/HU4/frontend/HU4-FE-001-Reserva-Cita.md-104-115 (1)
104-115: Incomplete error handling for appointment reservation.The mutation error handling only checks
response.okbut doesn't handle specific error codes mentioned in CA5 (acceptance criteria 5):
- Error 400: Slot no disponible
- Error 400: Paciente tiene cita activa
- Error 409: Slot ya reservado por otro paciente
The current implementation will show a generic error message for all failures.
♻️ Add specific error handling per CA5
if (!response.ok) { const error = await response.json(); - throw new Error(error.error || 'Error al reservar cita'); + + if (response.status === 409) { + throw new Error('Este horario ya fue reservado por otro paciente. Por favor selecciona otro horario.'); + } else if (response.status === 400) { + throw new Error(error.message || 'No se puede reservar la cita. Verifica que no tengas otra cita activa.'); + } else { + throw new Error(error.message || 'Error al reservar cita'); + } }documentation/HU1/frontend/HU1-FE-001-Registro-Paciente.md-196-199 (1)
196-199: Hardcoded language in ReCAPTCHA provider.Line 198 hardcodes the language to Spanish (
language="es"), but CA6 (acceptance criteria 6) states the system should support both Spanish and English, with automatic detection based on browser preferences.🌐 Dynamic language detection
+import { useLocale } from 'next-intl'; + export default function RootLayout({ children }) { + const locale = useLocale(); + return ( <GoogleReCaptchaProvider reCaptchaKey={process.env.NEXT_PUBLIC_RECAPTCHA_SITE_KEY} - language="es" + language={locale} > {children} </GoogleReCaptchaProvider> ); }Assuming
next-intlis configured to detect the browser language automatically, this will pass the correct locale to reCAPTCHA.documentation/HU4/backend/HU4-BE-001-Reserva-Cita.md-150-153 (1)
150-153: Notification queue is inside the transaction despite the comment.The comment on Line 150 states "fuera de transacción" (outside transaction), but the code shows
await this.notificationQueue.add()is called inside the transaction block (before the closing brace at Line 156). This means:
- If the queue operation fails, the entire appointment transaction will rollback
- The database transaction will be held open longer while the queue operation completes
- This contradicts the acceptance criteria CA6 which states notifications should be asynchronous and not block the response
🔧 Move notification queue outside transaction
// 5. Crear registro en historial await manager.save(AppointmentHistory, { appointmentId: savedAppointment.id, oldStatus: null, newStatus: 'confirmed', changeReason: 'Cita creada', changedBy: patientId, }); - // Encolar notificaciones (fuera de transacción) - await this.notificationQueue.add('send-appointment-confirmation', { - appointmentId: savedAppointment.id, - }); - return savedAppointment; }); + + // Encolar notificaciones (fuera de transacción) + await this.notificationQueue.add('send-appointment-confirmation', { + appointmentId: appointment.id, + }).catch(err => { + // Log error but don't fail the appointment creation + console.error('Failed to queue notification:', err); + }); + + return appointment; } }documentation/HU4/testing/HU4-TEST-001-Testing-Reserva-Cita.md-115-121 (1)
115-121: Test dependencies not properly set up.The test references
historyRepositoryandappointmentServiceat Lines 117 and 120, but these are not defined, injected, or imported in the test. For a NestJS integration test, these need to be properly retrieved from the testing module.♻️ Add proper dependency injection for test
describe('Transacciones ACID', () => { + let appointmentService: AppointmentService; + let historyRepository: Repository<AppointmentHistory>; + let appointmentRepository: Repository<Appointment>; + let slotRepository: Repository<Slot>; + + beforeAll(async () => { + // ... existing setup ... + + appointmentService = moduleFixture.get<AppointmentService>(AppointmentService); + historyRepository = moduleFixture.get('AppointmentHistoryRepository'); + appointmentRepository = moduleFixture.get('AppointmentRepository'); + slotRepository = moduleFixture.get('SlotRepository'); + }); + it('debe hacer rollback completo si falla cualquier paso', async () => { // Simular error en creación de historial jest.spyOn(historyRepository, 'save').mockRejectedValueOnce(new Error('DB Error'));documentation/HU4/testing/HU4-TEST-001-Testing-Reserva-Cita.md-62-78 (1)
62-78: Test setup is incomplete.Lines 72-73 have a comment indicating setup code for "Crear paciente y obtener token" and "Crear médico y slot disponible" but the actual implementation is missing. Additionally, Line 78 calls
createTestPatientAndGetToken()which is neither defined nor imported.Since this is testing documentation, clarify whether this is meant to be:
- A complete, runnable test example (in which case the setup needs to be implemented)
- A skeleton/outline showing the test structure (in which case add a note explaining developers need to implement these helpers)
💡 Add helper function implementation or placeholder
If this should be a complete example:
beforeAll(async () => { const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [AppModule], }).compile(); app = moduleFixture.createNestApplication(); await app.init(); dataSource = moduleFixture.get<DataSource>(DataSource); - // Crear paciente y obtener token - // Crear médico y slot disponible + // Crear paciente y obtener token + const patientResponse = await request(app.getHttpServer()) + .post('/api/v1/auth/register') + .send({ + email: 'patient1@test.com', + password: 'Test1234', + firstName: 'Test', + lastName: 'Patient', + role: 'patient', + }); + patientToken = patientResponse.body.accessToken; + + // Crear médico y slot disponible + const doctorResponse = await request(app.getHttpServer()) + .post('/api/v1/auth/register') + .send({ + email: 'doctor1@test.com', + password: 'Test1234', + firstName: 'Test', + lastName: 'Doctor', + role: 'doctor', + }); + doctorId = doctorResponse.body.user.id; + + // Crear slot disponible + const slotResponse = await dataSource + .getRepository(Slot) + .save({ + doctorId, + startTime: new Date('2026-02-01T10:00:00-06:00'), + endTime: new Date('2026-02-01T10:30:00-06:00'), + isAvailable: true, + }); + slotId = slotResponse.id; }); it('debe prevenir doble booking cuando dos pacientes intentan reservar el mismo slot', async () => { // Crear segundo paciente - const patient2Token = await createTestPatientAndGetToken(); + const patient2Response = await request(app.getHttpServer()) + .post('/api/v1/auth/register') + .send({ + email: 'patient2@test.com', + password: 'Test1234', + firstName: 'Test2', + lastName: 'Patient2', + role: 'patient', + }); + const patient2Token = patient2Response.body.accessToken;Or if it's meant as a skeleton, add a note:
beforeAll(async () => { // ... setup code ... - // Crear paciente y obtener token - // Crear médico y slot disponible + // TODO: Implementar creación de paciente de prueba y obtener token JWT + // TODO: Implementar creación de médico y slot disponible para pruebas + // Estos helpers deben configurarse según la estrategia de testing del proyecto });
🟡 Minor comments (12)
documentation/HU2/backend/HU2-BE-001-Registro-Medico.md-12-12 (1)
12-12: Clarify Spanish punctuation around “dirección”.There’s an extra comma in “geocodificación de dirección, y establece…”. Consider “dirección y establece…”.
documentation/HU2/backend/HU2-BE-001-Registro-Medico.md-40-42 (1)
40-42: Align DTO file path references.Line 41 points to
backend/src/dto/auth/register.dto.ts, but Line 146 listsbackend/src/dto/auth/register-doctor.dto.ts. Please make these consistent to avoid implementation ambiguity.Also applies to: 146-146
documentation/HU2/backend/HU2-BE-001-Registro-Medico.md-3-8 (1)
3-8: Heading block reads as a single line; consider spacing for readability.LanguageTool flags spacing/casing around the bullet list under “Información General”. Consider adding blank lines or standard list formatting to avoid run‑on text rendering.
documentation/HU7/backend/HU7-BE-001-Carga-Documentos-Verificacion.md-34-36 (1)
34-36: Clarify encryption scope: files vs. metadata.Line 36 mentions "claves de encriptación en .env (si se cifra metadatos)" but line 15 references "directorio encriptado".
Clarify whether:
- Files are encrypted at rest (and which encryption method: filesystem-level, application-level)
- Only metadata is encrypted
- Or both
This impacts key management, backup/restore procedures, and performance considerations.
documentation/HU7/backend/HU7-BE-001-Carga-Documentos-Verificacion.md-14-15 (1)
14-15: Document the deployment user context for the upload directory.The 700 permission specification is intentional per GDPR compliance requirements (documented in readme.md and ARQUITECTURA_MVP.md). However, this documentation doesn't clarify the deployment context needed to understand how the application successfully writes to this directory.
Add clarification about:
- The system user that owns
/var/www/citaya/storage/uploads/in the deployment environment- How the Node.js process user is configured to match that ownership (relevant since this is a Docker volume mount per ARQUITECTURA_MVP.md)
- Reference to the deployment setup that establishes this user/permission alignment
This prevents misunderstanding about why 700 permissions work in practice despite being owner-only.
documentation/HU3/backend/HU3-BE-001-Busqueda-Medicos.md-98-116 (1)
98-116:lat && lngcheck breaks valid coordinates at 0.Use explicit null/undefined checks for coordinates;
0is a valid value and currently disables proximity search.✅ Suggested fix
- if (filters.lat && filters.lng) { + if (filters.lat !== undefined && filters.lng !== undefined) {documentation/HU2/frontend/HU2-FE-001-Registro-Medico.md-159-162 (1)
159-162:Retry-Afteris in seconds, not minutes.The error message treats the header as minutes. Convert seconds to minutes (or display seconds) to avoid misleading UX.
✅ Suggested fix
- const retryAfter = response.headers.get('Retry-After'); - setError(t('auth.rateLimitExceeded', { minutes: retryAfter })); + const retryAfter = response.headers.get('Retry-After'); + const retrySeconds = retryAfter ? Number(retryAfter) : undefined; + const retryMinutes = retrySeconds ? Math.ceil(retrySeconds / 60) : undefined; + setError(t('auth.rateLimitExceeded', { minutes: retryMinutes }));prompts.md-31-35 (1)
31-35: Remove stray character.
The “ß” on Line 34 looks accidental and breaks the prompt text.🩹 Proposed fix
-* Administrador del sistema / Automatización -ß +* Administrador del sistema / Automatizaciónprompts.md-24-61 (1)
24-61: Use headings instead of bold for section titles (MD036).
Markdownlint flags bold text used as headings here; convert them to real headings to avoid lint failures.🧾 Suggested cleanup
-**OBJETIVO** +### OBJETIVO -**ACTORES** +### ACTORES -**DENTRO DEL ALCANCE (IN-SCOPE)** +### DENTRO DEL ALCANCE (IN-SCOPE) -**FUERA DEL ALCANCE (POST-MVP)** +### FUERA DEL ALCANCE (POST-MVP) -**FLUJOS CLAVE** +### FLUJOS CLAVE -**TAREA `#1`** +### TAREA `#1`documentation/HU8/database/HU8-DB-001-Tablas-DOCTOR_SCHEDULES-y-SLOTS.md-13-14 (1)
13-14: Schema should clarify the relationship between recurring schedules and the system timezone configuration.The
startTimeandendTimeTIME fields are appropriate for recurring weekly patterns, but the schema lacks context linking to the system-wide timezone (CDMX / America/Mexico_City, defined in SYSTEM_SETTINGS and documented in readme.md). The actual appointment times are stored in the SLOTS table with explicit timezone documentation; consider adding a note to DOCTOR_SCHEDULES indicating that times are interpreted in the system's configured timezone when generating SLOTS.The same applies to
created_atandupdated_atTIMESTAMP fields (line 19), which should clarify timezone handling for audit trails.documentation/HU4/testing/HU4-TEST-001-Testing-Reserva-Cita.md-139-156 (1)
139-156: Test references undefined method and variable.Line 143 calls
appointmentService.lockSlot(slotId, patientId)but this method is not defined in the backend specification (HU4-BE-001). The backend doc only showscreateAppointment()which handles locking internally.Additionally, Line 152 references
patient2Tokenwhich is not defined in this test's setup.Either:
- Update the backend spec to include a public
lockSlot()method if slot locking should be exposed separately, or- Modify this test to verify locking behavior through the
createAppointment()flow💡 Alternative test approach without lockSlot method
it('debe bloquear slot por 5 minutos al iniciar reserva', async () => { - const slot = await slotRepository.findOne({ where: { id: slotId } }); - - // Iniciar reserva (sin completar) - await appointmentService.lockSlot(slotId, patientId); + // Iniciar reserva del primer paciente (esto debe bloquear el slot) + const appointmentPromise = appointmentService.createAppointment( + patientId, + doctorId, + slotId, + new Date('2026-02-01T10:00:00-06:00') + ); + // Verificar que el slot está bloqueado mientras se procesa const lockedSlot = await slotRepository.findOne({ where: { id: slotId } }); expect(lockedSlot.lockedBy).toBe(patientId); expect(lockedSlot.lockedUntil).toBeDefined(); - // Verificar que otro paciente no puede reservar + // Mientras tanto, crear segundo paciente + const patient2Response = await request(app.getHttpServer()) + .post('/api/v1/auth/register') + .send({ + email: 'patient2@test.com', + password: 'Test1234', + firstName: 'Test2', + lastName: 'Patient2', + role: 'patient', + }); + const patient2Token = patient2Response.body.accessToken; + + // Intentar reservar desde otro paciente (debe fallar por lock) const response = await request(app.getHttpServer()) .post('/api/v1/appointments') .set('Authorization', `Bearer ${patient2Token}`) .send({ doctorId, slotId, appointmentDate }); expect(response.status).toBe(409); + + // Completar la primera reserva + await appointmentPromise; });documentation/HU4/frontend/HU4-FE-001-Reserva-Cita.md-148-149 (1)
148-149: Client-side time comparison vulnerable to clock skew.The locked slot detection at Lines 148-149 uses
new Date(slot.lockedUntil) > new Date()which compares the server-provided timestamp with client time. If the client's clock is incorrect or in a different timezone, slots may appear locked or unlocked incorrectly.Consider one of these approaches:
- Include a server timestamp in the API response and use it as the reference time
- Handle slot availability entirely server-side and return a simple
isCurrentlyLockedboolean flag- Document that slot availability is eventual and may have brief inconsistencies
The simplest solution is option 2:
💡 Server-side locked status
Update the Slot interface and let the backend calculate lock status:
interface Slot { id: string; startTime: string; endTime: string; isAvailable: boolean; - lockedUntil?: string; + isLocked: boolean; }Then in the component:
{slots?.map((slot: Slot) => { - const isLocked = slot.lockedUntil && new Date(slot.lockedUntil) > new Date(); const isSelected = selectedSlot?.id === slot.id; return ( <button key={slot.id} - onClick={() => !isLocked && slot.isAvailable && setSelectedSlot(slot)} - disabled={!slot.isAvailable || isLocked} + onClick={() => !slot.isLocked && slot.isAvailable && setSelectedSlot(slot)} + disabled={!slot.isAvailable || slot.isLocked} className={`p-4 rounded border ${ isSelected ? 'border-blue-500 bg-blue-50' - : slot.isAvailable && !isLocked + : slot.isAvailable && !slot.isLocked ? 'border-gray-300 hover:border-blue-300' : 'border-gray-200 bg-gray-100 opacity-50' }`} > {format(new Date(slot.startTime), 'HH:mm')} - {isLocked && <span className="text-xs text-orange-500">Reservando...</span>} + {slot.isLocked && <span className="text-xs text-orange-500">Reservando...</span>} </button> ); })}
🧹 Nitpick comments (32)
documentation/HU3/testing/HU3-TEST-001-Testing-Busqueda-Medicos.md (1)
18-20: Clarify pagination expectation (“20/50”).
Line 20 is ambiguous—specify whether 20/50 are page-size options, default size, or items-per-page vs total. This avoids inconsistent test implementations.documentation/HU7/testing/HU7-TEST-001-Testing-Carga-Documentos.md (1)
11-15: Add test strategy for malware detection.
Line 14 requires malware detection, but the plan doesn’t specify how to simulate it (mock AV service, fixture, or test flag). Adding this avoids flaky or blocked tests.documentation/HU9/backend/HU9-BE-001-Crear-Resena.md (1)
12-21: Clarify length validation order vs sanitization/trim.
Lines 12–20 define 10–1000 chars plus trim/sanitize, but don’t state whether the length check happens before or after sanitize/trim or how DB constraints enforce it. Specify the order to avoid inconsistencies between API validation and persistence.documentation/HU7/frontend/HU7-FE-001-Carga-Documentos-Verificacion.md (1)
21-24: Consider accessibility for file upload progress.While the optional progress bar (line 24) is mentioned, ensure it's implemented accessibly with ARIA attributes (
role="progressbar",aria-valuenow,aria-valuemin,aria-valuemax,aria-label) for screen reader users.documentation/HU10/backend/HU10-BE-001-Dashboard-Admin.md (1)
24-25: Consider monitoring and fallback for batch job failures.Line 25 describes a daily batch job (2:00 AM) that populates Redis cache with 24h TTL. If the batch job fails, metrics will become stale or unavailable.
Recommendations:
- Implement monitoring/alerting for batch job execution (success/failure)
- Consider a fallback mechanism to compute metrics on-demand if cache is empty (with rate limiting)
- Document the batch job failure recovery procedure
documentation/HU5/backend/HU5-BE-001-Reprogramacion-Cancelacion.md (1)
29-44: Clarify idempotency for cancel/reschedule.Spec doesn’t state expected behavior for repeated cancel/reschedule requests (e.g., client retries). Please document idempotent handling or explicit error responses to avoid duplicate history entries and slot state drift.
documentation/HU5/frontend/HU5-FE-001-Reprogramacion-Cancelacion.md (1)
35-38: Timezone requirement needs a concrete source.“Mostrar timezone CDMX” is ambiguous (system locale vs. fixed tz). Please specify the exact source/format (e.g.,
America/Mexico_Cityviadate-fns-tz) to keep FE/BE consistent.api-specification.yaml (3)
1-60: Add global security defaults to satisfy OpenAPI security scanners.
Defining global security and explicitly clearing it for public endpoints prevents accidental exposure and addresses CKV_OPENAPI_4/5.🔐 Suggested change
openapi: 3.1.0 info: title: CitaYa API @@ servers: - url: https://api.citaya.com/api/v1 description: Producción - url: https://staging-api.citaya.com/api/v1 description: Staging + +security: + - BearerAuth: [] @@ /auth/register: post: + security: [] @@ /auth/login: post: + security: []
584-591: Cap array sizes to matchlimitconstraints.
The appointments list is bounded bylimit(max 50) but the schema allows unbounded arrays. AddmaxItemsto reflect API limits (CKV_OPENAPI_21).📏 Suggested change
properties: appointments: type: array + maxItems: 50 items: $ref: '#/components/schemas/Appointment'
653-688: Make cancel vs reschedule mutually exclusive in PATCH/appointments/{id}.
The current schema allows sending both cancellation and reschedule fields (or neither). AoneOfsplit avoids ambiguous payloads.🧭 Suggested structure
- schema: - type: object - properties: - status: - type: string - enum: [cancelled] - slotId: - type: string - format: uuid - appointmentDate: - type: string - format: date-time - cancellationReason: - type: string - maxLength: 500 + schema: + oneOf: + - type: object + required: [status] + properties: + status: + type: string + enum: [cancelled] + cancellationReason: + type: string + maxLength: 500 + - type: object + required: [slotId, appointmentDate] + properties: + slotId: + type: string + format: uuid + appointmentDate: + type: string + format: date-timedocumentation/HU2/database/HU2-DB-001-Migracion-Tabla-DOCTORS.md (1)
34-37: Clarify “índice espacial” vs DECIMAL columns.
CA3 asks for a spatial index, but the snippet uses DECIMAL + standard index. In MySQL, spatial indexing requires aPOINTcolumn (with SRID) and a SPATIAL INDEX. Either update the schema accordingly or adjust the requirement text to “índice compuesto”.🧭 Suggested text tweak (if keeping DECIMAL)
-### CA3: Índices -- [ ] Índice en `verification_status` para búsquedas -- [ ] Índice espacial en `latitude`, `longitude` para búsquedas geográficas +### CA3: Índices +- [ ] Índice en `verification_status` para búsquedas +- [ ] Índice compuesto en (`latitude`, `longitude`) para búsquedas geográficas + - (Alternativa: usar `POINT` + SPATIAL INDEX si se desea geoespacial real)documentation/HISTORIAS_USUARIO.md (1)
637-644: Use AEAD (AES-256-GCM or ChaCha20-Poly1305) instead of CBC for document encryption.CBC provides confidentiality only and requires separate MAC implementation to prevent tampering; AEAD modes provide native authenticated encryption. Document the nonce/IV generation strategy and key management expectations—ensure nonce uniqueness per encryption operation and plan for key rotation within safe invocation limits.
Consider:
-- Se encripta el contenido antes de almacenar en base de datos (AES-256-CBC): +- Se encripta el contenido antes de almacenar en base de datos usando un esquema AEAD (p. ej., AES-256-GCM o ChaCha20-Poly1305): + - Nonce/IV: Único y aleatorio para cada operación de encriptación + - Validación de autenticidad: Verificar etiqueta de autenticación antes de desencriptar + - Rotación de claves: Definir política de rotación según invocaciones máximas permitidasdocumentation/HU1/database/HU1-DB-001-Migracion-Tabla-USERS.md (2)
64-69: Potential redundancy: unique index on email.The migration creates both a unique constraint on the email column (line 67:
isUnique: true) and a separate unique indexIDX_USERS_EMAIL(lines 126-133). In most databases, settingisUnique: trueon a column already creates a unique index, making the explicit index creation redundant.♻️ Consider removing redundant index creation
If TypeORM automatically creates a unique index from the
isUnique: truecolumn property, you can remove the explicit index creation:}), true, ); - - // Crear índice único en email - await queryRunner.createIndex( - 'USERS', - new TableIndex({ - name: 'IDX_USERS_EMAIL', - columnNames: ['email'], - isUnique: true, - }), - ); // Crear índice en role para búsquedasAnd update the down() method accordingly:
public async down(queryRunner: QueryRunner): Promise<void> { // Eliminar índices primero await queryRunner.dropIndex('USERS', 'IDX_USERS_ROLE'); - await queryRunner.dropIndex('USERS', 'IDX_USERS_EMAIL'); // Eliminar tablaVerify this behavior with your TypeORM version before making changes.
Also applies to: 126-133
206-211: Consider using a TypeScript enum for role values.The entity defines the role field with an inline array of string literals for the enum values. For better type safety and maintainability, consider defining a TypeScript enum that can be reused across the application.
♻️ Proposed refactor using TypeScript enum
Create a shared enum file (e.g.,
backend/src/enums/user-role.enum.ts):export enum UserRole { PATIENT = 'patient', DOCTOR = 'doctor', ADMIN = 'admin', }Then update the entity:
+import { UserRole } from '../enums/user-role.enum'; + `@Entity`('USERS') `@Index`('IDX_USERS_EMAIL', ['email'], { unique: true }) `@Index`('IDX_USERS_ROLE', ['role']) export class User { // ... other fields ... `@Column`({ type: 'enum', - enum: ['patient', 'doctor', 'admin'], + enum: UserRole, - default: 'patient', + default: UserRole.PATIENT, }) - role: string; + role: UserRole;This provides compile-time type checking and prevents typos when working with roles throughout the codebase.
documentation/HU8/testing/HU8-TEST-001-Testing-Horarios.md (2)
10-15: Consider adding concurrency test scenarios.The backend test plan doesn't explicitly mention testing concurrent schedule operations. For a booking system, consider adding tests for:
- Two requests creating overlapping schedules simultaneously
- Race conditions when updating/deleting schedules while slots are being booked
- Atomic operations for slot generation
📋 Suggested additional test scenarios
Add to the Backend section:
### Backend - Crear horario genera slots correctos (duración+pausa) para 4 semanas. - Solapamiento mismo día → 400. - Actualizar horario: elimina slots futuros no reservados y regenera. - Eliminar horario: marca deleted_at y elimina slots futuros no reservados. - Solo doctor dueño puede gestionar (403). + **Concurrencia**: Dos requests simultáneos creando horarios solapados → solo uno tiene éxito. + **Integridad**: Actualizar horario mientras se reserva un slot → manejo correcto de conflictos.
18-18: Frontend validation note: clarify validation timing.Line 18 states "Form valida endTime>startTime, solapamiento manejado por error backend." Consider adding client-side overlap detection to improve UX, showing a warning before the user submits if there might be overlaps with existing schedules (soft validation, with backend as the source of truth).
documentation/HU9/frontend/HU9-FE-001-Crear-Resena.md (1)
13-13: Document the moderation workflow.The success message mentions "pendiente de moderación", but the moderation workflow isn't detailed. Consider adding a reference to admin/moderation documentation or a note about how moderators will process pending reviews.
📝 Add reference to moderation workflow
- Mensajes: éxito "pendiente de moderación"; errores 403/404/400/409. +- **Nota**: Las reseñas permanecen en estado 'pending' hasta aprobación por administrador (ver HU10-Dashboard). - i18n ES/EN.documentation/HU10/database/HU10-DB-001-Soporte-Dashboard.md (1)
12-15: Consider providing more detailed view definitions.The optional SQL views are described at a high level. For the 2-hour estimation to be realistic, consider adding sample CREATE VIEW statements or at minimum specifying the columns and join conditions expected in each view.
For example,
vw_admin_reservationscould include: appointment_id, patient_name, doctor_name, specialty_name, appointment_date, status, created_at.documentation/HU10/frontend/HU10-FE-001-Dashboard-Admin.md (2)
32-33: Clarify security boundary: backend vs. frontend.The security check should primarily rely on backend 403 responses (as mentioned in line 10), with the frontend role check serving only to improve UX by hiding unauthorized UI elements early. Ensure the documentation makes it clear that the client-side role check is not a security boundary.
Based on the overall context, this appears to be the intended design, but the wording "Comprobar role en cliente" could be misinterpreted.
29-31: Consider shorter cache TTL for admin metrics.A 24-hour TTL for admin dashboard metrics may result in stale data for a full day. For an admin dashboard where decisions are made (approving doctors/reviews), consider:
- Shorter TTL (e.g., 5-15 minutes for metrics)
- Automatic cache invalidation after approval/rejection actions
- The manual refetch is a good fallback, but proactive invalidation would improve UX
documentation/HU6/frontend/HU6-FE-001-Gestion-Perfil-Medico.md (1)
24-28: LGTM: Excellent geocoding failure handling.The approach of showing a warning but allowing the save operation to proceed when geocoding fails is excellent for resilience. This prevents third-party API issues from blocking critical user workflows.
One minor clarification: line 28 mentions "opcional, cache de búsqueda" for invalidation. Since doctor location updates affect search results, consider making search cache invalidation mandatory rather than optional to avoid stale search results.
documentation/HU10/testing/HU10-TEST-001-Testing-Dashboard-Admin.md (1)
9-24: Consider adding performance testing scenarios.The testing plan is comprehensive, but it doesn't include performance testing for the <2s SLA mentioned in HU10-DB-001. Consider adding:
- Load testing for aggregated queries under realistic data volumes
- Validation that cached metrics meet SLA targets
- Testing cache behavior during cache misses/cold starts
These tests would help verify that the database optimization strategy (indexes + caching) meets performance requirements.
documentation/HU8/backend/HU8-BE-001-Gestion-Horarios.md (2)
14-18: Clarify slot generation beyond the 4-week window.The automatic generation of slots for the next 4 weeks is clear, but consider documenting:
- How slots beyond 4 weeks are generated (e.g., background job, rolling window, user-triggered)
- What happens when patients try to book appointments >4 weeks out
Line 30 mentions an optional background cleanup job, which could potentially handle rolling slot generation as well. Clarifying this in the CA would help ensure complete implementation.
20-30: LGTM: Solid schedule management design.The overlap validation, soft delete semantics, and slot generation logic are well thought out. The approach of preserving reserved slots during updates is correct and prevents data integrity issues.
Minor clarification: Line 22 mentions dayOfWeek as int 0-6. Consider specifying whether 0=Sunday (ISO standard) or 0=Monday in the documentation to avoid confusion during implementation.
documentation/HU4/database/HU4-DB-001-Migracion-Tablas-Citas.md (1)
219-223: Verify notes length constraint.Line 36 in the CA specifies that notes should have a maximum of 500 characters, but the migration defines
notesas typetextwithout a length constraint. Consider:
- Adding a CHECK constraint in the migration to enforce the 500-character limit at the database level
- Or, document that validation will be enforced at the application layer only
Database-level constraints are generally more robust.
documentation/HU8/frontend/HU8-FE-001-Gestion-Horarios.md (1)
16-16: Clarify timezone with an IANA identifier.“TZ CDMX” is ambiguous in code. Consider specifying
America/Mexico_City(or the exact IANA zone you intend).documentation/HU1/testing/HU1-TEST-001-Testing-Registro-Paciente.md (1)
349-371: Document a deterministic reCAPTCHA strategy for E2E.The E2E flow doesn’t mention test keys or a bypass/mocking approach. Add a clear test-only path (e.g., test keys or a mock endpoint) to keep tests stable.
documentation/HU6/backend/HU6-BE-001-Gestion-Perfil-Medico.md (2)
27-27: Cache invalidation with wildcard pattern may be inefficient.Lines 27 and 32 specify cache invalidation using the pattern
doctors:*, which will match all doctor-related cache keys. If there are many doctors in the system, scanning for keys matching this pattern can be slow in Redis.Consider these alternatives:
- Specific key tracking: Maintain a Set of cache keys to invalidate per doctor
- Scoped patterns: Use more specific patterns like
doctors:search:{specialty}and only invalidate relevant searches- Time-based expiration: Set TTL on cache entries and accept brief inconsistency
- Tag-based invalidation: Use Redis modules or a separate structure to track which cache entries depend on a doctor
♻️ More efficient cache invalidation strategy
+ - Key de perfil: `doctor:{doctorId}` + - Keys de búsqueda: `doctors:search:{specialty}:*` para cada specialty del médico + - Evitar usar wildcard `doctors:*` por razones de rendimientoAnd update line 32:
+ - Perfil: `doctor:{id}` + - Búsqueda: `doctors:search:{specialty}:{sortBy}:{page}` + - Invalidar solo las búsquedas de las especialidades del médico actualizadoAlso applies to: 32-32
20-20: Clarify bio length validation (characters vs bytes).Line 20 specifies "bio <=1000" but doesn't clarify if this is 1000 characters or 1000 bytes. For multilingual text with emojis or special characters, this distinction matters:
- 1000 characters: Simpler for users but variable database storage
- 1000 bytes (UTF-8): Fixed database size but complex validation
Recommend specifying character count for better UX:
📝 Clarification
+ Nota: Para bio, validar longitud en caracteres (no bytes) para consistencia con UI.Additionally, specify in the DTO validation:
`@MaxLength`(1000, { message: 'Bio debe tener máximo 1000 caracteres' }) bio?: string;documentation/HU1/frontend/HU1-FE-001-Registro-Paciente.md (3)
139-139: Consider security implications of storing access tokens in localStorage.Line 139 stores the JWT access token in
localStorage, which is vulnerable to XSS attacks. If an attacker injects malicious JavaScript into the page, they can steal the token from localStorage.Consider these alternatives:
- Memory-only storage: Store in React state/context (lost on refresh but more secure)
- HttpOnly cookie: Store access token in httpOnly cookie (requires backend support)
- SessionStorage: Slightly better than localStorage (cleared when tab closes)
- Short-lived tokens: If using localStorage, ensure short expiration (5-15 minutes) and robust refresh token mechanism
Since the backend already handles
refreshTokenin httpOnly cookies (Line 141), consider moving the access token there as well, or using a hybrid approach where short-lived access tokens are in memory and refreshed automatically:🔒 More secure token storage approach
+// Create a context for auth state +import { createContext, useContext, useState } from 'react'; + +const AuthContext = createContext<{ + accessToken: string | null; + setAccessToken: (token: string | null) => void; +}>({ accessToken: null, setAccessToken: () => {} }); + +export function AuthProvider({ children }: { children: React.ReactNode }) { + const [accessToken, setAccessToken] = useState<string | null>(null); + + return ( + <AuthContext.Provider value={{ accessToken, setAccessToken }}> + {children} + </AuthContext.Provider> + ); +} + export default function RegisterForm() { + const { setAccessToken } = useContext(AuthContext); const { t } = useTranslation(); const router = useRouter(); // ... const onSubmit = async (data: RegisterFormData) => { // ... if (!response.ok) { // error handling... return; } - // Guardar tokens - localStorage.setItem('accessToken', result.accessToken); + // Guardar token en memoria (más seguro contra XSS) + setAccessToken(result.accessToken); // El refreshToken se guarda automáticamente en cookie httpOnly por el backendThen update fetch calls to use the context token:
const { data: slots, isLoading } = useQuery({ queryKey: ['slots', doctorId, selectedDate], queryFn: async () => { + const { accessToken } = useContext(AuthContext); const response = await fetch( `/api/v1/doctors/${doctorId}/slots?date=${format(selectedDate, 'yyyy-MM-dd')}`, { headers: { - Authorization: `Bearer ${localStorage.getItem('accessToken')}`, + Authorization: `Bearer ${accessToken}`, }, }, );Trade-off: Tokens stored in memory are lost on page refresh, requiring the app to use the refreshToken to get a new access token. This adds complexity but significantly improves security.
218-238: useAuth effect lacks cleanup and has performance concerns.Lines 218-238 define a
useEffectthat fetches user data on mount, but:
- No abort controller: If the component unmounts before the fetch completes, it may cause a memory leak or setState on unmounted component warning
- Runs on every mount: This could cause unnecessary API calls if the user is already authenticated
- Dependency array is empty: The effect should depend on whether a token exists
♻️ Add cleanup and optimization
useEffect(() => { const token = localStorage.getItem('accessToken'); - if (token) { + if (!token) { + setLoading(false); + return; + } + + // Avoid fetching if user is already loaded + if (user) { + setLoading(false); + return; + } + + const abortController = new AbortController(); + // Validar token y obtener información del usuario fetch('/api/v1/auth/me', { + signal: abortController.signal, headers: { Authorization: `Bearer ${token}`, }, }) .then((res) => res.json()) .then((data) => { setUser(data.user); }) .catch((err) => { + // Ignore abort errors + if (err.name === 'AbortError') return; + localStorage.removeItem('accessToken'); }) .finally(() => setLoading(false)); - } else { - setLoading(false); - } - }, []); + + return () => { + abortController.abort(); + }; + }, [user]);
72-75: Phone validation regex may be too permissive.The regex
/^\+?[1-9]\d{1,14}$/at Lines 72-73 attempts to validate E.164 phone format but has issues:
- The
+is optional, but E.164 international format should always include it- It allows 2-15 digits total (including country code) but doesn't validate country code structure
- It doesn't prevent invalid country codes like +0 or +00
📞 More robust phone validation
Consider using a library like
libphonenumber-jsfor proper phone validation:+import { isValidPhoneNumber } from 'libphonenumber-js'; + const registerSchema = z.object({ email: z.string().email('Email inválido'), password: z.string().min(8, 'La contraseña debe tener al menos 8 caracteres'), firstName: z.string().min(1, 'El nombre es obligatorio'), lastName: z.string().min(1, 'El apellido es obligatorio'), - phone: z.string().optional().refine( - (val) => !val || /^\+?[1-9]\d{1,14}$/.test(val), - 'Formato de teléfono inválido' - ), + phone: z.string().optional().refine( + (val) => !val || isValidPhoneNumber(val), + 'Formato de teléfono inválido. Usa formato internacional: +52 1 555 1234567' + ), role: z.literal('patient'), });Add to package dependencies:
"dependencies": { "react-hook-form": "^7.48.0", "@hookform/resolvers": "^3.3.0", "zod": "^3.22.0", "next-intl": "^3.0.0", "react-google-recaptcha-v3": "^1.10.0", - "zustand": "^4.5.0" + "zustand": "^4.5.0", + "libphonenumber-js": "^1.10.0" }
| {/* Lista de resultados */} | ||
| <div className="space-y-4"> | ||
| {data?.doctors?.map((doctor) => ( | ||
| <div key={doctor.id} className="border p-4 rounded"> | ||
| <h3>{doctor.firstName} {doctor.lastName}</h3> | ||
| <p>{doctor.address}</p> | ||
| {doctor.distanceKm && <p>{doctor.distanceKm} km</p>} | ||
| <p>⭐ {doctor.ratingAverage} ({doctor.totalReviews} reseñas)</p> | ||
| <button onClick={() => router.push(`/doctors/${doctor.id}`)}> | ||
| {t('search.viewProfile')} | ||
| </button> |
There was a problem hiding this comment.
router is used but never defined.
router.push is called without a router instance in scope. Add const router = useRouter(); and import it from next/navigation.
✅ Suggested fix
+import { useRouter } from 'next/navigation';
...
export default function DoctorSearch() {
const { t } = useTranslation();
+ const router = useRouter();🤖 Prompt for AI Agents
In `@documentation/HU3/frontend/HU3-FE-001-Busqueda-Medicos.md` around lines 231 -
241, The JSX uses router.push inside the component when rendering doctor cards
but never defines router; import and initialize Next's client router by adding
an import for useRouter from "next/navigation" and calling const router =
useRouter() at the top of the component (the same component rendering the doctor
list), then leave the existing router.push(`/doctors/${doctor.id}`) call
unchanged so navigation works; ensure the useRouter import and const router =
useRouter() are placed in the component scope that contains the map over
data?.doctors.
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ queryKey: ['appointments'] }); | ||
| router.push(`/appointments/${data.id}`); | ||
| }, |
There was a problem hiding this comment.
Undefined variables 'router' and 'data'.
Line 113 references router.push() but router is not imported or defined in the component. Additionally, data.id is referenced but data doesn't exist in the onSuccess callback scope. The mutation result should be accessed via the callback parameter.
🐛 Fix undefined variables
+import { useRouter } from 'next/navigation';
+
export default function SlotSelector({ doctorId }: { doctorId: string }) {
+ const router = useRouter();
const [selectedDate, setSelectedDate] = useState(new Date());
const [selectedSlot, setSelectedSlot] = useState<Slot | null>(null);
const [note, setNote] = useState('');And fix the onSuccess callback:
- onSuccess: () => {
+ onSuccess: (data) => {
queryClient.invalidateQueries({ queryKey: ['appointments'] });
router.push(`/appointments/${data.id}`);
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onSuccess: () => { | |
| queryClient.invalidateQueries({ queryKey: ['appointments'] }); | |
| router.push(`/appointments/${data.id}`); | |
| }, | |
| onSuccess: (data) => { | |
| queryClient.invalidateQueries({ queryKey: ['appointments'] }); | |
| router.push(`/appointments/${data.id}`); | |
| }, |
🤖 Prompt for AI Agents
In `@documentation/HU4/frontend/HU4-FE-001-Reserva-Cita.md` around lines 111 -
114, The onSuccess callback uses undefined symbols: `router` and `data`. Import
and initialize the router inside the component (e.g., via useRouter) so
`router.push` is available, and accept the mutation result as the onSuccess
parameter (e.g., onSuccess: (result) => { ... }) then use that parameter to read
the created appointment id (e.g., result.id or result.data.id depending on your
mutation shape) when calling router.push and invalidating queries; update the
`onSuccess` signature and add the router initialization accordingly.
1 participant
docs: documentar MVP CitaYa con prompts, arquitectura y API
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.