From 8241a2ca694fd9fdf2f10cd47376f24e5be86d5a Mon Sep 17 00:00:00 2001 From: KrishP147 Date: Wed, 23 Sep 2026 23:09:10 -0400 Subject: [PATCH] MealList: surface duplicate/edit/replace errors, fix compound-duplicate fields - add actionError state + alert banner, cleared per action - handleDuplicate: guard user, surface insert errors, fix component copy to use base_* fields (was calories/protein_g etc, silently dropping data), roll back orphan meal if component insert fails - saveEdit: stop on first component-update error, surface meal-update errors on both compound and simple paths, keep editor open on failure - handleReplaceSimpleMeal: surface update errors, keep modal open on failure - fetchMeals: bail out early if no user Co-Authored-By: Claude Opus 5.5 (1M context) --- frontend/src/components/MealList.jsx | 71 ++++++- .../components/__tests__/MealList.test.jsx | 176 +++++++++++++++++- 2 files changed, 234 insertions(+), 13 deletions(-) diff --git a/frontend/src/components/MealList.jsx b/frontend/src/components/MealList.jsx index f1f293d..fd700c9 100644 --- a/frontend/src/components/MealList.jsx +++ b/frontend/src/components/MealList.jsx @@ -27,6 +27,7 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, }); const [editMealName, setEditMealName] = useState(''); const [originalMealName, setOriginalMealName] = useState(''); + const [actionError, setActionError] = useState(null); // Dark theme color scheme const colors = { @@ -52,6 +53,11 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, setLoading(true); const { data: { user } } = await supabase.auth.getUser(); + if (!user) { + setLoading(false); + return; + } + let query = supabase .from('meals') .select('*') @@ -129,8 +135,14 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, }; const handleDuplicate = async (meal) => { + setActionError(null); const { data: { user } } = await supabase.auth.getUser(); + if (!user) { + setActionError('You must be signed in to duplicate a meal.'); + return; + } + // Create duplicate meal with current time const duplicatedMeal = { user_id: user.id, @@ -157,7 +169,7 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, if (error) { console.error('Error duplicating meal:', error); - alert('Failed to duplicate meal'); + setActionError('Failed to duplicate meal: ' + error.message); return; } @@ -170,17 +182,24 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, component_name: c.component_name, portion_size: c.portion_size, portion_unit: c.portion_unit, - calories: c.calories, - protein_g: c.protein_g, - carbs_g: c.carbs_g, - fat_g: c.fat_g, - fiber_g: c.fiber_g, - custom_food_id: c.custom_food_id || null + base_calories: c.base_calories, + base_protein_g: c.base_protein_g, + base_carbs_g: c.base_carbs_g, + base_fat_g: c.base_fat_g, + base_fiber_g: c.base_fiber_g ?? 0, + custom_food_id: c.custom_food_id ?? null })); - await supabase + const { error: componentError } = await supabase .from('meal_components') .insert(duplicatedComponents); + + if (componentError) { + console.error('Error duplicating meal components:', componentError); + await supabase.from('meals').delete().eq('id', newMeal.id); + setActionError('Failed to duplicate meal components: ' + componentError.message); + return; + } } } @@ -545,6 +564,7 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, }; const handleReplaceSimpleMeal = async (newFoodData) => { + setActionError(null); // Update the meal in the meals list const meal = meals.find(m => m.id === replacingSimpleMeal); if (!meal) return; @@ -564,7 +584,7 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, }; // Persist to database - await supabase + const { error } = await supabase .from('meals') .update({ meal_name: updatedMeal.meal_name, @@ -578,11 +598,18 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, }) .eq('id', meal.id); + if (error) { + console.error('Error replacing meal:', error); + setActionError('Failed to replace food: ' + error.message); + return; + } + setMeals(meals.map(m => m.id === meal.id ? updatedMeal : m)); setReplacingSimpleMeal(null); }; const saveEdit = async (meal) => { + setActionError(null); // Parse editQuantity with smart input parsing let portionSize; const containsLetters = /[a-zA-Z]/.test(editQuantity); @@ -600,7 +627,7 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, if (meal.is_compound && editComponents.length > 0) { // Update each component (both portion and nutrition if edited) for (const component of editComponents) { - await supabase + const { error: componentError } = await supabase .from('meal_components') .update({ portion_size: component.portion_size, @@ -611,6 +638,12 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, base_fiber_g: component.base_fiber_g }) .eq('id', component.id); + + if (componentError) { + console.error('Error updating component:', componentError); + setActionError('Failed to save component: ' + componentError.message); + return; + } } // Recalculate total macros from components @@ -668,6 +701,9 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, setEditingMealId(null); setEditComponents([]); if (onMealUpdated) onMealUpdated(); + } else { + console.error('Error saving meal:', error); + setActionError('Failed to save meal: ' + error.message); } } else { // Simple food - use manually edited macros if available, otherwise calculate from base @@ -712,6 +748,9 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, setEditQuantity('1'); setEditingSimpleMacros(false); if (onMealUpdated) onMealUpdated(); + } else { + console.error('Error saving meal:', error); + setActionError('Failed to save meal: ' + error.message); } } }; @@ -742,6 +781,18 @@ export default function MealList({ refreshTrigger, onMealDeleted, onMealUpdated, return (
+ {actionError && ( +
+ {actionError} + +
+ )} {meals.map((meal) => (
{editingMealId === meal.id ? ( diff --git a/frontend/src/components/__tests__/MealList.test.jsx b/frontend/src/components/__tests__/MealList.test.jsx index c11561d..643e347 100644 --- a/frontend/src/components/__tests__/MealList.test.jsx +++ b/frontend/src/components/__tests__/MealList.test.jsx @@ -1,13 +1,15 @@ /** * Tests for MealList component - edit mode input contrast (issue #3) + * plus error-handling coverage for duplicate/edit/replace (issue #7) */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import MealList from '../MealList'; +import { supabase } from '../../supabaseClient'; // All vi.mock calls MUST use inline factory functions (no external const references), // so shared fixtures go through vi.hoisted. -const { mockMeal, createBuilder } = vi.hoisted(() => { +const { mockMeal, mockCompoundMeal, mockComponent, createBuilder } = vi.hoisted(() => { const mockMeal = { id: 'meal-1', user_id: 'test-user-id', @@ -25,6 +27,26 @@ const { mockMeal, createBuilder } = vi.hoisted(() => { notes: '', }; + const mockCompoundMeal = { + ...mockMeal, + id: 'meal-1', + is_compound: true, + }; + + const mockComponent = { + id: 'component-1', + meal_id: 'meal-1', + component_name: 'Rice', + portion_size: 100, + portion_unit: 'g', + base_calories: 130, + base_protein_g: 2.7, + base_carbs_g: 28, + base_fat_g: 0.3, + base_fiber_g: 0.4, + custom_food_id: null, + }; + const createBuilder = (result) => { const builder = {}; ['select', 'eq', 'gte', 'lte', 'order', 'limit', 'delete', 'insert', 'update'].forEach((fn) => { @@ -35,7 +57,7 @@ const { mockMeal, createBuilder } = vi.hoisted(() => { return builder; }; - return { mockMeal, createBuilder }; + return { mockMeal, mockCompoundMeal, mockComponent, createBuilder }; }); vi.mock('../../supabaseClient', () => ({ @@ -82,3 +104,151 @@ describe('MealList - edit mode input contrast', () => { }); }); }); + +// Builder for the 'meals' table that returns a full row from .single() (insert) +// and the compound meal list from .then() (select), like the real query chains. +function makeMealsBuilder(singleResult) { + const b = {}; + ['select', 'eq', 'gte', 'lte', 'order', 'limit', 'insert', 'update'].forEach((fn) => { + b[fn] = () => b; + }); + b.single = () => Promise.resolve(singleResult); + b.then = (resolve, reject) => + Promise.resolve({ data: [mockCompoundMeal], error: null }).then(resolve, reject); + return b; +} + +// Builder for 'meal_components' select (.then) with an overridable insert spy. +function makeComponentsBuilder(insertImpl) { + const b = {}; + ['select', 'eq', 'update', 'delete'].forEach((fn) => { + b[fn] = () => b; + }); + b.insert = insertImpl; + b.then = (resolve, reject) => + Promise.resolve({ data: [mockComponent], error: null }).then(resolve, reject); + return b; +} + +describe('MealList - duplicate/edit/replace error handling (issue #7)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('duplicates a compound meal, carrying base_* fields for every component', async () => { + const newMealRow = { ...mockCompoundMeal, id: 'meal-2' }; + const mealsBuilder = makeMealsBuilder({ data: newMealRow, error: null }); + const insertSpy = vi.fn(() => Promise.resolve({ error: null })); + const componentsBuilder = makeComponentsBuilder(insertSpy); + + supabase.from.mockImplementation((table) => { + if (table === 'meals') return mealsBuilder; + if (table === 'meal_components') return componentsBuilder; + return { select: () => ({ eq: () => Promise.resolve({ data: [], error: null }) }) }; + }); + + render(); + + const copyButton = await screen.findByRole('button', { name: 'Copy' }); + fireEvent.click(copyButton); + + await waitFor(() => expect(insertSpy).toHaveBeenCalled()); + + const payload = insertSpy.mock.calls[0][0]; + expect(payload).toHaveLength(1); + expect(payload[0]).toMatchObject({ + meal_id: 'meal-2', + component_name: mockComponent.component_name, + portion_size: mockComponent.portion_size, + portion_unit: mockComponent.portion_unit, + base_calories: mockComponent.base_calories, + base_protein_g: mockComponent.base_protein_g, + base_carbs_g: mockComponent.base_carbs_g, + base_fat_g: mockComponent.base_fat_g, + base_fiber_g: mockComponent.base_fiber_g, + custom_food_id: null, + }); + expect(payload[0]).not.toHaveProperty('calories'); + expect(payload[0]).not.toHaveProperty('protein_g'); + expect(payload[0]).not.toHaveProperty('carbs_g'); + expect(payload[0]).not.toHaveProperty('fat_g'); + expect(payload[0]).not.toHaveProperty('fiber_g'); + }); + + it('rolls back the duplicated meal and shows an error when the component insert fails', async () => { + const newMealRow = { ...mockCompoundMeal, id: 'meal-2' }; + const mealsBuilder = makeMealsBuilder({ data: newMealRow, error: null }); + let deletedId = null; + mealsBuilder.delete = () => ({ + eq: (_col, val) => { + deletedId = val; + return Promise.resolve({ error: null }); + }, + }); + const componentsBuilder = makeComponentsBuilder( + vi.fn(() => Promise.resolve({ error: { message: 'component insert failed' } })) + ); + + supabase.from.mockImplementation((table) => { + if (table === 'meals') return mealsBuilder; + if (table === 'meal_components') return componentsBuilder; + return { select: () => ({ eq: () => Promise.resolve({ data: [], error: null }) }) }; + }); + + render(); + + const copyButton = await screen.findByRole('button', { name: 'Copy' }); + fireEvent.click(copyButton); + + const alert = await screen.findByRole('alert'); + expect(alert).toHaveTextContent('component insert failed'); + await waitFor(() => expect(deletedId).toBe('meal-2')); + }); + + it('shows an error banner when saving an edit fails and keeps the editor open', async () => { + const mealsBuilder = {}; + ['select', 'eq', 'gte', 'lte', 'order', 'limit', 'insert', 'delete'].forEach((fn) => { + mealsBuilder[fn] = () => mealsBuilder; + }); + mealsBuilder.single = () => Promise.resolve({ data: mockMeal, error: null }); + let mode = 'select'; + mealsBuilder.update = () => { + mode = 'update'; + return mealsBuilder; + }; + mealsBuilder.then = (resolve, reject) => { + const result = + mode === 'update' + ? { data: null, error: { message: 'boom' } } + : { data: [mockMeal], error: null }; + return Promise.resolve(result).then(resolve, reject); + }; + + supabase.from.mockImplementation((table) => { + if (table === 'meals') return mealsBuilder; + return { select: () => ({ eq: () => Promise.resolve({ data: [], error: null }) }) }; + }); + + render(); + + const editButton = await screen.findByRole('button', { name: 'Edit' }); + fireEvent.click(editButton); + + const saveButton = await screen.findByRole('button', { name: 'Save Changes' }); + fireEvent.click(saveButton); + + const alert = await screen.findByRole('alert'); + expect(alert).toHaveTextContent('boom'); + expect(screen.getByRole('button', { name: 'Save Changes' })).toBeInTheDocument(); + }); + + it('handles a missing user in fetchMeals without crashing', async () => { + supabase.auth.getUser.mockResolvedValueOnce({ data: { user: null } }); + + render(); + + expect( + await screen.findByText('No meals logged yet. Start by adding your first meal!') + ).toBeInTheDocument(); + }); +});