Skip to content

Commit

Permalink
fix: remove unnecessary user queries from avatar, properly bind showO…
Browse files Browse the repository at this point in the history
…nlineIndicator input
  • Loading branch information
szuperaz committed Oct 19, 2023
1 parent 55a3d31 commit 8c6cef3
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
</ng-template>
<div
data-testid="online-indicator"
*ngIf="isOnline"
*ngIf="isOnline && showOnlineIndicator"
class="str-chat__avatar--online-indicator"
></div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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: {
Expand All @@ -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();
Expand All @@ -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"', () => {
Expand Down
14 changes: 2 additions & 12 deletions projects/stream-chat-angular/src/lib/avatar/avatar.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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();
}
Expand Down

0 comments on commit 8c6cef3

Please sign in to comment.