From fc09848a33be276f346ed084c9bc513fb1e59b26 Mon Sep 17 00:00:00 2001 From: mohamad Date: Sat, 7 Jun 2025 15:04:49 +0200 Subject: [PATCH] Add position attribute to Item model for reordering functionality - Introduced a new 'position' column in the Item model to facilitate item ordering. - Updated the List model's relationship to order items by position and creation date. - Enhanced CRUD operations to handle item creation and updates with position management. - Implemented drag-and-drop reordering in the frontend, ensuring proper position updates on item movement. - Adjusted item update logic to accommodate reordering and version control. --- ...c100f5b_add_position_to_item_model_for_.py | 133 ++++++++++++++++++ be/app/crud/item.py | 40 +++++- be/app/models.py | 11 +- be/app/schemas/item.py | 1 + fe/src/layouts/MainLayout.vue | 1 - fe/src/pages/ListDetailPage.vue | 25 +++- 6 files changed, 199 insertions(+), 12 deletions(-) create mode 100644 be/alembic/versions/91d00c100f5b_add_position_to_item_model_for_.py diff --git a/be/alembic/versions/91d00c100f5b_add_position_to_item_model_for_.py b/be/alembic/versions/91d00c100f5b_add_position_to_item_model_for_.py new file mode 100644 index 0000000..47a740f --- /dev/null +++ b/be/alembic/versions/91d00c100f5b_add_position_to_item_model_for_.py @@ -0,0 +1,133 @@ +"""Add position to Item model for reordering + +Revision ID: 91d00c100f5b +Revises: 0001_initial_schema +Create Date: 2025-06-07 14:59:48.761124 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision: str = '91d00c100f5b' +down_revision: Union[str, None] = '0001_initial_schema' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index('ix_apscheduler_jobs_next_run_time', table_name='apscheduler_jobs') + op.drop_table('apscheduler_jobs') + op.alter_column('chore_assignments', 'is_complete', + existing_type=sa.BOOLEAN(), + server_default=None, + existing_nullable=False) + op.alter_column('expenses', 'currency', + existing_type=sa.VARCHAR(), + server_default=None, + existing_nullable=False) + op.alter_column('expenses', 'is_recurring', + existing_type=sa.BOOLEAN(), + server_default=None, + existing_nullable=False) + op.drop_index('ix_expenses_recurring_next_occurrence', table_name='expenses', postgresql_where='(is_recurring = true)') + op.alter_column('invites', 'is_active', + existing_type=sa.BOOLEAN(), + server_default=None, + existing_nullable=False) + op.add_column('items', sa.Column('position', sa.Integer(), server_default='0', nullable=False)) + op.alter_column('items', 'is_complete', + existing_type=sa.BOOLEAN(), + server_default=None, + existing_nullable=False) + op.create_index('ix_items_list_id_position', 'items', ['list_id', 'position'], unique=False) + op.alter_column('lists', 'is_complete', + existing_type=sa.BOOLEAN(), + server_default=None, + existing_nullable=False) + op.alter_column('recurrence_patterns', 'interval', + existing_type=sa.INTEGER(), + server_default=None, + existing_nullable=False) + op.create_index(op.f('ix_settlement_activities_id'), 'settlement_activities', ['id'], unique=False) + op.create_index('ix_settlement_activity_created_by_user_id', 'settlement_activities', ['created_by_user_id'], unique=False) + op.create_index('ix_settlement_activity_expense_split_id', 'settlement_activities', ['expense_split_id'], unique=False) + op.create_index('ix_settlement_activity_paid_by_user_id', 'settlement_activities', ['paid_by_user_id'], unique=False) + op.alter_column('users', 'is_active', + existing_type=sa.BOOLEAN(), + server_default=None, + existing_nullable=False) + op.alter_column('users', 'is_superuser', + existing_type=sa.BOOLEAN(), + server_default=None, + existing_nullable=False) + op.alter_column('users', 'is_verified', + existing_type=sa.BOOLEAN(), + server_default=None, + existing_nullable=False) + # ### end Alembic commands ### + + +def downgrade() -> None: + """Downgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('users', 'is_verified', + existing_type=sa.BOOLEAN(), + server_default=sa.text('false'), + existing_nullable=False) + op.alter_column('users', 'is_superuser', + existing_type=sa.BOOLEAN(), + server_default=sa.text('false'), + existing_nullable=False) + op.alter_column('users', 'is_active', + existing_type=sa.BOOLEAN(), + server_default=sa.text('true'), + existing_nullable=False) + op.drop_index('ix_settlement_activity_paid_by_user_id', table_name='settlement_activities') + op.drop_index('ix_settlement_activity_expense_split_id', table_name='settlement_activities') + op.drop_index('ix_settlement_activity_created_by_user_id', table_name='settlement_activities') + op.drop_index(op.f('ix_settlement_activities_id'), table_name='settlement_activities') + op.alter_column('recurrence_patterns', 'interval', + existing_type=sa.INTEGER(), + server_default=sa.text('1'), + existing_nullable=False) + op.alter_column('lists', 'is_complete', + existing_type=sa.BOOLEAN(), + server_default=sa.text('false'), + existing_nullable=False) + op.drop_index('ix_items_list_id_position', table_name='items') + op.alter_column('items', 'is_complete', + existing_type=sa.BOOLEAN(), + server_default=sa.text('false'), + existing_nullable=False) + op.drop_column('items', 'position') + op.alter_column('invites', 'is_active', + existing_type=sa.BOOLEAN(), + server_default=sa.text('true'), + existing_nullable=False) + op.create_index('ix_expenses_recurring_next_occurrence', 'expenses', ['is_recurring', 'next_occurrence'], unique=False, postgresql_where='(is_recurring = true)') + op.alter_column('expenses', 'is_recurring', + existing_type=sa.BOOLEAN(), + server_default=sa.text('false'), + existing_nullable=False) + op.alter_column('expenses', 'currency', + existing_type=sa.VARCHAR(), + server_default=sa.text("'USD'::character varying"), + existing_nullable=False) + op.alter_column('chore_assignments', 'is_complete', + existing_type=sa.BOOLEAN(), + server_default=sa.text('false'), + existing_nullable=False) + op.create_table('apscheduler_jobs', + sa.Column('id', sa.VARCHAR(length=191), autoincrement=False, nullable=False), + sa.Column('next_run_time', sa.DOUBLE_PRECISION(precision=53), autoincrement=False, nullable=True), + sa.Column('job_state', postgresql.BYTEA(), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint('id', name='apscheduler_jobs_pkey') + ) + op.create_index('ix_apscheduler_jobs_next_run_time', 'apscheduler_jobs', ['next_run_time'], unique=False) + # ### end Alembic commands ### diff --git a/be/app/crud/item.py b/be/app/crud/item.py index b7bcaae..c1d9183 100644 --- a/be/app/crud/item.py +++ b/be/app/crud/item.py @@ -7,6 +7,7 @@ from sqlalchemy.exc import SQLAlchemyError, IntegrityError, OperationalError from typing import Optional, List as PyList from datetime import datetime, timezone import logging # Add logging import +from sqlalchemy import func from app.models import Item as ItemModel, User as UserModel # Import UserModel for type hints if needed for selectinload from app.schemas.item import ItemCreate, ItemUpdate @@ -23,15 +24,21 @@ from app.core.exceptions import ( logger = logging.getLogger(__name__) # Initialize logger async def create_item(db: AsyncSession, item_in: ItemCreate, list_id: int, user_id: int) -> ItemModel: - """Creates a new item record for a specific list.""" + """Creates a new item record for a specific list, setting its position.""" try: async with db.begin_nested() if db.in_transaction() else db.begin() as transaction: + # Get the current max position in the list + max_pos_stmt = select(func.max(ItemModel.position)).where(ItemModel.list_id == list_id) + max_pos_result = await db.execute(max_pos_stmt) + max_pos = max_pos_result.scalar_one_or_none() or 0 + db_item = ItemModel( name=item_in.name, quantity=item_in.quantity, list_id=list_id, added_by_id=user_id, - is_complete=False + is_complete=False, + position=max_pos + 1 # Set the new position ) db.add(db_item) await db.flush() # Assigns ID @@ -104,18 +111,41 @@ async def get_item_by_id(db: AsyncSession, item_id: int) -> Optional[ItemModel]: raise DatabaseQueryError(f"Failed to query item: {str(e)}") async def update_item(db: AsyncSession, item_db: ItemModel, item_in: ItemUpdate, user_id: int) -> ItemModel: - """Updates an existing item record, checking for version conflicts.""" + """Updates an existing item record, checking for version conflicts and handling reordering.""" try: async with db.begin_nested() if db.in_transaction() else db.begin() as transaction: if item_db.version != item_in.version: - # No need to rollback here, as the transaction hasn't committed. - # The context manager will handle rollback if an exception is raised. raise ConflictError( f"Item '{item_db.name}' (ID: {item_db.id}) has been modified. " f"Your version is {item_in.version}, current version is {item_db.version}. Please refresh." ) update_data = item_in.model_dump(exclude_unset=True, exclude={'version'}) + + # --- Handle Reordering --- + if 'position' in update_data: + new_position = update_data.pop('position') # Remove from update_data to handle separately + + # We need the full list to reorder, making sure it's loaded and ordered + list_id = item_db.list_id + stmt = select(ItemModel).where(ItemModel.list_id == list_id).order_by(ItemModel.position.asc(), ItemModel.created_at.asc()) + result = await db.execute(stmt) + items_in_list = result.scalars().all() + + # Find the item to move + item_to_move = next((it for it in items_in_list if it.id == item_db.id), None) + if item_to_move: + items_in_list.remove(item_to_move) + # Insert at the new position (adjust for 1-based index from frontend) + # Clamp position to be within bounds + insert_pos = max(0, min(new_position - 1, len(items_in_list))) + items_in_list.insert(insert_pos, item_to_move) + + # Re-assign positions + for i, item in enumerate(items_in_list): + item.position = i + 1 + + # --- End Handle Reordering --- if 'is_complete' in update_data: if update_data['is_complete'] is True: diff --git a/be/app/models.py b/be/app/models.py index 544d749..8c18196 100644 --- a/be/app/models.py +++ b/be/app/models.py @@ -189,7 +189,12 @@ class List(Base): # --- Relationships --- creator = relationship("User", back_populates="created_lists") # Link to User.created_lists group = relationship("Group", back_populates="lists") # Link to Group.lists - items = relationship("Item", back_populates="list", cascade="all, delete-orphan", order_by="Item.created_at") # Link to Item.list, cascade deletes + items = relationship( + "Item", + back_populates="list", + cascade="all, delete-orphan", + order_by="Item.position.asc(), Item.created_at.asc()" # Default order by position, then creation + ) # --- Relationships for Cost Splitting --- expenses = relationship("Expense", foreign_keys="Expense.list_id", back_populates="list", cascade="all, delete-orphan") @@ -199,6 +204,9 @@ class List(Base): # === NEW: Item Model === class Item(Base): __tablename__ = "items" + __table_args__ = ( + Index('ix_items_list_id_position', 'list_id', 'position'), + ) id = Column(Integer, primary_key=True, index=True) list_id = Column(Integer, ForeignKey("lists.id", ondelete="CASCADE"), nullable=False) # Belongs to which list @@ -206,6 +214,7 @@ class Item(Base): quantity = Column(String, nullable=True) # Flexible quantity (e.g., "1", "2 lbs", "a bunch") is_complete = Column(Boolean, default=False, nullable=False) price = Column(Numeric(10, 2), nullable=True) # For cost splitting later (e.g., 12345678.99) + position = Column(Integer, nullable=False, server_default='0') # For ordering added_by_id = Column(Integer, ForeignKey("users.id"), nullable=False) # Who added this item completed_by_id = Column(Integer, ForeignKey("users.id"), nullable=True) # Who marked it complete created_at = Column(DateTime(timezone=True), server_default=func.now(), nullable=False) diff --git a/be/app/schemas/item.py b/be/app/schemas/item.py index fe2f2f6..33d3348 100644 --- a/be/app/schemas/item.py +++ b/be/app/schemas/item.py @@ -32,5 +32,6 @@ class ItemUpdate(BaseModel): quantity: Optional[str] = None is_complete: Optional[bool] = None price: Optional[Decimal] = None # Price added here for update + position: Optional[int] = None # For reordering version: int # completed_by_id will be set internally if is_complete is true \ No newline at end of file diff --git a/fe/src/layouts/MainLayout.vue b/fe/src/layouts/MainLayout.vue index 4928b41..c83bca0 100644 --- a/fe/src/layouts/MainLayout.vue +++ b/fe/src/layouts/MainLayout.vue @@ -190,7 +190,6 @@ const handleLogout = async () => { .page-container { flex-grow: 1; - padding: 1rem; // Add some default padding padding-bottom: calc(var(--footer-height) + 1rem); // Space for fixed footer } diff --git a/fe/src/pages/ListDetailPage.vue b/fe/src/pages/ListDetailPage.vue index a0d0392..17644f9 100644 --- a/fe/src/pages/ListDetailPage.vue +++ b/fe/src/pages/ListDetailPage.vue @@ -346,9 +346,9 @@ @@ -1248,22 +1248,28 @@ const handleCheckboxChange = (item: ItemWithUI, event: Event) => { const handleDragEnd = async (evt: any) => { if (!list.value || evt.oldIndex === evt.newIndex) return; + const originalList = [...list.value.items]; // Store original order const item = list.value.items[evt.newIndex]; - const newPosition = evt.newIndex + 1; // Assuming backend uses 1-based indexing + const newPosition = evt.newIndex + 1; // Assuming backend uses 1-based indexing for position try { + // The v-model on draggable has already updated the list.value.items order optimistically. await apiClient.put( API_ENDPOINTS.LISTS.ITEM(String(list.value.id), String(item.id)), { position: newPosition, version: item.version } ); - item.version++; + // On success, we need to update the version of the moved item + const updatedItemInList = list.value.items.find(i => i.id === item.id); + if (updatedItemInList) { + updatedItemInList.version++; + } notificationStore.addNotification({ message: t('listDetailPage.notifications.itemReorderedSuccess'), type: 'success' }); } catch (err) { // Revert the order on error - list.value.items = [...list.value.items]; + list.value.items = originalList; notificationStore.addNotification({ message: getApiErrorMessage(err, 'listDetailPage.errors.reorderItemFailed'), type: 'error' @@ -1391,6 +1397,7 @@ const handleDragEnd = async (evt: any) => { .page-padding { padding: 1rem; + padding-inline: 0; max-width: 1200px; margin: 0 auto; } @@ -1810,12 +1817,20 @@ const handleDragEnd = async (evt: any) => { } /* New item input styling */ +.new-item-input-container { + list-style: none !important; + padding-inline: 3rem; + padding-bottom: 1.2rem; +} + .new-item-input-container .neo-checkbox-label { + width: 100%; } .neo-new-item-input { all: unset; + height: 100%; width: 100%; font-size: 1.05rem; font-weight: 500;