Skip to content

fix how relationships are saved #214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 3, 2017
Merged

fix how relationships are saved #214

merged 3 commits into from
Aug 3, 2017

Conversation

erizocosmico
Copy link
Contributor

Fixes #211

There has been a major change here:

  • Recursive saves (even with circular dependencies) are now supported!
  • Model interface has changed, but it's not considered public API, so will not trigger a major version bump.

And also a bug has been fixed as a result:

  • If the virtual column corresponding to a relationship foreign key is empty (because, for example, the relationship has not been retrieved in the query) but exists in the database, now it is ignored. Before it was deleted without checking if it existed in the database. This does not break any expected behavior because relationships should not be removed by just removing them from the object but using the Remove* methods from the store. All this behavior is now heavily tested.

Thanks to @roobre for reporting this and adding the reproduction case.

nadiamoe and others added 3 commits July 28, 2017 16:38
There has been a major change here:
- Recursive saves (even with circular dependencies) are now supported!

And also a bug has been fixed as a result:
- If the virtual column corresponding to a relationship foreign key is empty (because, for example, the relationship has not been retrieved in the query) but exists in the database, now it is ignored. Before it was deleted without checking if it existed in the database. This does not break any expected behavior because relationships should not be removed by just removing them from the object but using the Remove* methods from the store.

Signed-off-by: Miguel Molina <[email protected]>
@codecov
Copy link

codecov bot commented Aug 2, 2017

Codecov Report

Merging #214 into master will decrease coverage by 0.12%.
The diff coverage is 68.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #214      +/-   ##
==========================================
- Coverage   79.27%   79.14%   -0.13%     
==========================================
  Files          19       19              
  Lines        3628     3635       +7     
==========================================
+ Hits         2876     2877       +1     
- Misses        543      548       +5     
- Partials      209      210       +1
Impacted Files Coverage Δ
store.go 87.26% <100%> (ø) ⬆️
generator/template.go 92.78% <100%> (-0.04%) ⬇️
model.go 57.05% <60%> (-1.46%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 398b52f...0ad2e1a. Read the comment docs.

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.

Update() destroys existing second-level relationships
4 participants