refactor: MedicalAppointments CRUD operations

[lethib]

No description provided.

[claude]

Claude finished @lethib's task in 3m 50s —— View job

---

Review of PR #249 — MedicalAppointments CRUD operations

Tasks

• Gather context (diff, changed files)
• Review code quality, security, performance
• Review test coverage
• Post inline comments for actionable findings
• Summarize review

Summary

This is a mechanical refactor migrating MedicalAppointments, CompanyInterventions, and PractitionerCompanies CRUD off model-level create/update_from_params methods onto a new ActiveModel::builder() / ActiveModelEx pattern (from the sea-ORM v2 #[sea_orm::model] codegen), with the params structs and validation logic relocated into new src/services/* modules. Authorization checks, validation rules (VAT values, address validation, email format), and trimming behavior are all preserved — I didn't find behavioral regressions or security issues.

Two inline comments posted with actionable feedback:

1. src/controllers/patient.rs (generate_invoice, ~L141-151) — this call site wasn't migrated to the new builder pattern like its sibling in medical_appointment.rs; it still builds the ActiveModel manually with raw ActiveValue::Set(...). Worth converting for consistency.
2. src/services/practitioner_companies.rs (~L22-53) — the address-validation logic that used to live in a single shared validate_address_params helper is now duplicated verbatim in both create and update. company_interventions.rs kept its equivalent validate_vat_values as a shared function in the same PR, so this is an inconsistency worth fixing.

Note: I was unable to run cargo check/cargo test or git fetch in this sandbox (Bash commands required approval that wasn't available in this automated context), so this review is based on static reading of the full diff and surrounding code only. If you'd like compilation/test verification, please update --allowedTools to permit cargo and git fetch.

[claude]

This call site still constructs the ActiveModel manually with raw ActiveValue::Set(...) fields, while the sibling create/update in src/controllers/medical_appointment.rs were migrated to the new medical_appointments::ActiveModel::builder() pattern in this same PR. For consistency (and to drop the now-unused ActiveModel as MedicalAppointments / ActiveValue imports), consider:

medical_appointments::ActiveModel::builder()
  .set_date(NaiveDate::parse_from_str(&params.invoice_params.date, "%Y-%m-%d")?)
  .set_practitioner_office_id(params.invoice_params.office_id)
  .set_price_in_cents((params.invoice_params.amount * 100.0).round() as i32)
  .set_user_id(ctx.current_user.id)
  .set_patient_id(patient_id)
  .set_payment_method(params.payment_method)
  .insert(&ctx.db)
  .await?;

Fix this →