Track alter-event in all the right places (#57)

Refs #53 (comment)

We are not supporting pre-Datasette 1.0a9 any more

Closes #54

.github/workflows/test.yml

@@ -11,7 +11,6 @@ jobs:
     strategy:
       matrix:
         python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
-        datasette-version: ["<1.0", ">=1.0a8"]
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python ${{ matrix.python-version }}
@@ -28,7 +27,6 @@ jobs:
     - name: Install dependencies
       run: |
         pip install -e '.[test]'
-        pip install "datasette${{ matrix.datasette-version }}"
     - name: Run tests
       run: |
         pytest

datasette_edit_schema/__init__.py

@@ -1,4 +1,5 @@
 from datasette import hookimpl
+from datasette.events import CreateTableEvent, AlterTableEvent, DropTableEvent
 from datasette.utils.asgi import Response, NotFound, Forbidden
 from datasette.utils import sqlite3
 from urllib.parse import quote_plus, unquote_plus
@@ -20,13 +21,6 @@
 FOREIGN_KEY_DETECTION_LIMIT = 10_000
 
 
-async def track_event(datasette, klass_name, **kwargs):
-    if not hasattr(datasette, "track_event"):
-        return
-    klass = getattr(events, klass_name)
-    await datasette.track_event(klass(**kwargs))
-
-
 @hookimpl
 def permission_allowed(actor, action, resource):
     if (
@@ -275,13 +269,13 @@ def create_the_table(conn):
         else:
             datasette.add_message(request, "Table has been created")
             path = datasette.urls.table(database_name, table_name)
-            await track_event(
-                datasette,
-                "CreateTableEvent",
-                actor=request.actor,
-                database=database_name,
-                table=table_name,
-                schema=schema,
+            await datasette.track_event(
+                CreateTableEvent(
+                    actor=request.actor,
+                    database=database_name,
+                    table=table_name,
+                    schema=schema,
+                )
             )
 
         return Response.redirect(path)
@@ -318,6 +312,29 @@ async def edit_schema_table(request, datasette):
         raise NotFound("Table not found")
 
     if request.method == "POST":
+
+        def get_schema(conn):
+            table_obj = sqlite_utils.Database(conn)[table]
+            if not table_obj.exists():
+                return None
+            return table_obj.schema
+
+        before_schema = await database.execute_fn(get_schema)
+
+        async def track_analytics():
+            after_schema = await database.execute_fn(get_schema)
+            # Don't track drop tables, which happen when after_schema is None
+            if after_schema is not None and after_schema != before_schema:
+                await datasette.track_event(
+                    AlterTableEvent(
+                        actor=request.actor,
+                        database=database_name,
+                        table=table,
+                        before_schema=before_schema,
+                        after_schema=after_schema,
+                    )
+                )
+
         formdata = await request.post_vars()
         if formdata.get("action") == "update_columns":
             types = {}
@@ -373,31 +390,35 @@ def transform_the_table(conn):
             await database.execute_write_fn(transform_the_table, block=True)
 
             datasette.add_message(request, "Changes to table have been saved")
-
+            await track_analytics()
             return Response.redirect(request.path)
 
         if formdata.get("action") == "update_foreign_keys":
-            return await update_foreign_keys(
+            response = await update_foreign_keys(
                 request, datasette, database, table, formdata
             )
         elif formdata.get("action") == "update_primary_key":
-            return await update_primary_key(
+            response = await update_primary_key(
                 request, datasette, database, table, formdata
             )
         elif "drop_table" in formdata:
-            return await drop_table(request, datasette, database, table)
+            response = await drop_table(request, datasette, database, table)
         elif "add_column" in formdata:
-            return await add_column(request, datasette, database, table, formdata)
+            response = await add_column(request, datasette, database, table, formdata)
         elif "rename_table" in formdata:
-            return await rename_table(request, datasette, database, table, formdata)
+            response = await rename_table(request, datasette, database, table, formdata)
         elif "add_index" in formdata:
             column = formdata.get("add_index_column") or ""
             unique = formdata.get("add_index_unique")
-            return await add_index(request, datasette, database, table, column, unique)
+            response = await add_index(
+                request, datasette, database, table, column, unique
+            )
         elif any(key.startswith("drop_index_") for key in formdata.keys()):
-            return await drop_index(request, datasette, database, table, formdata)
+            response = await drop_index(request, datasette, database, table, formdata)
         else:
-            return Response.html("Unknown operation", status=400)
+            response = Response.html("Unknown operation", status=400)
+        await track_analytics()
+        return response
 
     def get_columns_and_schema_and_fks_and_pks_and_indexes(conn):
         db = sqlite_utils.Database(conn)
@@ -456,13 +477,15 @@ def get_columns_and_schema_and_fks_and_pks_and_indexes(conn):
     all_columns_to_manage_foreign_keys = [
         {
             "name": column["name"],
-            "foreign_key": foreign_keys_by_column.get(column["name"])[0]
-            if foreign_keys_by_column.get(column["name"])
-            else None,
+            "foreign_key": (
+                foreign_keys_by_column.get(column["name"])[0]
+                if foreign_keys_by_column.get(column["name"])
+                else None
+            ),
             "suggestions": [],
-            "options": integer_primary_keys
-            if column["type"] is int
-            else string_primary_keys,
+            "options": (
+                integer_primary_keys if column["type"] is int else string_primary_keys
+            ),
         }
         for column in columns
     ]
@@ -607,12 +630,12 @@ def do_drop_table(conn):
         await database.execute_write_fn(do_drop_table)
 
     datasette.add_message(request, "Table has been deleted")
-    await track_event(
-        datasette,
-        "DropTableEvent",
-        actor=request.actor,
-        database=database.name,
-        table=table,
+    await datasette.track_event(
+        DropTableEvent(
+            actor=request.actor,
+            database=database.name,
+            table=table,
+        )
     )
     return Response.redirect("/-/edit-schema/" + database.name)
 
@@ -677,6 +700,9 @@ async def rename_table(request, datasette, database, table, formdata):
         return redirect
 
     try:
+        before_schema = await database.execute_fn(
+            lambda conn: sqlite_utils.Database(conn)[table].schema
+        )
         await database.execute_write(
             """
             ALTER TABLE [{}] RENAME TO [{}];
@@ -685,9 +711,22 @@ async def rename_table(request, datasette, database, table, formdata):
             ),
             block=True,
         )
+        after_schema = await database.execute_fn(
+            lambda conn: sqlite_utils.Database(conn)[new_name].schema
+        )
         datasette.add_message(
             request, "Table renamed to '{}'".format(new_name), datasette.INFO
         )
+        await datasette.track_event(
+            AlterTableEvent(
+                actor=request.actor,
+                database=database.name,
+                table=new_name,
+                before_schema=before_schema,
+                after_schema=after_schema,
+            )
+        )
+
     except Exception as error:
         datasette.add_message(
             request, "Error renaming table: {}".format(str(error)), datasette.ERROR

tests/test_edit_schema.py

@@ -19,10 +19,6 @@ def get_last_event(datasette):
     events = getattr(datasette, "_tracked_events", [])
     if events:
         return events[-1]
-    else:
-        # If we are running Datasette >=1.0a8 this is an error
-        if hasattr(datasette, "track_event"):
-            raise Exception("No event was tracked")
 
 
 @pytest.mark.asyncio
@@ -347,6 +343,7 @@ async def test_transform_table(
     cookies = {"ds_actor": ds.sign({"a": {"id": "root"}}, "actor")}
     db = sqlite_utils.Database(db_path)
     table = db["creatures"]
+    before_schema = table.schema
     assert table.columns_dict == {"name": str, "description": str}
     csrftoken = (
         await ds.client.get("/-/edit-schema/data/creatures", cookies=cookies)
@@ -365,6 +362,11 @@ async def test_transform_table(
     assert [c.name for c in table.columns] == expected_order
     assert len(messages) == 1
     assert messages[0][0] == expected_message
+    # Should have tracked an event
+    event = get_last_event(ds)
+    assert event.name == "alter-table"
+    assert event.before_schema == before_schema
+    assert event.after_schema == table.schema
 
 
 @pytest.mark.asyncio
@@ -612,6 +614,7 @@ async def test_rename_table(db_path, new_name, should_work, expected_message):
     csrftoken = (
         await ds.client.get("/-/edit-schema/data/creatures", cookies=cookies)
     ).cookies["ds_csrftoken"]
+    before_schema = sqlite_utils.Database(db_path)["creatures"].schema
     response = await ds.client.post(
         "/-/edit-schema/data/creatures",
         data={
@@ -624,12 +627,29 @@ async def test_rename_table(db_path, new_name, should_work, expected_message):
     assert response.status_code == 302
     if should_work:
         expected_path = "/-/edit-schema/data/{}".format(new_name)
+        if expected_message != "Table name was the same":
+            event = get_last_event(ds)
+            if event:
+                assert event.name == "alter-table"
+                assert event.table == new_name
+                assert new_name in event.properties()["after_schema"]
+                assert "creatures" in event.properties()["before_schema"]
+
     else:
         expected_path = "/-/edit-schema/data/creatures"
     assert response.headers["location"] == expected_path
     messages = ds.unsign(response.cookies["ds_messages"], "messages")
     assert len(messages) == 1
     assert messages[0][0] == expected_message
+    if should_work:
+        # Should have tracked alter-table against the new table name
+        event = get_last_event(ds)
+        if expected_message == "Table name was the same":
+            assert event is None
+        else:
+            assert event.name == "alter-table"
+            assert event.before_schema == before_schema
+            assert event.after_schema == sqlite_utils.Database(db_path)[new_name].schema
 
 
 @pytest.mark.asyncio
@@ -925,10 +945,10 @@ async def test_create_table(db_path, post_data, expected_message, expected_schem
     if expected_schema is not None:
         db = sqlite_utils.Database(db_path)
         assert db[post_data["table_name"]].columns_dict == expected_schema
-        # And create-table should have been tracked
-        event = get_last_event(ds)
-        if event:
-            assert event.name == "create-table"
+    # create-table should have been tracked
+    event = get_last_event(ds)
+    if event:
+        assert event.name == "create-table"
 
 
 def test_examples_for_columns():