diff --git a/backend/maint-scripts/update_title_flavours_from_books.py b/backend/maint-scripts/update_title_flavours_from_books.py new file mode 100755 index 0000000..2e8afa9 --- /dev/null +++ b/backend/maint-scripts/update_title_flavours_from_books.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python3 +# ruff: noqa: T201 +"""This script sets the title flavours from the set of all books belonging to the title. + +It also fixes books whose flavours start with an underscore. +""" + +from typing import Any + +from sqlalchemy import select +from sqlalchemy.orm import Session as OrmSession + +from cms_backend import logger +from cms_backend.db import Session +from cms_backend.db.models import Book, Title +from cms_backend.db.title import get_title_by_id + + +def update_title_flavour(session: OrmSession, title: Title) -> tuple[bool, str]: + if title.archived: + logger.info(f"Skipping archived title {title.id} ({title.name})") + return (False, "Title is archived") + + books = session.scalars( + select(Book) + .join(Title, Book.title_id == Title.id) + .distinct() + .order_by(Book.flavour) + .where(Book.flavour.isnot(None), Book.title_id == title.id) + ).all() + for book in books: + if book.flavour and book.flavour.startswith("_"): + new_flavour = book.flavour[1:] + logger.info( + f"Updated book {book.name} from {book.flavour} to {new_flavour}" + ) + book.flavour = new_flavour + session.add(book) + + flavours = [book.flavour for book in books if book.flavour is not None] + title.flavours = [] + + if not flavours: + logger.info( + f"No flavours found in books belonging to title {title.id} ({title.name})" + ) + return ( + False, + f"No flavours found in books belonging to title {title.id} ({title.name})", + ) + + logger.info(f"✓ Updated title {title.id} ({title.name}) flavours to {flavours}") + return (True, "") + + +def main(): + + with Session.begin() as session: + title_ids = session.scalars(select(Title.id)).all() + logger.info(f"Found {len(title_ids)} titles to process") + nb_titles_updated = 0 + nb_titles_skipped = 0 + reasons: list[dict[str, Any]] = [] + + for title_id in title_ids: + title = get_title_by_id(session, title_id=title_id) + processed, reason = update_title_flavour(session, title) + if processed: + nb_titles_updated += 1 + else: + nb_titles_skipped += 1 + reasons.append({title.name: reason}) + + logger.info( + f"Updated {nb_titles_updated} title(s) metadata, skipped " + f"{nb_titles_skipped} titles(s)" + ) + + if reasons: + print("\nSkipped titles summary:") + print("| Title Name | Reason |") + print("|------------|--------|") + for entry in reasons: + for title_name, reason in entry.items(): + print(f"| {title_name} | {reason} |") + + +if __name__ == "__main__": + main() diff --git a/backend/src/cms_backend/api/main.py b/backend/src/cms_backend/api/main.py index 6ee91c1..48c5983 100644 --- a/backend/src/cms_backend/api/main.py +++ b/backend/src/cms_backend/api/main.py @@ -105,6 +105,14 @@ async def request_validation_error_handler(_, exc: RequestValidationError): ) +@app.exception_handler(ValueError) +async def value_error_handler(_, exc: ValueError): + return JSONResponse( + status_code=HTTPStatus.BAD_REQUEST, + content={"success": False, "message": exc.args[0]}, + ) + + @app.exception_handler(ValidationError) async def validation_error_handler(_, exc: ValidationError): # transform the pydantic validation errors to a dictionary mapping diff --git a/backend/src/cms_backend/api/routes/titles.py b/backend/src/cms_backend/api/routes/titles.py index 4ce18f4..8761918 100644 --- a/backend/src/cms_backend/api/routes/titles.py +++ b/backend/src/cms_backend/api/routes/titles.py @@ -66,6 +66,7 @@ class BaseTitleCreateUpdateSchema(BaseModel): publisher: NotEmptyString | None = None language: NotEmptyString | None = None illustration_48x48_at_1: Base64Str | None = None + flavours: list[str] | None = None @model_validator(mode="after") def validate_unique_collection_titles(self) -> Self: @@ -158,6 +159,7 @@ def create_title( source=title_data.source, long_description=title_data.long_description, description=title_data.description, + flavours=title_data.flavours, ) return create_title_light_schema(title) @@ -188,6 +190,7 @@ def update_title( license_=title_data.license, relation=title_data.relation, source=title_data.source, + flavours=title_data.flavours, ) return create_title_light_schema(title) diff --git a/backend/src/cms_backend/db/book.py b/backend/src/cms_backend/db/book.py index 4d169be..e0f6a16 100644 --- a/backend/src/cms_backend/db/book.py +++ b/backend/src/cms_backend/db/book.py @@ -100,6 +100,9 @@ def create_book_full_schema(book: Book) -> BookFullSchema: current_locations=current_locations, target_locations=target_locations, title_archived=book.title.archived if book.title else False, + has_flavour_mismatch=book.flavour not in book.title.flavours + if book.title + else False, ) @@ -251,6 +254,12 @@ def move_book( if not book.title: raise ValueError(f"Book {book_id} has no associated title.") + if destination == "prod" and book.flavour not in book.title.flavours: + raise ValueError( + f"Book flavour '{book.flavour}' is not in title expected flavours " + f"{book.title.flavours}" + ) + existing_filename = current_location.filename goes_to_staging = destination == "staging" diff --git a/backend/src/cms_backend/db/books.py b/backend/src/cms_backend/db/books.py index c2e766e..0e21e9b 100644 --- a/backend/src/cms_backend/db/books.py +++ b/backend/src/cms_backend/db/books.py @@ -46,13 +46,8 @@ def get_books( Book.date, Book.flavour, Book.issues, - ).order_by( - Book.has_error.desc(), - Book.location_kind, - Book.needs_file_operation.desc(), - Book.created_at.desc(), - Book.id, - ) + Title.flavours, + ).join(Title, Book.title_id == Title.id, isouter=True) if book_id is not None: stmt = stmt.where(Book.id.cast(String).ilike(f"%{book_id}%")) @@ -124,6 +119,9 @@ def get_books( date=date, flavour=flavour, issues=book_issues, + has_flavour_mismatch=flavour not in title_flavours + if title_flavours is not None + else False, ) for ( book_id_result, @@ -138,6 +136,7 @@ def get_books( date, flavour, book_issues, + title_flavours, ) in session.execute( stmt.offset(skip) .limit(limit) @@ -146,6 +145,7 @@ def get_books( Book.location_kind, Book.needs_file_operation, Book.created_at.desc(), + Book.id, ) ).all() ], @@ -327,7 +327,7 @@ def get_book_languages(session: OrmSession) -> BookLanguagesSchema: def get_book_flavours(session: OrmSession) -> ListResult[str]: - """Get a list of book falavours""" + """Get a list of book flavours""" stmt = ( select(Book.flavour) .distinct() diff --git a/backend/src/cms_backend/db/models.py b/backend/src/cms_backend/db/models.py index 2f24f85..f3c1f21 100644 --- a/backend/src/cms_backend/db/models.py +++ b/backend/src/cms_backend/db/models.py @@ -213,6 +213,9 @@ class Title(Base): maturity: Mapped[str] = mapped_column(init=False, index=True, default="unstable") events: Mapped[list[str]] = mapped_column(init=False, default_factory=list) archived: Mapped[bool] = mapped_column(default=False, server_default=false()) + flavours: Mapped[list[str]] = mapped_column( + default_factory=list, server_default="{}" + ) books: Mapped[list["Book"]] = relationship( back_populates="title", diff --git a/backend/src/cms_backend/db/title.py b/backend/src/cms_backend/db/title.py index c9cf060..4161800 100644 --- a/backend/src/cms_backend/db/title.py +++ b/backend/src/cms_backend/db/title.py @@ -50,6 +50,7 @@ def create_title_full_schema(title: Title) -> TitleFullSchema: license=title.license, relation=title.relation, source=title.source, + flavours=title.flavours, books=[ BookLightSchema( id=book.id, @@ -64,6 +65,7 @@ def create_title_full_schema(title: Title) -> TitleFullSchema: date=book.date, flavour=book.flavour, issues=book.issues, + has_flavour_mismatch=book.flavour not in title.flavours, ) for book in sorted( title.books, @@ -107,6 +109,7 @@ def create_title_light_schema(title: Title) -> TitleLightSchema: license=title.license, relation=title.relation, source=title.source, + flavours=title.flavours, ) @@ -183,6 +186,7 @@ def get_titles( Title.license.label("title_license"), Title.relation.label("title_relation"), Title.source.label("title_source"), + Title.flavours.label("title_flavours"), ) .join(CollectionTitle, CollectionTitle.title_id == Title.id, isouter=True) .join(Collection, CollectionTitle.collection_id == Collection.id, isouter=True) @@ -226,6 +230,7 @@ def get_titles( license=title_license, relation=title_relation, source=title_source, + flavours=title_flavours, ) for ( title_id, @@ -242,6 +247,7 @@ def get_titles( title_license, title_relation, title_source, + title_flavours, ) in session.execute(stmt.offset(skip).limit(limit)).all() ], ) @@ -263,6 +269,7 @@ def create_title( license_: str | None = None, relation: str | None = None, source: str | None = None, + flavours: list[str] | None = None, ) -> Title: """Create a new title""" @@ -281,6 +288,7 @@ def create_title( title.source = source title.description = description title.long_description = long_description + title.flavours = [] if flavours is None else flavours title.events.append(f"{getnow()}: title created") session.add(title) @@ -332,6 +340,7 @@ def update_title( license_: str | None = None, relation: str | None = None, source: str | None = None, + flavours: list[str] | None = None, ) -> Title: """Update a title's details @@ -428,6 +437,9 @@ def update_title( title.source = source title.events.append(f"{getnow()}: source updated from {old_source} to {source}") + if flavours is not None: + title.flavours = flavours + name_changed: bool = False # Update name if provided if name and name != title.name: diff --git a/backend/src/cms_backend/migrations/versions/9dc25bcae26f_add_flavours_to_title.py b/backend/src/cms_backend/migrations/versions/9dc25bcae26f_add_flavours_to_title.py new file mode 100644 index 0000000..9c1f946 --- /dev/null +++ b/backend/src/cms_backend/migrations/versions/9dc25bcae26f_add_flavours_to_title.py @@ -0,0 +1,37 @@ +"""add flavours to title + +Revision ID: 9dc25bcae26f +Revises: a8f64135b053 +Create Date: 2026-05-26 12:24:41.086893 + +""" + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "9dc25bcae26f" +down_revision = "e70e0c595eb9" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column( + "title", + sa.Column( + "flavours", + postgresql.ARRAY(sa.String()), + server_default="{}", + nullable=False, + ), + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("title", "flavours") + # ### end Alembic commands ### diff --git a/backend/src/cms_backend/mill/processors/title.py b/backend/src/cms_backend/mill/processors/title.py index f207e0d..3a91782 100644 --- a/backend/src/cms_backend/mill/processors/title.py +++ b/backend/src/cms_backend/mill/processors/title.py @@ -55,21 +55,29 @@ def add_book_to_title(session: OrmSession, book: Book, title: Title): title.relation = book.zim_metadata.get("Relation") title.source = book.zim_metadata.get("Source") + issues: list[str] = [] + different_metadata_keys = get_differing_metadata_keys(book) if different_metadata_keys: - book.issues = ["metadata mismatch"] + issues.append("metadata mismatch") book.events.append( f"{getnow()}: book metadata is different from title metadata: " f"{','.join(different_metadata_keys)}" ) + if title.flavours and book.flavour not in title.flavours: + issues.append("flavour mismatch") + book.events.append( + f"{getnow()}: book flavour is not in list of title flavours" + ) + + book.issues = issues + # Determine if this book goes to staging or prod based on # - title maturity: For now, only 'stable' maturity move straight to prod, # other maturity moves through staging first - # - if book has different metadata from title - goes_to_staging = ( - title.maturity != "stable" or len(different_metadata_keys) != 0 - ) + # - issues: books with any issues move to staging regardless of maturity + goes_to_staging = title.maturity != "stable" or len(issues) != 0 target_locations = ( [ diff --git a/backend/src/cms_backend/schemas/orms.py b/backend/src/cms_backend/schemas/orms.py index 5b45a4e..b41577f 100644 --- a/backend/src/cms_backend/schemas/orms.py +++ b/backend/src/cms_backend/schemas/orms.py @@ -33,6 +33,7 @@ class TitleLightSchema(BaseModel): license: str | None relation: str | None source: str | None + flavours: list[str] class BaseTitleCollectionSchema(BaseModel): @@ -116,6 +117,7 @@ class BookLightSchema(BaseModel): date: str | None flavour: str | None issues: list[str] + has_flavour_mismatch: bool class BookFullSchema(BookLightSchema): diff --git a/backend/tests/api/routes/test_titles.py b/backend/tests/api/routes/test_titles.py index ce34f6b..2f6a148 100644 --- a/backend/tests/api/routes/test_titles.py +++ b/backend/tests/api/routes/test_titles.py @@ -58,6 +58,7 @@ def test_get_titles( "relation", "source", "license", + "flavours", } assert data["items"][0]["name"] == "wikipedia_fr_all" @@ -326,6 +327,7 @@ def test_get_title_by_id( "relation", "source", "license", + "flavours", } # Verify field values @@ -379,6 +381,7 @@ def test_get_title_by_id_with_books( "flavour", "deletion_date", "issues", + "has_flavour_mismatch", } assert data["books"][0]["title_id"] == str(title.id) assert data["books"][1]["title_id"] == str(title.id) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 094704b..e706069 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -167,6 +167,7 @@ def _create_title( license: str | None = None, # noqa: A002 relation: str | None = None, source: str | None = None, + flavours: list[str] | None = None, ) -> Title: db_title = Title( name=name, @@ -181,6 +182,7 @@ def _create_title( license=license, relation=relation, source=source, + flavours=flavours if flavours is not None else [], ) dbsession.add(db_title) dbsession.flush() diff --git a/backend/tests/db/test_books.py b/backend/tests/db/test_books.py index ba2dc52..570765d 100644 --- a/backend/tests/db/test_books.py +++ b/backend/tests/db/test_books.py @@ -633,11 +633,11 @@ def test_move_book_staging_to_prod( monkeypatch: pytest.MonkeyPatch, ): """Test moving a book from staging to prod""" - title = create_title(name="test_en_all") + title = create_title(name="test_en_all", flavours=["mini", "maxi"]) collection = create_collection(warehouse=warehouse) create_collection_title(title=title, collection=collection, path=Path("zim")) - book = create_book(name="test_en_all", date="2024-01") + book = create_book(name="test_en_all", date="2024-01", flavour="maxi") book.title = title book.location_kind = "staging" create_book_location( @@ -666,6 +666,39 @@ def test_move_book_staging_to_prod( assert target_locations[0].filename == "test_en_all_2024-01.zim" +def test_move_book_with_different_flavor_from_title_to_prod( + dbsession: OrmSession, + warehouse: Warehouse, + create_book: Callable[..., Book], + create_title: Callable[..., Title], + create_collection: Callable[..., Collection], + create_collection_title: Callable[..., CollectionTitle], + create_book_location: Callable[..., BookLocation], + monkeypatch: pytest.MonkeyPatch, +): + """Test moving a book with different flavor from it's title from staging to prod""" + title = create_title(name="test_en_all", flavours=["mini", "maxi"]) + collection = create_collection(warehouse=warehouse) + create_collection_title(title=title, collection=collection, path=Path("zim")) + + book = create_book(name="test_en_all", date="2024-01", flavour="nopic") + book.title = title + book.location_kind = "staging" + create_book_location( + book=book, + warehouse_id=Context.staging_warehouse_id, + path=Context.staging_base_path, + filename="test_en_all_2024-01.zim", + status="current", + ) + dbsession.flush() + + now = getnow() + monkeypatch.setattr("cms_backend.db.book.getnow", lambda: now) + with pytest.raises(ValueError): + move_book(dbsession, book_id=book.id, destination="prod") + + def test_move_book_same_destination_raises_error( dbsession: OrmSession, create_book: Callable[..., Book], diff --git a/backend/tests/mill/processors/test_zimfarm_notification.py b/backend/tests/mill/processors/test_zimfarm_notification.py index ed68402..da3dcb0 100644 --- a/backend/tests/mill/processors/test_zimfarm_notification.py +++ b/backend/tests/mill/processors/test_zimfarm_notification.py @@ -582,6 +582,54 @@ def test_moves_book_to_staging_due_to_diffrent_metadata_from_title( assert book.needs_file_operation is True assert book.needs_processing is False + def test_moves_book_to_staging_due_to_diffrent_flavour_from_title( + self, + dbsession: OrmSession, + warehouse: Warehouse, # noqa: ARG002 + create_zimfarm_notification: Callable[..., ZimfarmNotification], + create_title: Callable[..., Title], + create_collection: Callable[..., Collection], + create_warehouse: Callable[..., Warehouse], + ): + """ + Test that book goes to staging because there is a flavour mismatch between + it and it's title + """ + + title = create_title(name="test_en_all", flavours=["maxi", "mini"]) + title.maturity = "stable" + + prod = create_warehouse( + name="prod", warehouse_id=UUID("00000000-0000-0000-0000-000000000003") + ) + collection = create_collection(warehouse=prod) + + ct = CollectionTitle(path=Path("wikipedia")) + ct.title = title + ct.collection = collection + dbsession.add(ct) + dbsession.flush() + + content = VALID_NOTIFICATION_CONTENT.copy() + content["folder_name"] = "" + + notification = create_zimfarm_notification(content=content) + dbsession.flush() + + process_notification(dbsession, notification) + + assert notification.status == "processed" + + book = dbsession.query(Book).filter_by(id=notification.id).first() + assert book is not None + assert book.title_id == title.id + assert book.location_kind == "staging" + assert len(book.issues) == 1 + assert set(book.issues) == {"flavour mismatch"} + assert book.has_error is False + assert book.needs_file_operation is True + assert book.needs_processing is False + class TestValidNotificationOnArchivedTitle: """Test valid notifications that are associated to an archived title.""" diff --git a/frontend/src/components/BookTable.vue b/frontend/src/components/BookTable.vue index 7539e93..ec0b373 100644 --- a/frontend/src/components/BookTable.vue +++ b/frontend/src/components/BookTable.vue @@ -41,8 +41,18 @@ - {{ item.flavour }} - - + + {{ item.flavour }} + + + + + mdi-alert + + + Book flavour does not match title flavour + + diff --git a/frontend/src/components/EditBookForm.vue b/frontend/src/components/EditBookForm.vue index 8af6eed..4ce5427 100644 --- a/frontend/src/components/EditBookForm.vue +++ b/frontend/src/components/EditBookForm.vue @@ -1,7 +1,7 @@ - + () const formRef = ref() +const formValid = ref(false) const localBook = ref({ flavour: props.book?.flavour || '', @@ -65,7 +66,7 @@ const rules = { } const isDisabled = computed(() => { - return props.loading || !hasChanges.value + return props.loading || !hasChanges.value || !formValid.value }) const formattedFlavourOptions = computed(() => { @@ -87,6 +88,8 @@ watch( localBook.value = { flavour: newBook.flavour || '', } + // Reset form validation when book changes + formRef.value?.resetValidation() } }, { deep: true, immediate: true }, diff --git a/frontend/src/components/TitleForm.vue b/frontend/src/components/TitleForm.vue index 68e5c79..7b5ad17 100644 --- a/frontend/src/components/TitleForm.vue +++ b/frontend/src/components/TitleForm.vue @@ -29,6 +29,24 @@ + + + + + @@ -342,6 +360,7 @@