Skip to content
2 changes: 2 additions & 0 deletions sources/packages/backend/apps/api/src/app.aest.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ import {
DisbursementScheduleAESTController,
FormSubmissionAESTController,
MSFAANumberAESTController,
FormSubmissionControllerService,
} from "./route-controllers";
import { AuthModule } from "./auth/auth.module";
import {
Expand Down Expand Up @@ -248,6 +249,7 @@ import { ECertIntegrationModule } from "@sims/integrations/esdc-integration";
StudentAppealActionsProcessor,
StudentAppealCreateAssessmentAction,
StudentAppealUpdateModifiedIndependentAction,
FormSubmissionControllerService,
// Form validators.
ConfigurationContextValidator,
PendingConcurrencyValidator,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
FormSubmissionItemDecisionAPIInDTO,
FormSubmissionMinistryAPIOutDTO,
FormSubmissionPendingSummaryAPIOutDTO,
FormSubmissionsAPIOutDTO,
} from "./models/form-submission.dto";
import { getUserFullName } from "../../utilities";
import { FormSubmissionDecisionStatus } from "@sims/sims-db";
Expand All @@ -46,6 +47,7 @@ import {
FormSubmissionPendingPaginationOptionsAPIInDTO,
PaginatedResultsAPIOutDTO,
} from "../models/pagination.dto";
import { FormSubmissionControllerService } from "./form-submission.controller.service";

/**
* Roles allowed to update the form submission item decision
Expand All @@ -64,6 +66,7 @@ export class FormSubmissionAESTController extends BaseController {
constructor(
private readonly formSubmissionApprovalService: FormSubmissionApprovalService,
private readonly formSubmissionService: FormSubmissionService,
private readonly formSubmissionControllerService: FormSubmissionControllerService,
) {
super();
}
Expand Down Expand Up @@ -95,6 +98,28 @@ export class FormSubmissionAESTController extends BaseController {
};
}

/**
* Gets the list of form submissions for a student,
* including the individual form items and their details.
* @param studentId student ID to retrieve the form submission history for.
* @returns list of form submissions for a student.
*/
@Get("student/:studentId")
async getFormSubmissionHistory(
@Param("studentId", ParseIntPipe) studentId: number,
): Promise<FormSubmissionsAPIOutDTO> {
// Kept the includeBasicDecisionDetails as false since the details controlled by
// the flag are not required to be returned by this endpoint.
const submissions =
await this.formSubmissionControllerService.getFormSubmissions(studentId, {
includeBasicDecisionDetails: false,
keepPendingDecisionsWhilePendingFormSubmission: false,
Comment on lines +115 to +116
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it not as good as not passing the options?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added comments and minor refactor to getFormSubmissions hope that it helps.

});
return {
submissions,
};
}

