Skip to content
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

No related save for non inline #594

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

philipn
Copy link
Contributor

@philipn philipn commented Aug 2, 2012

This fixes #397. Based on tests and work by @joshbohde.

Beyond simply improving performance, saving related fields when they don't need to be saved can cause undesirable things to happen (signal handlers, etc).

@buzzeante
Copy link

@philipn Love the patch. This same issue was bugging me. I've changed slightly using also obj._state_adding in save_related.

You can check it here https://github.com/TwoApart/django-tastypie/tree/no_related_save_for_non_inline

Cheers,
Javi

georgedorn added a commit that referenced this pull request Mar 18, 2013
@georgedorn
Copy link
Contributor

I've merged this manually (the code's drifted too much in the last 6 months to do it automatically) but I'm pushing to a new branch pending a test for a specific regression I'm worried about:

If you are posting a change to an explicit through model, does that change get saved? The relationship model already exists, so we're not adding it, but we are changing the extra data stored within that model. I tried to find a test that exercised this, but we don't appear to currently have such a test, thus my trepidation at pushing this to master.

The manual merge changeset is here: toastdriven@e4a43ed

@numan
Copy link
Contributor

numan commented Apr 15, 2013

@georgedorn I added a test for the senario you described in d11d224.

The test is failing. I'm not sure about the best way to fix it.

@georgedorn
Copy link
Contributor

One approach I can think of is to check to see whether the relationship model has additional data (probably by interrogating the relationship manager) and if so, save it. Otherwise, it's safe to skip it.

I'm not sure if it's the best approach, but if it makes the test pass and it works for the 80% of cases where it's just a normal ManyToManyField, I'm fine with that.

georgedorn added a commit that referenced this pull request Apr 22, 2013
…hers.

Extra tests assuring through model metadata is saved; fixing backwards/forwards incompatability with MockRequest
numan pushed a commit to numan/django-tastypie that referenced this pull request Jul 21, 2013
…, django-tastypie#594 and others.

Extra tests assuring through model metadata is saved; fixing backwards/forwards incompatability with MockRequest
@SeanHayes SeanHayes force-pushed the master branch 2 times, most recently from cec6b98 to 77cad1b Compare April 8, 2016 07:28
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.

Related fields (ForeignKey) models get saved for no reason. [Performance Issue]
5 participants