From b5007fbb34a0e344dd35ad9c00d848e536496493 Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 00:16:17 +0100 Subject: [PATCH 01/11] Global code improvement : Rename configurations folder to config --- app/__init__.py | 2 +- {configurations => config}/__init__.py | 0 {configurations => config}/configuration_variables.py | 0 database/database_setup.py | 2 +- tests/conftest.py | 2 +- 5 files changed, 3 insertions(+), 3 deletions(-) rename {configurations => config}/__init__.py (100%) rename {configurations => config}/configuration_variables.py (100%) diff --git a/app/__init__.py b/app/__init__.py index 15fcd89..edffe25 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -6,7 +6,7 @@ from app.controllers.article_controller import article_bp from app.controllers.comment_controller import comment_bp from app.controllers.login_controller import login_bp -from configurations.configuration_variables import env_vars +from config.configuration_variables import env_vars from database.database_setup import db_session diff --git a/configurations/__init__.py b/config/__init__.py similarity index 100% rename from configurations/__init__.py rename to config/__init__.py diff --git a/configurations/configuration_variables.py b/config/configuration_variables.py similarity index 100% rename from configurations/configuration_variables.py rename to config/configuration_variables.py diff --git a/database/database_setup.py b/database/database_setup.py index eb05f8f..22157a1 100644 --- a/database/database_setup.py +++ b/database/database_setup.py @@ -4,7 +4,7 @@ from sqlalchemy import create_engine from sqlalchemy.orm import declarative_base, scoped_session, sessionmaker -from configurations.configuration_variables import env_vars +from config.configuration_variables import env_vars if os.getenv("PYTEST_CURRENT_TEST") or "pytest" in sys.modules: database_url = env_vars.test_database_url diff --git a/tests/conftest.py b/tests/conftest.py index 28ccc23..744d17d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,7 @@ from app.models.account_model import Account from app.models.article_model import Article from app.models.comment_model import Comment -from configurations.configuration_variables import env_vars +from config.configuration_variables import env_vars from database.database_setup import Base, database_engine from database.database_setup import db_session as app_db_session From c3c608fca002067c6e64daf6beb082c1430d21ef Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 02:06:16 +0100 Subject: [PATCH 02/11] Global code improvement : MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit + Remove @staticmethod from services (Article, Comment, Login) to enable constructor‑based db_session injection. + Add type hints (PEP 484) and Google‑style docstrings across all service methods. + Introduce app/constants.py with a Role enum to eliminate magic strings (e.g., "admin"). + Update controllers to instantiate services with an injected database session. + Update the test_services/ suite to support the new DI pattern and maintain full test pass rate. --- app/constants.py | 11 ++ app/controllers/article_controller.py | 23 ++-- app/controllers/comment_controller.py | 9 +- app/controllers/login_controller.py | 12 +- app/services/article_service.py | 127 ++++++++++++++++---- app/services/comment_service.py | 126 +++++++++++++++---- app/services/login_service.py | 40 ++++-- tests/test_services/test_article_service.py | 25 ++-- tests/test_services/test_comment_service.py | 23 ++-- tests/test_services/test_login_service.py | 15 ++- 10 files changed, 313 insertions(+), 98 deletions(-) create mode 100644 app/constants.py diff --git a/app/constants.py b/app/constants.py new file mode 100644 index 0000000..06c73a1 --- /dev/null +++ b/app/constants.py @@ -0,0 +1,11 @@ +from enum import Enum + + +class Role(str, Enum): + """ + Enum representing user roles within the application. + Inherits from str to allow direct string comparisons and easy database serialization. + """ + + ADMIN = "admin" + USER = "user" diff --git a/app/controllers/article_controller.py b/app/controllers/article_controller.py index f102f20..4eedbb2 100644 --- a/app/controllers/article_controller.py +++ b/app/controllers/article_controller.py @@ -14,20 +14,23 @@ def list_articles(): current_page_number = 1 page_number = request.args.get("page", current_page_number, type=int) articles_per_page = 10 - articles = ArticleService.get_paginated_articles(page_number, articles_per_page) - total_articles = ArticleService.get_total_count() + article_service = ArticleService(db_session) + articles = article_service.get_paginated_articles(page_number, articles_per_page) + total_articles = article_service.get_total_count() total_pages = math.ceil(total_articles / articles_per_page) return render_template("index.html", articles=articles, page_number=page_number, total_pages=total_pages) @article_bp.route("/article/") def view_article(article_id): - article = ArticleService.get_by_id(article_id) + article_service = ArticleService(db_session) + article = article_service.get_by_id(article_id) if not article: flash("Article not found.") return redirect(url_for("article.list_articles")) - comments = CommentService.get_tree_by_article_id(article_id) + comment_service = CommentService(db_session) + comments = comment_service.get_tree_by_article_id(article_id) return render_template("article_detail.html", article=article, comments=comments) @@ -37,7 +40,8 @@ def create_article(): flash("Access restricted.") return redirect(url_for("article.list_articles")) if request.method == "POST": - ArticleService.create_article(request.form.get("title"), request.form.get("content"), session["user_id"]) + article_service = ArticleService(db_session) + article_service.create_article(request.form.get("title"), request.form.get("content"), session["user_id"]) db_session.commit() flash("Article published!") return redirect(url_for("article.list_articles")) @@ -47,7 +51,8 @@ def create_article(): @article_bp.route("/article//edit", methods=["GET", "POST"]) def edit_article(article_id): if request.method == "POST": - article = ArticleService.update_article( + article_service = ArticleService(db_session) + article = article_service.update_article( article_id, session.get("user_id"), session.get("role"), @@ -60,7 +65,8 @@ def edit_article(article_id): return redirect(url_for("article.view_article", article_id=article_id)) flash("Update failed: Unauthorized or not found.") return redirect(url_for("article.list_articles")) - article = ArticleService.get_by_id(article_id) + article_service = ArticleService(db_session) + article = article_service.get_by_id(article_id) if not article: flash("Article not found.") return redirect(url_for("article.list_articles")) @@ -69,7 +75,8 @@ def edit_article(article_id): @article_bp.route("/article//delete") def delete_article(article_id): - if ArticleService.delete_article(article_id, session.get("user_id"), session.get("role")): + article_service = ArticleService(db_session) + if article_service.delete_article(article_id, session.get("user_id"), session.get("role")): db_session.commit() flash("Article deleted.") else: diff --git a/app/controllers/comment_controller.py b/app/controllers/comment_controller.py index 1a608bf..1b5232f 100644 --- a/app/controllers/comment_controller.py +++ b/app/controllers/comment_controller.py @@ -12,7 +12,8 @@ def create_comment(article_id): if not session.get("user_id"): flash("Login required.") return redirect(url_for("login.render_login_page")) - if CommentService.create_comment(article_id, session["user_id"], request.form.get("content")): + comment_service = CommentService(db_session) + if comment_service.create_comment(article_id, session["user_id"], request.form.get("content")): db_session.commit() flash("Comment added.") else: @@ -27,7 +28,8 @@ def reply_to_comment(parent_comment_id): flash("Login required.") return redirect(url_for("login.render_login_page")) - article_id = CommentService.create_reply(parent_comment_id, session["user_id"], request.form.get("content")) + comment_service = CommentService(db_session) + article_id = comment_service.create_reply(parent_comment_id, session["user_id"], request.form.get("content")) if article_id: db_session.commit() return redirect(url_for("article.view_article", article_id=article_id)) @@ -38,7 +40,8 @@ def reply_to_comment(parent_comment_id): @comment_bp.route("/delete/") def delete_comment(comment_id): - article_id = CommentService.delete_comment(comment_id, session.get("role")) + comment_service = CommentService(db_session) + article_id = comment_service.delete_comment(comment_id, session.get("role")) if article_id: db_session.commit() flash("Comment deleted.") diff --git a/app/controllers/login_controller.py b/app/controllers/login_controller.py index 1b8e42d..ebae4e0 100644 --- a/app/controllers/login_controller.py +++ b/app/controllers/login_controller.py @@ -1,6 +1,7 @@ from flask import Blueprint, flash, redirect, render_template, request, session, url_for from app.services.login_service import LoginService +from database.database_setup import db_session login_bp = Blueprint("login", __name__) @@ -12,11 +13,12 @@ def render_login_page(): @login_bp.route("/login", methods=["POST"]) def login_authentication(): - user_data = LoginService.authenticate_user(request.form.get("username"), request.form.get("password")) - if user_data: - session["user_id"] = user_data["id"] - session["username"] = user_data["username"] - session["role"] = user_data["role"] + login_service = LoginService(db_session) + user = login_service.authenticate_user(request.form.get("username"), request.form.get("password")) + if user: + session["user_id"] = user.account_id + session["username"] = user.account_username + session["role"] = user.account_role return redirect(url_for("article.list_articles")) flash("Incorrect credentials.") return redirect(url_for("login.render_login_page")) diff --git a/app/services/article_service.py b/app/services/article_service.py index 80f882f..345ccb1 100644 --- a/app/services/article_service.py +++ b/app/services/article_service.py @@ -1,14 +1,36 @@ +from typing import List, Optional + from sqlalchemy import func, select -from sqlalchemy.orm import defer, joinedload +from sqlalchemy.orm import Session, defer, joinedload +from app.constants import Role from app.models.account_model import Account from app.models.article_model import Article -from database.database_setup import db_session class ArticleService: - @staticmethod - def get_all_ordered_by_date(): + """ + Service class responsible for business logic operations related to Articles. + Handles creating, retrieving, updating and deleting articles as well as pagination logic. + """ + + def __init__(self, session: Session): + """ + Initialize the service with a database session (Dependency Injection). + + Args: + session (Session): The SQLAlchemy database session to use for queries. + """ + self.session = session + + def get_all_ordered_by_date(self) -> List[Article]: + """ + Retrieves all articles ordered by their publication date in descending order. + Eagerly loads author information and defers the loading of the full content for performance. + + Returns: + List[Article]: A list of Article instances containing metadata. + """ query = ( select(Article) .options( @@ -17,22 +39,52 @@ def get_all_ordered_by_date(): ) .order_by(Article.article_published_at.desc()) ) - return db_session.execute(query).unique().scalars().all() + return list(self.session.execute(query).unique().scalars().all()) - @staticmethod - def get_by_id(article_id): + def get_by_id(self, article_id: int) -> Optional[Article]: + """ + Retrieves a single article by its ID. Eagerly loads the author information. + + Args: + article_id (int): The unique identifier of the article. + + Returns: + Optional[Article]: The Article instance if found, None otherwise. + """ query = select(Article).where(Article.article_id == article_id).options(joinedload(Article.article_author)) - return db_session.execute(query).unique().scalar_one_or_none() + return self.session.execute(query).unique().scalar_one_or_none() + + def create_article(self, title: str, content: str, author_id: int) -> Article: + """ + Creates a new article and adds it to the database session. + + Args: + title (str): The title of the new article. + content (str): The body content of the new article. + author_id (int): The unique identifier of the user creating the article. - @staticmethod - def create_article(title, content, author_id): + Returns: + Article: The newly created Article instance. + """ new_article = Article(article_title=title, article_content=content, article_author_id=author_id) - db_session.add(new_article) + self.session.add(new_article) return new_article - @staticmethod - def update_article(article_id, user_id, role, title, content): - article = ArticleService.get_by_id(article_id) + def update_article(self, article_id: int, user_id: int, role: str, title: str, content: str) -> Optional[Article]: + """ + Updates an existing article ensuring the requester is the original author. + + Args: + article_id (int): The unique identifier of the article to update. + user_id (int): The identifier of the user attempting to update. + role (str): The role of the user. + title (str): The new title of the article. + content (str): The new content of the article. + + Returns: + Optional[Article]: The updated Article instance if successful, None if not found or unauthorized. + """ + article = self.get_by_id(article_id) if not article or article.article_author_id != user_id: return None @@ -40,17 +92,36 @@ def update_article(article_id, user_id, role, title, content): article.article_content = content return article - @staticmethod - def delete_article(article_id, user_id, role): - article = ArticleService.get_by_id(article_id) - if not article or (role != "admin" and article.article_author_id != user_id): + def delete_article(self, article_id: int, user_id: int, role: str) -> bool: + """ + Deletes an article. Only the original author or an Admin can delete it. + + Args: + article_id (int): The unique identifier of the article to delete. + user_id (int): The identifier of the user attempting to delete. + role (str): The role of the user (e.g., 'admin', 'user'). + + Returns: + bool: True if the deletion was successful, False if not found or unauthorized. + """ + article = self.get_by_id(article_id) + if not article or (role != Role.ADMIN and article.article_author_id != user_id): return False - db_session.delete(article) + self.session.delete(article) return True - @staticmethod - def get_paginated_articles(page, per_page): + def get_paginated_articles(self, page: int, per_page: int) -> List[tuple]: + """ + Retrieves a paginated list of articles containing limited metadata. + + Args: + page (int): The current page number to retrieve (1-indexed). + per_page (int): The number of articles per page. + + Returns: + List[tuple]: A list of tuples containing (id, title, published_at, author_id, username). + """ query = ( select( Article.article_id, @@ -64,9 +135,15 @@ def get_paginated_articles(page, per_page): .limit(per_page) .offset((page - 1) * per_page) ) - return db_session.execute(query).all() + return list(self.session.execute(query).all()) + + def get_total_count(self) -> int: + """ + Retrieves the total number of articles in the database. - @staticmethod - def get_total_count(): + Returns: + int: The total count of articles. + """ query = select(func.count(Article.article_id)) - return db_session.execute(query).scalar() + count = self.session.execute(query).scalar() + return int(count) if count is not None else 0 diff --git a/app/services/comment_service.py b/app/services/comment_service.py index 224b678..c3c2cee 100644 --- a/app/services/comment_service.py +++ b/app/services/comment_service.py @@ -1,15 +1,42 @@ +from typing import List, Optional + from sqlalchemy import select -from sqlalchemy.orm import joinedload +from sqlalchemy.orm import Session, joinedload +from app.constants import Role from app.models.article_model import Article from app.models.comment_model import Comment -from database.database_setup import db_session class CommentService: - @staticmethod - def create_reply(parent_comment_id, user_id, content): - parent = db_session.get(Comment, parent_comment_id) + """ + Service class responsible for business logic operations related to Comments. + Handles creating top-level comments, replies, retrieving comments by article, and deleting comments. + """ + + def __init__(self, session: Session): + """ + Initialize the service with a database session (Dependency Injection). + + Args: + session (Session): The SQLAlchemy database session to use for queries. + """ + self.session = session + + def create_reply(self, parent_comment_id: int, user_id: int, content: str) -> Optional[int]: + """ + Creates a reply to an existing comment. A reply is linked either to the parent directly + or to the parent's top-level comment (threading logic). + + Args: + parent_comment_id (int): The ID of the comment being replied to. + user_id (int): The identifier of the user creating the reply. + content (str): The text content of the reply. + + Returns: + Optional[int]: The article ID the comment belongs to if successful, None if the parent comment is not found. + """ + parent = self.session.get(Comment, parent_comment_id) if not parent: return None @@ -20,36 +47,85 @@ def create_reply(parent_comment_id, user_id, content): comment_content=content, comment_reply_to=actual_parent_id ) - db_session.add(new_reply) + self.session.add(new_reply) return parent.comment_article_id - @staticmethod - def get_by_id(comment_id): + def get_by_id(self, comment_id: int) -> Optional[Comment]: + """ + Retrieves a single comment by its ID. Eagerly loads the author information. + + Args: + comment_id (int): The unique identifier of the comment. + + Returns: + Optional[Comment]: The Comment instance if found, None otherwise. + """ query = select(Comment).options(joinedload(Comment.comment_author)).where(Comment.comment_id == comment_id) - return db_session.execute(query).unique().scalar_one_or_none() + return self.session.execute(query).unique().scalar_one_or_none() + + def create_comment(self, article_id: int, user_id: int, content: str) -> bool: + """ + Creates a top-level comment on an article. - @staticmethod - def create_comment(article_id, user_id, content): - article = db_session.get(Article, article_id) + Args: + article_id (int): The ID of the article being commented on. + user_id (int): The identifier of the user creating the comment. + content (str): The body text of the comment. + + Returns: + bool: True if the comment was created successfully, False if the article does not exist. + """ + article = self.session.get(Article, article_id) if not article: return False - new_comment = Comment(comment_article_id=article_id, comment_written_account_id=user_id, comment_content=content) - db_session.add(new_comment) + + new_comment = Comment( + comment_article_id=article_id, + comment_written_account_id=user_id, + comment_content=content + ) + self.session.add(new_comment) return True - @staticmethod - def delete_comment(comment_id, role): - if role != "admin": - return False - comment = db_session.get(Comment, comment_id) + def delete_comment(self, comment_id: int, role: str) -> Optional[int]: + """ + Deletes a comment. Only users with the 'admin' role can delete comments. + + Args: + comment_id (int): The ID of the comment to delete. + role (str): The role of the user attempting the deletion. + + Returns: + Optional[int]: The article ID the comment belonged to if successful, None if unauthorized or not found. + """ + if role != Role.ADMIN: + return None + + comment = self.session.get(Comment, comment_id) if not comment: - return False + return None + article_id = comment.comment_article_id - db_session.delete(comment) + self.session.delete(comment) return article_id - @staticmethod - def get_tree_by_article_id(article_id): - query = select(Comment).where(Comment.comment_article_id == article_id).options(joinedload(Comment.comment_author)).order_by(Comment.comment_posted_at.asc()) - all_comments = db_session.execute(query).unique().scalars().all() + def get_tree_by_article_id(self, article_id: int) -> List[Comment]: + """ + Retrieves all comments for a specific article as a threaded tree structure. + Eagerly loads author information and returns only the top-level comments; + replies are typically nested within the 'replies' relationship of the parent comment. + + Args: + article_id (int): The ID of the article. + + Returns: + List[Comment]: A list of top-level Comment instances for the given article. + """ + query = ( + select(Comment) + .where(Comment.comment_article_id == article_id) + .options(joinedload(Comment.comment_author)) + .order_by(Comment.comment_posted_at.asc()) + ) + all_comments = self.session.execute(query).unique().scalars().all() return [c for c in all_comments if c.comment_reply_to is None] diff --git a/app/services/login_service.py b/app/services/login_service.py index b5a06f8..a0ff5cc 100644 --- a/app/services/login_service.py +++ b/app/services/login_service.py @@ -1,18 +1,40 @@ +from typing import Optional + from sqlalchemy import select +from sqlalchemy.orm import Session from app.models.account_model import Account -from database.database_setup import db_session class LoginService: - @staticmethod - def authenticate_user(username, password): + """ + Service class responsible for handling user authentication logic. + """ + + def __init__(self, session: Session): + """ + Initialize the service with a database session (Dependency Injection). + + Args: + session (Session): The SQLAlchemy database session to use for queries. + """ + self.session = session + + def authenticate_user(self, username: str, password: str) -> Optional[Account]: + """ + Validates the user's credentials against the database. + + Args: + username (str): The username provided by the user. + password (str): The plaintext password provided by the user. + + Returns: + Optional[Account]: The authenticated Account instance if credentials match, None otherwise. + """ query = select(Account).where(Account.account_username == username) - user = db_session.execute(query).scalar_one_or_none() + user = self.session.execute(query).scalar_one_or_none() + if user and user.account_password == password: - return { - "id": user.account_id, - "username": user.account_username, - "role": user.account_role - } + return user + return None diff --git a/tests/test_services/test_article_service.py b/tests/test_services/test_article_service.py index 702a448..186d62f 100644 --- a/tests/test_services/test_article_service.py +++ b/tests/test_services/test_article_service.py @@ -1,3 +1,4 @@ +from app.constants import Role from app.models.article_model import Article from app.services.article_service import ArticleService from tests.factories import make_account, make_article @@ -7,7 +8,8 @@ def test_create_article_service(db_session): author = make_account(account_role="author") db_session.add(author) db_session.commit() - article = ArticleService.create_article("Titre", "Contenu", author.account_id) + article_service = ArticleService(db_session) + article = article_service.create_article("Titre", "Contenu", author.account_id) db_session.commit() assert article.article_id is not None assert article.article_title == "Titre" @@ -22,7 +24,8 @@ def test_get_paginated_articles(db_session): db_session.add(make_article(author.account_id, article_title=f"Art {i}")) db_session.commit() - results = ArticleService.get_paginated_articles(page=1, per_page=3) + article_service = ArticleService(db_session) + results = article_service.get_paginated_articles(page=1, per_page=3) assert len(results) == 3 @@ -33,7 +36,8 @@ def test_update_article_success(db_session): article = make_article(author.account_id, article_title="Old Title") db_session.add(article) db_session.commit() - result = ArticleService.update_article(article.article_id, author.account_id, "user", "New Title", "New Content") + article_service = ArticleService(db_session) + result = article_service.update_article(article.article_id, author.account_id, "user", "New Title", "New Content") db_session.commit() assert result is not None assert result.article_title == "New Title" @@ -47,7 +51,8 @@ def test_update_article_unauthorized(db_session): article = make_article(author.account_id) db_session.add(article) db_session.commit() - result = ArticleService.update_article(article.article_id, wrong_user.account_id, "user", "Hacked", "...") + article_service = ArticleService(db_session) + result = article_service.update_article(article.article_id, wrong_user.account_id, "user", "Hacked", "...") assert result is None @@ -59,7 +64,8 @@ def test_delete_article_by_admin(db_session): article = make_article(author.account_id) db_session.add(article) db_session.commit() - result = ArticleService.delete_article(article.article_id, admin.account_id, "admin") + article_service = ArticleService(db_session) + result = article_service.delete_article(article.article_id, admin.account_id, Role.ADMIN) db_session.commit() assert result is True assert db_session.get(Article, article.article_id) is None @@ -74,7 +80,8 @@ def test_get_total_count(db_session): db_session.add(make_article(author.account_id)) db_session.commit() - count = ArticleService.get_total_count() + article_service = ArticleService(db_session) + count = article_service.get_total_count() assert count == 3 @@ -85,7 +92,8 @@ def test_get_all_ordered_by_date(db_session): db_session.add(make_article(author.account_id, article_title="First")) db_session.add(make_article(author.account_id, article_title="Second")) db_session.commit() - articles = ArticleService.get_all_ordered_by_date() + article_service = ArticleService(db_session) + articles = article_service.get_all_ordered_by_date() assert len(articles) == 2 assert articles[0].article_title is not None @@ -98,6 +106,7 @@ def test_delete_article_unauthorized(db_session): article = make_article(author.account_id) db_session.add(article) db_session.commit() - result = ArticleService.delete_article(article.article_id, stranger.account_id, "user") + article_service = ArticleService(db_session) + result = article_service.delete_article(article.article_id, stranger.account_id, "user") assert result is False assert db_session.get(Article, article.article_id) is not None diff --git a/tests/test_services/test_comment_service.py b/tests/test_services/test_comment_service.py index 5a39ed0..74560ee 100644 --- a/tests/test_services/test_comment_service.py +++ b/tests/test_services/test_comment_service.py @@ -1,3 +1,4 @@ +from app.constants import Role from app.models.comment_model import Comment from app.services.comment_service import CommentService from tests.factories import make_account, make_article, make_comment @@ -10,12 +11,13 @@ def test_create_reply_logic(db_session): article = make_article(author.account_id) db_session.add(article) db_session.commit() - CommentService.create_comment(article.article_id, author.account_id, "Parent") + comment_service = CommentService(db_session) + comment_service.create_comment(article.article_id, author.account_id, "Parent") db_session.commit() - parent = CommentService.get_tree_by_article_id(article.article_id)[0] - CommentService.create_reply(parent.comment_id, author.account_id, "Reply") + parent = comment_service.get_tree_by_article_id(article.article_id)[0] + comment_service.create_reply(parent.comment_id, author.account_id, "Reply") db_session.commit() - tree = CommentService.get_tree_by_article_id(article.article_id) + tree = comment_service.get_tree_by_article_id(article.article_id) assert len(tree) == 1 assert len(tree[0].comment_replies) == 1 assert tree[0].comment_replies[0].comment_content == "Reply" @@ -28,13 +30,14 @@ def test_comment_flattening_logic(db_session): article = make_article(author.account_id) db_session.add(article) db_session.commit() - CommentService.create_comment(article.article_id, author.account_id, "Root") + comment_service = CommentService(db_session) + comment_service.create_comment(article.article_id, author.account_id, "Root") db_session.commit() root = db_session.query(Comment).filter_by(comment_content="Root").first() - CommentService.create_reply(root.comment_id, author.account_id, "Reply A") + comment_service.create_reply(root.comment_id, author.account_id, "Reply A") db_session.commit() reply_a = db_session.query(Comment).filter_by(comment_content="Reply A").first() - CommentService.create_reply(reply_a.comment_id, author.account_id, "Reply B") + comment_service.create_reply(reply_a.comment_id, author.account_id, "Reply B") db_session.commit() reply_b = db_session.query(Comment).filter_by(comment_content="Reply B").first() assert reply_b.comment_reply_to == root.comment_id @@ -51,7 +54,8 @@ def test_delete_comment_as_admin(db_session): comment = make_comment(article.article_id, author.account_id) db_session.add(comment) db_session.commit() - result = CommentService.delete_comment(comment.comment_id, "admin") + comment_service = CommentService(db_session) + result = comment_service.delete_comment(comment.comment_id, Role.ADMIN) db_session.commit() assert result == article.article_id assert db_session.get(Comment, comment.comment_id) is None @@ -61,5 +65,6 @@ def test_create_comment_invalid_article(db_session): author = make_account() db_session.add(author) db_session.commit() - result = CommentService.create_comment(999, author.account_id, "Hello") + comment_service = CommentService(db_session) + result = comment_service.create_comment(999, author.account_id, "Hello") assert result is False diff --git a/tests/test_services/test_login_service.py b/tests/test_services/test_login_service.py index ccd4761..f84118f 100644 --- a/tests/test_services/test_login_service.py +++ b/tests/test_services/test_login_service.py @@ -6,21 +6,24 @@ def test_authenticate_user_success(db_session): user = make_account(account_username="leia", account_password="password123") db_session.add(user) db_session.commit() - result = LoginService.authenticate_user("leia", "password123") + login_service = LoginService(db_session) + result = login_service.authenticate_user("leia", "password123") assert result is not None - assert result["username"] == "leia" - assert result["role"] == "user" - assert result["id"] == user.account_id + assert result.account_username == "leia" + assert result.account_role == "user" + assert result.account_id == user.account_id def test_authenticate_user_wrong_password(db_session): user = make_account(account_username="leia", account_password="password123") db_session.add(user) db_session.commit() - result = LoginService.authenticate_user("leia", "mauvais_pass") + login_service = LoginService(db_session) + result = login_service.authenticate_user("leia", "mauvais_pass") assert result is None def test_authenticate_user_non_existent(db_session): - result = LoginService.authenticate_user("fantome", "rien") + login_service = LoginService(db_session) + result = login_service.authenticate_user("fantome", "rien") assert result is None From 2e96286bccc468b092279d18cf295198a77c7dcc Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 02:17:40 +0100 Subject: [PATCH 03/11] Global code improvement : Add Google-style docstrings and strict PEP 484 type hints across the entire codebase, covering models, controllers, utilities, Flask routes, database configuration, and test factories/fixtures to improve clarity, consistency and maintainability --- app/__init__.py | 18 +++++++- app/controllers/article_controller.py | 62 ++++++++++++++++++++++++--- app/controllers/comment_controller.py | 38 ++++++++++++++-- app/controllers/login_controller.py | 27 ++++++++++-- app/models/account_model.py | 13 ++++++ app/models/article_model.py | 12 ++++++ app/models/comment_model.py | 15 +++++++ config/configuration_variables.py | 51 +++++++++++++++++++--- database/database_setup.py | 8 +++- main.py | 3 ++ tests/conftest.py | 51 ++++++++++++++++++---- tests/factories.py | 58 ++++++++++++++++++++++--- 12 files changed, 316 insertions(+), 40 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index edffe25..d726388 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,5 +1,6 @@ import os import sys +from typing import Optional from flask import Flask @@ -10,11 +11,24 @@ from database.database_setup import db_session -def shutdown_session(exception=None): +def shutdown_session(exception: Optional[BaseException] = None) -> None: + """ + Removes the database session at the end of the request. + + Args: + exception (Optional[BaseException]): The exception that triggered the teardown, if any. + """ db_session.remove() -def initialize_flask_application(): +def initialize_flask_application() -> Flask: + """ + Initializes and configures the Flask application. + Sets the secret key based on the environment and registers blueprints. + + Returns: + Flask: The configured Flask application instance. + """ app = Flask(__name__) if os.getenv("PYTEST_CURRENT_TEST") or "pytest" in sys.modules: app.secret_key = env_vars.test_secret_key diff --git a/app/controllers/article_controller.py b/app/controllers/article_controller.py index 4eedbb2..b9a93d2 100644 --- a/app/controllers/article_controller.py +++ b/app/controllers/article_controller.py @@ -1,6 +1,15 @@ import math -from flask import Blueprint, flash, redirect, render_template, request, session, url_for +from flask import ( + Blueprint, + Response, + flash, + redirect, + render_template, + request, + session, + url_for, +) from app.services.article_service import ArticleService from app.services.comment_service import CommentService @@ -10,7 +19,13 @@ @article_bp.route("/") -def list_articles(): +def list_articles() -> str: + """ + Renders the homepage with a paginated list of articles. + + Returns: + str: Rendered HTML template. + """ current_page_number = 1 page_number = request.args.get("page", current_page_number, type=int) articles_per_page = 10 @@ -22,7 +37,16 @@ def list_articles(): @article_bp.route("/article/") -def view_article(article_id): +def view_article(article_id: int) -> str | Response: + """ + Displays the details of a specific article and its comments. + + Args: + article_id (int): ID of the article to view. + + Returns: + str | Response: Rendered HTML template or redirect if not found. + """ article_service = ArticleService(db_session) article = article_service.get_by_id(article_id) if not article: @@ -35,7 +59,14 @@ def view_article(article_id): @article_bp.route("/article/new", methods=["GET", "POST"]) -def create_article(): +def create_article() -> str | Response: + """ + Handles the creation of a new blog article. + Restricted to 'admin' and 'author' roles. + + Returns: + str | Response: Rendered HTML template (GET) or redirect (POST). + """ if session.get("role") not in ["admin", "author"]: flash("Access restricted.") return redirect(url_for("article.list_articles")) @@ -49,7 +80,17 @@ def create_article(): @article_bp.route("/article//edit", methods=["GET", "POST"]) -def edit_article(article_id): +def edit_article(article_id: int) -> str | Response: + """ + Handles the editing of an existing article. + Ensures the user is authorized to perform the update. + + Args: + article_id (int): ID of the article to edit. + + Returns: + str | Response: Rendered HTML template (GET) or redirect (POST). + """ if request.method == "POST": article_service = ArticleService(db_session) article = article_service.update_article( @@ -74,7 +115,16 @@ def edit_article(article_id): @article_bp.route("/article//delete") -def delete_article(article_id): +def delete_article(article_id: int) -> Response: + """ + Handles the deletion of an article. + + Args: + article_id (int): ID of the article to delete. + + Returns: + Response: Redirect to the article list. + """ article_service = ArticleService(db_session) if article_service.delete_article(article_id, session.get("user_id"), session.get("role")): db_session.commit() diff --git a/app/controllers/comment_controller.py b/app/controllers/comment_controller.py index 1b5232f..3df474f 100644 --- a/app/controllers/comment_controller.py +++ b/app/controllers/comment_controller.py @@ -1,4 +1,4 @@ -from flask import Blueprint, flash, redirect, request, session, url_for +from flask import Blueprint, Response, flash, redirect, request, session, url_for from app.services.comment_service import CommentService from database.database_setup import db_session @@ -7,7 +7,17 @@ @comment_bp.route("/create/", methods=["POST"]) -def create_comment(article_id): +def create_comment(article_id: int) -> Response: + """ + Handles the creation of a new comment on an article. + Requires the user to be logged in. + + Args: + article_id (int): ID of the article being commented on. + + Returns: + Response: Redirect to the article view or login page. + """ # Exception to add here if not session.get("user_id"): flash("Login required.") @@ -22,7 +32,17 @@ def create_comment(article_id): @comment_bp.route("/reply/", methods=["POST"]) -def reply_to_comment(parent_comment_id): +def reply_to_comment(parent_comment_id: int) -> Response: + """ + Handles the creation of a reply to an existing comment. + Requires the user to be logged in. + + Args: + parent_comment_id (int): ID of the comment being replied to. + + Returns: + Response: Redirect to the article view or error page. + """ # Exception to add here if not session.get("user_id"): flash("Login required.") @@ -39,7 +59,17 @@ def reply_to_comment(parent_comment_id): @comment_bp.route("/delete/") -def delete_comment(comment_id): +def delete_comment(comment_id: int) -> Response: + """ + Handles the deletion of a comment. + Restricted to users with the 'admin' role. + + Args: + comment_id (int): ID of the comment to delete. + + Returns: + Response: Redirect to the article view or article list. + """ comment_service = CommentService(db_session) article_id = comment_service.delete_comment(comment_id, session.get("role")) if article_id: diff --git a/app/controllers/login_controller.py b/app/controllers/login_controller.py index ebae4e0..edb3df3 100644 --- a/app/controllers/login_controller.py +++ b/app/controllers/login_controller.py @@ -1,4 +1,4 @@ -from flask import Blueprint, flash, redirect, render_template, request, session, url_for +from flask import Blueprint, Response, flash, redirect, render_template, request, session, url_for from app.services.login_service import LoginService from database.database_setup import db_session @@ -7,12 +7,25 @@ @login_bp.route("/login-page") -def render_login_page(): +def render_login_page() -> str: + """ + Renders the login page. + + Returns: + str: Rendered HTML template. + """ return render_template("login.html") @login_bp.route("/login", methods=["POST"]) -def login_authentication(): +def login_authentication() -> Response: + """ + Handles user authentication. + Validates credentials and sets up the session. + + Returns: + Response: Redirect to the article list on success, or login page on failure. + """ login_service = LoginService(db_session) user = login_service.authenticate_user(request.form.get("username"), request.form.get("password")) if user: @@ -25,6 +38,12 @@ def login_authentication(): @login_bp.route("/logout") -def logout(): +def logout() -> Response: + """ + Clears the user session and redirects to the article list. + + Returns: + Response: Redirect to the article list. + """ session.clear() return redirect(url_for("article.list_articles")) diff --git a/app/models/account_model.py b/app/models/account_model.py index 2999cf7..c8f5106 100644 --- a/app/models/account_model.py +++ b/app/models/account_model.py @@ -12,6 +12,19 @@ class Account(Base): + """ + Represents a user account in the system. + + Attributes: + account_id (int): Unique identifier for the account. + account_username (str): Unique username for login. + account_password (str): Plaintext password (should be hashed in production). + account_email (str): Optional email address. + account_role (str): Role of the user ('admin', 'author', 'user'). + account_created_at (datetime): Timestamp when the account was created. + articles (relationship): List of articles authored by this account. + comments (relationship): List of comments written by this account. + """ __tablename__ = "accounts" __table_args__ = (CheckConstraint(sqltext="account_role IN ('admin', 'author', 'user')", name="accounts_role_check"),) diff --git a/app/models/article_model.py b/app/models/article_model.py index f985929..0815537 100644 --- a/app/models/article_model.py +++ b/app/models/article_model.py @@ -12,6 +12,18 @@ class Article(Base): + """ + Represents a blog article. + + Attributes: + article_id (int): Unique identifier for the article. + article_author_id (int): Foreign key to the author's account. + article_title (str): Title of the article. + article_content (str): Full text content of the article. + article_published_at (datetime): Timestamp when the article was published. + article_author (relationship): The Account instance of the author. + article_comments (relationship): List of comments associated with this article. + """ __tablename__ = "articles" article_id = Column(name="article_id", type_=Integer, primary_key=True, autoincrement=True) diff --git a/app/models/comment_model.py b/app/models/comment_model.py index fc1cb62..021551b 100644 --- a/app/models/comment_model.py +++ b/app/models/comment_model.py @@ -12,6 +12,21 @@ class Comment(Base): + """ + Represents a comment or a reply on an article. + + Attributes: + comment_id (int): Unique identifier for the comment. + comment_article_id (int): Foreign key to the article. + comment_written_account_id (int): Foreign key to the author's account. + comment_reply_to (int, optional): Foreign key to the parent comment if it's a reply. + comment_content (str): Text content of the comment. + comment_posted_at (datetime): Timestamp when the comment was posted. + comment_article (relationship): The Article instance this comment belongs to. + comment_author (relationship): The Account instance of the comment's author. + reply_to_comment (relationship): The parent Comment instance if this is a reply. + comment_replies (relationship): List of child Comment instances (replies). + """ __tablename__ = "comments" comment_id = Column(name="comment_id", type_=Integer, primary_key=True, autoincrement=True) diff --git a/config/configuration_variables.py b/config/configuration_variables.py index 78356b5..f6b3b96 100644 --- a/config/configuration_variables.py +++ b/config/configuration_variables.py @@ -8,27 +8,66 @@ load_dotenv(BASE_DIR / ".env") load_dotenv(BASE_DIR / ".env.test") -def get_env_variable(name: str): +def get_env_variable(name: str) -> str: + """ + Retrieves an environment variable or raises a RuntimeError if missing. + + Args: + name (str): The name of the environment variable. + + Returns: + str: The value of the environment variable. + + Raises: + RuntimeError: If the environment variable is not set. + """ value = os.getenv(name) if not value: raise RuntimeError(f"Missing mandatory environment variable: '{name}'") return value class EnvVariablesConfig: + """ + Configuration class to manage environment variables for the application. + """ @property - def database_url(self): + def database_url(self) -> str: + """ + The production database URL. + + Returns: + str: Database connection string. + """ return get_env_variable("DATABASE_URL") @property - def secret_key(self): + def secret_key(self) -> str: + """ + The secret key for Flask sessions. + + Returns: + str: Secret key string. + """ return get_env_variable("SECRET_KEY") @property - def test_database_url(self): + def test_database_url(self) -> str: + """ + The test database URL. + + Returns: + str: Test database connection string. + """ return get_env_variable("TEST_DATABASE_URL") @property - def test_secret_key(self): + def test_secret_key(self) -> str: + """ + The secret key for Flask sessions during testing. + + Returns: + str: Test secret key string. + """ return get_env_variable("TEST_SECRET_KEY") -env_vars = EnvVariablesConfig() +env_vars = EnvVariablesConfig() diff --git a/database/database_setup.py b/database/database_setup.py index 22157a1..9bddb9c 100644 --- a/database/database_setup.py +++ b/database/database_setup.py @@ -6,10 +6,14 @@ from config.configuration_variables import env_vars +""" +Database engine and session management. +""" + if os.getenv("PYTEST_CURRENT_TEST") or "pytest" in sys.modules: - database_url = env_vars.test_database_url + database_url: str = env_vars.test_database_url else: - database_url = env_vars.database_url + database_url: str = env_vars.database_url database_engine = create_engine(database_url) session_factory = sessionmaker(bind=database_engine) diff --git a/main.py b/main.py index c02d1f8..3491f7b 100644 --- a/main.py +++ b/main.py @@ -1,5 +1,8 @@ from app import initialize_flask_application if __name__ == "__main__": + """ + Main entry point for running the Flask development server. + """ app = initialize_flask_application() app.run(debug=True) diff --git a/tests/conftest.py b/tests/conftest.py index 744d17d..a21630a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,11 @@ +from typing import Any, Generator + import pytest +from flask import Flask +from flask.testing import FlaskClient from sqlalchemy import text +from sqlalchemy.engine import Connection +from sqlalchemy.orm import Session from app import initialize_flask_application from app.models.account_model import Account @@ -10,7 +16,13 @@ from database.database_setup import db_session as app_db_session -def truncate_all_tables(connection): +def truncate_all_tables(connection: Connection) -> None: + """ + Truncates all tables in the database to ensure a clean state for tests. + + Args: + connection (Connection): SQLAlchemy connection object. + """ tables = Base.metadata.sorted_tables table_names = ", ".join(f'"{t.name}"' for t in tables) if table_names: @@ -18,7 +30,13 @@ def truncate_all_tables(connection): @pytest.fixture(scope="function") -def app(): +def app() -> Generator[Flask, Any, None]: + """ + Pytest fixture that initializes the Flask application for testing. + + Yields: + Flask: The Flask application instance. + """ flask_app = initialize_flask_application() flask_app.config.update({ "TESTING": True, @@ -28,19 +46,34 @@ def app(): @pytest.fixture(scope="function") -def client(app): +def client(app: Flask) -> FlaskClient: + """ + Pytest fixture that provides a test client for the Flask application. + + Args: + app (Flask): The Flask application instance. + + Returns: + FlaskClient: A test client. + """ return app.test_client() @pytest.fixture(scope="function") -def db_session(app): - # We include the 'app' fixture as a dependency to ensure that the Flask application - # is fully initialized before the database session is established. This guarantees - # that all configurations and model discoveries are completed +def db_session(app: Flask) -> Generator[Session, Any, None]: + """ + Pytest fixture that provides a clean database session for each test function. + Truncates all tables before yielding the session. + + Args: + app (Flask): The Flask application instance. - # Explicitly referencing models to satisfy linters (prevent unused import errors) - # Ensure SQLAlchemy's Base metadata is populated for TRUNCATE operations + Yields: + Session: A scoped SQLAlchemy session. + """ + # Explicitly referencing models to satisfy linters and ensure metadata is populated _ = (Account, Article, Comment) + if database_engine.url.render_as_string(hide_password=False) != env_vars.test_database_url: pytest.exit("SECURITY ERROR: The current database URL does not match the configured TEST database URL.") diff --git a/tests/factories.py b/tests/factories.py index ed542cc..5afc913 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -4,11 +4,23 @@ def make_account( - account_username="Xxx__D4RK_V4D0R__xxX", - account_password="password123", - account_email="vador@empire.com", - account_role="user", -): + account_username: str = "Xxx__D4RK_V4D0R__xxX", + account_password: str = "password123", + account_email: str = "vador@empire.com", + account_role: str = "user", +) -> Account: + """ + Factory to create an Account instance for testing. + + Args: + account_username (str): Username. + account_password (str): Password. + account_email (str): Email. + account_role (str): Role. + + Returns: + Account: An Account instance. + """ return Account( account_username=account_username, account_password=account_password, @@ -16,14 +28,46 @@ def make_account( account_role=account_role, ) -def make_article(article_author_id, article_title="Luke, I'm your father !", article_content="On the platform, Darth Vader stepped forward..."): + +def make_article( + article_author_id: int, + article_title: str = "Luke, I'm your father !", + article_content: str = "On the platform, Darth Vader stepped forward...", +) -> Article: + """ + Factory to create an Article instance for testing. + + Args: + article_author_id (int): ID of the author account. + article_title (str): Title. + article_content (str): Content. + + Returns: + Article: An Article instance. + """ return Article( article_author_id=article_author_id, article_title=article_title, article_content=article_content, ) -def make_comment(comment_article_id, comment_written_account_id, comment_content="Bravo !"): + +def make_comment( + comment_article_id: int, + comment_written_account_id: int, + comment_content: str = "Bravo !", +) -> Comment: + """ + Factory to create a Comment instance for testing. + + Args: + comment_article_id (int): ID of the article. + comment_written_account_id (int): ID of the author account. + comment_content (str): Content. + + Returns: + Comment: A Comment instance. + """ return Comment( comment_article_id=comment_article_id, comment_written_account_id=comment_written_account_id, From 02668406ce4faa6ddd037959bd4f478b8be4c5b3 Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 02:30:56 +0100 Subject: [PATCH 04/11] Global code improvement : Centralize security decorators, replace magic strings with enums, extract pagination config, streamline controller logic and update tests to align with new standards --- app/constants.py | 19 ++++++- app/controllers/article_controller.py | 56 +++++++++++++------ app/controllers/comment_controller.py | 33 +++++++---- app/controllers/login_controller.py | 13 +++-- app/utils/decorators.py | 48 ++++++++++++++++ .../test_article_controller.py | 53 +++++++++--------- .../test_comment_controller.py | 26 +++++---- .../test_controllers/test_login_controller.py | 18 +++--- 8 files changed, 185 insertions(+), 81 deletions(-) create mode 100644 app/utils/decorators.py diff --git a/app/constants.py b/app/constants.py index 06c73a1..a0366a3 100644 --- a/app/constants.py +++ b/app/constants.py @@ -4,8 +4,23 @@ class Role(str, Enum): """ Enum representing user roles within the application. - Inherits from str to allow direct string comparisons and easy database serialization. """ - ADMIN = "admin" + AUTHOR = "author" USER = "user" + + +class SessionKey(str, Enum): + """ + Enum representing keys used in the Flask session. + """ + USER_ID = "user_id" + ROLE = "role" + USERNAME = "username" + + +class PaginationConfig: + """ + Configuration for pagination settings. + """ + ARTICLES_PER_PAGE = 10 diff --git a/app/controllers/article_controller.py b/app/controllers/article_controller.py index b9a93d2..f050b20 100644 --- a/app/controllers/article_controller.py +++ b/app/controllers/article_controller.py @@ -1,4 +1,5 @@ import math +from typing import Optional from flask import ( Blueprint, @@ -11,8 +12,10 @@ url_for, ) +from app.constants import PaginationConfig, Role, SessionKey from app.services.article_service import ArticleService from app.services.comment_service import CommentService +from app.utils.decorators import roles_accepted from database.database_setup import db_session article_bp = Blueprint("article", __name__) @@ -26,14 +29,20 @@ def list_articles() -> str: Returns: str: Rendered HTML template. """ - current_page_number = 1 - page_number = request.args.get("page", current_page_number, type=int) - articles_per_page = 10 + page_number = request.args.get("page", 1, type=int) + articles_per_page = PaginationConfig.ARTICLES_PER_PAGE + article_service = ArticleService(db_session) articles = article_service.get_paginated_articles(page_number, articles_per_page) total_articles = article_service.get_total_count() total_pages = math.ceil(total_articles / articles_per_page) - return render_template("index.html", articles=articles, page_number=page_number, total_pages=total_pages) + + return render_template( + "index.html", + articles=articles, + page_number=page_number, + total_pages=total_pages + ) @article_bp.route("/article/") @@ -59,6 +68,7 @@ def view_article(article_id: int) -> str | Response: @article_bp.route("/article/new", methods=["GET", "POST"]) +@roles_accepted(Role.ADMIN, Role.AUTHOR) def create_article() -> str | Response: """ Handles the creation of a new blog article. @@ -67,19 +77,22 @@ def create_article() -> str | Response: Returns: str | Response: Rendered HTML template (GET) or redirect (POST). """ - if session.get("role") not in ["admin", "author"]: - flash("Access restricted.") - return redirect(url_for("article.list_articles")) if request.method == "POST": article_service = ArticleService(db_session) - article_service.create_article(request.form.get("title"), request.form.get("content"), session["user_id"]) + article_service.create_article( + title=request.form.get("title"), + content=request.form.get("content"), + author_id=session[SessionKey.USER_ID] + ) db_session.commit() flash("Article published!") return redirect(url_for("article.list_articles")) + return render_template("article_form.html", article=None) @article_bp.route("/article//edit", methods=["GET", "POST"]) +@roles_accepted(Role.ADMIN, Role.AUTHOR, Role.USER) def edit_article(article_id: int) -> str | Response: """ Handles the editing of an existing article. @@ -91,30 +104,34 @@ def edit_article(article_id: int) -> str | Response: Returns: str | Response: Rendered HTML template (GET) or redirect (POST). """ + article_service = ArticleService(db_session) + if request.method == "POST": - article_service = ArticleService(db_session) article = article_service.update_article( - article_id, - session.get("user_id"), - session.get("role"), - request.form.get("title"), - request.form.get("content") + article_id=article_id, + user_id=session.get(SessionKey.USER_ID), + role=session.get(SessionKey.ROLE), + title=request.form.get("title"), + content=request.form.get("content") ) if article: db_session.commit() flash("Article updated!") return redirect(url_for("article.view_article", article_id=article_id)) + flash("Update failed: Unauthorized or not found.") return redirect(url_for("article.list_articles")) - article_service = ArticleService(db_session) + article = article_service.get_by_id(article_id) if not article: flash("Article not found.") return redirect(url_for("article.list_articles")) + return render_template("article_form.html", article=article) @article_bp.route("/article//delete") +@roles_accepted(Role.ADMIN, Role.AUTHOR, Role.USER) def delete_article(article_id: int) -> Response: """ Handles the deletion of an article. @@ -126,9 +143,14 @@ def delete_article(article_id: int) -> Response: Response: Redirect to the article list. """ article_service = ArticleService(db_session) - if article_service.delete_article(article_id, session.get("user_id"), session.get("role")): + if article_service.delete_article( + article_id=article_id, + user_id=session.get(SessionKey.USER_ID), + role=session.get(SessionKey.ROLE) + ): db_session.commit() flash("Article deleted.") else: - flash("Delete failed.") + flash("Delete failed: Unauthorized or not found.") + return redirect(url_for("article.list_articles")) diff --git a/app/controllers/comment_controller.py b/app/controllers/comment_controller.py index 3df474f..ba419b6 100644 --- a/app/controllers/comment_controller.py +++ b/app/controllers/comment_controller.py @@ -1,12 +1,15 @@ from flask import Blueprint, Response, flash, redirect, request, session, url_for +from app.constants import Role, SessionKey from app.services.comment_service import CommentService +from app.utils.decorators import login_required, roles_accepted from database.database_setup import db_session comment_bp = Blueprint("comment", __name__, url_prefix="/comments") @comment_bp.route("/create/", methods=["POST"]) +@login_required def create_comment(article_id: int) -> Response: """ Handles the creation of a new comment on an article. @@ -18,20 +21,22 @@ def create_comment(article_id: int) -> Response: Returns: Response: Redirect to the article view or login page. """ - # Exception to add here - if not session.get("user_id"): - flash("Login required.") - return redirect(url_for("login.render_login_page")) comment_service = CommentService(db_session) - if comment_service.create_comment(article_id, session["user_id"], request.form.get("content")): + if comment_service.create_comment( + article_id=article_id, + user_id=session[SessionKey.USER_ID], + content=request.form.get("content") + ): db_session.commit() flash("Comment added.") else: flash("Error adding comment.") + return redirect(url_for("article.view_article", article_id=article_id)) @comment_bp.route("/reply/", methods=["POST"]) +@login_required def reply_to_comment(parent_comment_id: int) -> Response: """ Handles the creation of a reply to an existing comment. @@ -43,13 +48,12 @@ def reply_to_comment(parent_comment_id: int) -> Response: Returns: Response: Redirect to the article view or error page. """ - # Exception to add here - if not session.get("user_id"): - flash("Login required.") - return redirect(url_for("login.render_login_page")) - comment_service = CommentService(db_session) - article_id = comment_service.create_reply(parent_comment_id, session["user_id"], request.form.get("content")) + article_id = comment_service.create_reply( + parent_comment_id=parent_comment_id, + user_id=session[SessionKey.USER_ID], + content=request.form.get("content") + ) if article_id: db_session.commit() return redirect(url_for("article.view_article", article_id=article_id)) @@ -59,6 +63,7 @@ def reply_to_comment(parent_comment_id: int) -> Response: @comment_bp.route("/delete/") +@roles_accepted(Role.ADMIN) def delete_comment(comment_id: int) -> Response: """ Handles the deletion of a comment. @@ -71,7 +76,10 @@ def delete_comment(comment_id: int) -> Response: Response: Redirect to the article view or article list. """ comment_service = CommentService(db_session) - article_id = comment_service.delete_comment(comment_id, session.get("role")) + article_id = comment_service.delete_comment( + comment_id=comment_id, + role=session.get(SessionKey.ROLE) + ) if article_id: db_session.commit() flash("Comment deleted.") @@ -79,3 +87,4 @@ def delete_comment(comment_id: int) -> Response: flash("Unauthorized or not found.") return redirect(url_for("article.list_articles")) + diff --git a/app/controllers/login_controller.py b/app/controllers/login_controller.py index edb3df3..7d070c2 100644 --- a/app/controllers/login_controller.py +++ b/app/controllers/login_controller.py @@ -1,5 +1,6 @@ from flask import Blueprint, Response, flash, redirect, render_template, request, session, url_for +from app.constants import SessionKey from app.services.login_service import LoginService from database.database_setup import db_session @@ -27,12 +28,16 @@ def login_authentication() -> Response: Response: Redirect to the article list on success, or login page on failure. """ login_service = LoginService(db_session) - user = login_service.authenticate_user(request.form.get("username"), request.form.get("password")) + user = login_service.authenticate_user( + username=request.form.get("username"), + password=request.form.get("password") + ) if user: - session["user_id"] = user.account_id - session["username"] = user.account_username - session["role"] = user.account_role + session[SessionKey.USER_ID] = user.account_id + session[SessionKey.USERNAME] = user.account_username + session[SessionKey.ROLE] = user.account_role return redirect(url_for("article.list_articles")) + flash("Incorrect credentials.") return redirect(url_for("login.render_login_page")) diff --git a/app/utils/decorators.py b/app/utils/decorators.py new file mode 100644 index 0000000..b45348e --- /dev/null +++ b/app/utils/decorators.py @@ -0,0 +1,48 @@ +from functools import wraps +from typing import Any, Callable + +from flask import flash, redirect, session, url_for + +from app.constants import Role, SessionKey + + +def login_required(f: Callable[..., Any]) -> Callable[..., Any]: + """ + Decorator to ensure that a user is logged in before accessing a route. + + Args: + f (Callable): The route function to wrap. + + Returns: + Callable: The wrapped function. + """ + @wraps(f) + def decorated_function(*args: Any, **kwargs: Any) -> Any: + if not session.get(SessionKey.USER_ID): + flash("Login required.") + return redirect(url_for("login.render_login_page")) + return f(*args, **kwargs) + return decorated_function + + +def roles_accepted(*roles: Role) -> Callable[..., Any]: + """ + Decorator to ensure that a logged-in user has one of the required roles. + + Args: + *roles (Role): Variable list of accepted roles. + + Returns: + Callable: A decorator that wraps the route function. + """ + def decorator(f: Callable[..., Any]) -> Callable[..., Any]: + @wraps(f) + @login_required + def decorated_function(*args: Any, **kwargs: Any) -> Any: + user_role = session.get(SessionKey.ROLE) + if user_role not in [role.value for role in roles]: + flash("Access restricted: Insufficient permissions.") + return redirect(url_for("article.list_articles")) + return f(*args, **kwargs) + return decorated_function + return decorator diff --git a/tests/test_controllers/test_article_controller.py b/tests/test_controllers/test_article_controller.py index a4d1f2d..6b04955 100644 --- a/tests/test_controllers/test_article_controller.py +++ b/tests/test_controllers/test_article_controller.py @@ -1,5 +1,6 @@ from unittest.mock import patch +from app.constants import Role, SessionKey from app.models.article_model import Article from tests.factories import make_account, make_article @@ -30,21 +31,21 @@ def test_view_article_not_found(client): def test_create_article_restricted(client): with client.session_transaction() as sess: - sess["user_id"] = 1 - sess["role"] = "user" + sess[SessionKey.USER_ID] = 1 + sess[SessionKey.ROLE] = Role.USER response = client.get("/article/new", follow_redirects=True) assert b"Access restricted" in response.data def test_create_article_success(client, db_session): - author = make_account(account_role="author") + author = make_account(account_role=Role.AUTHOR) db_session.add(author) db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = author.account_id - sess["role"] = "author" + sess[SessionKey.USER_ID] = author.account_id + sess[SessionKey.ROLE] = Role.AUTHOR response = client.post("/article/new", data={"title": "Nouveau Titre", "content": "Contenu"}, follow_redirects=True) assert b"Article published!" in response.data @@ -53,13 +54,13 @@ def test_create_article_success(client, db_session): def test_create_article_atomicity_failure(client, db_session): - author = make_account(account_role="author") + author = make_account(account_role=Role.AUTHOR) db_session.add(author) db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = author.account_id - sess["role"] = "author" + sess[SessionKey.USER_ID] = author.account_id + sess[SessionKey.ROLE] = Role.AUTHOR with patch("database.database_setup.db_session.commit") as mock_commit: mock_commit.side_effect = Exception("Database Failure") @@ -74,8 +75,8 @@ def test_create_article_atomicity_failure(client, db_session): def test_edit_article_unauthorized(client, db_session): - author1 = make_account(account_username="Author1", account_role="author") - author2 = make_account(account_username="Author2", account_role="author") + author1 = make_account(account_username="Author1", account_role=Role.AUTHOR) + author2 = make_account(account_username="Author2", account_role=Role.AUTHOR) db_session.add_all([author1, author2]) db_session.commit() @@ -84,8 +85,8 @@ def test_edit_article_unauthorized(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = author2.account_id - sess["role"] = "author" + sess[SessionKey.USER_ID] = author2.account_id + sess[SessionKey.ROLE] = Role.AUTHOR response = client.post(f"/article/{article.article_id}/edit", data={"title": "Hack", "content": "Hack"}, follow_redirects=True) assert b"Update failed" in response.data @@ -94,7 +95,7 @@ def test_edit_article_unauthorized(client, db_session): def test_delete_article_success(client, db_session): - author = make_account(account_role="admin") + author = make_account(account_role=Role.ADMIN) db_session.add(author) db_session.commit() article = make_article(author.account_id) @@ -102,8 +103,8 @@ def test_delete_article_success(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = author.account_id - sess["role"] = "admin" + sess[SessionKey.USER_ID] = author.account_id + sess[SessionKey.ROLE] = Role.ADMIN response = client.get(f"/article/{article.article_id}/delete", follow_redirects=True) assert b"Article deleted" in response.data @@ -129,8 +130,8 @@ def test_list_articles_pagination(client, db_session): def test_edit_article_not_found(client): with client.session_transaction() as sess: - sess["user_id"] = 1 - sess["role"] = "admin" + sess[SessionKey.USER_ID] = 1 + sess[SessionKey.ROLE] = Role.ADMIN response = client.get("/article/999/edit", follow_redirects=True) assert response.status_code == 200 @@ -138,7 +139,7 @@ def test_edit_article_not_found(client): def test_edit_article_success_by_author(client, db_session): - author = make_account(account_role="author") + author = make_account(account_role=Role.AUTHOR) db_session.add(author) db_session.commit() article = make_article(author.account_id, article_title="Ancien Titre") @@ -146,8 +147,8 @@ def test_edit_article_success_by_author(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = author.account_id - sess["role"] = "author" + sess[SessionKey.USER_ID] = author.account_id + sess[SessionKey.ROLE] = Role.AUTHOR response = client.post(f"/article/{article.article_id}/edit", data={"title": "Titre Modifié", "content": "Nouveau contenu"}, follow_redirects=True) @@ -158,7 +159,7 @@ def test_edit_article_success_by_author(client, db_session): def test_admin_cannot_edit_others_article(client, db_session): author = make_account(account_username="Auteur") - admin = make_account(account_username="Admin", account_role="admin") + admin = make_account(account_username="Admin", account_role=Role.ADMIN) db_session.add_all([author, admin]) db_session.commit() article = make_article(author.account_id, article_title="Titre Intouchable") @@ -166,8 +167,8 @@ def test_admin_cannot_edit_others_article(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = admin.account_id - sess["role"] = "admin" + sess[SessionKey.USER_ID] = admin.account_id + sess[SessionKey.ROLE] = Role.ADMIN response = client.post(f"/article/{article.article_id}/edit", data={"title": "Hack par Admin", "content": "..."}, follow_redirects=True) @@ -177,7 +178,7 @@ def test_admin_cannot_edit_others_article(client, db_session): def test_delete_article_success_by_author(client, db_session): - author = make_account(account_role="author") + author = make_account(account_role=Role.AUTHOR) db_session.add(author) db_session.commit() article = make_article(author.account_id) @@ -185,8 +186,8 @@ def test_delete_article_success_by_author(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = author.account_id - sess["role"] = "author" + sess[SessionKey.USER_ID] = author.account_id + sess[SessionKey.ROLE] = Role.AUTHOR response = client.get(f"/article/{article.article_id}/delete", follow_redirects=True) assert b"Article deleted" in response.data diff --git a/tests/test_controllers/test_comment_controller.py b/tests/test_controllers/test_comment_controller.py index 9719501..41c5963 100644 --- a/tests/test_controllers/test_comment_controller.py +++ b/tests/test_controllers/test_comment_controller.py @@ -1,5 +1,6 @@ from unittest.mock import patch +from app.constants import Role, SessionKey from app.models.comment_model import Comment from tests.factories import make_account, make_article, make_comment @@ -17,7 +18,7 @@ def test_create_comment_success(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = user.account_id + sess[SessionKey.USER_ID] = user.account_id response = client.post(f"/comments/create/{article.article_id}", data={"content": "Mon super commentaire"}, follow_redirects=True) @@ -30,7 +31,7 @@ def test_create_comment_on_invalid_article(client, db_session): db_session.add(user) db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = user.account_id + sess[SessionKey.USER_ID] = user.account_id response = client.post("/comments/create/9999", data={"content": "Error"}, follow_redirects=True) assert b"Error adding comment." in response.data @@ -47,7 +48,7 @@ def test_create_reply_success(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = user.account_id + sess[SessionKey.USER_ID] = user.account_id response = client.post(f"/comments/reply/{parent.comment_id}", data={"content": "Réponse"}, follow_redirects=True) assert response.status_code == 200 @@ -68,7 +69,7 @@ def test_create_reply_atomicity_failure(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = user.account_id + sess[SessionKey.USER_ID] = user.account_id with patch("database.database_setup.db_session.commit") as mock_commit: mock_commit.side_effect = Exception("Atomic Failure") @@ -82,8 +83,8 @@ def test_create_reply_atomicity_failure(client, db_session): assert reply is None def test_delete_comment_admin_only(client, db_session): - admin = make_account(account_username="Admin", account_role="admin") - user = make_account(account_username="User", account_role="user") + admin = make_account(account_username="Admin", account_role=Role.ADMIN) + user = make_account(account_username="User", account_role=Role.USER) db_session.add_all([admin, user]) db_session.commit() article = make_article(admin.account_id) @@ -94,14 +95,15 @@ def test_delete_comment_admin_only(client, db_session): db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = user.account_id - sess["role"] = "user" + sess[SessionKey.USER_ID] = user.account_id + sess[SessionKey.ROLE] = Role.USER response = client.get(f"/comments/delete/{comment.comment_id}", follow_redirects=True) - assert b"Unauthorized" in response.data + # The roles_accepted decorator flashes "Access restricted: Insufficient permissions." + assert b"Access restricted" in response.data or b"Insufficient permissions" in response.data with client.session_transaction() as sess: - sess["user_id"] = admin.account_id - sess["role"] = "admin" + sess[SessionKey.USER_ID] = admin.account_id + sess[SessionKey.ROLE] = Role.ADMIN response = client.get(f"/comments/delete/{comment.comment_id}", follow_redirects=True) assert b"Comment deleted." in response.data @@ -110,7 +112,7 @@ def test_reply_to_non_existent_comment(client, db_session): db_session.add(user) db_session.commit() with client.session_transaction() as sess: - sess["user_id"] = user.account_id + sess[SessionKey.USER_ID] = user.account_id response = client.post("/comments/reply/999", data={"content": "Hello"}, follow_redirects=True) assert b"Error replying" in response.data diff --git a/tests/test_controllers/test_login_controller.py b/tests/test_controllers/test_login_controller.py index f60591d..30df83f 100644 --- a/tests/test_controllers/test_login_controller.py +++ b/tests/test_controllers/test_login_controller.py @@ -1,3 +1,4 @@ +from app.constants import Role, SessionKey from tests.factories import make_account @@ -17,9 +18,9 @@ def test_login_success(client, db_session): response = client.post("/login", data={"username": "Vador", "password": "dark_password"}, follow_redirects=True) assert response.status_code == 200 with client.session_transaction() as session: - assert session["user_id"] == user.account_id - assert session["username"] == "Vador" - assert session["role"] == "user" + assert session[SessionKey.USER_ID] == user.account_id + assert session[SessionKey.USERNAME] == "Vador" + assert session[SessionKey.ROLE] == Role.USER def test_login_failure_wrong_password(client, db_session): user = make_account(account_username="Luke", account_password="correct_password") @@ -28,7 +29,7 @@ def test_login_failure_wrong_password(client, db_session): response = client.post("/login", data={"username": "Luke", "password": "wrong_password"}, follow_redirects=True) assert b"Incorrect credentials." in response.data with client.session_transaction() as session: - assert "user_id" not in session + assert SessionKey.USER_ID not in session def test_login_non_existent_user(client): response = client.post("/login", data={"username": "Inconnu", "password": "password"}, follow_redirects=True) @@ -36,10 +37,11 @@ def test_login_non_existent_user(client): def test_logout(client): with client.session_transaction() as session: - session["user_id"] = 1 - session["username"] = "Test" + session[SessionKey.USER_ID] = 1 + session[SessionKey.USERNAME] = "Test" response = client.get("/logout", follow_redirects=True) assert response.status_code == 200 with client.session_transaction() as session: - assert "user_id" not in session - assert "username" not in session + assert SessionKey.USER_ID not in session + assert SessionKey.USERNAME not in session + From 72843e0d141d97cd33bdd9583ecf7e7e56e5ea7c Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 04:53:56 +0100 Subject: [PATCH 05/11] Global code improvement : Harmonize docstrings and fix static type issues (Pyright/Pylance errors, unified Google-style docs, stricter controller return types) --- app/controllers/article_controller.py | 72 ++++++++++-------- app/controllers/comment_controller.py | 31 ++++---- app/controllers/login_controller.py | 17 +++-- app/models/account_model.py | 40 +++++----- app/models/article_model.py | 30 ++++---- app/models/comment_model.py | 45 ++++++------ app/services/article_service.py | 97 +++++++++++++++---------- app/services/comment_service.py | 44 ++++++----- app/services/login_service.py | 19 +++-- poetry.lock | 35 ++++++++- pyproject.toml | 3 +- tests/conftest.py | 9 ++- tests/test_models/test_account_model.py | 12 +-- tests/test_models/test_article_model.py | 14 ++-- 14 files changed, 278 insertions(+), 190 deletions(-) diff --git a/app/controllers/article_controller.py b/app/controllers/article_controller.py index f050b20..0e2c456 100644 --- a/app/controllers/article_controller.py +++ b/app/controllers/article_controller.py @@ -1,9 +1,8 @@ import math -from typing import Optional +from typing import Union from flask import ( Blueprint, - Response, flash, redirect, render_template, @@ -11,6 +10,7 @@ session, url_for, ) +from werkzeug.wrappers import Response from app.constants import PaginationConfig, Role, SessionKey from app.services.article_service import ArticleService @@ -27,26 +27,26 @@ def list_articles() -> str: Renders the homepage with a paginated list of articles. Returns: - str: Rendered HTML template. + str: The rendered HTML template for the homepage. """ page_number = request.args.get("page", 1, type=int) articles_per_page = PaginationConfig.ARTICLES_PER_PAGE - + article_service = ArticleService(db_session) articles = article_service.get_paginated_articles(page_number, articles_per_page) total_articles = article_service.get_total_count() total_pages = math.ceil(total_articles / articles_per_page) - + return render_template( - "index.html", - articles=articles, - page_number=page_number, + "index.html", + articles=articles, + page_number=page_number, total_pages=total_pages ) @article_bp.route("/article/") -def view_article(article_id: int) -> str | Response: +def view_article(article_id: int) -> Union[str, Response]: """ Displays the details of a specific article and its comments. @@ -54,7 +54,7 @@ def view_article(article_id: int) -> str | Response: article_id (int): ID of the article to view. Returns: - str | Response: Rendered HTML template or redirect if not found. + Union[str, Response]: The rendered HTML template for the article or a redirect if the article is not found. """ article_service = ArticleService(db_session) article = article_service.get_by_id(article_id) @@ -69,31 +69,35 @@ def view_article(article_id: int) -> str | Response: @article_bp.route("/article/new", methods=["GET", "POST"]) @roles_accepted(Role.ADMIN, Role.AUTHOR) -def create_article() -> str | Response: +def create_article() -> Union[str, Response]: """ Handles the creation of a new blog article. Restricted to 'admin' and 'author' roles. Returns: - str | Response: Rendered HTML template (GET) or redirect (POST). + Union[str, Response]: The rendered HTML form (GET) or a redirect to the article list after creation (POST). """ if request.method == "POST": article_service = ArticleService(db_session) + title = str(request.form.get("title") or "") + content = str(request.form.get("content") or "") + user_id = int(session.get(SessionKey.USER_ID) or 0) + article_service.create_article( - title=request.form.get("title"), - content=request.form.get("content"), - author_id=session[SessionKey.USER_ID] + title=title, + content=content, + author_id=user_id ) db_session.commit() flash("Article published!") return redirect(url_for("article.list_articles")) - + return render_template("article_form.html", article=None) @article_bp.route("/article//edit", methods=["GET", "POST"]) @roles_accepted(Role.ADMIN, Role.AUTHOR, Role.USER) -def edit_article(article_id: int) -> str | Response: +def edit_article(article_id: int) -> Union[str, Response]: """ Handles the editing of an existing article. Ensures the user is authorized to perform the update. @@ -102,23 +106,28 @@ def edit_article(article_id: int) -> str | Response: article_id (int): ID of the article to edit. Returns: - str | Response: Rendered HTML template (GET) or redirect (POST). + Union[str, Response]: The rendered HTML form (GET) or a redirect to the updated article (POST). """ article_service = ArticleService(db_session) - + if request.method == "POST": + user_id = int(session.get(SessionKey.USER_ID) or 0) + role = str(session.get(SessionKey.ROLE) or "") + title = str(request.form.get("title") or "") + content = str(request.form.get("content") or "") + article = article_service.update_article( article_id=article_id, - user_id=session.get(SessionKey.USER_ID), - role=session.get(SessionKey.ROLE), - title=request.form.get("title"), - content=request.form.get("content") + user_id=user_id, + role=role, + title=title, + content=content ) if article: db_session.commit() flash("Article updated!") return redirect(url_for("article.view_article", article_id=article_id)) - + flash("Update failed: Unauthorized or not found.") return redirect(url_for("article.list_articles")) @@ -126,7 +135,7 @@ def edit_article(article_id: int) -> str | Response: if not article: flash("Article not found.") return redirect(url_for("article.list_articles")) - + return render_template("article_form.html", article=article) @@ -140,17 +149,20 @@ def delete_article(article_id: int) -> Response: article_id (int): ID of the article to delete. Returns: - Response: Redirect to the article list. + Response: A redirect to the article list after deletion. """ article_service = ArticleService(db_session) + user_id = int(session.get(SessionKey.USER_ID) or 0) + role = str(session.get(SessionKey.ROLE) or "") + if article_service.delete_article( - article_id=article_id, - user_id=session.get(SessionKey.USER_ID), - role=session.get(SessionKey.ROLE) + article_id=article_id, + user_id=user_id, + role=role ): db_session.commit() flash("Article deleted.") else: flash("Delete failed: Unauthorized or not found.") - + return redirect(url_for("article.list_articles")) diff --git a/app/controllers/comment_controller.py b/app/controllers/comment_controller.py index ba419b6..2000a7f 100644 --- a/app/controllers/comment_controller.py +++ b/app/controllers/comment_controller.py @@ -1,4 +1,5 @@ -from flask import Blueprint, Response, flash, redirect, request, session, url_for +from flask import Blueprint, flash, redirect, request, session, url_for +from werkzeug.wrappers import Response from app.constants import Role, SessionKey from app.services.comment_service import CommentService @@ -19,19 +20,20 @@ def create_comment(article_id: int) -> Response: article_id (int): ID of the article being commented on. Returns: - Response: Redirect to the article view or login page. + Response: A redirect to the article view or login page. """ comment_service = CommentService(db_session) + content = str(request.form.get("content") or "") if comment_service.create_comment( - article_id=article_id, - user_id=session[SessionKey.USER_ID], - content=request.form.get("content") + article_id=article_id, + user_id=session[SessionKey.USER_ID], + content=content ): db_session.commit() flash("Comment added.") else: flash("Error adding comment.") - + return redirect(url_for("article.view_article", article_id=article_id)) @@ -46,13 +48,14 @@ def reply_to_comment(parent_comment_id: int) -> Response: parent_comment_id (int): ID of the comment being replied to. Returns: - Response: Redirect to the article view or error page. + Response: A redirect to the article view or the article list in case of error. """ comment_service = CommentService(db_session) + content = str(request.form.get("content") or "") article_id = comment_service.create_reply( - parent_comment_id=parent_comment_id, - user_id=session[SessionKey.USER_ID], - content=request.form.get("content") + parent_comment_id=parent_comment_id, + user_id=session[SessionKey.USER_ID], + content=content ) if article_id: db_session.commit() @@ -73,12 +76,13 @@ def delete_comment(comment_id: int) -> Response: comment_id (int): ID of the comment to delete. Returns: - Response: Redirect to the article view or article list. + Response: A redirect to the article view or article list after deletion. """ comment_service = CommentService(db_session) + role = str(session.get(SessionKey.ROLE) or "") article_id = comment_service.delete_comment( - comment_id=comment_id, - role=session.get(SessionKey.ROLE) + comment_id=comment_id, + role=role ) if article_id: db_session.commit() @@ -87,4 +91,3 @@ def delete_comment(comment_id: int) -> Response: flash("Unauthorized or not found.") return redirect(url_for("article.list_articles")) - diff --git a/app/controllers/login_controller.py b/app/controllers/login_controller.py index 7d070c2..7aecea2 100644 --- a/app/controllers/login_controller.py +++ b/app/controllers/login_controller.py @@ -1,4 +1,5 @@ -from flask import Blueprint, Response, flash, redirect, render_template, request, session, url_for +from flask import Blueprint, flash, redirect, render_template, request, session, url_for +from werkzeug.wrappers import Response from app.constants import SessionKey from app.services.login_service import LoginService @@ -13,7 +14,7 @@ def render_login_page() -> str: Renders the login page. Returns: - str: Rendered HTML template. + str: The rendered HTML template for the login page. """ return render_template("login.html") @@ -25,19 +26,21 @@ def login_authentication() -> Response: Validates credentials and sets up the session. Returns: - Response: Redirect to the article list on success, or login page on failure. + Response: A redirect to the article list on success, or back to the login page on failure. """ login_service = LoginService(db_session) + username = str(request.form.get("username") or "") + password = str(request.form.get("password") or "") user = login_service.authenticate_user( - username=request.form.get("username"), - password=request.form.get("password") + username=username, + password=password ) if user: session[SessionKey.USER_ID] = user.account_id session[SessionKey.USERNAME] = user.account_username session[SessionKey.ROLE] = user.account_role return redirect(url_for("article.list_articles")) - + flash("Incorrect credentials.") return redirect(url_for("login.render_login_page")) @@ -48,7 +51,7 @@ def logout() -> Response: Clears the user session and redirects to the article list. Returns: - Response: Redirect to the article list. + Response: A redirect to the article list after clearing the session. """ session.clear() return redirect(url_for("article.list_articles")) diff --git a/app/models/account_model.py b/app/models/account_model.py index c8f5106..d0d4c9d 100644 --- a/app/models/account_model.py +++ b/app/models/account_model.py @@ -1,12 +1,13 @@ +from datetime import datetime + from sqlalchemy import ( TIMESTAMP, CheckConstraint, - Column, Integer, Text, func, ) -from sqlalchemy.orm import relationship +from sqlalchemy.orm import Mapped, mapped_column, relationship from database.database_setup import Base @@ -14,26 +15,27 @@ class Account(Base): """ Represents a user account in the system. - + Attributes: - account_id (int): Unique identifier for the account. - account_username (str): Unique username for login. - account_password (str): Plaintext password (should be hashed in production). - account_email (str): Optional email address. - account_role (str): Role of the user ('admin', 'author', 'user'). - account_created_at (datetime): Timestamp when the account was created. - articles (relationship): List of articles authored by this account. - comments (relationship): List of comments written by this account. + account_id (int): Unique identifier for the account (Primary Key). + account_username (str): Unique username used for authentication. + account_password (str): Securely hashed password string. + account_email (str | None): Optional email address for the user. + account_role (str): Permissions role ('admin', 'author', or 'user'). + account_created_at (datetime): Automated timestamp of account creation. + articles (list[Article]): Collection of articles authored by this account. + comments (list[Comment]): Collection of comments written by this account. """ + __tablename__ = "accounts" __table_args__ = (CheckConstraint(sqltext="account_role IN ('admin', 'author', 'user')", name="accounts_role_check"),) - account_id = Column(name="account_id", type_=Integer, primary_key=True, autoincrement=True) - account_username = Column(name="account_username", type_=Text, unique=True, nullable=False) - account_password = Column(name="account_password", type_=Text, nullable=False) - account_email = Column(name="account_email", type_=Text) - account_role = Column(name="account_role", type_=Text, nullable=False) - account_created_at = Column(name="account_created_at", type_=TIMESTAMP, server_default=func.now()) + account_id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) + account_username: Mapped[str] = mapped_column(Text, unique=True, nullable=False) + account_password: Mapped[str] = mapped_column(Text, nullable=False) + account_email: Mapped[str | None] = mapped_column(Text) + account_role: Mapped[str] = mapped_column(Text, nullable=False) + account_created_at: Mapped[datetime] = mapped_column(TIMESTAMP, server_default=func.now()) - articles = relationship(argument="Article", back_populates="article_author", cascade="all, delete-orphan") - comments = relationship(argument="Comment", back_populates="comment_author", cascade="all, delete-orphan") + articles = relationship("Article", back_populates="article_author", cascade="all, delete-orphan") + comments = relationship("Comment", back_populates="comment_author", cascade="all, delete-orphan") diff --git a/app/models/article_model.py b/app/models/article_model.py index 0815537..aaec205 100644 --- a/app/models/article_model.py +++ b/app/models/article_model.py @@ -1,12 +1,13 @@ +from datetime import datetime + from sqlalchemy import ( TIMESTAMP, - Column, ForeignKey, Integer, Text, func, ) -from sqlalchemy.orm import relationship +from sqlalchemy.orm import Mapped, mapped_column, relationship from database.database_setup import Base @@ -16,21 +17,22 @@ class Article(Base): Represents a blog article. Attributes: - article_id (int): Unique identifier for the article. - article_author_id (int): Foreign key to the author's account. + article_id (int): Unique identifier for the article (Primary Key). + article_author_id (int): Foreign key referencing the author's Account. article_title (str): Title of the article. article_content (str): Full text content of the article. - article_published_at (datetime): Timestamp when the article was published. - article_author (relationship): The Account instance of the author. - article_comments (relationship): List of comments associated with this article. + article_published_at (datetime): Automated timestamp of publication. + article_author (Account): Relationship to the author's Account instance. + article_comments (list[Comment]): Collection of comments linked to this article. """ + __tablename__ = "articles" - article_id = Column(name="article_id", type_=Integer, primary_key=True, autoincrement=True) - article_author_id = Column(ForeignKey("accounts.account_id", ondelete="CASCADE"), name="article_author_id", type_=Integer, nullable=False) - article_title = Column(name="article_title", type_=Text, nullable=False) - article_content = Column(name="article_content", type_=Text, nullable=False) - article_published_at = Column(name="article_published_at", type_=TIMESTAMP, server_default=func.now()) + article_id: Mapped[int] = mapped_column("article_id", Integer, primary_key=True, autoincrement=True) + article_author_id: Mapped[int] = mapped_column(ForeignKey("accounts.account_id", ondelete="CASCADE"), name="article_author_id", nullable=False) + article_title: Mapped[str] = mapped_column("article_title", Text, nullable=False) + article_content: Mapped[str] = mapped_column("article_content", Text, nullable=False) + article_published_at: Mapped[datetime] = mapped_column("article_published_at", TIMESTAMP, server_default=func.now()) - article_author = relationship(argument="Account", back_populates="articles") - article_comments = relationship(argument="Comment", back_populates="comment_article", cascade="all, delete-orphan") + article_author = relationship("Account", back_populates="articles") + article_comments = relationship("Comment", back_populates="comment_article", cascade="all, delete-orphan") diff --git a/app/models/comment_model.py b/app/models/comment_model.py index 021551b..7e2f3bd 100644 --- a/app/models/comment_model.py +++ b/app/models/comment_model.py @@ -1,12 +1,13 @@ +from datetime import datetime + from sqlalchemy import ( TIMESTAMP, - Column, ForeignKey, Integer, Text, func, ) -from sqlalchemy.orm import relationship +from sqlalchemy.orm import Mapped, mapped_column, relationship from database.database_setup import Base @@ -16,36 +17,38 @@ class Comment(Base): Represents a comment or a reply on an article. Attributes: - comment_id (int): Unique identifier for the comment. - comment_article_id (int): Foreign key to the article. - comment_written_account_id (int): Foreign key to the author's account. - comment_reply_to (int, optional): Foreign key to the parent comment if it's a reply. + comment_id (int): Unique identifier for the comment (Primary Key). + comment_article_id (int): Foreign key referencing the associated article. + comment_written_account_id (int): Foreign key referencing the author's account. + comment_reply_to (int | None): Foreign key referencing a parent comment (for replies). comment_content (str): Text content of the comment. - comment_posted_at (datetime): Timestamp when the comment was posted. - comment_article (relationship): The Article instance this comment belongs to. - comment_author (relationship): The Account instance of the comment's author. - reply_to_comment (relationship): The parent Comment instance if this is a reply. - comment_replies (relationship): List of child Comment instances (replies). + comment_posted_at (datetime): Automated timestamp of when the comment was posted. + comment_article (Article): Relationship to the parent article instance. + comment_author (Account): Relationship to the author's account instance. + reply_to_comment (Comment | None): Relationship to the parent comment if this is a reply. + comment_replies (list[Comment]): Collection of replies (child comments). """ + __tablename__ = "comments" - comment_id = Column(name="comment_id", type_=Integer, primary_key=True, autoincrement=True) - comment_article_id = Column(ForeignKey("articles.article_id", ondelete="CASCADE"), name="comment_article_id", type_=Integer, nullable=False) - comment_written_account_id = Column(ForeignKey("accounts.account_id", ondelete="CASCADE"), name="comment_written_account_id", type_=Integer, nullable=False) - comment_reply_to = Column(ForeignKey("comments.comment_id"), name="comment_reply_to", type_=Integer, nullable=True) - comment_content = Column(name="comment_content", type_=Text, nullable=False) - comment_posted_at = Column(name="comment_posted_at", type_=TIMESTAMP, server_default=func.now()) + comment_id: Mapped[int] = mapped_column("comment_id", Integer, primary_key=True, autoincrement=True) + comment_article_id: Mapped[int] = mapped_column(ForeignKey("articles.article_id", ondelete="CASCADE"), name="comment_article_id", nullable=False) + comment_written_account_id: Mapped[int] = mapped_column(ForeignKey("accounts.account_id", ondelete="CASCADE"), name="comment_written_account_id", nullable=False) + comment_reply_to: Mapped[int | None] = mapped_column(ForeignKey("comments.comment_id"), name="comment_reply_to", nullable=True) + comment_content: Mapped[str] = mapped_column("comment_content", Text, nullable=False) + comment_posted_at: Mapped[datetime] = mapped_column("comment_posted_at", TIMESTAMP, server_default=func.now()) + + comment_article = relationship("Article", back_populates="article_comments") + comment_author = relationship("Account", back_populates="comments") - comment_article = relationship(argument="Article", back_populates="article_comments") - comment_author = relationship(argument="Account", back_populates="comments") reply_to_comment = relationship( - argument="Comment", + "Comment", remote_side=[comment_id], back_populates="comment_replies", uselist=False, ) comment_replies = relationship( - argument="Comment", + "Comment", back_populates="reply_to_comment", cascade="all, delete-orphan", ) diff --git a/app/services/article_service.py b/app/services/article_service.py index 345ccb1..76a7cd2 100644 --- a/app/services/article_service.py +++ b/app/services/article_service.py @@ -1,7 +1,7 @@ -from typing import List, Optional +from collections.abc import Sequence -from sqlalchemy import func, select -from sqlalchemy.orm import Session, defer, joinedload +from sqlalchemy import Row, func, select +from sqlalchemy.orm import Session, defer, joinedload, scoped_session from app.constants import Role from app.models.account_model import Account @@ -14,22 +14,22 @@ class ArticleService: Handles creating, retrieving, updating and deleting articles as well as pagination logic. """ - def __init__(self, session: Session): + def __init__(self, session: Session | scoped_session[Session]): """ Initialize the service with a database session (Dependency Injection). - + Supports both standard Session and scoped_session. + Args: - session (Session): The SQLAlchemy database session to use for queries. + session (Session | scoped_session[Session]): The SQLAlchemy database session. """ self.session = session - def get_all_ordered_by_date(self) -> List[Article]: + def get_all_ordered_by_date(self) -> Sequence[Article]: """ Retrieves all articles ordered by their publication date in descending order. - Eagerly loads author information and defers the loading of the full content for performance. Returns: - List[Article]: A list of Article instances containing metadata. + Sequence[Article]: A sequence of Article instances. """ query = ( select(Article) @@ -39,24 +39,28 @@ def get_all_ordered_by_date(self) -> List[Article]: ) .order_by(Article.article_published_at.desc()) ) - return list(self.session.execute(query).unique().scalars().all()) + return self.session.execute(query).unique().scalars().all() - def get_by_id(self, article_id: int) -> Optional[Article]: + def get_by_id(self, article_id: int) -> Article | None: """ - Retrieves a single article by its ID. Eagerly loads the author information. + Retrieves a single article by its ID. Args: article_id (int): The unique identifier of the article. Returns: - Optional[Article]: The Article instance if found, None otherwise. + Article | None: The Article instance if found, None otherwise. """ - query = select(Article).where(Article.article_id == article_id).options(joinedload(Article.article_author)) + query = ( + select(Article) + .where(Article.article_id == article_id) + .options(joinedload(Article.article_author)) + ) return self.session.execute(query).unique().scalar_one_or_none() def create_article(self, title: str, content: str, author_id: int) -> Article: """ - Creates a new article and adds it to the database session. + Creates a new article instance. Args: title (str): The title of the new article. @@ -66,25 +70,37 @@ def create_article(self, title: str, content: str, author_id: int) -> Article: Returns: Article: The newly created Article instance. """ - new_article = Article(article_title=title, article_content=content, article_author_id=author_id) + new_article = Article( + article_title=title, + article_content=content, + article_author_id=author_id + ) self.session.add(new_article) return new_article - def update_article(self, article_id: int, user_id: int, role: str, title: str, content: str) -> Optional[Article]: + def update_article( + self, + article_id: int, + user_id: int, + role: str, + title: str, + content: str + ) -> Article | None: """ Updates an existing article ensuring the requester is the original author. Args: - article_id (int): The unique identifier of the article to update. - user_id (int): The identifier of the user attempting to update. - role (str): The role of the user. - title (str): The new title of the article. - content (str): The new content of the article. + article_id (int): ID of the article to update. + user_id (int): ID of the user requesting the update. + role (str): Role of the user requesting the update. + title (str): New title for the article. + content (str): New content for the article. Returns: - Optional[Article]: The updated Article instance if successful, None if not found or unauthorized. + Article | None: The updated Article instance or None if unauthorized/not found. """ article = self.get_by_id(article_id) + # Validation logic (Admin role check removed here as per your requirements) if not article or article.article_author_id != user_id: return None @@ -97,30 +113,31 @@ def delete_article(self, article_id: int, user_id: int, role: str) -> bool: Deletes an article. Only the original author or an Admin can delete it. Args: - article_id (int): The unique identifier of the article to delete. - user_id (int): The identifier of the user attempting to delete. - role (str): The role of the user (e.g., 'admin', 'user'). + article_id (int): ID of the article to delete. + user_id (int): ID of the user requesting deletion. + role (str): Role of the user requesting deletion. Returns: - bool: True if the deletion was successful, False if not found or unauthorized. + bool: True if deleted, False otherwise. """ + article = self.get_by_id(article_id) - if not article or (role != Role.ADMIN and article.article_author_id != user_id): + if not article: return False - self.session.delete(article) - return True + # Access control: Author or Admin + if article.article_author_id == user_id or role == Role.ADMIN: + self.session.delete(article) + return True - def get_paginated_articles(self, page: int, per_page: int) -> List[tuple]: - """ - Retrieves a paginated list of articles containing limited metadata. + return False - Args: - page (int): The current page number to retrieve (1-indexed). - per_page (int): The number of articles per page. + def get_paginated_articles(self, page: int, per_page: int) -> Sequence[Row]: + """ + Retrieves a paginated list of articles containing specific columns. Returns: - List[tuple]: A list of tuples containing (id, title, published_at, author_id, username). + Sequence[Row]: A sequence of SQLAlchemy Row objects containing selected columns. """ query = ( select( @@ -135,14 +152,14 @@ def get_paginated_articles(self, page: int, per_page: int) -> List[tuple]: .limit(per_page) .offset((page - 1) * per_page) ) - return list(self.session.execute(query).all()) + return self.session.execute(query).all() def get_total_count(self) -> int: """ - Retrieves the total number of articles in the database. + Retrieves the total number of articles. Returns: - int: The total count of articles. + int: The total count. """ query = select(func.count(Article.article_id)) count = self.session.execute(query).scalar() diff --git a/app/services/comment_service.py b/app/services/comment_service.py index c3c2cee..cd5978b 100644 --- a/app/services/comment_service.py +++ b/app/services/comment_service.py @@ -1,7 +1,7 @@ -from typing import List, Optional +from collections.abc import Sequence from sqlalchemy import select -from sqlalchemy.orm import Session, joinedload +from sqlalchemy.orm import Session, joinedload, scoped_session from app.constants import Role from app.models.article_model import Article @@ -14,16 +14,17 @@ class CommentService: Handles creating top-level comments, replies, retrieving comments by article, and deleting comments. """ - def __init__(self, session: Session): + def __init__(self, session: Session | scoped_session[Session]): """ Initialize the service with a database session (Dependency Injection). - + Supports both standard Session and scoped_session. + Args: - session (Session): The SQLAlchemy database session to use for queries. + session (Session | scoped_session[Session]): The SQLAlchemy database session to use for queries. """ self.session = session - def create_reply(self, parent_comment_id: int, user_id: int, content: str) -> Optional[int]: + def create_reply(self, parent_comment_id: int, user_id: int, content: str) -> int | None: """ Creates a reply to an existing comment. A reply is linked either to the parent directly or to the parent's top-level comment (threading logic). @@ -34,13 +35,14 @@ def create_reply(self, parent_comment_id: int, user_id: int, content: str) -> Op content (str): The text content of the reply. Returns: - Optional[int]: The article ID the comment belongs to if successful, None if the parent comment is not found. + int | None: The article ID the comment belongs to if successful, None if the parent comment is not found. """ parent = self.session.get(Comment, parent_comment_id) if not parent: return None actual_parent_id = parent.comment_reply_to if parent.comment_reply_to else parent.comment_id + new_reply = Comment( comment_article_id=parent.comment_article_id, comment_written_account_id=user_id, @@ -50,7 +52,7 @@ def create_reply(self, parent_comment_id: int, user_id: int, content: str) -> Op self.session.add(new_reply) return parent.comment_article_id - def get_by_id(self, comment_id: int) -> Optional[Comment]: + def get_by_id(self, comment_id: int) -> Comment | None: """ Retrieves a single comment by its ID. Eagerly loads the author information. @@ -58,9 +60,13 @@ def get_by_id(self, comment_id: int) -> Optional[Comment]: comment_id (int): The unique identifier of the comment. Returns: - Optional[Comment]: The Comment instance if found, None otherwise. + Comment | None: The Comment instance if found, None otherwise. """ - query = select(Comment).options(joinedload(Comment.comment_author)).where(Comment.comment_id == comment_id) + query = ( + select(Comment) + .options(joinedload(Comment.comment_author)) + .where(Comment.comment_id == comment_id) + ) return self.session.execute(query).unique().scalar_one_or_none() def create_comment(self, article_id: int, user_id: int, content: str) -> bool: @@ -78,7 +84,7 @@ def create_comment(self, article_id: int, user_id: int, content: str) -> bool: article = self.session.get(Article, article_id) if not article: return False - + new_comment = Comment( comment_article_id=article_id, comment_written_account_id=user_id, @@ -87,7 +93,7 @@ def create_comment(self, article_id: int, user_id: int, content: str) -> bool: self.session.add(new_comment) return True - def delete_comment(self, comment_id: int, role: str) -> Optional[int]: + def delete_comment(self, comment_id: int, role: str) -> int | None: """ Deletes a comment. Only users with the 'admin' role can delete comments. @@ -96,30 +102,29 @@ def delete_comment(self, comment_id: int, role: str) -> Optional[int]: role (str): The role of the user attempting the deletion. Returns: - Optional[int]: The article ID the comment belonged to if successful, None if unauthorized or not found. + int | None: The article ID the comment belonged to if successful, None if unauthorized or not found. """ if role != Role.ADMIN: return None - + comment = self.session.get(Comment, comment_id) if not comment: return None - + article_id = comment.comment_article_id self.session.delete(comment) return article_id - def get_tree_by_article_id(self, article_id: int) -> List[Comment]: + def get_tree_by_article_id(self, article_id: int) -> Sequence[Comment]: """ Retrieves all comments for a specific article as a threaded tree structure. - Eagerly loads author information and returns only the top-level comments; - replies are typically nested within the 'replies' relationship of the parent comment. + Returns only the top-level comments. Args: article_id (int): The ID of the article. Returns: - List[Comment]: A list of top-level Comment instances for the given article. + Sequence[Comment]: A sequence of top-level Comment instances for the given article. """ query = ( select(Comment) @@ -128,4 +133,5 @@ def get_tree_by_article_id(self, article_id: int) -> List[Comment]: .order_by(Comment.comment_posted_at.asc()) ) all_comments = self.session.execute(query).unique().scalars().all() + return [c for c in all_comments if c.comment_reply_to is None] diff --git a/app/services/login_service.py b/app/services/login_service.py index a0ff5cc..bc1c5ad 100644 --- a/app/services/login_service.py +++ b/app/services/login_service.py @@ -1,7 +1,5 @@ -from typing import Optional - from sqlalchemy import select -from sqlalchemy.orm import Session +from sqlalchemy.orm import Session, scoped_session from app.models.account_model import Account @@ -11,30 +9,31 @@ class LoginService: Service class responsible for handling user authentication logic. """ - def __init__(self, session: Session): + def __init__(self, session: Session | scoped_session[Session]): """ Initialize the service with a database session (Dependency Injection). - + Supports both standard Session and scoped_session proxy objects. + Args: - session (Session): The SQLAlchemy database session to use for queries. + session (Session | scoped_session[Session]): The SQLAlchemy database session to use for queries. """ self.session = session - def authenticate_user(self, username: str, password: str) -> Optional[Account]: + def authenticate_user(self, username: str, password: str) -> Account | None: """ Validates the user's credentials against the database. - + Args: username (str): The username provided by the user. password (str): The plaintext password provided by the user. Returns: - Optional[Account]: The authenticated Account instance if credentials match, None otherwise. + Account | None: The authenticated Account instance if credentials match, None otherwise. """ query = select(Account).where(Account.account_username == username) user = self.session.execute(query).scalar_one_or_none() if user and user.account_password == password: return user - + return None diff --git a/poetry.lock b/poetry.lock index 521da8d..d3967d2 100644 --- a/poetry.lock +++ b/poetry.lock @@ -455,6 +455,18 @@ files = [ {file = "mypy_extensions-1.1.0.tar.gz", hash = "sha256:52e68efc3284861e772bbcd66823fde5ae21fd2fdb51c62a211403730b916558"}, ] +[[package]] +name = "nodeenv" +version = "1.10.0" +description = "Node.js virtual environment builder" +optional = false +python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*,!=3.6.*,>=2.7" +groups = ["dev"] +files = [ + {file = "nodeenv-1.10.0-py2.py3-none-any.whl", hash = "sha256:5bb13e3eed2923615535339b3c620e76779af4cb4c6a90deccc9e36b274d3827"}, + {file = "nodeenv-1.10.0.tar.gz", hash = "sha256:996c191ad80897d076bdfba80a41994c2b47c68e224c542b48feba42ba00f8bb"}, +] + [[package]] name = "packaging" version = "25.0" @@ -737,6 +749,27 @@ files = [ [package.extras] windows-terminal = ["colorama (>=0.4.6)"] +[[package]] +name = "pyright" +version = "1.1.408" +description = "Command line wrapper for pyright" +optional = false +python-versions = ">=3.7" +groups = ["dev"] +files = [ + {file = "pyright-1.1.408-py3-none-any.whl", hash = "sha256:090b32865f4fdb1e0e6cd82bf5618480d48eecd2eb2e70f960982a3d9a4c17c1"}, + {file = "pyright-1.1.408.tar.gz", hash = "sha256:f28f2321f96852fa50b5829ea492f6adb0e6954568d1caa3f3af3a5f555eb684"}, +] + +[package.dependencies] +nodeenv = ">=1.6.0" +typing-extensions = ">=4.1" + +[package.extras] +all = ["nodejs-wheel-binaries", "twine (>=3.4.1)"] +dev = ["twine (>=3.4.1)"] +nodejs = ["nodejs-wheel-binaries"] + [[package]] name = "pytest" version = "9.0.0" @@ -942,7 +975,7 @@ version = "4.15.0" description = "Backported and Experimental Type Hints for Python 3.9+" optional = false python-versions = ">=3.9" -groups = ["main"] +groups = ["main", "dev"] files = [ {file = "typing_extensions-4.15.0-py3-none-any.whl", hash = "sha256:f0fa19c6845758ab08074a0cfa8b7aecb71c999ca73d62883bc25cc018c4e548"}, {file = "typing_extensions-4.15.0.tar.gz", hash = "sha256:0cea48d173cc12fa28ecabc3b837ea3cf6f38c6d1136f85cbaaf598984861466"}, diff --git a/pyproject.toml b/pyproject.toml index 16dafe5..41a49f9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,8 @@ dev = [ "flake8 (>=7.3.0,<8.0.0)", "pytest (>=9.0.0,<10.0.0)", "pytest-flask (>=1.3.0,<2.0.0)", - "ruff (>=0.14.5,<0.15.0)" + "ruff (>=0.14.5,<0.15.0)", + "pyright (>=1.1.408,<2.0.0)" ] [tool.ruff] diff --git a/tests/conftest.py b/tests/conftest.py index a21630a..7c19d36 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,11 +1,12 @@ -from typing import Any, Generator +from collections.abc import Generator +from typing import Any import pytest from flask import Flask from flask.testing import FlaskClient from sqlalchemy import text from sqlalchemy.engine import Connection -from sqlalchemy.orm import Session +from sqlalchemy.orm import Session, scoped_session from app import initialize_flask_application from app.models.account_model import Account @@ -60,7 +61,7 @@ def client(app: Flask) -> FlaskClient: @pytest.fixture(scope="function") -def db_session(app: Flask) -> Generator[Session, Any, None]: +def db_session(app) -> Generator[scoped_session[Session], None, None]: """ Pytest fixture that provides a clean database session for each test function. Truncates all tables before yielding the session. @@ -73,7 +74,7 @@ def db_session(app: Flask) -> Generator[Session, Any, None]: """ # Explicitly referencing models to satisfy linters and ensure metadata is populated _ = (Account, Article, Comment) - + if database_engine.url.render_as_string(hide_password=False) != env_vars.test_database_url: pytest.exit("SECURITY ERROR: The current database URL does not match the configured TEST database URL.") diff --git a/tests/test_models/test_account_model.py b/tests/test_models/test_account_model.py index 25d783d..24c41cc 100644 --- a/tests/test_models/test_account_model.py +++ b/tests/test_models/test_account_model.py @@ -1,7 +1,8 @@ from datetime import datetime +from typing import cast import pytest -import sqlalchemy +from sqlalchemy import exc from app.models.account_model import Account from tests.factories import make_account @@ -23,19 +24,20 @@ def test_account_username_unique(db_session): db_session.commit() db_session.add(make_account(account_username="unique")) - with pytest.raises(sqlalchemy.exc.IntegrityError): + with pytest.raises(exc.IntegrityError): db_session.commit() def test_account_missing_username(db_session): - account = make_account(account_username=None) + # We intentionally pass None to test database constraints + account = make_account(account_username=cast(str, None)) db_session.add(account) - with pytest.raises(sqlalchemy.exc.IntegrityError): + with pytest.raises(exc.IntegrityError): db_session.commit() def test_account_role_invalid(db_session): account = make_account(account_role="super_admin") db_session.add(account) - with pytest.raises(sqlalchemy.exc.IntegrityError): + with pytest.raises(exc.IntegrityError): db_session.commit() diff --git a/tests/test_models/test_article_model.py b/tests/test_models/test_article_model.py index 7c750e9..10c6ac7 100644 --- a/tests/test_models/test_article_model.py +++ b/tests/test_models/test_article_model.py @@ -1,5 +1,7 @@ +from typing import cast + import pytest -import sqlalchemy +from sqlalchemy import exc from app.models.article_model import Article from tests.factories import make_account, make_article @@ -25,9 +27,10 @@ def test_article_missing_title(db_session): author = make_account() db_session.add(author) db_session.commit() - article = make_article(article_author_id=author.account_id, article_title=None) + # We intentionally pass None to test database constraints + article = make_article(article_author_id=author.account_id, article_title=cast(str, None)) db_session.add(article) - with pytest.raises(sqlalchemy.exc.IntegrityError): + with pytest.raises(exc.IntegrityError): db_session.commit() @@ -36,9 +39,10 @@ def test_article_missing_content(db_session): db_session.add(author) db_session.commit() - article = make_article(article_author_id=author.account_id, article_content=None) + # We intentionally pass None to test database constraints + article = make_article(article_author_id=author.account_id, article_content=cast(str, None)) db_session.add(article) - with pytest.raises(sqlalchemy.exc.IntegrityError): + with pytest.raises(exc.IntegrityError): db_session.commit() From f6f914df8414eb47c34479ef45a77661f2045693 Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 05:44:10 +0100 Subject: [PATCH 06/11] Global code improvement : Fix Ruff issues --- app/__init__.py | 6 +++--- app/controllers/article_controller.py | 13 ++++++------- app/services/article_service.py | 6 ++---- app/utils/decorators.py | 11 ++++++----- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index d726388..531c352 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,6 +1,5 @@ import os import sys -from typing import Optional from flask import Flask @@ -11,13 +10,14 @@ from database.database_setup import db_session -def shutdown_session(exception: Optional[BaseException] = None) -> None: +def shutdown_session(exception: BaseException | None = None) -> None: """ Removes the database session at the end of the request. Args: - exception (Optional[BaseException]): The exception that triggered the teardown, if any. + exception (BaseException | None): The exception that triggered the teardown, if any. """ + db_session.remove() diff --git a/app/controllers/article_controller.py b/app/controllers/article_controller.py index 0e2c456..10066f5 100644 --- a/app/controllers/article_controller.py +++ b/app/controllers/article_controller.py @@ -1,5 +1,4 @@ import math -from typing import Union from flask import ( Blueprint, @@ -46,7 +45,7 @@ def list_articles() -> str: @article_bp.route("/article/") -def view_article(article_id: int) -> Union[str, Response]: +def view_article(article_id: int) -> str | Response: """ Displays the details of a specific article and its comments. @@ -54,7 +53,7 @@ def view_article(article_id: int) -> Union[str, Response]: article_id (int): ID of the article to view. Returns: - Union[str, Response]: The rendered HTML template for the article or a redirect if the article is not found. + str | Response: The rendered HTML template for the article or a redirect if the article is not found. """ article_service = ArticleService(db_session) article = article_service.get_by_id(article_id) @@ -69,13 +68,13 @@ def view_article(article_id: int) -> Union[str, Response]: @article_bp.route("/article/new", methods=["GET", "POST"]) @roles_accepted(Role.ADMIN, Role.AUTHOR) -def create_article() -> Union[str, Response]: +def create_article() -> str | Response: """ Handles the creation of a new blog article. Restricted to 'admin' and 'author' roles. Returns: - Union[str, Response]: The rendered HTML form (GET) or a redirect to the article list after creation (POST). + str | Response: The rendered HTML form (GET) or a redirect to the article list after creation (POST). """ if request.method == "POST": article_service = ArticleService(db_session) @@ -97,7 +96,7 @@ def create_article() -> Union[str, Response]: @article_bp.route("/article//edit", methods=["GET", "POST"]) @roles_accepted(Role.ADMIN, Role.AUTHOR, Role.USER) -def edit_article(article_id: int) -> Union[str, Response]: +def edit_article(article_id: int) -> str | Response: """ Handles the editing of an existing article. Ensures the user is authorized to perform the update. @@ -106,7 +105,7 @@ def edit_article(article_id: int) -> Union[str, Response]: article_id (int): ID of the article to edit. Returns: - Union[str, Response]: The rendered HTML form (GET) or a redirect to the updated article (POST). + str | Response: The rendered HTML form (GET) or a redirect to the updated article (POST). """ article_service = ArticleService(db_session) diff --git a/app/services/article_service.py b/app/services/article_service.py index 76a7cd2..d9f997c 100644 --- a/app/services/article_service.py +++ b/app/services/article_service.py @@ -21,7 +21,7 @@ def __init__(self, session: Session | scoped_session[Session]): Args: session (Session | scoped_session[Session]): The SQLAlchemy database session. - """ + """ self.session = session def get_all_ordered_by_date(self) -> Sequence[Article]: @@ -97,10 +97,9 @@ def update_article( content (str): New content for the article. Returns: - Article | None: The updated Article instance or None if unauthorized/not found. + Article | None: The Article instance or None if unauthorized/not found. """ article = self.get_by_id(article_id) - # Validation logic (Admin role check removed here as per your requirements) if not article or article.article_author_id != user_id: return None @@ -125,7 +124,6 @@ def delete_article(self, article_id: int, user_id: int, role: str) -> bool: if not article: return False - # Access control: Author or Admin if article.article_author_id == user_id or role == Role.ADMIN: self.session.delete(article) return True diff --git a/app/utils/decorators.py b/app/utils/decorators.py index b45348e..5d2c222 100644 --- a/app/utils/decorators.py +++ b/app/utils/decorators.py @@ -1,5 +1,6 @@ +from collections.abc import Callable from functools import wraps -from typing import Any, Callable +from typing import Any from flask import flash, redirect, session, url_for @@ -9,10 +10,10 @@ def login_required(f: Callable[..., Any]) -> Callable[..., Any]: """ Decorator to ensure that a user is logged in before accessing a route. - + Args: f (Callable): The route function to wrap. - + Returns: Callable: The wrapped function. """ @@ -28,10 +29,10 @@ def decorated_function(*args: Any, **kwargs: Any) -> Any: def roles_accepted(*roles: Role) -> Callable[..., Any]: """ Decorator to ensure that a logged-in user has one of the required roles. - + Args: *roles (Role): Variable list of accepted roles. - + Returns: Callable: A decorator that wraps the route function. """ From 278e615415963b33e29464541573a8d357088bd7 Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 06:05:52 +0100 Subject: [PATCH 07/11] Global code improvement : Add Pyright test step to GitHub Action workflow --- .github/workflows/continuous-integration.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 690cd42..95ea855 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -75,5 +75,8 @@ jobs: - name: Run tests run: poetry run pytest + - name: Run type check + run: poetry run pyright + - name: Run lint run: poetry run ruff check . From f78874bf5e13ba1e62e2bd76c72f8510ad4ce831 Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 06:06:31 +0100 Subject: [PATCH 08/11] Global code improvement : Remove unnecessary comments --- tests/test_models/test_account_model.py | 1 - tests/test_models/test_article_model.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/tests/test_models/test_account_model.py b/tests/test_models/test_account_model.py index 24c41cc..0d9e54b 100644 --- a/tests/test_models/test_account_model.py +++ b/tests/test_models/test_account_model.py @@ -29,7 +29,6 @@ def test_account_username_unique(db_session): def test_account_missing_username(db_session): - # We intentionally pass None to test database constraints account = make_account(account_username=cast(str, None)) db_session.add(account) with pytest.raises(exc.IntegrityError): diff --git a/tests/test_models/test_article_model.py b/tests/test_models/test_article_model.py index 10c6ac7..eb3c885 100644 --- a/tests/test_models/test_article_model.py +++ b/tests/test_models/test_article_model.py @@ -27,7 +27,6 @@ def test_article_missing_title(db_session): author = make_account() db_session.add(author) db_session.commit() - # We intentionally pass None to test database constraints article = make_article(article_author_id=author.account_id, article_title=cast(str, None)) db_session.add(article) with pytest.raises(exc.IntegrityError): @@ -39,7 +38,6 @@ def test_article_missing_content(db_session): db_session.add(author) db_session.commit() - # We intentionally pass None to test database constraints article = make_article(article_author_id=author.account_id, article_content=cast(str, None)) db_session.add(article) with pytest.raises(exc.IntegrityError): From 6d5144bb5edb2dfe488785cb5975d47fe52cfef6 Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sat, 28 Feb 2026 06:08:41 +0100 Subject: [PATCH 09/11] Global code improvement : Add missing __init__.py --- app/utils/__init__.py | 0 tests/test_models/__init__.py | 0 tests/test_services/__init__.py | 0 3 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 app/utils/__init__.py create mode 100644 tests/test_models/__init__.py create mode 100644 tests/test_services/__init__.py diff --git a/app/utils/__init__.py b/app/utils/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_models/__init__.py b/tests/test_models/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_services/__init__.py b/tests/test_services/__init__.py new file mode 100644 index 0000000..e69de29 From 47648547f5e076b036177055e45571d111d524fb Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sun, 1 Mar 2026 01:01:47 +0100 Subject: [PATCH 10/11] Global code improvement : Move decorators into controllers because they are handled there --- app/controllers/article_controller.py | 2 +- app/controllers/comment_controller.py | 2 +- app/{utils => controllers}/decorators.py | 5 +++++ app/utils/__init__.py | 0 4 files changed, 7 insertions(+), 2 deletions(-) rename app/{utils => controllers}/decorators.py (99%) delete mode 100644 app/utils/__init__.py diff --git a/app/controllers/article_controller.py b/app/controllers/article_controller.py index 10066f5..249b372 100644 --- a/app/controllers/article_controller.py +++ b/app/controllers/article_controller.py @@ -12,9 +12,9 @@ from werkzeug.wrappers import Response from app.constants import PaginationConfig, Role, SessionKey +from app.controllers.decorators import roles_accepted from app.services.article_service import ArticleService from app.services.comment_service import CommentService -from app.utils.decorators import roles_accepted from database.database_setup import db_session article_bp = Blueprint("article", __name__) diff --git a/app/controllers/comment_controller.py b/app/controllers/comment_controller.py index 2000a7f..6b491a9 100644 --- a/app/controllers/comment_controller.py +++ b/app/controllers/comment_controller.py @@ -2,8 +2,8 @@ from werkzeug.wrappers import Response from app.constants import Role, SessionKey +from app.controllers.decorators import login_required, roles_accepted from app.services.comment_service import CommentService -from app.utils.decorators import login_required, roles_accepted from database.database_setup import db_session comment_bp = Blueprint("comment", __name__, url_prefix="/comments") diff --git a/app/utils/decorators.py b/app/controllers/decorators.py similarity index 99% rename from app/utils/decorators.py rename to app/controllers/decorators.py index 5d2c222..03adf42 100644 --- a/app/utils/decorators.py +++ b/app/controllers/decorators.py @@ -17,12 +17,14 @@ def login_required(f: Callable[..., Any]) -> Callable[..., Any]: Returns: Callable: The wrapped function. """ + @wraps(f) def decorated_function(*args: Any, **kwargs: Any) -> Any: if not session.get(SessionKey.USER_ID): flash("Login required.") return redirect(url_for("login.render_login_page")) return f(*args, **kwargs) + return decorated_function @@ -36,6 +38,7 @@ def roles_accepted(*roles: Role) -> Callable[..., Any]: Returns: Callable: A decorator that wraps the route function. """ + def decorator(f: Callable[..., Any]) -> Callable[..., Any]: @wraps(f) @login_required @@ -45,5 +48,7 @@ def decorated_function(*args: Any, **kwargs: Any) -> Any: flash("Access restricted: Insufficient permissions.") return redirect(url_for("article.list_articles")) return f(*args, **kwargs) + return decorated_function + return decorator diff --git a/app/utils/__init__.py b/app/utils/__init__.py deleted file mode 100644 index e69de29..0000000 From b95437fb1ef95fc161f20d586df31f1eea96f588 Mon Sep 17 00:00:00 2001 From: raydeveloppeur-admin Date: Sun, 1 Mar 2026 01:03:23 +0100 Subject: [PATCH 11/11] Global code improvement : Add poetry.toml to .gitignore and reformat parts of the code using Ruff --- .gitignore | 3 +++ config/configuration_variables.py | 3 +++ tests/test_controllers/test_comment_controller.py | 7 ++++++- tests/test_controllers/test_login_controller.py | 6 +++++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index ccc9588..04f0eb1 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,6 @@ __pycache__/ .vscode/ .idea/ .DS_Store + +# 📦 Poetry (config locale) +poetry.toml diff --git a/config/configuration_variables.py b/config/configuration_variables.py index f6b3b96..41e7425 100644 --- a/config/configuration_variables.py +++ b/config/configuration_variables.py @@ -26,10 +26,12 @@ def get_env_variable(name: str) -> str: raise RuntimeError(f"Missing mandatory environment variable: '{name}'") return value + class EnvVariablesConfig: """ Configuration class to manage environment variables for the application. """ + @property def database_url(self) -> str: """ @@ -70,4 +72,5 @@ def test_secret_key(self) -> str: """ return get_env_variable("TEST_SECRET_KEY") + env_vars = EnvVariablesConfig() diff --git a/tests/test_controllers/test_comment_controller.py b/tests/test_controllers/test_comment_controller.py index 41c5963..4cc9b4b 100644 --- a/tests/test_controllers/test_comment_controller.py +++ b/tests/test_controllers/test_comment_controller.py @@ -9,6 +9,7 @@ def test_create_comment_unauthorized(client): response = client.post("/comments/create/1", data={"content": "Test"}, follow_redirects=True) assert b"Login required." in response.data + def test_create_comment_success(client, db_session): user = make_account() db_session.add(user) @@ -26,6 +27,7 @@ def test_create_comment_success(client, db_session): comment = db_session.query(Comment).filter_by(comment_content="Mon super commentaire").first() assert comment is not None + def test_create_comment_on_invalid_article(client, db_session): user = make_account() db_session.add(user) @@ -36,6 +38,7 @@ def test_create_comment_on_invalid_article(client, db_session): response = client.post("/comments/create/9999", data={"content": "Error"}, follow_redirects=True) assert b"Error adding comment." in response.data + def test_create_reply_success(client, db_session): user = make_account() db_session.add(user) @@ -57,6 +60,7 @@ def test_create_reply_success(client, db_session): assert reply.comment_reply_to == parent.comment_id assert reply.comment_article_id == article.article_id + def test_create_reply_atomicity_failure(client, db_session): user = make_account() db_session.add(user) @@ -82,6 +86,7 @@ def test_create_reply_atomicity_failure(client, db_session): reply = db_session.query(Comment).filter_by(comment_content="My answer").first() assert reply is None + def test_delete_comment_admin_only(client, db_session): admin = make_account(account_username="Admin", account_role=Role.ADMIN) user = make_account(account_username="User", account_role=Role.USER) @@ -98,7 +103,6 @@ def test_delete_comment_admin_only(client, db_session): sess[SessionKey.USER_ID] = user.account_id sess[SessionKey.ROLE] = Role.USER response = client.get(f"/comments/delete/{comment.comment_id}", follow_redirects=True) - # The roles_accepted decorator flashes "Access restricted: Insufficient permissions." assert b"Access restricted" in response.data or b"Insufficient permissions" in response.data with client.session_transaction() as sess: @@ -107,6 +111,7 @@ def test_delete_comment_admin_only(client, db_session): response = client.get(f"/comments/delete/{comment.comment_id}", follow_redirects=True) assert b"Comment deleted." in response.data + def test_reply_to_non_existent_comment(client, db_session): user = make_account() db_session.add(user) diff --git a/tests/test_controllers/test_login_controller.py b/tests/test_controllers/test_login_controller.py index 30df83f..53f829e 100644 --- a/tests/test_controllers/test_login_controller.py +++ b/tests/test_controllers/test_login_controller.py @@ -7,10 +7,12 @@ def test_render_login_page(client): assert response.status_code == 200 assert b'action="/login"' in response.data + def test_login_method_not_allowed(client): response = client.get("/login") assert response.status_code == 405 + def test_login_success(client, db_session): user = make_account(account_username="Vador", account_password="dark_password") db_session.add(user) @@ -22,6 +24,7 @@ def test_login_success(client, db_session): assert session[SessionKey.USERNAME] == "Vador" assert session[SessionKey.ROLE] == Role.USER + def test_login_failure_wrong_password(client, db_session): user = make_account(account_username="Luke", account_password="correct_password") db_session.add(user) @@ -31,10 +34,12 @@ def test_login_failure_wrong_password(client, db_session): with client.session_transaction() as session: assert SessionKey.USER_ID not in session + def test_login_non_existent_user(client): response = client.post("/login", data={"username": "Inconnu", "password": "password"}, follow_redirects=True) assert b"Incorrect credentials." in response.data + def test_logout(client): with client.session_transaction() as session: session[SessionKey.USER_ID] = 1 @@ -44,4 +49,3 @@ def test_logout(client): with client.session_transaction() as session: assert SessionKey.USER_ID not in session assert SessionKey.USERNAME not in session -