Browse Source

Feature: Support multiple taskbar entries for one object and user for different apps

Mantas Masalskis 2 years ago
parent
commit
7e870152d6

+ 0 - 1
app/assets/javascripts/app/lib/app_post/task_manager.coffee

@@ -239,7 +239,6 @@ class _taskManagerSingleton extends App.Controller
         key:       params.key
         params:    params.params
         callback:  params.controller
-        client_id: 123
         prio:      @newPrio()
         notify:    false
         active:    params.show

+ 1 - 1
app/assets/javascripts/app/models/taskbar.coffee

@@ -1,5 +1,5 @@
 class App.Taskbar extends App.Model
-  @configure 'Taskbar', 'key', 'client_id', 'callback', 'state', 'params', 'prio', 'notify', 'active', 'attachments', 'updated_at'
+  @configure 'Taskbar', 'key', 'callback', 'state', 'params', 'prio', 'notify', 'active', 'attachments', 'updated_at'
 #  @extend Spine.Model.Local
   @extend Spine.Model.Ajax
   @url: @apiPath + '/taskbar'

+ 47 - 41
app/models/taskbar.rb

@@ -11,6 +11,8 @@ class Taskbar < ApplicationModel
 
   belongs_to :user
 
+  validates :app, inclusion: { in: %w[desktop mobile] }
+
   before_create   :update_last_contact, :set_user, :update_preferences_infos
   before_update   :update_last_contact, :set_user, :update_preferences_infos
 
@@ -25,6 +27,12 @@ class Taskbar < ApplicationModel
 
   attr_accessor :local_update
 
+  scope :related_taskbars, lambda { |taskbar|
+    where(key: taskbar.key)
+      .where.not(id: taskbar.id)
+      .order(:created_at, :id)
+  }
+
   def state_changed?
     return false if state.blank?
 
@@ -57,6 +65,16 @@ class Taskbar < ApplicationModel
     add_attachments_to_attributes(super)
   end
 
+  def preferences_task_info
+    output = { user_id:, last_contact:, changed: state_changed?, apps: [app] }
+    output[:id] = id if persisted?
+    output
+  end
+
+  def related_taskbars
+    self.class.related_taskbars(self)
+  end
+
   private
 
   def update_last_contact
@@ -84,57 +102,45 @@ class Taskbar < ApplicationModel
   end
 
   def update_preferences_infos
-    return true if key == 'Search'
-    return true if local_update
+    return if key == 'Search'
+    return if local_update
 
-    # find other same open tasks
-    if !preferences
-      self.preferences = {}
-    end
-    preferences[:tasks] = []
-    Taskbar.where(key: key).order(:created_at, :id).each do |taskbar|
-      if taskbar.id == id
-        local_changed = state_changed?
-        local_last_contact = last_contact
-      else
-        local_changed = taskbar.state_changed?
-        local_last_contact = taskbar.last_contact
-      end
-      task = {
-        id:           taskbar.id,
-        user_id:      taskbar.user_id,
-        last_contact: local_last_contact,
-        changed:      local_changed,
-      }
-      preferences[:tasks].push task
-    end
-    if !id
-      changed = state_changed?
-      task = {
-        user_id:      user_id,
-        last_contact: last_contact,
-        changed:      changed,
-      }
-      preferences[:tasks].push task
+    preferences = self.preferences || {}
+    preferences[:tasks] = collect_related_tasks
+
+    update_related_taskbars(preferences)
+
+    # remember preferences for current taskbar
+    self.preferences = preferences if !destroyed?
+  end
+
+  def collect_related_tasks
+    related_taskbars
+      .map(&:preferences_task_info)
+      .push(preferences_task_info)
+      .each_with_object({}) { |elem, memo| reduce_related_tasks(elem, memo) }
+      .values
+      .sort_by { |elem| elem[:id] || Float::MAX } # sort by IDs to pass old tests
+  end
+
+  def reduce_related_tasks(elem, memo)
+    if memo[elem[:user_id]]
+      memo[elem[:user_id]][:apps].concat elem[:apps]
+      memo[elem[:user_id]][:changed] = true if elem[:changed]
+      return
     end
 
-    # update other taskbars
-    Taskbar.where(key: key).order(:created_at, :id).each do |taskbar|
-      next if taskbar.id == id
+    memo[elem[:user_id]] = elem
+  end
 
+  def update_related_taskbars(preferences)
+    related_taskbars.each do |taskbar|
       taskbar.with_lock do
         taskbar.preferences = preferences
         taskbar.local_update = true
         taskbar.save!
       end
     end
-
-    return true if destroyed?
-
-    # remember preferences for current taskbar
-    self.preferences = preferences
-
-    true
   end
 
   def notify_clients

+ 1 - 2
db/migrate/20120101000001_create_base.rb

@@ -309,7 +309,6 @@ class CreateBase < ActiveRecord::Migration[4.2]
     create_table :taskbars do |t|
       t.references :user,                           null: false
       t.datetime :last_contact,                     null: false, limit: 3
-      t.string :client_id,                          null: false
       t.string :key,                   limit: 100,  null: false
       t.string :callback,              limit: 100,  null: false
       t.text :state,                   limit: 20.megabytes + 1, null: true
@@ -318,10 +317,10 @@ class CreateBase < ActiveRecord::Migration[4.2]
       t.integer :prio,                              null: false
       t.boolean :notify,                            null: false, default: false
       t.boolean :active,                            null: false, default: false
+      t.string :app,                                null: false, default: 'desktop'
       t.timestamps limit: 3, null: false
     end
     add_index :taskbars, [:user_id]
-    add_index :taskbars, [:client_id]
     add_index :taskbars, [:key]
     add_foreign_key :taskbars, :users
 

+ 15 - 0
db/migrate/20230120163410_taskbars_add_apps_support.rb

@@ -0,0 +1,15 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class TaskbarsAddAppsSupport < ActiveRecord::Migration[6.1]
+  def up
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    change_table :taskbars do |t|
+      t.remove :client_id
+      t.column :app, :string, null: false, default: 'desktop'
+    end
+
+    Taskbar.reset_column_information
+  end
+end

+ 1 - 1
lib/session_helper/collection_base.rb

@@ -10,7 +10,7 @@ module SessionHelper::CollectionBase
     collections[ Locale.to_app_model ]                = Locale.where(active: true)
     collections[ User::OverviewSorting.to_app_model ] = User::OverviewSorting.where(user: user)
 
-    collections[ Taskbar.to_app_model ] = Taskbar.where(user_id: user.id)
+    collections[ Taskbar.to_app_model ] = Taskbar.where(user_id: user.id, app: :desktop)
     collections[ Taskbar.to_app_model ].each do |item|
       assets = item.assets(assets)
     end

+ 1 - 2
spec/db/migrate/20171023000001_fixed_store_upgrade_ror_45_spec.rb

@@ -11,9 +11,8 @@ RSpec.describe FixedStoreUpgradeRor45, type: :db_migration do
   context 'when DB contains `store`d attributes saved as unpermitted ActionController::Parameters' do
     before do
       ActiveRecord::Base.connection.execute(<<~SQL.tap { |sql| sql.delete!('`') if !mysql? }) # rubocop:disable Rails/SquishedSQLHeredocs
