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 #16622 - Disambiguate resource_name column name in #hosts_count #3863

Closed
wants to merge 1 commit into from
Closed

Conversation

brandonweeks
Copy link
Member

No description provided.

@theforeman-bot
Copy link
Member

There were the following issues with the commit message:

  • length of the first commit message line for f52df73 exceeds 65 characters

If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@domcleal
Copy link
Contributor

Could you add a test to test/unit/helpers/application_helper_test.rb please? You can create a user with a limited filter joining another table (like hostgroups), then with as_user check that the function succeeds.

Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

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

LGTM

@lzap
Copy link
Member

lzap commented Sep 21, 2016

Whoops, I tested tho, works fine.

@domcleal
Copy link
Contributor

Here's a suggestion for a unit test:

test '#hosts_count works for non-admin user with joining filter' do
  host = FactoryGirl.create(:host, :with_hostgroup)
  user_role = FactoryGirl.create(:user_user_role)
  FactoryGirl.create(:filter, :role => user_role.role, :permissions => Permission.where(:name => 'view_hosts'), :search => "hostgroup = #{host.hostgroup.name}")
  as_user user_role.owner do
    assert_equal({host.operatingsystem_id => 1}, hosts_count('operatingsystem'))
  end
end

The issue was fixed in Rails 4.2.7 via rails/rails@2c894cb, so this only affects RPM installations which use an older version (I'll simultaneously patch them). You can reproduce it with the test above by changing the Rails version to 4.2.6 in the Gemfile.

@domcleal
Copy link
Contributor

domcleal commented Nov 3, 2016

Thanks for the PR, I'm going to close it now as I think the issue's been fixed sufficiently in Rails (gems and packages) without needing this patch.

@domcleal domcleal closed this Nov 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants