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.

Sign up for GitHub

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

Toolbar refactoring: Ops #417

Merged
merged 13 commits into from
Feb 24, 2017
Merged

Toolbar refactoring: Ops #417

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
2b51370
Create mixin with methods that determine the active tree and tab
Feb 16, 2017
9af4dce
Refactor schedule_run_now button in Ops
Feb 4, 2017
630cc05
Refactor buttons in ops' diagnostics tree
Feb 16, 2017
af8a4e2
Edit GenericFeatureButton's feature retrieval
Feb 17, 2017
c4d4d2b
Refactor buttons in ops' rbac_tree
Feb 20, 2017
b277753
Refactor buttons in ops' vmdb_tree
Feb 20, 2017
34ea18f
Remove #hide_button_ops method
Feb 20, 2017
f38754d
Review response: Simplify rbac_tree button assignements
Feb 23, 2017
5587124
Review response: Rollback changes done to TenantEdit class and its un…
Feb 24, 2017
b318fc7
Review response: Add icon to schedule_run_now button
Feb 24, 2017
e87b26f
Review response: Set ScheduleRunNow parent to Basic
Feb 24, 2017
164ce76
Assign reload_server_tree button to its class
Feb 24, 2017
0cd906f
Review response: Rollback GenericFeatureButton#initialize implementation
Feb 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions app/helpers/application_helper/button/rbac_common_feature_button.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class ApplicationHelper::Button::RbacCommonFeatureButton < ApplicationHelper::Button::GenericFeatureButton
delegate :rbac_common_feature_for_buttons, :to => :@view_context

def role_allows_feature?
role_allows?(:feature => rbac_common_feature_for_buttons(@feature))
Copy link
Member

@romanblanco romanblanco Feb 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@vecerek I was pretty confused at this point, because you check availability of a @feature for a role at a lot of buttons, where you don't even define the feature to check (in toolbar).

After explanation, I've understood, that's why you've changed feature loading in previous commit af8a4e2, but still, it seems i bit wrong.

Isn't it better to use GenericFeatureButton based class only for the two buttons, that the rbac_common_feature_for_buttons method checks?

/cc @PanSpagetka

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that it look confusing when you have button named RbacCommonFeatureButton and you intentionally don't set any feature. Also I think, that it will hide button, which probably isn't right.

end

def visible?
true
end
end
9 changes: 2 additions & 7 deletions app/helpers/application_helper/button/tenant_add.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
class ApplicationHelper::Button::TenantAdd < ApplicationHelper::Button::GenericFeatureButton
class ApplicationHelper::Button::TenantAdd < ApplicationHelper::Button::RbacCommonFeatureButton
needs :@record
delegate :role_allows?, :rbac_common_feature_for_buttons, :to => :@view_context

def role_allows_feature?
role_allows?(:feature => rbac_common_feature_for_buttons(self[:child_id]))
end

def visible?
true
[email protected]?
end
end
6 changes: 2 additions & 4 deletions app/helpers/application_helper/button/tenant_edit.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
class ApplicationHelper::Button::TenantEdit < ApplicationHelper::Button::Basic
needs :@record

class ApplicationHelper::Button::TenantEdit < ApplicationHelper::Button::RbacCommonFeatureButton
def disabled?
@record.source
@record.try!(:source)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to modify this class, because it is not needed to add this to summary toolbar.

