Add additional filters to page list endpoints (#1622)

Fixes #1617 Filters added: - reviewed: filter by page has approval or at least one note (true) or neither (false) - approved: filter by approval value (accepts list of strings, comma-separated, each of which are coerced into True, False, or None, or ignored if they are invalid values) - hasNotes: filter by has at least one note (true) or not (false) Tests have also been added to ensure that results are as expected.
2024-03-22 00:33:07 -04:00 · 2024-03-22 00:33:07 -04:00 · e9895e78a2
commit e9895e78a2
parent b3b1e0d7d8
3 changed files with 245 additions and 3 deletions
--- a/backend/btrixcloud/pages.py
+++ b/backend/btrixcloud/pages.py
@ -24,7 +24,7 @@ from .models import (
    PageNoteDelete,
 )
 from .pagination import DEFAULT_PAGE_SIZE, paginated_format
-from .utils import from_k8s_date
+from .utils import from_k8s_date, str_list_to_bools

 if TYPE_CHECKING:
    from .crawls import CrawlOps
@ -360,13 +360,16 @@ class PageOps:
        qa_gt: Optional[float] = None,
        qa_lte: Optional[float] = None,
        qa_lt: Optional[float] = None,
+        reviewed: Optional[bool] = None,
+        approved: Optional[List[Union[bool, None]]] = None,
+        has_notes: Optional[bool] = None,
        page_size: int = DEFAULT_PAGE_SIZE,
        page: int = 1,
        sort_by: Optional[str] = None,
        sort_direction: Optional[int] = -1,
    ) -> Tuple[Union[List[PageOut], List[PageOutWithSingleQA]], int]:
        """List all pages in crawl"""
-        # pylint: disable=duplicate-code, too-many-locals, too-many-branches
+        # pylint: disable=duplicate-code, too-many-locals, too-many-branches, too-many-statements
        # Zero-index page for query
        page = page - 1
        skip = page_size * page
@ -377,6 +380,24 @@ class PageOps:
        if org:
            query["oid"] = org.id

+        if reviewed:
+            query["$or"] = [
+                {"approved": {"$ne": None}},
+                {"notes.0": {"$exists": True}},
+            ]
+
+        if reviewed is False:
+            query["$and"] = [
+                {"approved": {"$eq": None}},
+                {"notes.0": {"$exists": False}},
+            ]
+
+        if approved:
+            query["approved"] = {"$in": approved}
+
+        if has_notes is not None:
+            query["notes.0"] = {"$exists": has_notes}
+
        if qa_run_id:
            query[f"qa.{qa_run_id}"] = {"$exists": True}

@ -576,15 +597,25 @@ def init_pages_api(app, mdb, crawl_ops, org_ops, storage_ops, user_dep):
    async def get_pages_list(
        crawl_id: str,
        org: Organization = Depends(org_crawl_dep),
+        reviewed: Optional[bool] = None,
+        approved: Optional[str] = None,
+        hasNotes: Optional[bool] = None,
        pageSize: int = DEFAULT_PAGE_SIZE,
        page: int = 1,
        sortBy: Optional[str] = None,
        sortDirection: Optional[int] = -1,
    ):
        """Retrieve paginated list of pages"""
+        formatted_approved: Optional[List[Union[bool, None]]] = None
+        if approved:
+            formatted_approved = str_list_to_bools(approved.split(","))
+
        pages, total = await ops.list_pages(
            crawl_id=crawl_id,
            org=org,
+            reviewed=reviewed,
+            approved=formatted_approved,
+            has_notes=hasNotes,
            page_size=pageSize,
            page=page,
            sort_by=sortBy,
@ -605,6 +636,9 @@ def init_pages_api(app, mdb, crawl_ops, org_ops, storage_ops, user_dep):
        gt: Optional[float] = None,
        lte: Optional[float] = None,
        lt: Optional[float] = None,
+        reviewed: Optional[bool] = None,
+        approved: Optional[str] = None,
+        hasNotes: Optional[bool] = None,
        org: Organization = Depends(org_crawl_dep),
        pageSize: int = DEFAULT_PAGE_SIZE,
        page: int = 1,
@ -612,6 +646,10 @@ def init_pages_api(app, mdb, crawl_ops, org_ops, storage_ops, user_dep):
        sortDirection: Optional[int] = -1,
    ):
        """Retrieve paginated list of pages"""
