Skip to content

Commit 45273cd

Browse files
committed
Refactor BudgetVsActualPage to filter out deleted categories and improve category handling
- Updated the BudgetVsActualPage component to filter out deleted categories and groups, ensuring they are not displayed in the UI. - Enhanced the logic for grouping and sorting categories, maintaining only active categories and special cases. - Modified related tests to verify the correct filtering behavior and ensure that deleted categories do not appear in the selection options. - Improved data transformation logic to prevent the inclusion of non-existent categories in budget calculations.
1 parent b660dee commit 45273cd

File tree

3 files changed

+145
-96
lines changed

3 files changed

+145
-96
lines changed

src/components/pages/BudgetVsActualPage.test.tsx

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ describe('BudgetVsActualPage', () => {
293293
expect(incomeIndex).toBeGreaterThan(groceriesIndex);
294294
});
295295

296-
it('sorts deleted categories last within their group', () => {
296+
it('filters out deleted categories', () => {
297297
const mockData = createMockWrappedData({
298298
budgetComparison: createMockBudgetComparison({
299299
categoryBudgets: [
@@ -338,12 +338,14 @@ describe('BudgetVsActualPage', () => {
338338
const anotherActiveIndex = options.findIndex(opt => opt.text === 'Another Active');
339339
const deletedIndex = options.findIndex(opt => opt.text === 'Deleted: Old Category');
340340

341-
// Deleted category should appear last
342-
expect(deletedIndex).toBeGreaterThan(activeIndex);
343-
expect(deletedIndex).toBeGreaterThan(anotherActiveIndex);
341+
// Active categories should be present
342+
expect(activeIndex).toBeGreaterThan(-1);
343+
expect(anotherActiveIndex).toBeGreaterThan(-1);
344+
// Deleted category should be filtered out
345+
expect(deletedIndex).toBe(-1);
344346
});
345347

346-
it('sorts deleted groups last in the group list', () => {
348+
it('filters out deleted categories even in deleted groups', () => {
347349
const mockData = createMockWrappedData({
348350
budgetComparison: createMockBudgetComparison({
349351
categoryBudgets: [
@@ -357,6 +359,16 @@ describe('BudgetVsActualPage', () => {
357359
totalVariance: -200,
358360
totalVariancePercentage: -20,
359361
},
362+
{
363+
categoryId: 'cat1b',
364+
categoryName: 'Another Category in Active Group',
365+
categoryGroup: 'Active Group',
366+
monthlyBudgets: [],
367+
totalBudgeted: 800,
368+
totalActual: 700,
369+
totalVariance: -100,
370+
totalVariancePercentage: -12.5,
371+
},
360372
{
361373
categoryId: 'cat2',
362374
categoryName: 'Deleted: Category in Deleted Group',
@@ -381,22 +393,21 @@ describe('BudgetVsActualPage', () => {
381393

382394
// Find the optgroups
383395
const activeGroupOption = options.find(opt => opt.text === 'Category in Active Group');
396+
const anotherActiveOption = options.find(
397+
opt => opt.text === 'Another Category in Active Group',
398+
);
384399
const deletedGroupOption = options.find(
385400
opt => opt.text === 'Deleted: Category in Deleted Group',
386401
);
387402

388-
// Both should exist
403+
// Active categories should exist
389404
expect(activeGroupOption).toBeDefined();
390-
expect(deletedGroupOption).toBeDefined();
391-
392-
// The deleted group category should appear after the active group category
393-
// (since groups are sorted and deleted groups go last)
394-
const activeIndex = options.findIndex(opt => opt === activeGroupOption);
395-
const deletedIndex = options.findIndex(opt => opt === deletedGroupOption);
396-
expect(deletedIndex).toBeGreaterThan(activeIndex);
405+
expect(anotherActiveOption).toBeDefined();
406+
// Deleted category should be filtered out
407+
expect(deletedGroupOption).toBeUndefined();
397408
});
398409

399-
it('handles groups with all deleted categories as deleted groups', () => {
410+
it('filters out groups with all deleted categories', () => {
400411
const mockData = createMockWrappedData({
401412
budgetComparison: createMockBudgetComparison({
402413
categoryBudgets: [
@@ -412,8 +423,8 @@ describe('BudgetVsActualPage', () => {
412423
},
413424
{
414425
categoryId: 'cat2',
415-
categoryName: 'Deleted: Category 1',
416-
categoryGroup: 'All Deleted Group',
426+
categoryName: 'Another Active',
427+
categoryGroup: 'Active Group',
417428
monthlyBudgets: [],
418429
totalBudgeted: 500,
419430
totalActual: 400,
@@ -422,14 +433,24 @@ describe('BudgetVsActualPage', () => {
422433
},
423434
{
424435
categoryId: 'cat3',
425-
categoryName: 'Deleted: Category 2',
436+
categoryName: 'Deleted: Category 1',
426437
categoryGroup: 'All Deleted Group',
427438
monthlyBudgets: [],
428439
totalBudgeted: 300,
429440
totalActual: 250,
430441
totalVariance: -50,
431442
totalVariancePercentage: -16.67,
432443
},
444+
{
445+
categoryId: 'cat4',
446+
categoryName: 'Deleted: Category 2',
447+
categoryGroup: 'All Deleted Group',
448+
monthlyBudgets: [],
449+
totalBudgeted: 200,
450+
totalActual: 150,
451+
totalVariance: -50,
452+
totalVariancePercentage: -25,
453+
},
433454
],
434455
groupSortOrder: new Map([
435456
['Active Group', 1],
@@ -443,11 +464,15 @@ describe('BudgetVsActualPage', () => {
443464
const options = Array.from(select.options);
444465

445466
const activeIndex = options.findIndex(opt => opt.text === 'Active Category');
467+
const anotherActiveIndex = options.findIndex(opt => opt.text === 'Another Active');
446468
const deleted1Index = options.findIndex(opt => opt.text === 'Deleted: Category 1');
447469
const deleted2Index = options.findIndex(opt => opt.text === 'Deleted: Category 2');
448470

449-
// All deleted group should appear after active group
450-
expect(deleted1Index).toBeGreaterThan(activeIndex);
451-
expect(deleted2Index).toBeGreaterThan(activeIndex);
471+
// Active categories should be present
472+
expect(activeIndex).toBeGreaterThan(-1);
473+
expect(anotherActiveIndex).toBeGreaterThan(-1);
474+
// Deleted categories should be filtered out
475+
expect(deleted1Index).toBe(-1);
476+
expect(deleted2Index).toBe(-1);
452477
});
453478
});

src/components/pages/BudgetVsActualPage.tsx

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -76,28 +76,44 @@ export function BudgetVsActualPage({ data }: BudgetVsActualPageProps) {
7676
const groupedCategories = useMemo(() => {
7777
const groups = new Map<string, typeof budgetComparison.categoryBudgets>();
7878

79+
// Filter out:
80+
// 1. Deleted categories (categories with names starting with "deleted: ")
81+
// 2. Categories that cannot be matched (deleted entirely from DB - category name is just the UUID)
82+
const activeCategories = budgetComparison.categoryBudgets.filter(cat => {
83+
// Filter out deleted categories
84+
if (cat.categoryName.toLowerCase().startsWith('deleted: ')) {
85+
return false;
86+
}
87+
// Filter out categories that cannot be matched (category name is just the UUID)
88+
// Special categories (uncategorized, off-budget, transfers) are always valid
89+
if (
90+
cat.categoryId === 'uncategorized' ||
91+
cat.categoryId === 'off-budget' ||
92+
cat.categoryId.startsWith('transfer:')
93+
) {
94+
return true;
95+
}
96+
// If category name matches the category ID (UUID format), it means the category doesn't exist in DB
97+
// UUIDs are typically long strings with hyphens (e.g., "123e4567-e89b-12d3-a456-426614174000")
98+
return cat.categoryName !== cat.categoryId;
99+
});
100+
79101
// Group all categories by their group (undefined group goes to "Other")
80-
budgetComparison.categoryBudgets.forEach(cat => {
102+
activeCategories.forEach(cat => {
81103
const groupName = cat.categoryGroup || 'Other';
82104
if (!groups.has(groupName)) {
83105
groups.set(groupName, []);
84106
}
85107
groups.get(groupName)!.push(cat);
86108
});
87109

88-
// Sort categories alphabetically within each group, but put deleted and income categories last
110+
// Sort categories alphabetically within each group, but put income categories last
89111
groups.forEach(categories => {
90112
categories.sort((a, b) => {
91-
const aIsDeleted = a.categoryName.toLowerCase().startsWith('deleted: ');
92-
const bIsDeleted = b.categoryName.toLowerCase().startsWith('deleted: ');
93113
const aIsIncome = a.categoryName.toLowerCase().includes('income');
94114
const bIsIncome = b.categoryName.toLowerCase().includes('income');
95115

96-
// Deleted categories go last
97-
if (aIsDeleted && !bIsDeleted) return 1;
98-
if (!aIsDeleted && bIsDeleted) return -1;
99-
100-
// Income categories go last (but before deleted)
116+
// Income categories go last
101117
if (aIsIncome && !bIsIncome) return 1;
102118
if (!aIsIncome && bIsIncome) return -1;
103119

@@ -110,18 +126,12 @@ export function BudgetVsActualPage({ data }: BudgetVsActualPageProps) {
110126
const groupSortOrder = budgetComparison.groupSortOrder || new Map<string, number>();
111127
const groupTombstones = budgetComparison.groupTombstones || new Map<string, boolean>();
112128
return Array.from(groups.entries()).sort((a, b) => {
113-
const [groupA, categoriesA] = [a[0], a[1]];
114-
const [groupB, categoriesB] = [b[0], b[1]];
115-
116-
// Check if groups are deleted (either from tombstone map or if all categories are deleted)
117-
const aIsDeleted =
118-
groupTombstones.get(groupA) === true ||
119-
(categoriesA.length > 0 &&
120-
categoriesA.every(cat => cat.categoryName.toLowerCase().startsWith('deleted: ')));
121-
const bIsDeleted =
122-
groupTombstones.get(groupB) === true ||
123-
(categoriesB.length > 0 &&
124-
categoriesB.every(cat => cat.categoryName.toLowerCase().startsWith('deleted: ')));
129+
const [groupA] = [a[0]];
130+
const [groupB] = [b[0]];
131+
132+
// Check if groups are deleted (from tombstone map)
133+
const aIsDeleted = groupTombstones.get(groupA) === true;
134+
const bIsDeleted = groupTombstones.get(groupB) === true;
125135

126136
const aIsIncome = groupA.toLowerCase().includes('income');
127137
const bIsIncome = groupB.toLowerCase().includes('income');

src/utils/dataTransform.ts

Lines changed: 69 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -171,71 +171,85 @@ function calculateBudgetComparison(
171171
actualSpendingMap.forEach((_, categoryId) => allCategoryIds.add(categoryId));
172172

173173
// Build category budgets
174-
const categoryBudgets: CategoryBudget[] = Array.from(allCategoryIds).map(categoryId => {
175-
let baseCategoryName: string;
176-
if (categoryId === 'uncategorized') {
177-
baseCategoryName = 'Uncategorized';
178-
} else if (categoryId === 'off-budget') {
179-
baseCategoryName = 'Off Budget';
180-
} else {
181-
baseCategoryName = categoryIdToName.get(categoryId) || categoryId;
182-
}
174+
// Filter out categories that don't exist in the database (unless they're special categories)
175+
const categoryBudgets: CategoryBudget[] = Array.from(allCategoryIds)
176+
.filter(categoryId => {
177+
// Keep special categories (uncategorized, off-budget, transfers)
178+
if (
179+
categoryId === 'uncategorized' ||
180+
categoryId === 'off-budget' ||
181+
categoryId.startsWith('transfer:')
182+
) {
183+
return true;
184+
}
185+
// Filter out categories that don't exist in the database
186+
return categoryIdToName.has(categoryId);
187+
})
188+
.map(categoryId => {
189+
let baseCategoryName: string;
190+
if (categoryId === 'uncategorized') {
191+
baseCategoryName = 'Uncategorized';
192+
} else if (categoryId === 'off-budget') {
193+
baseCategoryName = 'Off Budget';
194+
} else {
195+
baseCategoryName = categoryIdToName.get(categoryId) || categoryId;
196+
}
183197

184-
// Check if category is deleted and prefix with "Deleted: "
185-
const isDeleted = categoryIdToTombstone.get(categoryId) || false;
186-
const categoryName = isDeleted ? `Deleted: ${baseCategoryName}` : baseCategoryName;
198+
// Check if category is deleted and prefix with "Deleted: "
199+
const isDeleted = categoryIdToTombstone.get(categoryId) || false;
200+
const categoryName = isDeleted ? `Deleted: ${baseCategoryName}` : baseCategoryName;
187201

188-
const categoryGroup = categoryIdToGroup.get(categoryId);
202+
const categoryGroup = categoryIdToGroup.get(categoryId);
189203

190-
const budgetMapForCategory = budgetMap.get(categoryId) || new Map();
191-
const actualMapForCategory = actualSpendingMap.get(categoryId) || new Map();
204+
const budgetMapForCategory = budgetMap.get(categoryId) || new Map();
205+
const actualMapForCategory = actualSpendingMap.get(categoryId) || new Map();
192206

193-
// Calculate monthly budgets with carry forward logic
194-
let carryForwardFromPrevious = 0; // Carry forward from previous month
195-
const monthlyBudgets = MONTHS.map(monthName => {
196-
const budgetedAmount = budgetMapForCategory.get(monthName) || 0;
197-
const actualAmount = actualMapForCategory.get(monthName) || 0;
198-
const carryForward = carryForwardFromPrevious;
199-
const effectiveBudget = budgetedAmount + carryForward; // Available to spend this month
200-
const remaining = effectiveBudget - actualAmount; // What's left after spending
201-
const variance = actualAmount - effectiveBudget; // Variance against effective budget
202-
const variancePercentage = effectiveBudget !== 0 ? (variance / effectiveBudget) * 100 : 0;
207+
// Calculate monthly budgets with carry forward logic
208+
let carryForwardFromPrevious = 0; // Carry forward from previous month
209+
const monthlyBudgets = MONTHS.map(monthName => {
210+
const budgetedAmount = budgetMapForCategory.get(monthName) || 0;
211+
const actualAmount = actualMapForCategory.get(monthName) || 0;
212+
const carryForward = carryForwardFromPrevious;
213+
const effectiveBudget = budgetedAmount + carryForward; // Available to spend this month
214+
const remaining = effectiveBudget - actualAmount; // What's left after spending
215+
const variance = actualAmount - effectiveBudget; // Variance against effective budget
216+
const variancePercentage = effectiveBudget !== 0 ? (variance / effectiveBudget) * 100 : 0;
203217

204-
// Update carry forward for next month (only positive remaining amounts carry forward)
205-
carryForwardFromPrevious = remaining > 0 ? remaining : 0;
218+
// Update carry forward for next month (only positive remaining amounts carry forward)
219+
carryForwardFromPrevious = remaining > 0 ? remaining : 0;
220+
221+
return {
222+
month: monthName,
223+
budgetedAmount,
224+
actualAmount,
225+
carryForward,
226+
effectiveBudget,
227+
remaining,
228+
variance,
229+
variancePercentage,
230+
};
231+
});
232+
233+
const totalBudgeted = monthlyBudgets.reduce((sum, m) => sum + m.budgetedAmount, 0);
234+
const totalActual = monthlyBudgets.reduce((sum, m) => sum + m.actualAmount, 0);
235+
// Total variance should use effective budgets (which include carry forward)
236+
const totalEffectiveBudget = monthlyBudgets.reduce((sum, m) => sum + m.effectiveBudget, 0);
237+
const totalVariance = totalActual - totalEffectiveBudget;
238+
const totalVariancePercentage =
239+
totalEffectiveBudget !== 0 ? (totalVariance / totalEffectiveBudget) * 100 : 0;
206240

207241
return {
208-
month: monthName,
209-
budgetedAmount,
210-
actualAmount,
211-
carryForward,
212-
effectiveBudget,
213-
remaining,
214-
variance,
215-
variancePercentage,
242+
categoryId,
243+
categoryName,
244+
categoryGroup,
245+
monthlyBudgets,
246+
totalBudgeted,
247+
totalActual,
248+
totalVariance,
249+
totalVariancePercentage,
216250
};
217251
});
218252

219-
const totalBudgeted = monthlyBudgets.reduce((sum, m) => sum + m.budgetedAmount, 0);
220-
const totalActual = monthlyBudgets.reduce((sum, m) => sum + m.actualAmount, 0);
221-
// Total variance should use effective budgets (which include carry forward)
222-
const totalEffectiveBudget = monthlyBudgets.reduce((sum, m) => sum + m.effectiveBudget, 0);
223-
const totalVariance = totalActual - totalEffectiveBudget;
224-
const totalVariancePercentage =
225-
totalEffectiveBudget !== 0 ? (totalVariance / totalEffectiveBudget) * 100 : 0;
226-
227-
return {
228-
categoryId,
229-
categoryName,
230-
categoryGroup,
231-
monthlyBudgets,
232-
totalBudgeted,
233-
totalActual,
234-
totalVariance,
235-
totalVariancePercentage,
236-
};
237-
});
238-
239253
// Calculate monthly totals (using effective budgets which include carry forward)
240254
const monthlyTotals = MONTHS.map(monthName => {
241255
let totalBudgeted = 0;

0 commit comments

Comments
 (0)