Longer time limit for test_paginate_compound_keys

It was failing intermittently in Travis - see #209

datasette/app.py

@@ -524,15 +524,21 @@ class RowTableShared(BaseView):
                 cells.append({
                     'column': 'Link',
                     'value': jinja2.Markup(
-                        '<a href="/{database}/{table}/{flat_pks}">{flat_pks}</a>'.format(
+                        '<a href="/{database}/{table}/{flat_pks_quoted}">{flat_pks}</a>'.format(
                             database=database,
                             table=urllib.parse.quote_plus(table),
-                            flat_pks=path_from_row_pks(row, pks, not pks),
+                            flat_pks=str(jinja2.escape(path_from_row_pks(row, pks, not pks, False))),
+                            flat_pks_quoted=path_from_row_pks(row, pks, not pks)
                         )
                     ),
                 })
+
             for value, column_dict in zip(row, columns):
                 column = column_dict['name']
+                if link_column and len(pks) == 1 and column == pks[0]:
+                    # If there's a simple primary key, don't repeat the value as it's
+                    # already shown in the link column.
+                    continue
                 if (column, value) in labeled_fks:
                     other_table, label = labeled_fks[(column, value)]
                     display_value = jinja2.Markup(
@@ -559,17 +565,17 @@ class RowTableShared(BaseView):
                             url=jinja2.escape(value.strip())
                         )
                     )
+                elif column in table_metadata.get('units', {}) and value != '':
+                    # Interpret units using pint
+                    value = value * ureg(table_metadata['units'][column])
+                    # Pint uses floating point which sometimes introduces errors in the compact
+                    # representation, which we have to round off to avoid ugliness. In the vast
+                    # majority of cases this rounding will be inconsequential. I hope.
+                    value = round(value.to_compact(), 6)
+                    display_value = jinja2.Markup('{:~P}'.format(value).replace(' ', '&nbsp;'))
                 else:
-                    if column in table_metadata.get('units', {}) and value != '':
-                        # Interpret units using pint
-                        value = value * ureg(table_metadata['units'][column])
-                        # Pint uses floating point which sometimes introduces errors in the compact
-                        # representation, which we have to round off to avoid ugliness. In the vast
-                        # majority of cases this rounding will be inconsequential. I hope.
-                        value = round(value.to_compact(), 6)
-                        display_value = jinja2.Markup('{:~P}'.format(value).replace(' ', '&nbsp;'))
-                    else:
-                        display_value = str(value)
+                    display_value = str(value)
+
                 cells.append({
                     'column': column,
                     'value': display_value,
@@ -577,9 +583,15 @@ class RowTableShared(BaseView):
             cell_rows.append(cells)
 
         if link_column:
+            # Add the link column header.
+            # If it's a simple primary key, we have to remove and re-add that column name at
+            # the beginning of the header row.
+            if len(pks) == 1:
+                columns = [col for col in columns if col['name'] != pks[0]]
+
             columns = [{
-                'name': 'Link',
-                'sortable': False,
+                'name': pks[0] if len(pks) == 1 else 'Link',
+                'sortable': len(pks) == 1,
             }] + columns
         return columns, cell_rows
 

datasette/static/app.css

@@ -17,6 +17,9 @@ td {
     padding: 4px;
     vertical-align: top;
 }
+td.col-link {
+    font-weight: bold;
+}
 td em {
     font-style: normal;
     font-size: 0.8em;

datasette/templates/_rows_and_columns.html

@@ -20,7 +20,7 @@
     {% for row in display_rows %}
         <tr>
             {% for cell in row %}
-                <td>{{ cell.value }}</td>
+                <td class="col-{{ cell.column|lower }}">{{ cell.value }}</td>
             {% endfor %}
         </tr>
     {% endfor %}

datasette/utils.py

@@ -38,14 +38,19 @@ def urlsafe_components(token):
     ]
 
 
-def path_from_row_pks(row, pks, use_rowid):
+def path_from_row_pks(row, pks, use_rowid, quote=True):
+    """ Generate an optionally URL-quoted unique identifier
+        for a row from its primary keys."""
     if use_rowid:
-        return urllib.parse.quote_plus(str(row['rowid']))
-    bits = []
-    for pk in pks:
-        bits.append(
-            urllib.parse.quote_plus(str(row[pk]))
-        )
+        bits = [row['rowid']]
+    else:
+        bits = [row[pk] for pk in pks]
+
+    if quote:
+        bits = [urllib.parse.quote_plus(str(bit)) for bit in bits]
+    else:
+        bits = [str(bit) for bit in bits]
+
     return ','.join(bits)
 
 

tests/fixtures.py

@@ -9,7 +9,7 @@ import tempfile
 import time
 
 
-def app_client():
+def app_client(sql_time_limit_ms=None):
     with tempfile.TemporaryDirectory() as tmpdir:
         filepath = os.path.join(tmpdir, 'test_tables.db')
         conn = sqlite3.connect(filepath)
@@ -22,7 +22,7 @@ def app_client():
             [filepath],
             page_size=50,
             max_returned_rows=100,
-            sql_time_limit_ms=20,
+            sql_time_limit_ms=sql_time_limit_ms or 20,
             metadata=METADATA,
             plugins_dir=plugins_dir,
         )
@@ -32,6 +32,10 @@ def app_client():
         yield ds.app().test_client
 
 
+def app_client_longer_time_limit():
+    yield from app_client(200)
+
+
 def generate_compound_rows(num):
     for a, b, c in itertools.islice(
         itertools.product(string.ascii_lowercase, repeat=3), num

tests/test_api.py

@@ -1,11 +1,13 @@
 from .fixtures import (
     app_client,
+    app_client_longer_time_limit,
     generate_compound_rows,
     generate_sortable_rows,
 )
 import pytest
 
 pytest.fixture(scope='module')(app_client)
+pytest.fixture(scope='module')(app_client_longer_time_limit)
 
 
 def test_homepage(app_client):
@@ -387,15 +389,17 @@ def test_paginate_tables_and_views(app_client, path, expected_rows, expected_pag
     assert expected_pages == count
 
 
-def test_paginate_compound_keys(app_client):
+def test_paginate_compound_keys(app_client_longer_time_limit):
     fetched = []
     path = '/test_tables/compound_three_primary_keys.json?_shape=objects'
     page = 0
     while path:
         page += 1
-        response = app_client.get(path, gather_request=False)
-        fetched.extend(response.json['rows'])
-        path = response.json['next_url']
+        response = app_client_longer_time_limit.get(path, gather_request=False)
+        data = response.json
+        assert 'rows' in data
+        fetched.extend(data['rows'])
+        path = data['next_url']
         assert page < 100
     assert 1001 == len(fetched)
     assert 21 == page

tests/test_html.py

@@ -163,16 +163,18 @@ def test_sort_by_desc_redirects(app_client):
 ])
 def test_css_classes_on_body(app_client, path, expected_classes):
     response = app_client.get(path, gather_request=False)
+    assert response.status == 200
     classes = re.search(r'<body class="(.*)">', response.text).group(1).split()
     assert classes == expected_classes
 
 
 def test_table_html_simple_primary_key(app_client):
     response = app_client.get('/test_tables/simple_primary_key', gather_request=False)
+    assert response.status == 200
     table = Soup(response.body, 'html.parser').find('table')
     ths = table.findAll('th')
-    assert 'Link' == ths[0].string.strip()
-    for expected_col, th in zip(('id', 'content'), ths[1:]):
+    assert 'id' == ths[0].find('a').string.strip()
+    for expected_col, th in zip(('content',), ths[1:]):
         a = th.find('a')
         assert expected_col == a.string
         assert a['href'].endswith('/simple_primary_key?_sort={}'.format(
@@ -181,31 +183,29 @@ def test_table_html_simple_primary_key(app_client):
         assert ['nofollow'] == a['rel']
     assert [
         [
-            '<td><a href="/test_tables/simple_primary_key/1">1</a></td>',
-            '<td>1</td>',
-            '<td>hello</td>'
+            '<td class="col-link"><a href="/test_tables/simple_primary_key/1">1</a></td>',
+            '<td class="col-content">hello</td>'
         ], [
-            '<td><a href="/test_tables/simple_primary_key/2">2</a></td>',
-            '<td>2</td>',
-            '<td>world</td>'
+            '<td class="col-link"><a href="/test_tables/simple_primary_key/2">2</a></td>',
+            '<td class="col-content">world</td>'
         ], [
-            '<td><a href="/test_tables/simple_primary_key/3">3</a></td>',
-            '<td>3</td>',
-            '<td></td>'
+            '<td class="col-link"><a href="/test_tables/simple_primary_key/3">3</a></td>',
+            '<td class="col-content"></td>'
         ]
     ] == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
 
 
 def test_row_html_simple_primary_key(app_client):
     response = app_client.get('/test_tables/simple_primary_key/1', gather_request=False)
+    assert response.status == 200
     table = Soup(response.body, 'html.parser').find('table')
     assert [
         'id', 'content'
     ] == [th.string.strip() for th in table.select('thead th')]
     assert [
         [
-            '<td>1</td>',
-            '<td>hello</td>'
+            '<td class="col-id">1</td>',
+            '<td class="col-content">hello</td>'
         ]
     ] == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
 
@@ -218,6 +218,7 @@ def test_table_not_exists(app_client):
 
 def test_table_html_no_primary_key(app_client):
     response = app_client.get('/test_tables/no_primary_key', gather_request=False)
+    assert response.status == 200
     table = Soup(response.body, 'html.parser').find('table')
     # We have disabled sorting for this table using metadata.json
     assert [
@@ -225,12 +226,12 @@ def test_table_html_no_primary_key(app_client):
     ] == [th.string.strip() for th in table.select('thead th')[2:]]
     expected = [
         [
-            '<td><a href="/test_tables/no_primary_key/{}">{}</a></td>'.format(i, i),
-            '<td>{}</td>'.format(i),
-            '<td>{}</td>'.format(i),
-            '<td>a{}</td>'.format(i),
-            '<td>b{}</td>'.format(i),
-            '<td>c{}</td>'.format(i),
+            '<td class="col-link"><a href="/test_tables/no_primary_key/{}">{}</a></td>'.format(i, i),
+            '<td class="col-rowid">{}</td>'.format(i),
+            '<td class="col-content">{}</td>'.format(i),
+            '<td class="col-a">a{}</td>'.format(i),
+            '<td class="col-b">b{}</td>'.format(i),
+            '<td class="col-c">c{}</td>'.format(i),
         ] for i in range(1, 51)
     ]
     assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
@@ -238,17 +239,18 @@ def test_table_html_no_primary_key(app_client):
 
 def test_row_html_no_primary_key(app_client):
     response = app_client.get('/test_tables/no_primary_key/1', gather_request=False)
+    assert response.status == 200
     table = Soup(response.body, 'html.parser').find('table')
     assert [
         'rowid', 'content', 'a', 'b', 'c'
     ] == [th.string.strip() for th in table.select('thead th')]
     expected = [
         [
-            '<td>1</td>',
-            '<td>1</td>',
-            '<td>a1</td>',
-            '<td>b1</td>',
-            '<td>c1</td>',
+            '<td class="col-rowid">1</td>',
+            '<td class="col-content">1</td>',
+            '<td class="col-a">a1</td>',
+            '<td class="col-b">b1</td>',
+            '<td class="col-c">c1</td>',
         ]
     ]
     assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
@@ -256,6 +258,7 @@ def test_row_html_no_primary_key(app_client):
 
 def test_table_html_compound_primary_key(app_client):
     response = app_client.get('/test_tables/compound_primary_key', gather_request=False)
+    assert response.status == 200
     table = Soup(response.body, 'html.parser').find('table')
     ths = table.findAll('th')
     assert 'Link' == ths[0].string.strip()
@@ -267,10 +270,10 @@ def test_table_html_compound_primary_key(app_client):
         ))
     expected = [
         [
-            '<td><a href="/test_tables/compound_primary_key/a,b">a,b</a></td>',
-            '<td>a</td>',
-            '<td>b</td>',
-            '<td>c</td>',
+            '<td class="col-link"><a href="/test_tables/compound_primary_key/a,b">a,b</a></td>',
+            '<td class="col-pk1">a</td>',
+            '<td class="col-pk2">b</td>',
+            '<td class="col-content">c</td>',
         ]
     ]
     assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
@@ -278,13 +281,13 @@ def test_table_html_compound_primary_key(app_client):
 
 def test_table_html_foreign_key_links(app_client):
     response = app_client.get('/test_tables/foreign_key_references', gather_request=False)
+    assert response.status == 200
     table = Soup(response.body, 'html.parser').find('table')
     expected = [
         [
-            '<td><a href="/test_tables/foreign_key_references/1">1</a></td>',
-            '<td>1</td>',
-            '<td><a href="/test_tables/simple_primary_key/1">hello</a>\xa0<em>1</em></td>',
-            '<td><a href="/test_tables/primary_key_multiple_columns/1">1</a></td>'
+            '<td class="col-link"><a href="/test_tables/foreign_key_references/1">1</a></td>',
+            '<td class="col-foreign_key_with_label"><a href="/test_tables/simple_primary_key/1">hello</a>\xa0<em>1</em></td>',
+            '<td class="col-foreign_key_with_no_label"><a href="/test_tables/primary_key_multiple_columns/1">1</a></td>'
         ]
     ]
     assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
@@ -292,15 +295,16 @@ def test_table_html_foreign_key_links(app_client):
 
 def test_row_html_compound_primary_key(app_client):
     response = app_client.get('/test_tables/compound_primary_key/a,b', gather_request=False)
+    assert response.status == 200
     table = Soup(response.body, 'html.parser').find('table')
     assert [
         'pk1', 'pk2', 'content'
     ] == [th.string.strip() for th in table.select('thead th')]
     expected = [
         [
-            '<td>a</td>',
-            '<td>b</td>',
-            '<td>c</td>',
+            '<td class="col-pk1">a</td>',
+            '<td class="col-pk2">b</td>',
+            '<td class="col-content">c</td>',
         ]
     ]
     assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
@@ -308,20 +312,21 @@ def test_row_html_compound_primary_key(app_client):
 
 def test_view_html(app_client):
     response = app_client.get('/test_tables/simple_view', gather_request=False)
+    assert response.status == 200
     table = Soup(response.body, 'html.parser').find('table')
     assert [
         'content', 'upper_content'
     ] == [th.string.strip() for th in table.select('thead th')]
     expected = [
         [
-            '<td>hello</td>',
-            '<td>HELLO</td>'
+            '<td class="col-content">hello</td>',
+            '<td class="col-upper_content">HELLO</td>'
         ], [
-            '<td>world</td>',
-            '<td>WORLD</td>'
+            '<td class="col-content">world</td>',
+            '<td class="col-upper_content">WORLD</td>'
         ], [
-            '<td></td>',
-            '<td></td>'
+            '<td class="col-content"></td>',
+            '<td class="col-upper_content"></td>'
         ]
     ]
     assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
@@ -329,6 +334,7 @@ def test_view_html(app_client):
 
 def test_index_metadata(app_client):
     response = app_client.get('/', gather_request=False)
+    assert response.status == 200
     soup = Soup(response.body, 'html.parser')
     assert 'Datasette Title' == soup.find('h1').text
     assert 'Datasette Description' == inner_html(
@@ -339,6 +345,7 @@ def test_index_metadata(app_client):
 
 def test_database_metadata(app_client):
     response = app_client.get('/test_tables', gather_request=False)
+    assert response.status == 200
     soup = Soup(response.body, 'html.parser')
     # Page title should be the default
     assert 'test_tables' == soup.find('h1').text
@@ -352,6 +359,7 @@ def test_database_metadata(app_client):
 
 def test_table_metadata(app_client):
     response = app_client.get('/test_tables/simple_primary_key', gather_request=False)
+    assert response.status == 200
     soup = Soup(response.body, 'html.parser')
     # Page title should be custom and should be HTML escaped
     assert 'This &lt;em&gt;HTML&lt;/em&gt; is escaped' == inner_html(soup.find('h1'))