/**
* Get the details of a form submission, including the individual form items and their details.
* @param formSubmissionId ID of the form submission to retrieve the details for.
Expand All @@ -109,11 +134,10 @@ export class FormSubmissionAESTController extends BaseController {
@Param("formSubmissionId", ParseIntPipe) formSubmissionId: number,
@Query("itemId", new ParseIntPipe({ optional: true })) itemId?: number,
): Promise<FormSubmissionMinistryAPIOutDTO> {
const submission =
await this.formSubmissionApprovalService.getFormSubmissionById(
formSubmissionId,
{ itemId },
);
const [submission] = await this.formSubmissionService.getFormSubmissions(
{ formSubmissionId, itemId },
{ includeDecisionHistory: true, loadSubmittedData: true },
);
if (!submission) {
if (itemId) {
throw new NotFoundException(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Injectable, NotFoundException } from "@nestjs/common";
import { FormSubmissionService } from "../../services";
import {
FormSubmission,
FormSubmissionDecisionStatus,
FormSubmissionItem,
FormSubmissionStatus,
Expand All @@ -16,40 +17,91 @@ export class FormSubmissionControllerService {

/**
* Get the details of a form submission, including the individual form items and their details.
* @param formSubmissionId ID of the form submission to retrieve the details for.
* @param studentId ID used to validate the access to the student data.
* @param studentId ID of the student to have the data retrieved.
* @param options.
* - `includeBasicDecisionDetails`: optional flag to include basic decision details, besides
* - `formSubmissionId` allow searching for a specific form submission. When provided, it will validate if the form
* submission belongs to the student and throw a not found HTTP error if it does not.
* - `locationIds` restrict forms with an application scope to the provided locations. Used for institutions to have access
* only to the form submissions related to the locations they have access to.
* - `keepPendingDecisionsWhilePendingFormSubmission`, when true, will return "Pending" as the decision status for all items
* if the form submission is still pending. This is used to avoid showing decisions that are not final yet while the form
* submission is not completed. Default to true when not provided to expose less information.
* - `includeBasicDecisionDetails` optional flag to include basic decision details, besides
* the decision status. Used for institutions to have access to more details than the student
* to better support them.
* - `applicationId`: optional ID of the application, used to validate the access to the form submission
* @returns form submission details.
* to better support them. Default to false when not provided to expose less information. When keepPendingDecisionsWhilePendingFormSubmission
* is true, the decision details will not be included while the form submission is pending to avoid showing non-final decisions
* to be exposed.
* - `loadSubmittedData` includes the submitted data of each form item.
* @returns form submission details including individual form items and their details.
* @throws NotFoundException when the formSubmissionId is provided but no record is returned.
*/
async getFormSubmission(
formSubmissionId: number,
async getFormSubmissions(
studentId: number,
options?: {
formSubmissionId?: number;
locationIds?: number[];
keepPendingDecisionsWhilePendingFormSubmission?: boolean;
includeBasicDecisionDetails?: boolean;
applicationId?: number;
loadSubmittedData?: boolean;
},
): Promise<FormSubmissionAPIOutDTO> {
const submission = await this.formSubmissionService.getFormSubmissionById(
formSubmissionId,
studentId,
{ applicationId: options?.applicationId },
): Promise<FormSubmissionAPIOutDTO[]> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should you be using FormSubmissionsAPIOutDTO?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This controller service is used by API endpoints that return one or multiple records.
The API endpoints are returning the specific DTOs. What would be the benefit at this moment of having this one returning the FormSubmissionsAPIOutDTO?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thats fine, i just spotted the different returns, i guess my confusion was why have a separate FormSubmissionsAPIOutDTO and not use the array FormSubmissionAPIOutDTO[] directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We try to avoid using the direct array return as a pattern because what is an array today may be a property in the DTO that would need to be expanded in the future.

const submissions = await this.formSubmissionService.getFormSubmissions(
{ studentId, formSubmissionId: options?.formSubmissionId },
{
locationIds: options?.locationIds,
loadSubmittedData: options?.loadSubmittedData,
},
);
if (!submission) {
if (options?.formSubmissionId && !submissions?.length) {
throw new NotFoundException(
`Form submission with ID ${formSubmissionId} not found.`,
`Form submission with ID ${options?.formSubmissionId} not found.`,
);
}

// Set default value for the options that define how data will be returned considering the
// default behavior to expose less information and avoid showing non-final decisions.
const keepPendingDecisionsWhilePendingFormSubmission =
options?.keepPendingDecisionsWhilePendingFormSubmission ?? true;
let includeBasicDecisionDetails: boolean;
if (keepPendingDecisionsWhilePendingFormSubmission) {
includeBasicDecisionDetails = false;
} else {
includeBasicDecisionDetails =
options?.includeBasicDecisionDetails ?? false;
}
return submissions.map((submission) =>
this.mapSubmissionsToAPIOutDTO(
submission,
includeBasicDecisionDetails,
keepPendingDecisionsWhilePendingFormSubmission,
),
);
}

/**
* Convert a form submission record to the API output format,
* including the individual form items and their details.
* @param submission form submission record to be converted.
* @param includeBasicDecisionDetails flag to indicate if the basic decision details should be included in the response,
* besides the status that is always included.
* @param keepPendingDecisionsWhilePendingFormSubmission when true, will return "Pending" as the decision status for all items
* if the form submission is still pending. This is used to avoid showing decisions that are not final yet while the form
* submission is not completed.
* @returns form submission details including individual form items and their details in the API output format.
*/
private mapSubmissionsToAPIOutDTO(
submission: FormSubmission,
includeBasicDecisionDetails: boolean,
keepPendingDecisionsWhilePendingFormSubmission: boolean,
): FormSubmissionAPIOutDTO {
return {
id: submission.id,
formCategory: submission.formCategory,
status: submission.submissionStatus,
applicationId: submission.application?.id,
applicationNumber: submission.application?.applicationNumber,
submittedDate: submission.submittedDate,
assessedDate: submission.assessedDate,
submissionItems: submission.formSubmissionItems.map((item) => ({
id: item.id,
formType: item.dynamicFormConfiguration.formType,
Expand All @@ -60,7 +112,8 @@ export class FormSubmissionControllerService {
currentDecision: this.mapCurrentDecision(
submission.submissionStatus,
item,
!!options?.includeBasicDecisionDetails,
includeBasicDecisionDetails,
keepPendingDecisionsWhilePendingFormSubmission,
),
})),
};
Expand All @@ -80,15 +133,19 @@ export class FormSubmissionControllerService {
submissionStatus: FormSubmissionStatus,
submissionItem: FormSubmissionItem,
includeBasicDecisionDetails: boolean,
keepPendingDecisionsWhilePendingFormSubmission: boolean,
): FormSubmissionItemDecisionAPIOutDTO {
if (submissionStatus === FormSubmissionStatus.Pending) {
// For pending submissions, the decision details should not be returned.
return { decisionStatus: FormSubmissionDecisionStatus.Pending };
}
let decisionStatus =
keepPendingDecisionsWhilePendingFormSubmission &&
submissionStatus === FormSubmissionStatus.Pending
? FormSubmissionDecisionStatus.Pending
: submissionItem.currentDecision?.decisionStatus;
// Default to Pending if no decision exists.
decisionStatus = decisionStatus ?? FormSubmissionDecisionStatus.Pending;
return {
decisionStatus: submissionItem.currentDecision.decisionStatus,
decisionStatus,
decisionNoteDescription: includeBasicDecisionDetails
? submissionItem.currentDecision.decisionNote.description
? submissionItem.currentDecision?.decisionNote?.description
: undefined,
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
import { Controller, Get, Param, ParseIntPipe } from "@nestjs/common";
import { AuthorizedParties } from "../../auth";
import { AuthorizedParties, IInstitutionUserToken } from "../../auth";
import {
AllowAuthorizedParty,
HasStudentDataAccess,
IsBCPublicInstitution,
UserToken,
} from "../../auth/decorators";
import { ApiNotFoundResponse, ApiTags } from "@nestjs/swagger";
import BaseController from "../BaseController";
import { ClientTypeBaseRoute } from "../../types";
import { FormSubmissionControllerService } from "./form-submission.controller.service";
import { FormSubmissionAPIOutDTO } from "./models/form-submission.dto";
import {
FormSubmissionAPIOutDTO,
FormSubmissionsAPIOutDTO,
} from "./models/form-submission.dto";

@AllowAuthorizedParty(AuthorizedParties.institution)
@IsBCPublicInstitution()
@HasStudentDataAccess("studentId")
Copy link
Collaborator

Choose a reason for hiding this comment

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

While using the location ids, isn't this check redundant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The HasStudentDataAccess will ensure the student has an application under the institution, while the locations' IDs will ensure the user's access only for application-related forms associated with the locations.
The combination of both will ensure the forms with and without an application scope will be authorized.
As a general rule, I believe the @HasStudentDataAccess("studentId") should be enforced every time the student data is accessed to ensure the same minimal check every time.

@Controller("form-submission")
@ApiTags(`${ClientTypeBaseRoute.Institution}-form-submission`)
export class FormSubmissionInstitutionsController extends BaseController {
Expand All @@ -22,6 +27,27 @@ export class FormSubmissionInstitutionsController extends BaseController {
super();
}

/**
* Gets the list of form submissions for a student, including the individual form items and their details.
* The form submissions with application scope will be restricted to the locations the user has access.
* All form submissions without application scope can be retrieved as long as the user has access to the student data.
* @param studentId student ID to retrieve the form submission history for.
* @returns list of form submissions for a student.
*/
@Get("student/:studentId")
async getFormSubmissionHistory(
@Param("studentId", ParseIntPipe) studentId: number,
@UserToken() userToken: IInstitutionUserToken,
): Promise<FormSubmissionsAPIOutDTO> {
const submissions =
await this.formSubmissionControllerService.getFormSubmissions(studentId, {
locationIds: userToken.authorizations.getLocationsIds(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

});
return {
submissions,
};
}

/**
* Get the details of a form submission, including the individual form items and their details.
* Please note currently the institution can only access form submissions related to their students
Expand All @@ -34,19 +60,19 @@ export class FormSubmissionInstitutionsController extends BaseController {
* @returns form submission details including individual form items and their details.
*/
@ApiNotFoundResponse({ description: "Form submission not found." })
@HasStudentDataAccess("studentId", "applicationId")
@Get(
"student/:studentId/application/:applicationId/form-submission/:formSubmissionId",
)
@Get("student/:studentId/form-submission/:formSubmissionId")
async getFormSubmission(
@Param("studentId", ParseIntPipe) studentId: number,
@Param("applicationId", ParseIntPipe) applicationId: number,
@Param("formSubmissionId", ParseIntPipe) formSubmissionId: number,
@UserToken() userToken: IInstitutionUserToken,
): Promise<FormSubmissionAPIOutDTO> {
return this.formSubmissionControllerService.getFormSubmission(
formSubmissionId,
studentId,
{ includeBasicDecisionDetails: true, applicationId },
);
const [submission] =
await this.formSubmissionControllerService.getFormSubmissions(studentId, {
formSubmissionId,
includeBasicDecisionDetails: true,
loadSubmittedData: true,
locationIds: userToken.authorizations.getLocationsIds(),
});
return submission;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
FormSubmissionAPIInDTO,
FormSubmissionAPIOutDTO,
FormSubmissionConfigurationsAPIOutDTO,
FormSubmissionsAPIOutDTO,
FormSupplementaryDataAPIInDTO,
FormSupplementaryDataAPIOutDTO,
} from "./models/form-submission.dto";
Expand Down Expand Up @@ -100,6 +101,24 @@ export class FormSubmissionStudentsController extends BaseController {
};
}

/**
* Gets the list of form submissions for the student,
* including the individual form items and their details.
* @returns list of form submissions for the student.
*/
@Get()
async getFormSubmissionHistory(
@UserToken() userToken: StudentUserToken,
): Promise<FormSubmissionsAPIOutDTO> {
const submissions =
await this.formSubmissionControllerService.getFormSubmissions(
userToken.studentId,
);
return {
submissions,
};
}

/**
* Get the details of a form submission, including the individual form items and their details.
* @param formSubmissionId ID of the form submission to retrieve the details for.
Expand All @@ -111,10 +130,12 @@ export class FormSubmissionStudentsController extends BaseController {
@Param("formSubmissionId", ParseIntPipe) formSubmissionId: number,
@UserToken() userToken: StudentUserToken,
): Promise<FormSubmissionAPIOutDTO> {
return this.formSubmissionControllerService.getFormSubmission(
formSubmissionId,
userToken.studentId,
);
const [submission] =
await this.formSubmissionControllerService.getFormSubmissions(
userToken.studentId,
{ formSubmissionId, loadSubmittedData: true },
);
return submission;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ abstract class FormSubmissionBaseAPIOutDTO {
applicationId?: number;
applicationNumber?: string;
submittedDate: Date;
assessedDate?: Date;
}

/**
Expand Down Expand Up @@ -117,6 +118,14 @@ export class FormSubmissionAPIOutDTO extends FormSubmissionBaseAPIOutDTO {
submissionItems: FormSubmissionItemAPIOutDTO[];
}

/**
* List of form submissions for a student, including the
* individual form items and their details.
*/
export class FormSubmissionsAPIOutDTO {
submissions: FormSubmissionAPIOutDTO[];
}

/**
* Individual form items that will be part of a form submission with one to many forms
* for the Ministry, including the decision details.
Expand Down
Loading
Loading