Merge pull request #210 from openzim/display-books-that-need-attention

display books that need attention in inbox

Author: elfkuzco

backend/src/cms_backend/api/routes/books.py

@@ -38,6 +38,7 @@ class BooksGetSchema(BaseModel):
     has_error: bool | None = None
     needs_file_operation: bool | None = None
     location_kinds: list[NotEmptyString] | None = None
+    needs_attention: bool | None = None
 
 
 @router.get("")
@@ -57,6 +58,7 @@ def get_books(
         has_error=params.has_error,
         needs_file_operation=params.needs_file_operation,
         location_kinds=params.location_kinds,
+        needs_attention=params.needs_attention,
     )
 
     return ListResponse[BookLightSchema](

backend/src/cms_backend/db/books.py

@@ -1,7 +1,7 @@
 from uuid import UUID
 
 from pydantic import AnyUrl
-from sqlalchemy import String, and_, select
+from sqlalchemy import String, and_, or_, select
 from sqlalchemy.orm import Session as OrmSession
 
 from cms_backend.db import count_from_stmt
@@ -22,6 +22,7 @@ def get_books(
     has_error: bool | None = None,
     needs_file_operation: bool | None = None,
     location_kinds: list[str] | None = None,
+    needs_attention: bool | None = None,
 ) -> ListResult[BookLightSchema]:
     """Get a list of books"""
 
@@ -60,6 +61,25 @@ def get_books(
     if location_kinds is not None:
         stmt = stmt.where(Book.location_kind.in_(location_kinds))
 
+    if needs_attention is True:
+        stmt = stmt.where(
+            or_(
+                Book.title_id.is_(None),
+                Book.needs_processing.is_(True),
+                Book.needs_file_operation.is_(True),
+                Book.has_error.is_(True),
+            )
+        )
+    elif needs_attention is False:
+        stmt = stmt.where(
+            and_(
+                Book.title_id.is_not(None),
+                Book.needs_processing.is_(False),
+                Book.needs_file_operation.is_(False),
+                Book.has_error.is_(False),
+            )
+        )
+
     return ListResult[BookLightSchema](
         nb_records=count_from_stmt(session, stmt),
         records=[
@@ -88,7 +108,16 @@ def get_books(
                 name,
                 date,
                 flavour,
-            ) in session.execute(stmt.offset(skip).limit(limit)).all()
+            ) in session.execute(
+                stmt.offset(skip)
+                .limit(limit)
+                .order_by(
+                    Book.has_error,
+                    Book.location_kind,
+                    Book.needs_file_operation,
+                    Book.created_at.desc(),
+                )
+            ).all()
         ],
     )
 

backend/tests/api/routes/test_books.py

@@ -171,6 +171,68 @@ def test_get_books_combined_filters(
     assert response_doc["meta"]["count"] == 2
 
 
+@pytest.mark.parametrize(
+    "needs_attention,expected_count",
+    [
+        pytest.param(None, 5, id="no-filter"),
+        pytest.param(True, 4, id="needs-attention"),
+        pytest.param(False, 1, id="does-not-need-attention"),
+    ],
+)
+def test_get_books_filter_by_needs_attention(
+    client: TestClient,
+    create_book: Callable[..., Book],
+    create_title: Callable[..., Title],
+    needs_attention: bool | None,
+    expected_count: int,
+):
+    """Test get books endpoint filtering by needs_attention"""
+
+    title = create_title()
+
+    book_with_title = create_book(zim_metadata={"Name": title.name})
+    title.books.append(book_with_title)
+
+    book_without_title = create_book(zim_metadata={"Name": "different_name"})
+
+    book_needs_processing = create_book(zim_metadata={"Name": title.name})
+    title.books.append(book_needs_processing)
+    book_needs_processing.needs_processing = True
+
+    book_has_error = create_book(zim_metadata={"Name": title.name})
+    title.books.append(book_has_error)
+    book_has_error.has_error = True
+
+    book_needs_file_operation = create_book(zim_metadata={"Name": title.name})
+    title.books.append(book_needs_file_operation)
+    book_needs_file_operation.needs_file_operation = True
+
+    url = (
+        "/v1/books"
+        if needs_attention is None
+        else f"/v1/books?needs_attention={str(needs_attention).lower()}"
+    )
+    response = client.get(url)
+
+    assert response.status_code == HTTPStatus.OK
+    response_doc = response.json()
+    assert response_doc["meta"]["count"] == expected_count
+    assert len(response_doc["items"]) == expected_count
+
+    if needs_attention is True:
+        returned_ids = {item["id"] for item in response_doc["items"]}
+        assert returned_ids == {
+            str(book_without_title.id),
+            str(book_needs_processing.id),
+            str(book_has_error.id),
+            str(book_needs_file_operation.id),
+        }
+
+    if needs_attention is False:
+        returned_ids = {item["id"] for item in response_doc["items"]}
+        assert returned_ids == {str(book_with_title.id)}
+
+
 def test_get_books_filter_by_id(
     client: TestClient,
     create_book: Callable[..., Book],

backend/tests/db/test_books.py

@@ -265,6 +265,67 @@ def test_get_books_book_id_combined_with_other_filters(
     assert results.records[0].id == book3.id
 
 
+@pytest.mark.parametrize(
+    "needs_attention,expected_count",
+    [
+        pytest.param(None, 5, id="no-filter"),
+        pytest.param(True, 4, id="needs-attention"),
+        pytest.param(False, 1, id="does-not-need-attention"),
+    ],
+)
+def test_get_books_filter_by_needs_attention(
+    dbsession: OrmSession,
+    create_book: Callable[..., Book],
+    create_title: Callable[..., Title],
+    needs_attention: bool | None,
+    expected_count: int,
+):
+    """Test that get_books works correctly with needs_attention filter"""
+
+    title = create_title()
+
+    book_with_title = create_book(zim_metadata={"Name": title.name})
+    title.books.append(book_with_title)
+
+    book_without_title = create_book()
+
+    book_needs_processing = create_book(zim_metadata={"Name": title.name})
+    title.books.append(book_needs_processing)
+    book_needs_processing.needs_processing = True
+
+    book_has_error = create_book(zim_metadata={"Name": title.name})
+    title.books.append(book_has_error)
+    book_has_error.has_error = True
+
+    book_needs_file_operation = create_book(zim_metadata={"Name": title.name})
+    title.books.append(book_needs_file_operation)
+    book_needs_file_operation.needs_file_operation = True
+
+    dbsession.flush()
+
+    results = get_books(
+        dbsession,
+        skip=0,
+        limit=20,
+        needs_attention=needs_attention,
+    )
+
+    assert results.nb_records == expected_count
+    assert len(results.records) == expected_count
+
+    if needs_attention is True:
+        returned_ids = {record.id for record in results.records}
+        assert returned_ids == {
+            book_without_title.id,
+            book_needs_processing.id,
+            book_has_error.id,
+            book_needs_file_operation.id,
+        }
+    elif needs_attention is False:
+        returned_ids = {record.id for record in results.records}
+        assert returned_ids == {book_with_title.id}
+
+
 def test_get_zim_urls(
     dbsession: OrmSession,
     create_book: Callable[..., Book],

frontend/src/components/BookFilters.vue

@@ -27,6 +27,20 @@
             @click:clear="handleClearFilters"
           />
         </v-col>
+        <v-col cols="12" sm="6" md="3">
+          <v-select
+            v-model="localFilters.flag"
+            label="Flag"
+            :items="flagOptions"
+            placeholder="Select flag"
+            variant="outlined"
+            density="compact"
+            hide-details
+            :clearable="hasActiveFilters"
+            @update:model-value="emitFilters"
+            @click:clear="handleClearFilters"
+          />
+        </v-col>
         <v-col
           v-if="hasActiveFilters"
           cols="12"
@@ -50,6 +64,7 @@ interface Props {
   filters: {
     id: string
     location_kind: string
+    flag: string
   }
   locationKindOptions?: string[]
 }
@@ -64,6 +79,7 @@ const emit = defineEmits<{
     filters: {
       id: string
       location_kind: string
+      flag: string
     },
   ]
   clearFilters: []
@@ -73,6 +89,7 @@ const emit = defineEmits<{
 const localFilters = ref({
   id: props.filters.id,
   location_kind: props.filters.location_kind,
+  flag: props.filters.flag,
 })
 
 // Watch for prop changes and update local state
@@ -82,6 +99,7 @@ watch(
     localFilters.value = {
       id: newFilters.id,
       location_kind: newFilters.location_kind,
+      flag: newFilters.flag,
     }
   },
 )
@@ -93,15 +111,27 @@ const formattedLocationKindOptions = computed(() => {
   }))
 })
 
+const flagOptions = [
+  { title: 'Needs File Operation', value: 'needs_file_operation' },
+  { title: 'Needs Processing', value: 'needs_processing' },
+  { title: 'Has Error', value: 'has_error' },
+  { title: 'Pending Title', value: 'no_title' },
+]
+
 const hasActiveFilters = computed(() => {
-  return props.filters.id.length > 0 || props.filters.location_kind.length > 0
+  return (
+    props.filters.id.length > 0 ||
+    props.filters.location_kind.length > 0 ||
+    props.filters.flag?.length > 0
+  )
 })
 
 // Emit filters when they change
 function emitFilters() {
   emit('filtersChanged', {
     id: localFilters.value.id,
     location_kind: localFilters.value.location_kind,
+    flag: localFilters.value.flag,
   })
 }