From e6515c0dff601e2aca425275edd34cbf6970fdb6 Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Wed, 12 Oct 2016 15:38:25 +0200 Subject: [PATCH 01/14] Remove unused toolbar buttons (MIQ Server, MIQ Region) --- app/helpers/application_helper/toolbar_builder.rb | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/app/helpers/application_helper/toolbar_builder.rb b/app/helpers/application_helper/toolbar_builder.rb index d16b47a26a7..1269995b723 100644 --- a/app/helpers/application_helper/toolbar_builder.rb +++ b/app/helpers/application_helper/toolbar_builder.rb @@ -688,7 +688,7 @@ def build_toolbar_hide_button(id) case id when "role_start", "role_suspend", "promote_server", "demote_server" return true - when "log_download", "refresh_logs", "log_collect", "log_reload", "logdepot_edit", "processmanager_restart", "refresh_workers" + when "refresh_workers" return true end when "ServerRole" @@ -711,20 +711,8 @@ def build_toolbar_hide_button(id) @record.try(:middleware_server).try(:product) == 'Hawkular') when "NilClass" case id - when "log_download" - return true if ["workers", "download_logs"].include?(@lastaction) - when "log_collect" - return true if ["workers", "evm_logs", "audit_logs"].include?(@lastaction) - when "log_reload" - return true if ["workers", "download_logs"].include?(@lastaction) - when "logdepot_edit" - return true if ["workers", "evm_logs", "audit_logs"].include?(@lastaction) - when "processmanager_restart" - return true if ["download_logs", "evm_logs", "audit_logs"].include?(@lastaction) when "refresh_workers" return true if ["download_logs", "evm_logs", "audit_logs"].include?(@lastaction) - when "refresh_logs" - return true if ["audit_logs", "evm_logs", "workers"].include?(@lastaction) end end false # No reason to hide, allow the button to show From 25651b0af3d51d2c7edceca73ce50b43058e7d53 Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Wed, 12 Oct 2016 15:58:38 +0200 Subject: [PATCH 02/14] Remove specs for unused toolbar buttons (MIQ Server, MIQ Region) --- .../button/configured_system_provision.rb | 0 .../button/refresh_workers.rb | 0 .../configured_system_provision_spec.rb | 0 .../buttons/refresh_workers_spec.rb | 0 .../toolbar_builder_spec.rb | 49 +------------------ 5 files changed, 2 insertions(+), 47 deletions(-) create mode 100644 app/helpers/application_helper/button/configured_system_provision.rb create mode 100644 app/helpers/application_helper/button/refresh_workers.rb create mode 100644 spec/helpers/application_helper/buttons/configured_system_provision_spec.rb create mode 100644 spec/helpers/application_helper/buttons/refresh_workers_spec.rb diff --git a/app/helpers/application_helper/button/configured_system_provision.rb b/app/helpers/application_helper/button/configured_system_provision.rb new file mode 100644 index 00000000000..e69de29bb2d diff --git a/app/helpers/application_helper/button/refresh_workers.rb b/app/helpers/application_helper/button/refresh_workers.rb new file mode 100644 index 00000000000..e69de29bb2d diff --git a/spec/helpers/application_helper/buttons/configured_system_provision_spec.rb b/spec/helpers/application_helper/buttons/configured_system_provision_spec.rb new file mode 100644 index 00000000000..e69de29bb2d diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb new file mode 100644 index 00000000000..e69de29bb2d diff --git a/spec/helpers/application_helper/toolbar_builder_spec.rb b/spec/helpers/application_helper/toolbar_builder_spec.rb index 28239822577..5494ad92731 100644 --- a/spec/helpers/application_helper/toolbar_builder_spec.rb +++ b/spec/helpers/application_helper/toolbar_builder_spec.rb @@ -737,7 +737,7 @@ def method_missing(sym, *args) end ["role_start", "role_suspend", "promote_server", "demote_server", - "log_download", "refresh_logs", "log_collect", "log_reload", "logdepot_edit", "processmanager_restart", "refresh_workers"].each do |id| + "refresh_workers"].each do |id| it "and id = #{id}" do @id = id expect(subject).to be_truthy @@ -789,52 +789,7 @@ def method_missing(sym, *args) stub_user(:features => :all) end - ["log_download", "log_reload"].each do |id| - context "and id = #{id}" do - before { @id = id } - - it "and @lastaction = workers" do - @lastaction = "workers" - expect(subject).to be_truthy - end - - it "and @lastaction = download_logs" do - @lastaction = "download_logs" - expect(subject).to be_truthy - end - - it "otherwise" do - expect(subject).to be_falsey - end - end - end - - ["log_collect", "logdepot_edit", "refresh_logs"].each do |id| - context "and id = #{id}" do - before { @id = id } - - it "and @lastaction = workers" do - @lastaction = "workers" - expect(subject).to be_truthy - end - - it "and @lastaction = evm_logs" do - @lastaction = "evm_logs" - expect(subject).to be_truthy - end - - it "and @lastaction = audit_logs" do - @lastaction = "audit_logs" - expect(subject).to be_truthy - end - - it "otherwise" do - expect(subject).to be_falsey - end - end - end - - ["processmanager_restart", "refresh_workers"].each do |id| + ["refresh_workers"].each do |id| context "and id = #{id}" do before { @id = id } From d4567ffad21b7cc7f5f760097190d1e6f648be19 Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Wed, 12 Oct 2016 16:58:25 +0200 Subject: [PATCH 03/14] Create class for refresh_workers and remove unnecessary code --- app/helpers/application_helper/button/refresh_workers.rb | 7 +++++++ .../toolbar/diagnostics_server_center.rb | 3 ++- app/helpers/application_helper/toolbar_builder.rb | 5 ----- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/helpers/application_helper/button/refresh_workers.rb b/app/helpers/application_helper/button/refresh_workers.rb index e69de29bb2d..25107f33523 100644 --- a/app/helpers/application_helper/button/refresh_workers.rb +++ b/app/helpers/application_helper/button/refresh_workers.rb @@ -0,0 +1,7 @@ +class ApplicationHelper::Button::RefreshWorkers < ApplicationHelper::Button::Basic + needs :@lastaction + + def visible? + !%w(download_logs evm_logs audit_logs).include?(@lastaction) + end +end diff --git a/app/helpers/application_helper/toolbar/diagnostics_server_center.rb b/app/helpers/application_helper/toolbar/diagnostics_server_center.rb index 2e43e6cf608..0cc9f318bd3 100644 --- a/app/helpers/application_helper/toolbar/diagnostics_server_center.rb +++ b/app/helpers/application_helper/toolbar/diagnostics_server_center.rb @@ -9,7 +9,8 @@ class ApplicationHelper::Toolbar::DiagnosticsServerCenter < ApplicationHelper::T :refresh_workers, 'fa fa-repeat fa-lg', N_('Reload current workers display'), - nil), + nil, + :klass => ApplicationHelper::Button::RefreshWorkers), button( :refresh_audit_log, 'fa fa-repeat fa-lg', diff --git a/app/helpers/application_helper/toolbar_builder.rb b/app/helpers/application_helper/toolbar_builder.rb index 1269995b723..e5a55cb1ae0 100644 --- a/app/helpers/application_helper/toolbar_builder.rb +++ b/app/helpers/application_helper/toolbar_builder.rb @@ -709,11 +709,6 @@ def build_toolbar_hide_button(id) middleware_datasource_remove middleware_datasource_add).include?(id) && (@record.try(:product) == 'Hawkular' || @record.try(:middleware_server).try(:product) == 'Hawkular') - when "NilClass" - case id - when "refresh_workers" - return true if ["download_logs", "evm_logs", "audit_logs"].include?(@lastaction) - end end false # No reason to hide, allow the button to show end From eef8583c5f71b2a9f1659efb2425f6072c6b9256 Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Wed, 12 Oct 2016 16:59:50 +0200 Subject: [PATCH 04/14] Add spec for refresh_workers toolbar button --- .../buttons/refresh_workers_spec.rb | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb index e69de29bb2d..46748cbf370 100644 --- a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb +++ b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb @@ -0,0 +1,21 @@ +describe ApplicationHelper::Button::RefreshWorkers do + describe '#visible?' do + %w(download_logs evm_logs audit_logs).each do |lastaction| + context "action #{lastaction} being taken as last" do + it 'will be skipped for this record' do + view_context = setup_view_context_with_sandbox({}) + button = described_class.new(view_context, {}, {'lastaction' => lastaction}, {}) + expect(button.visible?).to be_falsey + end + end + end + + context 'other action being taken as last' do + it 'will not be skipped for this record' do + view_context = setup_view_context_with_sandbox({}) + button = described_class.new(view_context, {}, {'lastaction' => 'worker_logs'}, {}) + expect(button.visible?).to be_truthy + end + end + end +end \ No newline at end of file From 968989e00d48007f4d1ec2bc237dcaab3fd25b39 Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Wed, 12 Oct 2016 17:23:22 +0200 Subject: [PATCH 05/14] Remove refresh_worker from toolbar_builder --- app/helpers/application_helper/toolbar_builder.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/helpers/application_helper/toolbar_builder.rb b/app/helpers/application_helper/toolbar_builder.rb index e5a55cb1ae0..07d9274bf56 100644 --- a/app/helpers/application_helper/toolbar_builder.rb +++ b/app/helpers/application_helper/toolbar_builder.rb @@ -688,8 +688,6 @@ def build_toolbar_hide_button(id) case id when "role_start", "role_suspend", "promote_server", "demote_server" return true - when "refresh_workers" - return true end when "ServerRole" case id From 5aea60936c74b54fe14a030dbd86caece17aa7ab Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Thu, 13 Oct 2016 10:03:06 +0200 Subject: [PATCH 06/14] Add missing final newline --- spec/helpers/application_helper/buttons/refresh_workers_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb index 46748cbf370..55aa11c36ed 100644 --- a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb +++ b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb @@ -18,4 +18,4 @@ end end end -end \ No newline at end of file +end From 012d005af9a42050817df3579c24dfea08656b1b Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Thu, 13 Oct 2016 10:11:13 +0200 Subject: [PATCH 07/14] Delete empty files --- .../application_helper/button/configured_system_provision.rb | 0 .../buttons/configured_system_provision_spec.rb | 0 2 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 app/helpers/application_helper/button/configured_system_provision.rb delete mode 100644 spec/helpers/application_helper/buttons/configured_system_provision_spec.rb diff --git a/app/helpers/application_helper/button/configured_system_provision.rb b/app/helpers/application_helper/button/configured_system_provision.rb deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/spec/helpers/application_helper/buttons/configured_system_provision_spec.rb b/spec/helpers/application_helper/buttons/configured_system_provision_spec.rb deleted file mode 100644 index e69de29bb2d..00000000000 From 287cedeb8e87c9b985414319651cd79338d513d4 Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Thu, 13 Oct 2016 11:06:38 +0200 Subject: [PATCH 08/14] Add needed record to refresh_workers button class and remove old examples from toolbar_builder_spec --- .../button/refresh_workers.rb | 2 +- .../buttons/refresh_workers_spec.rb | 11 ++++-- .../toolbar_builder_spec.rb | 35 +------------------ 3 files changed, 11 insertions(+), 37 deletions(-) diff --git a/app/helpers/application_helper/button/refresh_workers.rb b/app/helpers/application_helper/button/refresh_workers.rb index 25107f33523..035b1798f1a 100644 --- a/app/helpers/application_helper/button/refresh_workers.rb +++ b/app/helpers/application_helper/button/refresh_workers.rb @@ -1,5 +1,5 @@ class ApplicationHelper::Button::RefreshWorkers < ApplicationHelper::Button::Basic - needs :@lastaction + needs :@record, :@lastaction def visible? !%w(download_logs evm_logs audit_logs).include?(@lastaction) diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb index 55aa11c36ed..26e5538c212 100644 --- a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb +++ b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb @@ -1,10 +1,17 @@ describe ApplicationHelper::Button::RefreshWorkers do + before :all do + @record = FactoryGirl.create( + :assigned_server_role, + :miq_server => FactoryGirl.create(:miq_server) + ) + end + describe '#visible?' do %w(download_logs evm_logs audit_logs).each do |lastaction| context "action #{lastaction} being taken as last" do it 'will be skipped for this record' do view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'lastaction' => lastaction}, {}) + button = described_class.new(view_context, {}, {'record' => @record, 'lastaction' => lastaction}, {}) expect(button.visible?).to be_falsey end end @@ -13,7 +20,7 @@ context 'other action being taken as last' do it 'will not be skipped for this record' do view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'lastaction' => 'worker_logs'}, {}) + button = described_class.new(view_context, {}, {'record' => @record, 'lastaction' => 'worker_logs'}, {}) expect(button.visible?).to be_truthy end end diff --git a/spec/helpers/application_helper/toolbar_builder_spec.rb b/spec/helpers/application_helper/toolbar_builder_spec.rb index 5494ad92731..232160919f4 100644 --- a/spec/helpers/application_helper/toolbar_builder_spec.rb +++ b/spec/helpers/application_helper/toolbar_builder_spec.rb @@ -736,8 +736,7 @@ def method_missing(sym, *args) stub_user(:features => :all) end - ["role_start", "role_suspend", "promote_server", "demote_server", - "refresh_workers"].each do |id| + ["role_start", "role_suspend", "promote_server", "demote_server"].each do |id| it "and id = #{id}" do @id = id expect(subject).to be_truthy @@ -783,38 +782,6 @@ def method_missing(sym, *args) end end - context "when with record = nil" do - before do - @record = nil - stub_user(:features => :all) - end - - ["refresh_workers"].each do |id| - context "and id = #{id}" do - before { @id = id } - - it "and @lastaction = download_logs" do - @lastaction = "download_logs" - expect(subject).to be_truthy - end - - it "and @lastaction = evm_logs" do - @lastaction = "evm_logs" - expect(subject).to be_truthy - end - - it "and @lastaction = audit_logs" do - @lastaction = "audit_logs" - expect(subject).to be_truthy - end - - it "otherwise" do - expect(subject).to be_falsey - end - end - end - end - context "NilClass" do before do @record = nil From 9cdc58fa609fffb03e8c414fddc2ed1a5b4a90e5 Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Tue, 18 Oct 2016 13:16:01 +0200 Subject: [PATCH 09/14] Fix Rubocop issues --- .../application_helper/buttons/refresh_workers_spec.rb | 5 ++--- spec/helpers/application_helper/toolbar_builder_spec.rb | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb index 26e5538c212..6cfcaae5b19 100644 --- a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb +++ b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb @@ -1,9 +1,8 @@ describe ApplicationHelper::Button::RefreshWorkers do before :all do @record = FactoryGirl.create( - :assigned_server_role, - :miq_server => FactoryGirl.create(:miq_server) - ) + :assigned_server_role, + :miq_server => FactoryGirl.create(:miq_server)) end describe '#visible?' do diff --git a/spec/helpers/application_helper/toolbar_builder_spec.rb b/spec/helpers/application_helper/toolbar_builder_spec.rb index 232160919f4..7edc408a460 100644 --- a/spec/helpers/application_helper/toolbar_builder_spec.rb +++ b/spec/helpers/application_helper/toolbar_builder_spec.rb @@ -736,7 +736,7 @@ def method_missing(sym, *args) stub_user(:features => :all) end - ["role_start", "role_suspend", "promote_server", "demote_server"].each do |id| + %w(role_start role_suspend promote_server demote_server).each do |id| it "and id = #{id}" do @id = id expect(subject).to be_truthy From 92a87556576ad1c2d24d108dbaa843d78ab3718a Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Thu, 20 Oct 2016 16:40:50 +0200 Subject: [PATCH 10/14] Remove extra occurence of refresh_workers in the toolbar_builder, extend the #visible? method by additional cases and add new spec examples accordingly --- .../application_helper/button/refresh_workers.rb | 16 ++++++++++++++-- .../application_helper/toolbar_builder.rb | 2 +- .../buttons/refresh_workers_spec.rb | 11 +++++++++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/app/helpers/application_helper/button/refresh_workers.rb b/app/helpers/application_helper/button/refresh_workers.rb index 035b1798f1a..5e1a912d1bb 100644 --- a/app/helpers/application_helper/button/refresh_workers.rb +++ b/app/helpers/application_helper/button/refresh_workers.rb @@ -1,7 +1,19 @@ class ApplicationHelper::Button::RefreshWorkers < ApplicationHelper::Button::Basic - needs :@record, :@lastaction + needs :@record, :@lastaction, @sb def visible? - !%w(download_logs evm_logs audit_logs).include?(@lastaction) + return true if diagnostics_worker_tab? && !miq? + return !%w(download_logs evm_logs audit_logs).include?(@lastaction) if @record.class == NilClass + true + end + + private + + def diagnostics_worker_tab? + @view_context.x_active_tree == :diagnostics_tree && @sb[:active_tab] == 'diagnostics_workers' + end + + def miq? + @record.class == MiqServer || @record.class == MiqRegion end end diff --git a/app/helpers/application_helper/toolbar_builder.rb b/app/helpers/application_helper/toolbar_builder.rb index 07d9274bf56..31172e1773c 100644 --- a/app/helpers/application_helper/toolbar_builder.rb +++ b/app/helpers/application_helper/toolbar_builder.rb @@ -413,7 +413,7 @@ def build_toolbar_hide_button_ops(id) when "diagnostics_summary" return !["refresh_server_summary", "restart_server"].include?(id) when "diagnostics_workers" - return !["refresh_workers", "restart_workers"].include?(id) + return !["restart_workers"].include?(id) else return true end diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb index 6cfcaae5b19..5a5148824cf 100644 --- a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb +++ b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb @@ -6,11 +6,18 @@ end describe '#visible?' do + it 'should be visible when active_tab == diagnostics_workers in :diagnostics_tree' do + view_context = setup_view_context_with_sandbox({}) + button = described_class.new(view_context, {}, {'record' => @record}, {}) + button.instance_variable_set(:@sb, {:active_tab => 'diagnostics_workers'}) + expect(button.visible?).to be_truthy + end + %w(download_logs evm_logs audit_logs).each do |lastaction| context "action #{lastaction} being taken as last" do it 'will be skipped for this record' do view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'record' => @record, 'lastaction' => lastaction}, {}) + button = described_class.new(view_context, {}, {'record' => nil, 'lastaction' => lastaction}, {}) expect(button.visible?).to be_falsey end end @@ -19,7 +26,7 @@ context 'other action being taken as last' do it 'will not be skipped for this record' do view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'record' => @record, 'lastaction' => 'worker_logs'}, {}) + button = described_class.new(view_context, {}, {'record' => nil, 'lastaction' => 'worker_logs'}, {}) expect(button.visible?).to be_truthy end end From 33f97819d85f8cc104a38d55623ca6a56f0a7db0 Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Fri, 21 Oct 2016 20:31:15 +0200 Subject: [PATCH 11/14] Put back the removed occurence of refresh_workers in #toolbar_builder to make the button visible again --- app/helpers/application_helper/toolbar_builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/application_helper/toolbar_builder.rb b/app/helpers/application_helper/toolbar_builder.rb index 31172e1773c..d543e347b6d 100644 --- a/app/helpers/application_helper/toolbar_builder.rb +++ b/app/helpers/application_helper/toolbar_builder.rb @@ -413,7 +413,7 @@ def build_toolbar_hide_button_ops(id) when "diagnostics_summary" return !["refresh_server_summary", "restart_server"].include?(id) when "diagnostics_workers" - return !["restart_workers"].include?(id) + return !["restart_workers", "refresh_workers"].include?(id) else return true end From a194d8e1d909041fbb9747d84dc810cf261b67ef Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Fri, 21 Oct 2016 20:45:12 +0200 Subject: [PATCH 12/14] Fix code style offenses detected by Rubocop --- app/helpers/application_helper/toolbar_builder.rb | 2 +- spec/helpers/application_helper/buttons/refresh_workers_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/application_helper/toolbar_builder.rb b/app/helpers/application_helper/toolbar_builder.rb index d543e347b6d..4957eaf7ca6 100644 --- a/app/helpers/application_helper/toolbar_builder.rb +++ b/app/helpers/application_helper/toolbar_builder.rb @@ -413,7 +413,7 @@ def build_toolbar_hide_button_ops(id) when "diagnostics_summary" return !["refresh_server_summary", "restart_server"].include?(id) when "diagnostics_workers" - return !["restart_workers", "refresh_workers"].include?(id) + return !%w(restart_workers refresh_workers).include?(id) else return true end diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb index 5a5148824cf..b6b3f8fdfaa 100644 --- a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb +++ b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb @@ -9,7 +9,7 @@ it 'should be visible when active_tab == diagnostics_workers in :diagnostics_tree' do view_context = setup_view_context_with_sandbox({}) button = described_class.new(view_context, {}, {'record' => @record}, {}) - button.instance_variable_set(:@sb, {:active_tab => 'diagnostics_workers'}) + button.instance_variable_set(:@sb, :active_tab => 'diagnostics_workers') expect(button.visible?).to be_truthy end From 0f79a50e0297a300d4e24678795aeed4f989fa0b Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Mon, 24 Oct 2016 11:31:53 +0200 Subject: [PATCH 13/14] Correct #visible? method so the button gets hidden when the record is miq_server or miq_region only if layout != ops and tailors the spec file accordingly --- .../button/refresh_workers.rb | 8 +++-- .../buttons/refresh_workers_spec.rb | 35 +++++++++++++------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/app/helpers/application_helper/button/refresh_workers.rb b/app/helpers/application_helper/button/refresh_workers.rb index 5e1a912d1bb..2c268ed3a3b 100644 --- a/app/helpers/application_helper/button/refresh_workers.rb +++ b/app/helpers/application_helper/button/refresh_workers.rb @@ -1,8 +1,8 @@ class ApplicationHelper::Button::RefreshWorkers < ApplicationHelper::Button::Basic - needs :@record, :@lastaction, @sb + needs :@record, :@lastaction, :@sb, :@layout def visible? - return true if diagnostics_worker_tab? && !miq? + return false if miq? && !(ops? && diagnostics_worker_tab?) return !%w(download_logs evm_logs audit_logs).include?(@lastaction) if @record.class == NilClass true end @@ -16,4 +16,8 @@ def diagnostics_worker_tab? def miq? @record.class == MiqServer || @record.class == MiqRegion end + + def ops? + @layout == 'ops' + end end diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb index b6b3f8fdfaa..001cbc75a9d 100644 --- a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb +++ b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb @@ -1,20 +1,35 @@ describe ApplicationHelper::Button::RefreshWorkers do before :all do - @record = FactoryGirl.create( - :assigned_server_role, - :miq_server => FactoryGirl.create(:miq_server)) + @records = [FactoryGirl.create(:miq_server), FactoryGirl.create(:miq_region)] end describe '#visible?' do - it 'should be visible when active_tab == diagnostics_workers in :diagnostics_tree' do - view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'record' => @record}, {}) - button.instance_variable_set(:@sb, :active_tab => 'diagnostics_workers') - expect(button.visible?).to be_truthy + context 'when layout == ops and active_tab == diagnostics_workers in :diagnostics_tree' do + it 'will not be skipped' do + view_context = setup_view_context_with_sandbox({}) + button = described_class.new(view_context, {}, {'record' => nil, 'layout' => 'ops'}, {}) + button.instance_variable_set(:@sb, :active_tab => 'diagnostics_workers') + allow(view_context).to receive(:x_active_tree).and_return(:diagnostics_tree) + expect(button.visible?).to be_truthy + end + end + + context 'when layout != ops and' do + %w(miq_server miq_region).each_with_index do |record, i| + context "record is #{record}" do + it 'will be skipped' do + view_context = setup_view_context_with_sandbox({}) + button = described_class.new(view_context, {}, {'record' => @records[i], 'layout' => 'not ops'}, {}) + button.instance_variable_set(:@sb, :active_tab => 'does not matter') + allow(view_context).to receive(:x_active_tree).and_return(:diagnostics_tree) + expect(button.visible?).to be_falsey + end + end + end end %w(download_logs evm_logs audit_logs).each do |lastaction| - context "action #{lastaction} being taken as last" do + context "when action #{lastaction} being taken as last" do it 'will be skipped for this record' do view_context = setup_view_context_with_sandbox({}) button = described_class.new(view_context, {}, {'record' => nil, 'lastaction' => lastaction}, {}) @@ -23,7 +38,7 @@ end end - context 'other action being taken as last' do + context 'when other action being taken as last' do it 'will not be skipped for this record' do view_context = setup_view_context_with_sandbox({}) button = described_class.new(view_context, {}, {'record' => nil, 'lastaction' => 'worker_logs'}, {}) From 2797d555fe3bf39b4e9d8b26c6e61c7dfb5c250a Mon Sep 17 00:00:00 2001 From: Attila Vecerek Date: Wed, 26 Oct 2016 20:31:11 +0200 Subject: [PATCH 14/14] Adopt the changes suggested by @jzigmund --- .../button/refresh_workers.rb | 18 +------ .../buttons/refresh_workers_spec.rb | 54 +++++++------------ 2 files changed, 19 insertions(+), 53 deletions(-) diff --git a/app/helpers/application_helper/button/refresh_workers.rb b/app/helpers/application_helper/button/refresh_workers.rb index 2c268ed3a3b..b2a4006a9ee 100644 --- a/app/helpers/application_helper/button/refresh_workers.rb +++ b/app/helpers/application_helper/button/refresh_workers.rb @@ -1,23 +1,7 @@ class ApplicationHelper::Button::RefreshWorkers < ApplicationHelper::Button::Basic - needs :@record, :@lastaction, :@sb, :@layout + needs :@record, :@sb def visible? - return false if miq? && !(ops? && diagnostics_worker_tab?) - return !%w(download_logs evm_logs audit_logs).include?(@lastaction) if @record.class == NilClass - true - end - - private - - def diagnostics_worker_tab? @view_context.x_active_tree == :diagnostics_tree && @sb[:active_tab] == 'diagnostics_workers' end - - def miq? - @record.class == MiqServer || @record.class == MiqRegion - end - - def ops? - @layout == 'ops' - end end diff --git a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb index 001cbc75a9d..45d1c64affc 100644 --- a/spec/helpers/application_helper/buttons/refresh_workers_spec.rb +++ b/spec/helpers/application_helper/buttons/refresh_workers_spec.rb @@ -1,47 +1,29 @@ describe ApplicationHelper::Button::RefreshWorkers do - before :all do - @records = [FactoryGirl.create(:miq_server), FactoryGirl.create(:miq_region)] - end + let(:view_context) { setup_view_context_with_sandbox({}) } + let(:record) { FactoryGirl.create(:miq_server) } describe '#visible?' do - context 'when layout == ops and active_tab == diagnostics_workers in :diagnostics_tree' do - it 'will not be skipped' do - view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'record' => nil, 'layout' => 'ops'}, {}) - button.instance_variable_set(:@sb, :active_tab => 'diagnostics_workers') + context 'when x_active_tree == diagnostics_tree and active_tab != diagnostics_workers' do + it 'will be skipped' do + button = described_class.new(view_context, {}, {'record' => record}, {}) + button.instance_variable_set(:@sb, :active_tab => 'does not matter') allow(view_context).to receive(:x_active_tree).and_return(:diagnostics_tree) - expect(button.visible?).to be_truthy - end - end - - context 'when layout != ops and' do - %w(miq_server miq_region).each_with_index do |record, i| - context "record is #{record}" do - it 'will be skipped' do - view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'record' => @records[i], 'layout' => 'not ops'}, {}) - button.instance_variable_set(:@sb, :active_tab => 'does not matter') - allow(view_context).to receive(:x_active_tree).and_return(:diagnostics_tree) - expect(button.visible?).to be_falsey - end - end + expect(button.visible?).to be_falsey end end - - %w(download_logs evm_logs audit_logs).each do |lastaction| - context "when action #{lastaction} being taken as last" do - it 'will be skipped for this record' do - view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'record' => nil, 'lastaction' => lastaction}, {}) - expect(button.visible?).to be_falsey - end + context 'when x_active_tree != diagnostics_tree and active_tab == diagnostics_workers' do + it 'will be skipped' do + button = described_class.new(view_context, {}, {'record' => record}, {}) + button.instance_variable_set(:@sb, :active_tab => 'diagnostics_workers') + allow(view_context).to receive(:x_active_tree).and_return(:does_not_matter) + expect(button.visible?).to be_falsey end end - - context 'when other action being taken as last' do - it 'will not be skipped for this record' do - view_context = setup_view_context_with_sandbox({}) - button = described_class.new(view_context, {}, {'record' => nil, 'lastaction' => 'worker_logs'}, {}) + context 'when x_active_tree == diagnostics_tree and active_tab == diagnostics_workers' do + it 'will not be skipped' do + button = described_class.new(view_context, {}, {'record' => record}, {}) + button.instance_variable_set(:@sb, :active_tab => 'diagnostics_workers') + allow(view_context).to receive(:x_active_tree).and_return(:diagnostics_tree) expect(button.visible?).to be_truthy end end