From 6925a48a8780385da5dfe0cc148b001bd306098b Mon Sep 17 00:00:00 2001 From: Harpreet Kataria Date: Wed, 20 May 2020 17:07:24 -0400 Subject: [PATCH 1/4] Migration for Startpage url updates for CB Reports UI PR https://github.com/ManageIQ/manageiq-ui-classic/pull/7058 --- ...200_update_chargeback_reports_startpage.rb | 25 ++++++++++ ...pdate_chargeback_reports_startpage_spec.rb | 47 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 db/migrate/20200520210200_update_chargeback_reports_startpage.rb create mode 100644 spec/migrations/20200520210200_update_chargeback_reports_startpage_spec.rb diff --git a/db/migrate/20200520210200_update_chargeback_reports_startpage.rb b/db/migrate/20200520210200_update_chargeback_reports_startpage.rb new file mode 100644 index 000000000..ef9ce8e54 --- /dev/null +++ b/db/migrate/20200520210200_update_chargeback_reports_startpage.rb @@ -0,0 +1,25 @@ +class UpdateChargebackReportsStartpage < ActiveRecord::Migration[5.2] + class User < ActiveRecord::Base + serialize :settings, Hash + end + + def up + say_with_time 'Updating starting page for users who had chargeback reports set' do + User.select(:id, :settings).each do |user| + if user.settings&.dig(:display, :startpage) == 'chargeback_reports/explorer' + user.update!(:settings => user.settings.deep_merge(:display => {:startpage => 'chargeback_report/show_list'})) + end + end + end + end + + def down + say_with_time 'Updating starting page for users who had non-explorer chargeback reports pages set' do + User.select(:id, :settings).each do |user| + if user.settings&.dig(:display, :startpage) == 'chargeback_report/show_list' + user.update!(:settings => user.settings.deep_merge(:display => {:startpage => 'chargeback_reports/explorer'})) + end + end + end + end +end diff --git a/spec/migrations/20200520210200_update_chargeback_reports_startpage_spec.rb b/spec/migrations/20200520210200_update_chargeback_reports_startpage_spec.rb new file mode 100644 index 000000000..4621fa4dc --- /dev/null +++ b/spec/migrations/20200520210200_update_chargeback_reports_startpage_spec.rb @@ -0,0 +1,47 @@ +require_migration + +describe UpdateChargebackReportsStartpage do + let(:user_stub) { migration_stub :User } + + migration_context :up do + describe 'starting page replace' do + it 'replaces user starting page if chargeback_reports/explorer' do + user = user_stub.create!(:settings => {:display => {:startpage => 'chargeback_reports/explorer'}}) + + migrate + user.reload + + expect(user.settings[:display][:startpage]).to eq('chargeback_report/show_list') + end + end + + it 'does not affect users without settings' do + user = user_stub.create! + + migrate + + expect(user_stub.find(user.id)).to eq(user) + end + end + + migration_context :down do + describe 'starting page replace' do + it "replaces user starting page to chargeback_reports/explorer if chargeback_report/show_list" do + user = user_stub.create!(:settings => {:display => {:startpage => 'chargeback_report/show_list'}}) + + migrate + user.reload + + expect(user.settings[:display][:startpage]).to eq('chargeback_reports/explorer') + end + + it 'does not affect users without settings' do + user = user_stub.create! + + migrate + + expect(user_stub.find(user.id)).to eq(user) + end + end + end +end From e8f034a558125d29a75236d6bee53239dddd0cd3 Mon Sep 17 00:00:00 2001 From: Harpreet Kataria Date: Wed, 20 May 2020 18:25:59 -0400 Subject: [PATCH 2/4] Migration for CB Reports features updates UI PR https://github.com/ManageIQ/manageiq-ui-classic/pull/7058 --- ...200_update_chargeback_reports_startpage.rb | 5 +- ...1047_adjust_chargeback_reports_features.rb | 52 +++++++++++++++ ...adjust_chargeback_reports_features_spec.rb | 64 +++++++++++++++++++ 3 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20200520211047_adjust_chargeback_reports_features.rb create mode 100644 spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb diff --git a/db/migrate/20200520210200_update_chargeback_reports_startpage.rb b/db/migrate/20200520210200_update_chargeback_reports_startpage.rb index ef9ce8e54..03eaa3556 100644 --- a/db/migrate/20200520210200_update_chargeback_reports_startpage.rb +++ b/db/migrate/20200520210200_update_chargeback_reports_startpage.rb @@ -1,11 +1,12 @@ class UpdateChargebackReportsStartpage < ActiveRecord::Migration[5.2] class User < ActiveRecord::Base serialize :settings, Hash + include ActiveRecord::IdRegions end def up say_with_time 'Updating starting page for users who had chargeback reports set' do - User.select(:id, :settings).each do |user| + User.in_my_region.select(:id, :settings).each do |user| if user.settings&.dig(:display, :startpage) == 'chargeback_reports/explorer' user.update!(:settings => user.settings.deep_merge(:display => {:startpage => 'chargeback_report/show_list'})) end @@ -15,7 +16,7 @@ def up def down say_with_time 'Updating starting page for users who had non-explorer chargeback reports pages set' do - User.select(:id, :settings).each do |user| + User.in_my_region.select(:id, :settings).each do |user| if user.settings&.dig(:display, :startpage) == 'chargeback_report/show_list' user.update!(:settings => user.settings.deep_merge(:display => {:startpage => 'chargeback_reports/explorer'})) end diff --git a/db/migrate/20200520211047_adjust_chargeback_reports_features.rb b/db/migrate/20200520211047_adjust_chargeback_reports_features.rb new file mode 100644 index 000000000..f24c33d29 --- /dev/null +++ b/db/migrate/20200520211047_adjust_chargeback_reports_features.rb @@ -0,0 +1,52 @@ +class AdjustChargebackReportsFeatures < ActiveRecord::Migration[5.2] + class MiqProductFeature < ActiveRecord::Base; end + class MiqRolesFeature < ActiveRecord::Base; end + + FEATURE_MAPPING = { + 'chargeback_download_csv' => 'chargeback_reports_download_csv', + 'chargeback_download_pdf' => 'chargeback_reports_download_pdf', + 'chargeback_download_text' => 'chargeback_reports_download_text', + 'chargeback_report_only' => 'chargeback_reports_report_only' + }.freeze + + def up + return if MiqProductFeature.none? + + say_with_time 'Adjusting chargeback reports features for the non-explorer views' do + new_feature = MiqProductFeature.find_or_create_by!(:identifier => 'chargeback_reports_view') + MiqRolesFeature.where(:miq_product_feature_id => old_feature_ids).each do |feature| + MiqRolesFeature.create!(:miq_product_feature_id => new_feature.id, :miq_user_role_id => feature.miq_user_role_id) + end + + # Direct renaming of features + FEATURE_MAPPING.each do |from, to| + MiqProductFeature.find_by(:identifier => from)&.update!(:identifier => to) + end + end + end + + def down + return if MiqProductFeature.none? + + say_with_time 'Adjusting chargeback reports features to explorer views' do + admin_feature = MiqProductFeature.find_or_create_by!(:identifier => 'chargeback_reports') + + return if MiqRolesFeature.none? + MiqRolesFeature.where(:miq_product_feature_id => admin_feature.id).each do |feature| + old_feature_ids.each do |id| + MiqRolesFeature.create!(:miq_product_feature_id => id, :miq_user_role_id => feature.miq_user_role_id) + end + end + + FEATURE_MAPPING.each do |to, from| + MiqProductFeature.find_by(:identifier => from)&.update!(:identifier => to) + end + end + end + + private + + def old_feature_ids + FEATURE_MAPPING.keys.map { |identifier| MiqProductFeature.find_or_create_by!(:identifier => identifier) } + end +end diff --git a/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb b/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb new file mode 100644 index 000000000..4d1eefe75 --- /dev/null +++ b/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb @@ -0,0 +1,64 @@ +require_migration + +describe AdjustChargebackReportsFeatures do + let(:user_role_id) { anonymous_class_with_id_regions.id_in_region(1, anonymous_class_with_id_regions.my_region_number) } + let(:feature_stub) { migration_stub :MiqProductFeature } + let(:roles_feature_stub) { migration_stub :MiqRolesFeature } + + migration_context :up do + describe 'product features update' do + AdjustChargebackReportsFeatures::FEATURE_MAPPING.keys.each do |identifier| + context "feature #{identifier}" do + it 'also sets the chargeback_reports_view feature' do + feature = feature_stub.create!(:identifier => identifier) + roles_feature_stub.create!(:miq_product_feature_id => feature.id, :miq_user_role_id => user_role_id) + + migrate + + new_roles_feature = roles_feature_stub.where(:miq_user_role_id => user_role_id).where.not(:miq_product_feature_id => feature.id).first + new_feature = feature_stub.find(new_roles_feature.miq_product_feature_id) + expect(new_feature.identifier).to eq('chargeback_reports_view') + end + end + end + + it 'renames the features' do + view_feature1 = feature_stub.create!(:identifier => 'chargeback_download_csv') + view_feature2 = feature_stub.create!(:identifier => 'chargeback_download_pdf') + view_feature3 = feature_stub.create!(:identifier => 'chargeback_download_text') + view_feature4 = feature_stub.create!(:identifier => 'chargeback_report_only') + + migrate + + expect(view_feature1.reload.identifier).to eq('chargeback_reports_download_csv') + expect(view_feature2.reload.identifier).to eq('chargeback_reports_download_pdf') + expect(view_feature3.reload.identifier).to eq('chargeback_reports_download_text') + expect(view_feature4.reload.identifier).to eq('chargeback_reports_report_only') + end + end + end + + migration_context :down do + let!(:view_feature1) { feature_stub.create!(:identifier => 'chargeback_reports_download_csv') } + let!(:view_feature2) { feature_stub.create!(:identifier => 'chargeback_reports_download_pdf') } + let!(:view_feature3) { feature_stub.create!(:identifier => 'chargeback_reports_download_text') } + let!(:view_feature4) { feature_stub.create!(:identifier => 'chargeback_reports_report_only') } + + describe 'product features update' do + it 'sets the chargeback_reports_view feature' do + feature = feature_stub.create!(:identifier => 'chargeback_reports') + roles_feature_stub.create!(:miq_product_feature_id => feature.id, :miq_user_role_id => user_role_id) + + migrate + + assigned = roles_feature_stub.where(:miq_user_role_id => user_role_id) + expect(assigned.count).to eq(5) + + expect(view_feature1.reload.identifier).to eq('chargeback_download_csv') + expect(view_feature2.reload.identifier).to eq('chargeback_download_pdf') + expect(view_feature3.reload.identifier).to eq('chargeback_download_text') + expect(view_feature4.reload.identifier).to eq('chargeback_report_only') + end + end + end +end From b7108ad7b1b96e7fbf66a1fa022f8357ab529349 Mon Sep 17 00:00:00 2001 From: Harpreet Kataria Date: Fri, 22 May 2020 18:07:53 -0400 Subject: [PATCH 3/4] Fixed to Add only 1 Role Feature for `chargeback_report_view` Previous commit was adding multiple records of same `chargeback_Report_view`. Cleaned up test a bit to verify the total counts. Added a spec test to verify that down migration is skipped when there are no role features set for user. --- ...1047_adjust_chargeback_reports_features.rb | 6 ++-- ...adjust_chargeback_reports_features_spec.rb | 31 ++++++++++++------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/db/migrate/20200520211047_adjust_chargeback_reports_features.rb b/db/migrate/20200520211047_adjust_chargeback_reports_features.rb index f24c33d29..bcde355b4 100644 --- a/db/migrate/20200520211047_adjust_chargeback_reports_features.rb +++ b/db/migrate/20200520211047_adjust_chargeback_reports_features.rb @@ -14,8 +14,9 @@ def up say_with_time 'Adjusting chargeback reports features for the non-explorer views' do new_feature = MiqProductFeature.find_or_create_by!(:identifier => 'chargeback_reports_view') - MiqRolesFeature.where(:miq_product_feature_id => old_feature_ids).each do |feature| - MiqRolesFeature.create!(:miq_product_feature_id => new_feature.id, :miq_user_role_id => feature.miq_user_role_id) + user_role_ids_with_old_features = MiqRolesFeature.where(:miq_product_feature_id => old_feature_ids).pluck(:miq_user_role_id).uniq + user_role_ids_with_old_features.each do |user_role_id| + MiqRolesFeature.create!(:miq_product_feature_id => new_feature.id, :miq_user_role_id => user_role_id) end # Direct renaming of features @@ -32,6 +33,7 @@ def down admin_feature = MiqProductFeature.find_or_create_by!(:identifier => 'chargeback_reports') return if MiqRolesFeature.none? + MiqRolesFeature.where(:miq_product_feature_id => admin_feature.id).each do |feature| old_feature_ids.each do |id| MiqRolesFeature.create!(:miq_product_feature_id => id, :miq_user_role_id => feature.miq_user_role_id) diff --git a/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb b/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb index 4d1eefe75..fec98c046 100644 --- a/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb +++ b/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb @@ -7,19 +7,21 @@ migration_context :up do describe 'product features update' do - AdjustChargebackReportsFeatures::FEATURE_MAPPING.keys.each do |identifier| - context "feature #{identifier}" do - it 'also sets the chargeback_reports_view feature' do - feature = feature_stub.create!(:identifier => identifier) - roles_feature_stub.create!(:miq_product_feature_id => feature.id, :miq_user_role_id => user_role_id) + it 'also sets the chargeback_reports_view feature' do + AdjustChargebackReportsFeatures::FEATURE_MAPPING.keys.each do |identifier| + feature = feature_stub.create!(:identifier => identifier) + roles_feature_stub.create!(:miq_product_feature_id => feature.id, :miq_user_role_id => user_role_id) + end - migrate + migrate - new_roles_feature = roles_feature_stub.where(:miq_user_role_id => user_role_id).where.not(:miq_product_feature_id => feature.id).first - new_feature = feature_stub.find(new_roles_feature.miq_product_feature_id) - expect(new_feature.identifier).to eq('chargeback_reports_view') - end - end + assigned = roles_feature_stub.where(:miq_user_role_id => user_role_id) + expect(assigned.count).to eq(5) + + feature = feature_stub.find_by(:identifier => 'chargeback_reports_view') + new_roles_feature = roles_feature_stub.where(:miq_user_role_id => user_role_id).where(:miq_product_feature_id => feature.id).first + new_feature = feature_stub.find(new_roles_feature.miq_product_feature_id) + expect(new_feature.identifier).to eq('chargeback_reports_view') end it 'renames the features' do @@ -59,6 +61,13 @@ expect(view_feature3.reload.identifier).to eq('chargeback_download_text') expect(view_feature4.reload.identifier).to eq('chargeback_report_only') end + + it 'skips feature renaming when no role features are set' do + migrate + + assigned = roles_feature_stub.where(:miq_user_role_id => user_role_id) + expect(assigned.count).to eq(0) + end end end end From 19856e92a0f5a42ed455ab7ceca5bf1cc5755e3a Mon Sep 17 00:00:00 2001 From: Harpreet Kataria Date: Mon, 1 Jun 2020 09:01:22 -0400 Subject: [PATCH 4/4] Removed `MiqRolesFeature.none?` check Adjusted test to verify that product features are renamed even when there are no MiqRoleFeatures. --- .../20200520211047_adjust_chargeback_reports_features.rb | 2 -- ...200520211047_adjust_chargeback_reports_features_spec.rb | 7 ++++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/db/migrate/20200520211047_adjust_chargeback_reports_features.rb b/db/migrate/20200520211047_adjust_chargeback_reports_features.rb index bcde355b4..5bf04810a 100644 --- a/db/migrate/20200520211047_adjust_chargeback_reports_features.rb +++ b/db/migrate/20200520211047_adjust_chargeback_reports_features.rb @@ -32,8 +32,6 @@ def down say_with_time 'Adjusting chargeback reports features to explorer views' do admin_feature = MiqProductFeature.find_or_create_by!(:identifier => 'chargeback_reports') - return if MiqRolesFeature.none? - MiqRolesFeature.where(:miq_product_feature_id => admin_feature.id).each do |feature| old_feature_ids.each do |id| MiqRolesFeature.create!(:miq_product_feature_id => id, :miq_user_role_id => feature.miq_user_role_id) diff --git a/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb b/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb index fec98c046..0a17e9a40 100644 --- a/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb +++ b/spec/migrations/20200520211047_adjust_chargeback_reports_features_spec.rb @@ -62,11 +62,16 @@ expect(view_feature4.reload.identifier).to eq('chargeback_report_only') end - it 'skips feature renaming when no role features are set' do + it 'product feature is renamed when no role features are set' do migrate assigned = roles_feature_stub.where(:miq_user_role_id => user_role_id) expect(assigned.count).to eq(0) + + expect(view_feature1.reload.identifier).to eq('chargeback_download_csv') + expect(view_feature2.reload.identifier).to eq('chargeback_download_pdf') + expect(view_feature3.reload.identifier).to eq('chargeback_download_text') + expect(view_feature4.reload.identifier).to eq('chargeback_report_only') end end end