-        INSERT INTO taskbars (`user_id`, `client_id`, `key`, `callback`, `state`, `params`, `prio`, `notify`, `active`, `preferences`, `last_contact`, `updated_at`, `created_at`)
+        INSERT INTO taskbars (`user_id`, `key`, `callback`, `state`, `params`, `prio`, `notify`, `active`, `preferences`, `last_contact`, `updated_at`, `created_at`)
         VALUES (#{user.id},
-                '123',
                 'Ticket-123',
                 'TicketZoom',
                 '#{state.to_yaml}',

+ 0 - 1
spec/factories/taskbar.rb

@@ -2,7 +2,6 @@
 
 FactoryBot.define do
   factory :taskbar do
-    client_id                { 123 }
     key                      { 'Ticket-1234' }
     add_attribute(:callback) { 'TicketZoom' }
     params                   { {} }

+ 13 - 0
spec/lib/session_helper_spec.rb

@@ -29,4 +29,17 @@ RSpec.describe SessionHelper do
       end
     end
   end
+
+  describe 'taskbars' do
+    let(:user)      { create(:user) }
+    let(:taskbar_1) { create(:taskbar, user: user) }
+    let(:taskbar_2) { create(:taskbar, user: user, app: 'mobile') }
+
+    before { taskbar_1 && taskbar_2 }
+
+    it 'returns desktop taskbars' do
+      collections = described_class.json_hash(user)[:collections]
+      expect(collections[Taskbar.to_app_model]).to eq([taskbar_1])
+    end
+  end
 end

+ 224 - 33
spec/models/taskbar_spec.rb

@@ -3,9 +3,15 @@
 require 'rails_helper'
 require 'models/taskbar/has_attachments_examples'
 
-RSpec.describe Taskbar do
+RSpec.describe Taskbar, type: :model do
   it_behaves_like 'Taskbar::HasAttachments'
 
+  context 'item' do
+    subject(:taskbar) { create(:taskbar) }
+
+    it { is_expected.to validate_inclusion_of(:app).in_array(%w[desktop mobile]) }
+  end
+
   context 'key = Search' do
 
     context 'multiple taskbars', current_user_id: 1 do
@@ -127,30 +133,28 @@ RSpec.describe Taskbar do
       described_class.destroy_all
       UserInfo.current_user_id = 1
       taskbar1 = described_class.create(
-        client_id: 123,
-        key:       'Ticket-1234',
-        callback:  'TicketZoom',
-        params:    {
+        key:      'Ticket-1234',
+        callback: 'TicketZoom',
+        params:   {
           id: 1234,
         },
-        state:     {},
-        prio:      1,
-        notify:    false,
-        user_id:   1,
+        state:    {},
+        prio:     1,
+        notify:   false,
+        user_id:  1,
       )
 
       UserInfo.current_user_id = 2
       taskbar2 = described_class.create(
-        client_id: 123,
-        key:       'Ticket-1234',
-        callback:  'TicketZoom',
-        params:    {
+        key:      'Ticket-1234',
+        callback: 'TicketZoom',
+        params:   {
           id: 1234,
         },
-        state:     {},
-        prio:      2,
-        notify:    false,
-        user_id:   1,
+        state:    {},
+        prio:     2,
+        notify:   false,
+        user_id:  1,
       )
 
       taskbar1.reload
@@ -168,16 +172,15 @@ RSpec.describe Taskbar do
       expect(taskbar2.preferences[:tasks][1][:changed]).to be(false)
 
       taskbar3 = described_class.create(
-        client_id: 123,
-        key:       'Ticket-4444',
-        callback:  'TicketZoom',
-        params:    {
+        key:      'Ticket-4444',
+        callback: 'TicketZoom',
+        params:   {
           id: 4444,
         },
-        state:     {},
-        prio:      2,
-        notify:    false,
-        user_id:   1,
+        state:    {},
+        prio:     2,
+        notify:   false,
+        user_id:  1,
       )
 
       taskbar1.reload
@@ -203,16 +206,15 @@ RSpec.describe Taskbar do
       UserInfo.current_user_id = agent_id
 
       taskbar4 = described_class.create(
-        client_id: 123,
-        key:       'Ticket-1234',
-        callback:  'TicketZoom',
-        params:    {
+        key:      'Ticket-1234',
+        callback: 'TicketZoom',
+        params:   {
           id: 1234,
         },
-        state:     {},
-        prio:      4,
-        notify:    false,
-        user_id:   1,
+        state:    {},
+        prio:     4,
+        notify:   false,
+        user_id:  1,
       )
 
       taskbar1.reload
@@ -461,4 +463,193 @@ RSpec.describe Taskbar do
     end
   end
 
+  describe '#preferences_task_info' do
+    it 'returns task info for an existing taskbar without changes' do
+      taskbar = create(:taskbar)
+
+      expect(taskbar.preferences_task_info)
+        .to eq({ id: taskbar.id, user_id: 1, last_contact: taskbar.last_contact, changed: false, apps: %w[desktop] })
+    end
+
+    it 'returns task info for an existing taskbar with changes' do
+      taskbar = create(:taskbar, state: { a: 123 })
+
+      expect(taskbar.preferences_task_info)
+        .to eq({ id: taskbar.id, user_id: 1, last_contact: taskbar.last_contact, changed: true, apps: %w[desktop] })
+    end
+
+    it 'returns task info for a new taskbar' do
+      taskbar = build(:taskbar)
+
+      expect(taskbar.preferences_task_info)
+        .to eq({ user_id: 1, last_contact: taskbar.last_contact, changed: false, apps: %w[desktop] })
+    end
+  end
+
+  describe '#update_preferences_infos' do
+    it 'do not process search taskbars' do
+      taskbar = build(:taskbar, key: 'Search')
+
+      allow(taskbar).to receive(:collect_related_tasks)
+      taskbar.save
+      expect(taskbar).not_to have_received(:collect_related_tasks)
+    end
+
+    it 'do not process items with local_update flag' do
+      taskbar = create(:taskbar)
+
+      allow(taskbar).to receive(:collect_related_tasks)
+      taskbar.state = { a: 'b' }
+      taskbar.local_update = true
+      taskbar.save
+      expect(taskbar).not_to have_received(:collect_related_tasks)
+    end
+
+    context 'with other taskbars' do
+      let(:key)           { Random.hex }
+      let(:other_user)    { create(:user) }
+      let(:other_taskbar) { create(:taskbar, key: key, user: other_user) }
+
+      before { other_taskbar }
+
+      it 'sets tasks when creating a taskbar' do
+        taskbar = create(:taskbar, key: key)
+
+        expect(taskbar.preferences[:tasks]).to include(include(user_id: other_user.id), include(user_id: 1))
+      end
+
+      it 'updates related items when creating a taskbar' do
+        create(:taskbar, key: key)
+
+        expect(other_taskbar.reload.preferences[:tasks]).to include(include(user_id: other_user.id), include(user_id: 1))
+      end
+
+      it 'sets tasks when updating a taskbar' do
+        taskbar = create(:taskbar, key: key)
+        taskbar.update_columns preferences: {}
+
+        taskbar.update! state: { a: :b }
+
+        expect(taskbar.preferences[:tasks]).to include(include(user_id: other_user.id), include(user_id: 1))
+      end
+
+      it 'sets tasks when updating a taskbar with same user but different app' do
+        taskbar = create(:taskbar, key: key, user: other_user, app: 'mobile')
+        taskbar.update_columns preferences: {}
+
+        taskbar.update! state: { a: :b }
+
+        expect(taskbar.preferences[:tasks]).to include(include(user_id: other_user.id, apps: include('desktop', 'mobile')))
+      end
+
+      it 'updates related items when updating a taskbar' do
+        taskbar = create(:taskbar, key: key)
+
+        other_taskbar.update_columns preferences: {}
+
+        taskbar.update! state: { a: :b }
+
+        expect(other_taskbar.reload.preferences[:tasks]).to include(include(user_id: other_user.id), include(user_id: 1))
+      end
+
+      it 'updates related items when destroying a taskbar' do
+        taskbar = create(:taskbar, key: key)
+        taskbar.destroy!
+
+        expect(other_taskbar.reload.preferences[:tasks]).to include(include(user_id: other_user.id))
+      end
+    end
+  end
+
+  describe '#collect_related_tasks' do
+    let(:key)       { Random.hex }
+    let(:taskbar_1) { create(:taskbar, key: key, user: create(:user)) }
+    let(:taskbar_2) { create(:taskbar, key: key, user: create(:user)) }
+
+    before { taskbar_2 }
+
+    it 'returns tasks for self and related items' do
+      expect(taskbar_1.send(:collect_related_tasks))
+        .to eq([taskbar_2.preferences_task_info, taskbar_1.preferences_task_info])
+    end
+
+    it 'returns tasks for a new taskbar' do
+      new_taskbar = build(:taskbar, key: key)
+
+      expect(new_taskbar.send(:collect_related_tasks))
+        .to eq([taskbar_2.preferences_task_info, new_taskbar.preferences_task_info])
+    end
+  end
+
+  describe '#reduce_related_tasks' do
+    let(:elem) { { user_id: 123, apps: ['desktop'], changed: false } }
+    let(:memo) { {} }
+
+    it 'adds new task details' do
+      taskbar = create(:taskbar)
+
+      taskbar.send(:reduce_related_tasks, elem, memo)
+
+      expect(memo).to include(elem[:user_id] => include(apps: include('desktop'), changed: false))
+    end
+
+    it 'extends existing task details with additional apps' do
+      taskbar = create(:taskbar)
+
+      another_elem = { user_id: 123, apps: ['mobile'], changed: true }
+
+      taskbar.send(:reduce_related_tasks, elem, memo)
+      taskbar.send(:reduce_related_tasks, another_elem, memo)
+
+      expect(memo).to include(elem[:user_id] => include(apps: include('desktop', 'mobile'), changed: true))
+    end
+  end
+
+  describe '#update_related_taskbars' do
+    let(:key)       { Random.hex }
+    let(:taskbar_1) { create(:taskbar, key: key, user: create(:user)) }
+    let(:taskbar_2) { create(:taskbar, key: key, user: create(:user)) }
+    let(:taskbar_3) { create(:taskbar, user: taskbar_1.user) }
+
+    before { taskbar_1 && taskbar_2 && taskbar_3 }
+
+    it 'updates related taskbars' do
+      taskbar_1.send(:update_related_taskbars, { a: 1 })
+
+      expect(taskbar_2.reload).to have_attributes(preferences: { a: 1 })
+      expect(taskbar_3.reload).not_to have_attributes(preferences: { a: 1 })
+    end
+  end
+
+  describe '#related_taskbars' do
+    let(:key)       { Random.hex }
+    let(:taskbar_1) { create(:taskbar, key: key, user: create(:user)) }
+    let(:taskbar_2) { create(:taskbar, key: key, user: taskbar_1.user, app: 'mobile') }
+    let(:taskbar_3) { create(:taskbar, key: key, user: create(:user)) }
+    let(:taskbar_4) { create(:taskbar, user: create(:user)) }
+
+    it 'calls related_taskbars scope' do
+      taskbar = create(:taskbar)
+
+      allow(described_class).to receive(:related_taskbars)
+
+      taskbar.related_taskbars
+
+      expect(described_class).to have_received(:related_taskbars).with(taskbar)
+    end
+  end
+
+  describe '.related_taskbars' do
+    let(:key)       { Random.hex }
+    let(:taskbar_1) { create(:taskbar, key: key, user: create(:user)) }
+    let(:taskbar_2) { create(:taskbar, key: key, user: taskbar_1.user, app: 'mobile') }
+    let(:taskbar_3) { create(:taskbar, key: key, user: create(:user)) }
+    let(:taskbar_4) { create(:taskbar, user: create(:user)) }
+
+    before { taskbar_1 && taskbar_2 && taskbar_3 && taskbar_4 }
+
+    it 'returns all taskbars with the same key except given taskbars' do
+      expect(described_class.related_taskbars(taskbar_1)).to match_array([taskbar_2, taskbar_3])
+    end
+  end
 end

Some files were not shown because too many files changed in this diff