SS-851: Stats Module — UserId on Records, Supervisor Flow, and Code Quality by hrandhawa13 · Pull Request #83 · bcgov/unified-scheduling

hrandhawa13 · 2026-05-05T12:16:52Z

Pull Request for JIRA Ticket: https://jira.justice.gov.bc.ca/browse/SS-932

Description

Adds UserId to stat records so each record is associated with the employee it belongs to (not just the audit creator). Introduces a supervisor flow that allows supervisors to enter
time on behalf of other users at a selected location. Also exposes roles from the auth endpoint directly, replacing fragile hardcoded claim type URLs on the frontend.

Changes

Data Model

Added UserId (Guid) and User navigation property to StatRecord
Added FK constraint (DeleteBehavior.Restrict) and index on UserId in StatRecordConfiguration
Added comment on StatRecord.Value documenting the numeric-only assumption
Migration required: dotnet ef migrations add AddUserIdToStatRecord --project db --startup-project api/Unified.Api

API — Stats

StatRecordRequest now requires UserId; added NotEmpty validation
StatRecordService.CreateBatchAsync enforces that non-supervisors can only submit records for themselves (looks up caller by IdirName against DB); supervisors bypass the check
entirely (no unnecessary DB query)
UnauthorizedAccessException is now thrown from the service and caught by GlobalExceptionHandler → HTTP 403, removing the manual try/catch from the controller
StatRecordsController.CreateBatch accepts IReadOnlyList (bound once by model binding; no redundant .ToList() in the service)
Removed the Generate test data endpoint from StatRecordsController
All response DTOs (StatCategoryResponse, StatGroupResponse, StatMetricResponse, StatRecordResponse, StatSignoffResponse, SubCategoryMetricResponse, SubCategoryResponse) converted
from primary constructor style to property-style init records

API — User Management

UserQueryParams gains IdirName (exact match) and LocationId (home location) filters on GET /api/users
UserInfo record gains IReadOnlyList Roles, populated from ClaimTypes.Role claims in AuthController
AuthController.GetUserInfo materializes User.Claims once instead of iterating twice

Infrastructure

GlobalExceptionHandler now handles UnauthorizedAccessException → HTTP 403 ProblemDetails

Frontend

auth store: added currentUser (UserResponse), isSupervisor computed (reads from userInfo.roles), setCurrentUser, updated clearUserInfo
Router auth guard: after login, fetches the current user by IdirName and stores in auth store
EnterHoursView: supervisors see an Employee picker that populates from the selected location's users; location change immediately clears the picker and reloads; stale concurrent
fetch responses are discarded via an activeLocationId guard
getApiUsersParams model: added IdirName and LocationId query params
userInfo model: added roles: string[]
stats.ts API layer: added userId to StatRecordRequest and StatRecordResponse

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

