Skip to content

Conversation

vinnyrose
Copy link
Contributor

...re.

Since the model is reloaded from database it is impossible to compare
between original parent (i.e. cached) and new parent in children objects
without explicitly passing in the previous parent id to the save method.

…mpare.

Since the model is reloaded from database it is impossible to compare
between original parent (i.e. cached) and new parent in children objects
without explicitly passing in the previous parent id to the save method.
@vinnyrose
Copy link
Contributor Author

For example:

Original parent cached here: https://github.com/edoburu/django-fluent-pages/blob/master/fluent_pages/models/db.py#L130

And used here: https://github.com/edoburu/django-fluent-pages/blob/master/fluent_pages/models/db.py#L295

But since the moved object is reloaded from the database after the move the cached parent is set equal to the now current parent. Passing in the previous parent allows this information to be preserved.

@vinnyrose
Copy link
Contributor Author

I'm testing this now. I am getting a 500 error but it is hidden behind an ajax call.

vinnyrose added a commit to vinnyrose/django-fluent-pages that referenced this pull request Jun 4, 2014
This is in case _original_parent becomes unreliable.
This depends on a pull request in django-polymorphic-tree:
django-polymorphic/django-polymorphic-tree#17
@vinnyrose
Copy link
Contributor Author

Okay that fixed the 500 I was getting.

@vdboor
Copy link
Member

vdboor commented Jun 4, 2014

Very cool to have this sorted out!
I just looked at the sources, and wonder, can't we just skip the re-fetching of the moved node? mptt 0.5 seems to be behaving nicely, and updates the moved node as well. Calling moved.save() would just work I think.

@vinnyrose
Copy link
Contributor Author

I was wondering the same thing. I don't know mptt well enough to have known whether or not that extra load was required. But I would prefer if the approach in this pull request was not needed and that we could remove that re-fetching, much cleaner, and doesn't add extra boilerplate like the save method.

@vinnyrose
Copy link
Contributor Author

I just tried it with that re-fetch removed and it works fine. I can send another pull request with the change unless you want to do it?

@vinnyrose
Copy link
Contributor Author

#18 with the re-fetch removed.

@vdboor
Copy link
Member

vdboor commented Jun 5, 2014

Really nice! I've merged #18 now, and I guess this one can be closed :)

@vdboor vdboor closed this Jun 5, 2014
@vdboor
Copy link
Member

vdboor commented Jun 5, 2014

I've released 0.9 now, with this fix included!

@vinnyrose vinnyrose deleted the pass_previous_parent_fix branch June 5, 2014 17:18
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.

2 participants