[AC-2559] - BUG - Refactor members search observable chains

[vincentsalucci]

Type of change

• Bug fix
• New feature development
• Tech debt (refactoring, code cleanup, dependency upgrades, etc)
• Build/deploy pipeline (DevOps)
• Other

Objective

Searching for members within an organization is broken. Refactor the observable chains to reduce boilerplate and restore the search functionality.

Code changes

apps/web/src/app/admin-console/common/base.people.component.ts

• Removed getter/setter/behavior subject for searchText tracking
• Added a FormControl for search text manipulation and information binding
• Added two observable chains that will pass along the search text / state to its subscribers
• Moved isPaging into one of the above mentioned observables

apps/web/src/app/admin-console/organizations/members/people.component.html

• Updated searchText references to take either the control or the read-only value
• Replaced isPaging method calls with the new observable, retrieving data via the async pipe

apps/web/src/app/admin-console/organizations/members/people.component.ts

• Set initial value if query param search text is available

bitwarden_license/bit-web/src/app/admin-console/providers/manage/people.component.html

• Updated searchText references to take either the control or the read-only value
• Replaced isPaging method calls with the new observable, retrieving data via the async pipe

bitwarden_license/bit-web/src/app/admin-console/providers/manage/people.component.ts

• Set initial value if query param search text is available

Screenshots

Screen.Recording.2024-05-16.at.9.15.12.PM.mov

Before you submit

• Please add unit tests where it makes sense to do so (encouraged but not required)
• If this change requires a documentation update - notify the documentation team
• If this change has particular deployment requirements - notify the DevOps team
• Ensure that all UI additions follow WCAG AA requirements

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 28.15%. Comparing base (0b95008) to head (c474baa).

Files	Patch %	Lines
.../app/admin-console/common/base.people.component.ts	0.00%	10 Missing ⚠️
...-console/organizations/members/people.component.ts	0.00%	4 Missing ⚠️
...admin-console/providers/manage/people.component.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9230   +/-   ##
=======================================
  Coverage   28.15%   28.15%           
=======================================
  Files        2372     2372           
  Lines       70068    70063    -5     
  Branches    13149    13149           
=======================================
  Hits        19729    19729           
+ Misses      48775    48770    -5     
  Partials     1564     1564           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Checkmarx One – Scan Summary & Details – 0aa306bb-5756-48d2-9cf4-7f0b8967bf96

No New Or Fixed Issues Found

[eliykat]

I'm concerned here we have 3 other variables that affect this callback, but because it's now an observable, it's only going to be recalculated when isSearching emits.

Thinking aloud here, those other variables are:

• this.users - part of this logic is that the component is paging when this.users.length exceeds the pre-set page size. this.users.length can change independent of isSearching$, so I think this needs to be an observable that can be combined with isSearching$.
• this.didScroll - the logic here seems to be, "if you scrolled down enough to get the next page of results, and then you search, reset the paging". I think that's still OK, because it's not actually triggered by a change in didScroll - it's more like a record of what the user's already done. So I think we can leave that.
• this.pageSize - this is a constant which will never change, so it's fine

I'm not super familiar with this paging logic (really it should be replaced by infinite scroll) but this seems like a potential gotcha that I want to make sure we think about.

[eliykat]

Thanks for syncing on this! Where we landed was that this is fine for now, and I've created AC-2673 to remove the old paging logic.