Skip to content

Performance optimizations for the participants list page - #1546

Open
sayravai wants to merge 1 commit into
apluslms:masterfrom
sayravai:fix_904
Open

sayravai wants to merge 1 commit into
apluslms:masterfrom
sayravai:fix_904

Conversation

@sayravai

Copy link
Copy Markdown
Contributor

Fixes #904

Description

This PR implements highlighting the removed users in the Participants list. Highlighting does not look pretty in dark mode currently (low contrast), but it is IMO the semantically correct way to do it with Bootstrap. They won't "most likely" implement color mode adaptive styling until v6, so we can patch the styles with custom CSS if it feels necessary. The light mode looks ok, and this feature is probably not that often required, so maybe we can live with it for now.

The PR also fixes an unreported (?) issue with exporting the list as CSV/copy: The old conversion from tag list to string used data.replace(/(<[^>]*>)+/g, ','), which replaces only runs of adjacent tags with a comma. The space between </span> and <span> breaks each run, so instead of collapsing them into one separator, each tag was replaced individually and the spaces survived as list items — yielding the , , entries, like "Aalto, ,Intermediate, ,Visitor,".

Finally, the PR fixes several efficiency issues in DOM handling/lookup, detailed explanation below.

Background

Before this change, much of the per-row content (action buttons, inline buttons, row highlighting, checkbox state) was not produced by the table's own rendering pipeline. Instead, a handful of separate draw event handlers walked over the rendered DOM after every redraw to patch it up. Every pagination click, sort, filter change or search keystroke therefore triggered several full traversals of the visible rows plus repeated DOM queries, scanning the entire participants array even when only a few rows were affected.

This change restructures that work so it scales with the page size (rows currently visible, typically 50) rather than the total number of participants, and removes duplicated scans.

What changed

1. Row content is now built by DataTables column renderers instead of post-draw DOM patching

  • The Remove/Ban buttons and the inline "Add tag" button used to be created as jQuery elements on every draw and injected into the cells (a draw.aplusActions handler). They are now returned as HTML from the columns' render functions, so DataTables creates them exactly once per row as part of normal row creation, and disposes of them automatically when the row is removed.
  • Their click handlers were converted to two delegated handlers on the table element, replacing per-row, per-draw handler bindings.
  • The CSV/clipboard export formatter strips the inline button from the tags column, so exported data is unchanged.

2. Five draw handlers merged into one

  • Row highlighting (table-danger for REMOVED/BANNED rows), selection-checkbox syncing, sticky-header offset recalculation and selection-counter updates ran in separate draw.* handlers, each traversing the rows independently. They are now a single draw.aplusMain handler with one traversal per redraw.

3. Removed redundant tag-popover initialization

  • add_colortag_buttons() was called manually on every draw and after every bulk tag operation, even though it internally installs a MutationObserver that already initializes popovers on newly added tag badges. The redundant calls were removed, and the function's one-time initialization scan was narrowed from the whole document to the table's own subtree (assets/js/tag_popover.js).

4. O(1) participant lookups

  • A Map from user_id to participant is built once; it replaces six places that located participants with participants.find(...) inside loops. Applying a tag to thousands of selected students, or opening the "remove tag" modal for a large selection, no longer scans the full participant list once per selected user.

5. Code deduplication / minor fixes

  • The "add a filter row to the table header" logic was copy-pasted in three places; it now lives in a single ensureFiltersRow() helper. As a side effect this also fixes a latent bug where a filters row built before DataTables initialization was silently discarded by DataTables.
  • The three global "…to selected" buttons shared a duplicated "collect selected + filtered ids" loop, now factored into
    selectedFilteredUserIds().
  • Added a small translation helper t() so labels rendered into rows tolerate the i18n catalog loading late (rows are re-rendered once when translations become ready).

Complexity summary

Operation Before After
One table redraw (page/sort/filter) ~5 row traversals + repeated DOM queries + per-cell DOM rebuilds 1 row traversal; row content built as part of rendering
Bulk tag add/remove for k users, N participants O(k·N) lookups + document-wide selector query O(k) lookups; new badges picked up by the existing MutationObserver
Tag popover init on bulk change O(all .colortag elements in the document), called on every draw O(1) per draw (observer-driven); initial scan scoped to the table

How to test

  • Load the participants list as a teacher and as a student; verify filters, search, sorting, paging and page-size selection still work.
  • Remove/Ban a participant: the row should turn red (REMOVED/BANNED), its Remove/Ban buttons become disabled, and the status counters update.
  • Add/remove tags individually and via the "to selected" buttons; tag badges should still show their popover on hover, and CSV/copy exports should contain the comma-separated tag names without any button leftovers.
  • Select students across pages and use "Add tag / Remove tag / Batch assess to selected"; the selected count in the header should stay correct while paging and filtering.

Testing

Remember to add or update unit tests for new features and changes.

What type of test did you run?

  • Accessibility test using the WAVE extension.
  • Django unit tests.
  • Playwright tests.
  • Other test. (Add a description below)
  • Manual testing.

[ADD A DESCRIPTION ABOUT WHAT YOU TESTED MANUALLY]

Did you test the changes in

  • Chrome
  • Firefox
  • This pull request cannot be tested in the browser.

Think of what is affected by these changes and could become broken

Translation

Programming style

  • Did you follow our style guides?
  • Did you use Python type hinting in all functions that you added or edited? (type hints for function parameters and return values)

Have you updated the README or other relevant documentation?

  • documents inside the doc directory.
  • README.md.
  • Aplus Manual.
  • Other documentation (mention below which documentation).

Is it Done?

  • Reviewer has finished the code review
  • After the review, the developer has made changes accordingly
  • Customer/Teacher has accepted the implementation of the feature

Clean up your git commit history before submitting the pull request!

@ihalaij1 ihalaij1 self-assigned this Oct 9, 2026
@ihalaij1
ihalaij1 self-requested a review October 9, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Under review

Development

Successfully merging this pull request may close these issues.

Highlight removed users in participant lists

3 participants