Slightly wonky implementation of pagination for user tables

csvbase/svc.py

@@ -4,7 +4,7 @@
 from contextlib import closing
 from datetime import datetime, timezone, date
 from logging import getLogger
-from typing import Iterable, Optional, Sequence, Tuple, cast, List
+from typing import Iterable, Optional, Sequence, Tuple, cast, List, Union
 from uuid import UUID, uuid4
 from dataclasses import dataclass
 
@@ -24,6 +24,7 @@
 from sqlalchemy.sql.expression import table as satable
 from sqlalchemy.dialects.postgresql import insert as pginsert
 import importlib_resources
+from typing_extensions import Literal
 
 from . import exc, models, table_io
 from .userdata import PGUserdataAdapter
@@ -41,7 +42,7 @@
     ContentType,
     BinaryOp,
 )
-from .constants import MIN_UUID, FAR_FUTURE
+from .constants import FAR_FUTURE, MAX_UUID
 from .follow.git import GitSource, get_repo_path
 from .repcache import RepCache
 
@@ -527,76 +528,55 @@ def is_valid_api_key(sesh: Session, username: str, hex_api_key: str) -> bool:
     return exists
 
 
-def tables_for_user(
-    sesh: Session, user_uuid: UUID, include_private: bool = False
-) -> Iterable[Table]:
-    rp = (
-        sesh.query(models.Table, models.User.username, models.GitUpstream)
-        .join(models.User)
-        .outerjoin(models.GitUpstream)
-        .filter(models.Table.user_uuid == user_uuid)
-        .order_by(models.Table.created.desc())
-    )
-    if not include_private:
-        rp = rp.filter(models.Table.public)
-    backend = PGUserdataAdapter(sesh)
-    for table_model, username, source in rp:
-        columns = backend.get_columns(table_model.table_uuid)
-        row_count = backend.count(table_model.table_uuid)
-        unique_column_names = (
-            sesh.query(func.array_agg(models.UniqueColumn.column_name))
-            .filter(models.UniqueColumn.table_uuid == table_model.table_uuid)
-            .scalar()
-        )
-        yield _make_table(
-            username, table_model, columns, row_count, source, unique_column_names
-        )
-
-
 @dataclass
 class UserTablePage:
-    has_more: bool
-    has_less: bool
+    has_next: bool
+    has_prev: bool
     tables: List[Table]
 
 
 def table_page(
     sesh: Session,
     user_uuid: UUID,
-    viewer: User,
-    op: BinaryOp = BinaryOp.LT,
-    key: Tuple[datetime, UUID] = (FAR_FUTURE, MIN_UUID),
+    viewer: Optional[User],
+    op: Union[Literal[BinaryOp.LT], Literal[BinaryOp.GT]] = BinaryOp.LT,
+    key: Tuple[datetime, UUID] = (FAR_FUTURE, MAX_UUID),
     count: int = 10,
 ) -> UserTablePage:
