test: verify relationship fill does not persist - #325
Open
elpete wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #120
Review
The requested workflow is a good fit for Quick: applications should be able to construct a related entity, populate it, and defer persistence. Quick already exposes this as
parent.relationship().fill( attributes ), which creates a new entity and pre-populates the relationship foreign keys without saving it.Recommendation: 9/10.
Reasons for
create().fill()vocabulary.Reasons against
newPost.author().associate( user )pattern remains blocked for unloaded entities; callers should use the relationship'sfill()method or the generated relationship setter.Attempted implementation/reproduction
The requested capability is already implemented on current
next:BaseRelationship#fillcallsnewEntity(), pre-associates the new model to its parent, fills the supplied attributes, and does not save.I also tested the issue's old direct
author().associate()shape. It still raisesQuickEntityNotLoaded, intentionally preserving the guard against querying relationships from unhydrated models. Weakening that global guard would be a larger and riskier semantic change than the requested unsaved-related-entity helper.This PR strengthens the public relationship test by asserting that only the parent lookup query runs. Creating and filling the related post performs no INSERT or UPDATE.
Validation
git diff --checkpassed