#2370 - Institution Read-Only User Type - PR 2 #4214
Merged
Conversation
This file contains 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
andrepestana-aot
changed the title
...ation-offering-change-request/application-offering-change-request.institutions.controller.ts
Show resolved
Hide resolved
Comment on lines
163
to
186
| /** | ||
| * Create a location with read only user access. | ||
| * This is useful for tests that need to assert that the API endpoints | ||
| * are properly restricted for read only users. | ||
| * @param db E2E testing data sources. | ||
| * @returns location the user will have read only access. | ||
| */ | ||
| export async function getReadOnlyAuthorizedLocation( | ||
| db: E2EDataSources, | ||
| institutionTokenType: InstitutionTokenTypes, | ||
| ) { | ||
| const { institution } = await getAuthRelatedEntities( | ||
| db.dataSource, | ||
| institutionTokenType, | ||
| ); | ||
| const location = createFakeInstitutionLocation({ institution }); | ||
| await authorizeUserTokenForLocation( | ||
| db.dataSource, | ||
| institutionTokenType, | ||
| location, | ||
| { institutionUserType: InstitutionUserTypes.readOnlyUser }, | ||
| ); | ||
| return location; | ||
| } |
There was a problem hiding this comment.
❤️ This method reduces the amount of duplicated codes for creating a location with read only user access in this PR, and I think it can be expanded further to create locations for other user types as well. A cleanup ticket is probably needed in the future considering there are quite a few duplicated codes like this in the E2E tests. This comment is not a blocker. Great method by the way.
There was a problem hiding this comment.
I would agree with @lewischen-aot. The method can be converted to a generic version that would return an authorized location with the InstitutionUserTypes provided. Not a blocker but it would be a nice improvement 😉
lewischen-aot
approved these changes
Jan 10, 2025
...end/apps/api/src/route-controllers/education-program/education-program.controller.service.ts
Show resolved
Hide resolved
sources/packages/backend/apps/api/src/testHelpers/auth/institution-auth-helpers.ts
Show resolved
Hide resolved
|
Backend Unit Tests Coverage Report
|
E2E Workflow Workers Coverage Report
|
E2E Queue Consumers Coverage Report
|
E2E SIMS API Coverage Report
|
andrewsignori-aot
approved these changes
Jan 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
@HasLocationAccess()to require the user to haveInstitutionUserTypes.userfor that location;InstitutionUserTypes.userorInstitutionUserTypes.adminfor any location and deny access to "read-only" users as education programs are at institution level;authorizeUserTokenForLocationto optionally receive the InstitutionUserTypes for the user;