Disable <Choose a Role> option to prevent error while adding Group #5933

hstastna · 2019-08-02T13:52:57Z

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1730402

Unexpected error occurs if we choose < Choose a Role > option in the drop down for specifying some Role while adding a new Group (after we had some Role already chosen). With disabling this option, we prevent occurring errors and we also bring more consistency because it is not possible to choose <Choose a Project/Tenant> option, too, for specifying a Project or Tenant.

With disabling option, we also prevent saving group with in the drop down.

Note:
Name, Role and Project or Tenant are required to specify for adding new Group.

Before:

After:

hstastna · 2019-08-02T13:54:05Z

@miq-bot add_label bug, ivanchuk/yes, hammer/yes

romanblanco · 2019-08-05T10:20:40Z

app/views/ops/_rbac_group_details.html.haml

@@ -70,8 +70,8 @@
            = link_to(@group.miq_user_role.name, "#", params)
        - else
          = select_tag('group_role',
-              options_for_select(@edit[:roles].sort, @edit[:new][:role]),
-              :class  => "selectpicker")
+                       options_for_select(@edit[:roles].sort, selected: @edit[:new][:role], disabled: ["<Choose a Role>", nil]),


@hstastna use arrows instead of the colon for selected and disabled parameters

@romanblanco Done ;)

romanblanco

Now, Add button get's enabled only after setting Project/Tenant field, but submitting the form without setting Role in the dropdown throws an error:

[----] F, [2019-08-05T12:04:41.126740 #27616:2b18c50e5070] FATAL -- : Error caught: [ActiveRecord::RecordNotFound] Couldn't find MiqUser
Role with 'id'=<Choose a Role>
/home/rblanco/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/activerecord-5.1.7/lib/active_record/relation/finder_methods.rb:343:in `rai
se_record_not_found_exception!'
/home/rblanco/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/activerecord-5.1.7/lib/active_record/relation/finder_methods.rb:460:in `fin
d_one'
/home/rblanco/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/activerecord-5.1.7/lib/active_record/relation/finder_methods.rb:440:in `fin
d_with_ids'
/home/rblanco/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/activerecord-5.1.7/lib/active_record/relation/finder_methods.rb:66:in `find
'
/home/rblanco/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/activerecord-5.1.7/lib/active_record/querying.rb:3:in `find'
/home/rblanco/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/activerecord-5.1.7/lib/active_record/core.rb:178:in `find'
/home/rblanco/devel/manageiq-ui-classic/app/controllers/ops_controller/ops_rbac.rb:1225:in `rbac_group_set_record_vars'
/home/rblanco/devel/manageiq-ui-classic/app/controllers/ops_controller/ops_rbac.rb:673:in `rbac_edit_save_or_add'
/home/rblanco/devel/manageiq-ui-classic/app/controllers/ops_controller/ops_rbac.rb:92:in `rbac_group_edit'

The Add button should be enabled after setting both Project/Tenant and Role fields.

Also "Description can't be blank" error message shows up if you skip the Description, so even this field should probably affect Add button, to prevent the error message.

@@ -1226,7 +1222,7 @@ module OpsController::OpsRbac

   # Set group record variables to new values
   def rbac_group_set_record_vars(group)
-    role = MiqUserRole.find(@edit[:new][:role])
+    role = MiqUserRole.find(@edit[:new][:role].second) if @edit[:new][:role].second.present?
     group.description = @edit[:new][:description]
     group.miq_user_role = role
     group.tenant = Tenant.find(@edit[:new][:group_tenant]) if @edit[:new][:group_tenant]

makes it available to submit the form without Role.

hstastna · 2019-08-05T13:17:33Z

@romanblanco So do you think that Add button should be enabled after setting Project/Tenant, Role AND Description (everything in Group Information section)? Thank you.

hstastna · 2019-08-05T13:33:23Z

I think we don't want to submit the form without Role. I was told that Name, Role and Project or Tenant are required to specify for adding new Group.

romanblanco · 2019-08-05T13:39:32Z

@romanblanco So do you think that Add button should be enabled after setting Project/Tenant, Role AND Description (everything in Group Information section)? Thank you.

it yells if Description is missing and fails if Role is missing, I think yes. ☺️

hstastna · 2019-08-05T13:45:30Z

@romanblanco This is the logic which allows Add button to be enabled only if some Role and Tenant is set. I can add the logic also for the Name there. But I am not sure if we want this in this PR. What do you think about it? Because it's little bit different issue (Add/Save button also does not respond properly for editing a Group). And this PR should be mainly about fixing the BZ :)

romanblanco

LGTM 👍

martinpovolny · 2019-08-05T15:05:38Z

app/views/ops/_rbac_group_details.html.haml

@@ -69,9 +69,11 @@
            %i.ff.ff-user-role{params}
            = link_to(@group.miq_user_role.name, "#", params)
        - else
+          - disabled_item = ["<Choose a Role>", nil]
+          - selected_item = @edit[:new][:role] ? @edit[:new][:role] : disabled_item


Perhaps shorter:

- selected_item = @edit[:new][:role] || disabled_item

hstastna · 2019-08-05T17:00:44Z

I've created an issue regarding Add/Save button ;)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1730402

miq-bot · 2019-08-07T09:17:07Z

Checked commits hstastna/manageiq-ui-classic@fc0c980~...029a9a0 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

Disable <Choose a Role> option to prevent error while adding Group (cherry picked from commit 8538572) https://bugzilla.redhat.com/show_bug.cgi?id=1730402

simaishi · 2019-08-07T22:34:20Z

Ivanchuk backport details:

$ git log -1
commit 03edbba27814be6cb25f7d2e7aa2640c12d2b35a
Author: Harpreet Kataria <[email protected]>
Date:   Wed Aug 7 12:04:19 2019 -0400

    Merge pull request #5933 from hstastna/Error_changing_Role_adding_Group
    
    Disable <Choose a Role> option to prevent error while adding Group
    
    (cherry picked from commit 85385728dc262b35e33cd46d048aceac7756c017)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1730402

miq-bot added bug hammer/yes labels Aug 2, 2019

hstastna changed the title ~~Disable <Choose a Role> option to prevent error while adding Group~~ [WIP] Disable <Choose a Role> option to prevent error while adding Group Aug 2, 2019

miq-bot added the wip label Aug 2, 2019

hstastna force-pushed the Error_changing_Role_adding_Group branch from a648843 to d6ae9a7 Compare August 2, 2019 15:13

hstastna changed the title ~~[WIP] Disable <Choose a Role> option to prevent error while adding Group~~ Disable <Choose a Role> option to prevent error while adding Group Aug 2, 2019

miq-bot removed the wip label Aug 2, 2019

romanblanco reviewed Aug 5, 2019

View reviewed changes

hstastna force-pushed the Error_changing_Role_adding_Group branch 2 times, most recently from 4c2426b to 6e6cf95 Compare August 5, 2019 10:46

romanblanco suggested changes Aug 5, 2019

View reviewed changes

hstastna force-pushed the Error_changing_Role_adding_Group branch from 6e6cf95 to 0efaef0 Compare August 5, 2019 12:43

hstastna force-pushed the Error_changing_Role_adding_Group branch 2 times, most recently from 47ee19c to 5025fcc Compare August 5, 2019 13:30

romanblanco approved these changes Aug 5, 2019

View reviewed changes

martinpovolny reviewed Aug 5, 2019

View reviewed changes

hstastna force-pushed the Error_changing_Role_adding_Group branch from 5025fcc to bd6a93c Compare August 5, 2019 15:13

hstastna mentioned this pull request Aug 5, 2019

Inconsistencies regarding responding of Add/Save button while adding/editing Group #5941

Closed

Hilda Stastna added 2 commits August 7, 2019 11:16

Disable <Choose a Role> option to prevent error while adding Group

fc0c980

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1730402

Add spec to check if 'Choose a Role' is disabled if present in drop down

029a9a0

hstastna force-pushed the Error_changing_Role_adding_Group branch from bd6a93c to 029a9a0 Compare August 7, 2019 09:16

h-kataria added the settings label Aug 7, 2019

h-kataria added the changelog/yes label Aug 7, 2019

h-kataria self-assigned this Aug 7, 2019

h-kataria added this to the Sprint 118 Ending Aug 19, 2019 milestone Aug 7, 2019

h-kataria merged commit 8538572 into ManageIQ:master Aug 7, 2019

hstastna mentioned this pull request Aug 7, 2019

Make Add/Save button response properly while adding/editing Group #5947

Merged

simaishi added ivanchuk/backported and removed ivanchuk/yes labels Aug 7, 2019

Fryguy added hammer/yes? and removed hammer/yes labels Nov 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable <Choose a Role> option to prevent error while adding Group #5933

Disable <Choose a Role> option to prevent error while adding Group #5933

hstastna commented Aug 2, 2019 •

edited

Loading

hstastna commented Aug 2, 2019

romanblanco Aug 5, 2019

hstastna Aug 5, 2019

romanblanco left a comment •

edited

Loading

hstastna commented Aug 5, 2019 •

edited

Loading

hstastna commented Aug 5, 2019

romanblanco commented Aug 5, 2019

hstastna commented Aug 5, 2019 •

edited

Loading

romanblanco left a comment

martinpovolny Aug 5, 2019

hstastna commented Aug 5, 2019

miq-bot commented Aug 7, 2019

simaishi commented Aug 7, 2019

Disable <Choose a Role> option to prevent error while adding Group #5933

Disable <Choose a Role> option to prevent error while adding Group #5933

Conversation

hstastna commented Aug 2, 2019 • edited Loading

hstastna commented Aug 2, 2019

romanblanco Aug 5, 2019

Choose a reason for hiding this comment

hstastna Aug 5, 2019

Choose a reason for hiding this comment

romanblanco left a comment • edited Loading

Choose a reason for hiding this comment

hstastna commented Aug 5, 2019 • edited Loading

hstastna commented Aug 5, 2019

romanblanco commented Aug 5, 2019

hstastna commented Aug 5, 2019 • edited Loading

romanblanco left a comment

Choose a reason for hiding this comment

martinpovolny Aug 5, 2019

Choose a reason for hiding this comment

hstastna commented Aug 5, 2019

miq-bot commented Aug 7, 2019

simaishi commented Aug 7, 2019

hstastna commented Aug 2, 2019 •

edited

Loading

romanblanco left a comment •

edited

Loading

hstastna commented Aug 5, 2019 •

edited

Loading

hstastna commented Aug 5, 2019 •

edited

Loading