#4873 - Ministry: ability to delete restrictions - UI Changes #5205

andrewsignori-aot · 2025-09-19T20:28:46Z

API Related Changes

Renamed the role student-delete-restrictions to student-delete-restriction to comply with the business AC.
Created a new endpoint to soft-delete a restriction.
Changed endpoints to return resolvedAt, resolvedBy, deletedAt, deletedBy.
- Some DTOs are shared between student and institution restrictions. The DTO properties were renamed for institutions, but they keep returning data from the modifier/updatedAt as before.
Ministry endpoints returning information to the student restriction UI were adapted to execute the queries, including soft-deleted records.

UI Related Changes

The status chip that displays restriction status was adapted to show the deleted status. For areas of the UI that will never display deleted restrictions (e.g., restriction bypasses), the API was not changed to return properties like deletedAt; the only purpose would be to display the correct status for records that will not be shown.
Update the restrictions list to show resolved information from the new fields.
Included the delete button for provincial restrictions only.
Returned ApiProcessError for possible concurrency error while deleting a restriction that is already deleted.

New deleted status for the Ministry

Please note that the restriction badge on the top, as shown below, does not consider the deleted records (as expected).

Resolved and later deleted restriction

Never resolved, and deleted restriction

Resolved restriction getting data from the new columns

Feferal restriction does not have "Delete" available

New delete modal

Copilot

Pull Request Overview

This PR implements the ability for ministry users to delete provincial student restrictions through both API and UI changes. The implementation includes soft-deletion functionality with audit trails and proper permission controls.

Adds soft-delete capability for provincial restrictions with note requirements and audit tracking
Updates UI components to display restriction status including deleted state and show delete actions
Renames role from plural to singular form to comply with business requirements

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
StudentRestrictions.vue	Adds delete restriction capability prop
Restrictions.vue	Simplifies status chip usage for institution restrictions
roles.ts	Renames role from plural to singular form
RestrictionContract.ts	Adds Deleted status enum value
Restriction.dto.ts	Updates DTOs with resolved/deleted tracking fields
RestrictionApi.ts	Adds delete restriction API method
RestrictionService.ts	Adds delete restriction service method
useRestriction.ts	Updates restriction status logic to handle deleted state
StatusChipRestriction.vue	Changes from status prop to isActive/deletedAt props
StudentRestrictions.vue	Implements delete functionality with modal and permission checks
ViewRestriction.vue	Updates to show resolved/deleted information from new fields
HistoryBypassedRestrictions.vue	Updates status chip usage
student-restriction-shared.service.ts	Removes error constants (moved to shared location)
error-code.constants.ts	Adds restriction-related error constants
student-restriction.service.ts	Implements delete restriction logic with concurrency handling
institution-restriction.service.ts	Updates import for moved error constant
restriction.controller.service.ts	Updates to return new resolved/deleted fields
restriction.aest.controller.ts	Adds delete endpoint with proper error handling
restriction.dto.ts	Adds delete restriction DTO
error-code.constants.ts	Adds restriction deletion error constants
roles.enum.ts	Updates role name to singular form

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

sources/packages/backend/apps/api/src/services/restriction/student-restriction.service.ts

sources/packages/web/src/components/common/students/StudentRestrictions.vue

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…i' of https://github.com/bcgov/SIMS into feature/#4873-ministry-ability-to-delete-restrictions-ui

Copilot

Pull Request Overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

sources/packages/web/src/components/generic/StatusChipRestriction.vue

sources/packages/web/src/components/common/students/StudentRestrictions.vue

sources/packages/backend/apps/api/src/services/restriction/student-restriction.service.ts

dheepak-aot · 2025-09-22T16:34:43Z

sources/packages/backend/apps/api/src/route-controllers/restriction/models/restriction.dto.ts

-  updatedBy: string;
+  resolvedBy: string;
+  deletedBy?: string;
+  deletedAt?: Date;


Minor. deletedAt comes from the summary API out DTO.

dheepak-aot · 2025-09-22T16:39:05Z

...s/packages/backend/apps/api/src/route-controllers/restriction/restriction.aest.controller.ts

+  @ApiUnprocessableEntityResponse({
+    description: "Provincial restriction is already set as deleted.",
+  })
+  @Patch("student/:studentId/student-restriction/:studentRestrictionId/delete")


I can see that existing APIs use student id on APIs that resolve and get restriction detail.
Are we keeping student id here to follow the existing APIs? or is there a different reason?

Following the existing API pattern.

dheepak-aot · 2025-09-22T17:00:50Z

sources/packages/backend/apps/api/src/services/restriction/student-restriction.service.ts

+    await this.dataSource.transaction(async (transactionalEntityManager) => {
+      const now = new Date();
+      const auditUser = { id: auditUserId } as User;
+      const note = await this.noteSharedService.createStudentNote(
+        studentId,
+        NoteType.Restriction,
+        noteDescription,
+        auditUserId,
+        transactionalEntityManager,
+      );
+      const updateResult = await transactionalEntityManager
+        .getRepository(StudentRestriction)
+        .update(
+          { id: restriction.id, deletedAt: IsNull() },
+          {
+            deletionNote: note,
+            deletedAt: now,
+            deletedBy: auditUser,
+            modifier: auditUser,
+            updatedAt: now,
+          },
+        );


There was a discussion about setting isActive to false while deleting a restriction(to reduce legacy integration side effects). Is it dropped to keep the deletion process simple?

Not dropped, but not part of this ticket ACs. After multiple requests from DEVs to have it added, it ended up not being added here. Hence, I believe that it will be part of the other one.

dheepak-aot · 2025-09-22T17:15:33Z

...s/packages/backend/apps/api/src/route-controllers/restriction/restriction.aest.controller.ts

+      resolvedAt: institutionRestriction.updatedAt,
      createdBy: getUserFullName(institutionRestriction.creator),
-      updatedBy: getUserFullName(institutionRestriction.modifier),
+      // Currently mapping modifier to resolvedBy as there is no resolvedBy for
+      // institutions restrictions but the API shares the same DTO.
+      resolvedBy: getUserFullName(institutionRestriction.modifier),


dheepak-aot · 2025-09-22T17:22:22Z

...ackages/backend/apps/api/src/route-controllers/restriction/restriction.controller.service.ts

+    if (options?.extendedDetails) {
+      return {
+        ...restrictionDetail,
+        resolvedAt: studentRestriction.resolvedAt,
+        deletedAt: studentRestriction.deletedAt,
+        resolvedBy: getUserFullName(studentRestriction.resolvedBy) || undefined,
+        deletedBy: getUserFullName(studentRestriction.deletedBy) || undefined,
+        resolutionNote: studentRestriction.resolutionNote?.description,
+        deletionNote: studentRestriction.deletionNote?.description,
+      };
+    }


dheepak-aot · 2025-09-22T17:30:03Z

...ckages/web/src/components/aest/students/bypassedRestrictions/HistoryBypassedRestrictions.vue

            {{ item.restrictionCode }}
          </template>
          <template #[`item.restrictionStatus`]="{ item }">
-            <status-chip-restriction


What happens to a bypass record when the student restriction associated to that is deleted? this was something we weren't sure about how typeorm is handling it.

Will be investigated and shared, not part of this PR.

sources/packages/web/src/components/common/restriction/ViewRestriction.vue

dheepak-aot · 2025-09-22T17:54:12Z

sources/packages/web/src/components/common/restriction/ViewRestriction.vue

-        (props.restrictionData.isActive ||
-          props.restrictionData.resolutionNote),
-    );
+        (props.restrictionData.isActive || props.restrictionData.resolutionNote)


.restrictionData.isActive and restrictionData.resolutionNote are mutually exclusive.

Condition can be appended as (props.restrictionData.resolutionNote || !restrictionData.deletedAt) which will achieve the same?

The mentioned code was already in place. I believe that it is better to read by keeping the existing logic and dealing with the deleted logic at the top.
Not willing to put effort into this.

By effort, if you are saying the effort to validate different situations, it's fair enough. It is not a blocker as the code here works.

Yes, this code was the same code that had issues recently. Effort-wise and I did not see an improvement in the suggested code, hence keeping it as it is since it was not considered as a blocker.

dheepak-aot · 2025-09-22T17:58:05Z

sources/packages/web/src/components/common/students/StudentRestrictions.vue

+                  @click="viewStudentRestriction(slotProps.data.restrictionId)"
+                  >View</v-btn
+                >
+                <check-permission-role :role="Role.StudentDeleteRestriction">


Minor. the vif can be added to check-permission-role to avoid check permission role component from being created.

Moved v-if.

I am not able to see the changes yet.

Missing a push, it should be updated now.

dheepak-aot · 2025-09-22T18:08:06Z

sources/packages/web/src/services/http/dto/Restriction.dto.ts

-  updatedBy: string;
+  resolvedBy: string;
+  deletedBy?: string;
+  deletedAt?: Date;


same here on deleteAt repeating.

Just changed it.

dheepak-aot

Great work enabling delete restrictions and other updates. I could see just 2 minor comments left.

sonarqubecloud · 2025-09-22T19:21:41Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.7% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions · 2025-09-22T19:22:16Z

Backend Unit Tests Coverage Report

Totals
Statements:	21.38% ( 4168 / 19493 )
Methods:	9.63% ( 239 / 2482 )
Lines:	24.74% ( 3609 / 14589 )
Branches:	13.21% ( 320 / 2422 )

github-actions · 2025-09-22T19:22:30Z

E2E Workflow Workers Coverage Report

Totals
Statements:	75.33% ( 974 / 1293 )
Methods:	78.08% ( 114 / 146 )
Lines:	78.25% ( 759 / 970 )
Branches:	57.06% ( 101 / 177 )

github-actions · 2025-09-22T19:25:15Z

E2E Queue Consumers Coverage Report

Totals
Statements:	86.43% ( 1548 / 1791 )
Methods:	84.62% ( 176 / 208 )
Lines:	88.75% ( 1278 / 1440 )
Branches:	65.73% ( 94 / 143 )

github-actions · 2025-09-22T19:25:51Z

E2E SIMS API Coverage Report

Totals
Statements:	74.34% ( 7571 / 10184 )
Methods:	72.13% ( 924 / 1281 )
Lines:	77.8% ( 5868 / 7542 )
Branches:	57.24% ( 779 / 1361 )

andrewsignori-aot added 6 commits September 17, 2025 13:56

Adding modal for restriction cancellation

b46247f

Minor layout adjustment to keep buttons side-by-side

2ff2fe4

API and UI changes

77b284b

Soft deleting from the restrictions list

93040f8

View restrictions adjustments

c50e0f2

Minor layout adjustment

a4deb10

andrewsignori-aot self-assigned this Sep 19, 2025

andrewsignori-aot added Institution Institution Features Ministry Ministry Features SIMS-Api SIMS-Api Web portal labels Sep 19, 2025

Constants reorganization

72cd5b6

andrewsignori-aot requested a review from Copilot September 19, 2025 22:19

Copilot AI reviewed Sep 19, 2025

View reviewed changes

sources/packages/backend/apps/api/src/services/restriction/student-restriction.service.ts Outdated Show resolved Hide resolved

sources/packages/web/src/components/common/students/StudentRestrictions.vue Outdated Show resolved Hide resolved

andrewsignori-aot and others added 3 commits September 19, 2025 15:27

Copilot suggestion for type declaration

ea28734

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Wrong deletedAt check

4b40fc7

Merge branch 'feature/#4873-ministry-ability-to-delete-restrictions-u…

8acc244

…i' of https://github.com/bcgov/SIMS into feature/#4873-ministry-ability-to-delete-restrictions-ui

andrewsignori-aot marked this pull request as ready for review September 19, 2025 22:32

andrewsignori-aot requested a review from Copilot September 19, 2025 23:01

Copilot AI reviewed Sep 19, 2025

View reviewed changes

dheepak-aot self-requested a review September 22, 2025 16:05