serval-admin: serval-builds: add search by marksvc · Pull Request #3907 · sillsdev/web-xforge

marksvc · 2026-05-25T22:32:49Z

This patch extends the search/filter capability of the Serval build
records. Previously it would filter by SF project ID, but now it
allows searching by project name, requesting user ID, reference
project name, etc.

Showing projects by SF project ID was changed to use the search control. (Although the Projects tab isn't hooked up to direct you to the Serval builds tab at this time to see it in action.)

I used the term "search" rather than "filter". "Search" is familiar
language. We are also checking the search term in many data fields
rather than having the user specify that they want to only show
records where a particular field contains a particular value.

Requester name etc. is included in what is searched. I chose to fetch
this from RealtimeServer, not include it in the backend
ServalBuildReportDto, mainly to keep the DTO from becoming
increasingly un-reusable in other parts of the application.
I also chose to query the requester information using the current
fetch and cache mechanism in requesterIdentity(), and at search time,
rather than pre-fetching values. I like that the proposed
implementation for searching user metadata is not as complex as some
other options.

This PR creates a serval-builds.copmonent.spec.ts helper method, createSearchRow(). I have a followup commit prepared that unifies the various row creation helper methods, that I did not include in this PR.

Screenshot showing search control:

Open in Devin Review

This change is

Copilot

Pull request overview

Extends the Serval Builds admin view to support a general-purpose “search” (via a q query param) across multiple build record fields, replacing the previous SF project ID-only filtering approach, and adds unit tests and UI styling to support the new behavior.

Changes:

Added a reactive search input wired to URL query param q, with async matching across project/build/user/reference metadata.
Updated requester identity typing and lookup usage to support searching requester name/display/email.
Expanded unit tests to cover new search behavior and query param synchronization; updated theme/styles for the new search UI.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.ts`	Adds search control, URL param sync, and async search filtering across many row fields.
`src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.spec.ts`	Adds/updates tests for search behavior, async filtering, and requester identity handling.
`src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.scss`	Styles the new search field and its active (populated) state.
`src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.html`	Adds the Material search input with clear button and helper info icon.
`src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/_serval-builds-theme.scss`	Introduces a CSS variable for populated-search background color.
`doc/code-rules-ai.md`	Clarifies frontend test helper placement guidance with an example.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

marksvc · 2026-05-25T22:42:04Z

+}
+
+.identifier-search-field {
+  width: min(14rem);


marksvc · 2026-05-25T22:42:34Z

+  // Search box. The populated search box has a background colour different than the
+  // application palaette colours to make it stand out that not all the records are
+  // being shown.


marksvc · 2026-05-25T22:44:18Z

+    void this.router.navigate([], {
+      relativeTo: this.route,
+      queryParams: { q: queryParam },
+      queryParamsHandling: 'merge'
+    });


marksvc · 2026-05-25T22:46:14Z

+    when(mockUserService.get(anything())).thenCall((requesterSFUserId: string) => {
+      const userData: RequesterInfo | undefined = this.requesterDataById.get(requesterSFUserId);
+      const changes$: Subject<void> | undefined = this.requesterChangesById.get(requesterSFUserId);
+      const userDoc = {
+        data: userData,
+        changes$: changes$
+      } as unknown as UserDoc;
+      return Promise.resolve(userDoc);
+    });


If a test here asks for information about a user ID that doesn't exist, it will be helpful for there to be an error. Not changing.

codecov · 2026-05-25T22:40:27Z

Codecov Report

❌ Patch coverage is 96.49123% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.94%. Comparing base (43d3a0d) to head (62d86aa).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...p/serval-administration/serval-builds.component.ts	96.49%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3907      +/-   ##
==========================================
+ Coverage   80.90%   80.94%   +0.03%     
==========================================
  Files         631      631              
  Lines       40681    40708      +27     
  Branches     6576     6580       +4     
==========================================
+ Hits        32913    32951      +38     
+ Misses       6741     6727      -14     
- Partials     1027     1030       +3

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

RaymondLuong3

Just a couple minor fixes. Nice work with the search box.

@RaymondLuong3 reviewed 6 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on marksvc).

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.scss line 77 at r2 (raw file):

:host ::ng-deep .identifier-search-field.identifier-search-field-active .mat-mdc-text-field-wrapper {
  background-color: var(--sf-serval-builds-search-background-populated);

Nit: It would be nice to have a fill colour that matches the app theme. The highlighter yellow is effective though.

Code quote:

  background-color: var(--sf-serval-builds-search-background-populated);

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.ts line 646 at r2 (raw file):

      requesterIdentity?.email,
      ...this.referenceProjectSearchTerms(row.trainingBooks),
      ...this.referenceProjectSearchTerms(row.translationBooks)

Wow, even the training and translation books can be searched. I guess that could be useful for someone, but maybe not.

Code quote:

      ...this.referenceProjectSearchTerms(row.trainingBooks),
      ...this.referenceProjectSearchTerms(row.translationBooks)

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.spec.ts line 159 at r2 (raw file):

    }));

    it('matches search terms using case-insensitive partial matching', fakeAsync(() => {

You could probably just leave a comment in the above test that says matching is case insensitive. This test is redundant.

Code quote:

    it('matches search terms using case-insensitive partial matching', fakeAsync(() => {

This patch extends the search/filter capability of the Serval build records. Previously it would filter by SF project ID, but now it allows searching by project name, requesting user ID, reference project name, etc. I used the term "search" rather than "filter". "Search" is familiar language. We are also checking the search term in many data fields rather than having the user specify that they want to only show records where a particular field contains a particular value. Requester name etc. is included in what is searched. I chose to fetch this from RealtimeServer, not include it in the backend ServalBuildReportDto, mainly to keep the DTO from becoming increasingly un-reusable in other parts of the application. I also chose to query the requester information using the current fetch and cache mechanism in requesterIdentity(), and at search time, rather than pre-fetching values. I like that the proposed implementation for searching user metadata is not as complex as some other options.

marksvc

@marksvc made 3 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on RaymondLuong3).

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.scss line 77 at r2 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Nit: It would be nice to have a fill colour that matches the app theme. The highlighter yellow is effective though.

In the current implementation, when a user clicks the Draft Jobs button in the Projects tab, they are sent to the Draft jobs tab where only matching projects are shown. On the Draft jobs tab, there is a big blue banner stating that draft jobs are being shown for the particular project. This banner was a motivation for making the box use yellow rather than a palette colour, so it stands out. Though I seemed to remember the Draft jobs banner seeming to be more concerned than I see it is when I open it up now.

I changed the background colour to --mat-sys-primary-container and the text on it to --mat-sys-on-primary-container.

Screenshots (which I'm not sure are accurately representing the colour? But it looks something like this):

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.ts line 646 at r2 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Wow, even the training and translation books can be searched. I guess that could be useful for someone, but maybe not.

Yeah. I can imagine it being handy. But we'll see.

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.spec.ts line 159 at r2 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

You could probably just leave a comment in the above test that says matching is case insensitive. This test is redundant.

I removed this and added some description to a previous test (a bit higher up) that mentions the case-insensitive and partial nature of the matching.

RaymondLuong3

@RaymondLuong3 reviewed 5 files and all commit messages, and resolved 3 discussions.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on marksvc).

marksvc requested a review from Copilot May 25, 2026 22:32

marksvc added the e2e Run e2e tests for this pull request label May 25, 2026

marksvc marked this pull request as draft May 25, 2026 22:32

Copilot started reviewing on behalf of marksvc May 25, 2026 22:33 View session

marksvc added the testing not required label May 25, 2026

Copilot AI reviewed May 25, 2026

View reviewed changes

marksvc temporarily deployed to e2e_tests May 25, 2026 22:36 — with GitHub Actions Inactive

marksvc temporarily deployed to e2e_tests May 25, 2026 22:37 — with GitHub Actions Inactive

marksvc temporarily deployed to screenshot_diff May 25, 2026 22:39 — with GitHub Actions Inactive

marksvc marked this pull request as ready for review May 25, 2026 22:47

marksvc temporarily deployed to e2e_tests May 25, 2026 22:49 — with GitHub Actions Inactive

marksvc temporarily deployed to screenshot_diff May 25, 2026 22:53 — with GitHub Actions Inactive

RaymondLuong3 self-assigned this May 29, 2026

RaymondLuong3 requested changes May 29, 2026

View reviewed changes

marksvc added 4 commits May 29, 2026 17:12

Fixes

3d7243e

Tests regarding case insensitivity and partial matching

c076a1e

Change search fill to primary container

0ddfe89

marksvc force-pushed the task/sb-search branch from 2f50cd2 to 0ddfe89 Compare May 29, 2026 23:13

marksvc commented May 29, 2026

View reviewed changes

marksvc temporarily deployed to e2e_tests May 29, 2026 23:16 — with GitHub Actions Inactive

De-lint

62d86aa

marksvc temporarily deployed to e2e_tests May 29, 2026 23:20 — with GitHub Actions Inactive

marksvc temporarily deployed to screenshot_diff May 29, 2026 23:23 — with GitHub Actions Inactive

RaymondLuong3 approved these changes May 30, 2026

View reviewed changes

RaymondLuong3 merged commit e354aab into master May 30, 2026
25 of 26 checks passed

RaymondLuong3 deleted the task/sb-search branch May 30, 2026 04:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

serval-admin: serval-builds: add search#3907

serval-admin: serval-builds: add search#3907
RaymondLuong3 merged 5 commits into
masterfrom
task/sb-search

marksvc commented May 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

marksvc May 25, 2026

Uh oh!

marksvc May 25, 2026

Uh oh!

marksvc May 25, 2026

Uh oh!

marksvc May 25, 2026

Uh oh!

codecov Bot commented May 25, 2026 •

edited

Loading

Uh oh!

RaymondLuong3 left a comment

Uh oh!

marksvc left a comment

Uh oh!

RaymondLuong3 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

marksvc commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

marksvc May 25, 2026

Choose a reason for hiding this comment

Uh oh!

marksvc May 25, 2026

Choose a reason for hiding this comment

Uh oh!

marksvc May 25, 2026

Choose a reason for hiding this comment

Uh oh!

marksvc May 25, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

RaymondLuong3 left a comment

Choose a reason for hiding this comment

Uh oh!

marksvc left a comment

Choose a reason for hiding this comment

Uh oh!

RaymondLuong3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marksvc commented May 25, 2026 •

edited

Loading

codecov Bot commented May 25, 2026 •

edited

Loading