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

0.8.7b10 colreorder - server side issues #151

Closed
johndegey opened this issue Jan 23, 2015 · 32 comments
Closed

0.8.7b10 colreorder - server side issues #151

johndegey opened this issue Jan 23, 2015 · 32 comments

Comments

@johndegey
Copy link

Hello,

I've tested the beta10...

  1. i remove the inclusion of the colreoder.js file and the capital R from the datatable DOM parameter
    -> if the yadcf are in the footer : no problem, my select filters have the good options and filters the good column
    -> if the adcf filters are in the header, my select filter haven't got the good option value? There is a shift (i have the hidden column and the first with no filter at all).

  2. i do not remove the inclusion of the colreorder.js file but i only remove the capital R from the datable DOM parameter

  • i have an error in firebug and filtering does not work at all
    TypeError: this.context[0]._colReorder is undefined
  1. my previous issue is still present
    yadcf + colreorder + server side : problem with the selected value with select filter #150

Thanks for your work, hope this will help you

@vedmack
Copy link
Owner

vedmack commented Jan 23, 2015

Please provide a working samples that reproduces the issues specified above. And IMO those issues are not related to colreorder because its issues arise only after doing the columns reordering which causing column index to be "confused".

@luivis07
Copy link

Hi Daniel, I'm not sure if this is the issue that OP is referring to but basically what is happening is that there is an additional filter container being created. If you look at the link I'm including here: http://tempdatatables.azurewebsites.net/ (I had to use this instead of a fiddle because it only happens server side). If you drag "Header 4" to position one and then reload the page there will be an additional filter in "name (1)" column.
For debugging purposes I've added two textbox at the top left of the page, the first one indicates the columns that had a filter originally, and the second textbox shows the "column_number" property from the "filter" object passed into the yadcf.init
The table is using ColReorder and StateSave = true.
Let me know if you have any questions or if I can help in anyway. Thanks.

@johndegey
Copy link
Author

Hello,

this one is with filter in the footer
http://yadcf.johndegey.org/listing.php

this one is with filter in the header
http://yadcf.johndegey.org/listing2.php

if you hide field1 and refresh, listing2 will have bad filter values for select

if you switch field2 and field3, next use field3's select filter, you'll see the problem

Maybe i do something in the wrong way, but i do not see what...

Thanks again
John

@luivis07
Copy link

@vedmack by the way the filters get added correctly when using version 0.8.6 maybe that bug was introduced with the fix for colReorder applying the filter to the correct column? #107

vedmack added a commit that referenced this issue Jan 27, 2015
@vedmack
Copy link
Owner

vedmack commented Jan 27, 2015

