Skip to content

Conversation

@andrewsignori-aot
Copy link
Collaborator

Saving the file hash for student-uploaded files to allow future comparison of exception files.

Migration rollback

image

@andrewsignori-aot andrewsignori-aot self-assigned this Aug 1, 2025
@andrewsignori-aot andrewsignori-aot added SIMS-Api SIMS-Api DB DB migration involved labels Aug 1, 2025
@andrewsignori-aot andrewsignori-aot marked this pull request as ready for review August 1, 2025 22:39
Copy link
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

This PR adds file hash storage functionality to the student file system to prevent duplicate exception file submissions. The implementation generates and stores SHA256 hashes of uploaded file content to enable future comparison for exception handling.

  • Adds a new file_hash column to the student_files table
  • Implements SHA256 hash generation during file upload
  • Includes database migration scripts for adding and removing the column

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
student-file.model.ts Adds optional fileHash property to the StudentFile entity
Add-file-hash-column.sql Migration script to add the file_hash column to student_files table
Rollback-add-file-hash-column.sql Rollback script to remove the file_hash column
AddStudentFileHash1754086479996.ts TypeORM migration class that executes the SQL scripts
student-file.service.ts Implements SHA256 hash calculation during file creation
Comments suppressed due to low confidence (1)

sources/packages/backend/apps/db-migrations/src/migrations/1754086479996-AddStudentFileHash.ts:4

  • The timestamp '1754086479996' represents a date in the year 2025 (August), which appears to be a future timestamp that may not be valid depending on when this migration needs to run.
export class AddStudentFileHash1754086479996 implements MigrationInterface {

@dheepak-aot dheepak-aot self-requested a review August 5, 2025 18:17
Comment on lines +77 to +79
newFile.fileHash = createHash("sha256")
.update(createFile.fileContent)
.digest("hex");
Copy link
Collaborator

@dheepak-aot dheepak-aot Aug 5, 2025

Choose a reason for hiding this comment

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

Would it be better if we consider the unique file name also when creating the file hash?

Copy link
Collaborator Author

@andrewsignori-aot andrewsignori-aot Aug 5, 2025

Choose a reason for hiding this comment

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

The unique file name is exactly what we need to prevent to cause any change in the hash.
The idea here is to hash the file content only to allow the file content only verification.

Copy link
Collaborator

@dheepak-aot dheepak-aot Aug 5, 2025

Choose a reason for hiding this comment

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

I was under the impression that we are expecting for (file name + file content) to be matching. If that is not required and business is on-board with that, I am good.

But can we have this AC updated?

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

I got the intention now which follows the AC. Thanks for the sync up.

Copy link
Collaborator

@sh16011993 sh16011993 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 👍

sims.student_files
ADD
COLUMN file_hash CHAR(64);
COLUMN file_hash CHAR(64) NOT NULL DEFAULT '';
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 5, 2025

@github-actions
Copy link

github-actions bot commented Aug 5, 2025

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 21.66% ( 4094 / 18905 )
Methods: 9.67% ( 234 / 2419 )
Lines: 25.01% ( 3541 / 14157 )
Branches: 13.7% ( 319 / 2329 )

@github-actions
Copy link

github-actions bot commented Aug 5, 2025

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 73.53% ( 714 / 971 )
Methods: 72.57% ( 82 / 113 )
Lines: 75.81% ( 564 / 744 )
Branches: 59.65% ( 68 / 114 )

@github-actions
Copy link

github-actions bot commented Aug 5, 2025

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 86.43% ( 1548 / 1791 )
Methods: 84.62% ( 176 / 208 )
Lines: 88.75% ( 1278 / 1440 )
Branches: 65.73% ( 94 / 143 )

@github-actions
Copy link

github-actions bot commented Aug 5, 2025

E2E SIMS API Coverage Report

Totals Coverage
Statements: 73.78% ( 7270 / 9853 )
Methods: 71.51% ( 896 / 1253 )
Lines: 77.21% ( 5624 / 7284 )
Branches: 56.99% ( 750 / 1316 )

Copy link
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 start to the Hash. Looks good 👍

@andrewsignori-aot andrewsignori-aot added this pull request to the merge queue Aug 5, 2025
Merged via the queue into main with commit eef817d Aug 5, 2025
22 checks passed
@andrewsignori-aot andrewsignori-aot deleted the feature/#4941-approved-exception-does-not-repeat-save-file-hash branch August 5, 2025 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DB DB migration involved SIMS-Api SIMS-Api

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants