From 53534b6e9d3956cdb6aa9c062d2beffdd89fa7d9 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Wed, 22 Nov 2017 20:03:46 -0800
Subject: [PATCH] Even more complicated redirect scheme

This:

    ?_filter_column_1=name&_filter_op_1=contains&_filter_value_1=hello
    &_filter_column_2=age&_filter_op_2=gte&_filter_value_2=12

Now redirects to this:

    ?name__contains=hello&age__gte=12

This is needed for the filter editing interface, refs #86
---
 datasette/app.py   | 16 ++++---------
 datasette/utils.py | 57 ++++++++++++++++++++++++++++++++++++++--------
 tests/test_app.py  | 26 +++++++++++++++++++++
 3 files changed, 78 insertions(+), 21 deletions(-)

diff --git a/datasette/app.py b/datasette/app.py
index 6ce8c35e..0b4a7e7e 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -22,6 +22,7 @@ from .utils import (
     detect_fts_sql,
     escape_css_string,
     escape_sqlite_table_name,
+    filters_should_redirect,
     get_all_foreign_keys,
     InvalidSql,
     path_from_row_pks,
@@ -425,18 +426,9 @@ class TableView(BaseView):
                 other_args[key] = value[0]
 
         # Handle ?_filter_column and redirect, if present
-        if '_filter_column' in special_args:
-            filter_column = special_args['_filter_column']
-            filter_op = special_args.get('_filter_op') or ''
-            filter_value = special_args.get('_filter_value') or ''
-            if '__' in filter_op:
-                filter_op, filter_value = filter_op.split('__', 1)
-            return self.redirect(request, path_with_added_args(request, {
-                '{}__{}'.format(filter_column, filter_op): filter_value,
-                '_filter_column': None,
-                '_filter_op': None,
-                '_filter_value': None,
-            }))
+        redirect_params = filters_should_redirect(special_args)
+        if redirect_params:
+            return self.redirect(request, path_with_added_args(request, redirect_params))
 
         filters = Filters(sorted(other_args.items()))
         where_clauses, params = filters.build_where_clauses()
diff --git a/datasette/utils.py b/datasette/utils.py
index c6ab18c8..52d145b2 100644
--- a/datasette/utils.py
+++ b/datasette/utils.py
@@ -78,17 +78,20 @@ def validate_sql_select(sql):
 
 
 def path_with_added_args(request, args):
-    current = {
-        key: value
+    if isinstance(args, dict):
+        args = args.items()
+    arg_keys = set(a[0] for a in args)
+    current = [
+        (key, value)
         for key, value in request.raw_args.items()
-        if key not in args
-    }
-    current.update({
-        key: value
-        for key, value in args.items()
+        if key not in arg_keys
+    ]
+    current.extend([
+        (key, value)
+        for key, value in args
         if value is not None
-    })
-    return request.path + '?' + urllib.parse.urlencode(sorted(current.items()))
+    ])
+    return request.path + '?' + urllib.parse.urlencode(sorted(current))
 
 
 def path_with_ext(request, ext):
@@ -384,3 +387,39 @@ class Filters:
                     params[param_id] = param
         return sql_bits, params
         return ' and '.join(sql_bits), params
+
+
+filter_column_re = re.compile(r'^_filter_column_\d+$')
+
+
+def filters_should_redirect(special_args):
+    print('special_args: ', special_args)
+    redirect_params = []
+    if '_filter_column' in special_args:
+        filter_column = special_args['_filter_column']
+        filter_op = special_args.get('_filter_op') or ''
+        filter_value = special_args.get('_filter_value') or ''
+        if '__' in filter_op:
+            filter_op, filter_value = filter_op.split('__', 1)
+        redirect_params.extend([
+            ('{}__{}'.format(filter_column, filter_op), filter_value),
+            ('_filter_column', None),
+            ('_filter_op', None),
+            ('_filter_value', None),
+        ])
+    # Now handle _filter_column_1=name&_filter_op_1=contains&_filter_value_1=hello
+    column_keys = [k for k in special_args if filter_column_re.match(k)]
+    for column_key in column_keys:
+        number = column_key.split('_')[-1]
+        column = special_args[column_key]
+        op = special_args.get('_filter_op_{}'.format(number)) or 'exact'
+        value = special_args.get('_filter_value_{}'.format(number)) or ''
+        if '__' in op:
+            op, value = op.split('__', 1)
+        redirect_params.extend([
+            ('{}__{}'.format(column, op), value),
+            ('_filter_column_{}'.format(number), None),
+            ('_filter_op_{}'.format(number), None),
+            ('_filter_value_{}'.format(number), None),
+        ])
+    return redirect_params
diff --git a/tests/test_app.py b/tests/test_app.py
index ab17143a..d9ebe5dd 100644
--- a/tests/test_app.py
+++ b/tests/test_app.py
@@ -278,6 +278,32 @@ def test_add_filter_redirects(app_client):
     assert response.headers['Location'].endswith('?content__isnull=5')
 
 
+def test_existing_filter_redirects(app_client):
+    filter_args = urllib.parse.urlencode({
+        '_filter_column_1': 'name',
+        '_filter_op_1': 'contains',
+        '_filter_value_1': 'hello',
+        '_filter_column_2': 'age',
+        '_filter_op_2': 'gte',
+        '_filter_value_2': '22',
+        '_filter_column_3': 'age',
+        '_filter_op_3': 'lt',
+        '_filter_value_3': '30',
+        '_filter_column_4': 'name',
+        '_filter_op_4': 'contains',
+        '_filter_value_4': 'world',
+    })
+    path_base = app_client.get(
+        '/test_tables/simple_primary_key', allow_redirects=False, gather_request=False
+    ).headers['Location']
+    path = path_base + '?' + filter_args
+    response = app_client.get(path, allow_redirects=False, gather_request=False)
+    assert response.status == 302
+    assert response.headers['Location'].endswith(
+        '?age__gte=22&age__lt=30&name__contains=hello&name__contains=world'
+    )
+
+
 TABLES = '''
 CREATE TABLE simple_primary_key (
   pk varchar(30) primary key,