Add client-side sortable columns to results.php table#6767
Add client-side sortable columns to results.php table#6767
Conversation
Co-authored-by: AenBleidd <3234209+AenBleidd@users.noreply.github.com>
… security Co-authored-by: AenBleidd <3234209+AenBleidd@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="html/user/results.php">
<violation number="1" location="html/user/results.php:34">
P2: External CDN resources should include `integrity` hashes for Subresource Integrity (SRI) protection. If the CDN is compromised, malicious code could be injected without detection. Add integrity hashes from https://datatables.net/download/ or generate them using `openssl dgst -sha384 -binary <file> | openssl base64 -A`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| // Add DataTables CSS and JS for client-side table sorting | ||
| $head_extra = ' | ||
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap4.min.css" crossorigin="anonymous"> |
There was a problem hiding this comment.
P2: External CDN resources should include integrity hashes for Subresource Integrity (SRI) protection. If the CDN is compromised, malicious code could be injected without detection. Add integrity hashes from https://datatables.net/download/ or generate them using openssl dgst -sha384 -binary <file> | openssl base64 -A.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At html/user/results.php, line 34:
<comment>External CDN resources should include `integrity` hashes for Subresource Integrity (SRI) protection. If the CDN is compromised, malicious code could be injected without detection. Add integrity hashes from https://datatables.net/download/ or generate them using `openssl dgst -sha384 -binary <file> | openssl base64 -A`.</comment>
<file context>
@@ -29,6 +29,13 @@
+// Add DataTables CSS and JS for client-side table sorting
+$head_extra = '
+<link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap4.min.css" crossorigin="anonymous">
+<script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/jquery.dataTables.min.js" crossorigin="anonymous"></script>
+<script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/dataTables.bootstrap4.min.js" crossorigin="anonymous"></script>
</file context>
There was a problem hiding this comment.
Pull request overview
This PR implements client-side table sorting for task results using the DataTables 1.13.8 library, addressing a long-standing feature request (issue #93) to enable column-based sorting. Users can now click column headers to sort results without triggering database queries, with support for multi-column sorting via Shift+Click.
Key changes:
- Added DataTables library includes (CSS and JavaScript) via CDN to results.php
- Modified the shared
result_table_start()function to output proper HTML5 table structure with<thead>/<tbody>sections and a hardcoded table ID - Initialized DataTables with pagination, search, and info display disabled to preserve existing BOINC navigation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| html/user/results.php | Adds DataTables CDN includes in $head_extra variable, passes it to page_head() calls, adds closing </tbody> tag, and includes DataTables initialization script |
| html/inc/result.inc | Replaces start_table() call with custom table HTML structure including hardcoded "results-table" ID, adds <thead> wrapper around headers, and opens <tbody> section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Use custom table start with ID for DataTables sorting | ||
| echo '<div class="table"> | ||
| <table width="100%" class="table table-condensed table-striped" id="results-table"> | ||
| <thead> | ||
| '; |
There was a problem hiding this comment.
The modified result_table_start() function introduces breaking changes for other files that use it. This function is also called in html/user/submit_status.php (line 35) and html/user/workunit.php (line 94), which will now generate tables with the hardcoded ID "results-table", causing duplicate IDs on the page if multiple tables exist, and breaking those pages since they don't include DataTables.
Additionally, workunit.php uses echo "</table>\n" to close the table instead of end_table(), which means it won't properly close the <div class="table"> wrapper that this function now adds.
Consider either:
- Making the table ID optional/parameterized (e.g., add a parameter to specify the ID, defaulting to null)
- Only applying the custom structure in results.php rather than modifying the shared function
- Ensuring all callers of this function are updated to handle the new structure
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap4.min.css" crossorigin="anonymous"> | ||
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/jquery.dataTables.min.js" crossorigin="anonymous"></script> | ||
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/dataTables.bootstrap4.min.js" crossorigin="anonymous"></script> |
There was a problem hiding this comment.
The DataTables Bootstrap 4 integration is being used (dataTables.bootstrap4.min.css and dataTables.bootstrap4.min.js), but it's unclear whether this BOINC installation actually uses Bootstrap 4. The codebase references bootstrap.min.css and bootstrap.min.js without version numbers in the filenames.
DataTables has different styling integrations for Bootstrap 3, 4, and 5. Using the wrong version can cause visual inconsistencies or styling issues. Verify which Bootstrap version is in use and ensure the correct DataTables integration files are loaded. If Bootstrap 3 is in use, you should use dataTables.bootstrap.min.css and dataTables.bootstrap.min.js instead.
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap4.min.css" crossorigin="anonymous"> | |
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/jquery.dataTables.min.js" crossorigin="anonymous"></script> | |
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/dataTables.bootstrap4.min.js" crossorigin="anonymous"></script> | |
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap.min.css" crossorigin="anonymous"> | |
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/jquery.dataTables.min.js" crossorigin="anonymous"></script> | |
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/dataTables.bootstrap.min.js" crossorigin="anonymous"></script> |
| $x[] = tra("Application"); | ||
| $a[] = null; | ||
| row_heading_array($x, $a); | ||
| echo "</thead>\n<tbody>\n"; |
There was a problem hiding this comment.
The closing </tbody> tag is added in results.php (line 110) rather than being part of the result_table_start() function structure. This creates an inconsistency: the function opens <thead> and <tbody> tags but doesn't close them. This makes the function's interface unclear and error-prone.
Consider moving the </tbody> closing tag into a companion function (e.g., before end_table() is called) or clearly document that callers must manually close the tbody tag. However, given that this function is used in multiple places, it would be better to keep the opening and closing tags paired within the same abstraction level.
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap4.min.css" crossorigin="anonymous"> | ||
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/jquery.dataTables.min.js" crossorigin="anonymous"></script> | ||
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/dataTables.bootstrap4.min.js" crossorigin="anonymous"></script> |
There was a problem hiding this comment.
The crossorigin="anonymous" attribute on the script tags may cause issues if the CDN requires credentials or if there are CORS configuration problems. While this is generally safe for public CDNs, consider using Subresource Integrity (SRI) hashes to ensure the loaded scripts haven't been tampered with, which would provide better security. DataTables provides SRI hashes for their CDN files on their download page.
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap4.min.css" crossorigin="anonymous"> | |
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/jquery.dataTables.min.js" crossorigin="anonymous"></script> | |
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/dataTables.bootstrap4.min.js" crossorigin="anonymous"></script> | |
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap4.min.css"> | |
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/jquery.dataTables.min.js"></script> | |
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/dataTables.bootstrap4.min.js"></script> |
| // Initialize DataTables for client-side sorting | ||
| echo ' | ||
| <script type="text/javascript"> | ||
| jQuery(document).ready(function() { | ||
| // Check if the results table exists | ||
| if (jQuery("#results-table").length) { | ||
| jQuery("#results-table").DataTable({ | ||
| "paging": false, // Disable DataTables pagination (use existing navigation) | ||
| "searching": false, // Disable search box (not needed) | ||
| "info": false, // Disable info text (e.g., "Showing 1 to 10 of 57 entries") | ||
| "order": [] // No default sort, keep database order | ||
| }); | ||
| } | ||
| }); | ||
| </script> | ||
| '; |
There was a problem hiding this comment.
The DataTables initialization script is placed after echo $nav; but before page_tail(). This means the script will execute even when there are no results (when the else branch at line 112 is taken), though the check if (jQuery("#results-table").length) will prevent errors. However, for better code organization and to avoid unnecessary DOM queries, consider conditionally including the initialization script only when results are displayed (inside the if (count($results)) block after line 103).
| // Use custom table start with ID for DataTables sorting | ||
| echo '<div class="table"> | ||
| <table width="100%" class="table table-condensed table-striped" id="results-table"> | ||
| <thead> | ||
| '; |
There was a problem hiding this comment.
The modified result_table_start() function now directly outputs the table opening structure instead of calling start_table('table-striped'). However, this replaces the original pattern where callers would use end_table() to properly close both the table and its wrapper div.
In html/user/workunit.php (line 99), the code uses echo "</table>\n" directly instead of calling end_table(), which means it won't close the <div class="table"> wrapper that this modified function now creates. This will result in broken HTML structure.
The other callers (results.php line 111 and submit_status.php line 39) do call end_table(), so they will properly close the structure, but they will also have the hardcoded "results-table" ID issue mentioned in the previous comment.
| // Use custom table start with ID for DataTables sorting | |
| echo '<div class="table"> | |
| <table width="100%" class="table table-condensed table-striped" id="results-table"> | |
| <thead> | |
| '; | |
| // Use standard table start helper to ensure consistent structure | |
| start_table('table-striped'); | |
| echo "<thead>\n"; |
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.13.8/css/dataTables.bootstrap4.min.css" crossorigin="anonymous"> | ||
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/jquery.dataTables.min.js" crossorigin="anonymous"></script> | ||
| <script type="text/javascript" src="https://cdn.datatables.net/1.13.8/js/dataTables.bootstrap4.min.js" crossorigin="anonymous"></script> |
There was a problem hiding this comment.
DataTables 1.13.8 was released in December 2023, which is over 2 years old at the time of this review (January 2026). Consider checking if a newer version is available that includes bug fixes and security updates. At minimum, verify that version 1.13.8 doesn't have any known security vulnerabilities.
Description of the Change
Implements client-side table sorting for task results using DataTables 1.13.8. Users can now click column headers to sort ascending/descending, or Shift+Click for multi-column sorting. All sorting occurs in the browser without database queries.
Changes:
result_table_start()to add table ID and proper HTML5<thead>/<tbody>structureDataTables configuration:
Alternate Designs
Release Notes
Results table columns are now sortable. Click any column header to sort, Shift+Click for multi-column sorting. All sorting is client-side with no server impact.
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
cdn.datatables.net/usr/bin/curl curl -s REDACTED --shared-client-connection(dns block)/usr/bin/curl curl -s REDACTED(dns block)/usr/bin/curl curl -sL REDACTED -o dt-bs4.css(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by cubic
Add client-side sorting to results.php so users can sort by column headers (single or multi-column) to quickly find recent results or errors, with no extra database load. Fixes #93.
Written for commit 90a0778. Summary will update on new commits.