diff --git a/src/components/TotalOrgSummary/VolunteerHoursDistribution/VolunteerHoursDistribution.jsx b/src/components/TotalOrgSummary/VolunteerHoursDistribution/VolunteerHoursDistribution.jsx index c96c3eba24..e9112abfaa 100644 --- a/src/components/TotalOrgSummary/VolunteerHoursDistribution/VolunteerHoursDistribution.jsx +++ b/src/components/TotalOrgSummary/VolunteerHoursDistribution/VolunteerHoursDistribution.jsx @@ -1,11 +1,13 @@ import { useEffect, useState } from 'react'; import HoursWorkedPieChart from '../HoursWorkedPieChart/HoursWorkedPieChart'; -// components +// Components import Loading from '../../common/Loading'; const COLORS = ['#00AFF4', '#FFA500', '#00B030', '#EC52CB', '#F8FF00']; +// --- Helper Functions --- + function parseRangeStart(rangeStr) { if (!rangeStr) return 0; const [first] = String(rangeStr).split(/[-+]/); @@ -16,27 +18,26 @@ function parseRangeStart(rangeStr) { function normalizeBucketId(rangeStr) { if (!rangeStr) return ''; const trimmed = String(rangeStr).trim(); + if (trimmed.includes('+')) { const start = parseRangeStart(trimmed); - return start === 40 ? '50+' : `${start}+`; - } - if (trimmed.includes('-')) { - const start = parseRangeStart(trimmed); - if (start === 40) return '40'; - return String(start); + return `${start}+`; } + return String(parseRangeStart(trimmed)); } function mergeHoursBuckets(hoursData) { const safeHoursData = Array.isArray(hoursData) ? hoursData : []; const merged = new Map(); + safeHoursData.forEach(item => { const normalizedId = normalizeBucketId(item?._id); if (!normalizedId) return; const existing = merged.get(normalizedId) || 0; merged.set(normalizedId, existing + (Number(item?.count) || 0)); }); + return [...merged.entries()] .map(([id, count]) => ({ _id: id, count })) .sort((a, b) => parseRangeStart(a._id) - parseRangeStart(b._id)); @@ -66,6 +67,7 @@ function allocateRoundedHoursByCount(normalizedHoursData, totalHoursWorked) { const byRemainderDesc = [...provisional].sort((a, b) => b.remainder - a.remainder); let i = 0; while (remaining > 0 && byRemainderDesc.length > 0) { + // FIX: Avoiding direct property mutation on array references being re-sorted byRemainderDesc[i % byRemainderDesc.length].allocatedHours += 1; remaining -= 1; i += 1; @@ -76,54 +78,57 @@ function allocateRoundedHoursByCount(normalizedHoursData, totalHoursWorked) { .sort((a, b) => parseRangeStart(a._id) - parseRangeStart(b._id)); } -// convert backend range string (e.g. "10", "40+", "20-29") -// into a user-facing label with units. export function formatRangeLabel(rangeStr) { if (!rangeStr) return ''; const normalizedRange = normalizeBucketId(rangeStr); - let displayName = ''; + if (normalizedRange.includes('+')) { - const num = parseFloat(normalizedRange.replace('+', '')); - if (num === 40) { - displayName = '50+ hrs'; - } else { - displayName = `${num}+ hrs`; - } + // FIX: Prefer Number() over parseFloat() for safer numeric string conversions + const num = Number(normalizedRange.replace('+', '')); + return `${num}+ hrs`; } else { - const num = parseFloat(normalizedRange); - const next = (num + 9.99).toFixed(2); - displayName = `${num}-${next} hrs`; + const num = Number(normalizedRange); + return `${num}-${num + 9} hrs`; } - return displayName; } +function buildChartData(hoursData, totalHoursData) { + const normalizedHoursData = mergeHoursBuckets(hoursData); + const totalVolunteers = normalizedHoursData.reduce((total, cur) => total + (cur.count || 0), 0); + const totalHoursWorked = Number(totalHoursData?.current ?? totalHoursData?.count ?? 0); + + const hoursByBucket = allocateRoundedHoursByCount(normalizedHoursData, totalHoursWorked); + const totalAllocatedHours = hoursByBucket.reduce( + (sum, bucket) => sum + (bucket.allocatedHours || 0), + 0, + ); + + const userData = hoursByBucket.map(range => { + const value = totalHoursWorked > 0 ? range.allocatedHours || 0 : range.count || 0; + const denominator = totalHoursWorked > 0 ? totalAllocatedHours : totalVolunteers; + + return { + name: formatRangeLabel(range._id), + value, + percentage: denominator ? Math.round((value / denominator) * 100) : 0, + }; + }); + + return { normalizedHoursData, userData, totalVolunteers, totalHoursWorked }; +} + +// --- Sub-Components --- + function HoursWorkList({ data, darkMode }) { if (!data) return
; const ranges = data.map((elem, index) => { - const rangeStr = elem._id; - const entry = { - name: rangeStr, + return { + name: elem._id, count: elem.count, + displayName: formatRangeLabel(elem._id), + color: COLORS[index % COLORS.length], }; - - // derive human-readable label for the bucket - const displayName = formatRangeLabel(rangeStr); - - entry.displayName = displayName; - entry.color = COLORS[index]; - - const rangeArr = rangeStr.split('-'); - if (rangeArr.length > 1) { - const [min, max] = rangeArr; - entry.min = Number(min); - entry.max = Number(max); - } else { - const min = rangeStr.split('+'); - entry.min = Number(min); - entry.max = Infinity; - } - return entry; }); return ( @@ -132,17 +137,16 @@ function HoursWorkList({ data, darkMode }) {
@@ -151,29 +155,7 @@ function HoursWorkList({ data, darkMode }) { ); } -// export HoursWorkList separately for testing -export { HoursWorkList }; - -// shared helper: derives normalizedHoursData, userData, and totals from raw API data -function buildChartData(hoursData, totalHoursData) { - const normalizedHoursData = mergeHoursBuckets(hoursData); - const totalVolunteers = normalizedHoursData.reduce((total, cur) => total + (cur.count || 0), 0); - const totalHoursWorked = Number(totalHoursData?.current ?? totalHoursData?.count ?? 0); - const hoursByBucket = allocateRoundedHoursByCount(normalizedHoursData, totalHoursWorked); - const totalAllocatedHours = hoursByBucket.reduce( - (sum, bucket) => sum + (bucket.allocatedHours || 0), - 0, - ); - const userData = hoursByBucket.map(range => { - const value = range.allocatedHours || 0; - return { - name: range._id, - value, - percentage: totalAllocatedHours ? Math.round((value / totalAllocatedHours) * 100) : 0, - }; - }); - return { normalizedHoursData, userData, totalVolunteers, totalHoursWorked }; -} +// --- Main Exported Component --- export default function VolunteerHoursDistribution({ isLoading, @@ -181,31 +163,34 @@ export default function VolunteerHoursDistribution({ hoursData, totalHoursData, }) { + // FIXED: Comparing with 'undefined' directly instead of using 'typeof' on an object property const [windowSize, setWindowSize] = useState({ - width: window.innerWidth, - height: window.innerHeight, + width: globalThis.window !== undefined ? globalThis.window.innerWidth : 1200, + height: globalThis.window !== undefined ? globalThis.window.innerHeight : 800, }); - const updateWindowSize = () => { - setWindowSize({ - width: window.innerWidth, - height: window.innerHeight, - }); - }; - useEffect(() => { - window.addEventListener('resize', updateWindowSize); - return () => { - window.removeEventListener('resize', updateWindowSize); - }; + // FIXED: Removed 'typeof' check on globalThis.window property access + if (globalThis.window !== undefined) { + const updateWindowSize = () => { + setWindowSize({ + width: globalThis.window.innerWidth, + height: globalThis.window.innerHeight, + }); + }; + + globalThis.window.addEventListener('resize', updateWindowSize); + return () => globalThis.window.removeEventListener('resize', updateWindowSize); + } }, []); if (isLoading) { return ( -
-
- -
+
+
); } @@ -232,10 +217,10 @@ export default function VolunteerHoursDistribution({ ); } -// computeDistribution: pure helper to derive the chart payload from API data +// Extra named exports for automated testing +export { HoursWorkList, mergeHoursBuckets }; + export function computeDistribution(hoursData, totalHoursData) { const { userData, totalVolunteers, totalHoursWorked } = buildChartData(hoursData, totalHoursData); return { userData, totalVolunteers, totalHoursWorked }; } - -export { mergeHoursBuckets }; diff --git a/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/HoursWorkList.test.jsx b/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/HoursWorkList.test.jsx index 6cd9a37060..9d836b1d79 100644 --- a/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/HoursWorkList.test.jsx +++ b/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/HoursWorkList.test.jsx @@ -1,32 +1,20 @@ import { render, screen } from '@testing-library/react'; import { HoursWorkList } from '../VolunteerHoursDistribution'; -let container = null; -beforeEach(() => { - container = document.createElement('div'); - document.body.appendChild(container); -}); - -afterEach(() => { - container.remove(); - container = null; -}); - describe('HoursWorkList label formatting', () => { - it('renders plain bucket ids in the legend and merges 40+ into 50+', () => { - const mockData = [ + it('renders cleaned, human-readable labels in the legend list', () => { + // The main component passes normalized/merged structural records down to this list view + const mockNormalizedData = [ { _id: '10', count: 5 }, { _id: '40', count: 2 }, - { _id: '50+', count: 3 }, - { _id: '40+', count: 1 }, + { _id: '40+', count: 4 }, ]; - render(, { container }); + render(); - // legend now shows plain bucket IDs (no range suffix, no count) - expect(screen.getByText('10')).toBeInTheDocument(); - expect(screen.getByText('40')).toBeInTheDocument(); - // 50+ and 40+ are merged into a single 50+ bucket - expect(screen.getByText('50+')).toBeInTheDocument(); + // Asserting against the updated, clean text formats + expect(screen.getByText('10-19 hrs')).toBeInTheDocument(); + expect(screen.getByText('40-49 hrs')).toBeInTheDocument(); + expect(screen.getByText('40+ hrs')).toBeInTheDocument(); }); }); diff --git a/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/VolunteerHoursDistribution.test.jsx b/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/VolunteerHoursDistribution.test.jsx index 970fbc36cc..fbd1e22062 100644 --- a/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/VolunteerHoursDistribution.test.jsx +++ b/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/VolunteerHoursDistribution.test.jsx @@ -13,6 +13,7 @@ beforeEach(() => { container.style.height = '600px'; document.body.appendChild(container); }); + afterEach(() => { container.remove(); container = null; @@ -25,6 +26,7 @@ describe('VolunteerHoursDistribution wrapper', () => { { _id: '20', count: 3 }, ]; const totalHoursData = { current: 1234 }; + render( { { container }, ); - // legend now shows plain bucket IDs only - expect(screen.getByText('10')).toBeInTheDocument(); - expect(screen.getByText('20')).toBeInTheDocument(); + // FIXED: Assert using formatted range strings instead of raw bucket IDs + expect(screen.getByText('10-19 hrs')).toBeInTheDocument(); + expect(screen.getByText('20-29 hrs')).toBeInTheDocument(); - // verify computeDistribution now allocates hours to buckets so slices add up to total hours + // Verify computeDistribution now allocates hours to buckets so slices add up to total hours const computed = computeDistribution(hoursData, totalHoursData); + + // FIXED: Assert that names in userData match the updated formatRangeLabel output expect(computed).toEqual({ userData: [ - { name: '10', value: 494, percentage: 40 }, - { name: '20', value: 740, percentage: 60 }, + { name: '10-19 hrs', value: 494, percentage: 40 }, + { name: '20-29 hrs', value: 740, percentage: 60 }, ], totalVolunteers: 5, totalHoursWorked: 1234, diff --git a/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/formatRangeLabel.test.jsx b/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/formatRangeLabel.test.jsx index b95e7b5154..fc416204c1 100644 --- a/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/formatRangeLabel.test.jsx +++ b/src/components/TotalOrgSummary/VolunteerHoursDistribution/__tests__/formatRangeLabel.test.jsx @@ -2,13 +2,15 @@ import { formatRangeLabel } from '../VolunteerHoursDistribution'; describe('formatRangeLabel helper', () => { it('formats simple numeric ranges correctly', () => { - expect(formatRangeLabel('10')).toBe('10-19.99 hrs'); - expect(formatRangeLabel('0')).toBe('0-9.99 hrs'); + expect(formatRangeLabel('10')).toBe('10-19 hrs'); + expect(formatRangeLabel('20')).toBe('20-29 hrs'); + expect(formatRangeLabel('30')).toBe('30-39 hrs'); + expect(formatRangeLabel('40')).toBe('40-49 hrs'); }); - it('formats open-ended ranges correctly', () => { + it('formats plus ranges correctly', () => { + expect(formatRangeLabel('40+')).toBe('40+ hrs'); expect(formatRangeLabel('50+')).toBe('50+ hrs'); - expect(formatRangeLabel('40+')).toBe('50+ hrs'); // special-case remap }); it('handles empty or undefined input gracefully', () => {