From 386fb11d42767039bb2b389ce98996673d780a42 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Sun, 19 Nov 2017 12:25:29 -0800
Subject: [PATCH] ?_filter_column=col&_filter_op=op&_filter_value=value
 redirect

Part of implementing the filters UI (refs #86) - the following:

    /trees/Trees?_filter_column=SiteOrder&_filter_op=gt&_filter_value=2

Now redirects to this;

    /trees/Trees?SiteOrder__gt=2
---
 datasette/app.py   | 22 +++++++++++++++++++---
 datasette/utils.py | 14 +++++++++++---
 tests/test_app.py  | 23 +++++++++++++++++++++++
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/datasette/app.py b/datasette/app.py
index 1a6200e8..9c0d29f1 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -57,7 +57,7 @@ class BaseView(HTTPMethodView):
         return r
 
     def redirect(self, request, path):
-        if request.query_string:
+        if request.query_string and '?' not in path:
             path = '{}?{}'.format(
                 path, request.query_string
             )
@@ -182,9 +182,13 @@ class BaseView(HTTPMethodView):
         template = self.template
         status_code = 200
         try:
-            data, extra_template_data = await self.data(
+            response_or_template_contexts = await self.data(
                 request, name, hash, **kwargs
             )
+            if isinstance(response_or_template_contexts, response.HTTPResponse):
+                return response_or_template_contexts
+            else:
+                data, extra_template_data = response_or_template_contexts
         except (sqlite3.OperationalError, InvalidSql) as e:
             data = {
                 'ok': False,
@@ -422,6 +426,18 @@ class TableView(BaseView):
             else:
                 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 ''
+            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,
+            }))
+
         if other_args:
             where_clauses, params = build_where_clauses(other_args)
         else:
@@ -433,7 +449,7 @@ class TableView(BaseView):
         fts_sql = detect_fts_sql(table)
         fts_rows = list(await self.execute(name, fts_sql))
         if fts_rows:
-            fts_table=fts_rows[0][0]
+            fts_table = fts_rows[0][0]
 
         search = special_args.get('_search')
         if search and fts_table:
diff --git a/datasette/utils.py b/datasette/utils.py
index 1b0d3531..a4badf7e 100644
--- a/datasette/utils.py
+++ b/datasette/utils.py
@@ -120,9 +120,17 @@ def validate_sql_select(sql):
 
 
 def path_with_added_args(request, args):
-    current = request.raw_args.copy()
-    current.update(args)
-    return request.path + '?' + urllib.parse.urlencode(current)
+    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 value is not None
+    })
+    return request.path + '?' + urllib.parse.urlencode(sorted(current.items()))
 
 
 def path_with_ext(request, ext):
diff --git a/tests/test_app.py b/tests/test_app.py
index 17d1fc30..5df5ea25 100644
--- a/tests/test_app.py
+++ b/tests/test_app.py
@@ -4,6 +4,7 @@ import pytest
 import sqlite3
 import tempfile
 import time
+import urllib.parse
 
 
 @pytest.fixture(scope='module')
@@ -227,6 +228,28 @@ def test_row(app_client):
     assert [{'pk': '1', 'content': 'hello'}] == response.json['rows']
 
 
+def test_add_filter_redirects(app_client):
+    filter_args = urllib.parse.urlencode({
+        '_filter_column': 'content',
+        '_filter_op': 'startswith',
+        '_filter_value': 'x'
+    })
+    # First we need to resolve the correct path before testing more redirects
+    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('?content__startswith=x')
+
+    # Adding a redirect to an existing querystring:
+    path = path_base + '?foo=bar&' + filter_args
+    response = app_client.get(path, allow_redirects=False, gather_request=False)
+    assert response.status == 302
+    assert response.headers['Location'].endswith('?content__startswith=x&foo=bar')
+
+
 TABLES = '''
 CREATE TABLE simple_primary_key (
   pk varchar(30) primary key,