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

Fix #131: A mapper function to map the frontend columns data with backend. A new yadcf_data params. Some small improvements. #132

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

kartikeyas00
Copy link

Regarding this issue, I have come up with a _map_columns_with_params private method in DataTables class in datatables.py module. This method compares columns data with the parameters data and map the correct column number to the columns data. As a lot of times in frontend columns are not in the correct order as they are in the backend. Also there are cases when extra dummy columns are added in the frontend and they disturb the sequencing, thus the results coming from the backend.

I am also proposing a new ColumnDT params which is yadcf_data

Why?

When the search method is set as yadcf_select, yadcf_multiselect and yadcf_autocomplete then always yadcf data object is being returned from the backend even when someone doesn't want it as in some cases the data can be defined in the frontend and no data is required from the backend.

For example: If a column contains only yearly data, then one can easily define the options in the frontend for yadcf and backend doesn't have to look at all the records in that column and retrieve the unique records as the option. It can also save server processing.

This PR provides the yadcf_data params for ColumnDT class which can be set to False when we don't want the yadcf_data. It would be default to True

Other improvements proposed

  1. yadcf_multi_select method in search_methods.py module have been improved to handle null from the JavaScript.

  2. In DataTables class add_columns method have been replaced with with_entities method in the query object as add_columns method was adding a column or columns on the top of the list of columns who already exists in the table or query but with_entities replaces the select list with the current entities which means it would only have the columns selected with the with_entities clause.

- new parameter yadcf_data is added to ColumnDT class
  This change is necessary as before when the search method is
  set as yadcf_select, yadcf_multiselect and yadcf_autocomplete
  then always yadcf data is being returned even when it is not
  required by the frontend as in some cases the data can be defined
  in the frontend and no need of data is required from the backend.
  This commit provides the yadcf_data params which can be set to
  False when we don't want the yadcf_data.

- new private method _map_columns_with_params in DataTables class
  This change is necessary as before when in frontend columns are
  not in the same order or extra then compared to frontend, it
  results in the wrong data to be sent to the frontend from the
  server. This method will compare the ColumnsDT data with the
  params data and would map the correct column number in the
  frontend to the ColumnDT data.

 - other improvements
   yadcf_multi_select in search_methods.py have been imporved to
   handle null from the JavaScript.
   In DataTables class add_column method have been changed to
   with_entities method in the query object as add_column add
   a column or columns to the list of columns but with_entities
   replaces the select list with the current entities.
- Add 3 new helper methods
  1) create_dt_params_with_mData
     This create params when the data source is set in the
     frontend
  2) create_dt_params_with_mData_shuffled
     It does the same thing as above but changes the order in
     the frontend as compared to the backend.
  3) create_dt_params_with_mData_with_extra_data
     It does the same thing as Pegase745#1 but adds an extra column in the
     params coming from the frontend.
- New tests
  1) test_with_yacdf_data_params in test_column_dt.py
     test the yacdf_data params
  2) parametrize test_fields_mData in test_fields.py
     so the test could be done on different cases of params
     from frontend
@Pegase745
Copy link
Owner

Thanks for this, will take a look.
Could you possibly revert all the linting diffs to make it easier to read please? Single quotes + 4 spaces indentation are the first differences I see

@tdamsma tdamsma mentioned this pull request Sep 12, 2020
@tdamsma
Copy link
Collaborator

tdamsma commented Sep 12, 2020

@Pegase745, I propose to run black formatter first (see #133), then rebase this PR on to get a proper diff.

@kartikeyas00 I don't fully understand what usecase this is solving. But then I haven't looked at this codebase in a while. If I recall correctly you can either use yadcf (or one of the other options) to

  • pass all data to the client unfiltered, filtering and sorting is done client-side
  • only pass requested data to the client, refresh data with and ajax server call on every filter/sorting interaction

@kartikeyas00
Copy link
Author

I can possibly come up with an example in the next couple of days. But yadcf_data parameter will basically tell the server to either send or not send the options data from the server to client, in case when the client doesn't want it. Also it happens in the case of these three search options yadcf_select, yadcf_multiselect and yadcf_autocomplete. I would suggest you to look at _set_yadcf_data method in datatables.py module.

@kartikeyas00
Copy link
Author

@tdamsma @Pegase745 Any update on this?

@tdamsma
Copy link
Collaborator

tdamsma commented Apr 7, 2021

@tdamsma @Pegase745 Any update on this?

There are still a bunch of merge conflicts, making it really hard to see what the scope of the changes is. Can you resolve them and run black on your code to minimize the diff?

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

Successfully merging this pull request may close these issues.

3 participants