@johndegey and @luivis07 , go get the 0.8.7.beta.11 , let me know how it works for you (this specific fix is for ColReorder + stateSave: true.

p.s @luivis07 the misplacement of the filters is related to the #119 (but anyway the whole ColReorder / statSave support is a brand new feature and not related to #119)

@johndegey
Copy link
Author

Hello Daniel,

The shift problem when i hide the first column seems to be fix.
Select options in the header or the footer have the good values.

The problem is still there if you apply a filter to col3, then shift col3 and col4 and refresh the page.
Here two new link with the b11.
http://yadcf.johndegey.org/listing3.php

http://yadcf.johndegey.org/listing4.php

John

Edit:

After a few test, i notice that the behavior of the datable was different when you reload the full html page than the one used when you use the next/prev button.
The table's column order is field1, field3 , field2 (the rest is in natural order)

If you reload (Ctrl F5 for windows / cmd R for os X) the html page, the xhr request use the initial position of all the columns. After the response, the datable apply the saved settings and re-order the column.

columns[0][data] field1
columns[0][name] field1
columns[0][orderable] true
columns[0][search][regex] false
columns[0][search][value]
columns[0][searchable] true
columns[1][data] field2
columns[1][name] field2
columns[1][orderable] true
columns[1][search][regex] false
columns[1][search][value]
columns[1][searchable] true
columns[2][data] field3
columns[2][name] field3
columns[2][orderable] true
columns[2][search][regex] false
columns[2][search][value]
columns[2][searchable] true

If you change the table page with next/prev, datatable will send the xhr request with the column ordering as it is right now.

columns[0][data] field1
columns[0][name] field1
columns[0][orderable] true
columns[0][search][regex] false
columns[0][search][value]
columns[0][searchable] true
columns[1][data] field3
columns[1][name] field3
columns[1][orderable] true
columns[1][search][regex] false
columns[1][search][value]
columns[1][searchable] true
columns[2][data] field2
columns[2][name] field2
columns[2][orderable] true
columns[2][search][regex] false
columns[2][search][value]
columns[2][searchable] true

columns[1] is now field3

I think that this may be a problem with the way yadcf works.

When the order is respected in the parameters, i reply with yadcf_data_2 and yadcf_data_3 corresponding to the good column.
But when the page is reloaded, yadcf_data_2 should go to field3, and yadcf_data_3 should go to field2.

Maybe you should do your work after the datatable reordering, or maybe you should change the yadcf_data_columnNumber to yadcf_data_columnName ???

Hope this helps

@vedmack
Copy link
Owner

vedmack commented Jan 28, 2015

@johndegey , please use this yadcf link file in your examples , so will be able to debug / fix it better and assign single meaningull value/filter_default_label into each select filter, e.g in field2 column place single value "field2_val" and assign it with label "field2_label" that will ease the debugging too

@johndegey
Copy link
Author

@vedmack i've made the modifications, is it good for u ? (all links use your file now)

@vedmack
Copy link
Owner

vedmack commented Jan 28, 2015

Nope, please assign single meaningull value to each filter and a meaningful filter_default_label to each filter. thanks

@johndegey
Copy link
Author

ok, all select have just one option value, and i set the filter_default_value for each filter, including the text ones

@vedmack
Copy link
Owner

vedmack commented Jan 29, 2015

I did fixed some, but... I noticed that you send the yadcf_data_X based on the reordering of the columns, the thing is that I expect that X means the original X before the re ordering of columns and yadcf will know to redirect it later on to the new position of X in the table, so my question is: can you fixate that data that belong to the original column number X? (in other words, yadcf_data_2 should hold the same values of the original column 2 no matter its current position) <-- can it be done on your side? While its not a big deal to add additional code on yadcf to take into consideration the new position of the columns when getting the yadcf_data_X I pretty sure that while I debugged your page I noticed that sometimes you are sending wrong data in those yadcf_data_X / yadcf_data__Y etc'. so I think it will be better the send those yadcf_data_X fixed to the original location of the columns.

Regarding the ctrl F5 and next/prev isn't its some datatables inconsistency and not related to yadcf?

@johndegey
Copy link
Author

I only have the column number send by DT. But i don't even have to use it at all because i set the "name" and "data" in the column parameter of DT. (i have to test but normally, i could remove the "name" and just keep the "data")
This allows me to send back rows from my sql request with no post treatment for re-ordering according to actual position in the table.
I use the same server code for multiple templates, so i can't know the original column order.

Maybe i can use some pre-treatment on the client side to add original column number to the xhr request, but i'm not sure this is the good way. Not that i don't want to try, but i really think that the plugin should do the re-ordering.
Based on actual number ?
Based on the column's name parameter ?.
Personally, I would prefer the name attribute.

Here is the portion of code for setting the yadcf_data_x

$this->aColumnsFilterValues = array(
    'field2' => array(array('value' => 'field2_val', 'label' => 'field2_label')),
    'field3' => array(array('value' => 'field3_val', 'label' => 'field3_label')),
    'field4' => array(array('value' => 'field4_val', 'label' => 'field4_label')),
    'field5' => array(array('value' => 'field5_val', 'label' => 'field5_label'))
);
foreach ($_POST['columns'] as $colNumber => $colInfos)
{
    if (array_key_exists($colInfos['name'],$this->aColumnsFilterValues) !== false)
    {
        $output['yadcf_data_'.$colNumber] = $this->aColumnsFilterValues[$colInfos['name']];
    }
}

I don't see why sometimes it should send data to wrong column number.

For the CRTL F5, i don't know... i haven't do debugging for this one yet.

Thx

@vedmack
Copy link
Owner

vedmack commented Feb 2, 2015

Regarding the columns[1][data] field2 (in F5) and columns[1][data] field3 (Paginate to next page) its really wrong IMO , I think that its must be consistent in both scenarios, so its either a your local server bug or a bug in datatables, I mean... why do you pass different values on F5 and on paginate?

@johndegey
Copy link
Author

@luivis07
Copy link

luivis07 commented Feb 4, 2015

@vedmack the fix you applied for stateSave = true, what exactly is it doing? because in the example here : http://tempdatatables.azurewebsites.net/, I'm already getting the ColReorder from localstorage and comparing it to the original order and setting the "column_number" to the correct value.
I think maybe in the fix you are re-applying this logic? You can inspect the element and scroll down the js function GetNewIndex(columnNum, newOrder)

@vedmack
Copy link
Owner

vedmack commented Feb 4, 2015

@luivis07 , please disable your logic on that you have mentioned above (it messes the original column_number and eventually yadcf too) , and also use this yadcf link for better debug, I have noticed that you are not sending any yadcf_data_XXX (where XXX is column number) you must send the yadcf_data_XXX for every filter that supposed to have values to filter the relevant column (select filters for example) otherwise the filter will be loaded with values that are loaded in the first page of the table (read more in the (showcase server example)[http://yadcf-showcase.appspot.com/server_side_source.html] just scroll down and read the comments

@luivis07
Copy link

luivis07 commented Feb 6, 2015

@vedmack after disabling the logic the filter are being added to the right columns (the only thing left now is #138). I do know about the 'yadcf_data_xxx' just didn't bother setting that up for the demo.

@vedmack
Copy link
Owner

vedmack commented Feb 6, 2015

@luivis07 are you using statesave too? And can you confirm it works for F5 and pagination ?

@luivis07
Copy link

luivis07 commented Feb 6, 2015

@vedmack yes I'm using stateSave, pagination work, by F5 you mean refresh right? if so that works too.

@vedmack
Copy link
Owner

vedmack commented Feb 6, 2015

Great news @luivis07 , so... @johndegey , maybe you compare your code with @luivis07 ?

@johndegey
Copy link
Author

@luivis07 : is this the good link for your test case ?
http://tempdatatables.azurewebsites.net/

vedmack added a commit that referenced this issue Feb 7, 2015
@luivis07
Copy link

luivis07 commented Feb 9, 2015

@johndegey yes that should be working now.

@johndegey
Copy link
Author

@vedmack there is now way i can add the original index to the xhr query right now
see here http://datatables.net/forums/discussion/25952/how-to-get-dt-api-in-ajax-data-function-with-server-side#latest

Is there a way that you add the possibility to use column's name like yadcf_data_name ?

@luivis07 your test case is not complete

  • the filtering is not working
  • you never send back any yadcf_data_xxx
  • i can just confirm that reloading the page or using the paging system has the same behavior than my test case
    => is it possible for you to make it work completely on http://tempdatatables.azurewebsites.net/ ?
    (my test case in on my own domain... so i can do what i want...)

John

@vedmack
Copy link
Owner

vedmack commented Feb 9, 2015

@johndegey , I think I understood what I was missing all this time, if you place a breakpoint on $(document).off('xhr.dt', oTable.selector).on('xhr.dt', oTable.selector, function (e, settings, json) { code and debug the F5 flow and pagination flow you will notice that the datatable is in different states, while in F5 the columns are in their original location, in the pagination flow the columns are already in their reordered location, (you can notice this by looking at the frozen browser view... well no I need to rethink how I should approach this case. Since I know now why I get what I get I hope it wont take much time. regarding the yadcf_data_name I guess you can open a new issue for that...

@luivis07
Copy link

@johndegey to be honest, it would take some time for me to set up the filtering and pagination server side since I just put that page up for 1 specific reason. However, I don't see how adding the filtering will help in reproducing the bug? about the yadcf_data_xxx, this goes back to the same thing I just didn't bother setting it up.

@johndegey
Copy link
Author

@luivis07 : without settings a correct yadcf_data_x response, i can't compare with my test case... but i think it is not necessary anymore ;)

@vedmack thanks for your work

@vedmack
Copy link
Owner

vedmack commented Feb 11, 2015

@johndegey , you can test your link now: http://yadcf.johndegey.org/listing4.php , I've tuned some code in yadcf (0.8.7.beta.14 on my dropbox) and it seems to work as expected

@johndegey
Copy link
Author

great... everything works fine, did not find any problem when in the header position
can you do the same when the filters are in the footer
http://yadcf.johndegey.org/listing3.php

i will try again the combination of FixedHeader and the beta14 (did not work previously)

Thx

@vedmack
Copy link
Owner

vedmack commented Feb 12, 2015

Should work now with beta15. I still need to check how show and hide works when filters in footer

@vedmack
Copy link
Owner

vedmack commented Feb 13, 2015

Looks like I'm done with this issue, should work for all combinations of header/footer/hidden columns, regarding the FixedHeader - its another feature that I will need to add support and not related to the current issue...

@johndegey
Copy link
Author

I've found a problem when in the footer (works in the header)
All column in order.
Select 'field_label2' in the field2_select
Put field2 after field3 and then hide field3 => problem with paging and F5

(i will open another case for FixedHeader when this one will be closed)

@vedmack
Copy link
Owner

vedmack commented Feb 15, 2015

@johndegey , 0.8.7.beta.17 is out and it looks like this issue came to an end :) , thanks for all your great test cases.

@vedmack vedmack closed this as completed Feb 15, 2015
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