Skip to content

feat: Add Actor pattern foundation for booking audit logging#24503

Closed
hariombalhara wants to merge 2 commits intofeat/booking-audit-logfrom
devin/booking-audit-integration-1760614786
Closed

feat: Add Actor pattern foundation for booking audit logging#24503
hariombalhara wants to merge 2 commits intofeat/booking-audit-logfrom
devin/booking-audit-integration-1760614786

Conversation

@hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Oct 16, 2025

What does this PR do?

This PR integrates the existing BookingAuditService into the booking lifecycle to log all booking creation, cancellation, and confirmation events. This builds upon the audit logging infrastructure added in the base branch (PR #22817).

Key Changes:

  • Added BookingAuditService calls to handleNewBooking for logging booking creation
  • Added BookingAuditService calls to handleCancelBooking for logging cancellations
  • Added BookingAuditService calls to handleConfirmation for logging confirmations
  • Fixed Prisma schema validation error (missing closing brace in BookingAudit model)
  • Added Actor type definition and optional parameter support in booking types

Important Notes:

  • This PR assumes the BookingAudit table and related infrastructure exist from the base branch
  • Actor parameter is defined but not fully utilized yet - audit logs currently use userId directly
  • Audit logging failures are caught and logged as errors (non-blocking)

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

Mandatory Tasks

  • I have self-reviewed the code
  • I have updated the developer docs if needed (N/A - internal feature)
  • I confirm automated tests are in place that prove my fix is effective or that my feature works

How should this be tested?

Prerequisites:

  1. Ensure the base branch (feat/booking-audit-log) migrations have been run to create the BookingAudit table
  2. Run npx prisma generate from packages/prisma/ to regenerate Prisma client with BookingAudit types

Test Scenarios:

  1. Create a booking - Verify a BookingAudit record is created with type RECORD_CREATED and action ACCEPTED
  2. Cancel a booking - Verify a BookingAudit record is created with type RECORD_UPDATED and action CANCELLED
  3. Confirm a pending booking - Verify a BookingAudit record is created with type RECORD_UPDATED and action ACCEPTED

Database Query to Verify:

SELECT * FROM "BookingAudit" WHERE "bookingId" = '<your_booking_id>' ORDER BY "timestamp" DESC;

Known Limitations:

  • May not cover all booking update operations (focus is on main lifecycle events)
  • Actor parameter infrastructure exists but is not yet fully integrated from caller side
  • Error handling silently logs failures - check server logs if audit records are missing

Review Checklist

Critical Items to Review:

  • Verify BookingAuditService is being called correctly in all three handlers
  • Check if there are other places where bookings are updated/deleted that need audit logging
  • Confirm error handling approach (currently silently fails with log) is acceptable
  • Consider if direct userId usage is sufficient or if Actor object should be fully integrated
  • Verify Prisma schema fix doesn't require a new migration

Type Safety Concerns:

  • BookingAudit types may show as missing until Prisma client is regenerated
  • This is expected and will resolve after running npx prisma generate

@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 16, 2025
@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Oct 16, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/booking-audit-integration-1760614786

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.

@CLAassistant
Copy link

CLAassistant commented Oct 16, 2025

CLA assistant check
All committers have signed the CLA.

@hariombalhara hariombalhara force-pushed the hariom/pri-305-bookingcreateservice-rename-handlers-to-services branch from 9ba3f1d to fb0bf72 Compare October 16, 2025 12:05
@vercel
Copy link

vercel bot commented Oct 16, 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 Oct 22, 2025 5:48am
cal-eu Ignored Ignored Oct 22, 2025 5:48am

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.

4 issues found across 27 files

Prompt for AI agents (all 4 issues)

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


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

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

`BookingAudit.timestamp` uses `now()` as the default, which issues a timezone-less `NOW()` at the database layer. The ``Prevent Direct NOW() Usage in Database Queries`` guideline requires explicit UTC handling to avoid inconsistent audit timelines. Please switch to a UTC-aware expression.</violation>
</file>

<file name="apps/web/modules/bookings/views/bookings-single-view.tsx">

<violation number="1" location="apps/web/modules/bookings/views/bookings-single-view.tsx:697">
Removing the `whitespace-pre-line` class means multiline booking notes will now collapse into a single paragraph, so any attendee-entered line breaks are lost. Please keep `whitespace-pre-line` so the additional notes preserve their formatting.</violation>
</file>

<file name="packages/prisma/migrations/20250730030812_init_booking_audit/migration.sql">

<violation number="1" location="packages/prisma/migrations/20250730030812_init_booking_audit/migration.sql:10">
Please add an index on bookingId so audit lookups for a specific booking avoid full-table scans.</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**

Audit log serialization must not expose attendee emails. Returning `Attendee:${actor.metadata.email}` records PII directly into logs, violating the policy against logging sensitive data. Replace this with a non-PII identifier (e.g., just &quot;Attendee&quot;) or a hashed token.</violation>
</file>

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

userId String?
type BookingAuditType
action BookingAuditAction?
timestamp DateTime @default(now())
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 17, 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

BookingAudit.timestamp uses now() as the default, which issues a timezone-less NOW() at the database layer. The Prevent Direct NOW() Usage in Database Queries guideline requires explicit UTC handling to avoid inconsistent audit timelines. Please switch to a UTC-aware expression.

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

<comment>`BookingAudit.timestamp` uses `now()` as the default, which issues a timezone-less `NOW()` at the database layer. The ``Prevent Direct NOW() Usage in Database Queries`` guideline requires explicit UTC handling to avoid inconsistent audit timelines. Please switch to a UTC-aware expression.</comment>

<file context>
@@ -2594,6 +2594,53 @@ model RolePermission {
+  userId    String?
+  type      BookingAuditType
+  action    BookingAuditAction?
+  timestamp DateTime            @default(now())
+  data      Json?
+}
</file context>
Suggested change
timestamp DateTime @default(now())
timestamp DateTime @default(dbgenerated("timezone('utc', now())"))
Fix with Cubic

<div className="mt-9 font-medium">{t("additional_notes")}</div>
<div className="col-span-2 mb-2 mt-9">
<p className="whitespace-pre-line break-words">{bookingInfo.description}</p>
<p className="break-words">{bookingInfo.description}</p>
Copy link
Contributor

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

Choose a reason for hiding this comment

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

Removing the whitespace-pre-line class means multiline booking notes will now collapse into a single paragraph, so any attendee-entered line breaks are lost. Please keep whitespace-pre-line so the additional notes preserve their formatting.

Prompt for AI agents
Address the following comment on apps/web/modules/bookings/views/bookings-single-view.tsx at line 697:

<comment>Removing the `whitespace-pre-line` class means multiline booking notes will now collapse into a single paragraph, so any attendee-entered line breaks are lost. Please keep `whitespace-pre-line` so the additional notes preserve their formatting.</comment>

<file context>
@@ -694,7 +694,7 @@ export default function Success(props: PageProps) {
                             &lt;div className=&quot;mt-9 font-medium&quot;&gt;{t(&quot;additional_notes&quot;)}&lt;/div&gt;
                             &lt;div className=&quot;col-span-2 mb-2 mt-9&quot;&gt;
-                              &lt;p className=&quot;whitespace-pre-line break-words&quot;&gt;{bookingInfo.description}&lt;/p&gt;
+                              &lt;p className=&quot;break-words&quot;&gt;{bookingInfo.description}&lt;/p&gt;
                             &lt;/div&gt;
                           &lt;/&gt;
</file context>
Suggested change
<p className="break-words">{bookingInfo.description}</p>
<p className="whitespace-pre-line break-words">{bookingInfo.description}</p>
Fix with Cubic

-- CreateTable
CREATE TABLE "BookingAudit" (
"id" TEXT NOT NULL,
"bookingId" TEXT NOT NULL,
Copy link
Contributor

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

Choose a reason for hiding this comment

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

Please add an index on bookingId so audit lookups for a specific booking avoid full-table scans.

Prompt for AI agents
Address the following comment on packages/prisma/migrations/20250730030812_init_booking_audit/migration.sql at line 10:

<comment>Please add an index on bookingId so audit lookups for a specific booking avoid full-table scans.</comment>

<file context>
@@ -0,0 +1,18 @@
+-- CreateTable
+CREATE TABLE &quot;BookingAudit&quot; (
+    &quot;id&quot; TEXT NOT NULL,
+    &quot;bookingId&quot; TEXT NOT NULL,
+    &quot;userId&quot; TEXT,
+    &quot;type&quot; &quot;BookingAuditType&quot; NOT NULL,
</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 17, 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 log serialization must not expose attendee emails. Returning Attendee:${actor.metadata.email} records PII directly into logs, violating the policy against logging sensitive data. Replace this with a non-PII identifier (e.g., just "Attendee") or a hashed token.

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

<comment>Audit log serialization must not expose attendee emails. Returning `Attendee:${actor.metadata.email}` records PII directly into logs, violating the policy against logging sensitive data. Replace this with a non-PII identifier (e.g., just &quot;Attendee&quot;) or a hashed token.</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>
Suggested change
return `Attendee:${actor.metadata.email}`;
return "Attendee";
Fix with Cubic

@hariombalhara hariombalhara force-pushed the hariom/pri-305-bookingcreateservice-rename-handlers-to-services branch from fb0bf72 to a9b00d4 Compare October 21, 2025 04:11
Base automatically changed from hariom/pri-305-bookingcreateservice-rename-handlers-to-services to main October 21, 2025 11:52
devin-ai-integration bot and others added 2 commits October 22, 2025 05:41
- Define Actor type to represent who performs booking actions (User/System/Attendee)
- Add helper functions for creating and working with actors
- Integrate actor parameter into CancelBookingInput for cancellation tracking
- Create comprehensive documentation for actor pattern implementation
- Foundation for passing actor through all booking services for audit logging

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
- Create logBookingAudit utility function for centralized audit logging
- Add Actor type import to CreateBookingMeta
- Update CreateBookingMeta to include optional actor parameter
- Actor parameter enables audit logging for who performed booking actions
- All actor parameters are optional for backward compatibility

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
@devin-ai-integration devin-ai-integration bot force-pushed the devin/booking-audit-integration-1760614786 branch from 2c6ca7b to dae9452 Compare October 22, 2025 05:48
@devin-ai-integration devin-ai-integration bot changed the base branch from main to feat/booking-audit-log October 22, 2025 05:48
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/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments