From 04cd6dafb095978f4d67b4884110b173f10af7e3 Mon Sep 17 00:00:00 2001 From: Francisco Penedo Alvarez Date: Fri, 20 Mar 2026 21:35:53 +0100 Subject: [PATCH] Add server-side validation and small improvements --- pyproject.toml | 1 + src/hxbooks/library.py | 75 ++++- src/hxbooks/main.py | 294 +++++++++++++----- src/hxbooks/static/style.css | 17 +- src/hxbooks/templates/base.html.j2 | 12 +- src/hxbooks/templates/book/list.html.j2 | 2 +- .../templates/components/book_form.html.j2 | 25 +- .../components/reading_status.html.j2 | 152 ++++++--- uv.lock | 15 + 9 files changed, 436 insertions(+), 157 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6efd7df..03e4a70 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,6 +14,7 @@ dependencies = [ "gunicorn>=25.1.0", "jinja2-fragments>=1.11.0", "pydantic>=2.12.5", + "pydantic-extra-types>=2.11.1", "pyparsing>=3.3.2", "requests>=2.32.5", "sqlalchemy>=2.0.48", diff --git a/src/hxbooks/library.py b/src/hxbooks/library.py index 4e16f7b..6ab12eb 100644 --- a/src/hxbooks/library.py +++ b/src/hxbooks/library.py @@ -35,6 +35,8 @@ def create_book( location_shelf: int | None = None, first_published: int | None = None, bought_date: date | None = None, + loaned_to: str | None = None, + loaned_date: date | None = None, ) -> Book: """Create a new book with the given details.""" book = Book( @@ -50,6 +52,8 @@ def create_book( location_shelf=location_shelf, first_published=first_published, bought_date=bought_date, + loaned_to=loaned_to or "", + loaned_date=loaned_date, ) db.session.add(book) @@ -90,6 +94,8 @@ def get_book(book_id: int) -> Book | None: def update_book( book_id: int, + set_all_fields: bool = False, # If True, all fields must be provided and will be set (even if None) + owner_id: int | None = None, title: str | None = None, authors: list[str] | None = None, genres: list[str] | None = None, @@ -103,6 +109,8 @@ def update_book( location_shelf: int | None = None, first_published: int | None = None, bought_date: date | None = None, + loaned_to: str | None = None, + loaned_date: date | None = None, ) -> Book | None: """Update a book with new details.""" book = get_book(book_id) @@ -110,43 +118,73 @@ def update_book( return None # Update scalar fields - if title is not None: + if title is not None or set_all_fields: + assert title is not None, "Title is required when set_all_fields is True" book.title = title - if isbn is not None: + if isbn is not None or set_all_fields: + assert isbn is not None, "ISBN is required when set_all_fields is True" book.isbn = isbn - if publisher is not None: + if publisher is not None or set_all_fields: + assert publisher is not None, ( + "Publisher is required when set_all_fields is True" + ) book.publisher = publisher - if edition is not None: + if edition is not None or set_all_fields: + assert edition is not None, "Edition is required when set_all_fields is True" book.edition = edition - if description is not None: + if description is not None or set_all_fields: + assert description is not None, ( + "Description is required when set_all_fields is True" + ) book.description = description - if notes is not None: + if notes is not None or set_all_fields: + assert notes is not None, "Notes is required when set_all_fields is True" book.notes = notes - if location_place is not None: + if location_place is not None or set_all_fields: + assert location_place is not None, ( + "Location place is required when set_all_fields is True" + ) book.location_place = location_place - if location_bookshelf is not None: + if location_bookshelf is not None or set_all_fields: + assert location_bookshelf is not None, ( + "Location bookshelf is required when set_all_fields is True" + ) book.location_bookshelf = location_bookshelf - if location_shelf is not None: + if location_shelf is not None or set_all_fields: book.location_shelf = location_shelf - if first_published is not None: + if first_published is not None or set_all_fields: book.first_published = first_published - if bought_date is not None: + if bought_date is not None or set_all_fields: book.bought_date = bought_date + if loaned_to is not None or set_all_fields: + assert loaned_to is not None, ( + "Loaned to is required when set_all_fields is True" + ) + book.loaned_to = loaned_to + if loaned_date is not None or set_all_fields: + book.loaned_date = loaned_date # Update authors - if authors is not None: + if authors is not None or set_all_fields: + assert authors is not None, ( + "Authors list is required when set_all_fields is True" + ) book.authors.clear() for author_name in [a_strip for a in authors if (a_strip := a.strip())]: author = _get_or_create_author(author_name) book.authors.append(author) # Update genres - if genres is not None: + if genres is not None or set_all_fields: + assert genres is not None, "Genres list is required when set_all_fields is True" book.genres.clear() for genre_name in [g_strip for g in genres if (g_strip := g.strip())]: genre = _get_or_create_genre(genre_name) book.genres.append(genre) + if owner_id is not None or set_all_fields: + book.owner_id = owner_id + db.session.commit() return book @@ -632,6 +670,17 @@ def get_reading_history(user_id: int, limit: int = 50) -> Sequence[Reading]: ) +def delete_reading(reading_id: int) -> bool: + """Delete a reading session.""" + reading = db.session.get(Reading, reading_id) + if not reading: + return False + + db.session.delete(reading) + db.session.commit() + return True + + def add_to_wishlist(book_id: int, user_id: int) -> Wishlist: """Add a book to user's wishlist.""" # Check if book exists diff --git a/src/hxbooks/main.py b/src/hxbooks/main.py index 76f4b21..d50dcb9 100644 --- a/src/hxbooks/main.py +++ b/src/hxbooks/main.py @@ -4,7 +4,8 @@ Main application routes for HXBooks frontend. Provides clean URL structure and integrates with library.py business logic. """ -from typing import Any +from datetime import date +from typing import Annotated, Any from flask import ( Blueprint, @@ -17,8 +18,16 @@ from flask import ( url_for, ) from flask.typing import ResponseReturnValue +from pydantic import ( + BaseModel, + BeforeValidator, + Field, + StringConstraints, + ValidationError, +) +from pydantic_extra_types.isbn import ISBN -from hxbooks.models import User +from hxbooks.models import Reading, User from . import library from .db import db @@ -26,25 +35,56 @@ from .db import db bp = Blueprint("main", __name__) -def save_search(user: User, search_name: str, query_params: str) -> bool: - """Save a search for a user. Mock implementation.""" - # Initialize saved_searches if None - - user.saved_searches = user.saved_searches | {search_name: query_params} # noqa: PLR6104 - print(f"{user.saved_searches=}") - db.session.commit() - return True +# Pydantic validation models +StrOrNone = Annotated[str | None, BeforeValidator(lambda v: v.strip() or None)] +StripStr = Annotated[str, StringConstraints(strip_whitespace=True)] +ISBNOrNone = Annotated[ISBN | None, BeforeValidator(lambda v: v.strip() or None)] +TextareaList = Annotated[ + list[str], + BeforeValidator( + lambda v: ( + [line.strip() for line in v.split("\n") if line.strip()] + if isinstance(v, str) + else v or [] + ) + ), +] +DateOrNone = Annotated[date | None, BeforeValidator(lambda v: v.strip() or None)] +IntOrNone = Annotated[int | None, BeforeValidator(lambda v: v.strip() or None)] -def delete_saved_search(user: User, search_name: str) -> bool: - """Delete a saved search for a user. Mock implementation.""" - if search_name in user.saved_searches: - user.saved_searches = { - k: v for k, v in user.saved_searches.items() if k != search_name - } # needs to be a new object to trigger SQLAlchemy change detection - db.session.commit() - return True - return False +class BookFormData(BaseModel): + title: StripStr = Field(min_length=1) + owner: StrOrNone = None + isbn: ISBNOrNone = None + authors: TextareaList = Field(default_factory=list) + genres: TextareaList = Field(default_factory=list) + first_published: IntOrNone = Field(default=None, le=2030) + publisher: StripStr = Field(default="") + edition: StripStr = Field(default="") + description: StripStr = Field(default="") + notes: StripStr = Field(default="") + location_place: StripStr = Field(default="") + location_bookshelf: StripStr = Field(default="") + location_shelf: IntOrNone = Field(default=None, ge=1) + loaned_to: StripStr = Field(default="") + loaned_date: DateOrNone = None + + +class ReadingFormData(BaseModel): + start_date: date + end_date: DateOrNone = None + dropped: bool = Field(default=False) + rating: IntOrNone = Field(default=None, ge=1, le=5) + comments: StripStr = Field(default="") + + +def _flash_validation_errors(e: ValidationError) -> None: + """Helper to flash validation errors.""" + error = e.errors()[0] + loc = " -> ".join(str(v) for v in error.get("loc", [])) + msg = error.get("msg", "Invalid input") + flash(f"Validation error in '{loc}': {msg}", "error") @bp.before_app_request @@ -101,61 +141,60 @@ def book_detail(book_id: int) -> ResponseReturnValue: flash("Book not found", "error") return redirect(url_for("main.index")) - for reading in book.readings: - print( - f"Reading: {reading}, user: {reading.user.username if reading.user else 'N/A'}, dropped: {reading.dropped}, finished: {reading.finished}, end_date: {reading.end_date}" - ) - return render_template("book/detail.html.j2", book=book) +def _get_or_create_user(username: str) -> int: + """Helper to get or create a user by username.""" + owner = library.get_user_by_username(username) + if not owner: + # Create new user if username doesn't exist + owner = library.create_user(username) + + return owner.id + + @bp.route("/book/new", methods=["GET", "POST"]) def create_book() -> ResponseReturnValue: """Create a new book.""" if request.method == "POST": - title = request.form.get("title", "").strip() - if not title: - flash("Title is required", "error") - return render_template("book/create.html.j2") - try: - # Get current viewing user as owner - viewing_user = g.get("viewing_user") + # Validate form data with Pydantic + form_data = BookFormData.model_validate(dict(request.form)) - # Process textarea inputs for authors and genres - authors = [ - author.strip() - for author in request.form.get("authors", "").split("\n") - if author.strip() - ] - genres = [ - genre.strip() - for genre in request.form.get("genres", "").split("\n") - if genre.strip() - ] + # Get owner ID if provided + owner_id = _get_or_create_user(form_data.owner) if form_data.owner else None - # Create book with submitted data + # Create book with validated data book = library.create_book( - title=title, - owner_id=viewing_user.id if viewing_user else None, - authors=authors, - genres=genres, - isbn=request.form.get("isbn"), - publisher=request.form.get("publisher"), - edition=request.form.get("edition"), - description=request.form.get("description"), - notes=request.form.get("notes"), - location_place=request.form.get("location_place"), - location_bookshelf=request.form.get("location_bookshelf"), - location_shelf=int(request.form.get("location_shelf") or 0) or None, - first_published=int(request.form.get("first_published") or 0) or None, + title=form_data.title, + owner_id=owner_id, + authors=form_data.authors, + genres=form_data.genres, + isbn=str(form_data.isbn) if form_data.isbn else None, + publisher=form_data.publisher, + edition=form_data.edition, + description=form_data.description, + notes=form_data.notes, + location_place=form_data.location_place, + location_bookshelf=form_data.location_bookshelf, + location_shelf=form_data.location_shelf, + first_published=form_data.first_published, + loaned_to=form_data.loaned_to, + loaned_date=form_data.loaned_date, ) - flash(f"Book '{title}' created successfully!", "success") + flash(f"Book '{form_data.title}' created successfully!", "success") return redirect(url_for("main.book_detail", book_id=book.id)) + except ValidationError as e: + _flash_validation_errors(e) + + return render_template("book/create.html.j2", form_data=request.form) + except Exception as e: flash(f"Error creating book: {e}", "error") + return render_template("book/create.html.j2", form_data=request.form) return render_template("book/create.html.j2") @@ -169,37 +208,39 @@ def update_book(book_id: int) -> ResponseReturnValue: return redirect(url_for("main.index")) try: - # Process textarea inputs for authors and genres - authors = [ - author.strip() - for author in request.form.get("authors", "").split("\n") - if author.strip() - ] - genres = [ - genre.strip() - for genre in request.form.get("genres", "").split("\n") - if genre.strip() - ] + # Validate form data with Pydantic + form_data = BookFormData.model_validate(dict(request.form)) - # Update book with form data + # Get owner ID if provided + owner_id = _get_or_create_user(form_data.owner) if form_data.owner else None + + # Update book with validated data library.update_book( book_id=book_id, - title=request.form.get("title"), - authors=authors, - genres=genres, - isbn=request.form.get("isbn"), - publisher=request.form.get("publisher"), - edition=request.form.get("edition"), - description=request.form.get("description"), - notes=request.form.get("notes"), - location_place=request.form.get("location_place"), - location_bookshelf=request.form.get("location_bookshelf"), - location_shelf=int(request.form.get("location_shelf") or 0) or None, - first_published=int(request.form.get("first_published") or 0) or None, + set_all_fields=True, # Ensure all fields are updated, even if None/empty + title=form_data.title, + owner_id=owner_id, + authors=form_data.authors, + genres=form_data.genres, + isbn=str(form_data.isbn) if form_data.isbn else None, + publisher=form_data.publisher, + edition=form_data.edition, + description=form_data.description, + notes=form_data.notes, + location_place=form_data.location_place, + location_bookshelf=form_data.location_bookshelf, + location_shelf=form_data.location_shelf, + first_published=form_data.first_published, + loaned_to=form_data.loaned_to, + loaned_date=form_data.loaned_date, ) flash("Book updated successfully!", "success") + except ValidationError as e: + # Format validation errors for display + _flash_validation_errors(e) + except Exception as e: flash(f"Error updating book: {e}", "error") @@ -273,6 +314,26 @@ def set_viewing_user(username: str = "") -> ResponseReturnValue: return redirect(request.referrer or url_for("main.index")) +def _save_search(user: User, search_name: str, query_params: str) -> bool: + """Save a search for a user. Mock implementation.""" + # Initialize saved_searches if None + + user.saved_searches = user.saved_searches | {search_name: query_params} # noqa: PLR6104 + db.session.commit() + return True + + +def _delete_saved_search(user: User, search_name: str) -> bool: + """Delete a saved search for a user. Mock implementation.""" + if search_name in user.saved_searches: + user.saved_searches = { + k: v for k, v in user.saved_searches.items() if k != search_name + } # needs to be a new object to trigger SQLAlchemy change detection + db.session.commit() + return True + return False + + @bp.route("/saved-search", methods=["POST"]) def save_search_route() -> ResponseReturnValue: """Save a search for the current user.""" @@ -283,16 +344,13 @@ def save_search_route() -> ResponseReturnValue: search_name = request.form.get("name", "").strip() query_params = request.form.get("query_params", "") - print( - f"Saving search for user {viewing_user.username}: {search_name} -> {query_params}" - ) if not search_name: flash("Search name is required", "error") return redirect(url_for("main.index", q=query_params)) try: - success = save_search(viewing_user, search_name, query_params) + success = _save_search(viewing_user, search_name, query_params) if success: flash(f"Search '{search_name}' saved successfully!", "success") else: @@ -411,6 +469,70 @@ def remove_from_wishlist_route(book_id: int) -> ResponseReturnValue: return redirect(url_for("main.book_detail", book_id=book_id)) +@bp.route("/book//reading//update", methods=["POST"]) +def update_reading_route(book_id: int, reading_id: int) -> ResponseReturnValue: + """Update a single reading session.""" + viewing_user = g.get("viewing_user") + if not viewing_user: + flash("You must select a user to update readings", "error") + return redirect(url_for("main.book_detail", book_id=book_id)) + + try: + # Get and verify the reading belongs to current user + reading = db.session.get(Reading, reading_id) + if not reading or reading.user_id != viewing_user.id: + flash("Reading not found or not yours to modify", "error") + return redirect(url_for("main.book_detail", book_id=book_id)) + + # Validate the form data + form_data = ReadingFormData.model_validate(dict(request.form)) + + # Update the reading with validated data + reading.start_date = form_data.start_date + reading.end_date = form_data.end_date + reading.dropped = form_data.dropped + reading.rating = form_data.rating + reading.comments = form_data.comments + + db.session.commit() + flash("Reading updated successfully!", "success") + + except ValidationError as e: + db.session.rollback() # Rollback any partial changes on validation error + _flash_validation_errors(e) + + except Exception as e: + db.session.rollback() # Rollback any partial changes on general error + flash(f"Error updating reading: {e}", "error") + + return redirect(url_for("main.book_detail", book_id=book_id)) + + +@bp.route("/book//reading//delete", methods=["POST"]) +def delete_reading_route(book_id: int, reading_id: int) -> ResponseReturnValue: + """Delete a reading session.""" + viewing_user = g.get("viewing_user") + if not viewing_user: + flash("You must select a user to delete readings", "error") + return redirect(url_for("main.book_detail", book_id=book_id)) + + try: + # Verify the reading belongs to the current user + reading = db.session.get(Reading, reading_id) + if reading and reading.user_id == viewing_user.id: + deleted = library.delete_reading(reading_id) + if deleted: + flash("Reading session deleted", "info") + else: + flash("Reading session not found", "warning") + else: + flash("Reading session not found or not yours to delete", "error") + except Exception as e: + flash(f"Error deleting reading: {e}", "error") + + return redirect(url_for("main.book_detail", book_id=book_id)) + + @bp.route("/saved-search//delete", methods=["GET", "POST"]) def delete_saved_search_route(search_name: str) -> ResponseReturnValue: """Delete a saved search (GET shows confirmation, POST performs deletion).""" @@ -428,7 +550,7 @@ def delete_saved_search_route(search_name: str) -> ResponseReturnValue: if request.method == "POST": # Perform the actual deletion try: - success = delete_saved_search(viewing_user, search_name) + success = _delete_saved_search(viewing_user, search_name) if success: flash(f"Saved search '{search_name}' deleted successfully!", "success") else: diff --git a/src/hxbooks/static/style.css b/src/hxbooks/static/style.css index c325fe5..36667c7 100644 --- a/src/hxbooks/static/style.css +++ b/src/hxbooks/static/style.css @@ -53,7 +53,7 @@ .book-card:hover { transform: translateY(-2px); - box-shadow: 0 4px 8px rgba(0,0,0,0.1); + box-shadow: 0 4px 8px rgba(0, 0, 0, 0.1); } .book-card .card-img-top { @@ -69,24 +69,19 @@ } .book-card-footer { - background-color: rgba(0,0,0,0.05); + background-color: rgba(0, 0, 0, 0.05); font-size: 0.875rem; } -/* Search Bar */ -.search-container { - max-width: 600px; -} - /* Form Styles */ -.form-floating > .form-control:focus, -.form-floating > .form-control:not(:placeholder-shown) { +.form-floating>.form-control:focus, +.form-floating>.form-control:not(:placeholder-shown) { padding-top: 1.625rem; padding-bottom: .625rem; } -.form-floating > .form-control:focus ~ label, -.form-floating > .form-control:not(:placeholder-shown) ~ label { +.form-floating>.form-control:focus~label, +.form-floating>.form-control:not(:placeholder-shown)~label { opacity: .65; transform: scale(.85) translateY(-.5rem) translateX(.15rem); } diff --git a/src/hxbooks/templates/base.html.j2 b/src/hxbooks/templates/base.html.j2 index 6ebf75d..510744a 100644 --- a/src/hxbooks/templates/base.html.j2 +++ b/src/hxbooks/templates/base.html.j2 @@ -7,15 +7,19 @@ {% block title %}{% endblock %} - HXBooks - + {# + #} - - - + {# + #} + {# + #} + {# + #}