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 conflict with Devise generator #23

Closed
wants to merge 2 commits into from

Conversation

walterdavis
Copy link

(This is a revised version of the original pull request, cleaning up the commit history to only the needed change.)

This patch allows Prototype-Rails to work with the latest Devise generators. As requested, this no longer uses Rails.env to check whether TestCase is loaded.

ActionView::TestCase.class_eval do
include ActionView::Helpers::PrototypeHelper
include ActionView::Helpers::ScriptaculousHelper
if ActionView::TestCase.present?
Copy link
Member

Choose a reason for hiding this comment

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

if defined? ActionView::TestCase

Copy link
Author

Choose a reason for hiding this comment

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

I missed this post, sorry for the delay.

@walterdavis walterdavis reopened this Feb 22, 2013
@walterdavis
Copy link
Author

Jeremy, any chance of motion forward on this? Anything further I need to do to make it more merge-able?

Thanks,

Walter

rafaelfranca pushed a commit that referenced this pull request Feb 6, 2014
This reverts commit 3f512ec, reversing
changes made to 2841450.

Reason: I'll use the solution in
#23 that doesn't check the
Rails environment
@rafaelfranca
Copy link
Member

@grosser could you check if this solution works for you? If so I'll release with it.

@grosser
Copy link
Contributor

grosser commented Feb 6, 2014

Afaik there is no way of doing this test, for example the following setup:

#action_view/test_case.rb: 
raise

rails runner 'puts ActionView.constants; puts defined?(ActionView::TestCase); puts ActionView::TestCase.name'

result:
LookupContext
TestCase
...

constant

bang RuntimeError

so defined? and constants are pretending it is there :)

not sure how exactly devise generator is failing, can it require test_case ?
or maybe Rails.env is the problem and the test can be
!defined?(Rails) || Rails.env.test?

@rafaelfranca
Copy link
Member

I didn't get. Seems something is requiring test_case in development already. So why do you need #28?

@rafaelfranca
Copy link
Member

Ok. Now I got, the constant is defined but the file was not required. So, seems you fix is the only one that will work. I'll put it back and release with it.

Thanks for looking it ❤️

@grosser
Copy link
Contributor

grosser commented Feb 6, 2014

maybe there is some magic ruby foo that would also work, I just ran out of ideas :)

@rafaelfranca
Copy link
Member

Yes. It was because of the autoload. It defines the constant but doesn't require the file until you use the constant the first time

@grosser
Copy link
Contributor

grosser commented Dec 11, 2014

@walterdavis do you remember what the problem with testing Rails.env.test was ?

@walterdavis
Copy link
Author

It was the first thing I wrote, and it worked, but Jeremy suggested that I use TestCase.present?, maybe because it was actually the error that was being thrown. This was my first pull request to a big project, so maybe I caved too soon.

@grosser
Copy link
Contributor

grosser commented Dec 12, 2014

Hmmm do you know how to reproduce the error you got ?

@walterdavis
Copy link
Author

The issue was that when I used the official prototype_rails gem, the Devise generators would die horribly whenever I tried to run them. I have been using this fork: https://github.com/anamba/prototype-rails which cooperates reliably. My patch also worked, but this was back in Rails 3, and I haven't kept up with it since I found the anamba version and standardized on it.

@grosser
Copy link
Contributor

grosser commented Dec 12, 2014

ok, so if defined?(Rails) && Rails.env.test? could work ?
or what was the reason it blew up ?

On Thu, Dec 11, 2014 at 7:03 PM, Walter Lee Davis notifications@github.com
wrote:

The issue was that when I used the official prototype_rails gem, the
Devise generators would die horribly whenever I tried to run them. I have
been using this fork: https://github.com/anamba/prototype-rails which
cooperates reliably. My patch also worked, but this was back in Rails 3,
and I haven't kept up with it since I found the anamba version and
standardized on it.


Reply to this email directly or view it on GitHub
#23 (comment).

@walterdavis
Copy link
Author

I would try it, my recollection is that it did.

Walter

On Dec 11, 2014, at 10:39 PM, Michael Grosser notifications@github.com wrote:

ok, so if defined?(Rails) && Rails.env.test? could work ?
or what was the reason it blew up ?

On Thu, Dec 11, 2014 at 7:03 PM, Walter Lee Davis notifications@github.com
wrote:

The issue was that when I used the official prototype_rails gem, the
Devise generators would die horribly whenever I tried to run them. I have
been using this fork: https://github.com/anamba/prototype-rails which
cooperates reliably. My patch also worked, but this was back in Rails 3,
and I haven't kept up with it since I found the anamba version and
standardized on it.


Reply to this email directly or view it on GitHub
#23 (comment).


Reply to this email directly or view it on GitHub.

saturnflyer pushed a commit to zendesk/prototype-rails that referenced this pull request Sep 27, 2019
This reverts commit 3f512ec, reversing
changes made to 2841450.

Reason: I'll use the solution in
rails#23 that doesn't check the
Rails environment
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.

4 participants