From 8c6cef3474e2eae31664e27ff5c572bad81f8f40 Mon Sep 17 00:00:00 2001 From: Zita Szupera Date: Thu, 19 Oct 2023 10:53:59 +0200 Subject: [PATCH] fix: remove unnecessary user queries from avatar, properly bind showOnlineIndicator input --- .../src/lib/avatar/avatar.component.html | 2 +- .../src/lib/avatar/avatar.component.spec.ts | 72 +++++++++---------- .../src/lib/avatar/avatar.component.ts | 14 +--- 3 files changed, 38 insertions(+), 50 deletions(-) diff --git a/projects/stream-chat-angular/src/lib/avatar/avatar.component.html b/projects/stream-chat-angular/src/lib/avatar/avatar.component.html index 5c77539e..ecc8f9a5 100644 --- a/projects/stream-chat-angular/src/lib/avatar/avatar.component.html +++ b/projects/stream-chat-angular/src/lib/avatar/avatar.component.html @@ -42,7 +42,7 @@
diff --git a/projects/stream-chat-angular/src/lib/avatar/avatar.component.spec.ts b/projects/stream-chat-angular/src/lib/avatar/avatar.component.spec.ts index f615d0ea..f6ab8221 100644 --- a/projects/stream-chat-angular/src/lib/avatar/avatar.component.spec.ts +++ b/projects/stream-chat-angular/src/lib/avatar/avatar.component.spec.ts @@ -14,18 +14,16 @@ describe('AvatarComponent', () => { let queryImg: () => HTMLImageElement | null; let queryFallbackImg: () => HTMLImageElement | null; let queryOnlineIndicator: () => HTMLElement | null; - let queryUsersMock: jasmine.Spy; let events$: Subject; let chatClientServiceMock: { - chatClient: { user: { id: string }; queryUsers: jasmine.Spy }; + chatClient: { user: { id: string } }; events$: Subject; }; beforeEach(() => { - queryUsersMock = jasmine.createSpy(); events$ = new Subject(); chatClientServiceMock = { - chatClient: { user: { id: 'current-user' }, queryUsers: queryUsersMock }, + chatClient: { user: { id: 'current-user' } }, events$, }; TestBed.configureTestingModule({ @@ -230,14 +228,18 @@ describe('AvatarComponent', () => { channel.state.members = { otheruser: { user_id: 'otheruser', - user: { id: 'otheruser', name: 'Jack', image: 'url/to/img' }, + user: { + id: 'otheruser', + name: 'Jack', + image: 'url/to/img', + online: true, + }, }, [chatClientServiceMock.chatClient.user.id]: { user_id: chatClientServiceMock.chatClient.user.id, user: { id: chatClientServiceMock.chatClient.user.id, name: 'Sara' }, }, }; - queryUsersMock.and.resolveTo({ users: [{ online: true }] }); component.channel = channel; void component.ngOnChanges({ channel: {} as SimpleChange }); await fixture.whenStable(); @@ -251,14 +253,18 @@ describe('AvatarComponent', () => { channel.state.members = { otheruser: { user_id: 'otheruser', - user: { id: 'otheruser', name: 'Jack', image: 'url/to/img' }, + user: { + id: 'otheruser', + name: 'Jack', + image: 'url/to/img', + online: false, + }, }, [chatClientServiceMock.chatClient.user.id]: { user_id: chatClientServiceMock.chatClient.user.id, user: { id: chatClientServiceMock.chatClient.user.id, name: 'Sara' }, }, }; - queryUsersMock.and.resolveTo({ users: [{ online: false }] }); component.channel = channel; void component.ngOnChanges({ channel: {} as SimpleChange }); await fixture.whenStable(); @@ -272,14 +278,18 @@ describe('AvatarComponent', () => { channel.state.members = { otheruser: { user_id: 'otheruser', - user: { id: 'otheruser', name: 'Jack', image: 'url/to/img' }, + user: { + id: 'otheruser', + name: 'Jack', + image: 'url/to/img', + online: false, + }, }, [chatClientServiceMock.chatClient.user.id]: { user_id: chatClientServiceMock.chatClient.user.id, user: { id: chatClientServiceMock.chatClient.user.id, name: 'Sara' }, }, }; - queryUsersMock.and.resolveTo({ users: [{ online: false }] }); component.channel = channel; void component.ngOnChanges({ channel: {} as SimpleChange }); await fixture.whenStable(); @@ -299,7 +309,7 @@ describe('AvatarComponent', () => { expect(queryOnlineIndicator()).not.toBeNull(); }); - it(`should handle query users error when displaying the online indicator`, async () => { + it(`shouldn't display online indicator in not 1:1 channels`, async () => { const channel = generateMockChannels()[0]; channel.state.members = { otheruser: { @@ -311,37 +321,20 @@ describe('AvatarComponent', () => { online: true, }, }, - [chatClientServiceMock.chatClient.user.id]: { - user_id: chatClientServiceMock.chatClient.user.id, - user: { id: chatClientServiceMock.chatClient.user.id, name: 'Sara' }, - }, - }; - queryUsersMock.and.rejectWith(new Error('Permission denied')); - component.channel = channel; - void component.ngOnChanges({ channel: {} as SimpleChange }); - await fixture.whenStable(); - fixture.detectChanges(); - - expect(queryOnlineIndicator()).not.toBeNull(); - }); - - it(`shouldn't display online indicator in not 1:1 channels`, async () => { - const channel = generateMockChannels()[0]; - channel.state.members = { - otheruser: { - user_id: 'otheruser', - user: { id: 'otheruser', name: 'Jack', image: 'url/to/img' }, - }, thirduser: { user_id: 'thirduser', - user: { id: 'thirduser', name: 'John', image: 'url/to/img' }, + user: { + id: 'thirduser', + name: 'John', + image: 'url/to/img', + online: true, + }, }, [chatClientServiceMock.chatClient.user.id]: { user_id: chatClientServiceMock.chatClient.user.id, user: { id: chatClientServiceMock.chatClient.user.id, name: 'Sara' }, }, }; - queryUsersMock.and.resolveTo({ users: [{ online: true }] }); component.channel = channel; void component.ngOnChanges({ channel: {} as SimpleChange }); await fixture.whenStable(); @@ -355,20 +348,25 @@ describe('AvatarComponent', () => { channel.state.members = { otheruser: { user_id: 'otheruser', - user: { id: 'otheruser', name: 'Jack', image: 'url/to/img' }, + user: { + id: 'otheruser', + name: 'Jack', + image: 'url/to/img', + online: true, + }, }, [chatClientServiceMock.chatClient.user.id]: { user_id: chatClientServiceMock.chatClient.user.id, user: { id: chatClientServiceMock.chatClient.user.id, name: 'Sara' }, }, }; - queryUsersMock.and.resolveTo({ users: [{ online: true }] }); component.channel = channel; + component.showOnlineIndicator = false; void component.ngOnChanges({ channel: {} as SimpleChange }); await fixture.whenStable(); fixture.detectChanges(); - expect(queryOnlineIndicator()).not.toBeNull(); + expect(queryOnlineIndicator()).toBeNull(); }); it('should display initials correctly, #initialsType is "first-letter-of-each-word"', () => { diff --git a/projects/stream-chat-angular/src/lib/avatar/avatar.component.ts b/projects/stream-chat-angular/src/lib/avatar/avatar.component.ts index c465018d..f0df65d5 100644 --- a/projects/stream-chat-angular/src/lib/avatar/avatar.component.ts +++ b/projects/stream-chat-angular/src/lib/avatar/avatar.component.ts @@ -72,11 +72,12 @@ export class AvatarComponent implements OnChanges { private ngZone: NgZone ) {} - async ngOnChanges(changes: SimpleChanges) { + ngOnChanges(changes: SimpleChanges) { if (changes['channel']) { if (this.channel) { const otherMember = this.getOtherMemberIfOneToOneChannel(); if (otherMember) { + this.isOnline = otherMember.online || false; this.isOnlineSubscription = this.chatClientService.events$ .pipe(filter((e) => e.eventType === 'user.presence.changed')) .subscribe((event) => { @@ -86,17 +87,6 @@ export class AvatarComponent implements OnChanges { }); } }); - try { - const response = await this.chatClientService.chatClient.queryUsers( - { - id: { $eq: otherMember.id }, - } - ); - this.isOnline = response.users[0]?.online || false; - } catch (error) { - // Fallback if we can't query user -> for example due to permission problems - this.isOnline = otherMember.online || false; - } } else { this.isOnlineSubscription?.unsubscribe(); }