Skip to content

Conversation

@andrewsignori-aot
Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot commented Nov 29, 2025

PR Scoppe

  • PR intended to be the first part of the UI changes for the ticket.
  • Only methods in the frontend/backend related to adding the new institution restriction were changed.
  • Viewing the data, updating the restrictions list, and resolving the restrictions are not part of this PR.

Add new restrictions modal

image image

Data load

  • Reasons (a.k.a. restrictions descriptions), program list, and locations list are loaded on demand only when the modal is shown.

Locations

  • For simplicity, reused the method allInstitutionLocations to get all locations available for an institution. The method does retrieve more data than required, but this was not considered an issue since it should return very few records ( 1 to 3 most of the time, maybe 15 for an exceptional case).

Program list:

  • Enabled the same endpoint, implemented as getProgramsListForInstitutions for institutions. The method was kept the same to be "client shared" by the Vue application. If there is a strong reason to create a different method from the institution's one, please share the motivation, and we can have it adjusted.
  • The current method was already returning approved and active programs, as per this requirement.
  • Enabled only client-side filtering since the amount of data expected to be loaded and the number of times the feature would be used do not seem strong enough to execute a server-side search.

Restrictions descriptions (a.k.a. reasons)

  • Endpoint method getRestrictionReasons received a minor refactor to allow the retrieval of institution restrictions.

Post New Restriction

  • Entity model institution.model.ts got new shortcut properties to allow a single query to retrieve the data to be validated.
  • The validation of the existence of an active restriction was treated as a possible user message to cover a possible race condition where two users would enter the same data, or if the system DB is under stress.

E2E Tests

RestrictionAESTController(e2e)-addInstitutionRestriction.
Should throw a bad request exception when no locations were provided. (99 ms)
√ Should add multiple institution restrictions when a valid payload with multiple locations IDs is submitted.
√ Should create the institution restriction when there is already an institution restriction, but it is inactive.
√ Should throw an unprocessable entity exception when an active institution restriction already exists for at least one location.
√ Should throw a not found exception when the provided institution is not found.
√ Should throw an unprocessable entity exception when the location does not exist associated with the institution.
√ Should throw an unprocessable entity exception when the program does not exist associated with the institution.
√ Should throw an unprocessable entity exception when the restriction ID does not exist.
√ Should throw a forbidden exception when the user does not have permission.
√ Should throw a bad request exception when no locations were provided.
√ Should throw a bad request exception when too many locations are provided.

Outside PR scope

  • Adjusted the modal min width and removed the max/min when in fullscreen.

@andrewsignori-aot andrewsignori-aot changed the title Feature/#4968 institution program restrictions UI #4968 - Institution Program Restrictions: Suspension - UI - Part 1 Dec 1, 2025

This comment was marked as resolved.

This comment was marked as outdated.

@andrewsignori-aot andrewsignori-aot marked this pull request as draft December 2, 2025 18:07
Comment on lines +12 to +13
:max-width="showFullScreen ? undefined : maxWidth"
:min-width="showFullScreen ? undefined : minWidth"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to verify, is smAndDown(showFullScreen) reactive and not required to use through computed property?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should be since the UI is adapting as expected and the previously existing smAndDown was assigned to the v-dialog fullscreen property.

item-value="id"
item-title="name"
class="mb-4"
label="Location"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From business perspective shouldn't it be called as Locations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will raise the question to biz. If they answer in time for this ticket, I will change; otherwise, this is following the ACs, and we should move forward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question raised: #4968 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted to Location(s). There is one "AC" updated in the ticket now.

* Every time that a user login to the system check is some of the readonly
* information (that must be changed on BCeID) changed.
* @param userId user id.
* @param bceidUserName user name on BCeID.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside the PR, I noticed this service skipping a category keeping institution restrictions in consideration.

Image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will adjust this and the UI component in the upcoming PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 3, 2025

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 20.62% ( 4294 / 20828 )
Methods: 9.87% ( 252 / 2554 )
Lines: 24.8% ( 3676 / 14822 )
Branches: 10.6% ( 366 / 3452 )

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 75.39% ( 1054 / 1398 )
Methods: 79.31% ( 115 / 145 )
Lines: 78.77% ( 768 / 975 )
Branches: 61.51% ( 171 / 278 )

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 85.47% ( 1559 / 1824 )
Methods: 84.76% ( 178 / 210 )
Lines: 88.39% ( 1241 / 1404 )
Branches: 66.67% ( 140 / 210 )

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

E2E SIMS API Coverage Report

Totals Coverage
Statements: 74.78% ( 8448 / 11297 )
Methods: 74.33% ( 1002 / 1348 )
Lines: 78.92% ( 6158 / 7803 )
Branches: 60.02% ( 1288 / 2146 )

Copy link
Collaborator

@dheepak-aot dheepak-aot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. Thanks for making the changes. Looks good 👍

@andrewsignori-aot andrewsignori-aot added this pull request to the merge queue Dec 4, 2025
Merged via the queue into main with commit ddfb65f Dec 4, 2025
22 checks passed
@andrewsignori-aot andrewsignori-aot deleted the feature/#4968-institution-program-restrictions-ui branch December 4, 2025 00:14
github-merge-queue bot pushed a commit that referenced this pull request Dec 4, 2025
…splay restriction) (#5450)

## Restrictions Summary

- Added `Location` and `Programs` columns to the table and enabled
sorting for them.
- Sorting on column `Status` was kept in order to allow it to display
all `Active` items at the top, following the same behavior that was in
place and the same as the students' restrictions.

<img width="1278" height="660" alt="image"
src="https://github.com/user-attachments/assets/268543e8-6edc-4e2b-ae06-292c193f502c"
/>

_Note:_ resolving restrictions is not part of this ticket. The value
added in the above table is a result of the DB manipulation with the
sole purpose of taking the screenshot.

<img width="733" height="523" alt="image"
src="https://github.com/user-attachments/assets/4933670e-c555-48ff-972f-5a76d85127fe"
/>

_Note:_ the only AC related to viewing a restriction is the one that
mentions "Add View Button". No work was actually done on this modal, and
it is here for the sake of demonstration that it is able to display the
data.

## Other Changes

- Included an extra E2E for institution restriction created as
[suggested during previous
PR](#5430 (comment)).
- Removed the hardcoded `Designation` institution type present on the
API and student "add restriction".
- Minor refactor in the student restrictions creation modal to remove
errors and adjust the `processing` status (`processing` was added to the
Vue template but not present at the `setup`).

## E2E Tests

RestrictionAESTController(e2e)-getReasonsOptionsList.
- √ Should throw a bad request exception when requesting federal
restrictions reasons.
- √ Should throw a bad request exception when category is not provided.
- Should get reasons options list for a restriction type when the
request is valid.
- √ Should get Provincial restrictions reasons list filtered by specific
category when Provincial restrictions are requested for a category.
- √ Should get Institution restrictions reasons list filtered by
specific category when Institution restrictions are requested for a
category.

RestrictionAESTController(e2e)-addInstitutionRestriction.
- √ Should throw an unprocessable entity exception when trying to add a
provincial restriction.

## Outside scope but open for discussion

- The method `getAllRestrictionCategories` was not refactored beyond the
minimal required to apply the accurate filter. The result is misleading
because the distinct will return the first item of a distinct group to
allow an ID to be returned, but the returned ID has no purpose, and
returning it seems to be a mistake.
- Proposed refactors:
  - Stop returning the ID;
  - Replace the `distinct on` with a regular `distinct`.
  - Pass the restriction type as a parameter.
  - Adjust the API/Web DTOs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants