Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filtered row count from server filtering #326

Closed
camallen opened this issue Jun 4, 2013 · 19 comments
Closed

filtered row count from server filtering #326

camallen opened this issue Jun 4, 2013 · 19 comments

Comments

@camallen
Copy link
Contributor

camallen commented Jun 4, 2013

hi @Mottie

I am using ajax pager with remote filtering and was looking to add the number of filtered rows and the total rows from the server where the filtering is happening.

Looks like the code currently only allows the filtering to be the hidden ("filtered") rows or the totalRows.

What do you think about moving away from an array based ajaxProcessing result to an object with the required properties, totalRows, totalFilteredRows, data, headers, etc? It would clear up the array indexing code and allow easy extension of the returned object.

Thoughts appreciated.
Cam

@Mottie
Copy link
Owner

Mottie commented Jun 4, 2013

The lastest version of the pager allows you to build and apply the table yourself. There are multiple scenarios shown within the ajaxProcessing option documentation. For example, you can just return the total rows with or without a jQuery object containing all of the new tbody content.

As far as filtering, it should be disabled when ajax is active. Adding the {filterList:fcol} variable to the ajaxUrl option allows you to send filter strings to your server.

@Mottie
Copy link
Owner

Mottie commented Jun 5, 2013

I won't have time to look into this further for a few months. So hopefully you'll resolve this issue, or sadly, you'll need to wait for a while. Sorry.

@camallen
Copy link
Contributor Author

camallen commented Jun 5, 2013

Hey @Mottie,

I've read the ajaxProcessing docs extensively and I've been using the ajaxURL {filterList:fcol} options to pass the column and filter constraints to the server.

My initial comments were directed towards the ajaxProcessing returning an array of values, i.e. [ totalRows, data, headers(optional) ] and this code to determine where the various results are in the array. I was proposing the use of a javascript object with the totalRows, data, headers attributes. That would remove the checking code (linked above) and allow more attributes to be set from the server / ajaxProcessing result that currently only allows (only totalRows, data, headers).

Re your comment about disabling filtering when ajax is active, how would the filter header boxes, and events be linked to the table?

I understand you've probably gone on sabbatical but wanted to run this by you before attempting any implementation.

Cheers,
Cam

@TheSin-
Copy link
Collaborator

TheSin- commented Jun 6, 2013

maybe I could help while Mottie is away? I'm not 100% sure I understand your request.

If you are using {filterList:fcol} then you know the filters only set those values and do not hide rows. This being said I'm not sure what you are asking, do you want a new field in the pager so you could do something like.

Showing 1 to 100 of 200 with 1000 filtered

?

@camallen
Copy link
Contributor Author

Hey @TheSin- ,

I'll rephrase my request more clearly.

As you stated, I wanted to push back to the client the number of filtered rows and the total number of rows, something like 1 to 10 of (300 filtered) from 1000 (total). When i looked at the code one has to determine where in the array the total count is in the returned array and if looking to add a new attribute (like number that the filter returns) it would have to be appended to the end of the array as there is fluid but expected ordering.

So this "issue" was to migrate from an array to a javascript object with the relevant attributes set, e.g. { totalRows: X, totalFiltered: Y, data: [], headers: []}. This would free up the array checking code above as one could just tell the object what you want (tell don't ask) and new attributes could be easily added in the future.

I wanted @Mottie's opinion on it before implementing, hence the issue.

Any thoughts appreciated.
Cam

@TheSin-
Copy link
Collaborator

TheSin- commented Jun 10, 2013

this really can not be done via JS with ajax, since the tablesorter ajax call doesn't filter. I currently am doing this but it's a function of the script, I get and set the total on load, then I pass the total via $_GET so I always know the original total, to set the filtered value take the original total - the current total.

rerunning the total before filters every time would be be an extra sql query in your ajax which isn't really the best use of cycles IMHO.

Anyhow point being this should not be done via JS at all. And it would be super easy to set the pager properly on load to display something like

output: '{startRow} to {endRow} of ({totalRows}), unfiltered ($_GET['total'])',

or something like that and no real extra code would be required.

@camallen
Copy link
Contributor Author

@TheSin-, I think we are talking about 2 very different things, please note i am talking about the ajax pager with filters.

All that the ajax is doing is firing a request to my server which takes the params and runs the paging / filtering, e.g. ?page=5&page_size=10&filter_col[0]=test. My server then can then do what it likes, I don't see why the JS client cares about what the server is doing. E.g. my server could get a raw count on the collection as well as return the number of filtered items, there would be no wasted cycles as both of these things i consider important.

If for instance my server could return the array [total, data, headers, num_filtered] OR as as am suggesting the JS object { totalRows: X, data: [], headers: [], num_filtered: Y } then the pager client could just get these values from the ajax result. Absolutely no processing on the client as the server does the paging / filtering.

As per the statement

Anyhow point being this should not be done via JS at all.

None of this is happening in JS, it's all server side, as per total Rows being returned.

Regarding

And it would be super easy to set the pager properly on load to display something like
output: '{startRow} to {endRow} of ({totalRows}), unfiltered ($_GET['total'])',

I don't understand how the "total" var is being set. My suggestion was to set this server side (where the filtering is happening) and return it at the same time as the totalRow, data, headers come back.

Perhaps i should just write the code and provide a pull request so you can see what i am proposing. It's nothing complex, really just modifying the ajax pager to expect an object with the attributes of totalRow, data, headers, and totalFiltered. That way i can easily extend the result from the server / ajaxProcessing to provide more variables for display in the pager.

@TheSin-
Copy link
Collaborator

TheSin- commented Jun 10, 2013

I fully understadn what you are saying, but to know how many are filtered via ajax you'd have to run 2 queries.

once without filters at all to get a total (which is the exact same number as a fresh load), and then a filtered run to get the rows to display.

I already do this with out any need for code and no need to run 2 queries per load.

You just take the total from the initial unfiltered load and store it in the pager load

I use total cause I wanted to keep it as a variable. But it could be static from the first load it wouldn't matter.

The point is that there is no need to add or change anything the way you are thinking about it is much much more complex then it is required to be.

I'm a little busy this morning since it's a monday, but I'll put up an example of it shortly to show you.

@camallen
Copy link
Contributor Author

I see what you are saying but think the original intent to this issue has been hijacked.

On the server side, yes i'd have to run 2 queries, 1 a collection total count (which could very well be cached at the DB level), and the other which is dynamic depending on the filters being applied.

Re storing volatile variables between requests, what if data changes on the backend between first load and any subsequent page / filtering requests? This value would be incorrect, hence the need to receive a fresh copy.

I think the current code is complex and could be simplified (using an object instead of testing attribute locations in an array), e.g.:

remove line 207 completely and lines 227-230 could become

c.totalRows = result[totalRows] || c.totalRows || 0;
d = result[data] || []; // row data
l = d.length; 
//i could add a new attributes, etc.
c.filteredRows = result[filteredRows] || 0;

@TheSin-
Copy link
Collaborator

TheSin- commented Jun 10, 2013

ahhh data set changes, didn't think of that one, that is a very valid point.

I'll dig deeper into it since I'm already using something similar and see if I can convert it.

@camallen
Copy link
Contributor Author

Ok.

So my original thought was to have the ajaxProcessing return an object, e.g.: {totalRows: X, data: [], headers: []} instead of the array.

As per my use case of returning the total count and the filtered count, this way one can extend the object and just get the attributes (as per code above) without breaking any array indexing.

@Mottie
Copy link
Owner

Mottie commented Sep 29, 2013

@camallen: That's a great idea!

In the next update, I'll have the renderAjax function save the ajax data to table.config.pager.ajaxData then if you return an object with ajaxProcessing, any variables within that result will be available for rendering in the display.

For example, if the returned ajax object looks like this:

 {
    "total": 100,
    "headers" : [ "ID", "Name", "Data", "Value" ],
    "rows" : [
        [ "a123", "abc", "xyz", 999 ],
        ...
        [ "z999", "zzz", "zxz", 1 ]
    ],
    "extra" : "I like cheese"
}

and the output option is set with something like this:

output : "{startRow} - {endRow} of {totalRows} of {extra}"

the result would be an output like this:

1 - 10 of 100 rows of I like cheese

But now, since my OCD is in overdrive, I'm wondering if I should include an index for the extra variable, in case the extra variables are saved as an array:

"extra" : [ "I like cheese", "More cheese Gromit!", ... "Last page of cheese!" ]

then you could do this with the output string to base the message on the page*:

output : "{startRow} - {endRow} of {totalRows} of {extra:{page-0}}"

results in:

11 - 20 of 100 rows of More cheese Gromit!

* Note that {page} returns a one-based index by default, so I added the ability to add or subtract numbers from it, similar to the ajaxUrl string replacement method, but since {page} is a one-based index anything with a plus or minus sign makes it start with a zero-based index number, so {page-0} is the zero-based index of the current page... {page+2} would be the zero-based page number index plus 2.

Is this extra stuff too confusing, or totally unnecessary?

@camallen
Copy link
Contributor Author

Hi @Mottie,

Great stuff re the ajaxProcessing result being an object. I think it's more flexible and will allow the server side to return extra variables with almost no extra coding effort. I've already outlined my use case above.

Re indexing the "extra " object attributes, this could be useful, however would the server always return a paged "extra" attribute? I know in my use case that I only wanted to return the number of filtered rows and not paging ones.

If you really want the ability to page into the returned attributes then perhaps you could use a pagingAttrs attribute (see code below) to identify which ones should use the paging index.

Also instead of using an indexable array for these "extra" paging attributes, I think you could just use another object with a positive integer as the attribute name. E.g:

{
...
    "customAttr": "I could put a non-paged value here",
    "pagingAttrs": [ "extra" ],
    "extra" : { 
        0: "I like cheese", 
        1: "More cheese Gromit!", 
        3: "Last page of cheese!"
    }
}

You could then just use the integer attribute in a similar fashion to the array indexing, e.g. extra[1] OR extra[page-0].

A setup like this would allow the library user to add both simple and paging custom attributes to the ajaxProcessing result object and thus make them available for use in the footers, etc.

Thoughts?

@Mottie
Copy link
Owner

Mottie commented Sep 30, 2013

Oh, I only used extra as an example. And no, the server doesn't have to return anything but the row data.

What I'm actually thinking is to make this into a separate module that the pager requires, so you can just load/build a table without requiring the pager (soon to be a widget).

Anyway, the way the code I've set up works is that ANY extra variables within the ajax data would be available:

{
...
    "cheese" : "fromage",
    "bread"   : "pain",
    "cake"    : "gateau",
    "coffee"   : "cafe"
}

Then these variables (including the rows & headers) would be stored within table.config.data and available to be used in the output:

output : 'page {page} of {cheese}, {bread}, {cake} and {coffee}'

Also, the way the script would access the extra variable above would essentially be the same for an object or array: extra[0] would work for either.

If you are interested in a preview of the code I have so far, I can share it here... but, I'll just need to (un)stash the work in git.

@camallen
Copy link
Contributor Author

Hi @Mottie,

Brilliant, allowing any attributes on the ajaxProcessing result to be available in output would fit my use case (server side filtering and paging) and other use cases (normal ajax paging)!

Looking at the array form for your paging custom variables vs what i proposed above - I agree, i think the array form is simpler (as my solution would require the attribute names / enumerate all attributes, so you may as well use an array).

Re I'll let you decide how to modularize your library, as you know it best!

I'd be interested in a preview of what you're proposing, cheers.

@Mottie
Copy link
Owner

Mottie commented Sep 30, 2013

Ack! I lost my work somewhere in stash purgatory.... guess I need to rewrite it again. Maybe it'll be even better ;P

@Mottie
Copy link
Owner

Mottie commented Oct 9, 2013

Hi @camallen!

I finally got around to rewriting that code from scratch...

Please note that internally, I changed c to equal the table.config and p to be table.config.pager to prevent confusion (for me) when reading the code.

I've also included a way to have the server provide custom output strings for each page.

Here is a preview (the line numbers differ, so I left them out) from inside the renderAjax function:

if ( exception ) {
  // ... exception code here
} else {
  // process ajax object
  if (toString.call(result) !== "[object Array]") {
    p.ajaxData = result;
    p.totalRows = result.total;
    th = result.headers;
    d = result.rows;
  } else {
    // allow [ total, rows, headers ]  or [ rows, total, headers ]
    t = isNaN(result[0]) && !isNaN(result[1]);
    //ensure a zero returned row count doesn't fail the logical ||
    rr_count = result[t ? 1 : 0];
    p.totalRows = isNaN(rr_count) ? p.totalRows || 0 : rr_count;
    d = result[t ? 0 : 1] || []; // row data
    th = result[2]; // headers
  }
  l = d.length;
  if (d instanceof jQuery) {
      // build table...

and from the updatePageDisplay function:

// ... inside `updatePageDisplay`
out = p.$container.find(p.cssPageDisplay);
// form the output string (can now get a new output string from the server)
s = ( p.ajaxData && p.ajaxData.hasOwnProperty('output') ? p.ajaxData.output || p.output : p.output )
    // {page} = one-based index; {page+#} = zero based index +/- value
    .replace(/\{page([\-+]\d+)?\}/gi, function(m,n){
        return p.page + (n ? parseInt(n, 10) : 1);
    })
    // {totalPages}, {extra}, {extra:0} (array) or {extra : key} (object)
    .replace(/\{\w+(\s*:\s*\w+)?\}/gi, function(m){
        var t = m.replace(/[{}\s]/g,''), a = t.split(':');
        return a.length > 1 && p.ajaxData && p.ajaxData[a[0]] ? p.ajaxData[a[0]][a[1]] : p[t] || p.ajaxData[t] || '';
    });
if (out.length) {
// ...

@camallen
Copy link
Contributor Author

Hi @Mottie,

Brilliant! I've tested on my codebase and this works really well.

One can now reference expected variables from the server response OR just override the output format on the server response object for custom per response formats, very cool, kudos. Will this change just appear in a new release?

One small change I had to make to get it working (note, i may have confused my p's & c's):

//this was failing as the ajaxData wasn't seutp on the initial load
//thus ajaxData[t] caused an undefined error.
return a.length > 1 && p.ajaxData && p.ajaxData[a[0]] ? p.ajaxData[a[0]][a[1]] : p[t] || p.ajaxData[t] || '';

Became

// ensure the ajax object is setup before attempting to reference an attribute on it, e.g. first load
return a.length > 1 && p.ajaxData && p.ajaxData[a[0]] ? p.ajaxData[a[0]][a[1]] : p[t] || (p.ajaxData ? p.ajaxData[t] : null) || '';

// OR in and easier to read format
if (a.length > 1 && p.ajaxData && p.ajaxData[a[0]]) { 
    return p.ajaxData[a[0]][a[1]]; 
} else {
    return p[t] || (p.ajaxData ? p.ajaxData[t] : null) || '';
}

Please go ahead and close this "issue".

@Mottie
Copy link
Owner

Mottie commented Oct 10, 2013

Thanks @camallen!

I'll make those changes :) And no need for me to close this issue... it will be automatically closed when I push the update.

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

No branches or pull requests

3 participants