Merge pull request #1092 from kubamahnert/TMA-732

TMA-732: fix edge cases for user input sanitized MUFs

Reviewed-by: https://github.com/kubamahnert

Author: gdgate

lib/gooddata/lcm/actions/synchronize_user_filters.rb

@@ -139,6 +139,7 @@ def call(params)
             CSV.foreach(File.open(data_source.realize(params), 'r:UTF-8'), headers: csv_with_headers, return_headers: false, encoding: 'utf-8') do |row|
               filters << row.to_hash
             end
+            fail 'The filter set can not be empty when using sync_multiple_projects_based_on_custom_id mode' if filters.empty?
             filters.group_by { |u| u[multiple_projects_column] }.flat_map do |client_id, new_filters|
               fail "Client id cannot be empty" if client_id.blank?
               project = domain.clients(client_id, data_product).project

lib/gooddata/models/user_filters/user_filter_builder.rb

@@ -539,13 +539,16 @@ def self.normalize_filters(filters)
     end
 
     # Removes MUFs from to_delete unless in user is in users_brick_input
+    # if this does not happen, users that are about to be deleted by users_brick
+    # would have all their filters removed now, which is not desirable
     def self.sanitize_filters_to_delete(to_delete, users_brick_input, project_users)
       return to_delete unless users_brick_input && users_brick_input.any?
       user_profiles = users_brick_input.map do |user|
         result = project_users.find { |u| u.login == user['login'] }
         next unless result
         result.profile_url
       end.compact
+      return to_delete unless user_profiles.any?
       to_delete.reject do |_, value|
         user_profiles.none? { |profile| profile == value.first.json[:related] }
       end

spec/unit/actions/synchronize_user_filters_spec.rb

@@ -3,6 +3,20 @@
 
 require_relative 'shared_examples_for_user_actions'
 
+shared_context 'using mode with custom_id' do
+  let(:CSV) { double('CSV') }
+
+  before do
+    allow(project).to receive(:metadata).and_return(
+      'GOODOT_CUSTOM_PROJECT_ID' => 'project-123'
+    )
+    allow(project).to receive(:uri).and_return('project-uri')
+    allow(project).to receive(:add_data_permissions)
+    allow(domain).to receive(:clients).and_return([])
+    allow(data_source).to receive(:realize).and_return('filepath')
+  end
+end
+
 describe GoodData::LCM2::SynchronizeUserFilters do
   let(:client) { double('client') }
   let(:user) { double('user') }
@@ -76,6 +90,7 @@
   end
 
   context 'when using sync_one_project_based_on_custom_id mode with multiple_projects_column' do
+    include_context 'using mode with custom_id'
     let(:params) do
       params = {
         GDC_GD_CLIENT: client,
@@ -88,17 +103,6 @@
       }
       GoodData::LCM2.convert_to_smart_hash(params)
     end
-    let(:CSV) { double('CSV') }
-
-    before do
-      allow(project).to receive(:metadata).and_return(
-        'GOODOT_CUSTOM_PROJECT_ID' => 'project-123'
-      )
-      allow(project).to receive(:uri).and_return('project-uri')
-      allow(project).to receive(:add_data_permissions)
-      allow(domain).to receive(:clients).and_return([])
-      allow(data_source).to receive(:realize).and_return('filepath')
-    end
 
     context 'when params do not match client data in domain' do
       before do
@@ -143,4 +147,26 @@
       end
     end
   end
+
+  context 'when using sync_multiple_projects_based_on_custom_id mode' do
+    include_context 'using mode with custom_id'
+    let(:params) do
+      params = {
+        input_source: 'foo',
+        domain: 'bar',
+        multiple_projects_column: 'id_column',
+        sync_mode: 'sync_multiple_projects_based_on_custom_id',
+        gdc_logger: logger,
+        GDC_GD_CLIENT: client,
+        filters_config: { labels: [] }
+      }
+      GoodData::LCM2.convert_to_smart_hash(params)
+    end
+
+    it 'fails if the MUF set is empty' do
+      expect(File).to receive(:open)
+      expect(CSV).to receive(:foreach)
+      expect { subject.class.call(params) }.to raise_error(/The filter set can not be empty/)
+    end
+  end
 end