Skip to content

#5878 - Student Forms e2e Tests - Ministry submitItemDecision#5966

Merged
andrewsignori-aot merged 2 commits intomainfrom
feature/#5878-forms-e2e-tests-part-5
Mar 27, 2026
Merged

#5878 - Student Forms e2e Tests - Ministry submitItemDecision#5966
andrewsignori-aot merged 2 commits intomainfrom
feature/#5878-forms-e2e-tests-part-5

Conversation

@andrewsignori-aot
Copy link
Copy Markdown
Collaborator

@andrewsignori-aot andrewsignori-aot commented Mar 27, 2026

PR Goal
Add some level of API endpoints E2E tests to all new endpoints introduced during the new forms implementation. At least one scenario for each endpoint must be present, and more scenarios can be added if time allows it. Suggestions for scenarios not covered are appreciated, but will be taken care of based on criticality and time available 😉

Ministry - submitItemDecision

  • "Should submit an item decision when there is no decision yet and the user has approval authorization.
  • Should submit an item decision, creating a history record when a decision is already in place and the user has approval authorization.
  • Should throw an unprocessable entity error when submitting an item decision for an application that was edited.
  • Should throw an unprocessable entity error when submitting an item decision for a completed form submission.
  • Should throw an unprocessable entity error when submitting a student form decision and the user is not authorized for the specific form category.
  • Should throw an unprocessable entity error when submitting a student appeal decision and the user is not authorized for the specific form category.
  • Should throw a forbidden error when submitting a form decision and the user is not authorized to access the endpoint.
  • Should throw an unprocessable entity error when submitting an item decision and last updated date is different.
  • Should throw a not found error when submitting an item decision and the item does not exist.

New JWT Token Helpers

  • Created the generic mockJWTToken to allow token manipulation.
  • Created the removeJWTUserRoles using the mockJWTToken as a helper to remove existing roles from a Ministry user to test authorization scenarios. More variations can be added as needed.
  • Existing student mockJWTUserInfo is now also consuming the mockJWTToken.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds end-to-end coverage for the AEST/Ministry submitItemDecision endpoint on form submission items, along with supporting test utilities to make JWT payload manipulation and timestamp assertions reliable/deterministic.

Changes:

  • Added comprehensive e2e tests for PATCH /aest/form-submission/items/:id/decision (success + common failure scenarios).
  • Introduced a reusable mockJWTToken helper to mutate JWT payloads in e2e tests (used to refactor student JWT mocking and enable role-removal scenarios).
  • Improved input handling for decision concurrency by transforming lastUpdateDate to a Date, and stabilized test data timestamps by setting FormSubmission.createdAt.

Reviewed changes

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

Show a summary per file
File Description
sources/packages/backend/libs/test-utils/src/factories/form-submission.ts Sets createdAt from the factory “now” value to keep timestamps deterministic in tests.
sources/packages/backend/apps/api/src/testHelpers/auth/token-helpers.ts Adds mockJWTToken helper to mutate JWT payloads via JwtStrategy.validate spying.
sources/packages/backend/apps/api/src/testHelpers/auth/student-user-helpers.ts Refactors mockJWTUserInfo to reuse mockJWTToken (reduces duplicated mocking logic).
sources/packages/backend/apps/api/src/testHelpers/auth/aest-token-helpers.ts Adds removeJWTUserRoles helper for AEST authorization/role-negative test scenarios.
sources/packages/backend/apps/api/src/route-controllers/form-submission/models/form-submission.dto.ts Ensures lastUpdateDate is transformed to Date for @IsDate() validation.
sources/packages/backend/apps/api/src/route-controllers/form-submission/tests/e2e/form-submission.aest.controller.submitItemDecision.e2e-spec.ts New e2e suite validating decision submission behavior, authorization, concurrency/outdated handling, and not-found cases.

@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 20.12% ( 4578 / 22754 )
Methods: 9.47% ( 262 / 2767 )
Lines: 24.49% ( 3930 / 16045 )
Branches: 9.79% ( 386 / 3942 )

@github-actions
Copy link
Copy Markdown

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 48.27% ( 2713 / 5620 )
Methods: 38.26% ( 285 / 745 )
Lines: 55.14% ( 2050 / 3718 )
Branches: 32.67% ( 378 / 1157 )

@github-actions
Copy link
Copy Markdown

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 79.33% ( 9156 / 11542 )
Methods: 78.47% ( 1181 / 1505 )
Lines: 82.66% ( 6882 / 8326 )
Branches: 63.88% ( 1093 / 1711 )

@github-actions
Copy link
Copy Markdown

E2E SIMS API Coverage Report

Totals Coverage
Statements: 66.89% ( 13264 / 19831 )
Methods: 63.96% ( 1567 / 2450 )
Lines: 70.3% ( 9589 / 13640 )
Branches: 56.35% ( 2108 / 3741 )

@andrewsignori-aot andrewsignori-aot marked this pull request as ready for review March 27, 2026 03:05
* Date when the decision record was last updated. Used for concurrency control
* to prevent overwriting a more recent decision.
*/
@Type(() => Date)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Did not find a reason why supertest is failing the DTO validation while Vue/axios is able to submit without any problem.
The recommendation seems to have the @Type(() => Date), hence I would like to keep it.
My best guess was some difference while setting the setGlobalPipes but we use the same method in E2E tests that is also used by the API.

@dheepak-aot dheepak-aot self-requested a review March 27, 2026 20:37
};
const endpoint = `/aest/form-submission/items/${formSubmissionItemA.id}/decision`;
const token = await getAESTToken(AESTGroups.BusinessAdministrators);
await removeJWTUserRoles(appModule, [Role.StudentApproveDeclineAppeals]);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

❤️

Copy link
Copy Markdown
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.

Good work @andrewsignori-aot 👍

.expect({
message:
"The application associated with the form submission is not in completed status.",
error: "Unprocessable Entity",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should introduce a constant for this at some point

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, we are more lenient for E2E tests for these framework constants.
I am not opposed if someone starts it.

.send(payload)
.auth(token, BEARER_AUTH_TYPE)
.expect(HttpStatus.UNPROCESSABLE_ENTITY)
.expect({
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm okay with this but i've been receiving the feedback to the use the new standard:

.expect(({ body }) => ...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is it about using the expect from supertest vs the expected from jest?
If yes, the idea of using the jest one for the body return was to allow easy troubleshooting for complex objects, especially when doing maintenance in existing E2E tests. I do not see it as necessary in this scenario.
@dheepak-aot FYI

Copy link
Copy Markdown
Collaborator

@tiago-graf tiago-graf left a comment

Choose a reason for hiding this comment

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

Just a couple of minor comments

@andrewsignori-aot andrewsignori-aot added this pull request to the merge queue Mar 27, 2026
Merged via the queue into main with commit 4352c63 Mar 27, 2026
22 checks passed
@andrewsignori-aot andrewsignori-aot deleted the feature/#5878-forms-e2e-tests-part-5 branch March 27, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants