Tests and better implementation for _col/_nocol, refs #615

- Allow both ?_col= and ?_nocol=
- De-duplicate if ?_col= passed multiple times
- 400 error if user tries to ?_nocol= a primary key

datasette/views/table.py

@@ -66,33 +66,38 @@ def __str__(self):
 class RowTableShared(DataView):
     async def columns_to_select(self, db, table, request):
         table_columns = await db.table_columns(table)
-        if "_col" in request.args and "_nocol" in request.args:
-            raise DatasetteError("Cannot use _col and _nocol at the same time")
+        pks = await db.primary_keys(table)
+        columns = list(table_columns)
         if "_col" in request.args:
-            new_columns = []
-            for column in request.args.getlist("_col"):
-                if column not in table_columns:
-                    raise DatasetteError("_col={} is an invalid column".format(column))
-                new_columns.append(column)
-            return new_columns
-        elif "_nocol" in request.args:
+            columns = list(pks)
+            _cols = request.args.getlist("_col")
+            bad_columns = [column for column in _cols if column not in table_columns]
+            if bad_columns:
+                raise DatasetteError(
+                    "_col={} - invalid columns".format(", ".join(bad_columns)),
+                    status=400,
+                )
+            # De-duplicate maintaining order:
+            columns.extend(dict.fromkeys(_cols))
+        if "_nocol" in request.args:
             # Return all columns EXCEPT these
             bad_columns = [
                 column
                 for column in request.args.getlist("_nocol")
-                if column not in table_columns
+                if (column not in table_columns) or (column in pks)
             ]
             if bad_columns:
                 raise DatasetteError(
-                    "_nocol={} - invalid columns".format(", ".join(bad_columns))
+                    "_nocol={} - invalid columns".format(", ".join(bad_columns)),
+                    status=400,
                 )
-            return [
+            tmp_columns = [
                 column
-                for column in table_columns
+                for column in columns
                 if column not in request.args.getlist("_nocol")
             ]
-        else:
-            return table_columns
+            columns = tmp_columns
+        return columns
 
     async def sortable_columns_for_table(self, database, table, use_rowid):
         db = self.ds.databases[database]

tests/test_api.py

@@ -2009,3 +2009,56 @@ def test_http_options_request(app_client):
     response = app_client.request("/fixtures", method="OPTIONS")
     assert response.status == 200
     assert response.text == "ok"
+
+
+@pytest.mark.parametrize(
+    "path,expected_columns",
+    (
+        ("/fixtures/facetable.json?_col=created", ["pk", "created"]),
+        (
+            "/fixtures/facetable.json?_nocol=created",
+            [
+                "pk",
+                "planet_int",
+                "on_earth",
+                "state",
+                "city_id",
+                "neighborhood",
+                "tags",
+                "complex_array",
+                "distinct_some_null",
+            ],
+        ),
+        (
+            "/fixtures/facetable.json?_col=state&_col=created",
+            ["pk", "state", "created"],
+        ),
+        (
+            "/fixtures/facetable.json?_col=state&_col=state",
+            ["pk", "state"],
+        ),
+        (
+            "/fixtures/facetable.json?_col=state&_col=created&_nocol=created",
+            ["pk", "state"],
+        ),
+    ),
+)
+def test_col_nocol(app_client, path, expected_columns):
+    response = app_client.get(path)
+    assert response.status == 200
+    columns = response.json["columns"]
+    assert columns == expected_columns
+
+
+@pytest.mark.parametrize(
+    "path,expected_error",
+    (
+        ("/fixtures/facetable.json?_col=bad", "_col=bad - invalid columns"),
+        ("/fixtures/facetable.json?_nocol=bad", "_nocol=bad - invalid columns"),
+        ("/fixtures/facetable.json?_nocol=pk", "_nocol=pk - invalid columns"),
+    ),
+)
+def test_col_nocol_errors(app_client, path, expected_error):
+    response = app_client.get(path)
+    assert response.status == 400
+    assert response.json["error"] == expected_error