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 issues with subclassing, such as when using ActiveRecord STI. #568

Merged
merged 2 commits into from
Jun 18, 2022

Conversation

mrbrdo
Copy link
Contributor

@mrbrdo mrbrdo commented Apr 14, 2022

Fixes mobility to work if subclassing a subclass of a mobility-enabled class, and looks up mobility_backend_classes later in execution (upon being called) to be more flexible.

fixes #566

Fixes mobility to work if subclassing a subclass of a mobility-enabled class, and looks up mobility_backend_classes later in execution (upon being called) to be more flexible.
@shioyama
Copy link
Owner

shioyama commented May 2, 2022

Thanks for the PR! I've been very busy recently and will be busy for the next few weeks but I'll have a look asap.

@shioyama
Copy link
Owner

shioyama commented May 3, 2022

Can you write a failing test as well to demonstrate the issue?

@mrbrdo
Copy link
Contributor Author

mrbrdo commented May 3, 2022

@shioyama I added a test for this, which fails on master but passes with this fix.
Writing a test for the other issue I described in #566 is really tricky, but it did fix that issue for me as well.

@mrbrdo
Copy link
Contributor Author

mrbrdo commented May 26, 2022

@shioyama so since I added the failing spec as requested, and it is passing with this fix, can you merge this?

@shioyama
Copy link
Owner

@mrbrdo Thanks very much for this, I see the issue now and this is a good fix. Sorry for the long delay in responding! I'll rebase on master and 1-2 and ship this.

@shioyama shioyama merged commit ca9f048 into shioyama:master Jun 18, 2022
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.

Does not work with subclassing in production only - dangerous bug
2 participants