Skip to content

Fix/applics 1683 improve loading speed examination timetable items#2968

Open
JanPhillips wants to merge 13 commits intomainfrom
fix/APPLICS-1683-improve-loading-speed-examination-timetable-items
Open

Fix/applics 1683 improve loading speed examination timetable items#2968
JanPhillips wants to merge 13 commits intomainfrom
fix/APPLICS-1683-improve-loading-speed-examination-timetable-items

Conversation

@JanPhillips
Copy link
Copy Markdown
Contributor

@JanPhillips JanPhillips commented Oct 5, 2025

Optimisation of loading speed for examination timetable items

CBOS Exam timetable page loading speed

The loading speed of the examination timetable items page is slow due to how the number of documents within a folder and its subfolders is counted. Currently, a recursive traversal of the folder structure, which involves calls to the database, this happens for each examination timetable item, when there are many timetable items on the page, this causes the page to load slowly. The document count is required to determine if an examination timetable item can be deleted, items with no files can be deleted.

This PR has sped up the loading time by utilising a concept called Materialised Paths. Two new columns have been added to the Folder table, documentCount and path, combined with new logic for setting, updating, and getting these values, only 1 database query is required to get the the document count for any folder. The documentCount for all ancestor folders are update when files are uploaded, deleted or moved. In terms of Big O notation I believe this changes the effort to get the document count of a folder from O(n) (where n = number of child folders) to O(1).

For existing Folders

Included in this PR is a script file, apps/api/src/database/seed/create-paths.js that can by run on a server to set the path value for each Folder. There is also a test script apps/api/src/database/test-scripts/create-paths-test-script.js that can be run afterwards to check the changes were successful.

Before running

Run a migration on the DB, to ensure that the Folder table gets the new columns documentCount and path

Issue ticket number and link

Type of change 🧩

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Other (please explain in the description section above)

Checklist before requesting a review

  • I have performed a self-review of my own code
  • I have double checked this work does not include any hardcoded secrets or passwords
  • I have made corresponding changes to the documentation
  • I have provided details on how I have tested my code
  • I have referenced the ticket number above
  • I have provided a description of the ticket
  • I have included unit tests to cover any testable code changes

@JanPhillips JanPhillips requested a review from Copilot October 5, 2025 12:17
@JanPhillips JanPhillips self-assigned this Oct 5, 2025
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

This PR improves the loading speed of examination timetable items by implementing a document count tracking system using folder document counts instead of potentially expensive real-time queries.

  • Adds documentCount and path fields to the Folder database model
  • Implements functions to efficiently track document counts when documents are added/removed/moved
  • Updates examination timetable logic to use the cached document count instead of querying submissions

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
apps/web/src/server/views/applications/case-timetable/components/timetable-items-list.component.njk Updates template logic to handle numeric submission counts and null states
apps/web/src/server/applications/case-timetable/tests/snapshots/applications-timetable.test.js.snap Updates test snapshots to reflect removal of delete links for folders with submissions
apps/api/src/server/repositories/folder.repository.js Adds document count tracking functions and folder path management
apps/api/src/server/applications/examination-timetable-items/examination-timetable-items.controller.js Replaces submission validation with document count lookup
apps/api/src/server/applications/application/documents/document.service.js Integrates document count updates when documents are created/deleted
apps/api/src/database/schema.prisma Adds documentCount and path fields to Folder model

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@JanPhillips JanPhillips requested a review from Copilot October 6, 2025 12:10
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

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +393 to +394
return databaseConnector.$executeRaw`UPDATE folder SET documentCount = documentCount - ${decreaseBy}
WHERE ${folder.path} LIKE CONCAT(path, '%')`;
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

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

This raw SQL query is vulnerable to SQL injection. The folder.path value should be parameterized or sanitized before being used in the query.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +412 to +413
return databaseConnector.$executeRaw`UPDATE folder SET documentCount = documentCount + ${increaseBy}
WHERE ${folder.path} LIKE CONCAT(path, '%')`;
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

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

This raw SQL query is vulnerable to SQL injection. The folder.path value should be parameterized or sanitized before being used in the query.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@JanPhillips JanPhillips Oct 6, 2025

Choose a reason for hiding this comment

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

@JanPhillips JanPhillips requested a review from Copilot October 6, 2025 13:29
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

Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@JanPhillips JanPhillips requested a review from Copilot October 6, 2025 14:17
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

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@JanPhillips JanPhillips force-pushed the fix/APPLICS-1683-improve-loading-speed-examination-timetable-items branch from a3664b6 to d05263f Compare October 6, 2025 14:26
@JanPhillips JanPhillips requested a review from Copilot October 6, 2025 14:27
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

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@JanPhillips JanPhillips requested a review from Copilot October 6, 2025 14:34
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

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


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@JanPhillips JanPhillips marked this pull request as ready for review October 10, 2025 12:14

// updateDocuments[0] is the array of Document objects updated
// updateDocuments[1] is array of DocumentVersion objects
if (updateDocuments[0].count === 0 || updateDocuments[1].count === 0) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we could rename updateDocuments[0] to updatedDocumentObjects and updateDocuments[1] could be renamed to documentVersionObjects something like const [updatedDocumentObjects, documentVersionObjects] = updateDocuments;

Copy link
Copy Markdown
Contributor

@ypk ypk left a comment

Choose a reason for hiding this comment

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

I added a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants