mirror of https://github.com/usememos/memos
feat(activity-calendar): aggregate by ViewContext.timeBasis
Fixes the inconsistency where switching the memo list to update_time
left the activity heatmap aggregating by created_time. The heatmap
now follows the same time basis as the list it sits next to.
Backend
- UserStats gains memo_updated_timestamps (additive proto field, tag 8).
- GetUserStats and ListAllUserStats populate it alongside the existing
memo_created_timestamps. No DB migration; memo.updated_ts already
exists on every row.
Frontend
- useFilteredMemoStats reads timeBasis from ViewContext and selects
the matching timestamp source.
- StatisticsView and MonthNavigator forward timeBasis through to
MonthCalendar / YearCalendar so tooltip text matches the basis
("X memos in DATE" vs "X memos updated on DATE").
- Falls back to memoCreatedTimestamps when an old server returns an
empty memoUpdatedTimestamps array (detected by length divergence,
since protobuf-es deserializes missing repeated fields as []).
Tests
- Backend: TestGetUserStats_MemoUpdatedTimestamps verifies the field
is populated and reflects post-creation updates.
- Frontend: filtered-memo-stats covers create/update source switching
and the old-server fallback path; activity-calendar-tooltip covers
basis-aware label selection.
Spec and implementation plan committed under docs/superpowers/.
pull/5925/head
Steven
4 weeks ago
parent
ea0625da45
commit
8daef1dc89
File diff suppressed because it is too large
Load Diff
@ -0,0 +1,124 @@
|
||||
# ActivityCalendar: Honor `timeBasis` (Create vs Update)
|
||||
|
||||
## Problem
|
||||
|
||||
The ActivityCalendar in `web/src/components/ActivityCalendar/` always aggregates by memo creation time, regardless of how the surrounding memo list is sorted.
|
||||
|
||||
The application already supports a global "time basis" toggle (`web/src/contexts/ViewContext.tsx:3`):
|
||||
|
||||
```ts
|
||||
export type MemoTimeBasis = "create_time" | "update_time";
|
||||
```
|
||||
|
||||
The toggle is persisted in `localStorage` and drives memo list ordering across the app. When a user switches the list to `update_time`, the heatmap below it continues to show creation counts — the two views literally disagree about what "today" means.
|
||||
|
||||
This is the user-visible bug we are fixing.
|
||||
|
||||
## Non-goals
|
||||
|
||||
The following were considered and explicitly excluded:
|
||||
|
||||
- **Tracking every individual edit event.** This would require resurrecting the `activity` table that was deliberately dropped in migration `0.27/03__drop_activity.sql`. The cost (write-path instrumentation, storage growth, privacy review) is not justified by this UI bug.
|
||||
- **Tracking archive / restore / delete events.** Housekeeping actions, not contributions; would also leak private behavior on public Profile/Explore pages.
|
||||
- **Adding comments or reactions to the heatmap count.** A reasonable separate feature, but a different decision (event-type expansion). Out of scope for this spec — one ticket, one problem.
|
||||
- **Renaming `ActivityCalendar` to `ContributionCalendar`.** Out of scope.
|
||||
|
||||
## Design
|
||||
|
||||
### Semantics
|
||||
|
||||
The heatmap aggregates one timestamp per memo:
|
||||
|
||||
- When `timeBasis === "create_time"`: use `memo.created_ts` (current behavior).
|
||||
- When `timeBasis === "update_time"`: use `memo.updated_ts`.
|
||||
|
||||
Each memo contributes exactly one cell of color, on the day of its chosen timestamp. This matches the list view's semantics exactly: in `update_time` mode, a memo edited on 5/1 and again on 5/2 appears once at 5/2 in the list, and the heatmap will show +1 on 5/2 and nothing on 5/1. The "lossiness" is identical to the lossiness already accepted by the list view — so by definition, the two are consistent.
|
||||
|
||||
### Backend
|
||||
|
||||
`UserStats` (proto/api/v1/user_service.proto) gains one field:
|
||||
|
||||
```proto
|
||||
// The latest update timestamps of the user's memos.
|
||||
repeated google.protobuf.Timestamp memo_updated_timestamps = 8;
|
||||
```
|
||||
|
||||
The implementation mirrors `memo_created_timestamps` in `server/router/api/v1/user_service_stats.go`: in the same loop that appends `memo.CreatedTs` (line 115), also append `memo.UpdatedTs` to a parallel slice. The same `FindMemo` filters apply automatically: `RowStatus: NORMAL` (archived excluded), `ExcludeComments: true`, and the viewer-based visibility filter. Both `GetUserStats` and `ListAllUserStats` paths must be updated symmetrically.
|
||||
|
||||
No DB migration. No new tables. No new write paths.
|
||||
|
||||
### Frontend
|
||||
|
||||
`web/src/hooks/useFilteredMemoStats.ts` reads `useView().timeBasis` and switches its data source:
|
||||
|
||||
- `create_time` → `userStats.memoCreatedTimestamps` (today's behavior, untouched)
|
||||
- `update_time` → `userStats.memoUpdatedTimestamps`
|
||||
|
||||
The `explore` context branch (which derives stats from the in-memory memo list rather than `userStats`) applies the same switch using `memo.createTime` vs `memo.updateTime` from the cached memos.
|
||||
|
||||
The `MonthCalendar` / `YearCalendar` components themselves require no changes — they receive an opaque `Record<date, count>` and render it. The change is confined to the data-source layer.
|
||||
|
||||
### Tooltip / labeling
|
||||
|
||||
A small but necessary clarification for the user: the cell tooltip should reflect which basis is active, e.g.
|
||||
|
||||
- `create_time` mode: "3 memos on May 2"
|
||||
- `update_time` mode: "3 memos updated on May 2"
|
||||
|
||||
This belongs in `ActivityCalendar/utils.ts:getTooltipText`, which already takes a `t` translator. Add a `timeBasis` argument and pick the right i18n key.
|
||||
|
||||
## Components
|
||||
|
||||
| Unit | Responsibility | Depends on |
|
||||
|---|---|---|
|
||||
| `UserStats` proto | Carry both timestamp arrays | — |
|
||||
| `GetUserStats` server impl | Populate both arrays from `memo` table | store |
|
||||
| `useFilteredMemoStats` | Pick the correct array based on `timeBasis`; aggregate by day | `useView`, `useUserStats`, `useMemos` |
|
||||
| `getTooltipText` | Render basis-aware tooltip | i18n |
|
||||
| `MonthCalendar` / `YearCalendar` | Unchanged — render `Record<date, count>` | — |
|
||||
|
||||
## Data flow
|
||||
|
||||
```
|
||||
ViewContext.timeBasis ──┐
|
||||
▼
|
||||
useFilteredMemoStats ── pick array ── countBy(day) ── Record<date,count> ── MonthCalendar
|
||||
▲
|
||||
userStats ───────────┤ (memo_created_timestamps OR memo_updated_timestamps)
|
||||
│
|
||||
memos cache ─────────┘ (createTime OR updateTime — explore context only)
|
||||
```
|
||||
|
||||
## Error handling
|
||||
|
||||
No new failure modes.
|
||||
|
||||
`protobuf-es` generates `repeated` fields as non-optional `T[]`, so an older server that doesn't populate the new field deserializes it as `[]` (never `undefined`). Naïvely treating empty as "no data" would be wrong, because a user with zero memos also gets `[]`. Detection uses **length divergence**: since `memo.updated_ts` is initialized to `created_ts` at row creation, the two arrays are the same length whenever there are any memos. So:
|
||||
|
||||
- `created.length === 0 && updated.length === 0` — user has no memos, render empty.
|
||||
- `created.length > 0 && updated.length === created.length` — new server, normal path.
|
||||
- `created.length > 0 && updated.length === 0` — old server, fall back to `memoCreatedTimestamps` regardless of `timeBasis`, with a one-line `console.warn`.
|
||||
|
||||
## Testing
|
||||
|
||||
- Unit test `useFilteredMemoStats`: given a fixed `userStats`, switching `timeBasis` returns aggregations matching the expected source array.
|
||||
- Unit test the new `getTooltipText` branch.
|
||||
- Manual verification: in dev, toggle the global time basis and confirm:
|
||||
- Heatmap recomputes
|
||||
- A memo edited yesterday but created last week shows up "yesterday" in update mode and "last week" in create mode
|
||||
- Tooltip text reflects the basis
|
||||
|
||||
## Migration / compatibility
|
||||
|
||||
- Proto field is additive (tag 8 is unused; tag 2 is `reserved`).
|
||||
- Old clients ignore the new field.
|
||||
- New clients tolerate old servers via the fallback above.
|
||||
- No DB migration.
|
||||
- No data backfill — `updated_ts` already exists on every memo row.
|
||||
|
||||
## Out-of-scope follow-ups (not part of this work)
|
||||
|
||||
These came up during brainstorming and are tracked here only so they aren't lost:
|
||||
|
||||
1. Adding comment / reaction event types to the heatmap count.
|
||||
2. A "Memo History / Versions" feature (per-edit snapshots, diffs, optional commit messages). If pursued, the heatmap would become a downstream consumer of that history, and the field added here may be revisited.
|
||||
File diff suppressed because one or more lines are too long
@ -0,0 +1,26 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { getTooltipText } from "@/components/ActivityCalendar/utils";
|
||||
|
||||
// Minimal stub for the i18n translate fn — returns a deterministic string we can assert on.
|
||||
const t = ((key: string, vars?: Record<string, unknown>) => {
|
||||
if (!vars) return key;
|
||||
const parts = Object.entries(vars).map(([k, v]) => `${k}=${String(v)}`);
|
||||
return `${key}|${parts.join(",")}`;
|
||||
}) as Parameters<typeof getTooltipText>[2];
|
||||
|
||||
describe("getTooltipText", () => {
|
||||
it("returns just the date when count is 0", () => {
|
||||
expect(getTooltipText(0, "2026-05-02", t)).toBe("2026-05-02");
|
||||
});
|
||||
|
||||
it("uses the created-tooltip key for create_time basis (default)", () => {
|
||||
const out = getTooltipText(3, "2026-05-02", t);
|
||||
expect(out.toLowerCase()).toContain("memo.count-memos-in-date");
|
||||
expect(out.toLowerCase()).not.toContain("updated");
|
||||
});
|
||||
|
||||
it("uses the updated-tooltip key for update_time basis", () => {
|
||||
const out = getTooltipText(3, "2026-05-02", t, "update_time");
|
||||
expect(out.toLowerCase()).toContain("memo.count-memos-updated-in-date");
|
||||
});
|
||||
});
|
||||
@ -0,0 +1,104 @@
|
||||
import { renderHook } from "@testing-library/react";
|
||||
import type { ReactNode } from "react";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// Mock dependencies BEFORE importing the hook under test.
|
||||
vi.mock("@/hooks/useUserQueries", () => ({
|
||||
useUserStats: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/hooks/useMemoQueries", () => ({
|
||||
useMemos: () => ({ data: undefined, isLoading: false }),
|
||||
}));
|
||||
vi.mock("@/hooks/useCurrentUser", () => ({
|
||||
default: () => ({ name: "users/test", id: 1 }),
|
||||
}));
|
||||
|
||||
const mockUseView = vi.fn();
|
||||
vi.mock("@/contexts/ViewContext", async () => {
|
||||
const actual = await vi.importActual<typeof import("@/contexts/ViewContext")>("@/contexts/ViewContext");
|
||||
return {
|
||||
...actual,
|
||||
useView: () => mockUseView(),
|
||||
};
|
||||
});
|
||||
|
||||
import { useUserStats } from "@/hooks/useUserQueries";
|
||||
import { useFilteredMemoStats } from "@/hooks/useFilteredMemoStats";
|
||||
|
||||
const wrapper = ({ children }: { children: ReactNode }) => children as never;
|
||||
|
||||
const ts = (year: number, month: number, day: number) => ({
|
||||
seconds: BigInt(Math.floor(Date.UTC(year, month - 1, day) / 1000)),
|
||||
nanos: 0,
|
||||
});
|
||||
|
||||
describe("useFilteredMemoStats", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(useUserStats).mockReturnValue({
|
||||
data: {
|
||||
memoCreatedTimestamps: [ts(2026, 5, 1), ts(2026, 5, 1), ts(2026, 5, 2)],
|
||||
memoUpdatedTimestamps: [ts(2026, 5, 3), ts(2026, 5, 3), ts(2026, 5, 3)],
|
||||
tagCount: {},
|
||||
},
|
||||
isLoading: false,
|
||||
} as ReturnType<typeof useUserStats>);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("aggregates by created timestamps when timeBasis is create_time", () => {
|
||||
mockUseView.mockReturnValue({
|
||||
timeBasis: "create_time",
|
||||
orderByTimeAsc: false,
|
||||
toggleSortOrder: vi.fn(),
|
||||
setTimeBasis: vi.fn(),
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useFilteredMemoStats({ userName: "users/test" }), { wrapper });
|
||||
|
||||
expect(result.current.statistics.activityStats).toEqual({ "2026-05-01": 2, "2026-05-02": 1 });
|
||||
expect(result.current.statistics.timeBasis).toBe("create_time");
|
||||
});
|
||||
|
||||
it("aggregates by updated timestamps when timeBasis is update_time", () => {
|
||||
mockUseView.mockReturnValue({
|
||||
timeBasis: "update_time",
|
||||
orderByTimeAsc: false,
|
||||
toggleSortOrder: vi.fn(),
|
||||
setTimeBasis: vi.fn(),
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useFilteredMemoStats({ userName: "users/test" }), { wrapper });
|
||||
|
||||
expect(result.current.statistics.activityStats).toEqual({ "2026-05-03": 3 });
|
||||
expect(result.current.statistics.timeBasis).toBe("update_time");
|
||||
});
|
||||
|
||||
it("falls back to created timestamps when updated array is empty (old server)", () => {
|
||||
// Old servers that don't know about the new field deserialize it as [].
|
||||
// Length divergence (created non-empty, updated empty) is the signal.
|
||||
vi.mocked(useUserStats).mockReturnValue({
|
||||
data: {
|
||||
memoCreatedTimestamps: [ts(2026, 5, 1)],
|
||||
memoUpdatedTimestamps: [],
|
||||
tagCount: {},
|
||||
},
|
||||
isLoading: false,
|
||||
} as ReturnType<typeof useUserStats>);
|
||||
mockUseView.mockReturnValue({
|
||||
timeBasis: "update_time",
|
||||
orderByTimeAsc: false,
|
||||
toggleSortOrder: vi.fn(),
|
||||
setTimeBasis: vi.fn(),
|
||||
});
|
||||
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const { result } = renderHook(() => useFilteredMemoStats({ userName: "users/test" }), { wrapper });
|
||||
|
||||
expect(result.current.statistics.activityStats).toEqual({ "2026-05-01": 1 });
|
||||
expect(warn).toHaveBeenCalled();
|
||||
warn.mockRestore();
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue