Editar agendamento #504#607
Conversation
- Replace parameter-based update method with a DTO-based approach. - Add support for updating multiple fields such as frequency, date and time. - Create a new appointment version by deactivating the current rule. - Preserve appointment history during updates. - Regenerate future appointments based on updated data. - Simplify logic by centralizing the update flow into a single method. - Update controller to use a single PATCH endpoint.
- Add PATCH handler to call the appointment update endpoint
- Remove rule-based appointment update endpoint
- Replace updateAppointmentRule with a updateAppointment function - Send a flexible DTO instead of rule-specific fields
- Remove legacy patterns and inconsistencies. - Finalize migration to the unified appointment update flow.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
apps/api/src/main/java/br/org/apae/api/appointment/application/internal/AppointmentApplicationServiceImpl.java (2)
448-451: Add meaningful exception messages toorElseThrow()calls.Using
orElseThrow()without an exception supplier will throw a genericNoSuchElementExceptionwithout context, making debugging difficult.♻️ Proposed fix
- Patient patient = patientRepo.findById(newRule.getAnnualRegistration().getPatientId()) - .orElseThrow(); - Guardian guardian = guardianRepo.findByPatientId(patient.getId()) - .orElseThrow(); + Patient patient = patientRepo.findById(newRule.getAnnualRegistration().getPatientId()) + .orElseThrow(() -> new EntityNotFoundException( + "Paciente não encontrado para o agendamento " + newRule.getId())); + Guardian guardian = guardianRepo.findByPatientId(patient.getId()) + .orElseThrow(() -> new EntityNotFoundException( + "Responsável não encontrado para o paciente " + patient.getId()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/main/java/br/org/apae/api/appointment/application/internal/AppointmentApplicationServiceImpl.java` around lines 448 - 451, Replace the parameterless orElseThrow() calls in AppointmentApplicationServiceImpl so they throw informative exceptions: when resolving patient via patientRepo.findById(newRule.getAnnualRegistration().getPatientId()) wrap the missing-case with an exception supplier that includes the patient id (e.g., throw new EntityNotFoundException or a domain-specific PatientNotFoundException with message "Patient not found with id: <id>"); do the same for guardianRepo.findByPatientId(patient.getId()) (e.g., "Guardian not found for patient id: <id>"). Ensure you reference patientRepo.findById, guardianRepo.findByPatientId, Patient and Guardian and propagate or translate the exceptions consistently with the service's error handling.
416-421: Consider throwing domain-specific exceptions for consistency.The method throws
IllegalArgumentExceptionandIllegalStateExceptionwhile other methods in this service use domain exceptions likeAppointmentNotFoundException. Consider usingAppointmentNotFoundExceptionfor consistency with the rest of the codebase.♻️ Proposed fix
Appointment current = appointmentRepo.findById(appointmentId) - .orElseThrow(() -> new IllegalArgumentException("Rule not found")); + .orElseThrow(AppointmentNotFoundException::new); if (!current.isActive()) { - throw new IllegalStateException("Only active rules can be edited"); + throw new ResponseStatusException(HttpStatus.CONFLICT, + "Apenas regras ativas podem ser editadas"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/main/java/br/org/apae/api/appointment/application/internal/AppointmentApplicationServiceImpl.java` around lines 416 - 421, Replace the generic exceptions in AppointmentApplicationServiceImpl around the appointment lookup and active-check: when retrieving via appointmentRepo.findById(appointmentId) throw AppointmentNotFoundException instead of IllegalArgumentException, and replace the IllegalStateException thrown when !current.isActive() with a domain-specific exception (e.g., AppointmentNotActiveException or AppointmentNotEditableException) to match the service’s existing error types; update the exception constructors/usages accordingly so callers receive the domain exceptions.apps/apae/src/app/services/appointmentService.ts (1)
639-666: Inconsistent mock data handling pattern.Other functions in this file (e.g.,
saveAppointment,getAppointments) checkUSE_MOCK_DATAat the start and return mock data immediately. This function skips that check and only falls back to mock on error. Consider aligning with the established pattern for consistency.♻️ Proposed improvement
export async function updateAppointment( id: UUID, dto: UpdateAppointmentDTO ): Promise<AppointmentResponseDTO> { + if (USE_MOCK_DATA) { + console.log("📦 [MOCK] Atualizando agendamento:", id, dto); + const appointment = + mockAppointments.find((a) => a.id === id) || mockAppointments[0]; + return mockFetch(appointment); + } + try {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/apae/src/app/services/appointmentService.ts` around lines 639 - 666, The updateAppointment function is inconsistent with other methods (saveAppointment, getAppointments): it only falls back to mock data on error instead of checking USE_MOCK_DATA up front. Modify updateAppointment to check the USE_MOCK_DATA flag at the top and return mockFetch(...) immediately when true (using the same mockAppointments selection logic as the catch block), then keep the existing try/catch for real network behavior; ensure you reference updateAppointment, USE_MOCK_DATA, mockAppointments, and mockFetch so the change matches the established pattern.apps/apae/src/components/forms/AppointmentForm.tsx (1)
243-244: Consider using Next.js router for navigation instead of full page reload.
window.location.reload()causes a full page refresh, losing any in-memory state. Consider usingrouter.refresh()fromnext/navigationfor a smoother user experience that preserves React state where appropriate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/apae/src/components/forms/AppointmentForm.tsx` around lines 243 - 244, Replace the full page reload call window.location.reload() in AppointmentForm.tsx with Next.js navigation refresh: import { useRouter } from 'next/navigation', get the router via const router = useRouter() (in the component) and call router.refresh() where window.location.reload() is currently invoked (e.g., in the submit/handleSubmit flow) so the page data is re-fetched without losing in-memory React state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/apae/src/app/api/appointments/`[id]/route.ts:
- Around line 100-105: The DELETE handler currently destructures params from the
second argument signature ({ params }: { params: { id: string } }) which is
inconsistent with Next.js 15+ route handler signatures; update the DELETE
function to accept a single Request-like argument and retrieve params via the
correct parameter object (match the fix applied elsewhere) so that the id is
obtained consistently (refer to the DELETE function and its use of params and
id) — adjust the function signature and extraction of id to mirror the corrected
GET/PUT handlers so routing params are read the same way across handlers.
- Around line 36-41: The PATCH handler is incorrectly typed to accept params as
a plain object whereas Next.js route params are Promises like the GET handler;
update the PATCH function signature to accept { params }: { params: Promise<{
id: string }> } and await the params inside the handler (e.g., const { id } =
await params) so handling of dynamic route params matches the GET handler and
avoids runtime errors.
- Around line 71-76: The PUT handler must use the Next.js 15+ params convention
consistently: ensure the function signature receives the params object (export
async function PUT(req: Request, { params }: { params: { id: string } })) and
then use params.id (or destructure const { id } = params immediately after
signature) rather than relying on any other scope for params; update the PUT
handler (function name PUT and its use of params/id) to match the other route
handlers' params handling so id is always sourced from the provided params
argument.
In `@apps/apae/src/app/services/appointmentService.ts`:
- Around line 643-647: The PATCH fetch to `/api/appointments/${id}` is missing
the Content-Type header so the backend may not parse the JSON body; update the
fetch call (the PATCH in appointmentService.ts that sends JSON dto) to include
headers: { "Content-Type": "application/json" } — mirror the header usage from
saveAppointment to ensure the request body is correctly treated as JSON (keep
JSON.stringify(dto) as-is).
In
`@apps/api/src/main/java/br/org/apae/api/appointment/application/internal/AppointmentApplicationServiceImpl.java`:
- Line 433: The cutoff used when cleaning up generated appointments keeps
appointments 0–59 days after rule deactivation; update the deletion to remove
all future generated appointments from the rule's start date instead of from 60
days out by changing the call in AppointmentApplicationServiceImpl that invokes
generatedRepo.deleteFutureByAppointmentId(current.getId(),
startDate.atStartOfDay().plusDays(60)) to use startDate.atStartOfDay() (i.e.,
delete from startDate onward) so no orphaned generated appointments remain for
the deactivated rule.
- Line 427: In AppointmentApplicationServiceImpl.update(), after calling
validateProfessionalAvailability(current.getProfessional(), startDate,
appointmentHour) and before creating the new appointment rule, add the same
conflict check used in create(): call
appointmentRepository.existsByProfessionalIdAndInitialDateAndHourAndIsActiveTrue(professionalId,
startDate, appointmentHour) and if true throw AppointmentConflictException; this
prevents creating a duplicate active appointment for the same
professional/date/hour. Ensure you reference the same repository method and
exception (existsByProfessionalIdAndInitialDateAndHourAndIsActiveTrue and
AppointmentConflictException) and pass the correct professional id and date/hour
values used elsewhere in update().
---
Nitpick comments:
In `@apps/apae/src/app/services/appointmentService.ts`:
- Around line 639-666: The updateAppointment function is inconsistent with other
methods (saveAppointment, getAppointments): it only falls back to mock data on
error instead of checking USE_MOCK_DATA up front. Modify updateAppointment to
check the USE_MOCK_DATA flag at the top and return mockFetch(...) immediately
when true (using the same mockAppointments selection logic as the catch block),
then keep the existing try/catch for real network behavior; ensure you reference
updateAppointment, USE_MOCK_DATA, mockAppointments, and mockFetch so the change
matches the established pattern.
In `@apps/apae/src/components/forms/AppointmentForm.tsx`:
- Around line 243-244: Replace the full page reload call
window.location.reload() in AppointmentForm.tsx with Next.js navigation refresh:
import { useRouter } from 'next/navigation', get the router via const router =
useRouter() (in the component) and call router.refresh() where
window.location.reload() is currently invoked (e.g., in the submit/handleSubmit
flow) so the page data is re-fetched without losing in-memory React state.
In
`@apps/api/src/main/java/br/org/apae/api/appointment/application/internal/AppointmentApplicationServiceImpl.java`:
- Around line 448-451: Replace the parameterless orElseThrow() calls in
AppointmentApplicationServiceImpl so they throw informative exceptions: when
resolving patient via
patientRepo.findById(newRule.getAnnualRegistration().getPatientId()) wrap the
missing-case with an exception supplier that includes the patient id (e.g.,
throw new EntityNotFoundException or a domain-specific PatientNotFoundException
with message "Patient not found with id: <id>"); do the same for
guardianRepo.findByPatientId(patient.getId()) (e.g., "Guardian not found for
patient id: <id>"). Ensure you reference patientRepo.findById,
guardianRepo.findByPatientId, Patient and Guardian and propagate or translate
the exceptions consistently with the service's error handling.
- Around line 416-421: Replace the generic exceptions in
AppointmentApplicationServiceImpl around the appointment lookup and
active-check: when retrieving via appointmentRepo.findById(appointmentId) throw
AppointmentNotFoundException instead of IllegalArgumentException, and replace
the IllegalStateException thrown when !current.isActive() with a domain-specific
exception (e.g., AppointmentNotActiveException or
AppointmentNotEditableException) to match the service’s existing error types;
update the exception constructors/usages accordingly so callers receive the
domain exceptions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7ed3820-e8ec-4f2e-b534-d4dc3835af81
📒 Files selected for processing (8)
apps/apae/src/app/api/appointments/[id]/route.tsapps/apae/src/app/api/appointments/[id]/rule/route.tsapps/apae/src/app/services/appointmentService.tsapps/apae/src/components/forms/AppointmentForm.tsxapps/api/src/main/java/br/org/apae/api/appointment/application/interfaces/AppointmentApplicationService.javaapps/api/src/main/java/br/org/apae/api/appointment/application/internal/AppointmentApplicationServiceImpl.javaapps/api/src/main/java/br/org/apae/api/appointment/interfaces/controllers/AppointmentController.javaapps/api/src/main/java/br/org/apae/api/controllers/appointment/AppointmentControllerImpl.java
💤 Files with no reviewable changes (1)
- apps/apae/src/app/api/appointments/[id]/rule/route.ts
JoaoGabrielCosta2004
left a comment
There was a problem hiding this comment.
dário, tá tudo certo ao meu ver, só precisa resolver o conflito. Tenta manter o máximo que puder do que está na dev, já que esse conflito é resultado da issue que manu fez de remover os mocks.
O que esse PR forneceu?
Unificação da atualização de agendamentos: o fluxo de atualização foi centralizado em um único endpoint (
PATCH /appointments/{id}), substituindo a abordagem anterior baseada em "rule".Suporte a atualização parcial: agora é possível atualizar apenas campos específicos do agendamento (como frequência, data e horário) utilizando um DTO flexível.
Versionamento de agendamentos: ao atualizar um agendamento, a regra atual é desativada e uma nova versão é criada, garantindo histórico de alterações.
Regeneração de agendamentos futuros: os agendamentos futuros são recalculados automaticamente com base nos novos dados informados.
Simplificação da API: remoção do endpoint
/appointments/{id}/rule, eliminando duplicação de lógica e consolidando tudo em um único ponto de atualização.Integração frontend-backend: o frontend foi atualizado para consumir o novo endpoint unificado, substituindo
updateAppointmentRuleporupdateAppointment.Padronização com PATCH: o fluxo de atualização foi alinhado ao uso correto do método PATCH para updates parciais.
Melhoria na manutenção do código: redução de complexidade ao remover estruturas antigas e centralizar regras de negócio no backend.
Coleta de evidências
2026-03-28.19-02-01.mp4
Simulando registros que não foram removidos por serem de datas que passaram:
2026-03-28.18-59-50.mp4
Summary by CodeRabbit
New Features
Improvements
Refactor