Maintenance: Add uniqueness index inside taskbar table.

Co-authored-by: Mantas Masalskis <[email protected]>
Co-authored-by: Dominik Klein <[email protected]>

app/models/taskbar.rb

@@ -6,7 +6,6 @@ class Taskbar < ApplicationModel
   include Taskbar::Assets
   include Taskbar::TriggersSubscriptions
   include Taskbar::List
-  include Taskbar::Validator
 
   TASKBAR_APPS = %w[desktop mobile].freeze
   TASKBAR_STATIC_ENTITIES = %w[
@@ -20,9 +19,12 @@ class Taskbar < ApplicationModel
   belongs_to :user
 
   validates :app, inclusion: { in: TASKBAR_APPS }
+  validates :key, uniqueness: { scope: %i[user_id app] }
 
-  before_create   :update_last_contact, :set_user, :update_preferences_infos
-  before_update   :update_last_contact, :set_user, :update_preferences_infos
+  before_validation :set_user
+
+  before_create   :update_last_contact, :update_preferences_infos
+  before_update   :update_last_contact, :update_preferences_infos
 
   after_update    :notify_clients
   after_destroy   :update_preferences_infos, :notify_clients

db/migrate/20120101000001_create_base.rb

@@ -334,6 +334,7 @@ def up
       t.string :app,                                null: false, default: 'desktop'
       t.timestamps limit: 3, null: false
     end
+    add_index :taskbars, %i[user_id key app], unique: true
     add_index :taskbars, [:user_id]
     add_index :taskbars, [:key]
     add_foreign_key :taskbars, :users

db/migrate/20241106073757_taskbar_add_uniqueness_index.rb

@@ -0,0 +1,28 @@
+# Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+class TaskbarAddUniquenessIndex < ActiveRecord::Migration[7.1]
+  def change
+    return if !Setting.exists?(name: 'system_init_done')
+
+    duplicates = Taskbar.select(:user_id, :app, :key).group(:user_id, :app, :key).having('COUNT(*) > 1').reorder(nil)
+
+    if duplicates.exists?
+      Taskbar
+        .where(
+          user_id: duplicates.pluck(:user_id),
+          app:     duplicates.pluck(:app),
+          key:     duplicates.pluck(:key)
+        )
+        .group_by { |taskbar| [taskbar.user_id, taskbar.app, taskbar.key] }
+        .each_value do |records|
+          newest_record = records.max_by(&:last_contact)
+
+          records.reject { |record| record == newest_record }.each(&:destroy)
+        end
+    end
+
+    add_index :taskbars, %i[user_id key app], unique: true
+
+    Taskbar.reset_column_information
+  end
+end

spec/db/migrate/issue5390_taskbar_remove_duplicated_key_by_user_spec.rb

@@ -2,8 +2,10 @@
 
 require 'rails_helper'
 
-RSpec.describe Issue5390TaskbarRemoveDuplicatedKeyByUser, type: :db_migration do
-  context 'when some user have duplicated taskbar key entries', db_strategy: :reset do
+RSpec.describe Issue5390TaskbarRemoveDuplicatedKeyByUser, db_strategy: :reset, type: :db_migration do
+  before { without_index(:taskbars, column: %i[user_id key app]) }
+
+  context 'when some user have duplicated taskbar key entries' do
     let(:user)        { create(:user) }
     let(:second_user) { create(:user) }
     let(:other_user)  { create(:user) }

spec/models/taskbar_spec.rb

@@ -12,33 +12,12 @@
     subject(:taskbar) { create(:taskbar) }
 
     it { is_expected.to validate_inclusion_of(:app).in_array(%w[desktop mobile]) }
-  end
-
-  context 'create and update' do
-    let(:user) { create(:user) }
-
-    context 'when creating' do
-      it 'validates uniqueness of key + app' do
-        create(:taskbar, user: user, key: 'Ticket-1234', app: 'desktop')
-
-        expect { create(:taskbar, user: user, key: 'Ticket-1234', app: 'desktop') }.to raise_error(ActiveRecord::RecordInvalid)
-      end
-    end
-
-    context 'when updating' do
-      it 'validates uniqueness of key + app' do
-        create(:taskbar, user: user, key: 'Ticket-1234', app: 'desktop')
-        taskbar = create(:taskbar, user: user, key: 'Ticket-1234', app: 'mobile')
-
-        expect { taskbar.update!(app: 'desktop') }.to raise_error(ActiveRecord::RecordInvalid)
-      end
-
-      it 'does not validate when no relevant field changes' do
-        taskbar = create(:taskbar, user: user, key: 'Ticket-1234', app: 'desktop')
 
-        allow(taskbar).to receive(:taskbar_exist?).and_call_original
-        taskbar.update!(state: { example: '1234' })
-        expect(taskbar).not_to have_received(:taskbar_exist?)
+    it do
+      if ActiveRecord::Base.connection_db_config.configuration_hash[:adapter] == 'mysql2'
+        expect(taskbar).to validate_uniqueness_of(:key).scoped_to(%w[user_id app]).with_message(%r{}).case_insensitive
+      else
+        expect(taskbar).to validate_uniqueness_of(:key).scoped_to(%w[user_id app]).with_message(%r{})
       end
     end
   end