Browse Source

Fixes #3402 - DataPrivacy Task fails in some situations.

Rolf Schmidt 4 years ago
parent
commit
050315bac9
2 changed files with 26 additions and 12 deletions
  1. 19 8
      app/models/user.rb
  2. 7 4
      spec/models/user_spec.rb

+ 19 - 8
app/models/user.rb

@@ -1177,22 +1177,33 @@ raise 'Minimum one user need to have admin permissions'
       next if class_name.blank?
       next if references.blank?
 
-      ref_class = class_name.constantize
+      ref_class          = class_name.constantize
+      ref_update_columns = []
       references.each do |column, reference_found|
         next if !reference_found
 
         if user_columns.include?(column)
-          ref_class.where(column => id).find_in_batches(batch_size: 1000) do |batch_list|
-            batch_list.each do |record|
-              record.update!(column => 1)
-            rescue => e
-              Rails.logger.error e
-            end
-          end
+          ref_update_columns << column
         elsif ref_class.exists?(column => id)
           raise "Failed deleting references! Check logic for #{class_name}->#{column}."
         end
       end
+
+      next if ref_update_columns.blank?
+
+      where_sql = ref_update_columns.map { |column| "#{column} = #{id}" }.join(' OR ')
+      ref_class.where(where_sql).find_in_batches(batch_size: 1000) do |batch_list|
+        batch_list.each do |record|
+          ref_update_columns.each do |column|
+            next if record[column] != id
+
+            record[column] = 1
+          end
+          record.save!
+        rescue => e
+          Rails.logger.error e
+        end
+      end
     end
 
     true

+ 7 - 4
spec/models/user_spec.rb

@@ -824,7 +824,7 @@ RSpec.describe User, type: :model do
       refs_known = { 'Group'                              => { 'created_by_id' => 1, 'updated_by_id' => 0 },
                      'Token'                              => { 'user_id' => 1 },
                      'Ticket::Article'                    =>
-                                                             { 'created_by_id' => 0, 'updated_by_id' => 0, 'origin_by_id' => 1 },
+                                                             { 'created_by_id' => 1, 'updated_by_id' => 1, 'origin_by_id' => 1 },
                      'Ticket::StateType'                  => { 'created_by_id' => 0, 'updated_by_id' => 0 },
                      'Ticket::Article::Sender'            => { 'created_by_id' => 0, 'updated_by_id' => 0 },
                      'Ticket::Article::Type'              => { 'created_by_id' => 0, 'updated_by_id' => 0 },
@@ -871,7 +871,7 @@ RSpec.describe User, type: :model do
                      'Macro'                              => { 'created_by_id' => 0, 'updated_by_id' => 0 },
                      'Channel'                            => { 'created_by_id' => 0, 'updated_by_id' => 0 },
                      'Role'                               => { 'created_by_id' => 0, 'updated_by_id' => 0 },
-                     'History'                            => { 'created_by_id' => 1 },
+                     'History'                            => { 'created_by_id' => 2 },
                      'Webhook'                            => { 'created_by_id' => 0, 'updated_by_id' => 0 },
                      'Overview'                           => { 'created_by_id' => 1, 'updated_by_id' => 0 },
                      'ActivityStream'                     => { 'created_by_id' => 0 },
@@ -906,7 +906,7 @@ RSpec.describe User, type: :model do
       group                 = create(:group, created_by_id: user.id)
       job                   = create(:job, updated_by_id: user.id)
       ticket                = create(:ticket, group: group_subject, owner: user)
-      ticket_article        = create(:ticket_article, ticket: ticket, origin_by_id: user.id)
+      ticket_article        = create(:ticket_article, ticket: ticket, created_by_id: user.id, updated_by_id: user.id, origin_by_id: user.id)
       customer_ticket1      = create(:ticket, group: group_subject, customer: user)
       customer_ticket2      = create(:ticket, group: group_subject, customer: user)
       customer_ticket3      = create(:ticket, group: group_subject, customer: user)
@@ -936,7 +936,10 @@ RSpec.describe User, type: :model do
       expect { group.reload }.to change(group, :created_by_id).to(1)
       expect { job.reload }.to change(job, :updated_by_id).to(1)
       expect { ticket.reload }.to change(ticket, :owner_id).to(1)
-      expect { ticket_article.reload }.to change(ticket_article, :origin_by_id).to(1)
+      expect { ticket_article.reload }
+        .to change(ticket_article, :origin_by_id).to(1)
+        .and change(ticket_article, :updated_by_id).to(1)
+        .and change(ticket_article, :created_by_id).to(1)
       expect { knowledge_base_answer.reload }
         .to change(knowledge_base_answer, :archived_by_id).to(1)
         .and change(knowledge_base_answer, :published_by_id).to(1)