fix: Handle tables with missing <thead> tag before attaching a DataTable

This commit fixes the following:
- The sorting indicators from DataTable lib were missing (#79). In order
    to fix it, it was decided to add explicit classnames and wrap the
table elements around custom classes due CSS specificity. After that, it
was matter of adding CSS rules for the said classes that add unicode
arrows. It takes advantage of the fact that DataTable lib changes the
class name of the table header cell 'th', depending if data is sorted
asc or desc
- DataTables requires the `<table>` to have an explicit `<thead>`.
    Therefore, it was added a mechanism to find potential safe table
headers, for instance, a `tr` full of `th` nodes. If its find, it makes
the necessary changes to move the `<tr>` into a new `<thead>` element

src/css/content_style.css

@@ -231,7 +231,9 @@ div.dropdown.right {
     background-color: transparent;
 }
 
-/* datatables */
+/**
+ * Datatables
+ */
 
 .dataTables_filter input[type="search"] {
     margin-left: 10px
@@ -250,4 +252,22 @@ div.dropdown.right {
 
 a.calendarBtn:hover {
     color: #ffffff;
-}
+}
+
+.SigTools__dt__table th.sorting::after,
+.SigTools__dt__table th.sorting_asc::after,
+.SigTools__dt__table th.sorting_desc::after {
+    font-size: 18px;
+    font-weight: normal;
+    font-family: monospace;
+    content: '\21C5'; /* up and down arrow */
+    margin-left: 3px;
+}
+
+.SigTools__dt__table th.sorting_asc::after {
+    content: '\2191'; /* up arrow */
+}
+
+.SigTools__dt__table th.sorting_desc::after {
+    content: '\2193'; /* down arrow */
+}

src/js/extractors/datatable.js

@@ -1,5 +1,54 @@
 "use strict";
 
+class TableUtils {
+    /**
+     * Checks if a table row, `<tr>`, is a header row
+     * 
+     * @param {*} $tr 
+     * @returns {boolean}
+     */
+    static isHeaderRow($tr) {
+        if(!$tr.is('tr'))
+            throw Error(`Expected <tr> element. Got ${$tr}`);
+        // A table row is considered as header if and only if all cells/children
+        // are `<th>`. According to MDN, a `tr` can have as children `td`, `th`
+        // and other script related elements, such `<script>` or `<template>`
+        // See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/tr#technical_summary
+
+        // Thus the condition below is: number of cells (`td` or `th`) must
+        // match the number of header cells. If its not, then at least one cell
+        // is `td` and is no longer safe to assume it as a table header
+        // Note: we use jQuery 'children' to only search direct children nodes.
+        // If the cells are deeper on the tree, it is unexpected behavior
+        return $tr.children("th,td").length === $tr.children("th").length;
+    }
+
+    /**
+     * Finds a sequence of table rows `<tr>` that can be header rows
+     * 
+     * @param {*} $table 
+     * @returns 
+     */
+    static getHeaderRows($table) {
+        // find the first table row (not necessarially the first table children (e.g. <caption>))
+        const $firstRow = $table.find("tr:first-child");
+        // if the first table row is a header row, iterate over the sibling table rows while they are headers
+        if (TableUtils.isHeaderRow($firstRow)) {
+            // iterate over the sibling rows, and collect the header ones
+            const $rows = $firstRow.find("~ tr");
+            const $headerRows = [$firstRow];
+            let foundHeader = true;
+            for (let i = 0; i < $rows.length && foundHeader; i++) {
+                foundHeader = TableUtils.isHeaderRow($($rows[i]));
+                if (foundHeader)
+                    $headerRows.push($($rows[i]));
+            }
+            return $headerRows;
+        }
+        // first table row is not a header, give up
+        return [];
+    }
+}
 class DataTable extends Extractor {
     constructor() {
         super();
@@ -26,72 +75,128 @@ class DataTable extends Extractor {
         }
     }
 
-
     attachIfPossible() {
-        $.each(this.tables, (_, t) => this.attachTableIfPossible($(t)))
+        // returned becaused of unit tests
+        return $.each(this.tables, (_, t) => this.attachTableIfPossible($(t)))
     }
 
-    attachTableIfPossible(table) {
-        // return if table not found or not applied
-        if (!table.length || !this.validTable(table)) return
-        if (!table.find("tr").toArray().length) return //if table is empty
-        if (this.disable_one_row && table.find("tr").toArray().length == 2) return //if table only has header and one row
+    attachTableIfPossible($table) {
+        // validate table based on user settings for minimum number of rows
+        if (!this.validTable($table))
+            return;
 
         // remove sigarra stuff that is useless
-        $("#ordenacao").remove()
-        $("th a").remove()
+        $("#ordenacao").remove();
+        $("th a").remove();
 
-        // inject dynamic tables
-        table.prev().after($(`<h2 class="noBorder">SigTools Dynamic Tables</h2>`))
-        table.prepend($(`<thead>${table.find("tr").html()}</thead>`))
-        table.find("tbody tr:has(> th)").remove()
+        // make the necessary transformations to the tables to make DataTable work
+        this.preprocessTable($table);
 
-        // sorting guide: https://www.datatables.net/plug-ins/sorting/
-        table.dataTable(DataTable.datatableOptions);
+        try {
+            // try to apply the DataTable
+            $table.dataTable(DataTable.datatableOptions);
+            // inject dynamic tables title
+            $table.prev().after($(`<h2 class="noBorder">SigTools Dynamic Tables</h2>`));
+        } catch (e) {
+            // TODO: cleanup
+            // if it fails, table may have a `dataTable` class that changes styling
+        }
     }
 
     /**
      * Check if the current table is valid for applying datatables
      */
-    validTable(table) {
-        this.performCustomValidation(table)
-        let cols = table.find("tr:has(> th)").find("th").toArray().length
-        let first = table.find("tr:has(> td)").eq(0).find("td").toArray().length
-        return cols == first && table.find("td[rowspan],td[colspan]").length == 0
+     validTable($table) {
+        if (this.disable_one_row)
+            return $table.find("tr").toArray().length >= 2;
+        else
+            return true;
     }
 
     /**
-     * Call specific functions for specific pages with strange tables
+     * Attempts to transform the tables to make them work with DataTable lib
+     * @param {*} $table 
      */
-    performCustomValidation(table) {
-        if(this.url.includes("coop_candidatura_geral.ver_colocacoes_aluno")) this.transformErasmus(table)
+    preprocessTable($table) {
+        // transform the table if it is the erasmus listing page
+        this.transformErasmus($table);
+
+        // add <thead> if missing, required by DataTable lib
+        this.transformAddTableHeader($table);
     }
 
     /**
      * Fix table for the erasmus listings page
      * @param {Table} table
      */
-    transformErasmus(table){
-        $(table.find("tr:first-child th[colspan=2]").replaceWith(table.find("tr:nth-child(2)").html()))
-        table.find("tr:nth-child(2)").remove()
-        table.find('th[rowspan=2]').attr('rowspan', '1');
+    transformErasmus(table) {
+        if (this.url.includes("coop_candidatura_geral.ver_colocacoes_aluno")) {
+            $(table.find("tr:first-child th[colspan=2]").replaceWith(table.find("tr:nth-child(2)").html()))
+            table.find("tr:nth-child(2)").remove()
+            table.find('th[rowspan=2]').attr('rowspan', '1');
+        }
+    }
+
+    /**
+     * If <thead> is missing, then it searches for candidate table header rows
+     * and moves them to a new <thead> tag
+     * @param {*} $table 
+     */
+    transformAddTableHeader($table) {
+        if ($table.has("thead").length === 0) {
+            // try to find consecutive header rows
+            const headers = TableUtils.getHeaderRows($table);
+
+            // if no rows are found, then table is not valid
+            if (headers.length === 0)
+                return false;
+
+            // create <thead> and add it at the beginning
+            const $thead = $("<thead>");
+            $table.prepend($thead);
+
+            // move each found header row to <thead>
+            for (const $tr of headers) {
+                // remove the row from the original place in the DOM
+                $tr.remove();
+                // append the row to the header
+                $thead.append($tr);
+            }
+        }
     }
 }
 
 // static property: options to use in the datatable calls
 DataTable.datatableOptions = {
     paging: false,
     order: [],
-    dom: 'Bfrtip',
-    buttons: ['copyHtml5', {
+    /**
+     * Define the table control elements to appear on the page and in what order
+     * @see {@link https://datatables.net/reference/option/dom}
+     *
+     * Moreover, we also use this to wrap around DataTable elements in `div`s with
+     * our own class names. This enables us to create CSS rules with more specificity
+     * thus overriding the library defaults with more ease
+     *
+     * 1. A wrapper div with class 'SigTools__dt'
+     * 2. B -> the buttons for copying and exporting table data
+     * 3. f -> filter inputs (search box)
+     * 4. r -> show a loading indicator (see: https://datatables.net/reference/option/processing)
+     * 5. t -> the table itself with class 'SigTools__dt__table'
+     * 6. i -> information summary
+     * 7. p -> pagination control
+     */
+    dom: `<"SigTools__dt"Bfr<"SigTools__dt__table"t>i>`,
+    buttons: ['copyHtml5', 'print', {
         extend: 'csvHtml5',
         charset: 'UTF-8',
         bom: true
     }, {
-        extend: 'excelHtml5',
-        charset: 'UTF-8',
-        bom: true
-    }, 'print'],
+            extend: 'excelHtml5',
+            charset: 'UTF-8',
+            bom: true
+        }
+    ],
 }
 
 /**
@@ -104,7 +209,7 @@ function removeDatatableIfExists(selector) {
         table.destroy()
         return el => el.dataTable(DataTable.datatableOptions)
     }
-    return (_) => {}
+    return (_) => { }
 }
 
 // add an instance to the EXTRACTORS variable, and also trigger attachIfPossible due to constructor