[Tables] Clean up filter code logic

[akhenry]

Summary

The TableRowCollection will conditionally clear the current contents of the table based on the value of a flag that is passed into the addRows function. Having addRows clear the current table is an unexpected side effect of a function called addRows. Instead the calling code should clear the table, then call addRows.

[JonanOribe]

So the idea is to extract the method that cleans the table and call addRows at the end, right?.Could I take this one?

[akhenry]

@JonanOribe Right, so find anywhere that addRows is being called with the type flag set tofilter, and instead of passing that parameter call .clear() and then addRows.

Once that's done, remove the 'type' parameter from addRows altogether.

And sure, it would be great if you could take a look at it! Thank you!

[JonanOribe]

@akhenry looking at the code I saw that .clear() emit a 'remove' instead of the 'filter' that is called by the actual addRows when 'filter' is passed as a parameter. So, if I make the changes as you say, we have some problems with the functionality and some test cases just fail.

A workaround that I found, and with all testing goes green, is to make this:

            addRows(rows) {
                let rowsToAdd = this.manageRowsFromTables(rows);

                this.sortAndMergeRows(rowsToAdd);

                // we emit filter no matter what to trigger
                // an update of visible rows
                if (rowsToAdd.length > 0) {
                    this.emit('add', rowsToAdd);
                }
            }

            clearRowsFromTableAndFilter(rows) {

                let rowsToAdd = this.manageRowsFromTables(rows);
                // Reset of all rows, need to wipe current rows
                this.rows = [];

                this.sortAndMergeRows(rowsToAdd);

                // we emit filter and update of visible rows
                this.emit('filter', rowsToAdd);
            }

            manageRowsFromTables(rows) {
                if (this.sortOptions === undefined) {
                    throw 'Please specify sort options';
                }

                let anyActiveFilters = Object.keys(this.columnFilters).length > 0;

                return !anyActiveFilters ? rows : rows.filter(this.matchesFilters, this);
            }

Now, we have one method for cleaning and filtering and another for adding rows. I did not change the .clear() because it is called in many different places on code, but 'filter' just on one, so in my opinion, is more secure to make this change only on the new function.

What do you think? if neccesary, I look for another solution. Thank you!

[JonanOribe]

I added the mentioned code on this PR

[akhenry]

Testing Notes

This is a non-functional change with a risk of introducing regressions into tables, particularly with table filtering.

1. Create a new table and drag-drop a telemetry point into it
2. Set the time conductor to real-time mode, with bounds set such that you get ~100 results.
3. Sort by timestamp and verify that the rows appear to be in the correct order
4. Reverse the sort order and verify that the rows appear to be in the correct order.
5. Sort by a value row (eg. Sine if using an SWG) and verify that the rows appear to be sorted correct.
6. Set a filter value and verify that the results match the filter.
7. Set a filter and a sort column and verify that the results match the filter and are sorted in the expected order

Testing Notes

Verify that you can filter rows and export as CSV and it reflects selection

[ozyx]

Testathon 12/28/2022:

Sorting works as expected.
Filters for Value column works but throws errors continually:

Filtering for non-value columns (timestamp, name) works as expected with no errors.

CSV selection and export works as expected.