Fix Manage policies and Check compliance buttons for Container Images and others #4206

hstastna · 2018-06-25T17:08:45Z

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

What:
Fix Manage policies button for Container Images which was "not yet implemented" even if this functionality is implemented and working if not accessing Container Images through Contaier provider's summary page but directly (Compute > Containers > Container Images).
Fix the same problem also for Container Replicators, Container Pods, Container Nodes.
Fix the same "Button not yet implemented" problem for Container Replicators, Pods, Nodes and Images also for Check Compliance of Last Known Configuration button.

Steps to reproduce:

Go to Compute > Containers > Providers
Choose some provider, display its summary screen
Under Relationships, click on Container Images
Choose some Container Image(s) (check the appropriate checkbox(es))
Under Policy (toolbar), click on Manage policies
=> nothing happens, just "Button not yet implemented" flash message is displayed!

Details:
Calling assign_policies method for managing policies was simply missing in EmsCommon module, in button method. This did not work for some other nested lists displayed through Containers provider's summary page. If we go directly to Compute > Containers > Container Images/Nodes/Pods/Replicators, all the buttons work well.

Before:
Managing policies of selected Container Image:

Checking Compliance of Last Known Configuration of selected Container Image:
(the same picture as above)

After:
Managing policies of selected Container Image:

Checking Compliance of Last Known Configuration of selected Container Image:

hstastna · 2018-06-25T17:09:08Z

@miq-bot add_label bug, gaprindashvili/yes

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1530259 Add missing assign_policies method calling to button method to make managing policies work for Container Images, Replicators etc.

hstastna · 2018-06-28T12:52:16Z

@martinpovolny @mzazrivec Could you, please, review? Thanks!

hstastna · 2018-06-29T10:37:11Z

@skateman Could you, please, review my specs? Thank you! ;)

skateman

Specs are beautiful, just small changes 😉

skateman · 2018-06-29T10:37:58Z

spec/controllers/ems_common_controller_spec.rb

@@ -202,8 +202,8 @@ def test_setting_few_fields
      end
    end

-    context "#button" do
-      before(:each) do
+    describe "#button" do


😍 😍 😍

skateman · 2018-06-29T10:38:06Z

spec/controllers/ems_common_controller_spec.rb

      context "updates provider with new token" do
-        before :each do
+        before do


😍 😍 😍

skateman · 2018-06-29T10:38:09Z

spec/controllers/ems_common_controller_spec.rb

@@ -126,9 +126,9 @@ def test_creating(emstype)
      end
    end

-    context "#update" do
+    describe "#update" do


😍 😍 😍

skateman · 2018-06-29T10:38:15Z

spec/controllers/ems_common_controller_spec.rb

-    context "#button" do
-      before(:each) do
+    describe "#button" do
+      before do


😍 😍 😍

skateman · 2018-06-29T10:40:18Z

spec/controllers/ems_common_controller_spec.rb

+          'container_replicator' => 'Container Replicators',
+          'container_node'       => 'Container Nodes',
+          'container_group'      => 'Container Pods'
+        }.each do |display_s, items|


You might not need the hash values, just use an array and then call humanize.pluralize on each item.

But.. what about Container Pods?

Do you really need it in the testing, or just in the describe/context/it strings?

If I removed the hash values and replaced them by an array of four items, I don't know how to set the string for context block properly and nicely (to see there for example 'tagging selected Container Pods' and not 'tagging selected Container Groups'). It's only for this there - for info what is being tested (context).

I consider this solution as the easiest and the most readable (at least for now). If something went wrong in some of the specs, we can easily recognize, what went wrong: we will not need to figure out that container_group is related to Container Pods because it's already there. And I need all of the contexts there to set params[:pressed] properly for each situation.

skateman · 2018-06-29T10:41:05Z

spec/controllers/ems_common_controller_spec.rb

+        let(:provider) { ManageIQ::Providers::ContainerManager.new }
+
+        before do
+          allow(controller).to receive(:javascript_redirect).and_return(true)


Do you really need the and_return here?

I will check it once again, thank you very much, David!

Add tests for the buttons for nested lists of Container Images/Nodes/Pods/Replicators.

miq-bot · 2018-06-29T12:44:13Z

Checked commits hstastna/manageiq-ui-classic@2c9ccc8~...e81fdf3 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

hstastna · 2018-07-11T09:32:02Z

@skateman @martinpovolny Any update about this? Could this be merged or any other things which I should add/change? Thank you! ;)

skateman · 2018-07-11T10:38:27Z

@hstastna I'll take a look after lunch

skateman

miq-bot added bug gaprindashvili/yes wip labels Jun 25, 2018

hstastna changed the title ~~[WIP] Fix Manage policies button for Container Images and others~~ [WIP] Fix Manage policies and Check compliance button for Container Images and others Jun 25, 2018

hstastna changed the title ~~[WIP] Fix Manage policies and Check compliance button for Container Images and others~~ [WIP] Fix Manage policies and Check compliance buttons for Container Images and others Jun 25, 2018

hstastna force-pushed the Manage_policies_button_not_yet_implemented_Container_Images branch 5 times, most recently from 99b1e89 to eb527c8 Compare June 26, 2018 12:15

Hilda Stastna added 2 commits June 28, 2018 11:41

Refactor part of button method of EmsCommon module

2c9ccc8

Fix Manage policies button for Container Images

b376835

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1530259 Add missing assign_policies method calling to button method to make managing policies work for Container Images, Replicators etc.

hstastna force-pushed the Manage_policies_button_not_yet_implemented_Container_Images branch from 007ab31 to d257fb5 Compare June 28, 2018 10:09

hstastna changed the title ~~[WIP] Fix Manage policies and Check compliance buttons for Container Images and others~~ Fix Manage policies and Check compliance buttons for Container Images and others Jun 28, 2018

miq-bot removed the wip label Jun 28, 2018

Fix Check compliance button for Container Images and others

7879ab1

hstastna force-pushed the Manage_policies_button_not_yet_implemented_Container_Images branch from d257fb5 to 7879ab1 Compare June 28, 2018 12:49

skateman suggested changes Jun 29, 2018

View reviewed changes

hstastna force-pushed the Manage_policies_button_not_yet_implemented_Container_Images branch from b00c996 to 7339c81 Compare June 29, 2018 12:35

Add tests for Tag, Manage policies and Check compliance buttons

e81fdf3

Add tests for the buttons for nested lists of Container Images/Nodes/Pods/Replicators.

hstastna force-pushed the Manage_policies_button_not_yet_implemented_Container_Images branch from 7339c81 to e81fdf3 Compare June 29, 2018 12:37

martinpovolny assigned skateman Jul 2, 2018

skateman approved these changes Jul 11, 2018

View reviewed changes

mzazrivec added this to the Sprint 90 Ending Jul 16, 2018 milestone Jul 11, 2018

mzazrivec merged commit 9279890 into ManageIQ:master Jul 11, 2018

mzazrivec added the compute/containers label Jul 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Manage policies and Check compliance buttons for Container Images and others #4206

Fix Manage policies and Check compliance buttons for Container Images and others #4206

hstastna commented Jun 25, 2018 •

edited

Loading

hstastna commented Jun 25, 2018

hstastna commented Jun 28, 2018 •

edited

Loading

hstastna commented Jun 29, 2018

skateman left a comment

skateman Jun 29, 2018

skateman Jun 29, 2018

skateman Jun 29, 2018

skateman Jun 29, 2018

skateman Jun 29, 2018

hstastna Jun 29, 2018

skateman Jun 29, 2018

hstastna Jun 29, 2018 •

edited

Loading

skateman Jul 2, 2018

skateman Jun 29, 2018

hstastna Jun 29, 2018

miq-bot commented Jun 29, 2018

hstastna commented Jul 11, 2018

skateman commented Jul 11, 2018

skateman left a comment

Fix Manage policies and Check compliance buttons for Container Images and others #4206

Fix Manage policies and Check compliance buttons for Container Images and others #4206

Conversation

hstastna commented Jun 25, 2018 • edited Loading

hstastna commented Jun 25, 2018

hstastna commented Jun 28, 2018 • edited Loading

hstastna commented Jun 29, 2018

skateman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hstastna Jun 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Jun 29, 2018

hstastna commented Jul 11, 2018

skateman commented Jul 11, 2018

skateman left a comment

Choose a reason for hiding this comment

hstastna commented Jun 25, 2018 •

edited

Loading

hstastna commented Jun 28, 2018 •

edited

Loading

hstastna Jun 29, 2018 •

edited

Loading