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

Fixes #7615 - filter ignore current context #2291

Closed
wants to merge 1 commit into from

Conversation

ares
Copy link
Member

@ares ares commented Apr 8, 2015

No description provided.

@@ -90,6 +90,14 @@ class FilterTest < ActiveSupport::TestCase
end
end

test 'filter is not automatically scoped to to taxonomies' do
Copy link
Member

Choose a reason for hiding this comment

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

Typo in the test description, scoped to any taxonomies.

In case you want to save the variable assignment (original_org) in the test:

@organization, Organization.current = Organization.current, @organization
assert_empty Filter.new.organizations
Organization.current, @organization = @organization, Organization.current

@dLobatog
Copy link
Member

dLobatog commented Apr 8, 2015

@ares 👍 by the way, thanks. I'll merge after fixing the typo at least.

@ares
Copy link
Member Author

ares commented Apr 9, 2015

good catch, fixed the typo and changed the assignment a bit, I don't want to touch the instance variable that is shared for other tests but the multiple assignment on first line is good trick to suggest that we're saving the original value because of Organization.current reassignment

@dLobatog
Copy link
Member

dLobatog commented Apr 9, 2015

Jenkins said: "Could not find gem 'rails (= 3.2.21) ruby' in the gems available on this", weird. [test]

@domcleal
Copy link
Contributor

domcleal commented Apr 9, 2015

Jenkins said: "Could not find gem 'rails (= 3.2.21) ruby' in the gems available on this", weird. [test]

The actual error is earlier in the log, that is just from the cleanup routine which fails as bundle install had failed (due to execjs, now fixed).

@dLobatog
Copy link
Member

dLobatog commented Apr 9, 2015

Merged as 76fca93, thanks @ares

@dLobatog dLobatog closed this Apr 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants