Skip to content

Comments

feat: Add comprehensive booking audit logging with Actor model#24656

Closed
hariombalhara wants to merge 18 commits intographite-base/24656from
devin/booking-audit-integration-1761111651
Closed

feat: Add comprehensive booking audit logging with Actor model#24656
hariombalhara wants to merge 18 commits intographite-base/24656from
devin/booking-audit-integration-1761111651

Conversation

@hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Oct 23, 2025

What does this PR do?

This PR centralizes booking audit logging through BookingEventHandlerService and introduces a new Actor model to track who performs booking actions. The changes ensure that all audit logging flows through a single service rather than having direct BookingAuditService calls scattered throughout the codebase.

Key Changes:

  1. Actor Model Introduction: Added a new Actor table to store actor information (USER, GUEST, or SYSTEM) separately from the User model. This enables audit retention even after user deletion.

  2. BookingAudit Refactoring: Modified BookingAudit to use actorId (references Actor) instead of userId for better audit trail preservation.

  3. Centralized Audit Logging:

    • Added audit logging wrapper methods to BookingEventHandlerService:
      • onBookingCreatedAudit
      • onBookingAcceptedAudit
      • onBookingCancelledAudit
      • onBookingUpdatedAudit
      • onRescheduleRequestedAudit
      • onAttendeeAddedAudit
      • onHostNoShowUpdatedAudit
    • Refactored all booking handlers to use BookingEventHandlerService instead of calling BookingAuditService directly
  4. Files Updated:

    • RegularBookingService.ts - booking creation
    • handleConfirmation.ts - booking confirmation
    • handleCancelBooking.ts - booking cancellation
    • roundRobinReassignment.ts - round robin reassignment
    • roundRobinManualReassignment.ts - manual reassignment
    • handleMarkNoShow.ts - no-show marking
    • addGuests.handler.ts - guest addition
    • requestReschedule.handler.ts - reschedule requests

Fixes #24656


Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox. N/A - Internal refactoring with no API changes
  • I confirm automated tests are in place that prove my fix is effective or that my feature works. ⚠️ No automated tests added - this is a refactoring PR that maintains existing behavior

How should this be tested?

Prerequisites:

  • Local development environment set up with PostgreSQL database
  • Run yarn prisma generate to update Prisma client after pulling this branch

Testing Steps:

  1. Database Migration:

    yarn workspace @calcom/prisma db-migrate
    • Verify the Actor table is created
    • Confirm the SYSTEM actor (id: 00000000-0000-0000-0000-000000000000) exists
  2. Booking Creation:

    • Create a new booking via the booking page
    • Check that a BookingAudit record is created with type=RECORD_CREATED and action=BOOKING_CREATED
    • Verify the actorId points to a valid Actor record
  3. Booking Confirmation:

    • Create a booking that requires confirmation
    • Confirm the booking
    • Verify audit log with type=RECORD_UPDATED and action=BOOKING_CONFIRMED
  4. Booking Cancellation:

    • Cancel an existing booking
    • Check audit log with type=RECORD_UPDATED and action=BOOKING_CANCELLED
    • Verify cancellation reason is captured in the audit data
  5. Round Robin Reassignment:

    • Trigger round robin reassignment
    • Verify audit log captures the reassignment with old/new user information
  6. Add Guests:

    • Add guests to an existing booking
    • Verify audit log shows attendee count changes
  7. Request Reschedule:

    • Request a reschedule on a booking
    • Check audit log with action=RESCHEDULE_REQUESTED

Expected Results:

  • All booking operations should complete successfully
  • Audit logs should be created for all operations
  • If audit logging fails, the booking operation should still succeed (errors are logged but not thrown)
  • Check database for BookingAudit and Actor records after each operation

Human Review Checklist