HemanthKona · 2026-05-05T14:28:25Z

    const { data: userInfo } = await getApiAuthUser();

    authStore.setUserInfo(userInfo.value);

    if (userInfo.value?.isAuthenticated) {
+      if (userInfo.value.name) {
+        const { data: users } = await getApiUsers({ IdirName: userInfo.value.name });
+        authStore.setCurrentUser(users.value?.[0] ?? null);
+      }


getApiAuthUser endpoint can return the current user details. We can modify the UserInfo response to return the DB.User.Id, seems like that is what is not being returned from UserInfo. I'm currently working on claimsTransformer so I can add the User Id from Db in claims so we can return that in the UserInfo.

https://github.com/bcgov/unified-scheduling/blob/SS-897-role-permissions/api/Unified.Authorization/Claims/PermissionClaimsTransformer.cs#L24

HemanthKona · 2026-05-05T14:32:45Z

 export type OriginalGetApiUsersParams = {
  Search?: string;
  IsEnabled?: boolean;
+  IdirName?: string;


We are usually using IdirId instead of IdirName. We probably don't need to query by IdirName Once we implement returning User.Id in UserInfo.

See my other comment about returning DB.User.Id in UserInfo.

HemanthKona · 2026-05-05T14:37:43Z

+  const resolvedUserId = authStore.isSupervisor ? selectedUserId.value : authStore.currentUser?.id;
+  if (!resolvedUserId) {
+    errors['user'] = authStore.isSupervisor
+      ? 'Please select a user to submit hours for.'
+      : 'Unable to determine current user. Please refresh and try again.';
+  }
+


Point for discussion: Wondering if we should add a permission, instead of checking role directly. To be consistent and flexible, if we have permissions then if in the future rules change then we do not have to change code, we just assign a permission to a different role.

cc: @BronzBierd

Yes please stick to only checking permissions.

this way we don't increase permutations checking for role + permisssion combinations. This doesn't scale well and we have terrible examples to scare any developer.

devinleighsmith · 2026-05-14T05:02:56Z

    [Required]
    public string PeriodType { get; set; } = string.Empty;

+    public Guid UserId { get; set; }


this is a non-null value so, this may be a breaking change if there are pre-existing StatRecord entities in the db.

alternatively, you can make this nullable public Guid? UserId { get; set; }

devinleighsmith · 2026-05-14T05:06:27Z

@@ -60,10 +60,27 @@
    }

    public async Task<IReadOnlyCollection<StatRecordResponse>> CreateBatchAsync(


it seems odd that only the batch method would enforce auth in this manner, should CreateAsync and UpdateAsync also enforce this? If so, a shared function would be desirable to avoid code duplication.

devinleighsmith · 2026-05-14T05:07:40Z

        CancellationToken cancellationToken = default
    )
    {
        var entity = MapToEntity(request);


similar issue exists here, were the user id from the request is being trusted and not checked against the logged-in user id.

devinleighsmith · 2026-05-14T16:33:24Z

+                .Select(u => u.Id)
+                .SingleOrDefaultAsync(cancellationToken);
+
+            if (requests.Any(r => r.UserId != callerId))


just a question, is the user's home location relevant for this? ie. can a user submit stat records for any location?

devinleighsmith · 2026-05-14T16:39:40Z

  selectedLocationId.value = value !== null && value !== undefined ? Number(value) : null;
+  if (authStore.isSupervisor) {
+    selectedUserId.value = null;
+    locationUsers.value = [];


should this be added here?

activeLocationId = selectedLocationId.value;

devinleighsmith · 2026-05-14T16:50:56Z

+            if (string.IsNullOrWhiteSpace(callerIdirName))
+                throw new UnauthorizedAccessException("Only supervisors can submit records on behalf of another user.");
+
+            var callerId = await db


Id be tempted to have a user service handle these user queries, (possibly by a call at the controller level). Otherwise if we have multiple service methods that need to do user checking they are all going to have to duplicate this code.

devinleighsmith · 2026-05-14T16:51:46Z

+        var result = await service.CreateBatchAsync(
+            requests,
+            User.Identity?.Name ?? string.Empty,
+            User.IsInRole("Supervisor"),


nit: could have an enum for the available roles instead of hardcoded strings.

init

b470722

hrandhawa13 self-assigned this May 5, 2026

hrandhawa13 added 2 commits May 5, 2026 12:52

update

a99b653

update

9c40e37

HemanthKona requested changes May 5, 2026

View reviewed changes

devinleighsmith reviewed May 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SS-851: Stats Module — UserId on Records, Supervisor Flow, and Code Quality #83

SS-851: Stats Module — UserId on Records, Supervisor Flow, and Code Quality #83
hrandhawa13 wants to merge 3 commits into
mainfrom
feat/SS-932

hrandhawa13 commented May 5, 2026 •

edited

Loading

Uh oh!

HemanthKona May 5, 2026

Uh oh!

HemanthKona May 5, 2026

Uh oh!

HemanthKona May 5, 2026

Uh oh!

BronzBierd May 6, 2026

Uh oh!

devinleighsmith May 14, 2026

Uh oh!

devinleighsmith May 14, 2026

Uh oh!

devinleighsmith May 14, 2026

Uh oh!

devinleighsmith May 14, 2026

Uh oh!

devinleighsmith May 14, 2026

Uh oh!

devinleighsmith May 14, 2026

Uh oh!

devinleighsmith May 14, 2026

Uh oh!

devinleighsmith May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -60,10 +60,27 @@
		}

		public async Task<IReadOnlyCollection<StatRecordResponse>> CreateBatchAsync(

Conversation

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

Uh oh!

Pull Request for JIRA Ticket: https://jira.justice.gov.bc.ca/browse/SS-932

Description

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hrandhawa13 commented May 5, 2026 •

edited

Loading