Skip to content

fix: Sea ORM V2 bump#247

Merged
lethib merged 1 commit into
mainfrom
fix/sea-orm-v2-bump
Jul 1, 2026
Merged

fix: Sea ORM V2 bump#247
lethib merged 1 commit into
mainfrom
fix/sea-orm-v2-bump

Conversation

@lethib

@lethib lethib commented Jul 1, 2026

Copy link
Copy Markdown
Owner

No description provided.

@lethib lethib self-assigned this Jul 1, 2026
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Claude finished @lethib's task in 2m 23s —— View job


Review of PR #247 — fix: Sea ORM V2 bump

  • Gather context (PR diff, changed files)
  • Review migration file
  • Review entity changes
  • Review practitioner_office.rs service changes
  • Post findings

Summary: This PR follows up the SeaORM v2 bump (#245) by reordering entity fields, converting the userspractitioner_offices relation to a via many-to-many, giving user_practitioner_offices a composite key in the entity, adding a physical unique index backing the existing #[sea_orm(unique)] on users.pid, and refactoring practitioner_office::create/service code to use the new ActiveModel::create helpers inside a DB transaction. CI build passed.

One issue found (posted as an inline comment):

  • src/models/_entities/user_practitioner_offices.rs — the entity now declares a composite primary key on (user_id, practitioner_office_id) and drops the id column, but no migration actually changes the physical table: it still has a surrogate id SERIAL PRIMARY KEY with no unique constraint on (user_id, practitioner_office_id). Since src/services/practitioner_office.rs:41 calls .update() on this entity, SeaORM will build the UPDATE using the (unenforced) composite key — if duplicate rows for the same pair ever exist, the update would silently touch all of them instead of one row. This should either be backed by a real DB constraint (migration dropping id / adding a unique index) or the entity should keep id as the primary key.

The idx_users_pid_unique migration itself looks correct and consistent with existing migration conventions in this repo (index-drop-by-name-only pattern matches other migrations, e.g. m20260508_155536 and m20250912_195210). The is_address_valid validation removed from services/practitioner_office.rs is preserved — it now lives inside practitioner_offices::ActiveModel::create (src/models/practitioner_offices.rs:43), so no regression there.

Comment thread src/models/_entities/user_practitioner_offices.rs
@lethib
lethib merged commit e958893 into main Jul 1, 2026
5 checks passed
@lethib
lethib deleted the fix/sea-orm-v2-bump branch July 1, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant