From 4934a7eead683e53b8ca61a983524d954203bb03 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Wed, 1 Apr 2026 13:57:34 -0400 Subject: [PATCH 1/7] Store role IDs instead of names in visibility column Related to manageiq-schema [1] which migrated existing role names to IDs in the visibility column. [1] https://github.com/ManageIQ/manageiq-schema/pull/852 Before: visibility[:roles] stored role names (strings) After: visibility[:roles] stores role IDs (integers) Changes: - Saving: Store IDs directly from form - Loading: Read IDs directly from database - Display: Query role names from IDs for UI --- .../application_controller/buttons.rb | 23 ++----- .../custom_buttons.rb | 5 +- app/controllers/report_controller/widgets.rb | 19 ++---- .../customization_buttons.cy.js | 68 +++++++++++++++++++ .../e2e/ui/Cloud-Intel/Reports/widgets.cy.js | 58 ++++++++++++++++ .../application_controller/buttons_spec.rb | 6 +- .../custom_buttons_spec.rb | 28 ++++++++ .../report_controller/widget_spec.rb | 39 ++++++++++- 8 files changed, 205 insertions(+), 41 deletions(-) create mode 100644 cypress/e2e/ui/Automation/Embedded-Automate/customization_buttons.cy.js create mode 100644 cypress/e2e/ui/Cloud-Intel/Reports/widgets.cy.js diff --git a/app/controllers/application_controller/buttons.rb b/app/controllers/application_controller/buttons.rb index 5c52d427473..c10dc3de8d9 100644 --- a/app/controllers/application_controller/buttons.rb +++ b/app/controllers/application_controller/buttons.rb @@ -852,12 +852,7 @@ def button_set_record_vars(button) end button.visibility ||= {} if @edit[:new][:visibility_typ] == "role" - roles = [] - @edit[:new][:roles].each do |r| - role = MiqUserRole.find_by(:id => r) - roles.push(role.name) if role - end - button.visibility[:roles] = roles + button.visibility[:roles] = @edit[:new][:roles] else button.visibility[:roles] = ["_ALL_"] end @@ -1023,16 +1018,7 @@ def button_set_form_vars # Visibility Box if @custom_button.visibility && @custom_button.visibility[:roles] @edit[:new][:visibility_typ] = @custom_button.visibility[:roles][0] == "_ALL_" ? "all" : "role" - if @custom_button.visibility[:roles][0] == "_ALL_" - @edit[:new][:roles] = ["_ALL_"] - else - @edit[:new][:roles] ||= [] - @custom_button.visibility[:roles].each do |r| - role = MiqUserRole.find_by(:name => r) - @edit[:new][:roles].push(role.id) if role - end - end - @edit[:new][:roles].sort! if @edit[:new][:roles].present? + @edit[:new][:roles] = @custom_button.visibility[:roles][0] == "_ALL_" ? ["_ALL_"] : @custom_button.visibility[:roles].sort end @edit[:sorted_user_roles] = [] @@ -1096,9 +1082,8 @@ def buttons_get_node_info(node) end @sb[:user_roles] = [] if @custom_button.visibility && @custom_button.visibility[:roles] && @custom_button.visibility[:roles][0] != "_ALL_" - MiqUserRole.all.sort_by(&:name).each do |r| - @sb[:user_roles].push(r.name) if @custom_button.visibility[:roles].include?(r.name) - end + role_ids = @custom_button.visibility[:roles] + @sb[:user_roles] = MiqUserRole.where(:id => role_ids).order(:name).pluck(:name) end @resolve[:new][:target_class] = "ServiceTemplate" dialog_id = @custom_button.resource_action.dialog_id diff --git a/app/controllers/miq_ae_customization_controller/custom_buttons.rb b/app/controllers/miq_ae_customization_controller/custom_buttons.rb index f6959c3b719..9ed2e6e60b2 100644 --- a/app/controllers/miq_ae_customization_controller/custom_buttons.rb +++ b/app/controllers/miq_ae_customization_controller/custom_buttons.rb @@ -76,9 +76,8 @@ def ab_get_node_info(node) end @sb[:user_roles] = [] if @custom_button.visibility && @custom_button.visibility[:roles] && @custom_button.visibility[:roles][0] != "_ALL_" - MiqUserRole.all.sort_by(&:name).each do |r| - @sb[:user_roles].push(r.name) if @custom_button.visibility[:roles].include?(r.name) - end + role_ids = @custom_button.visibility[:roles] + @sb[:user_roles] = MiqUserRole.where(:id => role_ids).order(:name).pluck(:name) end dialog_id = @custom_button.resource_action.dialog_id @sb[:dialog_label] = dialog_id ? Dialog.find(dialog_id).label : "" diff --git a/app/controllers/report_controller/widgets.rb b/app/controllers/report_controller/widgets.rb index 91515da840b..33999bd9dd4 100644 --- a/app/controllers/report_controller/widgets.rb +++ b/app/controllers/report_controller/widgets.rb @@ -292,9 +292,8 @@ def widget_get_node_info if @widget.visibility && @widget.visibility[:roles] @sb[:user_roles] = [] if @widget.visibility[:roles][0] != "_ALL_" - Rbac.filtered(MiqUserRole).sort_by(&:name).each do |r| - @sb[:user_roles].push(r.name) if @widget.visibility[:roles].include?(r.name) - end + role_ids = @widget.visibility[:roles] + @sb[:user_roles] = Rbac.filtered(MiqUserRole.where(:id => role_ids)).order(:name).pluck(:name) end elsif @widget.visibility && @widget.visibility[:groups] @sb[:groups] = [] @@ -332,12 +331,7 @@ def widget_set_form_vars if @widget.visibility if @widget.visibility[:roles] @edit[:new][:visibility_typ] = @widget.visibility[:roles][0] == "_ALL_" ? "all" : "role" - if @widget.visibility[:roles][0] == "_ALL_" - @edit[:new][:roles] = ["_ALL_"] - else - roles = Rbac.filtered(MiqUserRole.where(:name => @widget.visibility[:roles])) - @edit[:new][:roles] = roles.collect(&:id).sort - end + @edit[:new][:roles] = @widget.visibility[:roles][0] == "_ALL_" ? ["_ALL_"] : @widget.visibility[:roles].sort elsif @widget.visibility[:groups] @edit[:new][:visibility_typ] = "group" groups = Rbac.filtered(MiqGroup.in_my_region.where(:description => @widget.visibility[:groups])) @@ -573,12 +567,7 @@ def widget_set_record_vars(widget) widget.visibility.delete(:roles) if widget.visibility[:roles] else if @edit[:new][:visibility_typ] == "role" - roles = [] - @edit[:new][:roles].each do |r| - role = MiqUserRole.find(r) - roles.push(role.name) if r == role.id.to_s - end - widget.visibility[:roles] = roles + widget.visibility[:roles] = @edit[:new][:roles] else widget.visibility[:roles] = ["_ALL_"] end diff --git a/cypress/e2e/ui/Automation/Embedded-Automate/customization_buttons.cy.js b/cypress/e2e/ui/Automation/Embedded-Automate/customization_buttons.cy.js new file mode 100644 index 00000000000..25b83c8fae8 --- /dev/null +++ b/cypress/e2e/ui/Automation/Embedded-Automate/customization_buttons.cy.js @@ -0,0 +1,68 @@ +/* eslint-disable no-undef */ +import { flashClassMap } from '../../../../support/assertions/assertion_constants'; + +describe('Automation > Embedded Automate > Customization > Buttons', () => { + beforeEach(() => { + cy.login(); + cy.menu('Automation', 'Embedded Automate', 'Customization'); + cy.get('#explorer_title_text'); + cy.accordion('Buttons'); + }); + + afterEach(() => { + cy.appDbState('restore'); + }); + + describe('Button Creation', () => { + it('Creates a new button with role visibility', () => { + cy.selectAccordionItem(['Object Types', 'Availability Zone', '[Unassigned Buttons]']); + cy.toolbar('Configuration', 'Add a new Button'); + cy.get('#explorer_title_text').contains('Adding a new Button'); + + cy.get('#name').type('Test Button'); + cy.get('#description').type('Test button description'); + + cy.get('.icon-button').click(); + cy.get(':nth-child(1) > span > .ff').click(); + cy.get('.bx--modal-footer > .bx--btn--primary').click(); + + cy.tabs({ tabLabel: 'Advanced' }); + + cy.get('#object_request').scrollIntoView(); + cy.get('#object_request').type('test_request'); + + cy.get('#form_role_visibility button[data-id="visibility_typ"]').scrollIntoView(); + cy.get('#form_role_visibility button[data-id="visibility_typ"]').click(); + cy.get('#form_role_visibility .dropdown-menu [data-original-index="1"] > a').click(); + + cy.contains('User Roles').scrollIntoView(); + + cy.interceptApi({ + alias: 'checkAuditorRole', + urlPattern: '/miq_ae_customization/automate_button_field_changed', + triggerFn: () => cy.get('#form_role_visibility').contains('EvmRole-auditor').closest('td').find('input[type="checkbox"]').check(), + waitOnlyIfRequestIntercepted: true, + }); + + cy.interceptApi({ + alias: 'checkDesktopRole', + urlPattern: '/miq_ae_customization/automate_button_field_changed', + triggerFn: () => cy.get('#form_role_visibility').contains('EvmRole-desktop').closest('td').find('input[type="checkbox"]').check(), + waitOnlyIfRequestIntercepted: true, + }); + + // Click Add button + // TODO: cy.getFormButtonByTypeWithText expects this in the main_content div, but it's in the form_buttons_div + // within the full_content div - enhance the cy.getFormButtonByTypeWithText to accept a div id or normalize the location of the button + cy.get('#buttons_on > .btn-primary').click(); + cy.expect_flash(flashClassMap.success, 'was added'); + + cy.get('.clickable-row').contains('Test Button').click(); + + cy.get('#main_div').contains('Test Button'); + cy.get('#main_div').contains('test_request'); + cy.get('.visibility').contains('EvmRole-auditor'); + cy.get('.visibility').contains('EvmRole-desktop'); + }); + }); +}); diff --git a/cypress/e2e/ui/Cloud-Intel/Reports/widgets.cy.js b/cypress/e2e/ui/Cloud-Intel/Reports/widgets.cy.js new file mode 100644 index 00000000000..9612cbd8ada --- /dev/null +++ b/cypress/e2e/ui/Cloud-Intel/Reports/widgets.cy.js @@ -0,0 +1,58 @@ +/* eslint-disable no-undef */ +import { flashClassMap } from '../../../../support/assertions/assertion_constants'; + +describe('Cloud Intel > Reports > Widgets', () => { + beforeEach(() => { + cy.login(); + cy.menu('Cloud Intel', 'Reports'); + cy.get('#explorer_title_text'); + cy.accordion('Widgets'); + }); + + afterEach(() => { + cy.appDbState('restore'); + }); + + describe('Widget Creation', () => { + it('Creates a new widget with role visibility', () => { + cy.selectAccordionItem(['Report Widgets']); + cy.toolbar('Configuration', 'Add a new Widget'); + cy.get('#explorer_title_text').contains('Adding a new Widget'); + + cy.get('#title').type('Test Widget'); + cy.get('#description').type('Test widget description'); + + cy.get('button[data-id="repfilter_typ"]').click(); + cy.get('.dropdown-menu [data-original-index="1"] > a').click(); + + cy.get('button[data-id="visibility_typ"]').scrollIntoView(); + cy.get('button[data-id="visibility_typ"]').click(); + cy.get('.dropdown-menu [data-original-index="1"] > a').click(); + + cy.contains('User Roles').scrollIntoView(); + + cy.interceptApi({ + alias: 'checkAuditorRole', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => cy.get('#form_role_visibility').contains('EvmRole-auditor').closest('td').find('input[type="checkbox"]').check(), + waitOnlyIfRequestIntercepted: true, + }); + + cy.interceptApi({ + alias: 'checkDesktopRole', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => cy.get('#form_role_visibility').contains('EvmRole-desktop').closest('td').find('input[type="checkbox"]').check(), + waitOnlyIfRequestIntercepted: true, + }); + + cy.get('#buttons_on > .btn-primary').click(); + cy.expect_flash(flashClassMap.success, 'was saved'); + + cy.get('.clickable-row').contains('Test Widget').click(); + + cy.get('#main_div').contains('Test Widget'); + cy.get('.visibility').contains('EvmRole-auditor'); + cy.get('.visibility').contains('EvmRole-desktop'); + }); + }); +}); diff --git a/spec/controllers/application_controller/buttons_spec.rb b/spec/controllers/application_controller/buttons_spec.rb index 2d64b350a3f..6b4c7b028d0 100644 --- a/spec/controllers/application_controller/buttons_spec.rb +++ b/spec/controllers/application_controller/buttons_spec.rb @@ -325,7 +325,7 @@ :object_request => 'request', :open_url => true, :visibility_typ => 'role', - :roles => [role.id.to_s], + :roles => [role.id], :display_for => :list} } } @@ -340,13 +340,13 @@ it "sets new role visibility for custom button" do controller.send(:button_set_record_vars, custom_button) - expect(custom_button.visibility[:roles]).to eq([role.name]) + expect(custom_button.visibility[:roles]).to eq([role.id]) end it "sets new role and preserves old role for custom button" do edit[:new][:roles] = [old_role.id, role.id.to_s] # old roles are represented by int and new ones by string controller.send(:button_set_record_vars, custom_button) - expect(custom_button.visibility[:roles]).to eq([old_role.name, role.name]) + expect(custom_button.visibility[:roles]).to eq([old_role.id, role.id.to_s]) end end diff --git a/spec/controllers/miq_ae_customization_controller/custom_buttons_spec.rb b/spec/controllers/miq_ae_customization_controller/custom_buttons_spec.rb index 3e7d1f8b333..676328263f5 100644 --- a/spec/controllers/miq_ae_customization_controller/custom_buttons_spec.rb +++ b/spec/controllers/miq_ae_customization_controller/custom_buttons_spec.rb @@ -13,6 +13,34 @@ controller.send(:ab_get_node_info, "xx-ab_Host_cbg-10r95_cb-#{custom_button.id}") expect(assigns(:resolve)[:new][:target_class]).to eq("Host") end + + it "loads role names from role IDs for display" do + role1 = FactoryBot.create(:miq_user_role, :name => "TestRole1") + role2 = FactoryBot.create(:miq_user_role, :name => "TestRole2") + custom_button = FactoryBot.create(:custom_button, + :applies_to_class => "Host", + :name => "Button With Roles", + :visibility => {:roles => [role1.id, role2.id]}) + target_classes = {} + CustomButton.button_classes.each { |db| target_classes[ui_lookup(:model => db)] = db } + controller.instance_variable_set(:@sb, :target_classes => target_classes) + controller.send(:ab_get_node_info, "xx-ab_Host_cbg-10r95_cb-#{custom_button.id}") + + expect(assigns(:sb)[:user_roles]).to match_array(["TestRole1", "TestRole2"]) + end + + it "handles buttons with _ALL_ visibility" do + custom_button = FactoryBot.create(:custom_button, + :applies_to_class => "Host", + :name => "Button For All", + :visibility => {:roles => ["_ALL_"]}) + target_classes = {} + CustomButton.button_classes.each { |db| target_classes[ui_lookup(:model => db)] = db } + controller.instance_variable_set(:@sb, :target_classes => target_classes) + controller.send(:ab_get_node_info, "xx-ab_Host_cbg-10r95_cb-#{custom_button.id}") + + expect(assigns(:sb)[:user_roles]).to eq([]) + end end end render_views diff --git a/spec/controllers/report_controller/widget_spec.rb b/spec/controllers/report_controller/widget_spec.rb index 5c65351ae6c..b4d76e5c095 100644 --- a/spec/controllers/report_controller/widget_spec.rb +++ b/spec/controllers/report_controller/widget_spec.rb @@ -67,7 +67,7 @@ @edit_hash[:new][:visibility_typ] = 'role' @edit_hash[:new][:roles] = [role.id.to_s] controller.send(:widget_set_record_vars, @widget) - expect(@widget.visibility[:roles]).to eq([role.name]) + expect(@widget.visibility[:roles]).to eq([role.id.to_s]) end it "sets group visibility for widget" do @@ -79,4 +79,41 @@ end end + context "#widget_get_node_info" do + before do + user = FactoryBot.create(:user_with_group) + login_as user + allow(controller).to receive(:current_user).and_return(user) + allow(User).to receive(:server_timezone).and_return("UTC") + allow(MiqServer).to receive(:my_server).and_return(double("MiqServer", :logon_status => :ready, :id => 1)) + controller.instance_variable_set(:@sb, {}) + end + + it "loads role names from role IDs for display" do + role1 = FactoryBot.create(:miq_user_role, :name => "WidgetRole1") + role2 = FactoryBot.create(:miq_user_role, :name => "WidgetRole2") + widget = FactoryBot.create(:miq_widget, + :title => "Widget With Roles", + :visibility => {:roles => [role1.id, role2.id]}) + + controller.instance_variable_set(:@sb, {:trees => {:widgets_tree => {:active_node => "root-xx-#{widget.id}"}}}) + controller.x_node = "root-xx-#{widget.id}" + controller.send(:widget_get_node_info) + + expect(assigns(:sb)[:user_roles]).to match_array(["WidgetRole1", "WidgetRole2"]) + end + + it "handles widgets with _ALL_ visibility" do + widget = FactoryBot.create(:miq_widget, + :title => "Widget For All", + :visibility => {:roles => ["_ALL_"]}) + + controller.instance_variable_set(:@sb, {:trees => {:widgets_tree => {:active_node => "root-xx-#{widget.id}"}}}) + controller.x_node = "root-xx-#{widget.id}" + controller.send(:widget_get_node_info) + + expect(assigns(:sb)[:user_roles]).to eq([]) + end + end + end From 740008bad70890c9406b9a7bbcfe389044110630 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Wed, 1 Apr 2026 17:10:25 -0400 Subject: [PATCH 2/7] Fix widget spec and improve test coverage for role/group visibility - Fix widget_spec expectation to use group.id instead of group.description - Add test for group visibility display in widget_get_node_info - Remove obvious comments from Cypress tests (kept TODO comment) Related to ManageIQ/manageiq-schema#XXX where we converted visibility from storing role names/group descriptions to storing IDs. This ensures tests properly verify ID-based lookups and display of names/descriptions. --- app/controllers/report_controller/widgets.rb | 16 +- app/helpers/report_helper.rb | 12 +- .../customization_buttons.cy.js | 20 ++- .../e2e/ui/Cloud-Intel/Reports/widgets.cy.js | 146 ++++++++++++++++-- .../report_controller/widget_spec.rb | 18 ++- spec/helpers/report_helper_spec.rb | 28 ++++ 6 files changed, 209 insertions(+), 31 deletions(-) diff --git a/app/controllers/report_controller/widgets.rb b/app/controllers/report_controller/widgets.rb index 33999bd9dd4..84d9947b71b 100644 --- a/app/controllers/report_controller/widgets.rb +++ b/app/controllers/report_controller/widgets.rb @@ -296,10 +296,8 @@ def widget_get_node_info @sb[:user_roles] = Rbac.filtered(MiqUserRole.where(:id => role_ids)).order(:name).pluck(:name) end elsif @widget.visibility && @widget.visibility[:groups] - @sb[:groups] = [] - Rbac.filtered(MiqGroup.non_tenant_groups_in_my_region).sort_by(&:description).each do |r| - @sb[:groups].push(r.description) if @widget.visibility[:groups].include?(r.description) - end + group_ids = @widget.visibility[:groups] + @sb[:groups] = Rbac.filtered(MiqGroup.where(:id => group_ids)).order(:description).pluck(:description) end end end @@ -334,8 +332,7 @@ def widget_set_form_vars @edit[:new][:roles] = @widget.visibility[:roles][0] == "_ALL_" ? ["_ALL_"] : @widget.visibility[:roles].sort elsif @widget.visibility[:groups] @edit[:new][:visibility_typ] = "group" - groups = Rbac.filtered(MiqGroup.in_my_region.where(:description => @widget.visibility[:groups])) - @edit[:new][:groups] = groups.collect(&:id).sort + @edit[:new][:groups] = @widget.visibility[:groups][0] == "_ALL_" ? ["_ALL_"] : @widget.visibility[:groups].sort end end @edit[:sorted_user_roles] = @@ -558,12 +555,7 @@ def widget_set_record_vars(widget) widget.content_type = WIDGET_CONTENT_TYPE[@sb[:wtype]] widget.visibility ||= {} if @edit[:new][:visibility_typ] == "group" - groups = [] - @edit[:new][:groups].each do |g| - group = MiqGroup.find(g) - groups.push(group.description) if g == group.id.to_s - end - widget.visibility[:groups] = groups + widget.visibility[:groups] = @edit[:new][:groups] widget.visibility.delete(:roles) if widget.visibility[:roles] else if @edit[:new][:visibility_typ] == "role" diff --git a/app/helpers/report_helper.rb b/app/helpers/report_helper.rb index 6a480fddfe5..353236c4613 100644 --- a/app/helpers/report_helper.rb +++ b/app/helpers/report_helper.rb @@ -30,7 +30,17 @@ def visibility_options(widget) if values.first == "_ALL_" _("To All Users") else - _("By %{typ}: %{values}") % {:typ => typ.to_s.titleize, :values => values.join(',')} + # TODO: Come back to this, I think group and roles are both doing it wrong and once we always get ids here, we can remove this logic and get the description/name/etc. + display_values = if typ == :roles + # Convert role IDs to role names for display + MiqUserRole.where(:id => values).pluck(:name).join(',') + elsif typ == :groups + # Convert group IDs to group descriptions for display + MiqGroup.where(:id => values).pluck(:description).join(',') + else + values.join(',') + end + _("By %{typ}: %{values}") % {:typ => typ.to_s.titleize, :values => display_values} end end diff --git a/cypress/e2e/ui/Automation/Embedded-Automate/customization_buttons.cy.js b/cypress/e2e/ui/Automation/Embedded-Automate/customization_buttons.cy.js index 25b83c8fae8..94f72dd5570 100644 --- a/cypress/e2e/ui/Automation/Embedded-Automate/customization_buttons.cy.js +++ b/cypress/e2e/ui/Automation/Embedded-Automate/customization_buttons.cy.js @@ -23,17 +23,25 @@ describe('Automation > Embedded Automate > Customization > Buttons', () => { cy.get('#description').type('Test button description'); cy.get('.icon-button').click(); + cy.get('.cds--modal-container').should('be.visible'); cy.get(':nth-child(1) > span > .ff').click(); - cy.get('.bx--modal-footer > .bx--btn--primary').click(); + cy.get('.cds--modal-footer > .cds--btn--primary').click(); cy.tabs({ tabLabel: 'Advanced' }); cy.get('#object_request').scrollIntoView(); cy.get('#object_request').type('test_request'); - cy.get('#form_role_visibility button[data-id="visibility_typ"]').scrollIntoView(); - cy.get('#form_role_visibility button[data-id="visibility_typ"]').click(); - cy.get('#form_role_visibility .dropdown-menu [data-original-index="1"] > a').click(); + cy.interceptApi({ + alias: 'visibilityChanged', + urlPattern: '/miq_ae_customization/automate_button_field_changed', + triggerFn: () => { + cy.get('#form_role_visibility button[data-id="visibility_typ"]').scrollIntoView(); + cy.get('#form_role_visibility button[data-id="visibility_typ"]').click(); + cy.get('#form_role_visibility .dropdown-menu [data-original-index="1"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); cy.contains('User Roles').scrollIntoView(); @@ -51,9 +59,7 @@ describe('Automation > Embedded Automate > Customization > Buttons', () => { waitOnlyIfRequestIntercepted: true, }); - // Click Add button - // TODO: cy.getFormButtonByTypeWithText expects this in the main_content div, but it's in the form_buttons_div - // within the full_content div - enhance the cy.getFormButtonByTypeWithText to accept a div id or normalize the location of the button + // TODO: cy.getFormButtonByTypeWithText expects button in main_content div, but it's in form_buttons_div cy.get('#buttons_on > .btn-primary').click(); cy.expect_flash(flashClassMap.success, 'was added'); diff --git a/cypress/e2e/ui/Cloud-Intel/Reports/widgets.cy.js b/cypress/e2e/ui/Cloud-Intel/Reports/widgets.cy.js index 9612cbd8ada..e88262ed0ba 100644 --- a/cypress/e2e/ui/Cloud-Intel/Reports/widgets.cy.js +++ b/cypress/e2e/ui/Cloud-Intel/Reports/widgets.cy.js @@ -4,9 +4,9 @@ import { flashClassMap } from '../../../../support/assertions/assertion_constant describe('Cloud Intel > Reports > Widgets', () => { beforeEach(() => { cy.login(); - cy.menu('Cloud Intel', 'Reports'); + cy.menu('Overview', 'Reports'); cy.get('#explorer_title_text'); - cy.accordion('Widgets'); + cy.accordion('Dashboard Widgets'); }); afterEach(() => { @@ -15,21 +15,60 @@ describe('Cloud Intel > Reports > Widgets', () => { describe('Widget Creation', () => { it('Creates a new widget with role visibility', () => { - cy.selectAccordionItem(['Report Widgets']); + cy.selectAccordionItem(['All Widgets', 'Reports']); cy.toolbar('Configuration', 'Add a new Widget'); cy.get('#explorer_title_text').contains('Adding a new Widget'); cy.get('#title').type('Test Widget'); cy.get('#description').type('Test widget description'); - cy.get('button[data-id="repfilter_typ"]').click(); - cy.get('.dropdown-menu [data-original-index="1"] > a').click(); + cy.interceptApi({ + alias: 'filterChanged', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => { + cy.get('button[data-id="filter_typ"]').click(); + cy.get('button[data-id="filter_typ"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); + + cy.interceptApi({ + alias: 'subfilterChanged', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => { + cy.get('button[data-id="subfilter_typ"]').click(); + cy.get('button[data-id="subfilter_typ"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); + + cy.interceptApi({ + alias: 'reportChanged', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => { + cy.get('button[data-id="repfilter_typ"]').click(); + cy.get('button[data-id="repfilter_typ"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); - cy.get('button[data-id="visibility_typ"]').scrollIntoView(); - cy.get('button[data-id="visibility_typ"]').click(); - cy.get('.dropdown-menu [data-original-index="1"] > a').click(); + cy.get('button[data-id="chosen_pivot1"]').should('be.visible').and('not.be.disabled'); + cy.get('button[data-id="chosen_pivot1"]').click(); + cy.get('button[data-id="chosen_pivot1"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + + cy.interceptApi({ + alias: 'visibilityChanged', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => { + cy.get('button[data-id="visibility_typ"]').scrollIntoView(); + cy.get('button[data-id="visibility_typ"]').click(); + cy.get('button[data-id="visibility_typ"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); cy.contains('User Roles').scrollIntoView(); + cy.get('#form_role_visibility').find('input[type="checkbox"][id^="roles_"]').should('have.length.at.least', 2); cy.interceptApi({ alias: 'checkAuditorRole', @@ -51,8 +90,95 @@ describe('Cloud Intel > Reports > Widgets', () => { cy.get('.clickable-row').contains('Test Widget').click(); cy.get('#main_div').contains('Test Widget'); - cy.get('.visibility').contains('EvmRole-auditor'); - cy.get('.visibility').contains('EvmRole-desktop'); + cy.get('#main_div').contains('EvmRole-auditor'); + cy.get('#main_div').contains('EvmRole-desktop'); + }); + + it('Creates a new widget with group visibility', () => { + cy.selectAccordionItem(['All Widgets', 'Reports']); + cy.toolbar('Configuration', 'Add a new Widget'); + cy.get('#explorer_title_text').contains('Adding a new Widget'); + + cy.get('#title').type('Test Widget Group'); + cy.get('#description').type('Test widget with group visibility'); + + cy.interceptApi({ + alias: 'filterChanged', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => { + cy.get('button[data-id="filter_typ"]').click(); + cy.get('button[data-id="filter_typ"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); + + cy.interceptApi({ + alias: 'subfilterChanged', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => { + cy.get('button[data-id="subfilter_typ"]').click(); + cy.get('button[data-id="subfilter_typ"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); + + cy.interceptApi({ + alias: 'reportChanged', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => { + cy.get('button[data-id="repfilter_typ"]').click(); + cy.get('button[data-id="repfilter_typ"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); + + cy.get('button[data-id="chosen_pivot1"]').should('be.visible').and('not.be.disabled'); + cy.get('button[data-id="chosen_pivot1"]').click(); + cy.get('button[data-id="chosen_pivot1"]').parent().find('.dropdown-menu [data-original-index="1"] > a').click(); + + cy.interceptApi({ + alias: 'visibilityChanged', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => { + cy.get('button[data-id="visibility_typ"]').scrollIntoView(); + cy.get('button[data-id="visibility_typ"]').click(); + cy.get('button[data-id="visibility_typ"]').parent().find('.dropdown-menu [data-original-index="2"] > a').click(); + }, + waitOnlyIfRequestIntercepted: true, + }); + + cy.contains('Groups').scrollIntoView(); + cy.get('#form_role_visibility').find('input[type="checkbox"][id^="groups_"]').should('have.length.at.least', 2); + + cy.interceptApi({ + alias: 'checkFirstGroup', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => cy.get('#form_role_visibility').find('input[type="checkbox"][id^="groups_"]').eq(0).check(), + waitOnlyIfRequestIntercepted: true, + }); + + cy.interceptApi({ + alias: 'checkSecondGroup', + urlPattern: '/report/widget_form_field_changed', + triggerFn: () => cy.get('#form_role_visibility').find('input[type="checkbox"][id^="groups_"]').eq(1).check(), + waitOnlyIfRequestIntercepted: true, + }); + + cy.get('#form_role_visibility').find('input[type="checkbox"][id^="groups_"]').eq(0).closest('td').invoke('text').then((group1Text) => { + cy.get('#form_role_visibility').find('input[type="checkbox"][id^="groups_"]').eq(1).closest('td').invoke('text').then((group2Text) => { + const group1 = group1Text.trim(); + const group2 = group2Text.trim(); + + cy.get('#buttons_on > .btn-primary').click(); + cy.expect_flash(flashClassMap.success, 'was saved'); + + cy.get('.clickable-row').contains('Test Widget Group').click(); + + cy.get('#main_div').contains('Test Widget Group'); + cy.get('#main_div').contains(group1); + cy.get('#main_div').contains(group2); + }); + }); }); }); }); diff --git a/spec/controllers/report_controller/widget_spec.rb b/spec/controllers/report_controller/widget_spec.rb index b4d76e5c095..5e257d9dd57 100644 --- a/spec/controllers/report_controller/widget_spec.rb +++ b/spec/controllers/report_controller/widget_spec.rb @@ -75,7 +75,7 @@ @edit_hash[:new][:visibility_typ] = 'group' @edit_hash[:new][:groups] = [group.id.to_s] controller.send(:widget_set_record_vars, @widget) - expect(@widget.visibility[:groups]).to eq([group.description]) + expect(@widget.visibility[:groups]).to eq([group.id.to_s]) end end @@ -114,6 +114,22 @@ expect(assigns(:sb)[:user_roles]).to eq([]) end + + it "loads group descriptions from permitted group IDs for display" do + user = controller.send(:current_user) + group = user.current_group + group.update!(:description => "WidgetGroup1") + + widget = FactoryBot.create(:miq_widget, + :title => "Widget With Groups", + :visibility => {:groups => [group.id]}) + + controller.instance_variable_set(:@sb, {:trees => {:widgets_tree => {:active_node => "root-xx-#{widget.id}"}}}) + controller.x_node = "root-xx-#{widget.id}" + controller.send(:widget_get_node_info) + + expect(assigns(:sb)[:groups]).to eq(["WidgetGroup1"]) + end end end diff --git a/spec/helpers/report_helper_spec.rb b/spec/helpers/report_helper_spec.rb index 44cf5d52998..81df8c72fbe 100644 --- a/spec/helpers/report_helper_spec.rb +++ b/spec/helpers/report_helper_spec.rb @@ -65,4 +65,32 @@ def create_and_generate_report_for_user(report_name, user) expect(options).to eq(expected_array) end end + + describe '#visibility_options' do + let(:role1) { FactoryBot.create(:miq_user_role, :name => 'EvmRole-auditor') } + let(:role2) { FactoryBot.create(:miq_user_role, :name => 'EvmRole-desktop') } + + it 'returns "To All Users" when visibility is set to all' do + widget = FactoryBot.create(:miq_widget, :visibility => {:roles => ['_ALL_']}) + expect(helper.visibility_options(widget)).to eq('To All Users') + end + + it 'converts role IDs to role names for display' do + widget = FactoryBot.create(:miq_widget, :visibility => {:roles => [role1.id, role2.id]}) + result = helper.visibility_options(widget) + expect(result).to include('EvmRole-auditor') + expect(result).to include('EvmRole-desktop') + expect(result).to include('By Roles:') + end + + it 'converts group IDs to group descriptions for display' do + group1 = FactoryBot.create(:miq_group, :description => 'Group1') + group2 = FactoryBot.create(:miq_group, :description => 'Group2') + widget = FactoryBot.create(:miq_widget, :visibility => {:groups => [group1.id, group2.id]}) + result = helper.visibility_options(widget) + expect(result).to include('Group1') + expect(result).to include('Group2') + expect(result).to include('By Groups:') + end + end end From 2281d43c0d1fa0035a37dfe3060a143de61301ce Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Fri, 3 Apr 2026 12:30:55 -0400 Subject: [PATCH 3/7] Simplify the report helper by pushing widgets logic to the model https://github.com/ManageIQ/manageiq/pull/23803 --- app/helpers/report_helper.rb | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/app/helpers/report_helper.rb b/app/helpers/report_helper.rb index 353236c4613..f0d27c40216 100644 --- a/app/helpers/report_helper.rb +++ b/app/helpers/report_helper.rb @@ -30,16 +30,7 @@ def visibility_options(widget) if values.first == "_ALL_" _("To All Users") else - # TODO: Come back to this, I think group and roles are both doing it wrong and once we always get ids here, we can remove this logic and get the description/name/etc. - display_values = if typ == :roles - # Convert role IDs to role names for display - MiqUserRole.where(:id => values).pluck(:name).join(',') - elsif typ == :groups - # Convert group IDs to group descriptions for display - MiqGroup.where(:id => values).pluck(:description).join(',') - else - values.join(',') - end + display_values = widget.visibility_values.join(',') _("By %{typ}: %{values}") % {:typ => typ.to_s.titleize, :values => display_values} end end From c776fe9532b06824296f780ce76c6171df230f7e Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Tue, 7 Apr 2026 11:55:05 -0400 Subject: [PATCH 4/7] Consistently use numbers for group/role ids Previously, with the data migration and UI code that stored ids as numbers in the database, the code that lit up the checkboxes failed to match ids as strings with the numbers from the database. --- app/controllers/application_controller.rb | 2 +- spec/controllers/application_controller/buttons_spec.rb | 4 ++-- spec/controllers/report_controller/widget_spec.rb | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4cf9bebde56..1de5586801f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1691,7 +1691,7 @@ def visibility_box_edit params.each do |var, value| next unless var.starts_with?(prefix) - name = var.split(prefix).last + name = var.split(prefix).last.to_i if value == "1" @edit[:new][key] |= [name] # union elsif value.downcase == "null" diff --git a/spec/controllers/application_controller/buttons_spec.rb b/spec/controllers/application_controller/buttons_spec.rb index 6b4c7b028d0..fe6ccc307b1 100644 --- a/spec/controllers/application_controller/buttons_spec.rb +++ b/spec/controllers/application_controller/buttons_spec.rb @@ -344,9 +344,9 @@ end it "sets new role and preserves old role for custom button" do - edit[:new][:roles] = [old_role.id, role.id.to_s] # old roles are represented by int and new ones by string + edit[:new][:roles] = [old_role.id, role.id] controller.send(:button_set_record_vars, custom_button) - expect(custom_button.visibility[:roles]).to eq([old_role.id, role.id.to_s]) + expect(custom_button.visibility[:roles]).to eq([old_role.id, role.id]) end end diff --git a/spec/controllers/report_controller/widget_spec.rb b/spec/controllers/report_controller/widget_spec.rb index 5e257d9dd57..a8a2cfbc5fa 100644 --- a/spec/controllers/report_controller/widget_spec.rb +++ b/spec/controllers/report_controller/widget_spec.rb @@ -65,17 +65,17 @@ it "sets role visibility for widget" do role = FactoryBot.create(:miq_user_role, :name => "foo") @edit_hash[:new][:visibility_typ] = 'role' - @edit_hash[:new][:roles] = [role.id.to_s] + @edit_hash[:new][:roles] = [role.id] controller.send(:widget_set_record_vars, @widget) - expect(@widget.visibility[:roles]).to eq([role.id.to_s]) + expect(@widget.visibility[:roles]).to eq([role.id]) end it "sets group visibility for widget" do group = FactoryBot.create(:miq_group, :description => "foo_group") @edit_hash[:new][:visibility_typ] = 'group' - @edit_hash[:new][:groups] = [group.id.to_s] + @edit_hash[:new][:groups] = [group.id] controller.send(:widget_set_record_vars, @widget) - expect(@widget.visibility[:groups]).to eq([group.id.to_s]) + expect(@widget.visibility[:groups]).to eq([group.id]) end end From 94eb81f81647b8d198f04694dcf25a11f4e72453 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Tue, 7 Apr 2026 15:43:53 -0400 Subject: [PATCH 5/7] Fix widget test failures caused by FactoryBot sequence desync The miq_widget factory has separate sequences for both title and description attributes. When tests override :title, they skip the title sequence but the description sequence still increments, causing the sequences to become out of sync (e.g., title='widget_0000000000001', description='widget_0000000000002'). This caused test failures when: 1. A test overrode :title (causing sequence desync) 2. A subsequent test expected title and description to match 3. TreeBuilderReportWidgets uses widget.title for tree node display Fixed by: - Removing unnecessary :title overrides in widget_spec.rb (lines 96, 108, 124) These tests only care about visibility settings, not specific title text - Updating explorer_spec.rb to use widget.title instead of widget.description Added comment explaining TreeBuilderReportWidgets uses title attribute This makes tests resilient to execution order and prevents future sequence desync issues. The factory-generated titles are sufficient for these tests. --- spec/controllers/application_controller/explorer_spec.rb | 5 +++-- spec/controllers/report_controller/widget_spec.rb | 3 --- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/controllers/application_controller/explorer_spec.rb b/spec/controllers/application_controller/explorer_spec.rb index f3d9149d78a..2d245a72a10 100644 --- a/spec/controllers/application_controller/explorer_spec.rb +++ b/spec/controllers/application_controller/explorer_spec.rb @@ -69,10 +69,11 @@ :active_tree => :widgets_tree) TreeBuilderReportWidgets.new('widgets_tree', {}) nodes = controller.send(:tree_add_child_nodes, 'xx-r') + # TreeBuilderReportWidgets uses widget.title for tree node text (see x_get_tree_custom_kids) expected = [{:key => "xx-r_-#{widget.id}", - :text => widget.name, + :text => widget.title, :icon => 'fa fa-file-text-o', - :tooltip => widget.name, + :tooltip => widget.title, :state => {:expanded => false}, :selectable => true}] expect(nodes).to eq(expected) diff --git a/spec/controllers/report_controller/widget_spec.rb b/spec/controllers/report_controller/widget_spec.rb index a8a2cfbc5fa..b9f4a461ef6 100644 --- a/spec/controllers/report_controller/widget_spec.rb +++ b/spec/controllers/report_controller/widget_spec.rb @@ -93,7 +93,6 @@ role1 = FactoryBot.create(:miq_user_role, :name => "WidgetRole1") role2 = FactoryBot.create(:miq_user_role, :name => "WidgetRole2") widget = FactoryBot.create(:miq_widget, - :title => "Widget With Roles", :visibility => {:roles => [role1.id, role2.id]}) controller.instance_variable_set(:@sb, {:trees => {:widgets_tree => {:active_node => "root-xx-#{widget.id}"}}}) @@ -105,7 +104,6 @@ it "handles widgets with _ALL_ visibility" do widget = FactoryBot.create(:miq_widget, - :title => "Widget For All", :visibility => {:roles => ["_ALL_"]}) controller.instance_variable_set(:@sb, {:trees => {:widgets_tree => {:active_node => "root-xx-#{widget.id}"}}}) @@ -121,7 +119,6 @@ group.update!(:description => "WidgetGroup1") widget = FactoryBot.create(:miq_widget, - :title => "Widget With Groups", :visibility => {:groups => [group.id]}) controller.instance_variable_set(:@sb, {:trees => {:widgets_tree => {:active_node => "root-xx-#{widget.id}"}}}) From 31b0b398780dde70e9906d782c47f1eeb3335cda Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Tue, 7 Apr 2026 15:58:34 -0400 Subject: [PATCH 6/7] Sort roles/groups (numerically by id) with space separator --- app/helpers/report_helper.rb | 2 +- spec/helpers/report_helper_spec.rb | 30 ++++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/app/helpers/report_helper.rb b/app/helpers/report_helper.rb index f0d27c40216..b28d15f62ae 100644 --- a/app/helpers/report_helper.rb +++ b/app/helpers/report_helper.rb @@ -30,7 +30,7 @@ def visibility_options(widget) if values.first == "_ALL_" _("To All Users") else - display_values = widget.visibility_values.join(',') + display_values = widget.visibility_values.sort.join(', ') _("By %{typ}: %{values}") % {:typ => typ.to_s.titleize, :values => display_values} end end diff --git a/spec/helpers/report_helper_spec.rb b/spec/helpers/report_helper_spec.rb index 81df8c72fbe..e009955c1a9 100644 --- a/spec/helpers/report_helper_spec.rb +++ b/spec/helpers/report_helper_spec.rb @@ -75,22 +75,36 @@ def create_and_generate_report_for_user(report_name, user) expect(helper.visibility_options(widget)).to eq('To All Users') end - it 'converts role IDs to role names for display' do + it 'converts role IDs to role names and sorts alphabetically by name' do widget = FactoryBot.create(:miq_widget, :visibility => {:roles => [role1.id, role2.id]}) result = helper.visibility_options(widget) - expect(result).to include('EvmRole-auditor') - expect(result).to include('EvmRole-desktop') - expect(result).to include('By Roles:') + expect(result).to eq('By Roles: EvmRole-auditor, EvmRole-desktop') end - it 'converts group IDs to group descriptions for display' do + it 'sorts role names alphabetically regardless of ID order' do + role3 = FactoryBot.create(:miq_user_role, :name => 'EvmRole-zebra') + role4 = FactoryBot.create(:miq_user_role, :name => 'EvmRole-alpha') + widget = FactoryBot.create(:miq_widget, :visibility => {:roles => [role3.id, role4.id]}) + result = helper.visibility_options(widget) + # IDs are in order [zebra, alpha] but output should be alphabetically sorted + expect(result).to eq('By Roles: EvmRole-alpha, EvmRole-zebra') + end + + it 'converts group IDs to group descriptions and sorts alphabetically by description' do group1 = FactoryBot.create(:miq_group, :description => 'Group1') group2 = FactoryBot.create(:miq_group, :description => 'Group2') widget = FactoryBot.create(:miq_widget, :visibility => {:groups => [group1.id, group2.id]}) result = helper.visibility_options(widget) - expect(result).to include('Group1') - expect(result).to include('Group2') - expect(result).to include('By Groups:') + expect(result).to eq('By Groups: Group1, Group2') + end + + it 'sorts group descriptions alphabetically regardless of ID order' do + group3 = FactoryBot.create(:miq_group, :description => 'Zebra Group') + group4 = FactoryBot.create(:miq_group, :description => 'Alpha Group') + widget = FactoryBot.create(:miq_widget, :visibility => {:groups => [group3.id, group4.id]}) + result = helper.visibility_options(widget) + # IDs are in order [Zebra, Alpha] but output should be alphabetically sorted + expect(result).to eq('By Groups: Alpha Group, Zebra Group') end end end From f679c1ca405fe02f437c718b0b1a41edab28c4ad Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Fri, 10 Apr 2026 08:14:59 -0400 Subject: [PATCH 7/7] Restore rbac filtering of roles/groups to what the user can access --- app/controllers/report_controller/widgets.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/controllers/report_controller/widgets.rb b/app/controllers/report_controller/widgets.rb index 84d9947b71b..28fa2c5af62 100644 --- a/app/controllers/report_controller/widgets.rb +++ b/app/controllers/report_controller/widgets.rb @@ -329,10 +329,20 @@ def widget_set_form_vars if @widget.visibility if @widget.visibility[:roles] @edit[:new][:visibility_typ] = @widget.visibility[:roles][0] == "_ALL_" ? "all" : "role" - @edit[:new][:roles] = @widget.visibility[:roles][0] == "_ALL_" ? ["_ALL_"] : @widget.visibility[:roles].sort + if @widget.visibility[:roles][0] == "_ALL_" + @edit[:new][:roles] = ["_ALL_"] + else + role_ids = Rbac.filtered(MiqUserRole.where(:id => @widget.visibility[:roles])) + @edit[:new][:roles] = role_ids.pluck(:id).sort + end elsif @widget.visibility[:groups] @edit[:new][:visibility_typ] = "group" - @edit[:new][:groups] = @widget.visibility[:groups][0] == "_ALL_" ? ["_ALL_"] : @widget.visibility[:groups].sort + if @widget.visibility[:groups][0] == "_ALL_" + @edit[:new][:groups] = ["_ALL_"] + else + group_ids = Rbac.filtered(MiqGroup.where(:id => @widget.visibility[:groups])) + @edit[:new][:groups] = group_ids.pluck(:id).sort + end end end @edit[:sorted_user_roles] =