Here is a file in our codebase, in full:
/**
* Current policy versions.
*
* Bump these strings whenever you publish a new version of either policy.
* Any existing user whose stored version doesn't match will be shown the
* PolicyUpdateBanner the next time they visit the dashboard.
*/
export const CURRENT_TOS_VERSION = '1.12.0';
export const CURRENT_PRIVACY_VERSION = '1.7.0';
Nine lines. It is one of the higher leverage files we have, and the reason is that consent is not a boolean.
Why a boolean is the wrong shape
tos_accepted: true answers "did this user ever agree to something". That is not the question anybody actually needs answered. The real question is "did this user agree to the terms that currently apply to them", and a boolean cannot express it.
Once you change your terms, every true in that column is describing a document that no longer exists. There is no migration that fixes it, because you genuinely do not know what each user agreed to.
Storing the version turns re-consent into a comparison:
const needsPolicyAcceptance =
!dbUser ||
dbUser.tos_version !== CURRENT_TOS_VERSION ||
dbUser.privacy_policy_version !== CURRENT_PRIVACY_VERSION;
Publishing new terms is now: edit the policy page, bump the string, deploy. Everyone whose stored version no longer matches sees the banner on their next dashboard visit. No backfill, no cron job, no "clear the accepted flag for all users" migration that you run once and then worry about for a week.
Two independent versions, not one, because the two documents change for unrelated reasons. A privacy policy update triggered by a new subprocessor should not force re-acceptance of commercial terms.
!== rather than a semver comparison is deliberate. Any mismatch means re-consent, including a downgrade or a hand-edited value. Semver ordering would introduce a question ("is a patch bump material enough to re-ask?") that a legal document does not want you answering in code.
The actual bug
That comparison is computed in the dashboard layout, which is a server component. The original implementation did not just compute it. It acted on it:
It used to perform the policy auto-acceptance inline: an UPDATE followed by an audit-log INSERT, sequentially, in the render path.
And the reason that is wrong is worth quoting exactly as the comment puts it, because the ordering of the argument is the lesson:
Mutating during render is a correctness problem before it is a performance one. React may render a component more than once, and a layout re-render would re-issue the writes.
That is the ordering I want to press on. The performance cost was real and easy to see: every user's first dashboard load after any policy bump was blocked behind two sequential write round trips, with the entire dashboard subtree waiting.
But the correctness problem is the one that should have stopped it being written. React does not promise to render your component exactly once. It is allowed to render, throw away the result, and render again. In a server component that means the writes fire again, so the "acceptance" recorded is not a user action at all, it is a render count.
And here is the part that makes it genuinely bad rather than merely wasteful: the thing being recorded was consent. An auto-acceptance in a render path means the audit trail says a user accepted terms at a moment when all they did was load a page. That is exactly backwards from what an audit trail is for.
The fix is structural rather than clever. The layout became read only. It computes the flag and renders a banner. The banner, which is a client component responding to an actual click, calls an endpoint that already existed with identical logic:
export const PATCH = withApiHandler(
async ({ user, request }) => {
const now = new Date();
await db.update(usersTable).set({
tos_accepted_at: now,
tos_version: CURRENT_TOS_VERSION,
privacy_policy_accepted_at: now,
privacy_policy_version: CURRENT_PRIVACY_VERSION,
updated_at: now,
}).where(eq(usersTable.id, user!.id));
await logAuditAction({ /* ... */ });
return NextResponse.json({ success: true });
},
{ rateLimit: 'write', errorMessage: 'Failed to record policy acceptance' }
);
Removing the inline version also deleted a duplicate implementation of the same write, which is a good sign that the refactor was in the right direction. Two copies of "record consent" is one copy too many for something you may one day have to defend.
Note that both a timestamp and a version are stored. The version answers "which document", the timestamp answers "when". You need both: the version alone cannot order two acceptances, and the timestamp alone cannot identify what was agreed to.
The log underneath
The write is paired with an audit entry, and the audit logger has one property worth copying:
export async function logAuditAction(params: AuditLogParams): Promise<void> {
try {
await db.insert(auditLogsTable).values({ /* ... */ });
} catch (error) {
// Log error but don't fail the main operation
if (process.env.NODE_ENV === 'development') {
console.error('Failed to log audit action:', error);
}
}
}
It never throws. A failed audit insert must not fail the user's data export, their deletion request, or their policy acceptance. The record of the action is less important than the action, and inverting that means a logging outage becomes a product outage.
Reasonable people disagree here, and in a regulated setting the opposite choice is defensible. For a GDPR Article 30 record of processing activities, keeping the user's rights working is the right call, and the trade is explicit in the code rather than accidental.
The action and resource types are unions, not free strings:
export type AuditAction =
| 'data_export' | 'data_deletion' | 'session_deletion' | 'account_deletion'
| 'consent_change' | 'data_access' | 'preference_change';
Which makes the log queryable and stops a typo creating a category nobody can find. There is a comment on one member that shows the union earning its keep:
Onboarding answers (sector / role area / study level). Their own type rather than 'preferences' so the activity log a user exports distinguishes them from account settings changes.
That is a categorisation decided from the point of view of the person who will one day read their own exported activity log. If your audit taxonomy is designed only for your own debugging, it will read as noise to the user who requests it, and that export is a legal deliverable.
Bump the string, watch the banner
The two documents these versions point at are the terms and the privacy policy. The version strings in that nine line file correspond to what is published there, which is the property that makes the whole scheme work: the number in code and the document a user can read are the same fact.
If you keep a boolean consent flag today, the migration is smaller than it sounds. Add a version column, set every existing row to the version that was live when they accepted, and export two constants. The next time legal asks you to update the terms, the work is one string.
Top comments (0)