Browse Source

Maintenance: Switching between tabs in old UI causes unnecessary subscription triggering in new UI

Mantas 4 months ago
parent
commit
8759785146

+ 6 - 1
app/models/taskbar.rb

@@ -103,6 +103,12 @@ class Taskbar < ApplicationModel
     save!
   end
 
+  def saved_change_to_dirty?
+    return false if !saved_change_to_preferences?
+
+    !!preferences[:dirty] != !!preferences_previously_was[:dirty]
+  end
+
   private
 
   def update_last_contact
@@ -188,5 +194,4 @@ class Taskbar < ApplicationModel
       data,
     )
   end
-
 end

+ 19 - 8
app/models/taskbar/triggers_subscriptions.rb

@@ -7,20 +7,22 @@ module Taskbar::TriggersSubscriptions
   included do
     attr_accessor :skip_live_user_trigger, :skip_item_trigger
 
-    after_commit :trigger_live_user_subscriptions, unless: :skip_live_user_trigger
+    # Sends latest live users list to both mobile and desktop apps.
+    after_save_commit :trigger_live_user_subscriptions, unless: :skip_live_user_trigger
+
+    # Sends changes in taskbars list to desktop app
     after_create_commit  :trigger_taskbar_item_create_subscriptions,  unless: :skip_item_trigger
     after_update_commit  :trigger_taskbar_item_update_subscriptions,  unless: :skip_item_trigger
     after_destroy_commit :trigger_taskbar_item_destroy_subscriptions, unless: :skip_item_trigger
 
+    # Tells dekstop app that changes are available.
     after_update_commit  :trigger_taskbar_item_state_update_subscriptions
   end
 
   private
 
   def trigger_live_user_subscriptions
-    return true if !saved_change_to_attribute?('preferences')
-
-    return true if !persisted?
+    return if !saved_change_to_attribute?('preferences')
 
     Gql::Subscriptions::TicketLiveUserUpdates.trigger(
       self,
@@ -33,24 +35,33 @@ module Taskbar::TriggersSubscriptions
   end
 
   def trigger_taskbar_item_create_subscriptions
+    return if app != 'desktop'
+
     Gql::Subscriptions::User::Current::TaskbarItemUpdates.trigger_after_create(self)
   end
 
   def trigger_taskbar_item_update_subscriptions
+    return if app != 'desktop'
+
     # See specific subscription for prio changes / list sorting.
-    return true if saved_change_to_attribute?('prio')
+    # Active attribute is not sent by this subscription.
+    # Nor are live users, which are most of preferences content.
+    # However, there is dirty flag in preferences and it is checked separately.
+    return if !saved_change_to_dirty? &&
+              saved_changes.keys.without('active', 'preferences', 'prio', 'last_contact', 'updated_at').none?
 
     Gql::Subscriptions::User::Current::TaskbarItemUpdates.trigger_after_update(self)
   end
 
   def trigger_taskbar_item_destroy_subscriptions
+    return if app != 'desktop'
+
     Gql::Subscriptions::User::Current::TaskbarItemUpdates.trigger_after_destroy(self)
   end
 
   def trigger_taskbar_item_state_update_subscriptions
-    return true if !saved_change_to_attribute?('state')
-    return true if !app.eql?('desktop')
-    return true if destroyed?
+    return if !saved_change_to_attribute?('state')
+    return if app != 'desktop'
 
     Gql::Subscriptions::User::Current::TaskbarItemStateUpdates.trigger(
       nil,

+ 21 - 1
spec/models/taskbar/triggers_subscriptions_spec.rb

@@ -40,6 +40,26 @@ RSpec.describe Taskbar::TriggersSubscriptions, :aggregate_failures do
     end
   end
 
+  context 'when updating active' do
+    it 'triggers correctly' do
+      taskbar.active = !taskbar.active
+      taskbar.save!
+      expect(gqs::TicketLiveUserUpdates).to have_received(:trigger).twice
+      expect(gqs_uc::TaskbarItemUpdates).not_to have_received(:trigger_after_update)
+      expect(gqs_uc::TaskbarItemStateUpdates).not_to have_received(:trigger)
+    end
+  end
+
+  context 'when updating dirty' do
+    it 'triggers correctly' do
+      taskbar.preferences[:dirty] = !taskbar.preferences[:dirty]
+      taskbar.save!
+      expect(gqs::TicketLiveUserUpdates).to have_received(:trigger).twice
+      expect(gqs_uc::TaskbarItemUpdates).to have_received(:trigger_after_update).once
+      expect(gqs_uc::TaskbarItemStateUpdates).not_to have_received(:trigger)
+    end
+  end
+
   context 'when updating last_contact_at' do
     it 'triggers correctly' do
       taskbar.touch_last_contact!
@@ -67,7 +87,7 @@ RSpec.describe Taskbar::TriggersSubscriptions, :aggregate_failures do
         taskbar.state = { 'body' => 'test' }
         taskbar.save!
         expect(gqs::TicketLiveUserUpdates).to have_received(:trigger).exactly(2)
-        expect(gqs_uc::TaskbarItemUpdates).to have_received(:trigger_after_update).once # only for taskbar
+        expect(gqs_uc::TaskbarItemUpdates).not_to have_received(:trigger_after_update)
         expect(gqs_uc::TaskbarItemStateUpdates).not_to have_received(:trigger)
       end
     end

+ 39 - 0
spec/models/taskbar_spec.rb

@@ -656,4 +656,43 @@ RSpec.describe Taskbar, type: :model do
       expect(described_class.related_taskbars(taskbar_1)).to contain_exactly(taskbar_2, taskbar_3)
     end
   end
+
+  describe '#saved_chanegs_to_dirty?' do
+    let(:taskbar) { create(:taskbar) }
+
+    it 'fresh taskbar has no changes to dirty' do
+      expect(taskbar).not_to be_saved_change_to_dirty
+    end
+
+    it 'no changes to dirty after saving without dirty lag' do
+      taskbar.active = !taskbar.active
+      taskbar.save!
+
+      expect(taskbar).not_to be_saved_change_to_dirty
+    end
+
+    it 'no changes to dirty after marking as not dirty' do
+      taskbar.preferences[:dirty] = false
+      taskbar.save!
+
+      expect(taskbar).not_to be_saved_change_to_dirty
+    end
+
+    it 'dirty was changed after marking as dirty' do
+      taskbar.preferences[:dirty] = true
+      taskbar.save!
+
+      expect(taskbar).to be_saved_change_to_dirty
+    end
+
+    it 'dirty was changed after marking previously dirty item as not dirty' do
+      taskbar.preferences[:dirty] = true
+      taskbar.save!
+
+      taskbar.preferences[:dirty] = false
+      taskbar.save!
+
+      expect(taskbar).to be_saved_change_to_dirty
+    end
+  end
 end