Add filters to lists in dasboards #1299

koradon · 2017-11-14T09:02:36Z

Add filter capability for lists in dashboard.

sorting the list by clicking on the column names
filters corresponding with the displayed columns
UI should be compliant with Material Design. These designs include filter icon in the table header.
combine fields name, last name and email to one filter
add fancy datepicker for filters with dates and dates ranges

codecov-io · 2017-11-14T09:24:02Z

Codecov Report

Merging #1299 into master will increase coverage by 0.01%.
The diff coverage is 87.3%.

@@            Coverage Diff             @@
##           master    #1299      +/-   ##
==========================================
+ Coverage   81.97%   81.98%   +0.01%     
==========================================
  Files         115      123       +8     
  Lines        5598     5746     +148     
  Branches      644      667      +23     
==========================================
+ Hits         4589     4711     +122     
- Misses        850      878      +28     
+ Partials      159      157       -2

Impacted Files	Coverage Δ
saleor/dashboard/order/forms.py	`67.98% <ø> (-0.28%)`	⬇️
saleor/dashboard/discount/views.py	`69.62% <100%> (+1.19%)`	⬆️
saleor/dashboard/group/views.py	`100% <100%> (ø)`	⬆️
saleor/dashboard/group/filters.py	`100% <100%> (ø)`
saleor/core/templatetags/shop.py	`94.87% <100%> (+4.87%)`	⬆️
saleor/core/templatetags/materializecss.py	`89.47% <100%> (+1.71%)`	⬆️
saleor/dashboard/customer/filters.py	`100% <100%> (ø)`
saleor/dashboard/category/views.py	`57.69% <100%> (+1.11%)`	⬆️
saleor/dashboard/product/views.py	`70.07% <100%> (+0.46%)`	⬆️
saleor/dashboard/product/filters.py	`100% <100%> (ø)`
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7922c5...c44ad28. Read the comment docs.

Pacu2 · 2017-11-17T10:01:31Z

saleor/core/utils/filters.py

+
+def get_sort_by_choices(filter):
+    return [(choice[0], choice[1].lower()) for choice in
+            filter.filters['sort_by'].field.choices[1::2]]


You should not name your variables after built-in functions, please rename filter to something else

Any suggestions? I wanted to change it but...

We could name it filter_set after the class from django-filters.

Pacu2 · 2017-11-17T10:01:54Z

saleor/core/utils/filters.py

+
+
+def get_now_sorted_by(filter, fields, default_sort='name'):
+    sort_by = filter.form.cleaned_data.get('sort_by')


Same as above

Pacu2 · 2017-11-17T10:04:17Z

saleor/dashboard/order/views.py

-    else:
-        orders = orders_all.all()
+    orders = (Order.objects.prefetch_related(
+        'groups', 'payments', 'groups__items', 'user').order_by('-pk'))


When you prefetch some_field__somefields_field then some_field will be automatically prefetched.

Yea, this is not my code but i will change it.

I mean, I only added order_by and delete if :)
But this is exactly same code from invoice ;)

Pacu2 · 2017-11-17T10:07:03Z

templates/dashboard/includes/_filters.html

+    <div class="card-content">
+      <div class="col s12">
+        <span class="card-title">{% trans 'Filters' context 'Dashboard card title' %}</span>
+        <span>{% trans 'Total records found' context 'Dashboard filter' %}: {{ filter.qs|length }}</span>


Can we use count here? I think passing it from the backend would be better, as usually, we are trying to avoid templates firing up additional queries.

I will check if this triggers query.

It does. In case of a large dataset this would be really slow. Actually we don't display the number of items in queryset in the list views. Maybe it would be better to add it consistently to all lists in a different PR?

Hmmm i don't think that another PR is required. I already have to do this so why not now?

If we add it to every list then it's fine in this one.

Is knowing the precise number of results useful? When is it useful? When is it not? It would be nice to have some use cases before we add something to all list views.

For the reason @patrys just mentioned I would split it out to another issue/PR.

From the other hand we need a visual feedback that there's some filtering being applied and number of records found would partially serve that purpose.

Pacu2 · 2017-11-17T10:08:22Z

templates/dashboard/includes/_filters.html

+      <div class="row">
+        {% for field in filter.form %}
+          <div class="col s12">
+            {% if field.name == 'sort_by' %}


Not a fan of, I think rendering those fields separately would be a more error-proof option.

Those fields are taken from many filters, adding every possible field would create enormous if statement.

Pacu2 · 2017-11-17T10:09:38Z

templates/dashboard/customer/list.html

@@ -1,6 +1,7 @@
 {% extends "dashboard/base.html" %}
 {% load i18n %}
 {% load prices_i18n %}
+{% load shop %}


If you're using only one function, it would be more convenient to use {% load function from shop %}

Will do. But I think we need PR which will make those changes for every template for every templatetag library

maarcingebala · 2017-11-20T09:56:02Z

saleor/settings.py

@@ -289,7 +289,7 @@ def get_host():

 TEST_RUNNER = ''

-ALLOWED_HOSTS = get_list(os.environ.get('ALLOWED_HOSTS', 'localhost'))
+ALLOWED_HOSTS = ['*']


This looks like local settings.

koradon · 2017-11-23T10:18:06Z

@dominik-zeglen RangeWidget and DateRangeWidget have some issues with 'clicking aea'. Could you investigate it?

maarcingebala · 2017-11-27T12:30:03Z

@dominik-zeglen @koradon I've reviewed the UI and have a few remarks:

sort icons appear only in the product list view
I think the "filter" button should be flat; most buttons are flat e.g. in product details page or order details
there is an issue with opening the datepicker widget for the date or date-range filters
the text color in table headers should be always the same (regardless of whether the sorting is allowed or not), but we should show maybe some underline when hovering on headers of sortable columns?
we could show the "clear" button only when filters are enabled
include currency in price filters (see product edit form for reference)
add filtering by name in stock locations list view

Pacu2 · 2017-11-28T10:09:13Z

saleor/dashboard/templatetags/chips.py

+def handle_nullboolean(field):
+    value = yesno(
+        field.value(),
+        pgettext_lazy('Possible values of boolean filter', 'yes,no,all'))


descriptive 👍

Pacu2 · 2017-11-28T10:10:00Z

saleor/dashboard/templatetags/chips.py

+from django.template.defaultfilters import yesno
+from django.utils.translation import pgettext_lazy
+
+PATTERN = '%s: %s'


PATTERN of ..?

Chip formatting

Pacu2 · 2017-11-28T10:10:21Z

saleor/dashboard/templatetags/chips.py

@@ -0,0 +1,65 @@
+from django.template.defaultfilters import yesno


Missing unicode_literals import

Pacu2 · 2017-11-28T10:12:01Z

saleor/dashboard/staff/filters.py

+        label=pgettext_lazy('Staff list filter label', 'Groups'),
+        name='groups',
+        queryset=Group.objects.all())
+    is_active = ChoiceFilter(


Can we use BooleanField here? I think it's more obvious than returning '1' or '0' as str

Unfortunately BooleanFilter does not have 'empty_label' and 'choices' params. Sience we want a custom string 'All' if no option was chosen and a custom choices like 'Published'/'Not published' I had to use ChoiceFilter. And this is a recommended approach for this kind of customization.

Pacu2 · 2017-11-28T10:14:34Z

saleor/dashboard/product/filters.py

+        choices=PUBLISHED_CHOICES,
+        empty_label=pgettext_lazy('Filter empty choice label', 'All'),
+        widget=forms.Select)
+    is_featured = ChoiceFilter(


Same as with is_active

maarcingebala · 2017-11-28T11:04:55Z

saleor/core/templatetags/shop.py

+    request_get = request.GET.copy()
+    sort_by = request_get.get('sort_by')
+    arrow_src = ''
+    active = ''


I would rather expect a boolean variable is_active. If I'm correct below a CSS class is set as value of this variable which is not a good practice.

Yes, i wanted less logic in template

maarcingebala · 2017-11-28T11:05:56Z

saleor/core/templatetags/shop.py

+            if field == sort_by[1:]:
+                arrow_src = static('/images/arrow_down_icon.svg')
+                active = 'active'
+            else:


When this case happens? Isn't setting of these values already covered in lines 41 and 42?

Yes, some old code i guess. Deleted

maarcingebala · 2017-11-28T11:07:19Z

saleor/core/utils/filters.py

+from django.db.models import Q
+
+
+def get_sort_by_choices(filter_set):


We should add docstrings to all of these functions.

maarcingebala · 2017-11-28T11:12:33Z

saleor/core/templatetags/shop.py

+    request = context['request']
+    request_get = request.GET.copy()
+    sort_by = request_get.get('sort_by')
+    arrow_src = ''


I would rename it to sorting_icon. If we changed from arrows to something else we won't have to update the variable names :)

Add filters.py to dashboard/product. Update product_list view. Update dashboard/product/list.html.

Add template dashboard/_filters.html. Add include tag.

Move get_sort_by_choices and get_now_sorted_by to core.utils.filters.py.

Add filters to order list. Change include path from relative to absolute. Move _filters.html from dashboard to dashboard/includes. Delete _status_filters.html.

Add sort_by links to order and customer lists.

Move include _filters before check if QuerySet exists.

Change PATTERN to CHIPS_PATTERN.

Return just sort_by GET value instead of whole request.

patrys · 2017-12-04T16:16:18Z

saleor/dashboard/templatetags/chips.py

+    """Build a list of chips for NullBooleanField field."""
+    value = yesno(
+        field.value(),
+        pgettext_lazy('Possible values of boolean filter', 'yes,no,all'))


Please don't do this. If that's the only way to pass data do yesno then please don't use yesno as well.

koradon added dashboard in progress labels Nov 14, 2017

koradon added this to the 2017.11 milestone Nov 14, 2017

koradon self-assigned this Nov 14, 2017

koradon force-pushed the feature/dashboard_filters branch from 3a0ecd6 to aad9523 Compare November 15, 2017 08:41

koradon assigned dominik-zeglen Nov 16, 2017

Pacu2 reviewed Nov 17, 2017

View reviewed changes

maarcingebala reviewed Nov 20, 2017

View reviewed changes

koradon force-pushed the feature/dashboard_filters branch from 4bbee55 to 48b1b8a Compare November 22, 2017 10:40

koradon force-pushed the feature/dashboard_filters branch 2 times, most recently from 5ce3b88 to 4597cbd Compare November 27, 2017 12:15

Pacu2 reviewed Nov 28, 2017

View reviewed changes

maarcingebala reviewed Nov 28, 2017

View reviewed changes

koradon mentioned this pull request Nov 28, 2017

Hide "Clear filters" in when no filter applied #1376

Closed

koradon force-pushed the feature/dashboard_filters branch from 4be9cad to 21b67a1 Compare November 29, 2017 06:29

koradon added 11 commits November 29, 2017 15:24

Add filters for product list in dashboard

492b2af

Add filters.py to dashboard/product. Update product_list view. Update dashboard/product/list.html.

Update template

1e84cc8

Add template dashboard/_filters.html. Add include tag.

Update _fiters.html

6db8e49

Add filters to Sale list

0faf2f7

Move get_sort_by_choices and get_now_sorted_by to core.utils.filters.py.

Add sorting by clicking column headers

f148ba5

Add filters to voucher list

02b103d

Delete unused context data

254b6ae

Add clear filters link, remove irrelevant fields from VoucherFilter

f70e711

Add filter to Orders

50c1129

Add filters to order list. Change include path from relative to absolute. Move _filters.html from dashboard to dashboard/includes. Delete _status_filters.html.

Add filters to customer list

ca4f69a

Add sort_by links to order and customer lists.

Add filters for staff members

dd9d6cd

Move include _filters before check if QuerySet exists.

dominik-zeglen and others added 18 commits November 29, 2017 15:44

Fix non-clickable datepickers

877316b

Build query server-side

c781c3d

Fix apearing and disapearing of 'Clear' button in _filters.html

62858b5

Fix pep8

c082842

Add changes from review

f2c4fa2

Add missing import unicode_literals to chips.py

c693e73

Change PATTERN to CHIPS_PATTERN.

Fix pep8

654a9d5

Add self.currency to PriceRangeWidget

6f0416c

Fix styles

661f460

Delete duplicated 'Add' buttons from voucher and sale lists

c6e52de

Add price range input formatting

b0687c4

Fix "name" column width

75e1017

Move sort arrow to right side

d1b580b

Add PriceRangeWidget to total_net filter in OrderFilter

e31259d

Fix another column's width

5b15401

Fix sort_by not working after filtering

78fb0cf

Refactor chips logic

8000350

Preserve lists of parameters when generating cancel links

1291754

koradon force-pushed the feature/dashboard_filters branch from e0ea595 to 1291754 Compare November 29, 2017 14:44

maarcingebala and others added 8 commits November 29, 2017 16:37

Fix enabling proper sort_by

2dd2d61

Align sorter arrow to inner padding-box border

2ca6b4b

Unify content container width

060ace3

Change default text alignment in last columns to left

e63b3e9

Add tests for get_sort_by templatetags

2d036b4

Change params returned by add_filters

c46bc8e

Return just sort_by GET value instead of whole request.

Correct comment in get_sort_by_toggle

ed0df7c

Add tests for chips

c44ad28

maarcingebala merged commit a16bad9 into master Nov 30, 2017

maarcingebala deleted the feature/dashboard_filters branch November 30, 2017 14:19

patrys reviewed Dec 4, 2017

View reviewed changes



		def get_now_sorted_by(filter, fields, default_sort='name'):
		sort_by = filter.form.cleaned_data.get('sort_by')

		@@ -0,0 +1,65 @@
		from django.template.defaultfilters import yesno

		from django.db.models import Q


		def get_sort_by_choices(filter_set):

Add filters to lists in dasboards #1299

Add filters to lists in dasboards #1299

Conversation

koradon commented Nov 14, 2017 • edited by dominik-zeglen

codecov-io commented Nov 14, 2017 • edited

Codecov Report

Pacu2 Nov 17, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirekm Nov 17, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koradon commented Nov 23, 2017

maarcingebala commented Nov 27, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pacu2 Nov 28, 2017 • edited

Choose a reason for hiding this comment

koradon Nov 28, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koradon commented Nov 14, 2017 •

edited by dominik-zeglen

codecov-io commented Nov 14, 2017 •

edited

Pacu2 Nov 17, 2017 •

edited

mirekm Nov 17, 2017 •

edited

maarcingebala commented Nov 27, 2017 •

edited

Pacu2 Nov 28, 2017 •

edited

koradon Nov 28, 2017 •

edited