end
end
9 changes: 6 additions & 3 deletions app/helpers/application_helper/toolbar/miq_group_center.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ class ApplicationHelper::Toolbar::MiqGroupCenter < ApplicationHelper::Toolbar::B
:rbac_group_edit,
'pficon pficon-edit fa-lg',
t = N_('Edit this Group'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_group_delete,
'pficon pficon-delete fa-lg',
t = N_('Delete this Group'),
t,
:url_parms => "&refresh=y",
:confirm => N_("Are you sure you want to delete this Group?")),
:confirm => N_("Are you sure you want to delete this Group?"),
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand All @@ -34,7 +36,8 @@ class ApplicationHelper::Toolbar::MiqGroupCenter < ApplicationHelper::Toolbar::B
t = proc do
_('Edit \'%{customer_name}\' Tags for this Group') % {:customer_name => @view_context.session[:customer_name]}
end,
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand Down
15 changes: 10 additions & 5 deletions app/helpers/application_helper/toolbar/miq_groups_center.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@ class ApplicationHelper::Toolbar::MiqGroupsCenter < ApplicationHelper::Toolbar::
:rbac_group_add,
'pficon pficon-add-circle-o fa-lg',
t = N_('Add a new Group'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_group_edit,
'pficon pficon-edit fa-lg',
N_('Select a single Group to edit'),
N_('Edit the selected Group'),
:url_parms => "main_div",
:enabled => false,
:onwhen => "1"),
:onwhen => "1",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_group_delete,
'pficon pficon-delete fa-lg',
Expand All @@ -27,13 +29,15 @@ class ApplicationHelper::Toolbar::MiqGroupsCenter < ApplicationHelper::Toolbar::
:url_parms => "main_div",
:confirm => N_("Delete all selected Groups?"),
:enabled => false,
:onwhen => "1+"),
:onwhen => "1+",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
separator,
button(
:rbac_group_seq_edit,
'pficon pficon-edit fa-lg',
t = N_('Edit Sequence of User Groups for LDAP Look Up'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand All @@ -55,7 +59,8 @@ class ApplicationHelper::Toolbar::MiqGroupsCenter < ApplicationHelper::Toolbar::
t,
:url_parms => "main_div",
:enabled => false,
:onwhen => "1+"),
:onwhen => "1+",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand Down
9 changes: 6 additions & 3 deletions app/helpers/application_helper/toolbar/tenant_center.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,16 @@ class ApplicationHelper::Toolbar::TenantCenter < ApplicationHelper::Toolbar::Bas
:rbac_tenant_manage_quotas,
'pficon pficon-edit fa-lg',
t = N_('Manage Quotas'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_tenant_delete,
'pficon pficon-delete fa-lg',
t = N_('Delete this item'),
t,
:url_parms => "&refresh=y",
:confirm => N_("Are you sure you want to delete this item and all of it's children?")),
:confirm => N_("Are you sure you want to delete this item and all of it's children?"),
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand All @@ -56,7 +58,8 @@ class ApplicationHelper::Toolbar::TenantCenter < ApplicationHelper::Toolbar::Bas
t = proc do
_('Edit \'%{customer_name}\' Tags for this Tenant') % {:customer_name => @view_context.session[:customer_name]}
end,
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand Down
12 changes: 8 additions & 4 deletions app/helpers/application_helper/toolbar/tenants_center.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ class ApplicationHelper::Toolbar::TenantsCenter < ApplicationHelper::Toolbar::Ba
N_('Edit the selected item'),
:url_parms => "main_div",
:enabled => false,
:onwhen => "1"),
:onwhen => "1",
:klass => ApplicationHelper::Button::TenantEdit),
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this class because you can use Basic button.

