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 issue #402 - Destroying a record with habtm association using foreign keys on association table raises a exception #1428

Closed
wants to merge 1 commit into from

Conversation

tomas-stefano
Copy link

No description provided.

@ghost ghost assigned jonleighton May 31, 2011
@sikachu
Copy link
Member

sikachu commented Jun 1, 2011

Do you mind rebase your commits and squash them together, then force push to your branch so this pull request will be updated? I think it would be more useful to have just a single commit.

Reverting but left the test there

Fix issue #402
@tomas-stefano
Copy link
Author

Ok. Here is the join commit tomas-stefano@7b27d85
=D

@tomas-stefano
Copy link
Author

Excuse my curiosity, but I wonder if my patch will be accepted ... or not :)

@dmathieu
Copy link
Contributor

dmathieu commented Jul 6, 2011

This also fails on 3-1-stable. So it will have to be cherry-picked on 3-1-stable and master.

@josevalim
Copy link
Contributor

This looks good enough to be applied if @tenderlove / @jonleighton agrees. I would later refactor it to use only destroy_associations callbacks, without the need to mix it with after_destroy callbacks.

@tenderlove
Copy link
Member

Does the SQL work cross database?

@jonleighton
Copy link
Member

I made some fixups to this patch, ported it to master and 3-1-stable, and then backported to 3-0-stable. Thanks for contributing.

@jonleighton jonleighton closed this Jul 8, 2011
@jonleighton
Copy link
Member

FYI, the commits are:

@josevalim
Copy link
Contributor

❤️ @jonleighton

@tomas-stefano
Copy link
Author

Thanks for the code refactoring. ❤️ @jonleighton

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants