[AC-2559] - BUG - Refactor members search observable chains (#9230)

* fix: update observable chains for search, refs AC-2559

* fix: remove comment, reorganize variables, refs AC-2559

* chore: remove ngOnInit from base people component, refs AC-2559

* chore: remove async declaration from resetPaging, refs AC-2559

* chore: replace bit-search ngmodel with formControl, refs AC-2559

* chore: move destroy pattern out of base class, refs AC-2559

* fix: remove ngOnDestroy super call, refs AC-2559

* Improve performance issues

---------

Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
Co-authored-by: Thomas Rittson <trittson@bitwarden.com>
This commit is contained in:
Vincent Salucci 2024-05-23 09:05:58 -05:00 committed by GitHub
parent 92b70d983d
commit 8335d8f484
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 40 additions and 72 deletions

View File

@ -1,13 +1,6 @@
import { Directive, OnDestroy, OnInit, ViewChild, ViewContainerRef } from "@angular/core";
import {
BehaviorSubject,
Subject,
firstValueFrom,
from,
lastValueFrom,
switchMap,
takeUntil,
} from "rxjs";
import { Directive, ViewChild, ViewContainerRef } from "@angular/core";
import { FormControl } from "@angular/forms";
import { firstValueFrom, concatMap, map, lastValueFrom, startWith, debounceTime } from "rxjs";
import { SearchPipe } from "@bitwarden/angular/pipes/search.pipe";
import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe";
@ -40,10 +33,8 @@ const MaxCheckedCount = 500;
@Directive()
export abstract class BasePeopleComponent<
UserType extends ProviderUserUserDetailsResponse | OrganizationUserView,
>
implements OnInit, OnDestroy
{
UserType extends ProviderUserUserDetailsResponse | OrganizationUserView,
> {
@ViewChild("confirmTemplate", { read: ViewContainerRef, static: true })
confirmModalRef: ViewContainerRef;
@ -106,19 +97,22 @@ export abstract class BasePeopleComponent<
protected didScroll = false;
protected pageSize = 100;
protected destroy$ = new Subject<void>();
protected searchControl = new FormControl("", { nonNullable: true });
protected isSearching$ = this.searchControl.valueChanges.pipe(
debounceTime(500),
concatMap((searchText) => this.searchService.isSearchable(searchText)),
startWith(false),
);
protected isPaging$ = this.isSearching$.pipe(
map((isSearching) => {
if (isSearching && this.didScroll) {
this.resetPaging();
}
return !isSearching && this.users && this.users.length > this.pageSize;
}),
);
private pagedUsersCount = 0;
private _searchText$ = new BehaviorSubject<string>("");
private isSearching: boolean = false;
get searchText() {
return this._searchText$.value;
}
set searchText(value: string) {
this._searchText$.next(value);
}
constructor(
protected apiService: ApiService,
@ -143,22 +137,6 @@ export abstract class BasePeopleComponent<
abstract reinviteUser(id: string): Promise<void>;
abstract confirmUser(user: UserType, publicKey: Uint8Array): Promise<void>;
ngOnInit(): void {
this._searchText$
.pipe(
switchMap((searchText) => from(this.searchService.isSearchable(searchText))),
takeUntil(this.destroy$),
)
.subscribe((isSearchable) => {
this.isSearching = isSearchable;
});
}
ngOnDestroy(): void {
this.destroy$.next();
this.destroy$.complete();
}
async load() {
const response = await this.getUsers();
this.statusMap.clear();
@ -202,8 +180,6 @@ export abstract class BasePeopleComponent<
}
// Reset checkbox selecton
this.selectAll(false);
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.resetPaging();
}
@ -236,7 +212,7 @@ export abstract class BasePeopleComponent<
const filteredUsers = this.searchPipe.transform(
this.users,
this.searchText,
this.searchControl.value,
"name",
"email",
"id",
@ -249,7 +225,7 @@ export abstract class BasePeopleComponent<
}
}
async resetPaging() {
resetPaging() {
this.pagedUsers = [];
this.loadMore();
}
@ -418,16 +394,6 @@ export abstract class BasePeopleComponent<
}
}
isPaging() {
const searching = this.isSearching;
if (searching && this.didScroll) {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.resetPaging();
}
return !searching && this.users && this.users.length > this.pageSize;
}
protected revokeWarningMessage(): string {
return this.i18nService.t("revokeUserConfirmation");
}
@ -440,8 +406,6 @@ export abstract class BasePeopleComponent<
let index = this.users.indexOf(user);
if (index > -1) {
this.users.splice(index, 1);
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.resetPaging();
}

View File

@ -1,7 +1,7 @@
<app-header>
<bit-search
class="tw-grow"
[(ngModel)]="searchText"
[formControl]="searchControl"
[placeholder]="'searchMembers' | i18n"
></bit-search>
@ -48,9 +48,9 @@
<ng-container
*ngIf="
!loading &&
(isPaging()
((isPaging$ | async)
? pagedUsers
: (users | search: searchText : 'name' : 'email' : 'id')) as searchedUsers
: (users | search: searchControl.value : 'name' : 'email' : 'id')) as searchedUsers
"
>
<p *ngIf="!searchedUsers.length">{{ "noMembersInList" | i18n }}</p>
@ -66,7 +66,7 @@
<bit-table
infinite-scroll
[infiniteScrollDistance]="1"
[infiniteScrollDisabled]="!isPaging()"
[infiniteScrollDisabled]="!(isPaging$ | async)"
(scrolled)="loadMore()"
>
<ng-container header>

View File

@ -9,6 +9,7 @@ import {
map,
Observable,
shareReplay,
Subject,
switchMap,
takeUntil,
} from "rxjs";
@ -94,6 +95,8 @@ export class PeopleComponent extends BasePeopleComponent<OrganizationUserView> {
protected canUseSecretsManager$: Observable<boolean>;
private destroy$ = new Subject<void>();
constructor(
apiService: ApiService,
private route: ActivatedRoute,
@ -194,7 +197,7 @@ export class PeopleComponent extends BasePeopleComponent<OrganizationUserView> {
await this.load();
this.searchText = qParams.search;
this.searchControl.setValue(qParams.search);
if (qParams.viewEvents != null) {
const user = this.users.filter((u) => u.id === qParams.viewEvents);
if (user.length > 0 && user[0].status === OrganizationUserStatusType.Confirmed) {
@ -210,7 +213,8 @@ export class PeopleComponent extends BasePeopleComponent<OrganizationUserView> {
}
ngOnDestroy(): void {
super.ngOnDestroy();
this.destroy$.next();
this.destroy$.complete();
}
async load() {

View File

@ -1,5 +1,9 @@
<app-header>
<bit-search class="tw-grow" [(ngModel)]="searchText" [placeholder]="'search' | i18n"></bit-search>
<bit-search
class="tw-grow"
[formControl]="searchControl"
[placeholder]="'search' | i18n"
></bit-search>
<button type="button" bitButton buttonType="primary" (click)="invite()">
<i class="bwi bwi-plus bwi-fw" aria-hidden="true"></i>
{{ "inviteUsers" | i18n }}
@ -83,9 +87,9 @@
<ng-container
*ngIf="
!loading &&
(isPaging()
((isPaging$ | async)
? pagedUsers
: (users | search: searchText : 'name' : 'email' : 'id')) as searchedUsers
: (users | search: searchControl.value : 'name' : 'email' : 'id')) as searchedUsers
"
>
<p *ngIf="!searchedUsers.length">{{ "noUsersInList" | i18n }}</p>
@ -102,7 +106,7 @@
class="table table-hover table-list"
infiniteScroll
[infiniteScrollDistance]="1"
[infiniteScrollDisabled]="!isPaging()"
[infiniteScrollDisabled]="!(isPaging$ | async)"
(scrolled)="loadMore()"
>
<tbody>

View File

@ -103,7 +103,7 @@ export class PeopleComponent extends BasePeopleComponent<ProviderUserUserDetails
/* eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe, rxjs/no-nested-subscribe */
this.route.queryParams.pipe(first()).subscribe(async (qParams) => {
this.searchText = qParams.search;
this.searchControl.setValue(qParams.search);
if (qParams.viewEvents != null) {
const user = this.users.filter((u) => u.id === qParams.viewEvents);
if (user.length > 0 && user[0].status === ProviderUserStatusType.Confirmed) {
@ -116,10 +116,6 @@ export class PeopleComponent extends BasePeopleComponent<ProviderUserUserDetails
});
}
ngOnDestroy(): void {
super.ngOnDestroy();
}
getUsers(): Promise<ListResponse<ProviderUserUserDetailsResponse>> {
return this.apiService.getProviderUsers(this.providerId);
}