Support non-null HasOne#3118
Conversation
Huliiiiii
force-pushed
the
active-model-ex-refactor
branch
from
53a3207 to
b640eed
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b640eed202
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let clear_optional = if self.cardinality.is_optional() { | ||
| quote! { | ||
| ActiveHasOne::Set(None) => { | ||
| if !self.clear_parent_key::<#related_entity>()? { |
There was a problem hiding this comment.
When a generated optional belongs_to relation is cleared from a partial ActiveModelEx (for example, only the primary key plus clear_cake() is set), the FK column itself is still NotSet. This branch calls clear_parent_key, which delegates to clear_key_on_active_model and returns AttrNotSet for a NotSet FK instead of emitting SET fk = NULL, so nullable parent links cannot be cleared unless the caller first loads or manually sets the FK column.
Useful? React with 👍 / 👎.
|
sadly, the API doesn't feel right. this is too much to write. has_one is always optional, and having we tried to solve an edge case for belongs_to but it cascaded outwards |
…ctors The per-cardinality inherent impls each defined a same-named associated constructor (loaded / set), so the unqualified paths HasOne::loaded and ActiveHasOne::set were ambiguous (E0034) at every call site — the E: EntityTrait where-bound does not disambiguate associated-function path resolution. The crate's own tests and doctests failed to compile as a result. Define loaded / set once on the cardinality-generic impl, dispatching arg-wrapping through IntoHasOneLoaded / IntoActiveHasOneSet helper traits (one impl for the required E case, one for the optional Option<E> case). Bare model -> required, Option<model> -> optional, resolved unambiguously. Also fixes an entity_loader test that passed a bare model to a nullable relation (cake.bakery is HasOne<Option<..>>), which the ambiguity had masked.
…on unset FK Two fixes to nested-ActiveModel save behavior: 1. belongs_to relations that share a target entity with another relation used to silently generate no save/detach code (the action codegen was gated on the target entity being unique), so their nested writes were no-ops. They now always generate: a belongs_to with an explicit relation_enum, or a non-unique target, is keyed by its relation variant via `set_parent_key_for` / `clear_parent_key_for` (the default variant is inferred the same way the loader does); a unique target keeps the entity-keyed path. Fixes e.g. `user_follower` (two belongs_to to `user`). 2. `clear_<field>()` on a nullable belongs_to whose FK column was never set (a freshly-built ActiveModel) returned `DbErr::AttrNotSet`; it is now a no-op, since an unset nullable FK is already effectively clear. Adds `test_belongs_to_duplicate_target` and `test_clear_unset_belongs_to_is_noop`.
…:set With the E0034 root-cause fix in place, the unqualified constructors resolve unambiguously, so the per-site `::<Option<E>>::` turbofish is no longer needed. Revert it in the doc example, lib test modules, into_active_model bodies, and the integration tests so users see the clean bare-call form. The macro-generated code keeps its explicit turbofish (internal, not user-facing).
Huliiiiii
force-pushed
the
active-model-ex-refactor
branch
2 times, most recently
from
07f4d31 to
9bbd47a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bbd47a83c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Huliiiiii
force-pushed
the
active-model-ex-refactor
branch
from
9bbd47a to
8c1858c
Compare
Huliiiiii
force-pushed
the
active-model-ex-refactor
branch
from
8c1858c to
915dac4
Compare
2 participants
Adds support for non-nullable HasOne relations.
Refactor a significant amount of the related code, as the current implementation was difficult to read and modify.
Remove the
NotFoundrepresentation for empty HasOne relations, as this state should never be possible for non-nullable relations.Remove the
Deletevariant added in the previous PRs, since the related behavior has now been fully replaced bySet(Option::None).Remove the compatibility methods from the old implementation that automatically converted
Option/VecintoHasOne/HasMany.Remove internal macro helpers