Repository: rclement/datasette-dashboards (language: python)

Issue:
Row limit warning for charts
## Context

As pointed out by @havef and discussed in #142, Datasette sets limits on the number of rows returned by SQL queries (setting `max_returned_rows` defaulting to 1000). This can have an impact on rendered charts when more than `max_returned_rows` are expected to be displayed.

## Technical details

To be able to detect when a data truncation occurs, all charts should be using the JSON API with the `_shape=objects` query argument, allowing to get the `truncated` boolean value.

For instance, this query:

```
https://datasette-dashboards-demo.vercel.app/jobs.json?sql=SELECT date(date) as Date, title as Title, company as Company, source as Source, city as City, longitude, latitude FROM offers_view WHERE longitude IS NOT NULL AND latitude IS NOT NULL AND date >= date(:date_start) &date_start=2021-01-01&_shape=objects
```

yields the following JSON output:

```json
{
    "ok": true,
    "database": "jobs",
    "query_name": null,
    "rows": [
        {
            "Date": "2021-04-21",
            "Title": "Devops Python H/F",
            "Company": "Sully Group",
            "Source": "RegionsJob",
            "City": "Aix-en-Provence",
            "longitude": 5.4451153,
            "latitude": 43.5227288
        },
        ...
    ],
    "truncated": true,
    "columns": [
        "Date",
        "Title",
        "Company",
        "Source",
        "City",
        "longitude",
        "latitude"
    ],
    "query": {
        "sql": "SELECT\n  date(date) as Date,\n  title as Title,\n  company as Company,\n  source as Source,\n  city as City,\n  longitude,\n  latitude\nFROM offers_view WHERE longitude IS NOT NULL AND latitude IS NOT NULL\n   AND date >= date(:date_start) \n  \n  \n  ",
        "params": {
            "date_start": "2021-01-01"
        }
    },
    "error": null,
    "private": false,
    "allow_execute_sql": true,
    "query_ms": 6.713455999943108
}
```

## Design

As a reference, Here is how Metabase informs the user with a warning tooltip when the row limit is exceeded:

<img width="550" alt="Screenshot 2023-08-08 at 11 02 32" src="https://github.com/rclement/datasette-dashboards/assets/1238873/266b2c12-6f60-43aa-932e-4d6e693b3fde">

<img width="550" alt="Screenshot 2023-08-08 at 11 03 42" src="https://github.com/rclement/datasette-dashboards/assets/1238873/0b10534f-7e37-47ed-9e33-2ad287eef7aa">


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 37d38ef..0468c26 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 - Enable extra extensions for Markdown rendering
+- Chart row limit warning tooltip
 
 ### Changed
 - Update README with dashboard layout examples
diff --git a/datasette_dashboards/__init__.py b/datasette_dashboards/__init__.py
index 81772f4..4d8f241 100644
--- a/datasette_dashboards/__init__.py
+++ b/datasette_dashboards/__init__.py
@@ -158,6 +158,7 @@ async def _dashboard_view(
                 "query_string": query_string,
                 "dashboard": dashboard,
                 "embed": embed,
+                "row_limit": datasette.settings_dict().get("max_returned_rows"),
             },
             request=request,
         )
@@ -216,6 +217,7 @@ async def _dashboard_chart(
                 "dashboard": dashboard,
                 "chart": chart,
                 "embed": embed,
+                "row_limit": datasette.settings_dict().get("max_returned_rows"),
             },
             request=request,
         )
diff --git a/datasette_dashboards/static/dashboards.css b/datasette_dashboards/static/dashboards.css
index c96a6c5..08d5830 100644
--- a/datasette_dashboards/static/dashboards.css
+++ b/datasette_dashboards/static/dashboards.css
@@ -98,3 +98,9 @@
 #vg-tooltip-element * {
   all: revert;
 }
+
+.chart-tooltip {
+  visibility: hidden;
+  position: relative;
+  display: inline-block;
+}
diff --git a/datasette_dashboards/static/dashboards.js b/datasette_dashboards/static/dashboards.js
index 55c5f72..5f991f7 100644
--- a/datasette_dashboards/static/dashboards.js
+++ b/datasette_dashboards/static/dashboards.js
@@ -1,5 +1,18 @@
-function renderVegaChart(el, chart, query_string, absolute_url, full_height) {
+function getDataUrl(chart, query_string, absolute_url) {
   const query = encodeURIComponent(chart.query)
+  return `${absolute_url}${chart.db}.json?sql=${query}&${query_string}&_shape=objects`
+}
+
+function enableChartTooltip(chart_slug) {
+  let tooltip = document.querySelector(`#chart-tooltip-${chart_slug}`)
+  tooltip.style.visibility = 'visible'
+}
+
+async function renderVegaChart(chart_slug, chart, query_string, absolute_url, full_height) {
+  const dataUrl = getDataUrl(chart, query_string, absolute_url)
+  const results = await fetch(dataUrl)
+  const data = await results.json()
+
   let defaultSignals = [
     {
       'name': 'width',
@@ -32,19 +45,27 @@ function renderVegaChart(el, chart, query_string, absolute_url, full_height) {
     data: [
       {
         name: 'table',
-        url: `${absolute_url}${chart.db}.csv?sql=${query}&${query_string}`,
-        format: { 'type': 'csv', 'parse': 'auto' }
+        values: data.rows,
+        format: { 'type': 'json' }
       }
     ],
     signals: defaultSignals,
     ...chart.display
   };
 
-  vegaEmbed(el, spec);
+  const el = document.querySelector(`#chart-${chart_slug}`)
+  vegaEmbed(el, spec)
+
+  if (data.truncated) {
+    enableChartTooltip(chart_slug)
+  }
 }
 
-function renderVegaLiteChart(el, chart, query_string, absolute_url, full_height) {
-  const query = encodeURIComponent(chart.query)
+async function renderVegaLiteChart(chart_slug, chart, query_string, absolute_url, full_height) {
+  const dataUrl = getDataUrl(chart, query_string, absolute_url)
+  const results = await fetch(dataUrl)
+  const data = await results.json()
+
   const spec = {
     $schema: 'https://vega.github.io/schema/vega-lite/v5.json',
     description: chart.title,
@@ -61,20 +82,25 @@ function renderVegaLiteChart(el, chart, query_string, absolute_url, full_height)
       }
     },
     data: {
-      url: `${absolute_url}${chart.db}.csv?sql=${query}&${query_string}`,
-      format: { 'type': 'csv' }
+      values: data.rows,
+      format: { 'type': 'json' }
     },
     ...chart.display
   };
 
-  vegaEmbed(el, spec);
+  const el = document.querySelector(`#chart-${chart_slug}`)
+  vegaEmbed(el, spec)
+
+  if (data.truncated) {
+    enableChartTooltip(chart_slug)
+  }
 }
 
-async function renderMetricChart(el, chart, query_string, absolute_url, full_height) {
-  const query = encodeURIComponent(chart.query)
-  const results = await fetch(`${absolute_url}${chart.db}.json?sql=${query}&${query_string}&_shape=array`)
+async function renderMetricChart(chart_slug, chart, query_string, absolute_url, full_height) {
+  const dataUrl = getDataUrl(chart, query_string, absolute_url)
+  const results = await fetch(dataUrl)
   const data = await results.json()
-  const metric = data[0][chart.display.field]
+  const metric = data.rows[0][chart.display.field]
 
   const prefix = chart.display.prefix || ''
   const suffix = chart.display.suffix || ''
@@ -97,12 +123,17 @@ async function renderMetricChart(el, chart, query_string, absolute_url, full_hei
   wrapper.style.whiteSpace = 'nowrap'
   wrapper.style.textOverflow = 'ellipsis'
 
-  document.querySelector(el).appendChild(wrapper)
+  const el = document.querySelector(`#chart-${chart_slug}`)
+  el.appendChild(wrapper)
+
+  if (data.truncated) {
+    enableChartTooltip(chart_slug)
+  }
 }
 
-async function renderTableChart(el, chart, query_string, absolute_url, full_height) {
-  const query = encodeURIComponent(chart.query)
-  const results = await fetch(`${absolute_url}${chart.db}.json?sql=${query}&${query_string}`)
+async function renderTableChart(chart_slug, chart, query_string, absolute_url, full_height) {
+  const dataUrl = getDataUrl(chart, query_string, absolute_url)
+  const results = await fetch(dataUrl)
   const data = await results.json()
 
   const thead = document.createElement('thead')
@@ -117,7 +148,7 @@ async function renderTableChart(el, chart, query_string, absolute_url, full_heig
   const tbody = document.createElement('tbody')
   data.rows.forEach(row => {
     const tbody_tr = document.createElement('tr')
-    row.forEach(col => {
+    Object.values(row).forEach(col => {
       const tbody_td = document.createElement('td')
       tbody_td.innerHTML = col
       tbody_tr.appendChild(tbody_td)
@@ -135,20 +166,27 @@ async function renderTableChart(el, chart, query_string, absolute_url, full_heig
   wrapper.style.overflow = 'auto'
   wrapper.appendChild(table)
 
-  document.querySelector(el).appendChild(wrapper)
+  const el = document.querySelector(`#chart-${chart_slug}`)
+  el.appendChild(wrapper)
+
+  if (data.truncated) {
+    enableChartTooltip(chart_slug)
+  }
 }
 
-async function renderMapChart(el, chart, query_string, absolute_url, full_height) {
+async function renderMapChart(chart_slug, chart, query_string, absolute_url, full_height) {
   document.addEventListener("DOMContentLoaded", async () => {
-    const query = encodeURIComponent(chart.query)
-    const results = await fetch(`${absolute_url}${chart.db}.json?sql=${query}&${query_string}&_shape=array`)
+    const dataUrl = getDataUrl(chart, query_string, absolute_url)
+    const results = await fetch(dataUrl)
     const data = await results.json()
 
     const wrapper = document.createElement('div')
     wrapper.style.width = '100%'
     wrapper.style.height = '100%'
     wrapper.style.minHeight = '200px'
-    document.querySelector(el).appendChild(wrapper)
+
+    const el = document.querySelector(`#chart-${chart_slug}`)
+    el.appendChild(wrapper)
 
     const map = L.map(wrapper, { zoom: 12 })
 
@@ -164,7 +202,7 @@ async function renderMapChart(el, chart, query_string, absolute_url, full_height
     const longitude_column = options.longitude_column || 'longitude'
     const show_latlng_popup = options.show_latlng_popup || false
 
-    data.forEach(row => {
+    data.rows.forEach(row => {
       const marker = L.marker([row[latitude_column], row[longitude_column]])
       const popup = Object.entries(row)
         .filter(e => (e[0] === latitude_column || e[0] === longitude_column) ? show_latlng_popup : true)
@@ -173,13 +211,17 @@ async function renderMapChart(el, chart, query_string, absolute_url, full_height
       map.addLayer(marker)
     })
 
-    const coords = data.map(row => [row[latitude_column], row[longitude_column]])
+    const coords = data.rows.map(row => [row[latitude_column], row[longitude_column]])
     const bounds = new L.LatLngBounds(coords)
     map.fitBounds(bounds)
+
+    if (data.truncated) {
+      enableChartTooltip(chart_slug)
+    }
   })
 }
 
-async function renderChart(el, chart, query_string, absolute_url, full_height = false) {
+async function renderChart(chart_slug, chart, query_string, absolute_url, full_height = false) {
   renderers = new Map()
   renderers.set('vega', renderVegaChart)
   renderers.set('vega-lite', renderVegaLiteChart)
@@ -189,7 +231,7 @@ async function renderChart(el, chart, query_string, absolute_url, full_height =
 
   render = renderers.get(chart.library)
   if (render) {
-    await render(el, chart, query_string, absolute_url, full_height)
+    await render(chart_slug, chart, query_string, absolute_url, full_height)
   }
 }
 
diff --git a/datasette_dashboards/templates/dashboard_chart.html b/datasette_dashboards/templates/dashboard_chart.html
index da41a92..c693993 100644
--- a/datasette_dashboards/templates/dashboard_chart.html
+++ b/datasette_dashboards/templates/dashboard_chart.html
@@ -38,9 +38,14 @@
         <div class="page-header" style="border-color: black">
           <h1>{{ dashboard.title }}</h1>
         </div>
-        {% if chart.title %}
-        <p>{{ chart.title }}</p>
-        {% endif %}
+        <p>
+          {% if chart.title %}
+          {{ chart.title }}
+          {% endif %}
+          <span id="chart-tooltip-{{ slug }}" class="chart-tooltip" title="SQL query returning more than {{ row_limit }} rows">
+            ⚠️
+          </span>
+        </p>
       </div>
     </div>
 
@@ -53,7 +58,7 @@ <h1>{{ dashboard.title }}</h1>
     {% endif %}
 
     <div class="dashboard-card">
-      <div id="chart" class="chart-container">
+      <div id="chart-{{ slug }}" class="chart-container">
         {% if chart.library == 'markdown' %}
         {{ render_markdown(
               chart.display,
@@ -72,6 +77,6 @@ <h1>{{ dashboard.title }}</h1>
   <script src="{{ urls.static_plugins('datasette_dashboards', 'vega-embed.min.js') }}"></script>
   <script src="{{ urls.static_plugins('datasette_dashboards', 'dashboards.js') }}"></script>
   <script type="text/javascript">
-    renderChart('#chart', {{ chart|tojson }}, '{{ query_string|safe }}', '{{ absolute_url }}', true)
+    renderChart('{{ slug }}', {{ chart|tojson }}, '{{ query_string|safe }}', '{{ absolute_url }}', true)
   </script>
 {% endblock %}
diff --git a/datasette_dashboards/templates/dashboard_view.html b/datasette_dashboards/templates/dashboard_view.html
index 8f73d3e..2c22742 100644
--- a/datasette_dashboards/templates/dashboard_view.html
+++ b/datasette_dashboards/templates/dashboard_view.html
@@ -106,6 +106,9 @@ <h1>{{ dashboard.title }}</h1>
             {% if not embed %}
             </a>
             {% endif%}
+            <span id="chart-tooltip-{{ chart_slug}}" class="chart-tooltip" title="SQL query returning more than {{ row_limit }} rows">
+              ⚠️
+            </span>
           </p>
         </div>
         {% endif %}
@@ -132,7 +135,7 @@ <h1>{{ dashboard.title }}</h1>
   <script src="{{ urls.static_plugins('datasette_dashboards', 'dashboards.js') }}"></script>
   <script type="text/javascript">
     {% for chart_slug, chart in dashboard.charts.items() %}
-    renderChart('#chart-{{ chart_slug }}', {{ chart|tojson }}, '{{ query_string|safe }}', '{{ absolute_url }}')
+    renderChart('{{ chart_slug }}', {{ chart|tojson }}, '{{ query_string|safe }}', '{{ absolute_url }}')
     {% endfor %}
 
     {% if settings.autorefresh %}


Test patch:
diff --git a/tests/test_dashboard_chart.py b/tests/test_dashboard_chart.py
index 3e4c713..8d45759 100644
--- a/tests/test_dashboard_chart.py
+++ b/tests/test_dashboard_chart.py
@@ -17,10 +17,9 @@ async def test_dashboard_chart(datasette: Datasette) -> None:
             assert '<li><a href="/-/dashboards">Dashboards</a></li>' in response.text
             assert f'<h1>{dashboard["title"]}</h1>' in response.text
             if "title" in props:
-                assert f'<p>{chart["title"]}</p>' in response.text
+                assert chart["title"] in response.text
             if "db" in props and "query" in props:
-                assert f'<a href="/{chart["db"]}?sql={chart["query"]}">'
-                assert "View and edit SQL"
+                assert "View and edit SQL" in response.text
 
 
 @pytest.mark.asyncio
diff --git a/tests/test_dashboard_chart_embed.py b/tests/test_dashboard_chart_embed.py
index 0f83cc3..e674f5a 100644
--- a/tests/test_dashboard_chart_embed.py
+++ b/tests/test_dashboard_chart_embed.py
@@ -15,4 +15,4 @@ async def test_dashboard_chart_embed(datasette: Datasette) -> None:
             assert response.status_code == 200
             assert f'<h1>{dashboard["title"]}</h1>' in response.text
             if "title" in props:
-                assert f'<p>{chart["title"]}</p>' in response.text
+                assert f'{chart["title"]}' in response.text
diff --git a/tests/test_dashboard_view.py b/tests/test_dashboard_view.py
index 6f0eda0..d8df850 100644
--- a/tests/test_dashboard_view.py
+++ b/tests/test_dashboard_view.py
@@ -63,7 +63,7 @@ async def test_dashboard_views(datasette: Datasette) -> None:
             if chart["library"] != "markdown":
                 assert f'<a href="/-/dashboards/{slug}/{chart_slug}?' in response.text
                 assert f'{chart["title"]}' in response.text
-            assert f"renderChart('#chart-{chart_slug}', " in response.text
+            assert f"renderChart('{chart_slug}', " in response.text
 
 
 @pytest.mark.asyncio