Merge pull request #493 from GetStream/online-indicator-fix

fix: remove unnecessary user queries from avatar, properly bind showO…
GetStream · Oct 19, 2023 · c17db10 · c17db10
2 parents 55a3d31 + 8c6cef3
commit c17db10
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 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
@@ -42,7 +42,7 @@
   </ng-template>
   <div
     data-testid="online-indicator"
-    *ngIf="isOnline"
+    *ngIf="isOnline && showOnlineIndicator"
     class="str-chat__avatar--online-indicator"
   ></div>
 </div>
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
@@ -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<ClientEvent>;
   let chatClientServiceMock: {
-    chatClient: { user: { id: string }; queryUsers: jasmine.Spy };
+    chatClient: { user: { id: string } };
     events$: Subject<ClientEvent>;
   };
 
   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
@@ -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();
         }