Cleanup UI on primary pages - #88
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 54 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request adds shared Svelte controls, centralized application and icon utilities, responsive navigation, dashboard aggregates, filtered administration queries, and card-based administration views. It also updates relative-time display, secure values, tooltips, and theme-aware link colors. ChangesShared foundations
Navigation and dashboard
Administration query filtering
Administration card views
About version display
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant AdminPage
participant ServerAction
participant Prisma
Browser->>AdminPage: submit search or application filter
AdminPage->>ServerAction: enhanced POST request
ServerAction->>Prisma: query filtered records
ServerAction->>Prisma: count filtered records
Prisma-->>ServerAction: records and count
ServerAction-->>AdminPage: update cards and pagination
AdminPage-->>Browser: render responsive administration view
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FyreByrd
marked this pull request as ready for review
August 12, 2026 15:07
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
src/routes/(ui)/+layout.server.ts (1)
5-13: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRun independent Prisma queries concurrently.
Each load waits for unrelated database queries before it starts the next query. When these loads run, total latency becomes the sum of all query durations. Start the queries with
Promise.all.
src/routes/(ui)/+layout.server.ts#L5-L13: run the fivecount()calls concurrently, then constructcountfrom the resolved values.src/routes/(ui)/+page.server.ts#L5-L27: run the fourgroupBy()calls concurrently, then constructaggregatefrom the resolved values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/`(ui)/+layout.server.ts around lines 5 - 13, Run the independent Prisma queries concurrently: in src/routes/(ui)/+layout.server.ts lines 5-13, update the load result construction to await all five client, project, job, build, and release count() calls via Promise.all, then map the resolved values into count; in src/routes/(ui)/+page.server.ts lines 5-27, likewise await all four groupBy() calls via Promise.all and construct aggregate from their resolved values.src/lib/server/models/job.ts (1)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep
Job.AppTypealigned with the canonical application list.
src/lib/valibot.ts:124-129is the sharedapplicationTypessource, but this object repeats the four values.satisfies Record<string, ApplicationType>checks value membership only. It does not ensure that every canonical application has a namedJob.AppTypemember.Derive both definitions from one mapping or add an exhaustive key and value check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/server/models/job.ts` around lines 5 - 10, Update AppType in the job model to stay synchronized with the canonical applicationTypes mapping from valibot, ensuring every canonical application has a named member and no duplicated values can drift. Prefer deriving both definitions from one shared mapping, or add an exhaustive key-and-value consistency check while preserving the existing AppType member names.src/routes/(ui)/client-admin/+page.svelte (1)
29-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe declared
countin the result cast is never used.
onUpdatetypesdata.query.countbut assigns onlyclients, and the totals on lines 52-58 and 97 still readdata.count. This page has no search control, so the total cannot change and the current behaviour is correct. For consistency with the other four list pages, either track a reactivecountor dropcountfrom the cast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/`(ui)/client-admin/+page.svelte around lines 29 - 37, Update the onUpdate handler’s FormResult cast to remove the unused query.count field, since this page only assigns data.query.data to clients and continues reading the stable page-level data.count for totals.src/routes/(ui)/client-admin/+page.server.ts (1)
22-26: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRun the list query and the count in parallel.
findManyresolves beforecountstarts, so the load waits for two sequential round trips.♻️ Proposed change
export const load = (async () => { - const clients = await prisma.client.findMany({ select, take: 20, orderBy: { id: 'desc' } }); + const [clients, count] = await Promise.all([ + prisma.client.findMany({ select, take: 20, orderBy: { id: 'desc' } }), + prisma.client.count() + ]); return { clients, - count: await prisma.client.count(), + count,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/`(ui)/client-admin/+page.server.ts around lines 22 - 26, Update the load function to start prisma.client.findMany and prisma.client.count concurrently, await both results together, and return the existing clients and count fields without changing their query options or response shape.src/routes/(ui)/job-admin/+page.svelte (1)
92-92: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBoth card views cast a database string to
ApplicationTypewithout a check.getAppIconreturns''for an unregistered type, persrc/lib/icons/index.tslines 8-10. An emptysrcmakes the browser re-request the current document and renders a broken image.
src/routes/(ui)/job-admin/+page.svelte#L92-L92: resolve the icon into a{@const}and render the<img>only when the value is non-empty.src/routes/(ui)/project-admin/+page.svelte#L91-L95: apply the same guard around the project application icon.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/`(ui)/job-admin/+page.svelte at line 92, In src/routes/(ui)/job-admin/+page.svelte lines 92-92, resolve getAppIcon(job.app_id as ApplicationType) into a {`@const`} and render the image only when the resolved icon is non-empty. Apply the same guarded icon resolution and conditional rendering in src/routes/(ui)/project-admin/+page.svelte lines 91-95 for the project application icon.src/routes/(ui)/build-admin/+page.svelte (1)
89-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe status badge markup is duplicated verbatim. Both pages inline the same
result || statusderivation, the same four-way class chain, and the sameIcons.Unknowncheck. Any later change to the status colours must be applied twice.
src/routes/(ui)/build-admin/+page.svelte#L89-L108: move this block into a sharedStatusBadge.sveltecomponent that acceptsstatusand renders the badge and icon.src/routes/(ui)/release-admin/+page.svelte#L89-L108: replace the inlined block with the shared component.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/`(ui)/build-admin/+page.svelte around lines 89 - 108, Extract the duplicated status badge markup into a shared StatusBadge.svelte component accepting status and preserving the existing status-to-class mapping and Icons.Unknown handling. Update src/routes/(ui)/build-admin/+page.svelte lines 89-108 to use the component, and replace the equivalent inline block in src/routes/(ui)/release-admin/+page.svelte lines 89-108 with the same component.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/components/SecureDisplay.svelte`:
- Around line 25-27: Update the visibility toggle button in SecureDisplay to
include a state-specific aria-label and an aria-pressed attribute derived from
visible, while preserving the existing toggle handler and icon behavior.
In `@src/lib/utils/sorting.ts`:
- Around line 12-14: Update byString to explicitly handle null or undefined
inputs before localeCompare, using a consistent deterministic ordering for
missing values regardless of argument position. Preserve locale-aware comparison
for two present strings and ensure equal missing values compare as equal.
In `@src/routes/`(ui)/+layout.svelte:
- Around line 101-106: Replace the non-focusable <label> mobile navigation
control with a keyboard-operable <button> in the layout’s drawer toggle,
preserving its styling and hamburger icon. Bind the button’s aria-expanded
attribute to the mobile drawer state and retain the existing drawer target
behavior.
In `@src/routes/`(ui)/+page.svelte:
- Around line 46-50: Update the aggregate table’s <thead> row to give both
columns meaningful header text, identifying the type and count columns. Use
visible labels or screen-reader-only text while preserving the existing table
structure.
- Line 33: Update the cards container near the `id="cards"` element to use
`md:flex-row` instead of `lg:flex-row`, and replace the empty aggregate table
headers with `App ID` or `Result` and `Count`, adding `scope="col"` to each
header.
In `@src/routes/`(ui)/about/+page.svelte:
- Around line 66-67: Validate appVersion.appName against applicationTypes before
passing it to getAppIcon in the IconContainer; remove the unsafe type assertion
and use the established fallback icon behavior for unsupported values.
- Around line 19-20: Update the data-fetching query used by the page to order
appVersion records by updated descending before the results are consumed with
.at(0) at both call sites. Ensure the first record is consistently the newest
while preserving the existing getRelativeTime behavior.
In `@src/routes/`(ui)/build-admin/+page.svelte:
- Around line 22-39: Reset pagination before filtered list submissions: in
src/routes/(ui)/build-admin/+page.svelte lines 22-39, add a search() wrapper
that sets $form.page.page to 0 before submit(), and pass it to SearchBar and the
Enter handler; apply the same wrapper in src/routes/(ui)/job-admin/+page.svelte
lines 28-32 and src/routes/(ui)/project-admin/+page.svelte lines 27-31, also
resetting the page when appType changes; in
src/routes/(ui)/release-admin/+page.svelte lines 25-29, use the wrapper for the
search control.
In `@src/routes/`(ui)/client-admin/+page.server.ts:
- Around line 8-20: Remove access_token from the Prisma clientSelect projection
used by the list page so tokens are not included in load payloads, SSR HTML, or
action responses; keep token retrieval limited to the single-record view or
replace it with a server-generated masked prefix.
In `@src/routes/`(ui)/job-admin/+page.svelte:
- Around line 93-99: Update the job request link rendering in the page markup to
check whether env.PUBLIC_SCRIPTORIA_URL is set; render the existing anchor with
its URL and external-link icon only when present, otherwise render
job.request_id as plain text.
---
Nitpick comments:
In `@src/lib/server/models/job.ts`:
- Around line 5-10: Update AppType in the job model to stay synchronized with
the canonical applicationTypes mapping from valibot, ensuring every canonical
application has a named member and no duplicated values can drift. Prefer
deriving both definitions from one shared mapping, or add an exhaustive
key-and-value consistency check while preserving the existing AppType member
names.
In `@src/routes/`(ui)/+layout.server.ts:
- Around line 5-13: Run the independent Prisma queries concurrently: in
src/routes/(ui)/+layout.server.ts lines 5-13, update the load result
construction to await all five client, project, job, build, and release count()
calls via Promise.all, then map the resolved values into count; in
src/routes/(ui)/+page.server.ts lines 5-27, likewise await all four groupBy()
calls via Promise.all and construct aggregate from their resolved values.
In `@src/routes/`(ui)/build-admin/+page.svelte:
- Around line 89-108: Extract the duplicated status badge markup into a shared
StatusBadge.svelte component accepting status and preserving the existing
status-to-class mapping and Icons.Unknown handling. Update
src/routes/(ui)/build-admin/+page.svelte lines 89-108 to use the component, and
replace the equivalent inline block in
src/routes/(ui)/release-admin/+page.svelte lines 89-108 with the same component.
In `@src/routes/`(ui)/client-admin/+page.server.ts:
- Around line 22-26: Update the load function to start prisma.client.findMany
and prisma.client.count concurrently, await both results together, and return
the existing clients and count fields without changing their query options or
response shape.
In `@src/routes/`(ui)/client-admin/+page.svelte:
- Around line 29-37: Update the onUpdate handler’s FormResult cast to remove the
unused query.count field, since this page only assigns data.query.data to
clients and continues reading the stable page-level data.count for totals.
In `@src/routes/`(ui)/job-admin/+page.svelte:
- Line 92: In src/routes/(ui)/job-admin/+page.svelte lines 92-92, resolve
getAppIcon(job.app_id as ApplicationType) into a {`@const`} and render the image
only when the resolved icon is non-empty. Apply the same guarded icon resolution
and conditional rendering in src/routes/(ui)/project-admin/+page.svelte lines
91-95 for the project application icon.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e443b5b-f223-43b2-823f-234c12d25d8c
⛔ Files ignored due to path filters (4)
src/lib/icons/app-builders/dictionaryappbuilder.svgis excluded by!**/*.svgsrc/lib/icons/app-builders/keyboardappbuilder.svgis excluded by!**/*.svgsrc/lib/icons/app-builders/readingappbuilder.svgis excluded by!**/*.svgsrc/lib/icons/app-builders/scriptureappbuilder.svgis excluded by!**/*.svg
📒 Files selected for processing (34)
src/app.csssrc/lib/components/AppTypeSelector.sveltesrc/lib/components/Dropdown.sveltesrc/lib/components/IconContainer.sveltesrc/lib/components/SearchBar.sveltesrc/lib/components/SecureDisplay.sveltesrc/lib/components/SortTable.sveltesrc/lib/components/Tooltip.sveltesrc/lib/icons/ArrowDownIcon.sveltesrc/lib/icons/ArrowUpIcon.sveltesrc/lib/icons/IconContainer.sveltesrc/lib/icons/index.tssrc/lib/server/job-executors/system.tssrc/lib/server/models/job.tssrc/lib/server/utils.tssrc/lib/utils/sorting.tssrc/lib/utils/time.tssrc/lib/valibot.tssrc/routes/(api)/project/+server.tssrc/routes/(ui)/+layout.server.tssrc/routes/(ui)/+layout.sveltesrc/routes/(ui)/+page.server.tssrc/routes/(ui)/+page.sveltesrc/routes/(ui)/about/+page.sveltesrc/routes/(ui)/build-admin/+page.server.tssrc/routes/(ui)/build-admin/+page.sveltesrc/routes/(ui)/client-admin/+page.server.tssrc/routes/(ui)/client-admin/+page.sveltesrc/routes/(ui)/job-admin/+page.server.tssrc/routes/(ui)/job-admin/+page.sveltesrc/routes/(ui)/project-admin/+page.server.tssrc/routes/(ui)/project-admin/+page.sveltesrc/routes/(ui)/release-admin/+page.server.tssrc/routes/(ui)/release-admin/+page.svelte
💤 Files with no reviewable changes (4)
- src/lib/server/utils.ts
- src/lib/icons/ArrowDownIcon.svelte
- src/lib/components/IconContainer.svelte
- src/lib/icons/ArrowUpIcon.svelte
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/`(ui)/about/+page.server.ts:
- Line 6: Update the appVersion.findMany query in the page data loader to
explicitly order nullable updated timestamps with NULL values last while
retaining descending order for dated records, ensuring the first result remains
the latest dated version.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2af633db-f5e9-4c51-86d3-278a063dd083
📒 Files selected for processing (6)
src/routes/(ui)/+page.sveltesrc/routes/(ui)/about/+page.server.tssrc/routes/(ui)/build-admin/+page.sveltesrc/routes/(ui)/job-admin/+page.sveltesrc/routes/(ui)/project-admin/+page.sveltesrc/routes/(ui)/release-admin/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (4)
- src/routes/(ui)/build-admin/+page.svelte
- src/routes/(ui)/project-admin/+page.svelte
- src/routes/(ui)/release-admin/+page.svelte
- src/routes/(ui)/job-admin/+page.svelte
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Todo:
PUBLIC_SCRIPTORIA_URLto construct a link to the product in Scriptoria:Changes:
Summary by CodeRabbit
New Features
Style