Skip to content

fix(activity-logger): handle FK constraint violation for deleted pages - #211

Merged
2witstudios merged 1 commit into
masterfrom
claude/fix-activity-logs-fk-rFi9A
Jan 15, 2026
Merged

2witstudios merged 1 commit into
masterfrom
claude/fix-activity-logs-fk-rFi9A

Conversation

@2witstudios

@2witstudios 2witstudios commented Jan 15, 2026 •

Copy link
Copy Markdown
Owner

The activity_logs table has a foreign key constraint on pageId referencing the pages table. This was causing errors when:

  1. A page was permanently deleted and activity was logged for the delete operation (the page no longer exists when the log is inserted)

  2. Async fire-and-forget activity logging raced with page deletion (the page existed when logging was scheduled but was deleted before the async operation completed)

Changes:

  • logPageActivity: Skip setting pageId for 'delete' operations since the page no longer exists. Audit info is preserved via resourceId/resourceTitle.

  • logActivity: Catch FK constraint violation (code 23503) on pageId and retry the insert without pageId. This handles race conditions gracefully where a page is deleted during async logging.

Fixes foreign key constraint error:
"insert or update on table activity_logs violates foreign key constraint activity_logs_pageId_pages_id_fk"

Summary by CodeRabbit

Bug Fixes

  • Activity logs are now reliably captured and preserved even when associated pages are deleted from the system, maintaining complete and accurate audit trails
  • Enhanced error handling in activity logging gracefully manages database constraint violations, ensuring activity records are never lost during edge cases

✏️ Tip: You can customize this high-level summary in your review settings.

The activity_logs table has a foreign key constraint on pageId referencing
the pages table. This was causing errors when:

1. A page was permanently deleted and activity was logged for the delete
   operation (the page no longer exists when the log is inserted)

2. Async fire-and-forget activity logging raced with page deletion
   (the page existed when logging was scheduled but was deleted before
   the async operation completed)

Changes:
- logPageActivity: Skip setting pageId for 'delete' operations since the
  page no longer exists. Audit info is preserved via resourceId/resourceTitle.

- logActivity: Catch FK constraint violation (code 23503) on pageId and
  retry the insert without pageId. This handles race conditions gracefully
  where a page is deleted during async logging.

Fixes foreign key constraint error:
"insert or update on table activity_logs violates foreign key constraint
activity_logs_pageId_pages_id_fk"
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jan 15, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Modified activity logger to handle foreign key constraint violations when pageId references deleted pages. Added retry logic that reinserts activity logs without pageId on constraint failure. For delete operations, pageId is intentionally omitted since the referenced page no longer exists.

Changes

Cohort / File(s) Summary
Foreign Key Violation Handling & Delete Operation Support
packages/lib/src/monitoring/activity-logger.ts
Created internal insertActivityLog(pageId) helper and wrapped invocation in try/catch. On foreign key violation (23503), retries insertion with undefined pageId to log activity without page reference. For delete operations, logPageActivity now passes undefined pageId to preserve audit trail when page no longer exists.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit's ode to graceful deletion:
When pages vanish without a trace,
Our logs still capture their fond embrace,
A foreign key that learned to yield,
Delete operations now safely sealed,
History preserved, constraints appeased! 🌿

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main change: handling a foreign key constraint violation in the activity logger for deleted pages.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
packages/lib/src/monitoring/activity-logger.ts (1)

551-604: Consider documenting the FK handling difference for transactional logging.

Unlike logActivity, logActivityWithTx does not retry on FK constraint violations. This is correct for atomic operations, but callers logging page deletions transactionally should ensure the activity log insert precedes the page deletion within the same transaction, or omit pageId for delete operations similar to logPageActivity.

A brief doc comment noting this distinction could help future maintainers.

📝 Suggested documentation enhancement
 /**
  * Log an activity event using an existing transaction.
  * Intended for deterministic, atomic writes.
  * Computes hash chain data within the transaction for consistency.
  * Note: Broadcast happens after insert but within the transaction scope.
  * The broadcast is debounced, so it will fire after the transaction commits.
+ *
+ * Note: Unlike logActivity(), this function does not retry on FK constraint
+ * violations. For page delete operations, ensure pageId is omitted or the
+ * activity is logged before the page is deleted within the transaction.
  */
 export async function logActivityWithTx(

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d82f1a7 and d4e5a26.

📒 Files selected for processing (1)
  • packages/lib/src/monitoring/activity-logger.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Never use any types - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase with use prefix), Zustand stores (camelCase with use prefix), and React components (PascalCase)
Lint with Next/ESLint as configured in apps/web/eslint.config.mjs
Message content should always use the message parts structure with { parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from @pagespace/lib/permissions (e.g., getUserAccessLevel, canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from @pagespace/db package for database access
Use ESM modules throughout the codebase

**/*.{ts,tsx}: Never use any types - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting

Files:

  • packages/lib/src/monitoring/activity-logger.ts
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.ts: React hook files should use camelCase matching the exported hook name (e.g., useAuth.ts)
Zustand store files should use camelCase with use prefix (e.g., useAuthStore.ts)

Files:

  • packages/lib/src/monitoring/activity-logger.ts
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier

Files:

  • packages/lib/src/monitoring/activity-logger.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Lint & TypeScript Check
  • GitHub Check: Unit Tests
🔇 Additional comments (3)
packages/lib/src/monitoring/activity-logger.ts (3)

465-515: LGTM! Clean encapsulation of insert logic for retry support.

The helper function correctly:

  • Accepts pageId as a parameter to allow override on retry
  • Recomputes hash chain data fresh on each attempt (appropriate since retry is effectively a new log entry)
  • Handles broadcast within the same scope

Note that each call to insertActivityLog generates a new id and timestamp via prepareActivityInsert, which is the correct behavior for retry scenarios.


517-541: Solid FK constraint error handling with appropriate retry logic.

The implementation correctly:

  • Identifies the specific constraint violation (activity_logs_pageId_pages_id_fk)
  • Only retries when a pageId was actually provided
  • Maintains the fire-and-forget pattern by catching and logging retry failures
  • Returns early after successful retry to avoid duplicate error logging

The type narrowing via casting (error as { code: string }) is pragmatic for Postgres error objects. If stronger typing is desired in the future, consider creating a type guard for Postgres database errors.


641-654: LGTM! Proactive handling for delete operations.

Correctly distinguishes between delete (permanent, page is gone) and other operations like trash (page still exists). The audit trail remains complete via resourceId and resourceTitle, while the FK reference is appropriately omitted for delete operations.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants