yiekheng/cm_whatsapp_bot_v1
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(reminders): edit paused/ended one-off; lock list order; collapse swipe after action

Browse Source 
Three small bugs the user surfaced after the swipe rollout:

1. Editing a paused/ended one-off reminder threw "Time is in the past"
   ----------------------------------------------------------------------
   The four edit-section pages (account / message / groups / when) all
   POST through `updateReminderAction`. The action's "scheduledAt
   must be in the future" check fires on every submit — including the
   three section pages that don't change the time and just pass the
   original `scheduledAt` straight through. So a user editing the
   message body of a reminder they paused yesterday saw their save
   rejected with "Time is in the past".

   New pure helper `validateUpdateScheduledAt` in lib/reminder-update.ts
   keeps the future-time check in place for active reminders that are
   actually changing the time, but allows past timestamps when:
     - the reminder is paused or ended (won't fire while in those
       states regardless of what the row says about scheduledAt), OR
     - the submitted timestamp matches the existing one within a
       second of rounding (the form is a passthrough).

   Tests: 10 cases in `lib/reminder-update.test.ts` covering active
   future, active past, paused passthrough, ended passthrough, paused
   with deliberate change, sub-second drift tolerance, exact-NOW edge,
   null existing scheduledAt, malformed ISO.

   Also (drive-by, related): `updateReminderAction` no longer force-
   sets `status: "active"` on save. Editing a paused reminder's
   message shouldn't silently un-pause it. The user uses Restart for
   that.

2. Reminder list reshuffled after Pause/Restart
   ----------------------------------------------------------------------
   The list defaulted to `sort=scheduled_desc`, so clicking Restart on
   row N (which moves scheduledAt forward to the next occurrence) flipped
   the row to row 0. Felt like the wrong action ran. Fixed:
     - Page now hard-codes `sort = "created_desc"` (created_at never
       changes, so a row stays where it is).
     - Sort dropdown removed from `<ReminderFilterBar>` since it has
       nothing to drive anymore. Account + Group filters and the
       search box stay.

3. Swipe shelf stayed open after the action ran
   ----------------------------------------------------------------------
   `SwipeableRow` keeps its offset in component state. When a shelf
   button submits the form, the page revalidates and re-renders, but
   React keeps the same row instance (matched by `key={reminder.id}`),
   so the open offset stuck around. Now both row sites encode the
   "row state" into the key:
     - reminders: `key={\`${reminder.id}-${reminder.status}\`}`
     - activity:  `key={\`${run.id}-${run.archivedAt ? "1" : "0"}\`}`
   Status flip → key change → React unmounts/remounts → offset back
   to 0 → shelf closed. Costs nothing (these rows are cheap).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
yiekheng 2026-05-10 12:29:10 +08:00
parent 8023c8f357
commit 32319feeea
6 changed files with 212 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
apps/web/src/actions/reminders.ts
View File

@ -12,6 +12,7 @@ import { db } from "@/lib/db";
import { getSeededOperator } from "@/lib/operator";
import { checkRateLimit } from "@/lib/rate-limit";
import { pgNotifyBot } from "@/lib/notify";
import { validateUpdateScheduledAt } from "@/lib/reminder-update";
async function rateLimit(key: string) {
const h = await headers();
@ -427,13 +428,15 @@ export async function updateReminderAction(
}
scheduledAt = firstFire;
} else {
scheduledAt = DateTime.fromISO(scheduledAtIso, { zone: timezone }).toJSDate();
if (Number.isNaN(scheduledAt.getTime())) {
return { ok: false, error: "Invalid date" };
}
if (scheduledAt.getTime() <= Date.now()) {
return { ok: false, error: "Time is in the past" };
}
const validated = validateUpdateScheduledAt({
iso: scheduledAtIso,
timezone,
existingStatus: existing.status,
existingScheduledAt: existing.scheduledAt,
now: new Date(),
});
if (!validated.ok) return { ok: false, error: validated.error };
scheduledAt = validated.scheduledAt;
}
const groups = await db.query.whatsappGroups.findMany({
@ -456,7 +459,10 @@ export async function updateReminderAction(
scheduledAt,
rrule: rrule ?? null,
timezone,
status: "active",
// Preserve the lifecycle status. Editing fields shouldn't
// implicitly re-activate a paused or ended reminder — the
// user can use the explicit Restart action for that.
status: existing.status,
updatedAt: new Date(),
})
.where(eq(reminders.id, reminderId));

4
apps/web/src/app/activity/page.tsx
View File

@ -281,7 +281,9 @@ export default async function ActivityPage({ searchParams }: PageProps) {
return (
<SwipeableRow
key={run.id}
// Key includes the archived flag so flipping it
// remounts the row with a fresh offset (closed shelf).
key={`${run.id}-${run.archivedAt ? "1" : "0"}`}
// Right swipe → reveal left shelf → Archive (non-destructive).
leftActions={
<ArchiveShelfButton runId={run.id} isArchived={Boolean(run.archivedAt)} />

16
apps/web/src/app/reminders/page.tsx
View File

@ -131,9 +131,12 @@ export default async function RemindersPage({ searchParams }: PageProps) {
sp.filter === "active" || sp.filter === "ended" || sp.filter === "paused"
? sp.filter
: "all";
const sort: SortKey = (VALID_SORT_KEYS as string[]).includes(sp.sort ?? "")
? (sp.sort as SortKey)
: "scheduled_desc";
// Sort is now fixed to `created_desc`. Reordering on every status flip
// (Restart, Pause, Edit) was causing rows to jump around the list,
// which made the swipe gesture feel like the wrong thing happened.
// `created_at` never changes so the row stays put.
const sort: SortKey = "created_desc";
void VALID_SORT_KEYS; // kept for future use; no longer read from URL
const op = await getSeededOperator();
const tz = op.defaultTimezone ?? "UTC";
@ -322,7 +325,12 @@ export default async function RemindersPage({ searchParams }: PageProps) {
return (
<SwipeableRow
key={reminder.id}
// Key includes both id and status so a status change
// (Pause / Restart / Delete result) remounts the row,
// which resets its swipe offset back to closed. Without
// this, clicking a shelf button leaves the shelf open
// even after the row's content updates.
key={`${reminder.id}-${reminder.status}`}
leftActions={leftShelf}
rightActions={
<ReminderShelfButton

28
apps/web/src/components/reminder-filter-bar.tsx
View File

@ -5,7 +5,6 @@ import { usePathname, useRouter, useSearchParams } from "next/navigation";
import { SearchIcon, XIcon } from "lucide-react";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
import type { SortKey } from "@/lib/reminder-filter";
interface AccountOption {
id: string;
@ -22,13 +21,6 @@ interface FilterBarProps {
groups: GroupOption[];
}
const SORT_OPTIONS: Array<{ value: SortKey; label: string }> = [
{ value: "scheduled_desc", label: "Newest first" },
{ value: "scheduled_asc", label: "Oldest first" },
{ value: "created_desc", label: "Recently created" },
{ value: "name_asc", label: "Name A→Z" },
];
/**
* URL-driven filter row for /reminders. The page reads searchParams
* server-side and re-renders with the filtered+sorted dataset; this
@ -46,7 +38,6 @@ export function ReminderFilterBar({ accounts, groups }: FilterBarProps) {
q: searchParams.get("q") ?? "",
accountId: searchParams.get("accountId") ?? "",
groupId: searchParams.get("groupId") ?? "",
sort: (searchParams.get("sort") ?? "scheduled_desc") as SortKey,
};
const [q, setQ] = useState(initial.q);
@ -116,7 +107,7 @@ export function ReminderFilterBar({ accounts, groups }: FilterBarProps) {
)}
</div>
<div className="grid grid-cols-1 sm:grid-cols-3 gap-2">
<div className="grid grid-cols-1 sm:grid-cols-2 gap-2">
<div className="space-y-1">
<Label htmlFor="filter-account" className="text-xs text-muted-foreground">
Account
@ -156,23 +147,6 @@ export function ReminderFilterBar({ accounts, groups }: FilterBarProps) {
</select>
</div>
<div className="space-y-1">
<Label htmlFor="filter-sort" className="text-xs text-muted-foreground">
Sort
</Label>
<select
id="filter-sort"
value={initial.sort}
onChange={(e) => setParam("sort", e.target.value)}
className="h-8 w-full rounded-lg border border-input bg-transparent px-2 text-sm focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
>
{SORT_OPTIONS.map(({ value, label }) => (
<option key={value} value={value}>
{label}
</option>
))}
</select>
</div>
</div>
{hasActiveFilter && (

137
apps/web/src/lib/reminder-update.test.ts Normal file
View File

@ -0,0 +1,137 @@
import { describe, it, expect } from "vitest";
import { validateUpdateScheduledAt } from "./reminder-update";
const NOW = new Date("2026-05-13T10:00:00Z");
const PAST = new Date("2026-05-01T09:00:00Z");
const FUTURE = new Date("2026-05-20T09:00:00Z");
const TZ = "Asia/Kuala_Lumpur";
const isoOf = (d: Date) => d.toISOString();
describe("validateUpdateScheduledAt", () => {
it("rejects malformed ISO outright", () => {
const r = validateUpdateScheduledAt({
iso: "not-a-date",
timezone: TZ,
existingStatus: "active",
existingScheduledAt: PAST,
now: NOW,
});
expect(r.ok).toBe(false);
if (!r.ok) expect(r.error).toBe("Invalid date");
});
it("active reminder, future timestamp → ok", () => {
const r = validateUpdateScheduledAt({
iso: isoOf(FUTURE),
timezone: TZ,
existingStatus: "active",
existingScheduledAt: FUTURE,
now: NOW,
});
expect(r.ok).toBe(true);
});
it("active reminder, past timestamp that DIFFERS from existing → rejected", () => {
const r = validateUpdateScheduledAt({
iso: isoOf(PAST),
timezone: TZ,
existingStatus: "active",
existingScheduledAt: new Date("2026-05-02T09:00:00Z"),
now: NOW,
});
expect(r.ok).toBe(false);
if (!r.ok) expect(r.error).toBe("Time is in the past");
});
it("paused one-off, past timestamp matching existing → ALLOWED (the bug fix)", () => {
// The user paused a fired reminder; now they're editing the message
// body via /reminders/[id]/edit/message, which submits the original
// (past) scheduledAt unchanged. Must not 400.
const r = validateUpdateScheduledAt({
iso: isoOf(PAST),
timezone: TZ,
existingStatus: "paused",
existingScheduledAt: PAST,
now: NOW,
});
expect(r.ok).toBe(true);
if (r.ok) expect(r.scheduledAt.getTime()).toBe(PAST.getTime());
});
it("ended one-off, past timestamp matching existing → ALLOWED", () => {
const r = validateUpdateScheduledAt({
iso: isoOf(PAST),
timezone: TZ,
existingStatus: "ended",
existingScheduledAt: PAST,
now: NOW,
});
expect(r.ok).toBe(true);
});
it("paused reminder, past timestamp that's been changed → still allowed (paused doesn't fire)", () => {
// Paused reminders aren't on the firing path so a stale time isn't
// dangerous — even if the user submits a different past time, we
// accept it; it'll fire only if/when the user explicitly Restarts.
const r = validateUpdateScheduledAt({
iso: isoOf(new Date("2026-04-01T09:00:00Z")),
timezone: TZ,
existingStatus: "paused",
existingScheduledAt: PAST,
now: NOW,
});
expect(r.ok).toBe(true);
});
it("active reminder, past-but-equal-to-existing timestamp → allowed", () => {
// Edge case: the user opened the edit-message page on an active
// reminder that's about to fire; by the time they submit, NOW has
// ticked past existing.scheduledAt. Don't reject — it's the same
// moment the reminder already represents.
const existing = new Date("2026-05-13T09:59:00Z"); // 1 minute before NOW
const r = validateUpdateScheduledAt({
iso: isoOf(existing),
timezone: TZ,
existingStatus: "active",
existingScheduledAt: existing,
now: NOW,
});
expect(r.ok).toBe(true);
});
it("active reminder, exact NOW → rejected (must be strictly future)", () => {
const r = validateUpdateScheduledAt({
iso: isoOf(NOW),
timezone: TZ,
existingStatus: "active",
existingScheduledAt: new Date("2026-05-19T09:00:00Z"),
now: NOW,
});
expect(r.ok).toBe(false);
});
it("treats existingScheduledAt=null as 'no match' (active + past → reject)", () => {
const r = validateUpdateScheduledAt({
iso: isoOf(PAST),
timezone: TZ,
existingStatus: "active",
existingScheduledAt: null,
now: NOW,
});
expect(r.ok).toBe(false);
});
it("sub-second drift (~750ms) still counts as 'same as existing'", () => {
const exact = new Date("2026-05-13T05:00:00.000Z");
const drifted = new Date("2026-05-13T05:00:00.750Z");
const r = validateUpdateScheduledAt({
iso: isoOf(drifted),
timezone: TZ,
existingStatus: "active",
existingScheduledAt: exact,
now: NOW,
});
expect(r.ok).toBe(true);
});
});

45
apps/web/src/lib/reminder-update.ts Normal file
View File

@ -0,0 +1,45 @@
import { DateTime } from "luxon";
/**
* Resolve and validate the scheduledAt for `updateReminderAction`.
*
* Why this exists: the action is invoked from four edit forms
* (account / message / groups / when). Three of those don't change
* the time — they pass the original `scheduledAt` straight through.
* For a paused/ended one-off whose original time is now in the past
* (the operator paused it after a fire and is editing the message),
* a strict "must be future" check rejects the edit even though the
* operator only wanted to fix a typo. So:
*
* - Reject malformed ISO outright.
* - Allow past timestamps when:
* * the reminder's existing status is `paused` or `ended` —
* it won't fire in those states regardless, AND/OR
* * the submitted timestamp matches the existing one (within
* a second of rounding), i.e. the form is passing it through
* unchanged.
* - Otherwise reject past timestamps so an active reminder can't
* be scheduled into a moment that's already gone.
*
* Pure function — takes `now` as input so tests can pin the clock.
*/
export function validateUpdateScheduledAt(args: {
iso: string;
timezone: string;
existingStatus: string;
existingScheduledAt: Date | null;
now: Date;
}): { ok: true; scheduledAt: Date } | { ok: false; error: string } {
const dt = DateTime.fromISO(args.iso, { zone: args.timezone }).toJSDate();
if (Number.isNaN(dt.getTime())) {
return { ok: false, error: "Invalid date" };
}
const isPaused = args.existingStatus === "paused" || args.existingStatus === "ended";
const sameAsExisting =
args.existingScheduledAt !== null &&
Math.abs(args.existingScheduledAt.getTime() - dt.getTime()) < 1000;
if (dt.getTime() <= args.now.getTime() && !isPaused && !sameAsExisting) {
return { ok: false, error: "Time is in the past" };
}
return { ok: true, scheduledAt: dt };
}