-    """Return a page of tables"""
-    base_query = (
+    """Return a page of tables for the given user, as should be seen by the
+    perspective of the given viewer.
+
+    """
+    # all tables by the given user_uuid that viewer should be able to see
+    visible_tables = (
         sesh.query(models.Table, models.User.username, models.GitUpstream)
         .join(models.User)
         .outerjoin(models.GitUpstream)
         .filter(models.Table.user_uuid == user_uuid)
     )
-    if user_uuid != viewer.user_uuid:
-        base_query = base_query.filter(models.Table.public)
+    if viewer is not None and user_uuid != viewer.user_uuid:
+        visible_tables = visible_tables.filter(models.Table.public)
 
     table_key = satuple(models.Table.last_changed, models.Table.table_uuid)
-    page_key = satuple(*key)
-
-    rp = base_query
+    page_key = satuple(*key)  # type: ignore
+    page_of_tables = visible_tables
     if op is BinaryOp.LT:
-        rp = rp.filter(table_key < page_key).order_by(*(t.desc() for t in table_key))
-        rp = rp.limit(count)
+        page_of_tables = page_of_tables.filter(table_key < page_key).order_by(
+            table_key.desc()
+        )
+        page_of_tables = page_of_tables.limit(count)
     elif op is BinaryOp.GT:
-        rp = rp.filter(table_key > page_key).order_by(*table_key)
-        rp = rp.limit(count)
+        page_of_tables = page_of_tables.filter(table_key > page_key).order_by(table_key)
+        page_of_tables = page_of_tables.limit(count)
 
         # necessary to reverse in subquery to get the tables in the right order
         # FIXME: this is not 2.0 safe:
         # https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#selecting-from-the-query-itself-as-a-subquery-e-g-from-self
-        rp = rp.from_self().order_by(table_key.desc())
+        page_of_tables = page_of_tables.from_self().order_by(table_key.desc())
 
     backend = PGUserdataAdapter(sesh)
     tables = []
-    for table_model, username, source in rp:
+    for table_model, username, source in page_of_tables:
         columns = backend.get_columns(table_model.table_uuid)
         row_count = backend.count(table_model.table_uuid)
         unique_column_names = (
@@ -610,19 +590,19 @@ def table_page(
             )
         )
 
-    if len(tables) > 1:
+    if len(tables) > 0:
         first_table = (tables[0].last_changed, tables[0].table_uuid)
         last_table = (tables[-1].last_changed, tables[-1].table_uuid)
-        has_more = sesh.query(
-            base_query.filter(table_key < last_table).exists()
+        has_next = sesh.query(
+            visible_tables.filter(table_key < last_table).exists()
         ).scalar()
-        has_less = sesh.query(
-            base_query.filter(table_key > first_table).exists()
+        has_prev = sesh.query(
+            visible_tables.filter(table_key > first_table).exists()
         ).scalar()
     else:
         raise NotImplementedError()
 
-    return UserTablePage(has_more=has_more, has_less=has_less, tables=tables)
+    return UserTablePage(has_next=has_next, has_prev=has_prev, tables=tables)
 
 
 def _make_table(

csvbase/value_objs.py

@@ -89,12 +89,12 @@ class KeySet:
 
 @enum.unique
 class BinaryOp(enum.Enum):
-    EQ = 1
-    NQE = 2
-    GT = 3
-    GTE = 4
-    LT = 5
-    LTE = 6
+    EQ = "eq"
+    NQE = "nqe"
+    GT = "gt"
+    GTE = "gte"
+    LT = "lt"
+    LTE = "lte"
 
 
 @dataclass

csvbase/web/main/bp.py

@@ -2,7 +2,7 @@
 from uuid import UUID
 from pathlib import Path
 import shutil
-from datetime import timedelta, timezone
+from datetime import datetime, timedelta, timezone
 import codecs
 from logging import getLogger
 from typing import (
@@ -11,12 +11,13 @@
     Mapping,
     Optional,
     Sequence,
-    Union,
     cast,
     Iterator,
     IO,
     TypeVar,
     List,
+    Union,
+    Tuple,
 )
 from urllib.parse import urlsplit, urlunsplit, parse_qsl, urlencode
 import hashlib
@@ -77,8 +78,9 @@
     Row,
     Table,
     Backend,
+    BinaryOp,
 )
-from ...constants import COPY_BUFFER_SIZE
+from ...constants import COPY_BUFFER_SIZE, FAR_FUTURE, MAX_UUID
 from ..billing import svc as billing_svc
 from ...repcache import RepCache
 from ...config import get_config
@@ -1133,29 +1135,85 @@ def delete_row_for_browsers(username: str, table_name: str, row_id: int) -> Resp
 
 @bp.get("/<username>")
 def user(username: str) -> Response:
+    def get_op() -> Union[Literal[BinaryOp.LT], Literal[BinaryOp.GT]]:
+        """Reads the BinaryOp out of the request args"""
+        op_value = request.args.get("op", None)
+        if op_value is None:
+            return BinaryOp.LT
+        elif op_value not in {"gt", "lt"}:
+            raise exc.InvalidRequest()
+        else:
+            return BinaryOp(op_value)  # type: ignore
+
+    def get_key() -> Tuple[datetime, UUID]:
+        """Reads the key out of the request args"""
+        key_value = request.args.get("key", None)
+        if key_value is None:
+            return (FAR_FUTURE, MAX_UUID)
+        try:
+            datetime_str, uuid_str = key_value.split(",")
+            uuid = UUID(uuid_str)
+            dt = datetime.fromisoformat(datetime_str)
+            return dt, uuid
+        except Exception:
+            raise exc.InvalidRequest()
+
     sesh = get_sesh()
-    include_private = False
     current_user = get_current_user()
-    if current_user is not None and current_user.username == username:
-        include_private = True
-        has_subscription = billing_svc.has_subscription(sesh, current_user.user_uuid)
-    else:
-        has_subscription = False
+
+    content_type = negotiate_content_type([ContentType.JSON, ContentType.HTML])
+
     user = svc.user_by_name(sesh, username)
-    tables = svc.tables_for_user(
-        sesh,
-        user.user_uuid,
-        include_private=include_private,
+    table_page = svc.table_page(
+        sesh, user.user_uuid, current_user, op=get_op(), key=get_key()
     )
-    return make_response(
-        render_template(
-            "user.html",
-            user=user,
-            page_title=f"{username}",
-            tables=list(tables),
-            show_manage_subscription=has_subscription,
+
+    if table_page.has_next:
+        last_table = table_page.tables[-1]
+        next_page_url = url_for(
+            "csvbase.user",
+            username=username,
+            op="lt",
+            key=",".join(
+                [last_table.last_changed.isoformat(), str(last_table.table_uuid)]
+            ),
         )
-    )
+    else:
+        next_page_url = None
+
+    if table_page.has_prev:
+        first_table = table_page.tables[0]
+        prev_page_url = url_for(
+            "csvbase.user",
+            username=username,
+            op="gt",
+            key=",".join(
+                [first_table.last_changed.isoformat(), str(first_table.table_uuid)]
+            ),
+        )
+    else:
+        prev_page_url = None
+
+    if content_type is ContentType.HTML:
+        if current_user is not None and current_user.username == username:
+            has_subscription = billing_svc.has_subscription(
+                sesh, current_user.user_uuid
+            )
+        else:
+            has_subscription = False
+        return make_response(
+            render_template(
+                "user.html",
+                user=user,
+                page_title=f"{username}",
+                table_page=table_page,
+                show_manage_subscription=has_subscription,
+                next_page_url=next_page_url,
+                prev_page_url=prev_page_url,
+            )
+        )
+    else:
+        raise NotImplementedError()
 
 
 @bp.route("/<username>/settings", methods=["GET", "POST"])

csvbase/web/templates/user.html

@@ -3,6 +3,22 @@
 
 {% import 'table_macros.html' as table_macros %}
 
+{% macro pagination() %}
+  <nav>
+    <ul class="pagination">
+      <li class="page-item {% if not table_page.has_prev%}disabled{% endif %}"><a class="page-link"
+                               {% if table_page.has_prev %}href="{{prev_page_url}}"{% else %}href="#"{% endif %}
+                               >Previous</a>
+      </li>
+      <li class="page-item {% if not table_page.has_next%}disabled{% endif %}"><a class="page-link"
+                               {% if table_page.has_next %}href="{{next_page_url}}"{% else %}href="#"{% endif %}
+                               >Next</a>
+      </li>
+    </ul>
+  </nav>
+{% endmacro %}
+
+
 {% block main %}
   <div class="container">
     <h1><a href="{{ url_for('csvbase.user', username=user.username) }}">{{ user.username }}</a></h1>
@@ -19,9 +35,11 @@ <h2>Your subscription</h2>
       <p>You are currently subscribed. <a href="{{ url_for('billing.manage') }}">Manage your subscription</a>.</p>
     {% endif %}
 
-
     <h2>Tables</h2>
-    {% for table in tables %}
+
+    {{ pagination() }}
+
+    {% for table in table_page.tables %}
       <div class="row">
         <div class="col-md-8">
           {{ table_macros.table_card(table) }}

tests/test_table_pagination.py

@@ -24,8 +24,8 @@ def test_first_page(sesh, user_with_tables):
     page = svc.table_page(sesh, user_with_tables.user_uuid, user_with_tables, count=2)
     table_names = [t.table_name for t in page.tables]
     assert table_names == ["table-10", "table-9"]
-    assert not page.has_less
-    assert page.has_more
+    assert not page.has_prev
+    assert page.has_next
 
 
 def test_second_page(sesh, user_with_tables):
@@ -43,8 +43,8 @@ def test_second_page(sesh, user_with_tables):
     )
     table_names = [t.table_name for t in second_page.tables]
     assert table_names == ["table-8", "table-7"]
-    assert second_page.has_less
-    assert second_page.has_more
+    assert second_page.has_prev
+    assert second_page.has_next
 
 
 def test_back_to_first_page(sesh, user_with_tables):
@@ -73,8 +73,8 @@ def test_back_to_first_page(sesh, user_with_tables):
 
     table_names = [t.table_name for t in back_to_first_page.tables]
     assert table_names == ["table-10", "table-9"]
-    assert not back_to_first_page.has_less
-    assert back_to_first_page.has_more
+    assert not back_to_first_page.has_prev
+    assert back_to_first_page.has_next
 
 
 @pytest.mark.xfail(reason="test not implemented")