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

Fix SystemStackError when paranoid models have a circular dependent: :destroy #446

Open
wants to merge 1 commit into
base: core
Choose a base branch
from

Conversation

andyklimczak
Copy link

@andyklimczak andyklimczak commented Jun 10, 2018

@andyklimczak
Copy link
Author

andyklimczak commented Jun 10, 2018

Based on the travis results, a different approach will be needed I think. But the failing tests show that the test case does reproduce the error

@mcmegavolt
Copy link

Are there plans to fix it soon?

@andyklimczak
Copy link
Author

andyklimczak commented Jan 8, 2019

Yeah, I'll take another look. Shoutouts to everyone who 👍 and subscribed to this PR. If anyone has an insight, tips or ideas, please feel free to post here. Any help is appreciated

belongs_to_dependent_destroy.destroy

assert_equal false, has_one_dependent_destroy.reload.deleted_at.nil?
assert_equal false, belongs_to_dependent_destroy.reload.deleted_at.nil?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should just be refute?

@andyklimczak andyklimczak force-pushed the circular-dependent-destroy branch 5 times, most recently from 2e3b3ad to ffcac89 Compare January 9, 2019 03:30
@andyklimczak
Copy link
Author

@andyklimczak andyklimczak force-pushed the circular-dependent-destroy branch 2 times, most recently from 3a4b295 to 4704336 Compare January 9, 2019 03:56
@andyklimczak
Copy link
Author

Ran into some build errors for this project with the bundler 2.0 release, that's my blocker atm. Working through it

@andyklimczak andyklimczak force-pushed the circular-dependent-destroy branch 4 times, most recently from 07ffb12 to 760f3d9 Compare August 23, 2019 17:35
@andyklimczak andyklimczak force-pushed the circular-dependent-destroy branch 3 times, most recently from a46430e to 9cd1593 Compare September 13, 2019 18:44
before_install: gem update --system
before_install:
- gem uninstall -v '>= 2' -i $(rvm gemdir)@global -ax bundler || true
- gem install bundler -v '< 2'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

taken from here

@andyklimczak
Copy link
Author

andyklimczak commented Sep 13, 2019

Caught up with travisci issues. Ready for review! 🎉

…t: :destroy`

- Previously, paranoid models with a circular `dependent: :destroy` would recursively try to delete eachother
- Add a variable to check if the destroy callback has already been
called for that model
- Similar fix to rails/rails#18548
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants