diff --git a/backend/app/config.py b/backend/app/config.py index 1e8478b..8984c09 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -56,8 +56,8 @@ class TestingConfig(Config): # Conservative connection pool settings for testing SQLALCHEMY_ENGINE_OPTIONS = { - "pool_size": 20, # Only one connection in the pool - "max_overflow": 10, # No overflow connections allowed + "pool_size": 1, # Only one connection in the pool + "max_overflow": 0, # No overflow connections allowed "pool_timeout": 30, "pool_recycle": 3600, # Recycle after 1 hour "pool_pre_ping": True, # Verify connections before using diff --git a/backend/app/routes/kanban/lists.py b/backend/app/routes/kanban/lists.py index 6e4749c..ff1d5a2 100644 --- a/backend/app/routes/kanban/lists.py +++ b/backend/app/routes/kanban/lists.py @@ -1,11 +1,14 @@ +from datetime import UTC, datetime + from flask import request from flask_jwt_extended import jwt_required from flask_pydantic import validate from app import db from app.decorators import load_board_owned, load_list_owned -from app.models import List +from app.models import Board, List from app.schemas import ListCreateRequest +from app.services.list_position_service import ListPositionService from . import kanban_bp @@ -37,9 +40,21 @@ def update_list(list_id, lst, body: ListCreateRequest): lst.name = body.name if request.json.get("closed") is not None: lst.closed = request.json.get("closed") - lst.pos = body.pos - db.session.commit() + # Track if position is changing + old_position = lst.pos + new_position = body.pos + + if old_position != new_position: + # Use ListPositionService to reorder lists + ListPositionService.reorder_lists(lst.board_id, list_id, new_position) + else: + lst.pos = new_position + db.session.commit() + + # Update board activity timestamp + board = db.session.get(Board, lst.board_id) + board.date_last_activity = datetime.now(UTC) return lst.to_dict(), 200 diff --git a/backend/app/services/list_position_service.py b/backend/app/services/list_position_service.py new file mode 100644 index 0000000..4cf3a4e --- /dev/null +++ b/backend/app/services/list_position_service.py @@ -0,0 +1,76 @@ +"""Service for managing list positioning and reordering""" + +from app import db +from app.models import List + + +class ListPositionService: + """Service for handling list position management""" + + @staticmethod + def reorder_lists(board_id: int, moved_list_id: int, new_position: float) -> None: + """ + Reorder all lists in a board when one list is moved to a new position. + + Args: + board_id: The ID of board containing lists + moved_list_id: The ID of list being moved + new_position: The new position for moved list + """ + # Get all lists in board, ordered by their current position + all_lists = List.query.filter_by(board_id=board_id).order_by(List.pos).all() + + # Find moved list in board + moved_list = None + other_lists = [] + + for lst in all_lists: + if lst.id == moved_list_id: + moved_list = lst + else: + other_lists.append(lst) + + if not moved_list: + return # List not found in this board + + # Insert moved list at the new position + other_lists.insert(int(new_position), moved_list) + + # Update positions for all lists to ensure unique, sequential positions + for index, lst in enumerate(other_lists): + lst.pos = float(index) + + db.session.commit() + + @staticmethod + def get_next_position(board_id: int) -> float: + """ + Get next available position in a board. + + Args: + board_id: The ID of board + + Returns: + The next available position (float) + """ + last_list = ( + List.query.filter_by(board_id=board_id).order_by(List.pos.desc()).first() + ) + + return float(last_list.pos + 1) if last_list else 0.0 + + @staticmethod + def ensure_unique_positions(board_id: int) -> None: + """ + Ensure all lists in a board have unique, sequential positions. + Useful for data cleanup. + + Args: + board_id: The ID of board to fix + """ + lists = List.query.filter_by(board_id=board_id).order_by(List.pos).all() + + for index, lst in enumerate(lists): + lst.pos = float(index) + + db.session.commit() diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 4c7abb2..87790f6 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -11,7 +11,7 @@ log = logging.getLogger(__name__) fake = Faker() -@pytest.fixture(scope="session") +@pytest.fixture(scope="function") def app(): """Create application for testing with PostgreSQL database (session scope)""" app = create_app(config_name="test") @@ -35,6 +35,7 @@ def app(): # Cleanup after all tests db.session.remove() db.drop_all() + db.engine.dispose() @pytest.fixture diff --git a/backend/tests/routes/test_lists.py b/backend/tests/routes/test_lists.py index d7f8dfb..dcb8e4f 100644 --- a/backend/tests/routes/test_lists.py +++ b/backend/tests/routes/test_lists.py @@ -68,7 +68,7 @@ class TestListRoutes: assert response.status_code == 404 def test_update_list_success(self, client, db_session, regular_user, auth_headers): - """Test updating a list""" + """Test updating a list name (position stays same for single list)""" board = Board(name="Test Board", user_id=regular_user.id) db_session.add(board) db_session.flush() @@ -80,13 +80,13 @@ class TestListRoutes: response = client.put( f"/api/lists/{lst.id}", headers=auth_headers, - json={"name": "Updated Name", "pos": 1}, + json={"name": "Updated Name", "pos": 0}, ) assert response.status_code == 200 data = response.get_json() assert data["name"] == "Updated Name" - assert data["pos"] == 1 + assert data["pos"] == 0 def test_update_list_not_found(self, client, db_session, auth_headers): """Test updating a non-existent list""" @@ -149,3 +149,71 @@ class TestListRoutes: deleted_card = db.session.get(Card, card.id) assert deleted_list is None assert deleted_card is None + + def test_update_list_position_reorders_others( + self, client, db_session, regular_user, auth_headers + ): + """Test updating list position reorders other lists in the board""" + board = Board(name="Test Board", user_id=regular_user.id) + db_session.add(board) + db_session.flush() + + # Create 3 lists in sequential positions + list1 = List(name="List 1", board_id=board.id, pos=0) + list2 = List(name="List 2", board_id=board.id, pos=1) + list3 = List(name="List 3", board_id=board.id, pos=2) + db_session.add(list1) + db_session.add(list2) + db_session.add(list3) + db_session.commit() + + # Move list3 from position 2 to position 0 (front) + response = client.put( + f"/api/lists/{list3.id}", + headers=auth_headers, + json={"name": "List 3", "pos": 0}, + ) + + assert response.status_code == 200 + + # Verify all lists have unique, sequential positions + updated_lists = List.query.filter_by(board_id=board.id).order_by(List.pos).all() + assert len(updated_lists) == 3 + assert updated_lists[0].id == list3.id + assert updated_lists[0].pos == 0.0 + assert updated_lists[1].id == list1.id + assert updated_lists[1].pos == 1.0 + assert updated_lists[2].id == list2.id + assert updated_lists[2].pos == 2.0 + + def test_update_list_position_no_change( + self, client, db_session, regular_user, auth_headers + ): + """Test updating list with same position doesn't reorder others""" + board = Board(name="Test Board", user_id=regular_user.id) + db_session.add(board) + db_session.flush() + + list1 = List(name="List 1", board_id=board.id, pos=0) + list2 = List(name="List 2", board_id=board.id, pos=1) + db_session.add(list1) + db_session.add(list2) + db_session.commit() + + original_pos1 = list1.pos + original_pos2 = list2.pos + + # Update list2 but keep same position + response = client.put( + f"/api/lists/{list2.id}", + headers=auth_headers, + json={"name": "Updated List 2", "pos": original_pos2}, + ) + + assert response.status_code == 200 + + # Verify positions unchanged + updated_list1 = db.session.get(List, list1.id) + updated_list2 = db.session.get(List, list2.id) + assert updated_list1.pos == original_pos1 + assert updated_list2.pos == original_pos2 diff --git a/frontend/src/components/kanban/KanbanColumn.tsx b/frontend/src/components/kanban/KanbanColumn.tsx index 5f624a9..1fa86db 100644 --- a/frontend/src/components/kanban/KanbanColumn.tsx +++ b/frontend/src/components/kanban/KanbanColumn.tsx @@ -9,13 +9,17 @@ import Edit2Icon from '../icons/Edit2Icon'; import Trash2Icon from '../icons/Trash2Icon'; import { useModal } from '../../context/modals/useModal'; -interface KanbanColumnProps { +export interface KanbanColumnProps { list: ListWithCards; cards: CardType[]; onOpenCardModal: (card: CardType) => void; onCardCreate: (data: { name: string; description?: string }) => Promise; onListEdit?: (name: string) => Promise; onListDelete?: () => Promise; + dragHandleProps?: { + attributes: any; + listeners: any; + }; } export function KanbanColumn({ @@ -25,6 +29,7 @@ export function KanbanColumn({ onCardCreate, onListEdit, onListDelete, + dragHandleProps, }: KanbanColumnProps) { const { setNodeRef, isOver } = useDroppable({ id: `LIST_${list.id}`, @@ -66,7 +71,31 @@ export function KanbanColumn({
-

