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.

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

Fix creating a group after validation message 'already exists' #5354

Merged

Conversation

hstastna
Copy link

@hstastna hstastna commented Mar 19, 2019

Fixes issue:
#5140
BZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1690889

What:
Fix creating a group (Configuration > Access Control > Groups) for the second time, after a validation message like 'Description is not unique...' appears.

Steps to reproduce:

  1. Go to Configuration > Access Control > Groups accordion
  2. Configuration > Add a new Group and create a group
  3. Try to create another group with the same description as from step 2
    => error flash message occurs
  4. Follow the message and change the description of the new group, press Add button
    => nothing happens in the UI + error:
[----] I, [2019-03-19T17:28:55.994747 #11522:2ad94de6c2f0]  INFO -- : Started POST "/ops/rbac_group_edit?button=add" for ::1 at 2019-03-19 17:28:55 +0100
[----] I, [2019-03-19T17:28:56.033217 #11522:2ad94de6c2f0]  INFO -- : Processing by OpsController#rbac_group_edit as JS
[----] I, [2019-03-19T17:28:56.033346 #11522:2ad94de6c2f0]  INFO -- :   Parameters: {"button"=>"add"}
[----] F, [2019-03-19T17:28:56.045202 #11522:2ad94de6c2f0] FATAL -- : Error caught: [NoMethodError] undefined method `empty?' for nil:NilClass
/home/hstastna/manageiq/manageiq-ui-classic/app/controllers/ops_controller/ops_rbac.rb:1377:in `rbac_group_validate?'
/home/hstastna/manageiq/manageiq-ui-classic/app/controllers/ops_controller/ops_rbac.rb:674:in `rbac_edit_save_or_add'
/home/hstastna/manageiq/manageiq-ui-classic/app/controllers/ops_controller/ops_rbac.rb:92:in `rbac_group_edit'

TODO:

  • specs

Note:
This issue is also present in the latest gaprindashvili and hammer branches.

Details:
In rbac_group_set_record_vars method, in https://github.com/ManageIQ/manageiq-ui-classic/pull/5354/files#diff-8078c760e06b820719715450b2a112e0L1203, @edit[:new][:filter_expression] was set to nil, so then in rbac_group_validate? method, in https://github.com/ManageIQ/manageiq-ui-classic/pull/5354/files#diff-8078c760e06b820719715450b2a112e0L1377 a problem occurred, because empty? does not work with nil, obviously. Also setting @edit[:new][:filter_expression] to nil is in conflict with what is in https://github.com/ManageIQ/manageiq-ui-classic/pull/5354/files#diff-8078c760e06b820719715450b2a112e0L1111 where we set it to {}.


Step 3 from steps to reproduce:
step3

Before: (nothing happens after clicking on Add button, test2 group is not saved properly)
test_before

After:(test2 group saved without any problems, flash message appears)
test_after

@hstastna
Copy link
Author

@miq-bot add_label bug, hammer/yes

@miq-bot
Copy link
Member

miq-bot commented Mar 19, 2019

Checked commit hstastna@7090474 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@hstastna
Copy link
Author

@lpichler, what do you think about the fix? Thanks! 🎇

@mzazrivec mzazrivec self-assigned this Mar 20, 2019
@mzazrivec mzazrivec added this to the Sprint 108 Ending Apr 1, 2019 milestone Mar 20, 2019
@mzazrivec mzazrivec merged commit 8b2f589 into ManageIQ:master Mar 20, 2019
@hstastna
Copy link
Author

@miq-bot add_label gaprindashvili/yes

hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this pull request Mar 20, 2019
hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this pull request Mar 21, 2019
hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this pull request Mar 21, 2019
simaishi pushed a commit that referenced this pull request May 21, 2019
Fix creating a group after validation message 'already exists'

(cherry picked from commit 8b2f589)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1712440
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit f3a5226f87bf42c6cde7e5f7db5674efe1ee1032
Author: Milan Zázrivec <[email protected]>
Date:   Wed Mar 20 10:25:20 2019 +0100

    Merge pull request #5354 from hstastna/Cannot_create_group_already_exist
    
    Fix creating a group after validation message 'already exists'
    
    (cherry picked from commit 8b2f589ea5d0775dabd1667012b9f36fab8534cb)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1712440

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants