mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 14:16:12 +00:00
fix: bound the activity feed to the window it promises
The revoke dialog tells an owner that "who entered what" stays answerable for N days, and the activity list prints the same number under its heading. The query behind that list had no date filter: on an instance whose retention was shortened it showed rows the sentence said were already gone, and the number was a caption over an unbounded read rather than its limit. It is the filter now, resolved once so the bound and the sentence cannot drift apart. The list also stopped at a hundred rows and said nothing about it, which is the more dangerous of the two silences — an owner who scrolls to the bottom reads the oldest visible line as the beginning, and a busy household reaches the cap in a fraction of the window. The response says which of the two limits it hit, and the view states it only when it did: a caveat that is always on is one people learn to skip. The self-hosting page said an empty list means nobody opened the record. It means nobody did so within the window. Corrected, with the row cap named alongside it.
This commit is contained in:
1 parent
b1667ef692
commit
c7e847a766
14 files changed
+213
-16
No files matched your search
@@ -36916,11 +36916,17 @@ components:
|
||||
minimum: -9007199254740991
|
||||
maximum: 9007199254740991
|
||||
description: How far back this feed can see, in days. Resolved from the instance's audit retention setting (365 by
|
||||
default, operator-configurable), NOT a constant — an instance running a 90-day window sends 90. Show it
|
||||
beside the rows.
|
||||
default, operator-configurable), NOT a constant — an instance running a 90-day window sends 90. The query is
|
||||
bounded to this window, so it is the real limit of the list and not only a caption. Show it beside the rows.
|
||||
truncated:
|
||||
type: boolean
|
||||
description: True when the read reached its row ceiling, so `entries` is the most recent activity inside the window
|
||||
rather than all of it. A client must say so — a list that stops at its own cap in silence lets the oldest
|
||||
row read as the beginning. False means the list is everything inside the window.
|
||||
required:
|
||||
- entries
|
||||
- retentionDays
|
||||
- truncated
|
||||
additionalProperties: false
|
||||
description: The owner's record of what somebody else did in their record, bounded by the instance's audit retention.
|
||||
RecordActivityEntry:
|
||||
|
||||
@@ -104,9 +104,21 @@ Below that, a list of the days somebody else opened it and how many times
|
||||
That list is bounded by the instance's audit retention, and the page says
|
||||
so with the actual number rather than a hardcoded one. The default is 365
|
||||
days (`AUDIT_LOG_RETENTION_DAYS`); an operator who shortens it shortens
|
||||
this view too, and the sentence under the heading changes to match. An
|
||||
empty list means nobody opened the record within that window — not that
|
||||
nobody ever did.
|
||||
this view too, and the sentence under the heading changes to match. The
|
||||
query is bounded to that window, so the sentence is the real limit of the
|
||||
list rather than a caption over an unbounded one.
|
||||
|
||||
Two things the list does not claim, both worth reading before treating it
|
||||
as an answer:
|
||||
|
||||
- **An empty list is not proof that nobody opened the record.** It means
|
||||
nobody did so within the window, and that any earlier row has been
|
||||
purged.
|
||||
- **The list is capped.** It returns at most the hundred most recent rows
|
||||
inside the window; when it hits that ceiling it says so under the
|
||||
heading. A busy household can reach the cap in far less time than the
|
||||
retention window, and the oldest line on screen is then the oldest line
|
||||
shown, not the oldest that exists.
|
||||
|
||||
## What this does not change about the instance
|
||||
|
||||
|
||||
+2
-1
@@ -9627,7 +9627,8 @@
|
||||
"loadError": "Die Aktivität konnte nicht geladen werden.",
|
||||
"opened": "{name} hat deine Akte {count}× geöffnet",
|
||||
"acted": "{name} hat etwas in deiner Akte geändert",
|
||||
"deletedAccount": "Ein gelöschtes Konto"
|
||||
"deletedAccount": "Ein gelöschtes Konto",
|
||||
"truncated": "Diese Liste zeigt die neueste Aktivität, nicht die gesamte."
|
||||
},
|
||||
"delegate": {
|
||||
"addOnlyNote": "Du kannst zu dieser Akte etwas hinzufügen. Einträge ändern oder löschen kann nur {name}."
|
||||
|
||||
+2
-1
@@ -9627,7 +9627,8 @@
|
||||
"loadError": "The activity could not be loaded.",
|
||||
"opened": "{name} opened your record {count}×",
|
||||
"acted": "{name} made a change in your record",
|
||||
"deletedAccount": "A deleted account"
|
||||
"deletedAccount": "A deleted account",
|
||||
"truncated": "This list shows the most recent activity, not all of it."
|
||||
},
|
||||
"delegate": {
|
||||
"addOnlyNote": "You can add to this record. Changing or removing entries is something only {name} can do."
|
||||
|
||||
+2
-1
@@ -9627,7 +9627,8 @@
|
||||
"loadError": "No se pudo cargar la actividad.",
|
||||
"opened": "{name} abrió tu historial {count}×",
|
||||
"acted": "{name} hizo un cambio en tu historial",
|
||||
"deletedAccount": "Una cuenta eliminada"
|
||||
"deletedAccount": "Una cuenta eliminada",
|
||||
"truncated": "Esta lista muestra la actividad más reciente, no toda."
|
||||
},
|
||||
"delegate": {
|
||||
"addOnlyNote": "Puedes añadir cosas a este historial. Modificar o borrar entradas solo lo puede hacer {name}."
|
||||
|
||||
+2
-1
@@ -9627,7 +9627,8 @@
|
||||
"loadError": "L'activité n'a pas pu être chargée.",
|
||||
"opened": "{name} a ouvert votre dossier {count}×",
|
||||
"acted": "{name} a fait une modification dans votre dossier",
|
||||
"deletedAccount": "Un compte supprimé"
|
||||
"deletedAccount": "Un compte supprimé",
|
||||
"truncated": "Cette liste montre l'activité la plus récente, pas la totalité."
|
||||
},
|
||||
"delegate": {
|
||||
"addOnlyNote": "Vous pouvez ajouter des éléments à ce dossier. Seul {name} peut modifier ou supprimer des entrées."
|
||||
|
||||
+2
-1
@@ -9627,7 +9627,8 @@
|
||||
"loadError": "Non è stato possibile caricare l'attività.",
|
||||
"opened": "{name} ha aperto la tua cartella {count}×",
|
||||
"acted": "{name} ha fatto una modifica nella tua cartella",
|
||||
"deletedAccount": "Un account eliminato"
|
||||
"deletedAccount": "Un account eliminato",
|
||||
"truncated": "Questo elenco mostra l'attività più recente, non tutta."
|
||||
},
|
||||
"delegate": {
|
||||
"addOnlyNote": "Puoi aggiungere voci a questa cartella. Solo {name} può modificarle o eliminarle."
|
||||
|
||||
+2
-1
@@ -9627,7 +9627,8 @@
|
||||
"loadError": "Nie udało się wczytać aktywności.",
|
||||
"opened": "{name} otworzył twoją kartę {count}×",
|
||||
"acted": "{name} wprowadził zmianę w twojej karcie",
|
||||
"deletedAccount": "Usunięte konto"
|
||||
"deletedAccount": "Usunięte konto",
|
||||
"truncated": "Ta lista pokazuje najnowszą aktywność, a nie całą."
|
||||
},
|
||||
"delegate": {
|
||||
"addOnlyNote": "Możesz dodawać wpisy do tej karty. Zmieniać je i usuwać może tylko {name}."
|
||||
|
||||
@@ -16,6 +16,20 @@
|
||||
* purge job uses, so the sentence in the UI cannot drift from the job that
|
||||
* makes it true — an instance running a 90-day window says 90.
|
||||
*
|
||||
* **And the window is now enforced, not only stated.** The query is bounded to
|
||||
* it. Without the filter the two halves of the same sentence disagreed: the
|
||||
* revoke dialog promised attribution "for the next N days" while this read
|
||||
* returned whatever the last hundred rows happened to be, which on a quiet
|
||||
* record reaches back past a shortened retention window and on a busy one
|
||||
* covers a fortnight. The bound the copy states is the bound the query uses.
|
||||
*
|
||||
* **The row cap says when it was hit.** A hundred rows is a display bound, and
|
||||
* a list that silently stops at its own ceiling is the more dangerous of the
|
||||
* two completeness lies — an owner scrolling to the bottom reads the oldest
|
||||
* line as the beginning. `truncated` is true when the cap was reached, and the
|
||||
* view says it is showing the most recent activity rather than all of it.
|
||||
* `false` means the list IS everything inside the window.
|
||||
*
|
||||
* Not delegable: bare `requireAuth()` refuses under a switch. A delegate must
|
||||
* not be able to read the owner's record of who has been in it — that list
|
||||
* names other delegates, and a grant is not an introduction to the household.
|
||||
@@ -34,6 +48,9 @@ export const dynamic = "force-dynamic";
|
||||
* How many rows the feed returns. One row per (delegate, day) for reads, so a
|
||||
* hundred covers months of a busy household — and a longer list answers a
|
||||
* question nobody asked while making the recent activity harder to see.
|
||||
*
|
||||
* The cap is a display bound, not a completeness claim, and the response says
|
||||
* which it hit. See {@link GET}.
|
||||
*/
|
||||
const MAX_ROWS = 100;
|
||||
|
||||
@@ -77,6 +94,11 @@ function readAccessCount(
|
||||
export const GET = apiHandler(async () => {
|
||||
const { user } = await requireAuth();
|
||||
|
||||
// The same number the response publishes and the purge job reads, resolved
|
||||
// once so the filter and the sentence cannot disagree.
|
||||
const retentionDays = getAuditLogRetentionDays();
|
||||
const windowStart = new Date(Date.now() - retentionDays * 86_400_000);
|
||||
|
||||
const rows = await prisma.auditLog.findMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
@@ -85,6 +107,10 @@ export const GET = apiHandler(async () => {
|
||||
// report. `actingActorFor` already refuses to stamp one, so this is the
|
||||
// second end of that rule rather than its only enforcement.
|
||||
NOT: { actorUserId: user.id },
|
||||
// The window the copy promises. The purge job makes it true eventually;
|
||||
// this makes it true now, which matters on an instance whose retention
|
||||
// was shortened after rows were already written.
|
||||
createdAt: { gte: windowStart },
|
||||
},
|
||||
orderBy: { createdAt: "desc" },
|
||||
take: MAX_ROWS,
|
||||
@@ -114,15 +140,23 @@ export const GET = apiHandler(async () => {
|
||||
createdAt: row.createdAt.toISOString(),
|
||||
}));
|
||||
|
||||
// Equality, not `>=`: `take` cannot return more than it was given, and a
|
||||
// `>` would be a comparison that can never be true.
|
||||
const truncated = rows.length === MAX_ROWS;
|
||||
|
||||
annotate({
|
||||
action: { name: "sharing.activity.list" },
|
||||
meta: { entry_count: entries.length },
|
||||
meta: { entry_count: entries.length, truncated },
|
||||
});
|
||||
|
||||
return apiSuccess({
|
||||
entries,
|
||||
// The bound the view states out loud. Resolved here so the UI never has to
|
||||
// hardcode a number the operator can change.
|
||||
retentionDays: getAuditLogRetentionDays(),
|
||||
retentionDays,
|
||||
// Whether the list is everything inside that window, or its most recent
|
||||
// hundred rows. The view says which; silence would let the oldest line
|
||||
// read as the beginning.
|
||||
truncated,
|
||||
});
|
||||
});
|
||||
@@ -205,14 +205,32 @@ describe("when my record was opened", () => {
|
||||
// see would imply a completeness it does not have: somebody checking last
|
||||
// spring would read an empty list as "no" when the honest answer is
|
||||
// "further back than this reaches".
|
||||
activityRef.value = { entries: [], retentionDays: 90 };
|
||||
activityRef.value = { entries: [], retentionDays: 90, truncated: false };
|
||||
const html = render(<RecordActivityCard />);
|
||||
expect(html).toContain('data-slot="record-activity-retention"');
|
||||
expect(html).toContain("last 90 days");
|
||||
});
|
||||
|
||||
it("says when the list is the most recent rows rather than all of them", () => {
|
||||
// The second completeness claim. The window bounds how far back; the row
|
||||
// cap bounds how many, and a list that stopped at its ceiling in silence
|
||||
// let the oldest visible line read as the beginning.
|
||||
activityRef.value = { entries: [], retentionDays: 365, truncated: true };
|
||||
const capped = render(<RecordActivityCard />);
|
||||
expect(capped).toContain('data-slot="record-activity-truncated"');
|
||||
expect(capped).toContain("most recent activity");
|
||||
|
||||
// And it is not an unconditional hedge: a caveat that is always on is one
|
||||
// people learn to skip, which would cost the sentence its meaning on the
|
||||
// one screen where it is true.
|
||||
activityRef.value = { entries: [], retentionDays: 365, truncated: false };
|
||||
const whole = render(<RecordActivityCard />);
|
||||
expect(whole).not.toContain('data-slot="record-activity-truncated"');
|
||||
expect(whole).toContain('data-slot="record-activity-retention"');
|
||||
});
|
||||
|
||||
it("prints the operator's window, never a compiled-in 365", () => {
|
||||
activityRef.value = { entries: [], retentionDays: 30 };
|
||||
activityRef.value = { entries: [], retentionDays: 30, truncated: false };
|
||||
const html = render(<RecordActivityCard />);
|
||||
expect(html).toContain("last 30 days");
|
||||
expect(html).not.toContain("365");
|
||||
@@ -229,6 +247,7 @@ describe("when my record was opened", () => {
|
||||
it("counts the openings rather than listing them one by one", () => {
|
||||
activityRef.value = {
|
||||
retentionDays: 365,
|
||||
truncated: false,
|
||||
entries: [
|
||||
{
|
||||
id: "a1",
|
||||
@@ -251,6 +270,7 @@ describe("when my record was opened", () => {
|
||||
// it themselves.
|
||||
activityRef.value = {
|
||||
retentionDays: 365,
|
||||
truncated: false,
|
||||
entries: [
|
||||
{
|
||||
id: "a2",
|
||||
|
||||
@@ -62,6 +62,20 @@ export function RecordActivityCard() {
|
||||
{t("recordSharing.activity.retention", {
|
||||
days: data.retentionDays,
|
||||
})}
|
||||
{/* The second completeness claim. The window says how far back
|
||||
this can see; the cap says how many rows came back, and a list
|
||||
that stopped at its ceiling without saying so lets the oldest
|
||||
line read as the beginning. Rendered only when the ceiling was
|
||||
actually hit — an unconditional hedge would teach people to
|
||||
ignore it. */}
|
||||
{data.truncated && (
|
||||
<>
|
||||
{" "}
|
||||
<span data-slot="record-activity-truncated">
|
||||
{t("recordSharing.activity.truncated")}
|
||||
</span>
|
||||
</>
|
||||
)}
|
||||
</p>
|
||||
)}
|
||||
|
||||
|
||||
@@ -210,7 +210,12 @@ const recordActivityResponse = z
|
||||
.number()
|
||||
.int()
|
||||
.describe(
|
||||
"How far back this feed can see, in days. Resolved from the instance's audit retention setting (365 by default, operator-configurable), NOT a constant — an instance running a 90-day window sends 90. Show it beside the rows.",
|
||||
"How far back this feed can see, in days. Resolved from the instance's audit retention setting (365 by default, operator-configurable), NOT a constant — an instance running a 90-day window sends 90. The query is bounded to this window, so it is the real limit of the list and not only a caption. Show it beside the rows.",
|
||||
),
|
||||
truncated: z
|
||||
.boolean()
|
||||
.describe(
|
||||
"True when the read reached its row ceiling, so `entries` is the most recent activity inside the window rather than all of it. A client must say so — a list that stops at its own cap in silence lets the oldest row read as the beginning. False means the list is everything inside the window.",
|
||||
),
|
||||
})
|
||||
.meta({
|
||||
|
||||
@@ -93,6 +93,15 @@ export interface RecordActivity {
|
||||
* the list is everything that ever happened.
|
||||
*/
|
||||
retentionDays: number;
|
||||
/**
|
||||
* True when the read hit its row ceiling, so the list is the most recent
|
||||
* activity inside the window rather than all of it.
|
||||
*
|
||||
* Two different completeness claims, and both have to be stated: the window
|
||||
* bounds how far back, this bounds how many. A list that silently stopped at
|
||||
* its own cap would let the oldest line read as the beginning.
|
||||
*/
|
||||
truncated: boolean;
|
||||
}
|
||||
|
||||
export function useAccountGrants(enabled = true) {
|
||||
|
||||
@@ -404,3 +404,94 @@ describe("a refused read is reported as a refused read", () => {
|
||||
expect(sentence).toBe("Alex sent a mood search this record could not read");
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* What the owner's activity feed claims, and what it can actually see.
|
||||
*
|
||||
* Two completeness claims live on this response and the surface states both.
|
||||
* They were both wrong in the same direction — too confident — and in opposite
|
||||
* ways:
|
||||
*
|
||||
* * the copy promised a window ("who entered what stays answerable for N
|
||||
* days") while the query had no date filter at all, so on an instance
|
||||
* whose retention was shortened the view showed rows the sentence said
|
||||
* were gone;
|
||||
* * the query stopped at a hundred rows and said nothing, so an owner who
|
||||
* scrolled to the bottom read the oldest visible line as the beginning.
|
||||
*
|
||||
* Driven against real rows rather than a mocked Prisma, because both claims
|
||||
* are about what the DATABASE returns for a given `where` — a stubbed client
|
||||
* would answer whatever the stub was told and prove neither.
|
||||
*/
|
||||
describe("the activity feed states its own limits", () => {
|
||||
async function feed() {
|
||||
const { GET } = await import("@/app/api/account/activity/route");
|
||||
const response = await GET();
|
||||
expect(response.status).toBe(200);
|
||||
return (await response.json()) as {
|
||||
data: {
|
||||
entries: { action: string }[];
|
||||
retentionDays: number;
|
||||
truncated: boolean;
|
||||
};
|
||||
};
|
||||
}
|
||||
|
||||
/** One delegated row, back-dated. */
|
||||
async function stampedRow(
|
||||
ownerId: string,
|
||||
delegateId: string,
|
||||
createdAt: Date,
|
||||
action = "measurement.update",
|
||||
) {
|
||||
await getPrismaClient().auditLog.create({
|
||||
data: { userId: ownerId, actorUserId: delegateId, action, createdAt },
|
||||
});
|
||||
}
|
||||
|
||||
it("does not show a row older than the window it promises", async () => {
|
||||
const { owner, delegate } = await household();
|
||||
await signIn(owner.id);
|
||||
|
||||
const days = (await feed()).data.retentionDays;
|
||||
expect(days).toBeGreaterThan(0);
|
||||
|
||||
const inside = new Date(Date.now() - 60_000);
|
||||
const outside = new Date(Date.now() - (days + 2) * 86_400_000);
|
||||
await stampedRow(owner.id, delegate.id, inside, "measurement.update");
|
||||
await stampedRow(owner.id, delegate.id, outside, "measurement.delete");
|
||||
|
||||
const { data } = await feed();
|
||||
const actions = data.entries.map((e) => e.action);
|
||||
// The positive control matters more than the absence: a filter that
|
||||
// excluded everything would satisfy the second assertion alone.
|
||||
expect(actions).toContain("measurement.update");
|
||||
expect(actions).not.toContain("measurement.delete");
|
||||
});
|
||||
|
||||
it("says when the list is the most recent rows rather than all of them", async () => {
|
||||
const { owner, delegate } = await household();
|
||||
await signIn(owner.id);
|
||||
|
||||
// Under the ceiling: the list IS everything, and says so.
|
||||
for (let i = 0; i < 3; i += 1) {
|
||||
await stampedRow(owner.id, delegate.id, new Date(Date.now() - i * 1000));
|
||||
}
|
||||
const small = await feed();
|
||||
expect(small.data.entries).toHaveLength(3);
|
||||
expect(small.data.truncated).toBe(false);
|
||||
|
||||
// Over it: same shape, different claim.
|
||||
await getPrismaClient().auditLog.createMany({
|
||||
data: Array.from({ length: 120 }, (_, i) => ({
|
||||
userId: owner.id,
|
||||
actorUserId: delegate.id,
|
||||
action: "measurement.update",
|
||||
createdAt: new Date(Date.now() - (i + 10) * 1000),
|
||||
})),
|
||||
});
|
||||
const large = await feed();
|
||||
expect(large.data.entries).toHaveLength(100);
|
||||
expect(large.data.truncated).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user