From 5b95262681fe85923b3c2c249c758f1be0670348 Mon Sep 17 00:00:00 2001 From: david Date: Fri, 1 May 2026 19:21:28 +0300 Subject: [PATCH] refactor active record usage --- .github/workflows/backend.yml | 2 - backend/app/decorators/decorators.py | 32 +++++---- backend/app/decorators/owned.py | 26 +++---- backend/app/models/base.py | 14 ++++ backend/app/models/board.py | 20 ++++-- backend/app/models/card.py | 29 ++++++-- backend/app/models/checklist.py | 9 ++- backend/app/models/list_model.py | 8 ++- backend/app/routes/kanban/boards.py | 52 +++++--------- frontend/src/components/CardEpics.tsx | 2 +- frontend/src/components/CardLinks.tsx | 31 ++++++++- .../src/components/CreateLinkedCardModal.tsx | 57 ++++++++++------ .../src/components/LinkExistingCardModal.tsx | 57 ++-------------- frontend/src/components/UnlinkCardModal.tsx | 40 +++++++++++ frontend/src/hooks/useDocumentTitle.ts | 16 +++++ frontend/src/hooks/useLinkExistingCard.ts | 68 +++++++++++++++++++ frontend/src/pages/BoardDetail.tsx | 2 + frontend/src/pages/CardDetail.tsx | 2 + frontend/src/pages/EpicDetail.tsx | 2 + frontend/src/pages/WikiDetail.tsx | 2 + frontend/src/types/index.ts | 6 -- 21 files changed, 319 insertions(+), 158 deletions(-) create mode 100644 frontend/src/components/UnlinkCardModal.tsx create mode 100644 frontend/src/hooks/useDocumentTitle.ts create mode 100644 frontend/src/hooks/useLinkExistingCard.ts diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml index b2e24a6..8e02de7 100644 --- a/.github/workflows/backend.yml +++ b/.github/workflows/backend.yml @@ -69,7 +69,6 @@ jobs: print(f'Created database: {new_db}') " - - name: Debug cache run: | echo "Listing PIP cache files:" @@ -83,7 +82,6 @@ jobs: - name: Run tests env: - UNIQUE_DB: test_db_${{ github.run_id }} TEST_DATABASE_URL: postgresql://test:test@postgres:5432/${{ env.UNIQUE_DB }} DATABASE_URL: postgresql://test:test@postgres:5432/${{ env.UNIQUE_DB }} SECRET_KEY: test-secret-key diff --git a/backend/app/decorators/decorators.py b/backend/app/decorators/decorators.py index 8c7eec2..ea6a032 100644 --- a/backend/app/decorators/decorators.py +++ b/backend/app/decorators/decorators.py @@ -29,16 +29,17 @@ def load_file_accessible(f): user_id = get_current_user_id() file_id = kwargs.get("file_id") - # Try to find file uploaded by user - attachment = FileAttachment.query.filter_by( - id=file_id, uploaded_by=user_id - ).first() + # Try to find active file uploaded by user + attachment = ( + FileAttachment.active().filter_by(id=file_id, uploaded_by=user_id).first() + ) # If not found, check if attached to a Card that belongs to user's board if not attachment: - # For Card attachments + # For Card attachments (only active files) card_attachment = ( - FileAttachment.query.join( + FileAttachment.active() + .join( Card, (FileAttachment.attachable_type == "Card") & (FileAttachment.attachable_id == Card.id), @@ -56,9 +57,10 @@ def load_file_accessible(f): # If still not found, check if attached # to a Comment that belongs to user's board if not attachment: - # For Comment attachments + # For Comment attachments (only active files) comment_attachment = ( - FileAttachment.query.join( + FileAttachment.active() + .join( Comment, (FileAttachment.attachable_type == "Comment") & (FileAttachment.attachable_id == Comment.id), @@ -98,10 +100,10 @@ def load_file_accessible_by_uuid(f): user_id = get_current_user_id() file_uuid = kwargs.get("file_uuid") - # Try to find file uploaded by user (only active files) + # Try to find active file uploaded by user attachment = ( - FileAttachment.query.filter_by(uuid=file_uuid, uploaded_by=user_id) - .filter(FileAttachment.status == "active") + FileAttachment.active() + .filter_by(uuid=file_uuid, uploaded_by=user_id) .first() ) @@ -109,7 +111,8 @@ def load_file_accessible_by_uuid(f): if not attachment: # For Card attachments (only active files) card_attachment = ( - FileAttachment.query.join( + FileAttachment.active() + .join( Card, (FileAttachment.attachable_type == "Card") & (FileAttachment.attachable_id == Card.id), @@ -118,7 +121,6 @@ def load_file_accessible_by_uuid(f): .filter( FileAttachment.uuid == file_uuid, Board.user_id == user_id, - FileAttachment.status == "active", ) .first() ) @@ -130,7 +132,8 @@ def load_file_accessible_by_uuid(f): if not attachment: # For Comment attachments (only active files) comment_attachment = ( - FileAttachment.query.join( + FileAttachment.active() + .join( Comment, (FileAttachment.attachable_type == "Comment") & (FileAttachment.attachable_id == Comment.id), @@ -140,7 +143,6 @@ def load_file_accessible_by_uuid(f): .filter( FileAttachment.uuid == file_uuid, Board.user_id == user_id, - FileAttachment.status == "active", ) .first() ) diff --git a/backend/app/decorators/owned.py b/backend/app/decorators/owned.py index d9ca106..d171a8b 100644 --- a/backend/app/decorators/owned.py +++ b/backend/app/decorators/owned.py @@ -20,7 +20,7 @@ def load_board_owned(f): board_id = kwargs.get("board_id") # SECURE QUERY: Filter by ID *and* User ID in the DB - board = Board.query.filter_by(id=board_id, user_id=user_id).first() + board = Board.active().filter_by(id=board_id, user_id=user_id).first() if not board: abort(404) @@ -45,10 +45,9 @@ def load_card_owned(f): # Join Board to check ownership and filter soft-deleted cards card = ( - Card.query.join(Board) - .filter( - Card.id == card_id, Board.user_id == user_id, Card.deleted_at.is_(None) - ) + Card.active() + .join(Board) + .filter(Card.id == card_id, Board.user_id == user_id) .first() ) @@ -70,7 +69,8 @@ def load_list_owned(f): list_id = kwargs.get("list_id") lst = ( - List.query.join(Board) + List.active() + .join(Board) .filter(List.id == list_id, Board.user_id == user_id) .first() ) @@ -93,7 +93,8 @@ def load_checklist_owned(f): checklist_id = kwargs.get("checklist_id") checklist = ( - Checklist.query.join(Card) + Checklist.active() + .join(Card) .join(Board) .filter(Checklist.id == checklist_id, Board.user_id == user_id) .first() @@ -117,7 +118,8 @@ def load_check_item_owned(f): item_id = kwargs.get("item_id") check_item = ( - CheckItem.query.join(Checklist) + CheckItem.active() + .join(Checklist) .join(Card) .join(Board) .filter(CheckItem.id == item_id, Board.user_id == user_id) @@ -144,7 +146,7 @@ def load_comment_owned(f): user_id = get_current_user_id() comment_id = kwargs.get("comment_id") - comment = Comment.query.filter_by(id=comment_id, user_id=user_id).first() + comment = Comment.active().filter_by(id=comment_id, user_id=user_id).first() if not comment: abort(404) @@ -167,9 +169,9 @@ def load_file_owned(f): file_id = kwargs.get("file_id") # Filter by ID and user ID - attachment = FileAttachment.query.filter_by( - id=file_id, uploaded_by=user_id - ).first() + attachment = ( + FileAttachment.active().filter_by(id=file_id, uploaded_by=user_id).first() + ) if not attachment: abort(404) diff --git a/backend/app/models/base.py b/backend/app/models/base.py index 6b03c1b..38bc946 100644 --- a/backend/app/models/base.py +++ b/backend/app/models/base.py @@ -25,6 +25,15 @@ class SoftDeleteMixin: # Restore instance.restore() db.session.commit() + + For relationship filtering: + class Parent(db.Model, SoftDeleteMixin): + children = db.relationship( + "Child", + primaryjoin="and_(Parent.id == Child.parent_id, " + "Child.status == 'active')", + ... + ) """ STATUS_ACTIVE = "active" @@ -48,6 +57,11 @@ class SoftDeleteMixin: """ return cls.query.filter(cls.status == cls.STATUS_ACTIVE) + @property + def is_active(self): + """Check if this record is active (not soft-deleted).""" + return self.status == self.STATUS_ACTIVE + def soft_delete(self): """Mark this record as deleted and cascade to child relationships. diff --git a/backend/app/models/board.py b/backend/app/models/board.py index d2faa8a..a654cf7 100644 --- a/backend/app/models/board.py +++ b/backend/app/models/board.py @@ -42,15 +42,27 @@ class Board(db.Model, SoftDeleteMixin): label_names = db.Column(JSONB) # label color mappings limits = db.Column(JSONB) # various limits - # Relationships + # Relationships - only active records lists = db.relationship( - "List", backref="board", cascade="all, delete-orphan", lazy="dynamic" + "List", + backref="board", + cascade="all, delete-orphan", + lazy="dynamic", + primaryjoin="and_(Board.id == List.board_id, List.status == 'active')", ) cards = db.relationship( - "Card", backref="board", cascade="all, delete-orphan", lazy="dynamic" + "Card", + backref="board", + cascade="all, delete-orphan", + lazy="dynamic", + primaryjoin="and_(Board.id == Card.board_id, Card.status == 'active')", ) labels = db.relationship( - "Label", backref="board", cascade="all, delete-orphan", lazy="dynamic" + "Label", + backref="board", + cascade="all, delete-orphan", + lazy="dynamic", + primaryjoin="and_(Board.id == Label.board_id, Label.status == 'active')", ) def to_dict(self): diff --git a/backend/app/models/card.py b/backend/app/models/card.py index f7d5477..6297d3e 100644 --- a/backend/app/models/card.py +++ b/backend/app/models/card.py @@ -51,32 +51,47 @@ class Card(db.Model, SoftDeleteMixin): cover = db.Column(JSONB) # cover settings desc_data = db.Column(JSONB) - # Relationships + # Relationships - only active records checklists = db.relationship( - "Checklist", backref="card", cascade="all, delete-orphan", lazy="dynamic" + "Checklist", + backref="card", + cascade="all, delete-orphan", + lazy="dynamic", + primaryjoin="and_(Card.id == Checklist.card_id, Checklist.status == 'active')", ) labels = db.relationship( - "CardLabel", backref="card", cascade="all, delete-orphan", lazy="dynamic" + "CardLabel", + backref="card", + cascade="all, delete-orphan", + lazy="dynamic", + primaryjoin="and_(Card.id == CardLabel.card_id, CardLabel.status == 'active')", ) comments = db.relationship( - "Comment", backref="card", cascade="all, delete-orphan", lazy="dynamic" + "Comment", + backref="card", + cascade="all, delete-orphan", + lazy="dynamic", + primaryjoin="and_(Card.id == Comment.card_id, Comment.status == 'active')", ) attachments = db.relationship( "FileAttachment", foreign_keys="FileAttachment.attachable_id", primaryjoin="""and_(FileAttachment.attachable_id == Card.id, - FileAttachment.attachable_type == 'Card')""", + FileAttachment.attachable_type == 'Card', + FileAttachment.status == 'active')""", cascade="all, delete-orphan", lazy="dynamic", ) - # Card link relationships (self-referential many-to-many) + # Card link relationships (self-referential many-to-many) - only active links child_links = db.relationship( "CardLink", foreign_keys="CardLink.parent_card_id", back_populates="parent_card", cascade="all, delete-orphan", lazy="dynamic", + primaryjoin="""and_(Card.id == CardLink.parent_card_id, + CardLink.status == 'active')""", ) parent_links = db.relationship( "CardLink", @@ -84,6 +99,8 @@ class Card(db.Model, SoftDeleteMixin): back_populates="child_card", cascade="all, delete-orphan", lazy="dynamic", + primaryjoin="""and_(Card.id == CardLink.child_card_id, + CardLink.status == 'active')""", ) def to_dict(self, include_linked=False): diff --git a/backend/app/models/checklist.py b/backend/app/models/checklist.py index e61c03d..826b87a 100644 --- a/backend/app/models/checklist.py +++ b/backend/app/models/checklist.py @@ -35,9 +35,14 @@ class Checklist(db.Model, SoftDeleteMixin): onupdate=lambda: datetime.now(UTC), ) - # Relationships + # Relationships - only active check items check_items = db.relationship( - "CheckItem", backref="checklist", cascade="all, delete-orphan", lazy="dynamic" + "CheckItem", + backref="checklist", + cascade="all, delete-orphan", + lazy="dynamic", + primaryjoin="""and_(Checklist.id == CheckItem.checklist_id, + CheckItem.status == 'active')""", ) def to_dict(self): diff --git a/backend/app/models/list_model.py b/backend/app/models/list_model.py index 68e2571..a76633f 100644 --- a/backend/app/models/list_model.py +++ b/backend/app/models/list_model.py @@ -30,9 +30,13 @@ class List(db.Model, SoftDeleteMixin): onupdate=lambda: datetime.now(UTC), ) - # Relationships + # Relationships - only active cards cards = db.relationship( - "Card", backref="list", cascade="all, delete-orphan", lazy="dynamic" + "Card", + backref="list", + cascade="all, delete-orphan", + lazy="dynamic", + primaryjoin="and_(List.id == Card.list_id, Card.status == 'active')", ) def to_dict(self): diff --git a/backend/app/routes/kanban/boards.py b/backend/app/routes/kanban/boards.py index e1395d9..26530ad 100644 --- a/backend/app/routes/kanban/boards.py +++ b/backend/app/routes/kanban/boards.py @@ -7,8 +7,7 @@ from flask_pydantic import validate from app import db from app.decorators import load_board_owned from app.decorators.decorators import get_current_user_id -from app.models import (Board, Card, CardLabel, CheckItem, Checklist, Comment, - Label, List) +from app.models import Board, Card, List from app.schemas import (BoardCreateRequest, BoardResponse, BoardWithDetailsResponse) @@ -31,61 +30,46 @@ def get_board(board_id, board): """Get a single board with all its details""" from app.models import User - # Get all lists for this board (filter out soft-deleted lists) + # Get all lists for this board lists_data = [] - for lst in ( - board.lists.filter_by(closed=False, deleted_at=None).order_by(List.pos).all() - ): + for lst in board.lists.filter_by(closed=False).order_by(List.pos).all(): cards_data = [] - # Filter out soft-deleted cards - for card in ( - lst.cards.filter_by(closed=False, deleted_at=None).order_by(Card.pos).all() - ): + for card in lst.cards.filter_by(closed=False).order_by(Card.pos).all(): card_dict = card.to_dict() - # Add labels for this card (filter out soft-deleted card-label associations) + # Add labels for this card card_dict["labels"] = [ - label.to_dict() - for label in ( - db.session.query(Label) - .join(CardLabel) - .filter( - CardLabel.card_id == card.id, CardLabel.deleted_at.is_(None) - ) - .all() - ) + card_label.label.to_dict() + for card_label in card.labels + if card_label.label and card_label.status == "active" ] - # Add comments for this card (filter out soft-deleted comments) + # Add comments for this card card_dict["comments"] = [] - for comment in card.comments.filter(Comment.deleted_at.is_(None)).all(): + for comment in card.comments.all(): comment_dict = comment.to_dict() user = db.session.get(User, comment.user_id) comment_dict["user"] = user.to_dict() if user else None card_dict["comments"].append(comment_dict) - # Add checklists with items for this - # card (filter out soft-deleted checklists and items) + # Add checklists with items + from app.models import CheckItem, Checklist + card_dict["checklists"] = [ { **checklist.to_dict(), "items": [ item.to_dict() - for item in checklist.check_items.filter( - CheckItem.deleted_at.is_(None) - ).all() + for item in CheckItem.active() + .filter_by(checklist_id=checklist.id) + .all() ], } - for checklist in card.checklists.filter( - Checklist.deleted_at.is_(None) - ).all() + for checklist in Checklist.active().filter_by(card_id=card.id).all() ] # Add epic for this card - if card.epic: - card_dict["epic"] = card.epic.to_dict() - else: - card_dict["epic"] = None + card_dict["epic"] = card.epic.to_dict() if card.epic else None cards_data.append(card_dict) diff --git a/frontend/src/components/CardEpics.tsx b/frontend/src/components/CardEpics.tsx index f16aac8..5a4fbff 100644 --- a/frontend/src/components/CardEpics.tsx +++ b/frontend/src/components/CardEpics.tsx @@ -32,7 +32,7 @@ export function CardEpics({ cardEpics, boardId, cardId, refetchCard }: CardEpics return; } - const success = await removeEpic(epicId, epicName); + const success = await removeEpic(epicId); if (success) { await refetchCard(); } diff --git a/frontend/src/components/CardLinks.tsx b/frontend/src/components/CardLinks.tsx index bcb5052..8b827b8 100644 --- a/frontend/src/components/CardLinks.tsx +++ b/frontend/src/components/CardLinks.tsx @@ -1,7 +1,9 @@ +import { useState } from 'react'; import { Link } from 'react-router-dom'; import type { CardLinksResponse } from '../types/kanban'; import LinkIcon from './icons/LinkIcon'; import UnlinkIcon from './icons/UnlinkIcon'; +import { UnlinkCardModal } from './UnlinkCardModal'; interface CardLinksProps { links: CardLinksResponse; @@ -13,6 +15,18 @@ export function CardLinks({ links, boardId, onUnlink }: CardLinksProps) { const { parent_cards: parentCards, child_cards: childCards } = links; const hasLinks = parentCards.length > 0 || childCards.length > 0; + const [pendingUnlink, setPendingUnlink] = useState<{ + id: number; + cardName: string; + } | null>(null); + + const handleConfirmUnlink = () => { + if (pendingUnlink) { + onUnlink(pendingUnlink.id); + setPendingUnlink(null); + } + }; + if (!hasLinks) { return null; } @@ -82,7 +96,12 @@ export function CardLinks({ links, boardId, onUnlink }: CardLinksProps) { {link.card.name}