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

Update root_ancestor_id of descendants when parent is made a root #5

Merged

Conversation

shirish-pampoorickal
Copy link
Member

Issue: When changing a child node with descendants to a root, the root_ancestor_id for all the descendants would be set to nil.

@naiyt @urnf Appreciate a look

@naiyt
Copy link

naiyt commented Feb 16, 2016

I'm sure this is fine, but at first glance it looks like in that scenario it would set each descendants root_ancestor_id to its own id, not the actual root. Would you be able to explain to me how this works? (Just for my benefit, I'm sure I'm just misunderstanding it.)

@urnf
Copy link

urnf commented Feb 16, 2016

It's implied that if root_ancestor_id does not exist, then the parent is the root, I think?

@shirish-pampoorickal
Copy link
Member Author

Yes, exactly what Kevin said. The method is updating the materialized_path & root_ancestor_id of it's descendants in an after_update. So, if the root_ancestor_id of the story is nil then the it becomes the root_ancestor for its children

@naiyt
Copy link

naiyt commented Feb 16, 2016

Alright, cool, LGTM then!

@shirish-pampoorickal
Copy link
Member Author

Thanks @naiyt & @urnf!

shirish-pampoorickal pushed a commit that referenced this pull request Feb 16, 2016
…el_stories

Update root_ancestor_id of descendants when parent is made a root
@shirish-pampoorickal shirish-pampoorickal merged commit b605aec into master Feb 16, 2016
@shirish-pampoorickal shirish-pampoorickal deleted the fix_ancestry_depth_for_multi_level_stories branch February 16, 2016 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants