fix: correct dose ID generation for empty takenBy arrays (#105)
The takenBy field is a string[]. Empty arrays [] are truthy in JavaScript,
causing d.takenBy ? [...] patterns to generate dose IDs with trailing
hyphens (e.g., '5-0-173...-') instead of base IDs ('5-0-173...').
This mismatch between ID generation and computeMissedPastDoseIds (which
correctly uses .length > 0) caused doses to always appear as missed.
Changes:
- Add expandDoseIds() helper function using correct .length > 0 check
- Replace 8 buggy inline patterns in DashboardPage.tsx
- Refactor SchedulePage.tsx to use shared expandDoseIds()
- Add backend startup repair to strip trailing hyphens from existing IDs
- Add 12 new tests (6 frontend + 6 backend)
This commit is contained in:
@@ -4,7 +4,7 @@ import { useAuth } from "../components/Auth";
|
||||
import { useAppContext } from "../context";
|
||||
import type { Coverage } from "../types";
|
||||
import { formatNumber, getExpiryClass, getSystemLocale } from "../utils/formatters";
|
||||
import { getStockStatus } from "../utils/schedule";
|
||||
import { expandDoseIds, getStockStatus } from "../utils/schedule";
|
||||
|
||||
// Helper for user-specific localStorage keys
|
||||
function userStorageKey(userId: number | undefined, key: string): string {
|
||||
@@ -490,11 +490,7 @@ export function DashboardPage() {
|
||||
{pastDays.length > 0 &&
|
||||
(() => {
|
||||
const missedCount = missedPastDoseIds.length;
|
||||
const totalPastDoses = pastDays.flatMap((d) =>
|
||||
d.meds.flatMap((m) =>
|
||||
m.doses.flatMap((dose) => (dose.takenBy ? [`${dose.id}-${dose.takenBy}`] : [dose.id]))
|
||||
)
|
||||
);
|
||||
const totalPastDoses = pastDays.flatMap((d) => d.meds.flatMap((m) => expandDoseIds(m.doses)));
|
||||
return (
|
||||
<div className="past-days-header">
|
||||
<div
|
||||
@@ -541,7 +537,10 @@ export function DashboardPage() {
|
||||
{showPastDays &&
|
||||
pastDays.map((day) => {
|
||||
const allDoseIds = day.meds.flatMap((item) =>
|
||||
item.doses.flatMap((d) => (d.takenBy ? [`${d.id}-${d.takenBy}`] : [d.id]))
|
||||
item.doses.flatMap((d) => {
|
||||
const takenByArray = Array.isArray(d.takenBy) ? d.takenBy : [];
|
||||
return takenByArray.length > 0 ? takenByArray.map((p) => `${d.id}-${p}`) : [d.id];
|
||||
})
|
||||
);
|
||||
const allDayTaken =
|
||||
allDoseIds.length > 0 && allDoseIds.every((id) => takenDoses.has(id) || dismissedDoses.has(id));
|
||||
@@ -586,7 +585,7 @@ export function DashboardPage() {
|
||||
const med = meds.find((m) => m.name === item.medName);
|
||||
const medCov = coverageByMed[item.medName];
|
||||
const isEmpty = medCov ? medCov.medsLeft <= 0 : false;
|
||||
const itemDoseIds = item.doses.flatMap((d) => (d.takenBy ? [`${d.id}-${d.takenBy}`] : [d.id]));
|
||||
const itemDoseIds = expandDoseIds(item.doses);
|
||||
const allTaken = itemDoseIds.every((id) => takenDoses.has(id));
|
||||
return (
|
||||
<div key={`${day.dateStr}-${item.medName}`} className={`time-row ${allTaken ? "taken" : ""}`}>
|
||||
@@ -676,9 +675,7 @@ export function DashboardPage() {
|
||||
{todayDay &&
|
||||
(() => {
|
||||
const day = todayDay;
|
||||
const allDoseIds = day.meds.flatMap((item) =>
|
||||
item.doses.flatMap((d) => (d.takenBy ? [`${d.id}-${d.takenBy}`] : [d.id]))
|
||||
);
|
||||
const allDoseIds = day.meds.flatMap((item) => expandDoseIds(item.doses));
|
||||
const allDayTaken = allDoseIds.length > 0 && allDoseIds.every((id) => takenDoses.has(id));
|
||||
const takenCount = allDoseIds.filter((id) => takenDoses.has(id)).length;
|
||||
|
||||
@@ -737,7 +734,7 @@ export function DashboardPage() {
|
||||
: medCoverage
|
||||
? getStockStatus(medCoverage.daysLeft, medCoverage.medsLeft, stockThresholds)
|
||||
: null;
|
||||
const itemDoseIds = item.doses.flatMap((d) => (d.takenBy ? [`${d.id}-${d.takenBy}`] : [d.id]));
|
||||
const itemDoseIds = expandDoseIds(item.doses);
|
||||
const allTaken = itemDoseIds.every((id) => takenDoses.has(id));
|
||||
return (
|
||||
<div key={`${day.dateStr}-${item.medName}`} className={`time-row ${allTaken ? "taken" : ""}`}>
|
||||
@@ -769,7 +766,7 @@ export function DashboardPage() {
|
||||
<div className="doses-col">
|
||||
{item.doses.map((dose) => {
|
||||
const isOverdue = dose.when < Date.now();
|
||||
const people = dose.takenBy ? [dose.takenBy] : [null];
|
||||
const people = dose.takenBy.length > 0 ? dose.takenBy : [null];
|
||||
const allTaken = people.every((person) => takenDoses.has(getDoseId(dose.id, person)));
|
||||
return (
|
||||
<div
|
||||
@@ -866,9 +863,7 @@ export function DashboardPage() {
|
||||
{/* Future days */}
|
||||
{showFutureDays &&
|
||||
futureDays.map((day) => {
|
||||
const allDoseIds = day.meds.flatMap((item) =>
|
||||
item.doses.flatMap((d) => (d.takenBy ? [`${d.id}-${d.takenBy}`] : [d.id]))
|
||||
);
|
||||
const allDoseIds = day.meds.flatMap((item) => expandDoseIds(item.doses));
|
||||
const allDayTaken = allDoseIds.length > 0 && allDoseIds.every((id) => takenDoses.has(id));
|
||||
const takenCount = allDoseIds.filter((id) => takenDoses.has(id)).length;
|
||||
|
||||
@@ -926,7 +921,7 @@ export function DashboardPage() {
|
||||
: medCoverage
|
||||
? getStockStatus(medCoverage.daysLeft, medCoverage.medsLeft, stockThresholds)
|
||||
: null;
|
||||
const itemDoseIds = item.doses.flatMap((d) => (d.takenBy ? [`${d.id}-${d.takenBy}`] : [d.id]));
|
||||
const itemDoseIds = expandDoseIds(item.doses);
|
||||
const allTaken = itemDoseIds.every((id) => takenDoses.has(id));
|
||||
return (
|
||||
<div key={`${day.dateStr}-${item.medName}`} className={`time-row ${allTaken ? "taken" : ""}`}>
|
||||
|
||||
@@ -3,6 +3,7 @@ import { MedicationAvatar } from "../components";
|
||||
import { useAuth } from "../components/Auth";
|
||||
import { useAppContext } from "../context";
|
||||
import type { Coverage } from "../types";
|
||||
import { expandDoseIds } from "../utils/schedule";
|
||||
|
||||
// Helper for user-specific localStorage keys
|
||||
function userStorageKey(userId: number | undefined, key: string): string {
|
||||
@@ -125,12 +126,7 @@ export function SchedulePage() {
|
||||
{/* Past days (when expanded) */}
|
||||
{showPastDays &&
|
||||
pastDays.map((day) => {
|
||||
const allDoseIds = day.meds.flatMap((item) =>
|
||||
item.doses.flatMap((d) => {
|
||||
const takenByArray = Array.isArray(d.takenBy) ? d.takenBy : [];
|
||||
return takenByArray.length > 0 ? takenByArray.map((p) => `${d.id}-${p}`) : [d.id];
|
||||
})
|
||||
);
|
||||
const allDoseIds = day.meds.flatMap((item) => expandDoseIds(item.doses));
|
||||
const allDayTaken = allDoseIds.length > 0 && allDoseIds.every((id) => takenDoses.has(id));
|
||||
const takenCount = allDoseIds.filter((id) => takenDoses.has(id)).length;
|
||||
const isManuallyExpanded = manuallyExpandedDays.has(day.dateStr);
|
||||
@@ -172,10 +168,7 @@ export function SchedulePage() {
|
||||
const med = meds.find((m) => m.name === item.medName);
|
||||
const medCov = coverageByMed[item.medName];
|
||||
const isEmpty = medCov ? medCov.medsLeft <= 0 : false;
|
||||
const itemDoseIds = item.doses.flatMap((d) => {
|
||||
const takenByArray = Array.isArray(d.takenBy) ? d.takenBy : [];
|
||||
return takenByArray.length > 0 ? takenByArray.map((p) => `${d.id}-${p}`) : [d.id];
|
||||
});
|
||||
const itemDoseIds = expandDoseIds(item.doses);
|
||||
const allTaken = itemDoseIds.every((id) => takenDoses.has(id));
|
||||
return (
|
||||
<div key={`${day.dateStr}-${item.medName}`} className={`time-row ${allTaken ? "taken" : ""}`}>
|
||||
@@ -277,10 +270,7 @@ export function SchedulePage() {
|
||||
: medCoverage
|
||||
? getStockStatus(medCoverage.daysLeft, medCoverage.medsLeft, settings)
|
||||
: null;
|
||||
const itemDoseIds = item.doses.flatMap((d) => {
|
||||
const takenByArray = Array.isArray(d.takenBy) ? d.takenBy : [];
|
||||
return takenByArray.length > 0 ? takenByArray.map((p) => `${d.id}-${p}`) : [d.id];
|
||||
});
|
||||
const itemDoseIds = expandDoseIds(item.doses);
|
||||
const allTaken = itemDoseIds.every((id) => takenDoses.has(id));
|
||||
return (
|
||||
<div key={`${day.dateStr}-${item.medName}`} className={`time-row ${allTaken ? "taken" : ""}`}>
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
buildSchedulePreview,
|
||||
calculateCoverage,
|
||||
computeMissedPastDoseIds,
|
||||
expandDoseIds,
|
||||
getNextReminderForMed,
|
||||
getReminderStatusText,
|
||||
getStockStatus,
|
||||
@@ -1147,3 +1148,52 @@ function groupEventsIntoPastDays(
|
||||
meds: Array.from(medMap.entries()).map(([medName, doses]) => ({ medName, doses })),
|
||||
}));
|
||||
}
|
||||
|
||||
describe("expandDoseIds", () => {
|
||||
it("returns base IDs when takenBy is empty array", () => {
|
||||
const doses = [
|
||||
{ id: "1-0-1729123200000", takenBy: [] as string[] },
|
||||
{ id: "2-0-1729123200000", takenBy: [] as string[] },
|
||||
];
|
||||
const result = expandDoseIds(doses);
|
||||
expect(result).toEqual(["1-0-1729123200000", "2-0-1729123200000"]);
|
||||
});
|
||||
|
||||
it("returns person-suffixed IDs when takenBy has entries", () => {
|
||||
const doses = [{ id: "1-0-1729123200000", takenBy: ["Alice"] }];
|
||||
const result = expandDoseIds(doses);
|
||||
expect(result).toEqual(["1-0-1729123200000-Alice"]);
|
||||
});
|
||||
|
||||
it("returns multiple IDs for multiple takenBy entries", () => {
|
||||
const doses = [{ id: "1-0-1729123200000", takenBy: ["Alice", "Bob"] }];
|
||||
const result = expandDoseIds(doses);
|
||||
expect(result).toEqual(["1-0-1729123200000-Alice", "1-0-1729123200000-Bob"]);
|
||||
});
|
||||
|
||||
it("handles mix of empty and non-empty takenBy", () => {
|
||||
const doses = [
|
||||
{ id: "1-0-1729123200000", takenBy: ["Alice"] },
|
||||
{ id: "2-0-1729123200000", takenBy: [] as string[] },
|
||||
{ id: "3-0-1729123200000", takenBy: ["Bob", "Carol"] },
|
||||
];
|
||||
const result = expandDoseIds(doses);
|
||||
expect(result).toEqual([
|
||||
"1-0-1729123200000-Alice",
|
||||
"2-0-1729123200000",
|
||||
"3-0-1729123200000-Bob",
|
||||
"3-0-1729123200000-Carol",
|
||||
]);
|
||||
});
|
||||
|
||||
it("returns empty array for empty doses", () => {
|
||||
expect(expandDoseIds([])).toEqual([]);
|
||||
});
|
||||
|
||||
it("handles non-array takenBy gracefully", () => {
|
||||
// In case of unexpected data, treat non-array as empty
|
||||
const doses = [{ id: "1-0-1729123200000", takenBy: null as unknown as string[] }];
|
||||
const result = expandDoseIds(doses);
|
||||
expect(result).toEqual(["1-0-1729123200000"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -446,6 +446,18 @@ export function isDoseDismissed(doseId: string, dismissedUntilDate: string | und
|
||||
* @param dismissedDoses - Set of dose IDs individually dismissed
|
||||
* @returns Array of dose IDs that are missed
|
||||
*/
|
||||
/**
|
||||
* Compute the full set of dose IDs for a list of doses, correctly handling
|
||||
* per-intake takenBy arrays. Empty arrays produce base IDs (no suffix),
|
||||
* non-empty arrays produce one ID per person with a `-person` suffix.
|
||||
*/
|
||||
export function expandDoseIds(doses: ReadonlyArray<{ id: string; takenBy: string[] }>): string[] {
|
||||
return doses.flatMap((d) => {
|
||||
const takenByArray = Array.isArray(d.takenBy) ? d.takenBy : [];
|
||||
return takenByArray.length > 0 ? takenByArray.map((p) => `${d.id}-${p}`) : [d.id];
|
||||
});
|
||||
}
|
||||
|
||||
export function computeMissedPastDoseIds(
|
||||
pastDays: ReadonlyArray<{
|
||||
meds: ReadonlyArray<{
|
||||
|
||||
Reference in New Issue
Block a user