Maintenance: Desktop View - Improve triggering of subscription for av…

…atar changes.

Co-authored-by: Dominik Klein <dk@zammad.com>

app/frontend/apps/desktop/pages/personal-setting/views/PersonalSettingAvatar.vue

@@ -3,7 +3,9 @@
 <script setup lang="ts">
 import { computed, shallowRef } from 'vue'
 import { storeToRefs } from 'pinia'
+import type { ApolloCache, NormalizedCacheObject } from '@apollo/client'
 
+import { getApolloClient } from '#shared/server/apollo/client.ts'
 import QueryHandler from '#shared/server/apollo/handler/QueryHandler.ts'
 import MutationHandler from '#shared/server/apollo/handler/MutationHandler.ts'
 
@@ -105,6 +107,23 @@ const cropImageFlyout = useFlyout({
     import('../components/PersonalSettingAvatarCropImageFlyout.vue'),
 })
 
+const modifyDefaultAvatarCache = (
+  cache: ApolloCache<NormalizedCacheObject>,
+  avatar: Avatar | undefined,
+  newValue: boolean,
+) => {
+  if (!avatar) return
+
+  cache.modify({
+    id: cache.identify(avatar),
+    fields: {
+      default() {
+        return newValue
+      },
+    },
+  })
+}
+
 const storeAvatar = (image: ImageFileData) => {
   if (!image) return
 
@@ -131,16 +150,7 @@ const storeAvatar = (image: ImageFileData) => {
         })
         if (newIdPresent) return
 
-        if (currentDefaultAvatar.value) {
-          cache.modify({
-            id: cache.identify(currentDefaultAvatar.value),
-            fields: {
-              default() {
-                return false
-              },
-            },
-          })
-        }
+        modifyDefaultAvatarCache(cache, currentDefaultAvatar.value, false)
 
         let existingAvatars = cache.readQuery<AccountAvatarListQuery>({
           query: AccountAvatarListDocument,
@@ -208,34 +218,36 @@ const loadAvatar = async (input?: HTMLInputElement) => {
 }
 
 const selectAvatar = (avatar: Avatar) => {
+  // Update the cache already before the
+  const { cache } = getApolloClient()
+  const oldDefaultAvatar = currentDefaultAvatar.value
+
+  modifyDefaultAvatarCache(cache, oldDefaultAvatar, false)
+  modifyDefaultAvatarCache(cache, avatar, true)
+
   const accountAvatarSelectMutation = new MutationHandler(
     useAccountAvatarSelectMutation(() => ({
       variables: { id: avatar.id },
-      update(cache) {
-        currentAvatars.value.forEach((currentAvatar) => {
-          cache.modify({
-            id: cache.identify(currentAvatar),
-            fields: {
-              default() {
-                return currentAvatar.id === avatar.id
-              },
-            },
-          })
-        })
-      },
     })),
     {
       errorNotificationMessage: __('The avatar could not be selected.'),
     },
   )
 
-  accountAvatarSelectMutation.send().then(() => {
-    notify({
-      id: 'avatar-select-success',
-      type: NotificationTypes.Success,
-      message: __('Your avatar has been changed.'),
+  accountAvatarSelectMutation
+    .send()
+    .then(() => {
+      notify({
+        id: 'avatar-select-success',
+        type: NotificationTypes.Success,
+        message: __('Your avatar has been changed.'),
+      })
+    })
+    .catch(() => {
+      // Reset the cache again if the mutation fails.
+      modifyDefaultAvatarCache(cache, oldDefaultAvatar, true)
+      modifyDefaultAvatarCache(cache, avatar, false)
     })
-  })
 }
 
 const deleteAvatar = (avatar: Avatar) => {
@@ -244,14 +256,7 @@ const deleteAvatar = (avatar: Avatar) => {
       variables: { id: avatar.id },
       update(cache) {
         if (avatar.default) {
-          cache.modify({
-            id: cache.identify(currentAvatars.value[0]),
-            fields: {
-              default() {
-                return true
-              },
-            },
-          })
+          modifyDefaultAvatarCache(cache, currentAvatars.value[0], true)
         }
 
         cache.evict({ id: cache.identify(avatar) })

app/frontend/shared/server/apollo/cache.ts

@@ -36,7 +36,6 @@ const createCache = (
   additionalCacheInitializerModules: CacheInitializerModules = {},
 ): InMemoryCache => {
   registerInitializeModules(additionalCacheInitializerModules)
-
   return new InMemoryCache(cacheConfig)
 }
 

app/frontend/tests/graphql/builders/mocks.ts

@@ -372,13 +372,14 @@ class MockLink extends ApolloLink {
 }
 
 const cacheInitializerModules: CacheInitializerModules = import.meta.glob(
-  '../../../../mobile/server/apollo/cache/initializer/*.ts',
+  '../../../shared/server/apollo/cache/initializer/*.ts',
   { eager: true },
 )
 
 const createMockClient = () => {
   const link = new MockLink()
   const cache = createCache(cacheInitializerModules)
+
   const client = new ApolloClient({
     cache,
     link,
@@ -390,6 +391,17 @@ const createMockClient = () => {
 // this enabled automocking - if this file is not imported somehow, fetch request will throw an error
 export const mockedApolloClient = createMockClient()
 
+vi.mock('#shared/server/apollo/client.ts', () => {
+  return {
+    clearApolloClientStore: async () => {
+      await mockedApolloClient.clearStore()
+    },
+    getApolloClient: () => {
+      return mockedApolloClient
+    },
+  }
+})
+
 afterEach(() => {
   mockedApolloClient.clearStore()
 })

app/frontend/tests/support/components/visitView.ts

@@ -17,21 +17,6 @@ import renderComponent, {
 } from './renderComponent.ts'
 import { getTestAppName } from './app.ts'
 
-vi.mock('#shared/server/apollo/client.ts', () => {
-  return {
-    clearApolloClientStore: () => {
-      return Promise.resolve()
-    },
-    getApolloClient: () => {
-      return {
-        cache: {
-          gc: () => [],
-        },
-      }
-    },
-  }
-})
-
 Object.defineProperty(window, 'fetch', {
   value: (path: string) => {
     throw new Error(`calling fetch on ${path}`)
@@ -58,6 +43,21 @@ export const visitView = async (
     : await import('#mobile/router/index.ts')
 
   if (options.mockApollo) {
+    vi.mock('#shared/server/apollo/client.ts', () => {
+      return {
+        clearApolloClientStore: () => {
+          return Promise.resolve()
+        },
+        getApolloClient: () => {
+          return {
+            cache: {
+              gc: () => [],
+            },
+          }
+        },
+      }
+    })
+
     mockApolloClient([])
   } else if (isDesktop) {
     // automocking is enabled when this file is imported because it happens on the top level

app/models/avatar/triggers_subscriptions.rb

@@ -5,18 +5,51 @@ module Avatar::TriggersSubscriptions
   extend ActiveSupport::Concern
 
   included do
-    after_update_commit :trigger_user_subscriptions
+    after_update_commit :update_avatar_subscription
+    after_destroy_commit :destroy_avatar_subscription
   end
 
   private
 
-  def trigger_user_subscriptions
-    return if ObjectLookup.by_id(object_lookup_id) != 'User'
+  def update_avatar_subscription
+    return if saved_changes.blank?
+    return if !saved_changes.key?('default')
+
+    # Following logic for checking if the subscription triggering is needed is applied:
+    #   If the default avatar was changed from false to true, we can skip the triggering if
+    #     a) the avatar is not initial.
+    #     b) there is already a default avatar that is not initial.
+
+    if saved_changes['default'].first == false && saved_changes['default'].last == true
+      return if !initial
 
-    return if saved_changes.empty? && !new_record? && !destroyed?
+      default_non_initial_avatar = Avatar.find_by(
+        object_lookup_id:,
+        o_id:,
+        default:          true,
+        initial:          false,
+      )
+
+      return if default_non_initial_avatar.present?
+    end
+
+    trigger_user_subscription
+  end
+
+  def destroy_avatar_subscription
+    # We need to check if the default avatar was deleted.
+    #   If yes, there is no need to trigger the subscription,
+    #     because it will be triggered by changing the default state of the remaining avatar.
+    return if default
+
+    trigger_user_subscription
+  end
+
+  def trigger_user_subscription
+    return if ObjectLookup.by_id(object_lookup_id) != 'User'
 
     Gql::Subscriptions::AccountAvatarUpdates.trigger(
-      self,
+      nil,
       arguments: {
         user_id: Gql::ZammadSchema.id_from_internal_id('User', o_id)
       }

spec/graphql/gql/subscriptions/account_avatar_updates_spec.rb

@@ -23,7 +23,7 @@
   end
   let(:mock_channel) { build_mock_channel }
   let(:target)       { create(:user) }
-  let(:avatar)       { create(:avatar, o_id: target.id, default: false) }
+  let(:avatar)       { create(:avatar, o_id: target.id, default: false, initial: true) }
   let(:variables)    { { userId: gql.id(target) } }
 
   context 'with authenticated user', authenticated_as: :target do