Critical Items to Review:

  • Database Migration Safety: Review the migration file to ensure it properly handles existing BookingAudit records and creates the SYSTEM actor
  • Actor Creation Logic: Verify that the actor lookup/creation logic in BookingAuditService doesn't introduce N+1 queries or performance issues
  • Error Handling: Confirm that audit logging failures don't break the booking flow (all audit calls are wrapped in try-catch)
  • Completeness: Verify that all places that previously called BookingAuditService directly now go through BookingEventHandlerService
  • SYSTEM Actor: Confirm the SYSTEM actor ID (00000000-0000-0000-0000-000000000000) is properly used for automated actions

Link to Devin run: https://app.devin.ai/sessions/76687fea19ab4681968b00150234a706
Requested by: hariom@cal.com (@hariombalhara)

@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions github-actions bot added the ❗️ migrations contains migration files label Oct 23, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "Add comprehensive booking audit logging with Actor model". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Oct 23, 2025
@vercel
Copy link

vercel bot commented Oct 23, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Nov 13, 2025 8:35am
cal-eu Ignored Ignored Nov 13, 2025 8:35am

@devin-ai-integration devin-ai-integration bot changed the title Add comprehensive booking audit logging with Actor model feat: Add comprehensive booking audit logging with Actor model Oct 23, 2025
@hariombalhara hariombalhara force-pushed the devin/booking-audit-integration-1761111651 branch from b0dd96c to 6e28f7d Compare October 23, 2025 12:57
@github-actions
Copy link
Contributor

This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 issues found across 14 files

Prompt for AI agents (all 7 issues)

Understand the root cause of the following 7 issues and fix them.


<file name="packages/prisma/migrations/20251022075606_add_actor_model_with_user_type/migration.sql">

<violation number="1" location="packages/prisma/migrations/20251022075606_add_actor_model_with_user_type/migration.sql:12">
This ALTER TABLE both drops existing booking audit user references and adds a NOT NULL actorId without backfilling, so the migration will fail on populated tables and would erase historical audit linkage even if it ran.</violation>
</file>

<file name="packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts">

<violation number="1" location="packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts:175">
bookingToReschedule.status is never selected from Prisma, so this audit entry logs an undefined oldValue instead of the previous booking status. Please select the status field (or otherwise fetch it) before using it here.</violation>
</file>

<file name="packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts">

<violation number="1" location="packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:86">
The new `uniqueGuests` filter no longer normalizes emails, so duplicates and blacklisted addresses in different casing are accepted. Restore the base-email normalization and deduplication before filtering.</violation>

<violation number="2" location="packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:208">
`sendAddGuestsEmails` should receive only the guests that were actually added; passing the original list will email users who were filtered out.</violation>
</file>

<file name="packages/lib/server/service/bookingAuditService.ts">

<violation number="1" location="packages/lib/server/service/bookingAuditService.ts:97">
Please add a select clause to this upsert so we only fetch actor.id; returning the whole Actor row pulls unused email/phone data and violates our Prisma query guidance.</violation>
</file>

<file name="packages/features/bookings/lib/handleCancelBooking.ts">

<violation number="1" location="packages/features/bookings/lib/handleCancelBooking.ts:485">
The actor passed to booking audit logging ignores the new `actor` input and always fabricates a user actor with id 0 when `userId` is absent, producing incorrect audit metadata. Please use the supplied actor (or fall back to a real system actor) instead of hard-coding `createUserActor(userId || 0)`.</violation>
</file>

<file name="packages/features/bookings/lib/types/actor.ts">

<violation number="1" location="packages/features/bookings/lib/types/actor.ts:82">
Rule violated: **Avoid Logging Sensitive Information**

Returning the attendee email in the audit log string exposes PII in logs, violating the Avoid Logging Sensitive Information rule. Remove the email from the logged string.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

CREATE TYPE "ActorType" AS ENUM ('user', 'guest', 'system');

-- AlterTable
ALTER TABLE "BookingAudit" DROP COLUMN "userId",
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ALTER TABLE both drops existing booking audit user references and adds a NOT NULL actorId without backfilling, so the migration will fail on populated tables and would erase historical audit linkage even if it ran.

