Add workspace health summary#46
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a workspace health summary to the workspaces list page, replacing the simple check count with a status breakdown showing how many checks are Up, Warning, Down, or Inactive for each workspace. It queries the latest check result status for each enabled check per workspace.
Changes:
- Added
UpCount,WarnCount, andDownCountproperties toWorkspaceDetailsViewModeland populated them via EF Core subqueries in bothGetWorkspacesAsyncandGetWorkspaceDetailsAsync - Replaced the simple "Checks" column in the workspace index page with a "Health" column displaying color-coded badges for each status category
- Added a
@usingdirective forSAMA.Shared.Constantsin the Razor page (though it appears unused)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
SAMA.Web/Models/WorkspaceDetailsViewModel.cs |
Added UpCount, WarnCount, DownCount properties to the view model |
SAMA.Web/Services/Queries/WorkspaceQueryService.cs |
Added EF Core subqueries to compute per-status check counts based on the latest check result for each enabled check, in both query methods |
SAMA.Web/Pages/Workspaces/Index.cshtml |
Replaced plain check count column with health summary badges (Up/Warn/Down/Inactive) and updated header popover |
| UpCount = w.Checks.Count(c => c.Enabled && c.CheckResults | ||
| .OrderByDescending(r => r.CheckedAt).Select(r => r.Status).FirstOrDefault() == CheckStatuses.Up), | ||
| WarnCount = w.Checks.Count(c => c.Enabled && c.CheckResults | ||
| .OrderByDescending(r => r.CheckedAt).Select(r => r.Status).FirstOrDefault() == CheckStatuses.Warn), | ||
| DownCount = w.Checks.Count(c => c.Enabled && c.CheckResults | ||
| .OrderByDescending(r => r.CheckedAt).Select(r => r.Status).FirstOrDefault() == CheckStatuses.Down), |
There was a problem hiding this comment.
The status count subquery logic (UpCount, WarnCount, DownCount) is duplicated verbatim between GetWorkspacesAsync (lines 41-46) and GetWorkspaceDetailsAsync (lines 69-74). Consider extracting a helper method or a shared Expression that builds the WorkspaceDetailsViewModel projection from a Workspace entity to avoid maintaining identical subqueries in two places. If one is updated without the other, the counts will diverge.
| UpCount = w.Checks.Count(c => c.Enabled && c.CheckResults | ||
| .OrderByDescending(r => r.CheckedAt).Select(r => r.Status).FirstOrDefault() == CheckStatuses.Up), | ||
| WarnCount = w.Checks.Count(c => c.Enabled && c.CheckResults | ||
| .OrderByDescending(r => r.CheckedAt).Select(r => r.Status).FirstOrDefault() == CheckStatuses.Warn), | ||
| DownCount = w.Checks.Count(c => c.Enabled && c.CheckResults | ||
| .OrderByDescending(r => r.CheckedAt).Select(r => r.Status).FirstOrDefault() == CheckStatuses.Down), |
There was a problem hiding this comment.
The new UpCount, WarnCount, and DownCount properties are not covered by the existing integration tests. The test GetWorkspacesAsyncShouldIncludeAllPropertiesAndCounts creates checks but no CheckResult records, so the new status counts are only tested at their default value of 0. Similarly, GetWorkspaceDetailsAsyncShouldCountRelatedEntities doesn't assert these new properties. Adding a test that creates checks with check results in different statuses (including a disabled check) would verify that the subqueries work correctly, especially since the OrderByDescending + FirstOrDefault pattern in EF LINQ translation can be subtle.
| @@ -1,4 +1,5 @@ | |||
| @page | |||
| @using SAMA.Shared.Constants | |||
There was a problem hiding this comment.
The @using SAMA.Shared.Constants directive is unused — no types from SAMA.Shared.Constants are referenced anywhere in this Razor page. It should be removed.
| @using SAMA.Shared.Constants |
No description provided.