-
Notifications
You must be signed in to change notification settings - Fork 13
#4877 - Reverse scholastic standing API #5045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#4877 - Reverse scholastic standing API #5045
Conversation
| default: | ||
| throw error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, the default is not required.
| SCHOLASTIC_STANDING_REVERSAL_NOT_UPDATED, | ||
| } from "../../constants"; | ||
| import { DataSource, IsNull, Repository } from "typeorm"; | ||
| /** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a blank line.
| { id: scholasticStandingId, reversalDate: IsNull() }, | ||
| { reversalDate: now, reversalBy: auditUser, reversalNote: note }, | ||
| ); | ||
| if (updateResult.affected !== 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, just a suggestion, I found the check as if (!updateResult.affected) better to read 😉
We have both ways in the solution, no need for change.
andrewsignori-aot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work with the different scenarions, looks good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a reverse scholastic standing API that allows authorized users to reverse student scholastic standing changes. The implementation handles different reversal scenarios based on the type of scholastic standing change, including creating re-assessments and updating application archive status when necessary.
Key Changes:
- Added new Keycloak role
student-reverse-scholastic-standingfor authorization - Created new service
ScholasticStandingReversalServiceto handle reversal logic - Implemented API endpoint to reverse scholastic standings with appropriate error handling
- Added comprehensive E2E tests covering various reversal scenarios
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sources/packages/web/src/types/contracts/aest/roles.ts | Added new role constant for scholastic standing reversal |
| sources/packages/backend/libs/test-utils/src/factories/student-scholastic-standing.ts | Enhanced factory to support reversal date initialization |
| sources/packages/backend/libs/test-utils/src/factories/education-program-offering.ts | Added offering versioning support for testing |
| sources/packages/backend/libs/sims-db/src/entities/education-program-offering.model.ts | Added versions relationship to track offering changes |
| sources/packages/backend/apps/api/src/services/student-scholastic-standings/scholastic-standing-reversal.service.ts | Core service implementing reversal business logic |
| sources/packages/backend/apps/api/src/route-controllers/student-scholastic-standings/student-scholastic-standings.aest.controller.ts | API endpoint for reversing scholastic standings |
| sources/packages/backend/apps/api/src/route-controllers/student-scholastic-standings/models/student-scholastic-standings.dto.ts | Request DTO for reversal payload |
| sources/packages/backend/apps/api/src/constants/error-code.constants.ts | Added error constants for reversal operations |
| sources/packages/backend/apps/api/src/auth/roles.enum.ts | Added role enum for backend authorization |
| sources/packages/backend/apps/api/src/app.aest.module.ts | Registered new service in DI container |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| if (isReAssessmentAndArchiveUpdateRequired) { | ||
| // Current version of the offering that was associated with the application before the scholastic standing was reported. | ||
| const [offeringBeforeScholasticStanding] = | ||
| scholasticStanding.referenceOffering.parentOffering.versions; |
Copilot
AI
Aug 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using array destructuring to get the first element assumes the array has at least one element, but there's no validation that versions array is not empty. This could cause runtime errors if no versions exist.
| scholasticStanding.referenceOffering.parentOffering.versions; | |
| const versions = scholasticStanding.referenceOffering.parentOffering.versions; | |
| if (!Array.isArray(versions) || versions.length === 0) { | |
| throw new CustomNamedError( | |
| "No offering versions found for the parent offering when attempting to reverse scholastic standing.", | |
| SCHOLASTIC_STANDING_REVERSAL_NOT_ALLOWED, | |
| ); | |
| } | |
| const [offeringBeforeScholasticStanding] = versions; |
|
|
||
| it( | ||
| "Should get scholastic standing summary for a student excluding the details from reversed scholastic standing(s)" + | ||
| " when a student has one ore more reversed scholastic standings.", |
Copilot
AI
Aug 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a spelling error: 'ore' should be 'or'.
| " when a student has one ore more reversed scholastic standings.", | |
| " when a student has one or more reversed scholastic standings.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dheepak-aot please review spelling check
|
bidyashish
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM



Reverse scholastic standing API
Keycloak role
student-reverse-scholastic-standingand added the role to the groupaest-business-administratorsin DEVAPI
Service
ScholasticStandingReversalServiceto handle all the operations for reversal.E2E Tests