mo/mitlist
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor CRUD operations in group, item, and list modules to remove unnecessary transaction context; enhance error handling and improve code readability. Update API endpoint for OCR processing in configuration and add confirmation dialogs for item actions in ListDetailPage.

Browse Source
This commit is contained in:
mohamad 2025-05-08 22:34:07 +02:00
parent 262505c898
commit f52b47f6df
5 changed files with 227 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
be/app/crud/group.py
View File

@ -4,6 +4,7 @@ from sqlalchemy.future import select
from sqlalchemy.orm import selectinload # For eager loading members from sqlalchemy.orm import selectinload # For eager loading members
from sqlalchemy.exc import SQLAlchemyError, IntegrityError, OperationalError from sqlalchemy.exc import SQLAlchemyError, IntegrityError, OperationalError
from typing import Optional, List from typing import Optional, List
from sqlalchemy import func
from app.models import User as UserModel, Group as GroupModel, UserGroup as UserGroupModel from app.models import User as UserModel, Group as GroupModel, UserGroup as UserGroupModel
from app.schemas.group import GroupCreate from app.schemas.group import GroupCreate
@ -47,14 +48,13 @@ async def create_group(db: AsyncSession, group_in: GroupCreate, creator_id: int)
async def get_user_groups(db: AsyncSession, user_id: int) -> List[GroupModel]: async def get_user_groups(db: AsyncSession, user_id: int) -> List[GroupModel]:
"""Gets all groups a user is a member of.""" """Gets all groups a user is a member of."""
try: try:
async with db.begin(): result = await db.execute(
result = await db.execute( select(GroupModel)
select(GroupModel) .join(UserGroupModel)
.join(UserGroupModel) .where(UserGroupModel.user_id == user_id)
.where(UserGroupModel.user_id == user_id) .options(selectinload(GroupModel.member_associations))
.options(selectinload(GroupModel.member_associations)) )
) return result.scalars().all()
return result.scalars().all()
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
@ -63,15 +63,14 @@ async def get_user_groups(db: AsyncSession, user_id: int) -> List[GroupModel]:
async def get_group_by_id(db: AsyncSession, group_id: int) -> Optional[GroupModel]: async def get_group_by_id(db: AsyncSession, group_id: int) -> Optional[GroupModel]:
"""Gets a single group by its ID, optionally loading members.""" """Gets a single group by its ID, optionally loading members."""
try: try:
async with db.begin(): result = await db.execute(
result = await db.execute( select(GroupModel)
select(GroupModel) .where(GroupModel.id == group_id)
.where(GroupModel.id == group_id) .options(
.options( selectinload(GroupModel.member_associations).selectinload(UserGroupModel.user)
selectinload(GroupModel.member_associations).selectinload(UserGroupModel.user)
)
) )
return result.scalars().first() )
return result.scalars().first()
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
@ -80,13 +79,12 @@ async def get_group_by_id(db: AsyncSession, group_id: int) -> Optional[GroupMode
async def is_user_member(db: AsyncSession, group_id: int, user_id: int) -> bool: async def is_user_member(db: AsyncSession, group_id: int, user_id: int) -> bool:
"""Checks if a user is a member of a specific group.""" """Checks if a user is a member of a specific group."""
try: try:
async with db.begin(): result = await db.execute(
result = await db.execute( select(UserGroupModel.id)
select(UserGroupModel.id) .where(UserGroupModel.group_id == group_id, UserGroupModel.user_id == user_id)
.where(UserGroupModel.group_id == group_id, UserGroupModel.user_id == user_id) .limit(1)
.limit(1) )
) return result.scalar_one_or_none() is not None
return result.scalar_one_or_none() is not None
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
@ -95,12 +93,11 @@ async def is_user_member(db: AsyncSession, group_id: int, user_id: int) -> bool:
async def get_user_role_in_group(db: AsyncSession, group_id: int, user_id: int) -> Optional[UserRoleEnum]: async def get_user_role_in_group(db: AsyncSession, group_id: int, user_id: int) -> Optional[UserRoleEnum]:
"""Gets the role of a user in a specific group.""" """Gets the role of a user in a specific group."""
try: try:
async with db.begin(): result = await db.execute(
result = await db.execute( select(UserGroupModel.role)
select(UserGroupModel.role) .where(UserGroupModel.group_id == group_id, UserGroupModel.user_id == user_id)
.where(UserGroupModel.group_id == group_id, UserGroupModel.user_id == user_id) )
) return result.scalar_one_or_none()
return result.scalar_one_or_none()
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
@ -146,11 +143,10 @@ async def remove_user_from_group(db: AsyncSession, group_id: int, user_id: int)
async def get_group_member_count(db: AsyncSession, group_id: int) -> int: async def get_group_member_count(db: AsyncSession, group_id: int) -> int:
"""Counts the number of members in a group.""" """Counts the number of members in a group."""
try: try:
async with db.begin(): result = await db.execute(
result = await db.execute( select(func.count(UserGroupModel.id)).where(UserGroupModel.group_id == group_id)
select(func.count(UserGroupModel.id)).where(UserGroupModel.group_id == group_id) )
) return result.scalar_one()
return result.scalar_one()
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
@ -170,22 +166,21 @@ async def check_group_membership(
GroupMembershipError: If the user_id is not a member of the group. GroupMembershipError: If the user_id is not a member of the group.
""" """
try: try:
async with db.begin(): # Check group existence first
# Check group existence first group_exists = await db.get(GroupModel, group_id)
group_exists = await db.get(GroupModel, group_id) if not group_exists:
if not group_exists: raise GroupNotFoundError(group_id)
raise GroupNotFoundError(group_id)
# Check membership
# Check membership membership = await db.execute(
membership = await db.execute( select(UserGroupModel.id)
select(UserGroupModel.id) .where(UserGroupModel.group_id == group_id, UserGroupModel.user_id == user_id)
.where(UserGroupModel.group_id == group_id, UserGroupModel.user_id == user_id) .limit(1)
.limit(1) )
) if membership.scalar_one_or_none() is None:
if membership.scalar_one_or_none() is None: raise GroupMembershipError(group_id, action=action)
raise GroupMembershipError(group_id, action=action) # If we reach here, the user is a member
# If we reach here, the user is a member return None
return None
except GroupNotFoundError: # Re-raise specific errors except GroupNotFoundError: # Re-raise specific errors
raise raise
except GroupMembershipError: except GroupMembershipError:

18
be/app/crud/item.py
View File

@ -43,13 +43,12 @@ async def create_item(db: AsyncSession, item_in: ItemCreate, list_id: int, user_
async def get_items_by_list_id(db: AsyncSession, list_id: int) -> PyList[ItemModel]: async def get_items_by_list_id(db: AsyncSession, list_id: int) -> PyList[ItemModel]:
"""Gets all items belonging to a specific list, ordered by creation time.""" """Gets all items belonging to a specific list, ordered by creation time."""
try: try:
async with db.begin(): result = await db.execute(
result = await db.execute( select(ItemModel)
select(ItemModel) .where(ItemModel.list_id == list_id)
.where(ItemModel.list_id == list_id) .order_by(ItemModel.created_at.asc()) # Or desc() if preferred
.order_by(ItemModel.created_at.asc()) # Or desc() if preferred )
) return result.scalars().all()
return result.scalars().all()
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
@ -58,9 +57,8 @@ async def get_items_by_list_id(db: AsyncSession, list_id: int) -> PyList[ItemMod
async def get_item_by_id(db: AsyncSession, item_id: int) -> Optional[ItemModel]: async def get_item_by_id(db: AsyncSession, item_id: int) -> Optional[ItemModel]:
"""Gets a single item by its ID.""" """Gets a single item by its ID."""
try: try:
async with db.begin(): result = await db.execute(select(ItemModel).where(ItemModel.id == item_id))
result = await db.execute(select(ItemModel).where(ItemModel.id == item_id)) return result.scalars().first()
return result.scalars().first()
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:

122
be/app/crud/list.py
View File

@ -45,45 +45,41 @@ async def create_list(db: AsyncSession, list_in: ListCreate, creator_id: int) ->
async def get_lists_for_user(db: AsyncSession, user_id: int) -> PyList[ListModel]: async def get_lists_for_user(db: AsyncSession, user_id: int) -> PyList[ListModel]:
"""Gets all lists accessible by a user.""" """Gets all lists accessible by a user."""
try: try:
async with db.begin(): group_ids_result = await db.execute(
group_ids_result = await db.execute( select(UserGroupModel.group_id).where(UserGroupModel.user_id == user_id)
select(UserGroupModel.group_id).where(UserGroupModel.user_id == user_id) )
) user_group_ids = group_ids_result.scalars().all()
user_group_ids = group_ids_result.scalars().all()
# Build conditions for the OR clause dynamically # Build conditions for the OR clause dynamically
conditions = [ conditions = [
and_(ListModel.created_by_id == user_id, ListModel.group_id.is_(None)) and_(ListModel.created_by_id == user_id, ListModel.group_id.is_(None))
] ]
if user_group_ids: # Only add the IN clause if there are group IDs if user_group_ids: # Only add the IN clause if there are group IDs
conditions.append(ListModel.group_id.in_(user_group_ids)) conditions.append(ListModel.group_id.in_(user_group_ids))
query = select(ListModel).where(or_(*conditions)).order_by(ListModel.updated_at.desc()) query = select(ListModel).where(or_(*conditions)).order_by(ListModel.updated_at.desc())
result = await db.execute(query) result = await db.execute(query)
return result.scalars().all() return result.scalars().all()
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
# It would be helpful to log the original error e here for more detailed debugging
# For example: logger.error(f"SQLAlchemyError in get_lists_for_user: {type(e).__name__} - {str(e)}")
raise DatabaseQueryError(f"Failed to query user lists: {str(e)}") raise DatabaseQueryError(f"Failed to query user lists: {str(e)}")
async def get_list_by_id(db: AsyncSession, list_id: int, load_items: bool = False) -> Optional[ListModel]: async def get_list_by_id(db: AsyncSession, list_id: int, load_items: bool = False) -> Optional[ListModel]:
"""Gets a single list by ID, optionally loading its items.""" """Gets a single list by ID, optionally loading its items."""
try: try:
async with db.begin(): query = select(ListModel).where(ListModel.id == list_id)
query = select(ListModel).where(ListModel.id == list_id) if load_items:
if load_items: query = query.options(
query = query.options( selectinload(ListModel.items)
selectinload(ListModel.items) .options(
.options( joinedload(ItemModel.added_by_user),
joinedload(ItemModel.added_by_user), joinedload(ItemModel.completed_by_user)
joinedload(ItemModel.completed_by_user)
)
) )
result = await db.execute(query) )
return result.scalars().first() result = await db.execute(query)
return result.scalars().first()
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
@ -139,29 +135,28 @@ async def delete_list(db: AsyncSession, list_db: ListModel) -> None:
async def check_list_permission(db: AsyncSession, list_id: int, user_id: int, require_creator: bool = False) -> ListModel: async def check_list_permission(db: AsyncSession, list_id: int, user_id: int, require_creator: bool = False) -> ListModel:
"""Fetches a list and verifies user permission.""" """Fetches a list and verifies user permission."""
try: try:
async with db.begin(): list_db = await get_list_by_id(db, list_id=list_id, load_items=True)
list_db = await get_list_by_id(db, list_id=list_id, load_items=True) if not list_db:
if not list_db: raise ListNotFoundError(list_id)
raise ListNotFoundError(list_id)
is_creator = list_db.created_by_id == user_id is_creator = list_db.created_by_id == user_id
if require_creator: if require_creator:
if not is_creator: if not is_creator:
raise ListCreatorRequiredError(list_id, "access") raise ListCreatorRequiredError(list_id, "access")
return list_db return list_db
if is_creator: if is_creator:
return list_db return list_db
if list_db.group_id: if list_db.group_id:
from app.crud.group import is_user_member from app.crud.group import is_user_member
is_member = await is_user_member(db, group_id=list_db.group_id, user_id=user_id) is_member = await is_user_member(db, group_id=list_db.group_id, user_id=user_id)
if not is_member: if not is_member:
raise ListPermissionError(list_id)
return list_db
else:
raise ListPermissionError(list_id) raise ListPermissionError(list_id)
return list_db
else:
raise ListPermissionError(list_id)
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:
@ -170,29 +165,28 @@ async def check_list_permission(db: AsyncSession, list_id: int, user_id: int, re
async def get_list_status(db: AsyncSession, list_id: int) -> ListStatus: async def get_list_status(db: AsyncSession, list_id: int) -> ListStatus:
"""Gets the update timestamps and item count for a list.""" """Gets the update timestamps and item count for a list."""
try: try:
async with db.begin(): list_query = select(ListModel.updated_at).where(ListModel.id == list_id)
list_query = select(ListModel.updated_at).where(ListModel.id == list_id) list_result = await db.execute(list_query)
list_result = await db.execute(list_query) list_updated_at = list_result.scalar_one_or_none()
list_updated_at = list_result.scalar_one_or_none()
if list_updated_at is None: if list_updated_at is None:
raise ListNotFoundError(list_id) raise ListNotFoundError(list_id)
item_status_query = ( item_status_query = (
select( select(
sql_func.max(ItemModel.updated_at).label("latest_item_updated_at"), sql_func.max(ItemModel.updated_at).label("latest_item_updated_at"),
sql_func.count(ItemModel.id).label("item_count") sql_func.count(ItemModel.id).label("item_count")
)
.where(ItemModel.list_id == list_id)
) )
item_result = await db.execute(item_status_query) .where(ItemModel.list_id == list_id)
item_status = item_result.first() )
item_result = await db.execute(item_status_query)
item_status = item_result.first()
return ListStatus( return ListStatus(
list_updated_at=list_updated_at, list_updated_at=list_updated_at,
latest_item_updated_at=item_status.latest_item_updated_at if item_status else None, latest_item_updated_at=item_status.latest_item_updated_at if item_status else None,
item_count=item_status.item_count if item_status else 0 item_count=item_status.item_count if item_status else 0
) )
except OperationalError as e: except OperationalError as e:
raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}") raise DatabaseConnectionError(f"Failed to connect to database: {str(e)}")
except SQLAlchemyError as e: except SQLAlchemyError as e:

2
fe/src/config/api-config.ts
View File

@ -73,7 +73,7 @@ export const API_ENDPOINTS = {
// OCR // OCR
OCR: { OCR: {
PROCESS: '/ocr/process', PROCESS: '/ocr/extract-items',
STATUS: (jobId: string) => `/ocr/status/${jobId}`, STATUS: (jobId: string) => `/ocr/status/${jobId}`,
RESULT: (jobId: string) => `/ocr/result/${jobId}`, RESULT: (jobId: string) => `/ocr/result/${jobId}`,
BATCH: '/ocr/batch', BATCH: '/ocr/batch',

118
fe/src/pages/ListDetailPage.vue
View File

@ -105,6 +105,8 @@
label="Item Name" label="Item Name"
:rules="[(val) => !!val || 'Name is required']" :rules="[(val) => !!val || 'Name is required']"
outlined outlined
ref="itemNameInput"
@keydown.enter.prevent="onAddItem"
/> />
</div> </div>
<div class="col-12 col-md-4"> <div class="col-12 col-md-4">
@ -114,6 +116,7 @@
label="Quantity (optional)" label="Quantity (optional)"
outlined outlined
min="1" min="1"
@keydown.enter.prevent="onAddItem"
/> />
</div> </div>
<div class="col-12 col-md-2"> <div class="col-12 col-md-2">
@ -142,8 +145,9 @@
<q-item-section avatar> <q-item-section avatar>
<q-checkbox <q-checkbox
v-model="item.is_complete" v-model="item.is_complete"
@update:model-value="updateItem(item)" @update:model-value="confirmUpdateItem(item)"
:loading="item.updating" :loading="item.updating"
:disable="item.updating"
/> />
</q-item-section> </q-item-section>
<q-item-section> <q-item-section>
@ -152,9 +156,36 @@
Quantity: {{ item.quantity }} Quantity: {{ item.quantity }}
</q-item-label> </q-item-label>
</q-item-section> </q-item-section>
<q-item-section side>
<q-btn
flat
round
dense
icon="delete"
color="negative"
@click="confirmDeleteItem(item)"
:loading="item.deleting"
:disable="item.deleting"
/>
</q-item-section>
</q-item> </q-item>
</q-list> </q-list>
</template> </template>
<!-- Confirmation Dialog -->
<q-dialog v-model="showConfirmDialog" persistent>
<q-card>
<q-card-section class="row items-center">
<q-avatar icon="warning" color="warning" text-color="white" />
<span class="q-ml-sm">{{ confirmDialogMessage }}</span>
</q-card-section>
<q-card-actions align="right">
<q-btn flat label="Cancel" color="primary" v-close-popup />
<q-btn flat label="Confirm" color="primary" @click="handleConfirmedAction" />
</q-card-actions>
</q-card>
</q-dialog>
</q-page> </q-page>
</template> </template>
@ -172,6 +203,7 @@ interface Item {
version: number; version: number;
updating?: boolean; updating?: boolean;
updated_at: string; updated_at: string;
deleting?: boolean;
} }
interface List { interface List {
@ -198,7 +230,7 @@ const list = ref<List>({
const loading = ref(true); const loading = ref(true);
const error = ref<string | null>(null); const error = ref<string | null>(null);
const addingItem = ref(false); const addingItem = ref(false);
const pollingInterval = ref<number | undefined>(undefined); const pollingInterval = ref<ReturnType<typeof setInterval> | undefined>(undefined);
const lastListUpdate = ref<string | null>(null); const lastListUpdate = ref<string | null>(null);
const lastItemUpdate = ref<string | null>(null); const lastItemUpdate = ref<string | null>(null);
@ -214,6 +246,14 @@ const ocrItems = ref<{ name: string }[]>([]);
const addingOcrItems = ref(false); const addingOcrItems = ref(false);
const ocrError = ref<string | null>(null); const ocrError = ref<string | null>(null);
// Add new refs for confirmation dialog
const showConfirmDialog = ref(false);
const confirmDialogMessage = ref('');
const pendingAction = ref<(() => Promise<void>) | null>(null);
// Add ref for item name input
const itemNameInput = ref<{ focus: () => void } | null>(null);
const fetchListDetails = async () => { const fetchListDetails = async () => {
loading.value = true; loading.value = true;
error.value = null; error.value = null;
@ -277,7 +317,13 @@ const stopPolling = () => {
}; };
const onAddItem = async () => { const onAddItem = async () => {
if (!newItem.value.name) return; if (!newItem.value.name) {
$q.notify({
type: 'warning',
message: 'Please enter an item name',
});
return;
}
addingItem.value = true; addingItem.value = true;
try { try {
@ -287,6 +333,8 @@ const onAddItem = async () => {
); );
list.value.items.push(response.data as Item); list.value.items.push(response.data as Item);
newItem.value = { name: '' }; newItem.value = { name: '' };
// Focus the input for the next item
itemNameInput.value?.focus();
} catch (err: unknown) { } catch (err: unknown) {
$q.notify({ $q.notify({
type: 'negative', type: 'negative',
@ -395,14 +443,68 @@ const addOcrItems = async () => {
} }
}; };
// Add keyboard shortcut handler
const handleKeyPress = (event: KeyboardEvent) => {
// Focus item name input when pressing 'n'
if (event.key === 'n' && !event.ctrlKey && !event.metaKey) {
event.preventDefault();
itemNameInput.value?.focus();
}
};
// Add confirmation dialog handlers
const confirmUpdateItem = (item: Item) => {
confirmDialogMessage.value = `Are you sure you want to mark "${item.name}" as ${item.is_complete ? 'complete' : 'incomplete'}?`;
pendingAction.value = () => updateItem(item);
showConfirmDialog.value = true;
};
const confirmDeleteItem = (item: Item) => {
confirmDialogMessage.value = `Are you sure you want to delete "${item.name}"?`;
pendingAction.value = () => deleteItem(item);
showConfirmDialog.value = true;
};
const handleConfirmedAction = async () => {
if (pendingAction.value) {
await pendingAction.value();
pendingAction.value = null;
}
showConfirmDialog.value = false;
};
// Add delete item function
const deleteItem = async (item: Item) => {
item.deleting = true;
try {
await apiClient.delete(
API_ENDPOINTS.LISTS.ITEM(list.value.id.toString(), item.id.toString())
);
const index = list.value.items.findIndex((i) => i.id === item.id);
if (index !== -1) {
list.value.items.splice(index, 1);
}
} catch (err: unknown) {
$q.notify({
type: 'negative',
message: (err as Error).message || 'Failed to delete item',
});
} finally {
item.deleting = false;
}
};
// Add keyboard event listeners
onMounted(() => { onMounted(() => {
void fetchListDetails().then(() => { void fetchListDetails().then(() => {
startPolling(); startPolling();
}); });
window.addEventListener('keydown', handleKeyPress);
}); });
onUnmounted(() => { onUnmounted(() => {
stopPolling(); stopPolling();
window.removeEventListener('keydown', handleKeyPress);
}); });
</script> </script>
@ -411,4 +513,14 @@ onUnmounted(() => {
text-decoration: line-through; text-decoration: line-through;
opacity: 0.7; opacity: 0.7;
} }
/* Add transition for item updates */
.q-item {
transition: all 0.3s ease;
}
.q-item.updating {
opacity: 0.7;
pointer-events: none;
}
</style> </style>