#2111-UI format validation bulk withdrawal upload #2209
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
Collaborator
guru-aot
commented
Aug 17, 2023
•
guru-aot
commented
- Only allow plain text files to be uploaded.
- Validate header record type.
- Validate withdrawal record type.
- Validate footer record type.
- Validate the application number to be numeric with 10 digits.
- Validate SIN to be numeric and with 9 digits.
- Validate the withdrawal to be a valid date in the format YYYYMMDD.
- Validate if the total number of records in the file matches the footer information.
- Succesfully validated
guru-aot
changed the title
guru-aot
requested review from
andrewsignori-aot,
sh16011993,
dheepak-aot,
ann-aot,
andrepestana-aot and
JasonCTang
and removed request for
andrewsignori-aot
...rollers/student-scholastic-standings/student-scholastic-standings.institutions.controller.ts
Outdated
Show resolved
Hide resolved
...rollers/student-scholastic-standings/student-scholastic-standings.institutions.controller.ts
Show resolved
Hide resolved
...apps/api/src/services/application-bulk-withdrawal/application-bulk-withdrawal-text.models.ts
Outdated
Show resolved
Hide resolved
dheepak-aot
reviewed
Aug 24, 2023
...rollers/student-scholastic-standings/student-scholastic-standings.institutions.controller.ts
Show resolved
Hide resolved
|
Thanks for doing the changes @guru-aot . Just a few minor comments. |
sh16011993
reviewed
Aug 25, 2023
| async bulkWithdrawal( | ||
| @UploadedFile() file: Express.Multer.File, | ||
| @Query("validation-only", new DefaultValuePipe(false), ParseBoolPipe) | ||
| validationOnly: boolean, |
There was a problem hiding this comment.
For my info, can we have a scenario when we process a file without doing the validations?
There was a problem hiding this comment.
I dont think, even with business validations it will not do the existing validations.
sh16011993
reviewed
Aug 25, 2023
| @@ -64,9 +64,14 @@ const fileFilter = ( | |||
| file: MulterFile, | |||
| allowedFileExtensions: string[], | |||
| callback: (error: Error | null, acceptFile: boolean) => void, | |||
| options?: { allowedMimeType?: string[] }, | |||
There was a problem hiding this comment.
Can we adjust the comment to include allowedFileExtensions and options
sh16011993
reviewed
Aug 25, 2023
| // Read the first line to check if the header code is the expected one. | ||
| const header = ApplicationBulkWithdrawalHeader.createFromLine( | ||
| fileLines.shift(), | ||
| ); // Read and remove header. |
There was a problem hiding this comment.
Should we move the comment above the execution line?
There was a problem hiding this comment.
its an inline comment
ann-aot
reviewed
Aug 25, 2023
sources/packages/web/src/views/institution/WithdrawalUpload.vue
Outdated
Show resolved
Hide resolved
ann-aot
reviewed
Aug 25, 2023
|
Kudos, SonarCloud Quality Gate passed!
|
Backend Unit Tests Coverage Report
|
E2E Queue Consumers Coverage Report
|
E2E Workflow Workers Coverage Report
|
dheepak-aot
approved these changes
Aug 25, 2023
There was a problem hiding this comment.
Thanks for the changes @guru-aot . Great start!
I would say to review the error message content with @JasonCTang once.
E2E SIMS API Coverage Report
|
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.