Cleaned up row/column display logic, fixed bug

Closes #167
This commit is contained in:
Simon Willison 2017-12-09 16:59:25 -08:00
commit 794c3bfcfc
No known key found for this signature in database
GPG key ID: 17E2DEA2588B7F52
2 changed files with 120 additions and 58 deletions

View file

@ -39,6 +39,7 @@ from .version import __version__
app_root = Path(__file__).parent.parent app_root = Path(__file__).parent.parent
HASH_BLOCK_SIZE = 1024 * 1024 HASH_BLOCK_SIZE = 1024 * 1024
HASH_LENGTH = 7
connections = threading.local() connections = threading.local()
@ -118,7 +119,7 @@ class BaseView(RenderMixin):
info = databases[name] info = databases[name]
except KeyError: except KeyError:
raise NotFound('Database not found: {}'.format(name)) raise NotFound('Database not found: {}'.format(name))
expected = info['hash'][:7] expected = info['hash'][:HASH_LENGTH]
if expected != hash: if expected != hash:
should_redirect = '/{}-{}'.format( should_redirect = '/{}-{}'.format(
name, expected, name, expected,
@ -335,7 +336,7 @@ class IndexView(RenderMixin):
database = { database = {
'name': key, 'name': key,
'hash': info['hash'], 'hash': info['hash'],
'path': '{}-{}'.format(key, info['hash'][:7]), 'path': '{}-{}'.format(key, info['hash'][:HASH_LENGTH]),
'tables_truncated': sorted( 'tables_truncated': sorted(
tables, tables,
key=lambda t: t['count'], key=lambda t: t['count'],
@ -415,12 +416,18 @@ class DatabaseDownload(BaseView):
class RowTableShared(BaseView): class RowTableShared(BaseView):
async def make_display_rows(self, database, database_hash, table, rows, display_columns, pks, is_view, use_rowid, is_row_display): async def display_columns_and_rows(self, database, table, description, rows, link_column=False, expand_foreign_keys=True):
# Get fancy with foreign keys "Returns columns, rows for specified table - including fancy foreign key treatment"
expanded = {} info = self.ds.inspect()[database]
tables = self.ds.inspect()[database]['tables'] database_hash = info['hash'][:HASH_LENGTH]
columns = [r[0] for r in description]
tables = info['tables']
table_info = tables.get(table) or {} table_info = tables.get(table) or {}
if table_info and not is_view: pks = await self.pks_for_table(database, table)
# Prefetch foreign key resolutions for later expansion:
expanded = {}
if table_info and expand_foreign_keys:
foreign_keys = table_info['foreign_keys']['outgoing'] foreign_keys = table_info['foreign_keys']['outgoing']
for fk in foreign_keys: for fk in foreign_keys:
label_column = tables.get(fk['other_table'], {}).get('label_column') label_column = tables.get(fk['other_table'], {}).get('label_column')
@ -443,52 +450,42 @@ class RowTableShared(BaseView):
for id, value in results: for id, value in results:
expanded[(fk['column'], id)] = (fk['other_table'], value) expanded[(fk['column'], id)] = (fk['other_table'], value)
to_return = [] cell_rows = []
for row in rows: for row in rows:
columns_to_loop = display_columns
cells = [] cells = []
# Unless we are a view, the first column is a link - either to the rowid # Unless we are a view, the first column is a link - either to the rowid
# or to the simple or compound primary key # or to the simple or compound primary key
if not is_view: if link_column:
# On row display, only show the extra first column for use_rowid cells.append({
if not is_row_display or (is_row_display and use_rowid): 'column': 'Link',
display_value = path_from_row_pks(row, pks, use_rowid) 'value': jinja2.Markup(
if not is_row_display: '<a href="/{database}-{database_hash}/{table}/{flat_pks}">{flat_pks}</a>'.format(
display_value = jinja2.Markup( database=database,
'<a href="/{database}-{database_hash}/{table}/{flat_pks}">{flat_pks}</a>'.format( database_hash=database_hash,
database=database, table=urllib.parse.quote_plus(table),
database_hash=database_hash, flat_pks=path_from_row_pks(row, pks, not pks),
table=urllib.parse.quote_plus(table),
flat_pks=path_from_row_pks(row, pks, use_rowid),
)
) )
cells.append({ ),
'column': 'rowid' if use_rowid else 'Link', })
'value': display_value, for value, column in zip(row, columns):
}) if (column, value) in expanded:
columns_to_loop = columns_to_loop[1:]
for value, column in zip(row, columns_to_loop):
if use_rowid and column == 'rowid':
# We already showed this in the linked first column
continue
elif (column, value) in expanded:
other_table, label = expanded[(column, value)] other_table, label = expanded[(column, value)]
display_value = jinja2.Markup( display_value = jinja2.Markup(
# TODO: Escape id/label/etc so no XSS here
'<a href="/{database}-{database_hash}/{table}/{id}">{label}</a>&nbsp;<em>{id}</em>'.format( '<a href="/{database}-{database_hash}/{table}/{id}">{label}</a>&nbsp;<em>{id}</em>'.format(
database=database, database=database,
database_hash=database_hash, database_hash=database_hash,
table=urllib.parse.quote_plus(other_table), table=urllib.parse.quote_plus(other_table),
id=value, id=str(jinja2.escape(value)),
label=label, label=str(jinja2.escape(label)),
) )
) )
elif value is None: elif value is None:
display_value = jinja2.Markup('&nbsp;') display_value = jinja2.Markup('&nbsp;')
elif is_url(str(value).strip()): elif is_url(str(value).strip()):
display_value = jinja2.Markup( display_value = jinja2.Markup(
'<a href="{url}">{url}</a>'.format(url=value.strip()) '<a href="{url}">{url}</a>'.format(
url=jinja2.escape(value.strip())
)
) )
else: else:
display_value = str(value) display_value = str(value)
@ -496,8 +493,11 @@ class RowTableShared(BaseView):
'column': column, 'column': column,
'value': display_value, 'value': display_value,
}) })
to_return.append(cells) cell_rows.append(cells)
return to_return
if link_column:
columns = ['Link'] + columns
return columns, cell_rows
class TableView(RowTableShared): class TableView(RowTableShared):
@ -648,11 +648,7 @@ class TableView(RowTableShared):
columns = [r[0] for r in description] columns = [r[0] for r in description]
rows = list(rows) rows = list(rows)
display_columns = columns filter_columns = columns[:]
filter_columns = columns
if not use_rowid and not is_view:
display_columns = ['Link'] + display_columns
if use_rowid and filter_columns[0] == 'rowid': if use_rowid and filter_columns[0] == 'rowid':
filter_columns = filter_columns[1:] filter_columns = filter_columns[1:]
@ -695,6 +691,9 @@ class TableView(RowTableShared):
human_description = filters.human_description(extra=search_description) human_description = filters.human_description(extra=search_description)
async def extra_template(): async def extra_template():
display_columns, display_rows = await self.display_columns_and_rows(
name, table, description, rows, link_column=not is_view, expand_foreign_keys=True
)
return { return {
'database_hash': hash, 'database_hash': hash,
'human_filter_description': human_description, 'human_filter_description': human_description,
@ -704,7 +703,7 @@ class TableView(RowTableShared):
'filters': filters, 'filters': filters,
'display_columns': display_columns, 'display_columns': display_columns,
'filter_columns': filter_columns, 'filter_columns': filter_columns,
'display_rows': await self.make_display_rows(name, hash, table, rows, display_columns, pks, is_view, use_rowid, is_row_display=False), 'display_rows': display_rows,
'custom_rows_and_columns_templates': [ 'custom_rows_and_columns_templates': [
'_rows_and_columns-{}-{}.html'.format(to_css_class(name), to_css_class(table)), '_rows_and_columns-{}-{}.html'.format(to_css_class(name), to_css_class(table)),
'_rows_and_columns-table-{}-{}.html'.format(to_css_class(name), to_css_class(table)), '_rows_and_columns-table-{}-{}.html'.format(to_css_class(name), to_css_class(table)),
@ -767,11 +766,14 @@ class RowView(RowTableShared):
raise NotFound('Record not found: {}'.format(pk_values)) raise NotFound('Record not found: {}'.format(pk_values))
async def template_data(): async def template_data():
display_columns, display_rows = await self.display_columns_and_rows(
name, table, description, rows, link_column=False, expand_foreign_keys=True
)
return { return {
'database_hash': hash, 'database_hash': hash,
'foreign_key_tables': await self.foreign_key_tables(name, table, pk_values), 'foreign_key_tables': await self.foreign_key_tables(name, table, pk_values),
'display_columns': columns, 'display_columns': display_columns,
'display_rows': await self.make_display_rows(name, hash, table, rows, columns, pks, is_view=False, use_rowid=use_rowid, is_row_display=True), 'display_rows': display_rows,
'custom_rows_and_columns_templates': [ 'custom_rows_and_columns_templates': [
'_rows_and_columns-{}-{}.html'.format(to_css_class(name), to_css_class(table)), '_rows_and_columns-{}-{}.html'.format(to_css_class(name), to_css_class(table)),
'_rows_and_columns-row-{}-{}.html'.format(to_css_class(name), to_css_class(table)), '_rows_and_columns-row-{}-{}.html'.format(to_css_class(name), to_css_class(table)),

View file

@ -95,7 +95,7 @@ def test_database_page(app_client):
'foreign_keys': {'incoming': [], 'outgoing': []}, 'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None, 'label_column': None,
}, { }, {
'columns': ['content'], 'columns': ['content', 'a', 'b', 'c'],
'name': 'no_primary_key', 'name': 'no_primary_key',
'count': 201, 'count': 201,
'hidden': False, 'hidden': False,
@ -192,9 +192,7 @@ def test_invalid_custom_sql(app_client):
assert 'Statement must be a SELECT' == response.json['error'] assert 'Statement must be a SELECT' == response.json['error']
def test_table_page(app_client): def test_table_json(app_client):
response = app_client.get('/test_tables/simple_primary_key', gather_request=False)
assert response.status == 200
response = app_client.get('/test_tables/simple_primary_key.jsono', gather_request=False) response = app_client.get('/test_tables/simple_primary_key.jsono', gather_request=False)
assert response.status == 200 assert response.status == 200
data = response.json data = response.json
@ -450,15 +448,15 @@ def test_table_html_simple_primary_key(app_client):
] == [th.string for th in table.select('thead th')] ] == [th.string for th in table.select('thead th')]
assert [ assert [
[ [
'<td><a href="/test_tables-0f4dfa9/simple_primary_key/1">1</a></td>', '<td><a href="/test_tables-c0e2850/simple_primary_key/1">1</a></td>',
'<td>1</td>', '<td>1</td>',
'<td>hello</td>' '<td>hello</td>'
], [ ], [
'<td><a href="/test_tables-0f4dfa9/simple_primary_key/2">2</a></td>', '<td><a href="/test_tables-c0e2850/simple_primary_key/2">2</a></td>',
'<td>2</td>', '<td>2</td>',
'<td>world</td>' '<td>world</td>'
], [ ], [
'<td><a href="/test_tables-0f4dfa9/simple_primary_key/3">3</a></td>', '<td><a href="/test_tables-c0e2850/simple_primary_key/3">3</a></td>',
'<td>3</td>', '<td>3</td>',
'<td></td>' '<td></td>'
] ]
@ -483,17 +481,39 @@ def test_table_html_no_primary_key(app_client):
response = app_client.get('/test_tables/no_primary_key', gather_request=False) response = app_client.get('/test_tables/no_primary_key', gather_request=False)
table = Soup(response.body, 'html.parser').find('table') table = Soup(response.body, 'html.parser').find('table')
assert [ assert [
'rowid', 'content' 'Link', 'rowid', 'content', 'a', 'b', 'c'
] == [th.string for th in table.select('thead th')] ] == [th.string for th in table.select('thead th')]
expected = [ expected = [
[ [
'<td><a href="/test_tables-0f4dfa9/no_primary_key/{}">{}</a></td>'.format(i, i), '<td><a href="/test_tables-c0e2850/no_primary_key/{}">{}</a></td>'.format(i, i),
'<td>{}</td>'.format(i), '<td>{}</td>'.format(i),
'<td>{}</td>'.format(i),
'<td>a{}</td>'.format(i),
'<td>b{}</td>'.format(i),
'<td>c{}</td>'.format(i),
] for i in range(1, 51) ] for i in range(1, 51)
] ]
assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')] assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
def test_row_html_no_primary_key(app_client):
response = app_client.get('/test_tables/no_primary_key/1', gather_request=False)
table = Soup(response.body, 'html.parser').find('table')
assert [
'rowid', 'content', 'a', 'b', 'c'
] == [th.string for th in table.select('thead th')]
expected = [
[
'<td>1</td>',
'<td>1</td>',
'<td>a1</td>',
'<td>b1</td>',
'<td>c1</td>',
]
]
assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
def test_table_html_compound_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) response = app_client.get('/test_tables/compound_primary_key', gather_request=False)
table = Soup(response.body, 'html.parser').find('table') table = Soup(response.body, 'html.parser').find('table')
@ -502,7 +522,7 @@ def test_table_html_compound_primary_key(app_client):
] == [th.string for th in table.select('thead th')] ] == [th.string for th in table.select('thead th')]
expected = [ expected = [
[ [
'<td><a href="/test_tables-0f4dfa9/compound_primary_key/a,b">a,b</a></td>', '<td><a href="/test_tables-c0e2850/compound_primary_key/a,b">a,b</a></td>',
'<td>a</td>', '<td>a</td>',
'<td>b</td>', '<td>b</td>',
'<td>c</td>', '<td>c</td>',
@ -511,6 +531,43 @@ def test_table_html_compound_primary_key(app_client):
assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')] assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
def test_row_html_compound_primary_key(app_client):
response = app_client.get('/test_tables/compound_primary_key/a,b', gather_request=False)
table = Soup(response.body, 'html.parser').find('table')
assert [
'pk1', 'pk2', 'content'
] == [th.string for th in table.select('thead th')]
expected = [
[
'<td>a</td>',
'<td>b</td>',
'<td>c</td>',
]
]
assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
def test_view_html(app_client):
response = app_client.get('/test_tables/simple_view', gather_request=False)
table = Soup(response.body, 'html.parser').find('table')
assert [
'content', 'upper_content'
] == [th.string for th in table.select('thead th')]
expected = [
[
'<td>hello</td>',
'<td>HELLO</td>'
], [
'<td>world</td>',
'<td>WORLD</td>'
], [
'<td></td>',
'<td></td>'
]
]
assert expected == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
TABLES = ''' TABLES = '''
CREATE TABLE simple_primary_key ( CREATE TABLE simple_primary_key (
pk varchar(30) primary key, pk varchar(30) primary key,
@ -527,7 +584,10 @@ CREATE TABLE compound_primary_key (
INSERT INTO compound_primary_key VALUES ('a', 'b', 'c'); INSERT INTO compound_primary_key VALUES ('a', 'b', 'c');
CREATE TABLE no_primary_key ( CREATE TABLE no_primary_key (
content text content text,
a text,
b text,
c text
); );
CREATE TABLE [123_starts_with_digits] ( CREATE TABLE [123_starts_with_digits] (
@ -572,6 +632,6 @@ CREATE VIEW simple_view AS
SELECT content, upper(content) AS upper_content FROM simple_primary_key; SELECT content, upper(content) AS upper_content FROM simple_primary_key;
''' + '\n'.join([ ''' + '\n'.join([
'INSERT INTO no_primary_key VALUES ({});'.format(i + 1) 'INSERT INTO no_primary_key VALUES ({i}, "a{i}", "b{i}", "c{i}");'.format(i=i + 1)
for i in range(201) for i in range(201)
]) ])