+        formatted_approved: Optional[List[Union[bool, None]]] = None
+        if approved:
+            formatted_approved = str_list_to_bools(approved.split(","))
+
        pages, total = await ops.list_pages(
            crawl_id=crawl_id,
            org=org,
@ -621,6 +659,9 @@ def init_pages_api(app, mdb, crawl_ops, org_ops, storage_ops, user_dep):
            qa_gt=gt,
            qa_lte=lte,
            qa_lt=lt,
+            reviewed=reviewed,
+            approved=formatted_approved,
+            has_notes=hasNotes,
            page_size=pageSize,
            page=page,
            sort_by=sortBy,
--- a/backend/btrixcloud/utils.py
+++ b/backend/btrixcloud/utils.py
@ -94,10 +94,30 @@ def parse_jsonl_error_messages(errors):
 def is_bool(stri: Optional[str]) -> bool:
    """Check if the string parameter is stringly true"""
    if stri:
-        return stri.lower() in ("true", "1", "yes")
+        return stri.lower() in ("true", "1", "yes", "on")
    return False


+def is_falsy_bool(stri: Optional[str]) -> bool:
+    """Check if the string parameter is stringly false"""
+    if stri:
+        return stri.lower() in ("false", "0", "no", "off")
+    return False
+
+
+def str_list_to_bools(str_list: List[str], allow_none=True) -> List[Union[bool, None]]:
+    """Return version of input string list cast to bool or None, ignoring other values"""
+    output: List[Union[bool, None]] = []
+    for val in str_list:
+        if is_bool(val):
+            output.append(True)
+        if is_falsy_bool(val):
+            output.append(False)
+        if val.lower() in ("none", "null") and allow_none:
+            output.append(None)
+    return output
+
+
 def slug_from_name(name: str) -> str:
    """Generate slug from name"""
    return slugify(name.replace("'", ""))
--- a/backend/test/test_run_crawl.py
+++ b/backend/test/test_run_crawl.py
@ -459,6 +459,21 @@ def test_crawl_pages(crawler_auth_headers, default_org_id, crawler_crawl_id):
    assert page.get("modified") is None
    assert page.get("approved") is None

+    # Test reviewed filter (page has no notes or approved so should show up in false)
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?reviewed=False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?reviewed=True",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
    # Update page with approval
    r = requests.patch(
        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages/{page_id}",
@ -470,6 +485,57 @@ def test_crawl_pages(crawler_auth_headers, default_org_id, crawler_crawl_id):
    assert r.status_code == 200
    assert r.json()["updated"]

+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages/{page_id}",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["approved"]
+
+    # Test approval filter
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=True",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=True,False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=None",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    # Test reviewed filter (page now approved so should show up in True)
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?reviewed=False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?reviewed=True",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
    r = requests.get(
        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages/{page_id}",
        headers=crawler_auth_headers,
@ -490,6 +556,44 @@ def test_crawl_pages(crawler_auth_headers, default_org_id, crawler_crawl_id):
    assert page["modified"]
    assert page["approved"]

+    # Set approved to False and test filter again
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages/{page_id}",
+        headers=crawler_auth_headers,
+        json={
+            "approved": False,
+        },
+    )
+    assert r.status_code == 200
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=True",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=True,False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=None",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+

 def test_re_add_crawl_pages(crawler_auth_headers, default_org_id, crawler_crawl_id):
    # Re-add pages and verify they were correctly added
@ -565,6 +669,83 @@ def test_crawl_page_notes(crawler_auth_headers, default_org_id, crawler_crawl_id
    assert first_note["userName"]
    assert first_note["text"] == note_text

+    # Make sure page approval is set to None and re-test filters
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages/{page_id}",
+        headers=crawler_auth_headers,
+        json={
+            "approved": None,
+        },
+    )
+    assert r.status_code == 200
+    assert r.json()["updated"]
+
+    # Test approved filter
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=True",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=True,False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=None",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?approved=true,false,none",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
+    # Test reviewed filter (page now has notes so should show up in True)
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?reviewed=False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?reviewed=True",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
+    # Test notes filter
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?hasNotes=False",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 0
+
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages?hasNotes=True",
+        headers=crawler_auth_headers,
+    )
+    assert r.status_code == 200
+    assert r.json()["total"] == 1
+
    # Add second note to test selective updates/deletes
    r = requests.post(
        f"{API_PREFIX}/orgs/{default_org_id}/crawls/{crawler_crawl_id}/pages/{page_id}/notes",