button(
:rbac_tenant_delete,
'pficon pficon-delete fa-lg',
Expand All @@ -22,15 +23,17 @@ class ApplicationHelper::Toolbar::TenantsCenter < ApplicationHelper::Toolbar::Ba
:url_parms => "main_div",
:confirm => N_("Delete all selected items and all of their children?"),
:enabled => false,
:onwhen => "1+"),
:onwhen => "1+",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_tenant_manage_quotas,
'pficon pficon-edit fa-lg',
N_('Select a single item to manage quotas'),
N_('Manage Quotas for the Selected Item'),
:url_parms => "main_div",
:enabled => false,
:onwhen => "1"),
:onwhen => "1",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand All @@ -52,7 +55,8 @@ class ApplicationHelper::Toolbar::TenantsCenter < ApplicationHelper::Toolbar::Ba
t,
:url_parms => "main_div",
:enabled => false,
:onwhen => "1+"),
:onwhen => "1+",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand Down
12 changes: 8 additions & 4 deletions app/helpers/application_helper/toolbar/user_center.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,22 @@ class ApplicationHelper::Toolbar::UserCenter < ApplicationHelper::Toolbar::Basic
:rbac_user_edit,
'pficon pficon-edit fa-lg',
t = N_('Edit this User'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_user_copy,
'fa fa-files-o fa-lg',
t = N_('Copy this User to a new User'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_user_delete,
'pficon pficon-delete fa-lg',
t = N_('Delete this User'),
t,
:url_parms => "&refresh=y",
:confirm => N_("Are you sure you want to delete this User?")),
:confirm => N_("Are you sure you want to delete this User?"),
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand All @@ -39,7 +42,8 @@ class ApplicationHelper::Toolbar::UserCenter < ApplicationHelper::Toolbar::Basic
t = proc do
_('Edit \'%{customer_name}\' Tags for this User') % {:customer_name => @view_context.session[:customer_name]}
end,
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand Down
9 changes: 6 additions & 3 deletions app/helpers/application_helper/toolbar/user_role_center.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,22 @@ class ApplicationHelper::Toolbar::UserRoleCenter < ApplicationHelper::Toolbar::B
:rbac_role_edit,
'pficon pficon-edit fa-lg',
t = N_('Edit this Role'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_role_copy,
'fa fa-files-o fa-lg',
t = N_('Copy this Role to a new Role'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_role_delete,
'pficon pficon-delete fa-lg',
t = N_('Delete this Role'),
t,
:url_parms => "&refresh=y",
:confirm => N_("Are you sure you want to delete this Role?")),
:confirm => N_("Are you sure you want to delete this Role?"),
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand Down
12 changes: 8 additions & 4 deletions app/helpers/application_helper/toolbar/user_roles_center.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,26 @@ class ApplicationHelper::Toolbar::UserRolesCenter < ApplicationHelper::Toolbar::
:rbac_role_add,
'pficon pficon-add-circle-o fa-lg',
t = N_('Add a new Role'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_role_edit,
'pficon pficon-edit fa-lg',
N_('Select a single Role to edit'),
N_('Edit the selected Role'),
:url_parms => "main_div",
:enabled => false,
:onwhen => "1"),
:onwhen => "1",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_role_copy,
'fa fa-files-o fa-lg',
N_('Select a single Role to copy'),
N_('Copy the selected Role to a new Role'),
:url_parms => "main_div",
:enabled => false,
:onwhen => "1"),
:onwhen => "1",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_role_delete,
'pficon pficon-delete fa-lg',
Expand All @@ -35,7 +38,8 @@ class ApplicationHelper::Toolbar::UserRolesCenter < ApplicationHelper::Toolbar::
:url_parms => "main_div",
:confirm => N_("Delete all selected Roles?"),
:enabled => false,
:onwhen => "1+"),
:onwhen => "1+",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand Down
15 changes: 10 additions & 5 deletions app/helpers/application_helper/toolbar/users_center.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,26 @@ class ApplicationHelper::Toolbar::UsersCenter < ApplicationHelper::Toolbar::Basi
:rbac_user_add,
'pficon pficon-add-circle-o fa-lg',
t = N_('Add a new User'),
t),
t,
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_user_edit,
'pficon pficon-edit fa-lg',
N_('Select a single User to edit'),
N_('Edit the selected User'),
:url_parms => "main_div",
:enabled => false,
:onwhen => "1"),
:onwhen => "1",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_user_copy,
'fa fa-files-o fa-lg',
N_('Select a single User to copy'),
N_('Copy the selected User to a new User'),
:url_parms => "main_div",
:enabled => false,
:onwhen => "1"),
:onwhen => "1",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
button(
:rbac_user_delete,
'pficon pficon-delete fa-lg',
Expand All @@ -35,7 +38,8 @@ class ApplicationHelper::Toolbar::UsersCenter < ApplicationHelper::Toolbar::Basi
:url_parms => "main_div",
:confirm => N_("Delete all selected Users?"),
:enabled => false,
:onwhen => "1+"),
:onwhen => "1+",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand All @@ -57,7 +61,8 @@ class ApplicationHelper::Toolbar::UsersCenter < ApplicationHelper::Toolbar::Basi
t,
:url_parms => "main_div",
:enabled => false,
:onwhen => "1+"),
:onwhen => "1+",
:klass => ApplicationHelper::Button::RbacCommonFeatureButton),
]
),
])
Expand Down
2 changes: 0 additions & 2 deletions app/helpers/application_helper/toolbar_builder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,6 @@ def hide_button_ops(id)
when :diagnostics_tree
return false
when :rbac_tree
return true unless role_allows?(:feature => rbac_common_feature_for_buttons(id))
return true if %w(rbac_project_add rbac_tenant_add).include?(id) && @record.project?
return false
when :vmdb_tree
return !["db_connections", "db_details", "db_indexes", "db_settings"].include?(@sb[:active_tab])
Expand Down
21 changes: 21 additions & 0 deletions spec/helpers/application_helper/buttons/rbac_common_feature_button_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
describe ApplicationHelper::Button::RbacCommonFeatureButton do
let(:session) { Hash.new }
let(:view_context) { setup_view_context_with_sandbox({}) }
let(:button) { described_class.new(view_context, {}, {}, {:options => {:feature => feature}}) }

before { login_as FactoryGirl.create(:user, :features => 'rbac_tenant_add') }

describe '#role_allows_feature?' do
subject { button.role_allows_feature? }
%w(rbac_tenant_add rbac_project_add).each do |feature|
context 'when user role allows feature' do
let(:feature) { feature }
it { expect(subject).to be_truthy }
end
end
context 'when user role does not allow feature' do
let(:feature) { 'not_allowed_feature' }
it { expect(subject).to be_falsey }
end
end
end
Loading