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

Alter how ControllerAdditions is included in ActionController #93

Merged
merged 1 commit into from
Jun 6, 2020

Conversation

JamesCarscadden
Copy link
Contributor

Alters how VersionCake::ControllerAdditions is included into ActionController. The original
ActionController::Base.send(:include, VersionCake::ControllerAdditions) appears to cause some sort of problem that is causing #76 (why exactly that is I have no idea).

If we include ControllerAdditions using similar methods to other gems I have seen (notably cribbed from Jbuilder), then this appears to fix the problem, while still maintaining compatibility with the older versions of rails.

(First ever pull request - go easy on me ;-P)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f82d103 on JamesCarscadden:fix-blank-views-76 into 2b05903 on bwillis:master.

@bwillis
Copy link
Owner

bwillis commented Jun 5, 2020

Thanks for digging into this @JamesCarscadden! I'll take a look and do some testing since this isn't easy to verify with automated testing.

@JamesCarscadden
Copy link
Contributor Author

no problem. In terms of debugging this, I eventually got down to just commenting out parts of Versioncake until the views started rendering again. Ultimately commenting the require/view_additions didn't do the trick, though that was my first instinct given my previous comments on the bug. Commenting require/controller_additions on the other hand, did fix the problem. So then it was a matter of removing parts of controller_additions until I could figure out what was causing the problem. However, commenting the various methods or the include did nothing to fix. The only line that made any difference was the one I cited above. As I said though why exactly that is causing a problem is beyond my understanding. The first fix I did was simply to remove the Base.send line and to include VersionCake::ControllerAdditions in my ApplicationController -- that fixed the problem with the views while still having Versioncake work properly. At that point it was a matter of figuring out how other gems tend to include themselves in controllers.

@bwillis
Copy link
Owner

bwillis commented Jun 6, 2020

Yeah your methodology makes a lot of sense when not understanding the complexities of the framework. I tested locally and verified the bug and that this fixes it. I think that leveraging the lazy load hooks makes a lot more sense, it wasn't an available API when I originally created this.

Thanks a lot again for digging into this! 🌈 I'll release a new version shortly.

@bwillis bwillis merged commit 058b215 into bwillis:master Jun 6, 2020
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.

All my response body are blank string
3 participants