diff --git a/app/controllers/ops_controller/ops_rbac.rb b/app/controllers/ops_controller/ops_rbac.rb index 3327b052fb2..ee4e2c62e41 100644 --- a/app/controllers/ops_controller/ops_rbac.rb +++ b/app/controllers/ops_controller/ops_rbac.rb @@ -670,6 +670,7 @@ def rbac_edit_save_or_add(what, rbac_suffix = what) when :group then record = @edit[:group_id] ? MiqGroup.find_by(:id => @edit[:group_id]) : MiqGroup.new validated = rbac_group_validate? + rbac_group_set_record_description_role(record) # Set new Description, Role and Tenant for a new Group rbac_group_set_record_vars(record) if validated when :role then record = @edit[:role_id] ? MiqUserRole.find_by(:id => @edit[:role_id]) : MiqUserRole.new @@ -756,17 +757,7 @@ def rbac_field_changed(rec_type) @edit[:new][:group] = rbac_user_get_group_ids.map(&:to_i) if rec_type == "user" - bad = case rec_type - when 'group' - @edit[:new].values_at(:role, :group_tenant, :description).any?(&:blank?) - when 'role' - @edit[:new][:name].blank? - else - false - end - - # We need to consider also bad variable, for proper response of Add button - session[:changed] = changed = @edit[:new] != @edit[:current] && !bad + session[:changed] = changed = @edit[:new] != @edit[:current] render :update do |page| page << javascript_prologue @@ -775,7 +766,6 @@ def rbac_field_changed(rec_type) page.replace("flash_msg_div", :partial => "layouts/flash_msg") if @refresh_div == "column_lists" page.replace(@refresh_div, :partial => @refresh_partial) end - bad = false else # only do following for user (adding/editing a user) if x_node.split("-").first == "u" || x_node == "xx-u" @@ -1227,10 +1217,6 @@ def rbac_group_filter_expression_vars(field_expression, field_expression_table) # Set group record variables to new values def rbac_group_set_record_vars(group) - role = MiqUserRole.find(@edit[:new][:role]) - group.description = @edit[:new][:description] - group.miq_user_role = role - group.tenant = Tenant.find(@edit[:new][:group_tenant]) if @edit[:new][:group_tenant] if @edit[:new][:use_filter_expression] @edit[:new][:filters].clear else @@ -1241,6 +1227,13 @@ def rbac_group_set_record_vars(group) rbac_group_set_filters(group) # Go set the filters for the group end + # Set group record variables such as Description, Role and Tenant to new values + def rbac_group_set_record_description_role(group) + group.description = @edit[:new][:description] + group.miq_user_role = MiqUserRole.find(@edit[:new][:role]) if @edit[:new][:role] + group.tenant = Tenant.find(@edit[:new][:group_tenant]) if @edit[:new][:group_tenant] + end + # Set filters in the group record from the @edit[:new] hash values def rbac_group_set_filters(group) group.entitlement ||= Entitlement.new @@ -1411,7 +1404,7 @@ def rbac_group_validate? @assigned_filters = [] if @edit[:new][:filters].empty? || @edit[:new][:use_filter_expression] @filter_expression = [] if @edit[:new][:filter_expression].empty? || @edit[:new][:use_filter_expression] == false if @edit[:new][:role].nil? || @edit[:new][:role] == "" - add_flash(_("A User Group must be assigned a Role"), :error) + add_flash(_("A Role must be assigned to this Group"), :error) return false end true diff --git a/spec/controllers/ops_controller/ops_rbac_spec.rb b/spec/controllers/ops_controller/ops_rbac_spec.rb index e3367696dfa..26a3ac9ce56 100644 --- a/spec/controllers/ops_controller/ops_rbac_spec.rb +++ b/spec/controllers/ops_controller/ops_rbac_spec.rb @@ -727,13 +727,13 @@ let(:params) { {:id => "new", :group_tenant => tenant.id.to_s} } let(:edit) { {:new => {:role => 1}} } - it 'sets session[:changed] to false while filling in role and tenant' do + it 'sets session[:changed] to true while filling in role and tenant' do controller.send(:rbac_field_changed, rec_type) - expect(controller.session[:changed]).to be(false) + expect(controller.session[:changed]).to be(true) end - context 'filling in all the required info' do - let(:edit) { {:new => {:role => 1, :description => 'new_group'}} } + context 'filling in description' do + let(:edit) { {:new => {:description => 'new_group'}} } it 'sets session[:changed] to true' do controller.send(:rbac_field_changed, rec_type) @@ -806,4 +806,23 @@ end end end + + describe '#rbac_edit_save_or_add' do + context 'adding new Group' do + before do + allow(controller).to receive(:load_edit).and_return(true) + allow(controller).to receive(:render_flash) + controller.instance_variable_set(:@edit, :new => {:description => 'Description', + :filters => {}, + :filter_expression => {'???' => '???'}, + :use_filter_expression => false}) + controller.params = {:button => 'add'} + end + + it 'sets @flash_array properly if user forgot to choose a Role for a Group' do + controller.send(:rbac_edit_save_or_add, 'group') + expect(controller.instance_variable_get(:@flash_array)).to eq([{:message => 'A Role must be assigned to this Group', :level => :error}]) + end + end + end end