serval-admin: serval-builds: add copyable SF IDs in Input card#3888
serval-admin: serval-builds: add copyable SF IDs in Input card#3888marksvc wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Serval Administration “Serval builds” Inputs card to make referenced Scripture Forge project IDs visible and easily copyable, improving admin workflow when cross-referencing builds to SF projects.
Changes:
- Added a small view-model (
BuildInputItem) and mapping helper to produce display-ready input rows. - Updated the Inputs card UI to show SF project ID + copy button, plus optional short name/project name with links.
- Added unit tests covering the new mapping helper and adjusted styling for the new layout.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.ts | Adds BuildInputItem and determineBuildInputs() to prepare input display data (IDs, links, names, formatted books). |
| src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.html | Renders project short name / name (linked), SF project ID in monospace, and a copy control in the Inputs card. |
| src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.scss | Adds layout styles for the new multi-line per-project input display and ID styling. |
| src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.spec.ts | Adds tests for determineBuildInputs() output (link, optional names, book formatting). |
Comments suppressed due to low confidence (1)
src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-builds.component.html:340
- Same as above:
projectBook.projectLinkis always populated bydetermineBuildInputs(), so this@if (projectBook.projectLink != null)check is currently redundant and leaves an unreachable@elsebranch. Simplify by makingprojectLinkrequired and removing the conditional, or returnundefinedwhen appropriate.
@if (projectBook.projectLink != null) {
<a
[href]="projectBook.projectLink"
target="_blank"
rel="noopener"
class="project-link"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @for (projectBook of determineBuildInputs(projectBooks); track projectBook.sfProjectId) { | ||
| <div class="detail-project-books-item"> | ||
| <div class="detail-project-books-meta detail-value-with-copy"> |
There was a problem hiding this comment.
I thought about this and started down the road of pre-computing the information for each record. But I changed course and used a pure pipe to do the transform. Rather than make a custom pipe for this particular transform, I made a generic pipe that lots me pass a lambda to perform a transform.
| @if (projectBook.projectLink != null) { | ||
| <a | ||
| [href]="projectBook.projectLink" | ||
| target="_blank" | ||
| rel="noopener" | ||
| class="project-link" | ||
| > | ||
| {{ projectBook.shortName }} | ||
| </a> | ||
| } @else { | ||
| <span class="project-link">{{ projectBook.shortName }}</span> | ||
| } | ||
| } | ||
| <span class="detail-project-books-id">{{ projectBook.sfProjectId }}</span> | ||
| <app-copy [value]="projectBook.sfProjectId"></app-copy> | ||
| </div> | ||
|
|
||
| @if (projectBook.projectName != null) { | ||
| @if (projectBook.projectLink != null) { | ||
| <a | ||
| [href]="projectBook.projectLink" | ||
| target="_blank" | ||
| rel="noopener" | ||
| class="project-link" | ||
| > | ||
| {{ projectBook.projectName }} | ||
| </a> | ||
| } @else { | ||
| <span class="project-link">{{ projectBook.projectName }}</span> | ||
| } |
There was a problem hiding this comment.
I changed servalAdminProjectLinkFor to return type string when the input is string, and undefined when the input is undefined. I removed some @ifs in the template.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3888 +/- ##
=======================================
Coverage 81.03% 81.03%
=======================================
Files 630 631 +1
Lines 40677 40688 +11
Branches 6601 6580 -21
=======================================
+ Hits 32962 32973 +11
Misses 6694 6694
Partials 1021 1021 ☔ View full report in Codecov by Sentry. |
The Inputs card lists training books and translation books. It also indicates the project they are from. This patch changes the display to include the SF project ID with a copy icon, to make it easy to get the ID for the referenced project.
RaymondLuong3
left a comment
RaymondLuong3
left a comment
There was a problem hiding this comment.
It looks like there are merge conflicts, but otherwise it looks good.
@RaymondLuong3 reviewed 6 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on marksvc).
The Inputs card lists training books and translation books. It also
indicates the project they are from. This patch changes the display to
include the SF project ID with a copy icon, to make it easy to get the
ID for the referenced project.
Screenshot of previous display:
Screenshot showing Input card, where you can see that there is now SF project IDs shown for the input projects, with a copy icon.
This change is