Merge pull request 'refactor: Update ListDetailPage and listDetailStore for improved type safety and state management' (#67) from ph5 into prod

Reviewed-on: #67

fe/src/pages/ListDetailPage.vue

@@ -443,7 +443,6 @@ import VToggleSwitch from '@/components/valerie/VToggleSwitch.vue';
 import draggable from 'vuedraggable';
 import { useCategoryStore } from '@/stores/categoryStore';
 import { storeToRefs } from 'pinia';
-import ExpenseCard from '@/components/ExpenseCard.vue';
 
 const { t } = useI18n();
 
@@ -555,9 +554,10 @@ const newItem = ref<{ name: string; quantity?: number | string; category_id?: nu
 const itemNameInputRef = ref<InstanceType<typeof VInput> | null>(null);
 
 const categoryOptions = computed(() => {
+  type CategoryOption = { id: number; name: string };
   return [
     { label: 'No Category', value: null },
-    ...categories.value.map(c => ({ label: c.name, value: c.id })),
+    ...categories.value.map((c: CategoryOption) => ({ label: c.name, value: c.id })),
   ];
 });
 
@@ -610,7 +610,8 @@ const showCreateExpenseForm = ref(false);
 // Define a more specific type for the offline item payload
 interface OfflineCreateItemPayload {
   name: string;
-  quantity?: string | number; // Align with the target type from the linter error
+  quantity?: string | number;
+  category_id?: number | null;
 }
 
 const formatCurrency = (value: string | number | undefined | null): string => {
@@ -1064,7 +1065,7 @@ watch(showCostSummaryDialog, (newVal) => {
 
 // --- Expense and Settlement Status Logic ---
 const listDetailStore = useListDetailStore();
-const expenses = computed(() => listDetailStore.getExpenses);
+const { getExpenses: expenses, currentList: listFromStore } = storeToRefs(listDetailStore);
 const allFetchedGroups = ref<Group[]>([]);
 
 const getGroupName = (groupId: number): string => {
@@ -1309,6 +1310,10 @@ const handleExpenseCreated = (expense: any) => {
   if (list.value?.id) {
     listDetailStore.fetchListWithExpenses(String(list.value.id));
   }
+  // Also, update the main expenses from the store
+  if (listDetailStore.currentList?.id) {
+    listDetailStore.fetchListWithExpenses(String(listDetailStore.currentList.id));
+  }
 };
 
 const handleCheckboxChange = (item: ItemWithUI, event: Event) => {
@@ -1366,10 +1371,11 @@ const isExpenseExpanded = (expenseId: number) => {
 const groupedItems = computed(() => {
   if (!list.value?.items) return [];
   const groups: Record<string, { categoryName: string; items: ItemWithUI[] }> = {};
+  type Category = { id: number; name: string };
 
   list.value.items.forEach(item => {
     const categoryId = item.category_id;
-    const category = categories.value.find(c => c.id === categoryId);
+    const category = categories.value.find((c: Category) => c.id === categoryId);
     const categoryName = category ? category.name : t('listDetailPage.items.noCategory');
 
     if (!groups[categoryName]) {

fe/src/stores/listDetailStore.ts

@@ -1,4 +1,5 @@
 import { defineStore } from 'pinia'
+import { ref, computed } from 'vue'
 import { apiClient, API_ENDPOINTS } from '@/services/api'
 import type {
   Expense,
@@ -15,30 +16,52 @@ export interface ListWithExpenses extends List {
   expenses: Expense[]
 }
 
-interface ListDetailState {
+export const useListDetailStore = defineStore('listDetail', () => {
-  currentList: ListWithExpenses | null
+  // State
-  isLoading: boolean
+  const currentList = ref<ListWithExpenses | null>(null)
-  error: string | null
+  const isLoading = ref(false)
-  isSettlingSplit: boolean
+  const error = ref<string | null>(null)
+  const isSettlingSplit = ref(false)
+
+  // Getters (as computed properties or methods)
+  const getExpenses = computed((): Expense[] => {
+    return currentList.value?.expenses || []
+  })
+
+  const getPaidAmountForSplit = (splitId: number): number => {
+    let totalPaid = 0
+    if (currentList.value && currentList.value.expenses) {
+      for (const expense of currentList.value.expenses) {
+        const split = expense.splits.find((s) => s.id === splitId)
+        if (split && split.settlement_activities) {
+          totalPaid = split.settlement_activities.reduce((sum, activity) => {
+            return sum + parseFloat(activity.amount_paid)
+          }, 0)
+          break
+        }
+      }
+    }
+    return totalPaid
   }
 
-export const useListDetailStore = defineStore('listDetail', {
+  const getExpenseSplitById = (splitId: number): ExpenseSplit | undefined => {
-  state: (): ListDetailState => ({
+    if (!currentList.value || !currentList.value.expenses) return undefined
-    currentList: null,
+    for (const expense of currentList.value.expenses) {
-    isLoading: false,
+      const split = expense.splits.find((s) => s.id === splitId)
-    error: null,
+      if (split) return split
-    isSettlingSplit: false,
+    }
-  }),
+    return undefined
+  }
 
-  actions: {
+  // Actions (as functions)
-    async fetchListWithExpenses(listId: string) {
+  async function fetchListWithExpenses(listId: string) {
     if (!listId || listId === 'undefined' || listId === 'null') {
-        this.error = 'Invalid list ID provided.';
+      error.value = 'Invalid list ID provided.'
-        console.warn(`fetchListWithExpenses called with invalid ID: ${listId}`);
+      console.warn(`fetchListWithExpenses called with invalid ID: ${listId}`)
-        return;
+      return
     }
-      this.isLoading = true
+    isLoading.value = true
-      this.error = null
+    error.value = null
     try {
       // Get list details
       const listEndpoint = API_ENDPOINTS.LISTS.BY_ID(listId)
@@ -51,96 +74,69 @@ export const useListDetailStore = defineStore('listDetail', {
       const expensesData = expensesResponse.data as Expense[]
 
       // Combine into ListWithExpenses
-        this.currentList = {
+      currentList.value = {
         ...listData,
         expenses: expensesData,
       } as ListWithExpenses
     } catch (err: any) {
-        this.error = err.response?.data?.detail || err.message || 'Failed to fetch list details'
+      error.value = err.response?.data?.detail || err.message || 'Failed to fetch list details'
-        this.currentList = null
+      currentList.value = null
       console.error('Error fetching list details:', err)
     } finally {
-        this.isLoading = false
+      isLoading.value = false
+    }
   }
-    },
 
-    async settleExpenseSplit(payload: {
+  async function settleExpenseSplit(payload: {
-      list_id_for_refetch: string // ID of the list to refetch after settlement
+    list_id_for_refetch: string
     expense_split_id: number
     activity_data: SettlementActivityCreate
   }): Promise<boolean> {
-      this.isSettlingSplit = true
+    isSettlingSplit.value = true
-      this.error = null
+    error.value = null
     try {
-        // Call the actual API endpoint using generic post method
       const endpoint = `/financials/expense_splits/${payload.expense_split_id}/settle`
-        const response = await apiClient.post(endpoint, payload.activity_data)
+      await apiClient.post(endpoint, payload.activity_data)
 
-        // Refresh list data to show updated statuses
       if (payload.list_id_for_refetch) {
-          await this.fetchListWithExpenses(payload.list_id_for_refetch)
+        await fetchListWithExpenses(payload.list_id_for_refetch)
-        } else if (this.currentList?.id) {
+      } else if (currentList.value?.id) {
-          // Fallback if list_id_for_refetch is not provided but currentList exists
+        await fetchListWithExpenses(String(currentList.value.id))
-          await this.fetchListWithExpenses(String(this.currentList.id))
       } else {
         console.warn(
           'Could not refetch list details: list_id_for_refetch not provided and no currentList available.',
         )
       }
 
-        this.isSettlingSplit = false
+      isSettlingSplit.value = false
-        return true // Indicate success
+      return true
     } catch (err: any) {
       const errorMessage =
         err.response?.data?.detail || err.message || 'Failed to settle expense split.'
-        this.error = errorMessage
+      error.value = errorMessage
       console.error('Error settling expense split:', err)
-        this.isSettlingSplit = false
+      isSettlingSplit.value = false
-        return false // Indicate failure
+      return false
+    }
   }
-    },
 
-    setError(errorMessage: string) {
+  function setError(errorMessage: string) {
-      this.error = errorMessage
+    error.value = errorMessage
-      this.isLoading = false
+    isLoading.value = false
-    },
+  }
-  },
 
-  getters: {
+  return {
-    getList(state: ListDetailState): ListWithExpenses | null {
+    currentList,
-      return state.currentList
+    isLoading,
-    },
+    error,
-    getExpenses(state: ListDetailState): Expense[] {
+    isSettlingSplit,
-      return state.currentList?.expenses || []
+    fetchListWithExpenses,
-    },
+    settleExpenseSplit,
-    getPaidAmountForSplit:
+    setError,
-      (state: ListDetailState) =>
+    getExpenses,
-        (splitId: number): number => {
+    getPaidAmountForSplit,
-          let totalPaid = 0
+    getExpenseSplitById,
-          if (state.currentList && state.currentList.expenses) {
-            for (const expense of state.currentList.expenses) {
-              const split = expense.splits.find((s) => s.id === splitId)
-              if (split && split.settlement_activities) {
-                totalPaid = split.settlement_activities.reduce((sum, activity) => {
-                  return sum + parseFloat(activity.amount_paid)
-                }, 0)
-                break
   }
-            }
-          }
-          return totalPaid
-        },
-    getExpenseSplitById:
-      (state: ListDetailState) =>
-        (splitId: number): ExpenseSplit | undefined => {
-          if (!state.currentList || !state.currentList.expenses) return undefined
-          for (const expense of state.currentList.expenses) {
-            const split = expense.splits.find((s) => s.id === splitId)
-            if (split) return split
-          }
-          return undefined
-        },
-  },
 })
 
 // Assuming List interface might be defined in fe/src/types/list.ts

fe/src/types/expense.ts

@@ -101,7 +101,7 @@ export interface Expense {
   description: string
   total_amount: string // String representation of Decimal
   currency: string
-  expense_date: string
+  expense_date?: string
   split_type: string
   list_id?: number | null
   group_id?: number | null
@@ -126,29 +126,3 @@ export interface Expense {
 
 export type SplitType = 'EQUAL' | 'EXACT_AMOUNTS' | 'PERCENTAGE' | 'SHARES' | 'ITEM_BASED';
 export type SettlementStatus = 'unpaid' | 'paid' | 'partially_paid';
-
-export interface Expense {
-  id: number;
-  description: string;
-  total_amount: string; // Decimal is string
-  currency: string;
-  expense_date?: string;
-  split_type: SplitType;
-  list_id?: number;
-  group_id?: number;
-  item_id?: number;
-  paid_by_user_id: number;
-  is_recurring: boolean;
-  recurrence_pattern?: any;
-  created_at: string;
-  updated_at: string;
-  version: number;
-  created_by_user_id: number;
-  splits: ExpenseSplit[];
-  paid_by_user?: UserPublic;
-  overall_settlement_status: SettlementStatus;
-  next_occurrence?: string;
-  last_occurrence?: string;
-  parent_expense_id?: number;
-  generated_expenses: Expense[];
-}