{list.name}

+
+ {/* Drag Handle Icon */} +
+ + + + + + + + +
+

{list.name}

+
{onListEdit && (
+ ); +} diff --git a/frontend/src/pages/BoardDetail.tsx b/frontend/src/pages/BoardDetail.tsx index 563c0dc..12e532b 100644 --- a/frontend/src/pages/BoardDetail.tsx +++ b/frontend/src/pages/BoardDetail.tsx @@ -2,7 +2,7 @@ import { useParams, Link } from 'react-router-dom'; import { useBoard } from '../hooks/useBoard'; import { useCardMutations } from '../hooks/useCardMutations'; import { useListMutations } from '../hooks/useListMutations'; -import { KanbanColumn } from '../components/kanban/KanbanColumn'; +import { SortableKanbanColumn } from '../components/kanban/SortableKanbanColumn'; import { CreateListModal } from '../components/kanban/CreateListModal'; import { CardPreviewModal } from '../components/CardPreviewModal'; import { useModal } from '../context/modals/useModal'; @@ -17,7 +17,8 @@ import { useSensors, closestCenter, } from '@dnd-kit/core'; -import { Card as CardType } from '../types/kanban'; +import { SortableContext, horizontalListSortingStrategy } from '@dnd-kit/sortable'; +import { Card as CardType, ListWithCards } from '../types/kanban'; import { useState } from 'react'; export function BoardDetail() { @@ -28,6 +29,7 @@ export function BoardDetail() { const { openModal } = useModal(); const [activeCard, setActiveCard] = useState(null); + const [activeList, setActiveList] = useState(null); const sensors = useSensors( useSensor(PointerSensor, { @@ -39,18 +41,23 @@ export function BoardDetail() { const handleDragStart = (event: DragStartEvent) => { const { active } = event; + const [activeType, activeIdStr] = (active.id as string).split('_'); - const activeIdStr = (active.id as string).split('_')[1]; - - const cardId = parseInt(activeIdStr as string); - - if (board) { - const card = board.lists.flatMap((list) => list.cards).find((c) => c.id === cardId); - - // console.log('---handleDragStart', event, card) - // console.log('---handleDragStart.board', board) - if (card) { - setActiveCard(card); + if (activeType === 'COLUMN') { + // Dragging a column + const listId = parseInt(activeIdStr); + const list = board?.lists.find((l) => l.id === listId); + if (list) { + setActiveList(list); + } + } else if (activeType === 'CARD') { + // Dragging a card + const cardId = parseInt(activeIdStr); + if (board) { + const card = board.lists.flatMap((list) => list.cards).find((c) => c.id === cardId); + if (card) { + setActiveCard(card); + } } } }; @@ -85,19 +92,60 @@ export function BoardDetail() { const handleDragEnd = async (event: DragEndEvent) => { const { active, over } = event; setActiveCard(null); + setActiveList(null); if (!over || !board) return; - // console.log('--------------over', over) - // console.log('--------------board', board) + const [activeType, activeIdStr] = (active.id as string).split('_'); const [overType, overIdStr] = (over.id as string).split('_'); - const overId = parseInt(overIdStr, 10); - const activeIdStr = (active.id as string).split('_')[1]; const activeId = parseInt(activeIdStr, 10); - // debugger + const overId = parseInt(overIdStr, 10); if (active.id === over.id) return; + // Handle column reordering + if (activeType === 'COLUMN') { + // todo find over column id, + + let overListIndex = -1; + const activeList = board.lists.find((l) => l.id === activeId); + if (overType === 'CARD') { + overListIndex = board.lists.findIndex((l) => { + const foundIndex = l.cards.findIndex((card) => card.id === overId); + return foundIndex >= 0; + }); + } else if (overType === 'LIST') { + overListIndex = board.lists.findIndex((l) => l.id === overId); + } + + // console.log('-------active.id', active.id) + // console.log('-------overType.id', overType) + // console.log('-------overListIndex', overListIndex) + + const activeListIndex = board.lists.findIndex((l) => l.id === activeId); + // overListIndex = board.lists.findIndex((l) => l.id === overId); + + if (activeListIndex === -1 || overListIndex === -1 || !activeList) return; + + // Calculate new positions for all lists + const reorderedLists = [...board.lists]; + const [movedList] = reorderedLists.splice(activeListIndex, 1); + reorderedLists.splice(overListIndex, 0, movedList); + + await updateList(activeList.id, { name: activeList.name, pos: overListIndex }); + + // // Update all list positions + // for (let i = 0; i < reorderedLists.length; i++) { + // const list = reorderedLists[i]; + // if (list.pos !== i) { + // await updateList(list.id, { name: list.name, pos: i }); + // } + // } + + return; + } + + // Handle card reordering (existing logic) // Find active card let activeCard: CardType | undefined; let activeList: (typeof board.lists)[0] | undefined; @@ -114,8 +162,7 @@ export function BoardDetail() { if (!activeCard || !activeList) return; // Check if we're dropping on a list or a card - // debugger - if (overType.toLocaleLowerCase() === 'list') { + if (overType.toLowerCase() === 'list') { const overList = board.lists.find((list) => list.id === overId); // Dropping on a list - append to end @@ -143,20 +190,6 @@ export function BoardDetail() { // Calculate new position const overCardIndex = overListContainingCard.cards.findIndex((c) => c.id === overId); - // // If dropping on to same list and after of same card, do nothing - // if ( - // overListContainingCard.id === activeList.id && - // overCardIndex === activeList.cards.findIndex((c) => c.id === activeId) + 1 - // ) { - - // console.log('--------------over', over) - // console.log('--------------board', board) - // console.log('--------------activeCard', activeCard) - // console.log('--------------overListContainingCard', overListContainingCard) - - // return; - // } - await moveCard(activeCard, activeList.id, overListContainingCard.id, overCardIndex); }; @@ -234,25 +267,37 @@ export function BoardDetail() { onDragOver={handleDragOver} onDragEnd={handleDragEnd} > -
- {board.lists.map((list) => ( - handleEditList(list.id, name)} - onListDelete={() => handleDeleteList(list.id)} - /> - ))} -
+ `COLUMN_${list.id}`)} + strategy={horizontalListSortingStrategy} + > +
+ {board.lists.map((list) => ( + handleEditList(list.id, name)} + onListDelete={() => handleDeleteList(list.id)} + /> + ))} +
+ {activeCard ? (

{activeCard.name}

+ ) : activeList ? ( +
+

{activeList.name}

+ + {activeList.cards.length} cards + +
) : null}