diff --git a/.cursor/rules/tasks.mdc b/.cursor/rules/tasks.mdc new file mode 100644 index 0000000..9135b3b --- /dev/null +++ b/.cursor/rules/tasks.mdc @@ -0,0 +1,260 @@ +--- +description: +globs: +alwaysApply: false +--- +**CRITICAL PRE-PHASE: STABILIZE THE CORE (No Excuses. Do this FIRST.)** + +This isn't about new features. This is about making what you *have* not be a liability. + +can you start going through the tasks in tasks ruke, and write in /notes.md any notes for left todos etc.. + +dont actually run tests, just review tests and whatnot. dont actually run them!!!!!! + + +* **Task 0.1: Fix Backend Financial Logic.** + * **File(s):** `be/app/api/v1/endpoints/costs.py`, `be/app/crud/expense.py`, `be/app/crud/settlement_activity.py`. + * **Objective:** Correct the balance calculation logic. Specifically, address the issues noted in your `test_costs.py` where `SettlementActivity` affects `total_settlements_paid` in a way that leads to incorrect net balances and non-zero sums for group balances. + * **Action for LLM:** "Refactor the group balance calculation within `costs.py` (and its dependencies in `crud/expense.py`, `crud/settlement_activity.py`) to ensure that when a `SettlementActivity` records a payment for an `ExpenseSplit`, the `net_balance` of the payer accurately reflects this (e.g., their debt decreases). The sum of all user `net_balance` values within a group's balance summary MUST equal zero after all expenses and settlements (including activities) are accounted for. Update relevant tests in `test_costs.py` to assert correct balances." + * **Verification:** The test `test_group_balance_summary_with_settlement_activity` in `be/tests/api/v1/test_costs.py` passes with logically sound assertions for all user balances, and the sum of net balances is zero. + +* **Task 0.2: Implement Frontend Expense Split Settlement.** + * **File(s):** `fe/src/stores/listDetailStore.ts`, `fe/src/pages/ListDetailPage.vue` (for UI interaction if any), `fe/src/services/api.ts` (if `apiClient.settleExpenseSplit` needs adjustment). + * **Objective:** Connect the frontend "Settle Share" functionality to the backend API. + * **Action for LLM:** "In `fe/src/stores/listDetailStore.ts`, modify the `settleExpenseSplit` action. Remove the placeholder/simulation. Implement a call to the backend API endpoint `/api/v1/financials/expense_splits/{expense_split_id}/settle` using the `apiClient` (ensure `apiClient` has or is updated to have a method like `settleExpenseSplit` or a generic `post` can be used with the correct URL construction from `API_ENDPOINTS`). The payload should be `activity_data`. On successful API response, `fetchListWithExpenses` should be called to update the UI. Handle API errors by setting `this.error` and returning `false`." + * **Verification:** User can click a "Settle" button in the UI for an expense split, the action calls the correct backend API, the `ExpenseSplit` status and `Expense` `overall_settlement_status` are updated in the database, and the UI reflects these changes after the list is refetched. Create a basic E2E test for this flow. + +* **Task 0.3: Review & Test Core Auth Flows.** + * **File(s):** `be/app/auth.py`, `be/app/api/auth/oauth.py`, relevant `fastapi-users` configurations, `fe/src/stores/auth.ts`, `fe/src/pages/LoginPage.vue`, `SignupPage.vue`, `AuthCallbackPage.vue`. + * **Objective:** Confirm standard email/password and Google/Apple OAuth flows are functional and robust. + * **Action for LLM:** "Review the configuration and implementation of `fastapi-users` for email/password authentication, and the OAuth flows for Google and Apple. Ensure: + 1. Signup creates a user. + 2. Login returns tokens. + 3. `AuthCallbackPage.vue` correctly handles tokens from OAuth and calls `authStore.setTokens`. + 4. `authStore.fetchCurrentUser` works after login. + 5. Logout clears session/tokens. + Write/verify Playwright E2E tests for each of these flows (email/password signup, email/password login, Google login, Apple login, logout)." + * **Verification:** All authentication E2E tests pass. Users can reliably sign up, log in (via all methods), and log out. + +--- + +**PHASE 1: FULL-FEATURED LIST & ITEM MANAGEMENT (Your V1 Baseline)** + +* **Task 1.1: Backend - Robust List CRUD & Permissions.** + * **File(s):** `be/app/api/v1/endpoints/lists.py`, `be/app/crud/list.py`, `be/app/models.py` (List model). + * **Objective:** Implement all list CRUD operations with correct permissions and optimistic locking. + * **Action for LLM (Iterate for each endpoint - Create, Get All, Get ID, Update, Delete):** "For the `[CREATE/GET/UPDATE/DELETE]` list endpoint in `lists.py` and its corresponding CRUD function in `crud_list.py`: + 1. Ensure user permissions are checked (creator for personal lists, group member for group lists; `require_creator` for delete/sensitive updates). + 2. For `UPDATE` and `DELETE`, implement optimistic locking using the `version` field (HTTP 409 on mismatch). + 3. For `CREATE`, handle potential `DatabaseIntegrityError` for unique list names within a group/user's personal lists by returning a 409 with the existing list's details (as noted in your endpoint). + 4. Ensure all relationships (creator, group, items) are correctly handled and loaded/returned as per `ListPublic` and `ListDetail` schemas. + 5. Write comprehensive unit tests in `be/tests/crud/test_list.py` and API tests for these scenarios." + * **Verification:** All API endpoints for lists function as documented in `mitlist_doc.md` (V1 scope), pass all tests, and handle permissions/versioning correctly. + +* **Task 1.2: Frontend - Full List UI/UX.** + * **File(s):** `fe/src/pages/ListsPage.vue`, `fe/src/pages/ListDetailPage.vue`, `fe/src/components/CreateListModal.vue`. + * **Objective:** Implement all UI for list management. + * **Action for LLM (Iterate per feature):** "Implement the UI in Vue components for: + 1. Displaying all accessible lists (`ListsPage.vue`). + 2. Creating new lists (personal or group-associated) using `CreateListModal.vue`. + 3. Viewing list details and items (`ListDetailPage.vue`). + 4. Updating list name/description (handle `version` for optimistic lock, show conflict notification). + 5. Deleting lists (with confirmation, handle `version` for optimistic lock)." + * **Verification:** All list management features are usable from the frontend. E2E tests (from `fe/e2e/lists.spec.ts`, unskip and fix) pass. + +* **Task 1.3: Backend - Robust Item CRUD & Permissions.** + * **File(s):** `be/app/api/v1/endpoints/items.py`, `be/app/crud/item.py`, `be/app/models.py` (Item model). + * **Objective:** Implement all item CRUD operations with permissions and optimistic locking. + * **Action for LLM (Iterate for each endpoint):** "Similar to Task 1.1, implement/review CRUD operations for items within a list. Ensure: + 1. Permissions are based on access to the parent list. + 2. `UPDATE` and `DELETE` use optimistic locking with `version`. + 3. Updating `is_complete` correctly sets/unsets `completed_by_id`. + 4. Item price can be added/updated. + 5. Write comprehensive unit and API tests." + * **Verification:** All item CRUD endpoints function correctly, pass tests, handle permissions/versioning. + +* **Task 1.4: Frontend - Full Item UI/UX in List Detail.** + * **File(s):** `fe/src/pages/ListDetailPage.vue`. + * **Objective:** Implement all UI for item management within a list. + * **Action for LLM:** "In `ListDetailPage.vue`, implement UI for: + 1. Adding new items (name, quantity). + 2. Displaying items with their details. + 3. Marking items as complete/incomplete (checkbox) and updating the backend (handle `version`). + 4. Adding/editing price for completed items (handle `version`). + 5. Editing item name/quantity (handle `version`). + 6. Deleting items (with confirmation, handle `version`). + Handle loading states and errors for each action." + * **Verification:** Item management is fully functional in the UI. E2E tests (from `fe/e2e/lists.spec.ts`, unskip and fix for items) pass. + +* **Task 1.5: Backend - OCR Integration (Gemini).** + * **File(s):** `be/app/core/gemini.py`, `be/app/api/v1/endpoints/ocr.py`. + * **Objective:** Reliable item extraction from images. + * **Action for LLM:** "Refine and test `gemini.py` and the `/ocr/extract-items` endpoint. + 1. Ensure robust error handling for API key issues, quota limits (`OCRQuotaExceededError`), service unavailability (`OCRServiceUnavailableError`), and general processing errors (`OCRProcessingError`). Return appropriate HTTP status codes. + 2. Thoroughly test the `OCR_ITEM_EXTRACTION_PROMPT` with various real-world receipt/list images to optimize accuracy. Handle cases where Gemini returns no usable text or indicates the image is not a list. + 3. Ensure `ALLOWED_IMAGE_TYPES` and `MAX_FILE_SIZE_MB` are enforced." + * **Verification:** OCR endpoint is stable, handles errors gracefully, and provides reasonably accurate item extraction. + +* **Task 1.6: Frontend - OCR UI Flow.** + * **File(s):** `fe/src/pages/ListDetailPage.vue` (or a dedicated OCR component). + * **Objective:** Smooth user experience for adding items via OCR. + * **Action for LLM:** "Implement the OCR flow in the Vue frontend: + 1. UI for image capture/upload (using `input capture` or `getUserMedia`). + 2. File validation (type, size) on the client side before upload. + 3. API call to the backend OCR endpoint with loading indicators. + 4. Display extracted items in an editable format (e.g., list of text inputs). + 5. Allow users to add, edit, or remove items from this extracted list. + 6. Button to add the confirmed items to the current shopping list (batch API calls if necessary)." + * **Verification:** Users can successfully use the OCR feature from image capture to adding items to a list. + +--- + +**PHASE 2: FULL-FEATURED COST SPLITTING & TRACEABILITY (Your V1 Differentiator)** + +This phase assumes Phase 0 (financial fixes) is COMPLETE. + +* **Task 2.1: Backend - Expense Creation with All Split Types.** + * **File(s):** `be/app/crud/expense.py` (specifically `_generate_expense_splits` and its helpers like `_create_equal_splits`, `_create_exact_amount_splits`, etc.), `be/app/api/v1/endpoints/financials.py` (`create_new_expense`). + * **Objective:** Support creation of expenses with all V1 split types (Equal, Exact Amounts, Percentage, Shares, Item-Based). + * **Action for LLM:** "For each `SplitTypeEnum` defined in your V1 scope: + 1. In `crud_expense.py`, ensure the corresponding helper function (`_create_exact_amount_splits`, `_create_percentage_splits`, `_create_shares_splits`, `_create_item_based_splits`) correctly calculates and creates `ExpenseSplitModel` instances based on the input `expense_in.splits_in` (for exact, percentage, shares) or item data (for item-based). + 2. Validate input: sum of exact amounts/percentages/shares must match total expense. Item-based splits should correctly sum item prices. + 3. Ensure `overall_settlement_status` on `ExpenseModel` and `status` on `ExpenseSplitModel` are initialized correctly (`unpaid`). + 4. Write unit tests in `be/tests/crud/test_expense.py` for each split type, covering valid and invalid scenarios." + * **Verification:** Expenses can be created with all V1 split types, and the resulting splits are mathematically correct and persisted. + +* **Task 2.2: Frontend - Expense Creation UI for All Split Types.** + * **File(s):** `fe/src/components/CreateExpenseForm.vue` (or similar). + * **Objective:** UI allows users to create expenses with any V1 split type. + * **Action for LLM:** "Enhance `CreateExpenseForm.vue`: + 1. Add UI elements (dynamic forms) to input split details when `split_type` is Exact Amounts, Percentage, or Shares (e.g., inputs for each user's amount/percentage/share). + 2. For Item-Based splits, the UI might involve selecting items from the list or this might be triggered automatically post-OCR/price entry – clarify flow. + 3. Ensure the form correctly constructs the `ExpenseCreate` payload, including `splits_in` where appropriate. + 4. Validate inputs on the frontend (e.g., sum of percentages is 100)." + * **Verification:** Users can create expenses with all supported split types through the UI. + +* **Task 2.3: Backend & Frontend - Viewing Expenses and Settlement Status.** + * **File(s):** `be/app/api/v1/endpoints/financials.py` (GET expenses), `be/app/schemas/expense.py` (ensure `ExpensePublic`, `ExpenseSplitPublic` include status fields), `fe/src/pages/ListDetailPage.vue` (or wherever expenses are displayed). + * **Objective:** Users can clearly see expenses, their splits, and the settlement status of each. + * **Action for LLM:** + 1. "BE: Ensure `ExpensePublic` and `ExpenseSplitPublic` schemas include `overall_settlement_status`, `status`, `paid_at`, and `settlement_activities`. Ensure GET expense endpoints populate these fields." + 2. "FE: In `ListDetailPage.vue`, display expenses associated with the list. For each expense, show its splits, including who owes what, the current `status` (Unpaid, Partially Paid, Paid), and when it was paid (`paid_at`). Display `overall_settlement_status` for the expense." + * **Verification:** UI accurately displays detailed expense and split information, including settlement statuses. + +* **Task 2.4: Backend & Frontend - Recording Settlement Activities.** + * **File(s):** `be/app/api/v1/endpoints/financials.py` (POST `/expense_splits/{expense_split_id}/settle`), `be/app/crud/settlement_activity.py`, `be/app/models.py` (`SettlementActivity`), `fe/src/stores/listDetailStore.ts`, `fe/src/pages/ListDetailPage.vue`. + * **Objective:** Users can mark their `ExpenseShare` (represented by `ExpenseSplit`) as paid, creating a traceable `SettlementActivity`. + * **Action for LLM:** + 1. "BE: Implement `crud_settlement_activity.create_settlement_activity`. When a `SettlementActivity` is created for an `ExpenseSplit`: + * Update the `ExpenseSplit.status` (to `partially_paid` or `paid`) based on `amount_paid` vs `owed_amount`. If fully paid, set `ExpenseSplit.paid_at`. + * Update the parent `Expense.overall_settlement_status` based on the status of all its splits. + 2. "BE: Ensure the `/financials/expense_splits/{expense_split_id}/settle` endpoint correctly calls the CRUD function and handles permissions (user can settle their own split, or group owner can settle for others)." + 3. "FE: In `ListDetailPage.vue`, provide a button/action for a user to "Settle My Share" for an `ExpenseSplit` they owe. This should trigger the `settleExpenseSplit` action in `listDetailStore.ts` (which now calls the real API - Task 0.2)." + * **Verification:** Users can settle their expense shares. The backend correctly updates statuses. UI reflects these changes. All settlement activities are logged in the `settlement_activities` table. + +--- + +**PHASE 3: FULL-FEATURED CHORE MANAGEMENT (Your V1 Scope)** + +* **Task 3.1: Backend - Chore CRUD (Personal & Group) with Recurrence.** + * **File(s):** `be/app/api/v1/endpoints/chores.py`, `be/app/crud/chore.py`, `be/app/core/chore_utils.py`. + * **Objective:** Full CRUD for personal and group chores, including all recurrence logic. + * **Action for LLM:** "Implement/Refine `create_chore`, `get_chore_by_id`, `get_personal_chores`, `get_chores_by_group_id`, `update_chore`, `delete_chore` in `crud_chore.py` and corresponding endpoints in `chores.py`. + 1. For `create_chore` and `update_chore`: If `frequency` is `custom`, `custom_interval_days` must be provided. Validate chore `type` (`personal` vs `group`) and `group_id` consistency. + 2. Implement `calculate_next_due_date` in `chore_utils.py` to accurately calculate the next due date based on `frequency`, `custom_interval_days`, and `last_completed_at`. Handle edge cases for monthly recurrence (e.g., due on 31st). + 3. Ensure appropriate permissions are checked (e.g., user must be member of group to create/view group chores)." + * **Verification:** All chore CRUD operations work for both personal and group chores. Recurrence settings are handled correctly. Permissions are enforced. Unit and API tests cover these. + +* **Task 3.2: Frontend - Chore Management UI.** + * **File(s):** `fe/src/pages/ChoresPage.vue`, `fe/src/pages/PersonalChoresPage.vue`. + * **Objective:** UI for users to manage personal and group chores. + * **Action for LLM:** "Implement UI in `ChoresPage.vue` (for all chores, filterable by group) and `PersonalChoresPage.vue`: + 1. Display lists of chores, showing name, description, frequency, next due date. + 2. Modals/forms for creating and editing chores (personal and group, including all recurrence options). + 3. Functionality to delete chores (with confirmation)." + * **Verification:** Users can manage personal and group chores via the UI. E2E tests for chore CRUD pass. + +* **Task 3.3: Backend & Frontend - Chore Assignments & Completion.** + * **File(s):** `be/app/models.py` (`ChoreAssignment`), `be/app/crud/chore.py` (new functions for assignments), `be/app/api/v1/endpoints/chores.py` (new endpoints for assignments), `fe/src/pages/ChoresPage.vue` (or a "My Chores" page). + * **Objective:** Chores can be assigned, and assignees can mark them complete, triggering recurrence updates. + * **Action for LLM:** + 1. "BE: Define `ChoreAssignment` model. Create CRUD functions and API endpoints for: + * Manually assigning a chore instance (from a recurring `Chore`) to a user for a specific `due_date`. + * Allowing an assigned user to mark their `ChoreAssignment` as complete. This should set `completed_at`. + * When a recurring chore's assignment is completed, update the parent `Chore.last_completed_at` and use `calculate_next_due_date` to set its new `Chore.next_due_date`. + 2. "FE: Implement UI for: + * Group admins/owners to assign chore instances to members. + * A "My Chores" view where users see their pending `ChoreAssignments`. + * Users to mark their assigned chores as complete." + * **Verification:** Chore assignment and completion workflow is functional. Recurrence updates correctly. + +--- + +**PHASE 4: FULL PWA OFFLINE & BACKGROUND SYNC (Your V1 Scope)** + +* **Task 4.1: Frontend - Implement Offline Action Queuing for All V1 Features.** + * **File(s):** `fe/src/stores/offline.ts`. + * **Objective:** All mutable actions from V1 (list/item/chore CRUD, expense/settlement creation, chore completion) are queued when offline. + * **Action for LLM:** "Extend `OfflineAction` type and `processAction` in `offline.ts` to handle all mutable operations for lists, items, expenses, settlements, and chores (CRUD, marking complete, etc.) as defined in your V1 feature set. Ensure each action type has a corresponding case in `processAction` that makes the correct API call using `fetch` (or a thin wrapper around `apiClient` if `apiClient` is made compatible with background sync context)." + * **Verification:** When offline, performing any V1 action adds it to the `pendingActions` queue in `localStorage`. + +* **Task 4.2: Frontend - Robust Background Sync & Retry.** + * **File(s):** `fe/src/sw.ts`, `fe/src/stores/offline.ts`. + * **Objective:** Queued actions are reliably synced via the service worker's Background Sync API. + * **Action for LLM:** "Ensure the `BackgroundSyncPlugin` in `sw.ts` is correctly configured for the `offline-actions-queue`. The `sync` event handler in the service worker must correctly iterate through actions from `offlineStore.pendingActions` (potentially via `IDBKeyval` or directly if store is accessible in SW) and attempt to replay them using `fetch`. Ensure `maxRetentionTime` is set. Test retry behavior for failed syncs." + * **Verification:** Offline actions are processed when online. Failed actions are retried by background sync. + +* **Task 4.3: Frontend - Implement Conflict Resolution UI & Logic.** + * **File(s):** `fe/src/components/ConflictResolutionDialog.vue`, `fe/src/stores/offline.ts`. + * **Objective:** Users can resolve data conflicts arising from offline sync. + * **Action for LLM:** + 1. "In `offline.ts`, when `processAction` receives a 409 conflict from the backend: + * Extract local data (from the `action.payload`) and server data (from the 409 response body). + * Populate `currentConflict` with this data and set `showConflictDialog = true`. + * Halt further queue processing until this conflict is resolved. + 2. "In `ConflictResolutionDialog.vue`, display the local and server versions of the conflicting data clearly. Provide options: 'Keep Local Version', 'Keep Server Version', and 'Merge Changes' (if applicable and simple merge logic can be defined, e.g., field-by-field choice). + 3. "Implement `handleConflictResolution` in `offline.ts`. Based on user's choice: + * 'Keep Local': Re-submit the local action, potentially with the server's latest `version` number to overcome the 409. + * 'Keep Server': Discard the local action. + * 'Merge': Construct merged data and PUT it to the server (with server's `version`). + * Remove the original conflicting action from `pendingActions` on successful resolution. Resume queue processing." + * **Verification:** Conflicts are detected. Dialog appears. User can choose a resolution strategy. The chosen strategy is applied, and the queue continues. + +--- + +**PHASE 5: FINAL TESTING, POLISHING, DEPLOYMENT (Your V1 Launch)** + +* **Task 5.1: Comprehensive E2E Testing of ALL V1 Features.** + * **Objective:** All user flows, including offline scenarios and conflict resolution, are covered by E2E tests. + * **Action for LLM:** "Write new Playwright E2E tests and update existing ones (`fe/e2e/`) to cover every feature in your `mitlist_doc.md` V1 scope. This includes: + * All CRUD for lists, items, group chores, personal chores. + * All expense creation types, viewing expenses/splits, settling shares. + * OCR item addition. + * Offline scenarios: create/update/delete an item offline, go online, verify sync. + * Simulate a conflict and test the conflict resolution dialog flow." + * **Verification:** All E2E tests pass reliably. + +* **Task 5.2: Final UI/UX Polish & Accessibility (Valerie UI).** + * **Action for LLM:** "Review all UI components and pages. Ensure consistent use of Valerie UI. Check for usability issues, clear error messaging, and intuitive workflows. Perform an accessibility check (e.g., using browser dev tools, axe-core plugin) and address any critical violations (keyboard navigation, ARIA attributes, color contrast)." + * **Verification:** App feels polished, is easy to use, and meets basic accessibility standards. + +* **Task 5.3: CI/CD Pipeline Enhancement.** + * **File(s):** `.gitea/workflows/ci.yml`. + * **Action for LLM:** "Update the Gitea CI workflow: + 1. Add steps to run all backend unit and integration tests. + 2. Add steps to build the frontend. + 3. Add steps to run all Playwright E2E tests against the built frontend (potentially using a preview server). + 4. Ensure the pipeline fails if any tests fail. + 5. (Optional Advanced) Add steps for automated deployment to staging/production on successful builds of specific branches." + * **Verification:** CI pipeline automatically tests backend and frontend, including E2E, on commits/PRs. + +* **Task 5.4: DevOps - Dockerization & Deployment Prep.** + * **File(s):** `be/Dockerfile`, `fe/Dockerfile`, `docker-compose.yml`. + * **Action for LLM:** "Review Dockerfiles for backend and frontend for production readiness (e.g., multi-stage builds, minimal image size, correct CMD). Ensure `docker-compose.yml` correctly sets up services for local development and can serve as a basis for production deployment environment variables. Document deployment steps for chosen cloud platforms (as per your doc)." + * **Verification:** Application can be built and run reliably using Docker locally. Deployment strategy is clear. + +* **Task 5.5: Error Tracking & Logging Review.** + * **File(s):** `be/app/main.py` (Sentry init), `fe/src/main.ts` (Sentry init). + * **Action for LLM:** "Verify Sentry is correctly initialized in both frontend and backend. Test that errors are captured in Sentry. Review backend logging (`LOG_LEVEL`, `LOG_FORMAT` in `be/app/config.py`) to ensure it provides useful information for debugging in production." + * **Verification:** Errors from both frontend and backend are reported to Sentry. Logs are informative. + diff --git a/be/app/api/v1/endpoints/chores.py b/be/app/api/v1/endpoints/chores.py index 04cbb1e..01a15d7 100644 --- a/be/app/api/v1/endpoints/chores.py +++ b/be/app/api/v1/endpoints/chores.py @@ -2,19 +2,38 @@ import logging from typing import List as PyList, Optional -from fastapi import APIRouter, Depends, HTTPException, status +from fastapi import APIRouter, Depends, HTTPException, status, Response from sqlalchemy.ext.asyncio import AsyncSession -from app.database import get_transactional_session +from app.database import get_transactional_session, get_session from app.auth import current_active_user from app.models import User as UserModel, Chore as ChoreModel, ChoreTypeEnum -from app.schemas.chore import ChoreCreate, ChoreUpdate, ChorePublic +from app.schemas.chore import ChoreCreate, ChoreUpdate, ChorePublic, ChoreAssignmentCreate, ChoreAssignmentUpdate, ChoreAssignmentPublic from app.crud import chore as crud_chore from app.core.exceptions import ChoreNotFoundError, PermissionDeniedError, GroupNotFoundError, DatabaseIntegrityError logger = logging.getLogger(__name__) router = APIRouter() +# Add this new endpoint before the personal chores section +@router.get( + "/all", + response_model=PyList[ChorePublic], + summary="List All Chores", + tags=["Chores"] +) +async def list_all_chores( + db: AsyncSession = Depends(get_session), # Use read-only session for GET + current_user: UserModel = Depends(current_active_user), +): + """Retrieves all chores (personal and group) for the current user in a single optimized request.""" + logger.info(f"User {current_user.email} listing all their chores") + + # Use the optimized function that reduces database queries + all_chores = await crud_chore.get_all_user_chores(db=db, user_id=current_user.id) + + return all_chores + # --- Personal Chores Endpoints --- @router.post( @@ -51,7 +70,7 @@ async def create_personal_chore( tags=["Chores", "Personal Chores"] ) async def list_personal_chores( - db: AsyncSession = Depends(get_transactional_session), + db: AsyncSession = Depends(get_session), current_user: UserModel = Depends(current_active_user), ): """Retrieves all personal chores for the current user.""" @@ -180,7 +199,7 @@ async def create_group_chore( ) async def list_group_chores( group_id: int, - db: AsyncSession = Depends(get_transactional_session), + db: AsyncSession = Depends(get_session), current_user: UserModel = Depends(current_active_user), ): """Retrieves all chores for a specific group, if the user is a member.""" @@ -266,4 +285,169 @@ async def delete_group_chore( raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=e.detail) except DatabaseIntegrityError as e: logger.error(f"DatabaseIntegrityError deleting group chore {chore_id} for {current_user.email}: {e.detail}", exc_info=True) + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.detail) + +# === CHORE ASSIGNMENT ENDPOINTS === + +@router.post( + "/assignments", + response_model=ChoreAssignmentPublic, + status_code=status.HTTP_201_CREATED, + summary="Create Chore Assignment", + tags=["Chore Assignments"] +) +async def create_chore_assignment( + assignment_in: ChoreAssignmentCreate, + db: AsyncSession = Depends(get_transactional_session), + current_user: UserModel = Depends(current_active_user), +): + """Creates a new chore assignment. User must have permission to manage the chore.""" + logger.info(f"User {current_user.email} creating assignment for chore {assignment_in.chore_id}") + try: + return await crud_chore.create_chore_assignment(db=db, assignment_in=assignment_in, user_id=current_user.id) + except ChoreNotFoundError as e: + logger.warning(f"Chore {e.chore_id} not found for assignment creation by user {current_user.email}.") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.detail) + except PermissionDeniedError as e: + logger.warning(f"Permission denied for user {current_user.email} creating assignment: {e.detail}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=e.detail) + except ValueError as e: + logger.warning(f"ValueError creating assignment for user {current_user.email}: {str(e)}") + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + except DatabaseIntegrityError as e: + logger.error(f"DatabaseIntegrityError creating assignment for {current_user.email}: {e.detail}", exc_info=True) + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.detail) + +@router.get( + "/assignments/my", + response_model=PyList[ChoreAssignmentPublic], + summary="List My Chore Assignments", + tags=["Chore Assignments"] +) +async def list_my_assignments( + include_completed: bool = False, + db: AsyncSession = Depends(get_session), # Use read-only session for GET + current_user: UserModel = Depends(current_active_user), +): + """Retrieves all chore assignments for the current user.""" + logger.info(f"User {current_user.email} listing their assignments (include_completed={include_completed})") + try: + return await crud_chore.get_user_assignments(db=db, user_id=current_user.id, include_completed=include_completed) + except Exception as e: + logger.error(f"Error listing assignments for user {current_user.email}: {e}", exc_info=True) + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed to retrieve assignments") + +@router.get( + "/chores/{chore_id}/assignments", + response_model=PyList[ChoreAssignmentPublic], + summary="List Chore Assignments", + tags=["Chore Assignments"] +) +async def list_chore_assignments( + chore_id: int, + db: AsyncSession = Depends(get_session), # Use read-only session for GET + current_user: UserModel = Depends(current_active_user), +): + """Retrieves all assignments for a specific chore.""" + logger.info(f"User {current_user.email} listing assignments for chore {chore_id}") + try: + return await crud_chore.get_chore_assignments(db=db, chore_id=chore_id, user_id=current_user.id) + except ChoreNotFoundError as e: + logger.warning(f"Chore {e.chore_id} not found for assignment listing by user {current_user.email}.") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.detail) + except PermissionDeniedError as e: + logger.warning(f"Permission denied for user {current_user.email} listing assignments for chore {chore_id}: {e.detail}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=e.detail) + +@router.put( + "/assignments/{assignment_id}", + response_model=ChoreAssignmentPublic, + summary="Update Chore Assignment", + tags=["Chore Assignments"] +) +async def update_chore_assignment( + assignment_id: int, + assignment_in: ChoreAssignmentUpdate, + db: AsyncSession = Depends(get_transactional_session), + current_user: UserModel = Depends(current_active_user), +): + """Updates a chore assignment. Only assignee can mark complete, managers can reschedule.""" + logger.info(f"User {current_user.email} updating assignment {assignment_id}") + try: + updated_assignment = await crud_chore.update_chore_assignment( + db=db, assignment_id=assignment_id, assignment_in=assignment_in, user_id=current_user.id + ) + if not updated_assignment: + raise ChoreNotFoundError(assignment_id=assignment_id) + return updated_assignment + except ChoreNotFoundError as e: + logger.warning(f"Assignment {assignment_id} not found for user {current_user.email} during update.") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.detail) + except PermissionDeniedError as e: + logger.warning(f"Permission denied for user {current_user.email} updating assignment {assignment_id}: {e.detail}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=e.detail) + except ValueError as e: + logger.warning(f"ValueError updating assignment {assignment_id} for user {current_user.email}: {str(e)}") + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + except DatabaseIntegrityError as e: + logger.error(f"DatabaseIntegrityError updating assignment {assignment_id} for {current_user.email}: {e.detail}", exc_info=True) + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.detail) + +@router.delete( + "/assignments/{assignment_id}", + status_code=status.HTTP_204_NO_CONTENT, + summary="Delete Chore Assignment", + tags=["Chore Assignments"] +) +async def delete_chore_assignment( + assignment_id: int, + db: AsyncSession = Depends(get_transactional_session), + current_user: UserModel = Depends(current_active_user), +): + """Deletes a chore assignment. User must have permission to manage the chore.""" + logger.info(f"User {current_user.email} deleting assignment {assignment_id}") + try: + success = await crud_chore.delete_chore_assignment(db=db, assignment_id=assignment_id, user_id=current_user.id) + if not success: + raise ChoreNotFoundError(assignment_id=assignment_id) + return Response(status_code=status.HTTP_204_NO_CONTENT) + except ChoreNotFoundError as e: + logger.warning(f"Assignment {assignment_id} not found for user {current_user.email} during delete.") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.detail) + except PermissionDeniedError as e: + logger.warning(f"Permission denied for user {current_user.email} deleting assignment {assignment_id}: {e.detail}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=e.detail) + except DatabaseIntegrityError as e: + logger.error(f"DatabaseIntegrityError deleting assignment {assignment_id} for {current_user.email}: {e.detail}", exc_info=True) + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.detail) + +@router.patch( + "/assignments/{assignment_id}/complete", + response_model=ChoreAssignmentPublic, + summary="Mark Assignment Complete", + tags=["Chore Assignments"] +) +async def complete_chore_assignment( + assignment_id: int, + db: AsyncSession = Depends(get_transactional_session), + current_user: UserModel = Depends(current_active_user), +): + """Convenience endpoint to mark an assignment as complete.""" + logger.info(f"User {current_user.email} marking assignment {assignment_id} as complete") + assignment_update = ChoreAssignmentUpdate(is_complete=True) + try: + updated_assignment = await crud_chore.update_chore_assignment( + db=db, assignment_id=assignment_id, assignment_in=assignment_update, user_id=current_user.id + ) + if not updated_assignment: + raise ChoreNotFoundError(assignment_id=assignment_id) + return updated_assignment + except ChoreNotFoundError as e: + logger.warning(f"Assignment {assignment_id} not found for user {current_user.email} during completion.") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.detail) + except PermissionDeniedError as e: + logger.warning(f"Permission denied for user {current_user.email} completing assignment {assignment_id}: {e.detail}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=e.detail) + except DatabaseIntegrityError as e: + logger.error(f"DatabaseIntegrityError completing assignment {assignment_id} for {current_user.email}: {e.detail}", exc_info=True) raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.detail) \ No newline at end of file diff --git a/be/app/api/v1/endpoints/costs.py b/be/app/api/v1/endpoints/costs.py index a016e98..7124715 100644 --- a/be/app/api/v1/endpoints/costs.py +++ b/be/app/api/v1/endpoints/costs.py @@ -409,9 +409,15 @@ async def get_group_balance_summary( # Calculate suggested settlements suggested_settlements = calculate_suggested_settlements(final_user_balances) + # Calculate overall totals for the group + overall_total_expenses = sum(expense.total_amount for expense in expenses) + overall_total_settlements = sum(settlement.amount for settlement in settlements) + return GroupBalanceSummary( group_id=db_group_for_check.id, group_name=db_group_for_check.name, + overall_total_expenses=overall_total_expenses.quantize(Decimal("0.01"), rounding=ROUND_HALF_UP), + overall_total_settlements=overall_total_settlements.quantize(Decimal("0.01"), rounding=ROUND_HALF_UP), user_balances=final_user_balances, suggested_settlements=suggested_settlements ) \ No newline at end of file diff --git a/be/app/api/v1/endpoints/financials.py b/be/app/api/v1/endpoints/financials.py index 33bad42..2f9a9ad 100644 --- a/be/app/api/v1/endpoints/financials.py +++ b/be/app/api/v1/endpoints/financials.py @@ -3,11 +3,19 @@ import logging from fastapi import APIRouter, Depends, HTTPException, status, Query, Response from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy import select +from sqlalchemy.orm import joinedload from typing import List as PyList, Optional, Sequence from app.database import get_transactional_session from app.auth import current_active_user -from app.models import User as UserModel, Group as GroupModel, List as ListModel, UserGroup as UserGroupModel, UserRoleEnum +from app.models import ( + User as UserModel, + Group as GroupModel, + List as ListModel, + UserGroup as UserGroupModel, + UserRoleEnum, + ExpenseSplit as ExpenseSplitModel +) from app.schemas.expense import ( ExpenseCreate, ExpensePublic, SettlementCreate, SettlementPublic, diff --git a/be/app/api/v1/endpoints/groups.py b/be/app/api/v1/endpoints/groups.py index 185292a..4aad4b9 100644 --- a/be/app/api/v1/endpoints/groups.py +++ b/be/app/api/v1/endpoints/groups.py @@ -5,7 +5,7 @@ from typing import List from fastapi import APIRouter, Depends, HTTPException, status from sqlalchemy.ext.asyncio import AsyncSession -from app.database import get_transactional_session +from app.database import get_transactional_session, get_session from app.auth import current_active_user from app.models import User as UserModel, UserRoleEnum # Import model and enum from app.schemas.group import GroupCreate, GroupPublic @@ -54,7 +54,7 @@ async def create_group( tags=["Groups"] ) async def read_user_groups( - db: AsyncSession = Depends(get_transactional_session), + db: AsyncSession = Depends(get_session), # Use read-only session for GET current_user: UserModel = Depends(current_active_user), ): """Retrieves all groups the current user is a member of.""" @@ -71,7 +71,7 @@ async def read_user_groups( ) async def read_group( group_id: int, - db: AsyncSession = Depends(get_transactional_session), + db: AsyncSession = Depends(get_session), # Use read-only session for GET current_user: UserModel = Depends(current_active_user), ): """Retrieves details for a specific group, including members, if the user is part of it.""" @@ -132,7 +132,7 @@ async def create_group_invite( ) async def get_group_active_invite( group_id: int, - db: AsyncSession = Depends(get_transactional_session), + db: AsyncSession = Depends(get_session), # Use read-only session for GET current_user: UserModel = Depends(current_active_user), ): """Retrieves the active invite code for the group. Requires group membership (owner/admin to be stricter later if needed).""" @@ -248,7 +248,7 @@ async def remove_group_member( ) async def read_group_lists( group_id: int, - db: AsyncSession = Depends(get_transactional_session), + db: AsyncSession = Depends(get_session), # Use read-only session for GET current_user: UserModel = Depends(current_active_user), ): """Retrieves all lists belonging to a specific group, if the user is a member.""" diff --git a/be/app/config.py b/be/app/config.py index 0f7073e..dbc8d71 100644 --- a/be/app/config.py +++ b/be/app/config.py @@ -83,7 +83,7 @@ Organic Bananas ROOT_MESSAGE: str = "Welcome to the Shared Lists API! Docs available at /api/docs" # --- Logging Settings --- - LOG_LEVEL: str = "INFO" + LOG_LEVEL: str = "WARNING" LOG_FORMAT: str = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" # --- Health Check Settings --- @@ -127,7 +127,7 @@ Organic Bananas # Session Settings SESSION_SECRET_KEY: str = "your-session-secret-key" # Change this in production - ACCESS_TOKEN_EXPIRE_MINUTES: int = 30 + ACCESS_TOKEN_EXPIRE_MINUTES: int = 480 # 8 hours instead of 30 minutes class Config: env_file = ".env" env_file_encoding = 'utf-8' diff --git a/be/app/crud/chore.py b/be/app/crud/chore.py index a7e1540..cbac95e 100644 --- a/be/app/crud/chore.py +++ b/be/app/crud/chore.py @@ -1,18 +1,69 @@ from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.future import select from sqlalchemy.orm import selectinload +from sqlalchemy import union_all from typing import List, Optional import logging -from datetime import date +from datetime import date, datetime -from app.models import Chore, Group, User, ChoreFrequencyEnum, ChoreTypeEnum -from app.schemas.chore import ChoreCreate, ChoreUpdate +from app.models import Chore, Group, User, ChoreAssignment, ChoreFrequencyEnum, ChoreTypeEnum, UserGroup +from app.schemas.chore import ChoreCreate, ChoreUpdate, ChoreAssignmentCreate, ChoreAssignmentUpdate from app.core.chore_utils import calculate_next_due_date from app.crud.group import get_group_by_id, is_user_member from app.core.exceptions import ChoreNotFoundError, GroupNotFoundError, PermissionDeniedError, DatabaseIntegrityError logger = logging.getLogger(__name__) +async def get_all_user_chores(db: AsyncSession, user_id: int) -> List[Chore]: + """Gets all chores (personal and group) for a user in optimized queries.""" + + # Get personal chores query + personal_chores_query = ( + select(Chore) + .where( + Chore.created_by_id == user_id, + Chore.type == ChoreTypeEnum.personal + ) + ) + + # Get user's group IDs first + user_groups_result = await db.execute( + select(UserGroup.group_id).where(UserGroup.user_id == user_id) + ) + user_group_ids = user_groups_result.scalars().all() + + all_chores = [] + + # Execute personal chores query + personal_result = await db.execute( + personal_chores_query + .options( + selectinload(Chore.creator), + selectinload(Chore.assignments).selectinload(ChoreAssignment.assigned_user) + ) + .order_by(Chore.next_due_date, Chore.name) + ) + all_chores.extend(personal_result.scalars().all()) + + # If user has groups, get all group chores in one query + if user_group_ids: + group_chores_result = await db.execute( + select(Chore) + .where( + Chore.group_id.in_(user_group_ids), + Chore.type == ChoreTypeEnum.group + ) + .options( + selectinload(Chore.creator), + selectinload(Chore.group), + selectinload(Chore.assignments).selectinload(ChoreAssignment.assigned_user) + ) + .order_by(Chore.next_due_date, Chore.name) + ) + all_chores.extend(group_chores_result.scalars().all()) + + return all_chores + async def create_chore( db: AsyncSession, chore_in: ChoreCreate, @@ -20,53 +71,56 @@ async def create_chore( group_id: Optional[int] = None ) -> Chore: """Creates a new chore, either personal or within a specific group.""" - if chore_in.type == ChoreTypeEnum.group: - if not group_id: - raise ValueError("group_id is required for group chores") - # Validate group existence and user membership - group = await get_group_by_id(db, group_id) - if not group: - raise GroupNotFoundError(group_id) - if not await is_user_member(db, group_id, user_id): - raise PermissionDeniedError(detail=f"User {user_id} not a member of group {group_id}") - else: # personal chore - if group_id: - raise ValueError("group_id must be None for personal chores") + # Use the transaction pattern from the FastAPI strategy + async with db.begin_nested() if db.in_transaction() else db.begin(): + if chore_in.type == ChoreTypeEnum.group: + if not group_id: + raise ValueError("group_id is required for group chores") + # Validate group existence and user membership + group = await get_group_by_id(db, group_id) + if not group: + raise GroupNotFoundError(group_id) + if not await is_user_member(db, group_id, user_id): + raise PermissionDeniedError(detail=f"User {user_id} not a member of group {group_id}") + else: # personal chore + if group_id: + raise ValueError("group_id must be None for personal chores") - db_chore = Chore( - **chore_in.model_dump(exclude_unset=True, exclude={'group_id'}), - group_id=group_id, - created_by_id=user_id, - ) - - # Specific check for custom frequency - if chore_in.frequency == ChoreFrequencyEnum.custom and chore_in.custom_interval_days is None: - raise ValueError("custom_interval_days must be set for custom frequency chores.") - - db.add(db_chore) - try: - await db.commit() - await db.refresh(db_chore) - result = await db.execute( - select(Chore) - .where(Chore.id == db_chore.id) - .options(selectinload(Chore.creator), selectinload(Chore.group)) + db_chore = Chore( + **chore_in.model_dump(exclude_unset=True, exclude={'group_id'}), + group_id=group_id, + created_by_id=user_id, ) - return result.scalar_one() - except Exception as e: - await db.rollback() - logger.error(f"Error creating chore: {e}", exc_info=True) - raise DatabaseIntegrityError(f"Could not create chore. Error: {str(e)}") -async def get_chore_by_id( - db: AsyncSession, - chore_id: int, -) -> Optional[Chore]: - """Gets a chore by its ID with creator and group info.""" + # Specific check for custom frequency + if chore_in.frequency == ChoreFrequencyEnum.custom and chore_in.custom_interval_days is None: + raise ValueError("custom_interval_days must be set for custom frequency chores.") + + db.add(db_chore) + await db.flush() # Get the ID for the chore + + try: + # Load relationships for the response with eager loading + result = await db.execute( + select(Chore) + .where(Chore.id == db_chore.id) + .options( + selectinload(Chore.creator), + selectinload(Chore.group), + selectinload(Chore.assignments) + ) + ) + return result.scalar_one() + except Exception as e: + logger.error(f"Error creating chore: {e}", exc_info=True) + raise DatabaseIntegrityError(f"Could not create chore. Error: {str(e)}") + +async def get_chore_by_id(db: AsyncSession, chore_id: int) -> Optional[Chore]: + """Gets a chore by ID.""" result = await db.execute( select(Chore) .where(Chore.id == chore_id) - .options(selectinload(Chore.creator), selectinload(Chore.group)) + .options(selectinload(Chore.creator), selectinload(Chore.group), selectinload(Chore.assignments)) ) return result.scalar_one_or_none() @@ -89,14 +143,17 @@ async def get_personal_chores( db: AsyncSession, user_id: int ) -> List[Chore]: - """Gets all personal chores for a user.""" + """Gets all personal chores for a user with optimized eager loading.""" result = await db.execute( select(Chore) .where( Chore.created_by_id == user_id, Chore.type == ChoreTypeEnum.personal ) - .options(selectinload(Chore.creator), selectinload(Chore.assignments)) + .options( + selectinload(Chore.creator), + selectinload(Chore.assignments).selectinload(ChoreAssignment.assigned_user) + ) .order_by(Chore.next_due_date, Chore.name) ) return result.scalars().all() @@ -106,7 +163,7 @@ async def get_chores_by_group_id( group_id: int, user_id: int ) -> List[Chore]: - """Gets all chores for a specific group, if the user is a member.""" + """Gets all chores for a specific group with optimized eager loading, if the user is a member.""" if not await is_user_member(db, group_id, user_id): raise PermissionDeniedError(detail=f"User {user_id} not a member of group {group_id}") @@ -116,7 +173,10 @@ async def get_chores_by_group_id( Chore.group_id == group_id, Chore.type == ChoreTypeEnum.group ) - .options(selectinload(Chore.creator), selectinload(Chore.assignments)) + .options( + selectinload(Chore.creator), + selectinload(Chore.assignments).selectinload(ChoreAssignment.assigned_user) + ) .order_by(Chore.next_due_date, Chore.name) ) return result.scalars().all() @@ -128,75 +188,78 @@ async def update_chore( user_id: int, group_id: Optional[int] = None ) -> Optional[Chore]: - """Updates a chore's details.""" - db_chore = await get_chore_by_id(db, chore_id) - if not db_chore: - raise ChoreNotFoundError(chore_id, group_id) - - # Check permissions - if db_chore.type == ChoreTypeEnum.group: - if not group_id: - raise ValueError("group_id is required for group chores") - if not await is_user_member(db, group_id, user_id): - raise PermissionDeniedError(detail=f"User {user_id} not a member of group {group_id}") - if db_chore.group_id != group_id: + """Updates a chore's details using proper transaction management.""" + async with db.begin_nested() if db.in_transaction() else db.begin(): + db_chore = await get_chore_by_id(db, chore_id) + if not db_chore: raise ChoreNotFoundError(chore_id, group_id) - else: # personal chore - if group_id: - raise ValueError("group_id must be None for personal chores") - if db_chore.created_by_id != user_id: - raise PermissionDeniedError(detail="Only the creator can update personal chores") - update_data = chore_in.model_dump(exclude_unset=True) + # Check permissions + if db_chore.type == ChoreTypeEnum.group: + if not group_id: + raise ValueError("group_id is required for group chores") + if not await is_user_member(db, group_id, user_id): + raise PermissionDeniedError(detail=f"User {user_id} not a member of group {group_id}") + if db_chore.group_id != group_id: + raise ChoreNotFoundError(chore_id, group_id) + else: # personal chore + if group_id: + raise ValueError("group_id must be None for personal chores") + if db_chore.created_by_id != user_id: + raise PermissionDeniedError(detail="Only the creator can update personal chores") - # Handle type change - if 'type' in update_data: - new_type = update_data['type'] - if new_type == ChoreTypeEnum.group and not group_id: - raise ValueError("group_id is required for group chores") - if new_type == ChoreTypeEnum.personal and group_id: - raise ValueError("group_id must be None for personal chores") + update_data = chore_in.model_dump(exclude_unset=True) - # Recalculate next_due_date if needed - recalculate = False - if 'frequency' in update_data and update_data['frequency'] != db_chore.frequency: - recalculate = True - if 'custom_interval_days' in update_data and update_data['custom_interval_days'] != db_chore.custom_interval_days: - recalculate = True + # Handle type change + if 'type' in update_data: + new_type = update_data['type'] + if new_type == ChoreTypeEnum.group and not group_id: + raise ValueError("group_id is required for group chores") + if new_type == ChoreTypeEnum.personal and group_id: + raise ValueError("group_id must be None for personal chores") - current_next_due_date_for_calc = db_chore.next_due_date - if 'next_due_date' in update_data: - current_next_due_date_for_calc = update_data['next_due_date'] - if not ('frequency' in update_data or 'custom_interval_days' in update_data): - recalculate = False + # Recalculate next_due_date if needed + recalculate = False + if 'frequency' in update_data and update_data['frequency'] != db_chore.frequency: + recalculate = True + if 'custom_interval_days' in update_data and update_data['custom_interval_days'] != db_chore.custom_interval_days: + recalculate = True - for field, value in update_data.items(): - setattr(db_chore, field, value) - - if recalculate: - db_chore.next_due_date = calculate_next_due_date( - current_due_date=current_next_due_date_for_calc, - frequency=db_chore.frequency, - custom_interval_days=db_chore.custom_interval_days, - last_completed_date=db_chore.last_completed_at - ) - - if db_chore.frequency == ChoreFrequencyEnum.custom and db_chore.custom_interval_days is None: - raise ValueError("custom_interval_days must be set for custom frequency chores.") + current_next_due_date_for_calc = db_chore.next_due_date + if 'next_due_date' in update_data: + current_next_due_date_for_calc = update_data['next_due_date'] + if not ('frequency' in update_data or 'custom_interval_days' in update_data): + recalculate = False - try: - await db.commit() - await db.refresh(db_chore) - result = await db.execute( - select(Chore) - .where(Chore.id == db_chore.id) - .options(selectinload(Chore.creator), selectinload(Chore.group)) - ) - return result.scalar_one() - except Exception as e: - await db.rollback() - logger.error(f"Error updating chore {chore_id}: {e}", exc_info=True) - raise DatabaseIntegrityError(f"Could not update chore {chore_id}. Error: {str(e)}") + for field, value in update_data.items(): + setattr(db_chore, field, value) + + if recalculate: + db_chore.next_due_date = calculate_next_due_date( + current_due_date=current_next_due_date_for_calc, + frequency=db_chore.frequency, + custom_interval_days=db_chore.custom_interval_days, + last_completed_date=db_chore.last_completed_at + ) + + if db_chore.frequency == ChoreFrequencyEnum.custom and db_chore.custom_interval_days is None: + raise ValueError("custom_interval_days must be set for custom frequency chores.") + + try: + await db.flush() # Flush changes within the transaction + result = await db.execute( + select(Chore) + .where(Chore.id == db_chore.id) + .options( + selectinload(Chore.creator), + selectinload(Chore.group), + selectinload(Chore.assignments).selectinload(ChoreAssignment.assigned_user) + ) + ) + return result.scalar_one() + except Exception as e: + logger.error(f"Error updating chore {chore_id}: {e}", exc_info=True) + raise DatabaseIntegrityError(f"Could not update chore {chore_id}. Error: {str(e)}") async def delete_chore( db: AsyncSession, @@ -204,30 +267,239 @@ async def delete_chore( user_id: int, group_id: Optional[int] = None ) -> bool: - """Deletes a chore and its assignments, ensuring user has permission.""" - db_chore = await get_chore_by_id(db, chore_id) - if not db_chore: - raise ChoreNotFoundError(chore_id, group_id) - - # Check permissions - if db_chore.type == ChoreTypeEnum.group: - if not group_id: - raise ValueError("group_id is required for group chores") - if not await is_user_member(db, group_id, user_id): - raise PermissionDeniedError(detail=f"User {user_id} not a member of group {group_id}") - if db_chore.group_id != group_id: + """Deletes a chore and its assignments using proper transaction management, ensuring user has permission.""" + async with db.begin_nested() if db.in_transaction() else db.begin(): + db_chore = await get_chore_by_id(db, chore_id) + if not db_chore: raise ChoreNotFoundError(chore_id, group_id) - else: # personal chore - if group_id: - raise ValueError("group_id must be None for personal chores") - if db_chore.created_by_id != user_id: - raise PermissionDeniedError(detail="Only the creator can delete personal chores") - await db.delete(db_chore) - try: - await db.commit() - return True - except Exception as e: - await db.rollback() - logger.error(f"Error deleting chore {chore_id}: {e}", exc_info=True) - raise DatabaseIntegrityError(f"Could not delete chore {chore_id}. Error: {str(e)}") + # Check permissions + if db_chore.type == ChoreTypeEnum.group: + if not group_id: + raise ValueError("group_id is required for group chores") + if not await is_user_member(db, group_id, user_id): + raise PermissionDeniedError(detail=f"User {user_id} not a member of group {group_id}") + if db_chore.group_id != group_id: + raise ChoreNotFoundError(chore_id, group_id) + else: # personal chore + if group_id: + raise ValueError("group_id must be None for personal chores") + if db_chore.created_by_id != user_id: + raise PermissionDeniedError(detail="Only the creator can delete personal chores") + + try: + await db.delete(db_chore) + await db.flush() # Ensure deletion is processed within the transaction + return True + except Exception as e: + logger.error(f"Error deleting chore {chore_id}: {e}", exc_info=True) + raise DatabaseIntegrityError(f"Could not delete chore {chore_id}. Error: {str(e)}") + +# === CHORE ASSIGNMENT CRUD FUNCTIONS === + +async def create_chore_assignment( + db: AsyncSession, + assignment_in: ChoreAssignmentCreate, + user_id: int +) -> ChoreAssignment: + """Creates a new chore assignment. User must be able to manage the chore.""" + async with db.begin_nested() if db.in_transaction() else db.begin(): + # Get the chore and validate permissions + chore = await get_chore_by_id(db, assignment_in.chore_id) + if not chore: + raise ChoreNotFoundError(chore_id=assignment_in.chore_id) + + # Check permissions to assign this chore + if chore.type == ChoreTypeEnum.personal: + if chore.created_by_id != user_id: + raise PermissionDeniedError(detail="Only the creator can assign personal chores") + else: # group chore + if not await is_user_member(db, chore.group_id, user_id): + raise PermissionDeniedError(detail=f"User {user_id} not a member of group {chore.group_id}") + # For group chores, check if assignee is also a group member + if not await is_user_member(db, chore.group_id, assignment_in.assigned_to_user_id): + raise PermissionDeniedError(detail=f"Cannot assign chore to user {assignment_in.assigned_to_user_id} who is not a group member") + + db_assignment = ChoreAssignment(**assignment_in.model_dump(exclude_unset=True)) + db.add(db_assignment) + await db.flush() # Get the ID for the assignment + + try: + # Load relationships for the response + result = await db.execute( + select(ChoreAssignment) + .where(ChoreAssignment.id == db_assignment.id) + .options( + selectinload(ChoreAssignment.chore).selectinload(Chore.creator), + selectinload(ChoreAssignment.assigned_user) + ) + ) + return result.scalar_one() + except Exception as e: + logger.error(f"Error creating chore assignment: {e}", exc_info=True) + raise DatabaseIntegrityError(f"Could not create chore assignment. Error: {str(e)}") + +async def get_chore_assignment_by_id(db: AsyncSession, assignment_id: int) -> Optional[ChoreAssignment]: + """Gets a chore assignment by ID.""" + result = await db.execute( + select(ChoreAssignment) + .where(ChoreAssignment.id == assignment_id) + .options( + selectinload(ChoreAssignment.chore).selectinload(Chore.creator), + selectinload(ChoreAssignment.assigned_user) + ) + ) + return result.scalar_one_or_none() + +async def get_user_assignments( + db: AsyncSession, + user_id: int, + include_completed: bool = False +) -> List[ChoreAssignment]: + """Gets all chore assignments for a user.""" + query = select(ChoreAssignment).where(ChoreAssignment.assigned_to_user_id == user_id) + + if not include_completed: + query = query.where(ChoreAssignment.is_complete == False) + + query = query.options( + selectinload(ChoreAssignment.chore).selectinload(Chore.creator), + selectinload(ChoreAssignment.assigned_user) + ).order_by(ChoreAssignment.due_date, ChoreAssignment.id) + + result = await db.execute(query) + return result.scalars().all() + +async def get_chore_assignments( + db: AsyncSession, + chore_id: int, + user_id: int +) -> List[ChoreAssignment]: + """Gets all assignments for a specific chore. User must have permission to view the chore.""" + chore = await get_chore_by_id(db, chore_id) + if not chore: + raise ChoreNotFoundError(chore_id=chore_id) + + # Check permissions + if chore.type == ChoreTypeEnum.personal: + if chore.created_by_id != user_id: + raise PermissionDeniedError(detail="Can only view assignments for own personal chores") + else: # group chore + if not await is_user_member(db, chore.group_id, user_id): + raise PermissionDeniedError(detail=f"User {user_id} not a member of group {chore.group_id}") + + result = await db.execute( + select(ChoreAssignment) + .where(ChoreAssignment.chore_id == chore_id) + .options( + selectinload(ChoreAssignment.chore).selectinload(Chore.creator), + selectinload(ChoreAssignment.assigned_user) + ) + .order_by(ChoreAssignment.due_date, ChoreAssignment.id) + ) + return result.scalars().all() + +async def update_chore_assignment( + db: AsyncSession, + assignment_id: int, + assignment_in: ChoreAssignmentUpdate, + user_id: int +) -> Optional[ChoreAssignment]: + """Updates a chore assignment. Only the assignee can mark it complete.""" + async with db.begin_nested() if db.in_transaction() else db.begin(): + db_assignment = await get_chore_assignment_by_id(db, assignment_id) + if not db_assignment: + raise ChoreNotFoundError(assignment_id=assignment_id) + + # Load the chore for permission checking + chore = await get_chore_by_id(db, db_assignment.chore_id) + if not chore: + raise ChoreNotFoundError(chore_id=db_assignment.chore_id) + + # Check permissions - only assignee can complete, but chore managers can reschedule + can_manage = False + if chore.type == ChoreTypeEnum.personal: + can_manage = chore.created_by_id == user_id + else: # group chore + can_manage = await is_user_member(db, chore.group_id, user_id) + + can_complete = db_assignment.assigned_to_user_id == user_id + + update_data = assignment_in.model_dump(exclude_unset=True) + + # Check specific permissions for different updates + if 'is_complete' in update_data and not can_complete: + raise PermissionDeniedError(detail="Only the assignee can mark assignments as complete") + + if 'due_date' in update_data and not can_manage: + raise PermissionDeniedError(detail="Only chore managers can reschedule assignments") + + # Handle completion logic + if 'is_complete' in update_data and update_data['is_complete']: + if not db_assignment.is_complete: # Only if not already complete + update_data['completed_at'] = datetime.utcnow() + + # Update parent chore's last_completed_at and recalculate next_due_date + chore.last_completed_at = update_data['completed_at'] + chore.next_due_date = calculate_next_due_date( + current_due_date=chore.next_due_date, + frequency=chore.frequency, + custom_interval_days=chore.custom_interval_days, + last_completed_date=chore.last_completed_at + ) + elif 'is_complete' in update_data and not update_data['is_complete']: + # If marking as incomplete, clear completed_at + update_data['completed_at'] = None + + # Apply updates + for field, value in update_data.items(): + setattr(db_assignment, field, value) + + try: + await db.flush() # Flush changes within the transaction + + # Load relationships for the response + result = await db.execute( + select(ChoreAssignment) + .where(ChoreAssignment.id == db_assignment.id) + .options( + selectinload(ChoreAssignment.chore).selectinload(Chore.creator), + selectinload(ChoreAssignment.assigned_user) + ) + ) + return result.scalar_one() + except Exception as e: + logger.error(f"Error updating chore assignment {assignment_id}: {e}", exc_info=True) + raise DatabaseIntegrityError(f"Could not update chore assignment {assignment_id}. Error: {str(e)}") + +async def delete_chore_assignment( + db: AsyncSession, + assignment_id: int, + user_id: int +) -> bool: + """Deletes a chore assignment. User must have permission to manage the chore.""" + async with db.begin_nested() if db.in_transaction() else db.begin(): + db_assignment = await get_chore_assignment_by_id(db, assignment_id) + if not db_assignment: + raise ChoreNotFoundError(assignment_id=assignment_id) + + # Load the chore for permission checking + chore = await get_chore_by_id(db, db_assignment.chore_id) + if not chore: + raise ChoreNotFoundError(chore_id=db_assignment.chore_id) + + # Check permissions + if chore.type == ChoreTypeEnum.personal: + if chore.created_by_id != user_id: + raise PermissionDeniedError(detail="Only the creator can delete personal chore assignments") + else: # group chore + if not await is_user_member(db, chore.group_id, user_id): + raise PermissionDeniedError(detail=f"User {user_id} not a member of group {chore.group_id}") + + try: + await db.delete(db_assignment) + await db.flush() # Ensure deletion is processed within the transaction + return True + except Exception as e: + logger.error(f"Error deleting chore assignment {assignment_id}: {e}", exc_info=True) + raise DatabaseIntegrityError(f"Could not delete chore assignment {assignment_id}. Error: {str(e)}") diff --git a/be/app/crud/group.py b/be/app/crud/group.py index aea3773..535e1f5 100644 --- a/be/app/crud/group.py +++ b/be/app/crud/group.py @@ -70,14 +70,16 @@ async def create_group(db: AsyncSession, group_in: GroupCreate, creator_id: int) raise DatabaseTransactionError(f"Database transaction error during group creation: {str(e)}") 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 with optimized eager loading.""" try: result = await db.execute( select(GroupModel) .join(UserGroupModel) .where(UserGroupModel.user_id == user_id) .options( - selectinload(GroupModel.member_associations).selectinload(UserGroupModel.user) + selectinload(GroupModel.member_associations).options( + selectinload(UserGroupModel.user) + ) ) ) return result.scalars().all() diff --git a/be/app/database.py b/be/app/database.py index eab4cbf..9fc9105 100644 --- a/be/app/database.py +++ b/be/app/database.py @@ -11,7 +11,7 @@ if not settings.DATABASE_URL: # pool_recycle=3600 helps prevent stale connections on some DBs engine = create_async_engine( settings.DATABASE_URL, - echo=True, # Log SQL queries (useful for debugging) + echo=False, # Disable SQL query logging for production (use DEBUG log level to enable) future=True, # Use SQLAlchemy 2.0 style features pool_recycle=3600, # Optional: recycle connections after 1 hour pool_pre_ping=True # Add this line to ensure connections are live @@ -33,30 +33,25 @@ Base = declarative_base() # Dependency to get DB session in path operations async def get_session() -> AsyncSession: # type: ignore """ - Dependency function that yields an AsyncSession. + Dependency function that yields an AsyncSession for read-only operations. Ensures the session is closed after the request. """ async with AsyncSessionLocal() as session: yield session # The 'async with' block handles session.close() automatically. - # Commit/rollback should be handled by the functions using the session. async def get_transactional_session() -> AsyncSession: # type: ignore """ Dependency function that yields an AsyncSession and manages a transaction. Commits the transaction if the request handler succeeds, otherwise rollbacks. Ensures the session is closed after the request. + + This follows the FastAPI-DB strategy for endpoint-level transaction management. """ async with AsyncSessionLocal() as session: - try: - await session.begin() + async with session.begin(): yield session - await session.commit() - except Exception: - await session.rollback() - raise - finally: - await session.close() + # Transaction is automatically committed on success or rolled back on exception # Alias for backward compatibility get_db = get_session \ No newline at end of file diff --git a/be/tests/api/v1/test_costs.py b/be/tests/api/v1/test_costs.py index f47ea4b..22b85c0 100644 --- a/be/tests/api/v1/test_costs.py +++ b/be/tests/api/v1/test_costs.py @@ -548,10 +548,7 @@ class TestEqualSplitExpenses: async def test_equal_split_with_remainder_distribution( self, client: httpx.AsyncClient, - db_session: AsyncSession, # Added db_session - test_user1_api_costs: Dict[str, Any], - client: httpx.AsyncClient, - db_session: AsyncSession, + db_session: AsyncSession, test_user1_api_costs: Dict[str, Any], # For headers test_group_api_costs_4_users: Dict[str, Any], ): diff --git a/fe/src/components/CreateExpenseForm.vue b/fe/src/components/CreateExpenseForm.vue index 8bd66a0..c7c1cf0 100644 --- a/fe/src/components/CreateExpenseForm.vue +++ b/fe/src/components/CreateExpenseForm.vue @@ -13,41 +13,33 @@