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

Add custom filters, extra data rows, more formatting #26

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

Conversation

apyrgio
Copy link

@apyrgio apyrgio commented Jun 19, 2014

Hi @noirbizarre,

I've been using this plugin for some time now and I must say that I've come to enjoy it a lot. Granted, I think there are a few things missing, so I decided to contribute to this project by adding them. I haven't managed to add documentation for them yet, but I provide explanations in the commit logs.

Side note: Given that it's been nine months since this repo has been updated, may I ask if it's still maintained?

@thepapermen
Copy link

Wow! That's really interesting!

However, note, that python < 2.7 doesn't support dict comprehensions.

@apyrgio
Copy link
Author

apyrgio commented Jun 19, 2014

I've fixed most of my issues (I think I have only one left). The main problem however is that the master branch of django-eztables itself cannot pass the tests, probably because Django has changed something in its url resolving since the last time they ran.

A typical strack trace is the following:

  File "/home/apyrgio/playground/django-eztables/eztables/tests.py", line 763, in test_column_search_custom
    response = self.get_response('custom-browsers', self.build_query(sSearch_1='1.1'))
  File "/home/apyrgio/playground/django-eztables/eztables/tests.py", line 1016, in get_response
    return self.client.post(reverse(name), data)
  File "/home/apyrgio/playground/django-eztables/.tox/py27/lib/python2.7/site-packages/django/core/urlresolvers.py", line 481, in reverse
    resolver = get_resolver(urlconf)
  File "/home/apyrgio/playground/django-eztables/.tox/py27/lib/python2.7/site-packages/django/utils/functional.py", line 30, in wrapper
    if mem_args in cache:
TypeError: unhashable type: 'list'

I'll look into it...

@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) when pulling 698063e on apyrgio:feature-synnefo-integration into daeff68 on noirbizarre:master.

@apyrgio
Copy link
Author

apyrgio commented Jun 20, 2014

The PR has been updated with a commit that fixes the failing tests that have to do with the url resolving.

Briefly put, the problem occurred in tests that requested the same url more than once. By adding another url resolver, it doesn't complain anymore. You can see more in the commit log of the fix.

Note that, the fix is rather hackish since I didn't have the time (or the necessary experience) to look into the Django internals. If @noirbizarre has time, I'd suggest he also take a look at it.

@sylvain-josserand
Copy link

The same CI problems happened with my pull request #24
I think I fix the problems for good, there were some incompatibilities with the new django 1.6 version and some other problems with python 3.4
Before anyone else fix these problems yet again, would it be possible to merge those fix?

@apyrgio
Copy link
Author

apyrgio commented Jun 20, 2014

Yes, I believe that the fix you proposed is much cleaner and consistent with the Django documentation.

Alex Pyrgiotis added 10 commits October 22, 2014 20:10
Allow the user to choose between server-side and client-side processing,
while still asking the data from the server in array or object format.
The user can set a "get_extra_data_row" method, in order to
collect extra data for each table row.
Instead of using always values(**...), we should use values_list, if the
user has asked for an array-based JSON.
Add `format_data_rows` method that checks if the user has set a
`format_data_row` method and maps it to each element of the aaData list.

Also, optimize row formatting, meaning that instead of always executing
a reqex for every field of every row, run it only when we're sure that
we have a formatted field.
The user can trigger his/her custom filters by sending a "sSearch_*"
argument in the Datatables request.

The user can add his/her own filters in the `filters` field of the
DatatablesView class or define one or more `filter_*` methods in this
class.  The latter way is similar to the custom sorting approach.

The former way is a bit more flexible and can integrate with the popular
django-filter plugin. More specifically, filters added in the `filters`
field must have one of the following formats:

* List format: [filter1, filter2, filter3]
  Note that this format requires that the filters have a `name`
  attribute and a `filter` method.
* Dict format: {'name1': filter1, 'name2': filter2, 'name3': filter3}
  Note that this format requires that the filters have a `filter`
  method.
* Django-filter FilterSet format: Simply a FilterSet class

In order to trigger a filter, its name must match the asterisk part of a
Datatables "sSearch_*" argument. The value of this argument will be
considered as the query for this filter.

Also, note that all list/dict filters should take as an argument a query
and a queryset and return a filtered queryset.
@coveralls
Copy link

Coverage Status

Coverage increased (+1.8%) when pulling 1f625f1 on apyrgio:feature-synnefo-integration into 347e74d on noirbizarre:master.

@apyrgio
Copy link
Author

apyrgio commented Oct 22, 2014

Hi @noirbizarre,

I had some time today to work on this pull request. I have added some documentation for the features that this pull request introduces (client-side processing, custom formatting, custom filtering with django-filter integration, extra data for each table row), so hopefully they are more clear to you. If you are interested in accepting these features, then I'd be more than happy to go through them with you and improve them. If not then, oh well, that'd be a bummer...

P.S. The only reason that this Travis CI build fails is because Django 1.7 does not support Python 2.6. The rest of the tests pass.

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.

4 participants