-
Notifications
You must be signed in to change notification settings - Fork 983
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 #14101 - Foreman exception triggered when editing/adding a role #3342
Conversation
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
[test] |
@@ -15,6 +15,7 @@ def self.authorized_associations(associations, klass_name = nil, should_raise_e | |||
|
|||
def self.permission_name(klass, permission, should_raise_exception) | |||
klass = klass.first if klass.is_a?(Array) | |||
klass = klass.class unless klass.is_a?(Class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the patch belongs in the previous method - authorized_associations
. associations_klass
might end up being just 'associations' ([casbios] in your case). Can you add a test for this behavior?
1.10.4 has now been tagged and is being released. As I believe the bug is fixed in 1.11-stable and above due to the Rails upgrade, I'd suggest we close this as a further 1.10 release is unlikely at this stage. |
I just installed 11.1. Let me check to make sure the bug is fixed. |
I'm still getting an exception in Foreman 1.11, although it is a different message now.
|
This seems to fix it in 1.11.x
def self.authorized_associations(associations, klass_name = nil, should_raise_exception = true, action = 'view')
if associations.included_modules.include?(Authorizable)
associations_klass = associations
if associations.respond_to?(:first) && associations.first
associations_klass = associations.first.class
elsif associations.respond_to?(:klass)
associations_klass = associations.klass
end
klass_name ||= associations_klass
permission = permission_name(klass_name, action, should_raise_exception)
associations.authorized(permission, associations_klass) if permission
else
associations
end
end |
The problem is that when associations is an However consulting the first element of the array gives the correct class.
|
Update:
def self.authorized_associations(associations, klass_name = nil, should_raise_exception = true, action = 'view')
if associations.included_modules.include?(Authorizable)
associations_klass = associations
if associations.respond_to?(:klass)
if associations.klass == Taxonomy
associations_klass = associations.first.class
else
associations_klass = associations.klass
end
end
klass_name ||= associations_klass
permission = permission_name(klass_name, action, should_raise_exception)
associations.authorized(permission, associations_klass) if permission
else
associations
end
end |
@edestecd 3 things:
|
|
@edestecd Awesome. There seem to be a number of tests failing with the current patch - not sure if they're worth looking into given the code is different. http://ci.theforeman.org/job/test_develop_pr_core/9291/#showFailuresLink |
Thanks for the PR, but another has been opened and merged for the same issue (#3779) which claims to have fixed it. The fix will be shipped in a release soon. |
Great! |
No description provided.