Prompt for AI agents
Address the following comment on packages/prisma/migrations/20251022075606_add_actor_model_with_user_type/migration.sql at line 12:

<comment>This ALTER TABLE both drops existing booking audit user references and adds a NOT NULL actorId without backfilling, so the migration will fail on populated tables and would erase historical audit linkage even if it ran.</comment>

<file context>
@@ -0,0 +1,48 @@
+CREATE TYPE &quot;ActorType&quot; AS ENUM (&#39;user&#39;, &#39;guest&#39;, &#39;system&#39;);
+
+-- AlterTable
+ALTER TABLE &quot;BookingAudit&quot; DROP COLUMN &quot;userId&quot;,
+ADD COLUMN     &quot;actorId&quot; TEXT NOT NULL;
+
</file context>
Fix with Cubic

});
await bookingEventHandlerService.onBookingCancelled(
String(updatedBooking.id),
createUserActor(userId || 0),
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actor passed to booking audit logging ignores the new actor input and always fabricates a user actor with id 0 when userId is absent, producing incorrect audit metadata. Please use the supplied actor (or fall back to a real system actor) instead of hard-coding createUserActor(userId || 0).

Prompt for AI agents
Address the following comment on packages/features/bookings/lib/handleCancelBooking.ts at line 485:

<comment>The actor passed to booking audit logging ignores the new `actor` input and always fabricates a user actor with id 0 when `userId` is absent, producing incorrect audit metadata. Please use the supplied actor (or fall back to a real system actor) instead of hard-coding `createUserActor(userId || 0)`.</comment>

<file context>
@@ -461,6 +472,28 @@ async function handler(input: CancelBookingInput) {
+      });
+      await bookingEventHandlerService.onBookingCancelled(
+        String(updatedBooking.id),
+        createUserActor(userId || 0),
+        {
+          cancellationReason: cancellationReason || undefined,
</file context>
Fix with Cubic

}

if (actor.type === "Attendee" && actor.metadata?.email) {
return `Attendee:${actor.metadata.email}`;
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Avoid Logging Sensitive Information

Returning the attendee email in the audit log string exposes PII in logs, violating the Avoid Logging Sensitive Information rule. Remove the email from the logged string.

Prompt for AI agents
Address the following comment on packages/features/bookings/lib/types/actor.ts at line 82:

<comment>Returning the attendee email in the audit log string exposes PII in logs, violating the Avoid Logging Sensitive Information rule. Remove the email from the logged string.</comment>

<file context>
@@ -0,0 +1,90 @@
+  }
+
+  if (actor.type === &quot;Attendee&quot; &amp;&amp; actor.metadata?.email) {
+    return `Attendee:${actor.metadata.email}`;
+  }
+
</file context>
Fix with Cubic

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed changes from recent commits (found 14 issues).

14 issues found across 46 files

Prompt for AI agents (all 14 issues)

Understand the root cause of the following 14 issues and fix them.


<file name="packages/features/ee/round-robin/roundRobinReassignment.ts">

<violation number="1" location="packages/features/ee/round-robin/roundRobinReassignment.ts:252">
Please keep using booking.userPrimaryEmail for oldValue so the audit log reflects the field that actually changes. Pulling booking.user?.email can differ when the host has updated their profile email, which produces incorrect audit data.</violation>
</file>

<file name="packages/features/booking-audit/lib/repository/PrismaActorRepository.ts">

<violation number="1" location="packages/features/booking-audit/lib/repository/PrismaActorRepository.ts:8">
The entire `PrismaActorRepository` class, including `findByUserId`, `upsertUserActor`, and `getSystemActor` methods, is an exact duplicate of the existing implementation in `packages/lib/server/repository/PrismaActorRepository.ts`.</violation>

<violation number="2" location="packages/features/booking-audit/lib/repository/PrismaActorRepository.ts:25">
This upsert will throw when a user shares an email with an existing guest/attendee actor: the create branch always stores `email: user.email`, but the Actor schema enforces `@@unique([email])`, so the first booking action for a user who previously booked as a guest will raise a unique constraint error instead of reusing that actor. Please reuse the existing actor (e.g., by looking it up via email) before attempting to create a new row.</violation>
</file>

<file name="packages/features/booking-audit/lib/actions/LocationChangedAuditActionHelperService.ts">

<violation number="1" location="packages/features/booking-audit/lib/actions/LocationChangedAuditActionHelperService.ts:33">
Please wrap the detail labels in getDisplayDetails with the provided `t` function instead of hard-coded English strings so the audit UI localizes correctly.</violation>
</file>

<file name="packages/prisma/schema.prisma">

<violation number="1" location="packages/prisma/schema.prisma:2650">
Using `onDelete: Restrict` on the actor→attendee relation means any attempt to delete an attendee (or delete a booking that cascades attendees) will now fail with a foreign-key violation. Retain the actor record while allowing attendee rows to be removed by switching the relation to `SetNull`.</violation>

<violation number="2" location="packages/prisma/schema.prisma:2686">
Rule violated: **Prevent Direct NOW() Usage in Database Queries**

This new audit timestamp still calls NOW() without forcing UTC, so it can record non-UTC times depending on the Postgres timezone. Please switch to a timezone-safe default (and do the same for the new Actor.createdAt field) to keep audit data consistent.</violation>
</file>

<file name="packages/features/booking-audit/lib/actions/StatusChangeAuditActionHelperService.ts">

<violation number="1" location="packages/features/booking-audit/lib/actions/StatusChangeAuditActionHelperService.ts:30">
`getDisplayDetails` ignores the provided status `changes`, so the audit UI never surfaces the old/new status values. Please use the collected `changes` data (like we do for location changes) to show the previous and new status for the entry.</violation>
</file>

<file name="packages/features/booking-audit/lib/actions/AttendeeAddedAuditActionHelperService.ts">

<violation number="1" location="packages/features/booking-audit/lib/actions/AttendeeAddedAuditActionHelperService.ts:28">
`t(&#39;audit.added_guests&#39;, …)` references an i18n key that is not defined anywhere in the locale files, so the UI will display the raw key instead of a localized message. Please add the translation entry before using it.</violation>

<violation number="2" location="packages/features/booking-audit/lib/actions/AttendeeAddedAuditActionHelperService.ts:33">
Hard-coding the labels `&#39;Added Guests&#39;` and `&#39;Count&#39;` bypasses localization. Please use the provided translator (`t`) so these labels can be translated.</violation>
</file>

<file name="packages/features/booking-audit/lib/actions/RejectionReasonUpdatedAuditActionHelperService.ts">

<violation number="1" location="packages/features/booking-audit/lib/actions/RejectionReasonUpdatedAuditActionHelperService.ts:29">
Please use the provided `t` translation function when populating UI labels so the &quot;Reason&quot; detail is localized for non-English users instead of hard-coded in English.</violation>
</file>

<file name="BOOKING_AUDIT_ARCHITECTURE.md">

<violation number="1" location="BOOKING_AUDIT_ARCHITECTURE.md:28">
Rule violated: **Prevent Direct NOW() Usage in Database Queries**

This Prisma schema example sets `createdAt` to `@default(now())`, which relies on the database session timezone and breaks the &quot;Prevent Direct NOW() Usage in Database Queries&quot; rule. Please switch to a UTC-qualified default so audit timestamps remain consistent across environments.

(Based on your team&#39;s feedback about the server defaulting to UTC, but explicit timezone is still required for cross-environment safety.) [FEEDBACK_USED]</violation>
</file>

<file name="packages/features/booking-audit/lib/actions/AssignmentAuditActionHelperService.ts">

<violation number="1" location="packages/features/booking-audit/lib/actions/AssignmentAuditActionHelperService.ts:18">
Rule violated: **Avoid Logging Sensitive Information**

Audit payload must not persist email addresses. Capturing the assigned user email in the audit log violates our sensitive data logging policy. Please remove the email field from the stored audit data.</violation>
</file>

<file name="packages/features/booking-audit/lib/actions/ReassignmentAuditActionHelperService.ts">

<violation number="1" location="packages/features/booking-audit/lib/actions/ReassignmentAuditActionHelperService.ts:39">
Rule violated: **Avoid Logging Sensitive Information**

Audit entries must not contain unredacted email addresses. This line stores both the old and new email values in the audit log payload, violating the &quot;Avoid Logging Sensitive Information&quot; rule.</violation>
</file>

<file name="packages/features/bookings/lib/types/actor.ts">

<violation number="1" location="packages/features/bookings/lib/types/actor.ts:86">
Rule violated: **Avoid Logging Sensitive Information**

`actorToAuditString` now logs guest email addresses in audit entries (`Guest:${email}`), which violates the &quot;Avoid Logging Sensitive Information&quot; rule that prohibits logging PII like emails.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

newBookingTitle = getEventName(eventNameObject);

const oldUserId = booking.userId;
const oldEmail = booking.user?.email || "";
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep using booking.userPrimaryEmail for oldValue so the audit log reflects the field that actually changes. Pulling booking.user?.email can differ when the host has updated their profile email, which produces incorrect audit data.

Prompt for AI agents
Address the following comment on packages/features/ee/round-robin/roundRobinReassignment.ts at line 252:

<comment>Please keep using booking.userPrimaryEmail for oldValue so the audit log reflects the field that actually changes. Pulling booking.user?.email can differ when the host has updated their profile email, which produces incorrect audit data.</comment>

<file context>
@@ -251,7 +249,7 @@ export const roundRobinReassignment = async ({
 
     const oldUserId = booking.userId;
-    const oldEmail = booking.userPrimaryEmail;
+    const oldEmail = booking.user?.email || &quot;&quot;;
     const oldTitle = booking.title;
 
</file context>
Suggested change
const oldEmail = booking.user?.email || "";
const oldEmail = booking.userPrimaryEmail;
Fix with Cubic


const SYSTEM_ACTOR_ID = "00000000-0000-0000-0000-000000000000";

export class PrismaActorRepository implements IActorRepository {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire PrismaActorRepository class, including findByUserId, upsertUserActor, and getSystemActor methods, is an exact duplicate of the existing implementation in packages/lib/server/repository/PrismaActorRepository.ts.

Prompt for AI agents
Address the following comment on packages/features/booking-audit/lib/repository/PrismaActorRepository.ts at line 8:

<comment>The entire `PrismaActorRepository` class, including `findByUserId`, `upsertUserActor`, and `getSystemActor` methods, is an exact duplicate of the existing implementation in `packages/lib/server/repository/PrismaActorRepository.ts`.</comment>

<file context>
@@ -0,0 +1,52 @@
+
+const SYSTEM_ACTOR_ID = &quot;00000000-0000-0000-0000-000000000000&quot;;
+
+export class PrismaActorRepository implements IActorRepository {
+    async findByUserId(userId: number): Promise&lt;Actor | null&gt; {
+        return prisma.actor.findUnique({
</file context>
Fix with Cubic

const locationChange = data.changes.find(c => c.field === 'location');
if (locationChange) {
return {
'Old Location': String(locationChange.oldValue || '-'),
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please wrap the detail labels in getDisplayDetails with the provided t function instead of hard-coded English strings so the audit UI localizes correctly.

Prompt for AI agents
Address the following comment on packages/features/booking-audit/lib/actions/LocationChangedAuditActionHelperService.ts at line 33:

<comment>Please wrap the detail labels in getDisplayDetails with the provided `t` function instead of hard-coded English strings so the audit UI localizes correctly.</comment>

<file context>
@@ -0,0 +1,42 @@
+        const locationChange = data.changes.find(c =&gt; c.field === &#39;location&#39;);
+        if (locationChange) {
+            return {
+                &#39;Old Location&#39;: String(locationChange.oldValue || &#39;-&#39;),
+                &#39;New Location&#39;: String(locationChange.newValue || &#39;-&#39;),
+            };
</file context>
Fix with Cubic

// References for different actor types
userId Int? // For USER type (nullable to allow user deletion)
attendeeId Int? // For ATTENDEE type
attendee Attendee? @relation(fields: [attendeeId], references: [id], onDelete: Restrict)
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using onDelete: Restrict on the actor→attendee relation means any attempt to delete an attendee (or delete a booking that cascades attendees) will now fail with a foreign-key violation. Retain the actor record while allowing attendee rows to be removed by switching the relation to SetNull.

Prompt for AI agents
Address the following comment on packages/prisma/schema.prisma at line 2650:

<comment>Using `onDelete: Restrict` on the actor→attendee relation means any attempt to delete an attendee (or delete a booking that cascades attendees) will now fail with a foreign-key violation. Retain the actor record while allowing attendee rows to be removed by switching the relation to `SetNull`.</comment>

<file context>
@@ -2631,25 +2634,36 @@ enum BookingAuditAction {
+  // References for different actor types
+  userId     Int?      // For USER type (nullable to allow user deletion)
+  attendeeId Int?      // For ATTENDEE type
+  attendee   Attendee? @relation(fields: [attendeeId], references: [id], onDelete: Restrict)
+  
+  // Identity fields (for GUEST/ATTENDEE types)
</file context>
Suggested change
attendee Attendee? @relation(fields: [attendeeId], references: [id], onDelete: Restrict)
attendee Attendee? @relation(fields: [attendeeId], references: [id], onDelete: SetNull)
Fix with Cubic

}

static getDisplayDetails(data: z.infer<typeof this.schema>, t: TFunction): Record<string, string> {
return {};
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getDisplayDetails ignores the provided status changes, so the audit UI never surfaces the old/new status values. Please use the collected changes data (like we do for location changes) to show the previous and new status for the entry.

Prompt for AI agents
Address the following comment on packages/features/booking-audit/lib/actions/StatusChangeAuditActionHelperService.ts at line 30:

<comment>`getDisplayDetails` ignores the provided status `changes`, so the audit UI never surfaces the old/new status values. Please use the collected `changes` data (like we do for location changes) to show the previous and new status for the entry.</comment>

<file context>
@@ -0,0 +1,35 @@
+    }
+
+    static getDisplayDetails(data: z.infer&lt;typeof this.schema&gt;, t: TFunction): Record&lt;string, string&gt; {
+        return {};
+    }
+}
</file context>
Fix with Cubic

phone String?
name String?

createdAt DateTime @default(now())
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: **Prevent Direct NOW() Usage in Database Queries** This Prisma schema example sets `createdAt` to `@default(now())`, which relies on the database session timezone and breaks the "Prevent Direct NOW() Usage in Database Queries" rule. Please switch to a UTC-qualified default so audit timestamps remain consistent across environments. (Based on your team's feedback about the server defaulting to UTC, but explicit timezone is still required for cross-environment safety.)

View Feedback

Prompt for AI agents ~~~ Address the following comment on BOOKING_AUDIT_ARCHITECTURE.md at line 28: This Prisma schema example sets `createdAt` to `@default(now())`, which relies on the database session timezone and breaks the "Prevent Direct NOW() Usage in Database Queries" rule. Please switch to a UTC-qualified default so audit timestamps remain consistent across environments. (Based on your team's feedback about the server defaulting to UTC, but explicit timezone is still required for cross-environment safety.) @@ -0,0 +1,885 @@ + phone String? + name String? + + createdAt DateTime @default(now()) + bookingAudits BookingAudit[] + ~~~
```suggestion createdAt DateTime @default(dbgenerated("timezone('utc', now())")) ``` Fix with Cubic

timestamp DateTime

// Database record timestamps
createdAt DateTime @default(now())
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Prevent Direct NOW() Usage in Database Queries

This new audit timestamp still calls NOW() without forcing UTC, so it can record non-UTC times depending on the Postgres timezone. Please switch to a timezone-safe default (and do the same for the new Actor.createdAt field) to keep audit data consistent.

Prompt for AI agents
Address the following comment on packages/prisma/schema.prisma at line 2686:

<comment>This new audit timestamp still calls NOW() without forcing UTC, so it can record non-UTC times depending on the Postgres timezone. Please switch to a timezone-safe default (and do the same for the new Actor.createdAt field) to keep audit data consistent.</comment>

<file context>
@@ -2661,13 +2675,22 @@ model BookingAudit {
+  timestamp DateTime
+  
+  // Database record timestamps
+  createdAt DateTime @default(now())
+  updatedAt DateTime @updatedAt
+  
</file context>
Suggested change
createdAt DateTime @default(now())
createdAt DateTime @default(dbgenerated("CURRENT_TIMESTAMP AT TIME ZONE 'UTC'"))
Fix with Cubic


static createData(params: {
assignmentMethod: 'manual' | 'round_robin' | 'salesforce' | 'routing_form' | 'crm_ownership';
assignedUser: { id: number; name: string; email: string };
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Avoid Logging Sensitive Information

Audit payload must not persist email addresses. Capturing the assigned user email in the audit log violates our sensitive data logging policy. Please remove the email field from the stored audit data.

Prompt for AI agents
Address the following comment on packages/features/booking-audit/lib/actions/AssignmentAuditActionHelperService.ts at line 18:

<comment>Audit payload must not persist email addresses. Capturing the assigned user email in the audit log violates our sensitive data logging policy. Please remove the email field from the stored audit data.</comment>

<file context>
@@ -0,0 +1,52 @@
+
+    static createData(params: {
+        assignmentMethod: &#39;manual&#39; | &#39;round_robin&#39; | &#39;salesforce&#39; | &#39;routing_form&#39; | &#39;crm_ownership&#39;;
+        assignedUser: { id: number; name: string; email: string };
+        previousUser?: { id: number; name: string; email: string };
+        teamId?: number;
</file context>
Fix with Cubic

},
changes: [
{ field: "userId", oldValue: params.userIdChange.oldValue, newValue: params.userIdChange.newValue },
{ field: "userPrimaryEmail", oldValue: params.emailChange.oldValue, newValue: params.emailChange.newValue },
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Avoid Logging Sensitive Information

Audit entries must not contain unredacted email addresses. This line stores both the old and new email values in the audit log payload, violating the "Avoid Logging Sensitive Information" rule.

Prompt for AI agents
Address the following comment on packages/features/booking-audit/lib/actions/ReassignmentAuditActionHelperService.ts at line 39:

<comment>Audit entries must not contain unredacted email addresses. This line stores both the old and new email values in the audit log payload, violating the &quot;Avoid Logging Sensitive Information&quot; rule.</comment>

<file context>
@@ -0,0 +1,64 @@
+            },
+            changes: [
+                { field: &quot;userId&quot;, oldValue: params.userIdChange.oldValue, newValue: params.userIdChange.newValue },
+                { field: &quot;userPrimaryEmail&quot;, oldValue: params.emailChange.oldValue, newValue: params.emailChange.newValue },
+            ],
+        };
</file context>
Suggested change
{ field: "userPrimaryEmail", oldValue: params.emailChange.oldValue, newValue: params.emailChange.newValue },
{ field: "userPrimaryEmail", oldValue: params.emailChange.oldValue ? "<redacted>" : null, newValue: "<redacted>" },
Fix with Cubic

}

if (actor.type === "Guest" && actor.metadata?.email) {
return `Guest:${actor.metadata.email}`;
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Avoid Logging Sensitive Information

actorToAuditString now logs guest email addresses in audit entries (Guest:${email}), which violates the "Avoid Logging Sensitive Information" rule that prohibits logging PII like emails.

Prompt for AI agents
Address the following comment on packages/features/bookings/lib/types/actor.ts at line 86:

<comment>`actorToAuditString` now logs guest email addresses in audit entries (`Guest:${email}`), which violates the &quot;Avoid Logging Sensitive Information&quot; rule that prohibits logging PII like emails.</comment>

<file context>
@@ -78,8 +82,8 @@ export function actorToAuditString(actor?: Actor): string | null {
-  if (actor.type === &quot;Attendee&quot; &amp;&amp; actor.metadata?.email) {
-    return `Attendee:${actor.metadata.email}`;
+  if (actor.type === &quot;Guest&quot; &amp;&amp; actor.metadata?.email) {
+    return `Guest:${actor.metadata.email}`;
   }
 
</file context>
Suggested change
return `Guest:${actor.metadata.email}`;
return "Guest";
Fix with Cubic

@hariombalhara hariombalhara force-pushed the devin/booking-audit-integration-1761111651 branch from 96148c7 to 1dd63c8 Compare October 31, 2025 13:41
@github-actions github-actions bot removed the Stale label Nov 1, 2025
@hariombalhara hariombalhara changed the base branch from feat/booking-audit-log to refactor-audit-abstraction-k7vXx November 1, 2025 06:09
@hariombalhara hariombalhara changed the base branch from refactor-audit-abstraction-k7vXx to booking-audit-foundation November 3, 2025 04:37
@hariombalhara hariombalhara force-pushed the booking-audit-foundation branch from 2a2a4bd to df53011 Compare November 3, 2025 04:47
@hariombalhara hariombalhara force-pushed the devin/booking-audit-integration-1761111651 branch from b8382e3 to 7e9066b Compare November 3, 2025 05:15
@hariombalhara hariombalhara force-pushed the booking-audit-foundation branch from 93cac6b to 2a2a4bd Compare November 3, 2025 05:15
Updated multiple booking audit action services to replace individual primary schemas with a unified structure using common change schemas. Changes include:
- Introduced `StringChangeSchema`, `BooleanChangeSchema`, `StringArrayChangeSchema`, and `NumberChangeSchema` for consistent old/new value tracking.
- Simplified the schema definitions in services such as `AttendeeAddedAuditActionService`, `CancelledAuditActionService`, and others.
- Enhanced clarity and maintainability of the codebase by centralizing schema definitions.

These updates improve the consistency of the audit trail across various booking actions.
- Update audit action services (Attendee, Cancellation, Reassignment, Rejection)
- Refactor booking-audit architecture documentation
- Integrate audit logging in handleCancelBooking
- Add audit logging to round-robin reassignment flows
- Clean up deprecated schemas
- Fix unused variable linting issues
- Create booking logs page component
- Add booking logs view module with timeline UI
- Implement getAuditLogs TRPC handler and schema
- Wire up audit logs endpoint to viewer router
- Add responsive audit log display with filtering and search
- Fix unused variable linting issues
…y nesting

Updated multiple audit action services to eliminate the unnecessary `primary` nesting in schema definitions. This change enhances clarity and consistency across services, including `AttendeeAddedAuditActionService`, `CancelledAuditActionService`, and others. The new structure directly tracks old and new values for each field, streamlining the audit logging process.
@github-actions
Copy link
Contributor

This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active.

@github-actions github-actions bot added the Stale label Nov 12, 2025
… services

Updated multiple audit action services to support per-action versioning, enhancing schema management and data integrity. Each service now includes a version wrapper for stored data, allowing for easier schema evolution and clearer separation of input and stored formats. Key changes were made in services such as `AttendeeAddedAuditActionService`, `CancelledAuditActionService`, and others, ensuring consistent handling of versioning across the booking audit system.
…pecific type imports

Refactored multiple audit action services to import types directly from their respective action service files, enhancing clarity and maintainability. This change simplifies the import structure and aligns with the recent schema updates, ensuring consistent type usage across the booking audit system. Additionally, removed the obsolete SCHEMA-MIGRATION-PLAN.md file to streamline documentation.
…amline action service schemas

Refactored multiple audit action services to enhance schema clarity by using fields instead of data in their definitions. This change improves the structure and consistency of the audit logging process across services, ensuring better maintainability and alignment with recent updates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ❗️ migrations contains migration files size/XXL Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants