Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade for compatibility with new Datasette 1.0 JSON format #19

Closed
brandonrobertz opened this issue Feb 15, 2023 · 10 comments
Closed

Upgrade for compatibility with new Datasette 1.0 JSON format #19

brandonrobertz opened this issue Feb 15, 2023 · 10 comments
Labels
bug Something isn't working

Comments

@brandonrobertz
Copy link

brandonrobertz commented Feb 15, 2023

Running datasette, version 1.0a2 this plugin seems broken.

When the search runs, it always gets undefined for the row count from data.filtered_table_rows_count.

var count = data.filtered_table_rows_count;

I changed this to data.count and the plugin works as it used to.

@simonw simonw added the bug Something isn't working label Jan 8, 2024
@simonw simonw changed the title Search all always displaying no results Upgrade for compatibility with new Datasette 1.0 JSON format Jan 8, 2024
@simonw
Copy link
Owner

simonw commented Jan 8, 2024

I also got this error:

TypeError: columns is undefined

@simonw
Copy link
Owner

simonw commented Jan 8, 2024

I think I need a Playwright test for this, since it's a bug in the JavaScript.

I'll borrow from https://github.com/datasette/datasette-comments/blob/a0f637f6a3d6927a00c678166d7e66363df1a7b5/tests/test_ui.py#L2

@simonw
Copy link
Owner

simonw commented Jan 8, 2024

Here's the code that's failing:

function displayResults(data, base_url) {
var rows = data.rows;
var database = data.database;
var table = data.table;
var columns = data.columns;
var count = data.filtered_table_rows_count;

I don't think I've exposed all of that stuff as ?_extra= yet.

@simonw
Copy link
Owner

simonw commented Jan 8, 2024

Actually I have: https://latest.datasette.io/fixtures/searchable.json?__search=dog&_extra=count&_extra=table&_extra=database&_extra=columns

{
  "ok": true,
  "next": null,
  "columns": [
    "pk",
    "text1",
    "text2",
    "name with . and spaces"
  ],
  "database": "fixtures",
  "count": 2,
  "table": "searchable",
  "rows": [
    {
      "pk": "pk",
      "text1": "text1",
      "text2": "text2",
      "name with . and spaces": "name with . and spaces"
    },
    {
      "pk": "pk",
      "text1": "text1",
      "text2": "text2",
      "name with . and spaces": "name with . and spaces"
    }
  ],
  "truncated": false
}

@simonw
Copy link
Owner

simonw commented Jan 8, 2024

Found a Datasette 1.0 bug: the ?_extra=columns parameter causes all rows to return `"column-name": "column-name": https://latest.datasette.io/fixtures/attraction_characteristic.json?_extra=columns&_size=1

{
  "ok": true,
  "next": "2,2",
  "columns": [
    "pk",
    "name"
  ],
  "rows": [
    {
      "pk": "pk",
      "name": "name"
    }
  ],
  "truncated": false
}

Should return this:

{
  "ok": true,
  "next": "2,2",
  "columns": [
    "pk",
    "name"
  ],
  "rows": [
    {
      "pk": 2,
      "name": "Paranormal"
    }
  ],
  "truncated": false
}

@simonw
Copy link
Owner

simonw commented Jan 8, 2024

That bug is a bit of a blocker on this task. For the moment I'll use the keys from the first returned row.

@simonw
Copy link
Owner

simonw commented Jan 8, 2024

Got that working - the tests are a bit ugly in terms of Playwright boilerplate:

pytestmark = pytest.mark.skipif(sync_api is None, reason="playwright not installed")
def test_ds_server(ds_server):
with sync_api.sync_playwright() as playwright:
browser = playwright.chromium.launch()
page = browser.new_page()
page.goto(ds_server + "/")
assert page.title() == "Datasette: data"
# It should have a search form
assert page.query_selector('form[action="/-/search"]')
def test_search(ds_server):
with sync_api.sync_playwright() as playwright:
browser = playwright.chromium.launch()
page = browser.new_page()
page.goto(ds_server + "/-/search?q=cleo")

Playwright includes pytest fixtures, you can use page to get a page to interact with.

I was getting "This event loop is already running" errors when I tried to use that, but it looks like the solution is to add nest_asyncio.apply() which seems to have fixed it.

@simonw
Copy link
Owner

simonw commented Jan 8, 2024

Much neater:

def test_ds_server(ds_server, page):
page.goto(ds_server + "/")
assert page.title() == "Datasette: data"
# It should have a search form
assert page.query_selector('form[action="/-/search"]')
def test_search(ds_server, page):
page.goto(ds_server + "/-/search?q=cleo")
# Should show search results, after fetching them
assert page.locator("table tr th:nth-child(1)").inner_text() == "rowid"

simonw added a commit that referenced this issue Jan 8, 2024
@simonw
Copy link
Owner

simonw commented Jan 8, 2024

Job matrix looks pretty good!

CleanShot 2024-01-08 at 12 01 50@2x

@simonw simonw closed this as completed Jan 8, 2024
simonw added a commit that referenced this issue Jan 8, 2024
@simonw
Copy link
Owner

simonw commented Jan 8, 2024

Tested this on Datasette Cloud as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants