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.

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)

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