Repository navigation
refactor(account): detach pending daily counter table - #1134
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # packages/worker/src/email/inbound.ts # packages/worker/src/email/outbound.ts # packages/worker/worker-configuration.d.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md # packages/worker/src/account/export.node.test.ts # packages/worker/src/account/export.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThe change retires ChangesPending-drop table retirement
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
🔎 Preview deployed: https://kody-pr-1134.kody-a99.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/contributing/architecture/entitlements.md (1)
365-375: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord the exact stage-3 exit actions.
The sequence currently says only “third, migration-only deploy drops the table.” Add migration-only
0126, removal of the pending-drop registry and retention exemption, and the post-deploy assertiondaily.mirrorRetired === true. These steps make the retirement runbook executable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/contributing/architecture/entitlements.md` around lines 365 - 375, Update the “Daily-counter mirror retirement (three-deploy)” sequence to explicitly document stage 3 as migration-only migration 0126, including removal of the pending-drop registry and retention exemption. Add the post-deploy verification that daily.mirrorRetired === true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/contributing/architecture/data-storage.md`:
- Around line 77-80: Update the documentation reference to point to
packages/worker/src/account/data-targets.node.test.ts, and revise the
description to identify it as the schema coverage guardrail rather than a
runtime deletion test.
---
Nitpick comments:
In `@docs/contributing/architecture/entitlements.md`:
- Around line 365-375: Update the “Daily-counter mirror retirement
(three-deploy)” sequence to explicitly document stage 3 as migration-only
migration 0126, including removal of the pending-drop registry and retention
exemption. Add the post-deploy verification that daily.mirrorRetired === true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 41db76e4-7856-49cf-aa1b-2ffe99257a54
📒 Files selected for processing (7)
docs/contributing/architecture/data-storage.mddocs/contributing/architecture/entitlements.mdpackages/worker/src/account/data-targets.node.test.tspackages/worker/src/account/data-targets.tspackages/worker/src/account/export.node.test.tspackages/worker/src/app/account-retention-dispositions.node.test.tspackages/worker/src/app/account-retention-dispositions.ts
| test in `packages/worker/src/app/account-deletion.node.test.ts` applies the | ||
| live migrations to SQLite and fails if a user-owned schema column is not | ||
| represented in the deletion target list or pending-drop registry, or if those | ||
| lists reference a stale column. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f -e ts 'account-deletion|data-targets' packages/worker
rg -n -C 4 'accountUserDataPendingDropTargets|entitlement_daily_counters|schema coverage' packages/workerRepository: kentcdodds/kody
Length of output: 34428
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs excerpt =="
sed -n '60,90p' docs/contributing/architecture/data-storage.md || true
echo
echo "== account deletion test outline/searches =="
wc -l packages/worker/src/app/account-deletion.node.test.ts packages/worker/src/account/data-targets.node.test.ts
rg -n 'pending-drop|pending drop|schema coverage|column.*represented|stale column|entitlement_daily_counters|accountUserDataPendingDropTargets|delete.*migration|migration.*deletion' packages/worker/src/app/account-deletion.node.test.ts packages/worker/src/account/data-targets.node.test.ts docs/contributing/architecture/data-storage.mdRepository: kentcdodds/kody
Length of output: 5274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== account deletion test relevant search =="
rg -n -C 5 "entitlement_daily_counters|pendingDrop|pending-drop|pending drop|accountUserDataPendingDropTargets|schema coverage|coveredColumns|live migrations|apply.*migration|migration.*live|live migrations" packages/worker/src/app/account-deletion.node.test.ts packages/worker/src/account/data-targets.node.test.ts packages/worker/src/account/data-targets.ts || true
echo
echo "== account deletion imports/subjects relevant to data targets =="
sed -n '1,220p' packages/worker/src/app/account-deletion.node.test.tsRepository: kentcdodds/kody
Length of output: 21668
Point the schema coverage guardrail at the right test.
Update this reference from packages/worker/src/app/account-deletion.node.test.ts to packages/worker/src/account/data-targets.node.test.ts, and mention schema coverage rather than runtime deletion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/contributing/architecture/data-storage.md` around lines 77 - 80, Update
the documentation reference to point to
packages/worker/src/account/data-targets.node.test.ts, and revise the
description to identify it as the schema coverage guardrail rather than a
runtime deletion test.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Summary
entitlement_daily_countersfrom runtime export/deletion SQLValidation
System recap — contracts account inventory wiring (low risk)
Mode: recap · Base:
main@a838a167· Head:fe1766baRuntime account operations no longer query the pending-drop table; schema coverage remains explicit.
Conductor report
Summary by CodeRabbit
Documentation
Bug Fixes
Tests