fix: Include properties in export/events Payload

[wikando-pm]

Fixes #281

Summary by CodeRabbit

• New Features

  • /export/events responses now include an event properties field when available.

• Tests

  • Added an automated test suite for the Export API covering basic export, event-type filtering, includes (profile), and date-range scenarios; shows progress and a pass/fail summary and exits with non-zero status on failures.

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

[vercel]

@wikando-pm is attempting to deploy a commit to the Coderax's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new test script that exercises the Export API /export/events endpoint and updates the export controller to include event properties in the returned payload so exported events include their properties.

Changes

Cohort / File(s)	Summary
Export API Test Script apps/api/scripts/test-export-api.ts	New TypeScript script performing authenticated requests to /export/events (basic, event filter, includes:['profile'], date range). Reads env vars (CLIENT_ID, CLIENT_SECRET, PROJECT_ID, API_URL), constructs query params, parses JSON defensively, validates presence of properties, logs results, exits non-zero on failures.
Export Controller apps/api/src/controllers/export.controller.ts	Adds properties: true to the select/options for event list queries so event properties are included in API responses.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped along the API trail,
Sniffed out fields behind the veil.
A tiny change, a test that sings,
Now events wear their property wings.
Nibble, leap — export delights!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding properties to the export/events API payload, which directly addresses the linked issue #281.
Linked Issues check	✅ Passed	The PR successfully implements the fix for issue #281 by adding the properties field to the export/events endpoint response through modifications to the export controller and comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the issue: the controller modification adds the properties field, and the test script validates the fix works across multiple scenarios.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@apps/api/scripts/test-export-api.ts`:
- Around line 104-127: The current test only logs when firstEvent.properties is
missing but doesn't fail the test; update the block that checks
firstEvent.properties (where basicResult and firstEvent are used) to treat a
missing properties field as a test failure by setting process.exitCode = 1 (or
throw a new Error) after logging the error so the CI fails; apply the same
change to the equivalent checks in Tests 2, 3, and 4 (the other result
variables/firstEvent equivalents) so any response missing the properties field
causes the script to exit non‑zero.

🧹 Nitpick comments (2)

apps/api/scripts/test-export-api.ts (2)

16-24: Consider removing non-null assertions for clearer intent.

The non-null assertions (!) on lines 16-18 are misleading since these values can be undefined. The validation on line 21 correctly handles this, but the assertions suggest the values are guaranteed to exist.

Suggested improvement

-const CLIENT_ID = process.env.CLIENT_ID!;
-const CLIENT_SECRET = process.env.CLIENT_SECRET!;
-const PROJECT_ID = process.env.PROJECT_ID!;
+const CLIENT_ID = process.env.CLIENT_ID;
+const CLIENT_SECRET = process.env.CLIENT_SECRET;
+const PROJECT_ID = process.env.PROJECT_ID;
 const API_BASE_URL = process.env.API_URL || 'http://localhost:3333';
 
 if (!CLIENT_ID || !CLIENT_SECRET || !PROJECT_ID) {
   console.error('CLIENT_ID, CLIENT_SECRET, and PROJECT_ID must be set');
   process.exit(1);
 }
+
+// After validation, we can safely assert these are defined
+const config = {
+  clientId: CLIENT_ID,
+  clientSecret: CLIENT_SECRET,
+  projectId: PROJECT_ID,
+  apiBaseUrl: API_BASE_URL,
+} as const;

---

119-120: Consider limiting logged data to avoid exposing sensitive event properties.

Logging the full event structure could expose sensitive user data (PII) in CI logs or test output. For a test script, consider logging only the keys or a redacted summary.

Suggested improvement

       // Log full first event for inspection
-      console.log(`  First event structure:`, JSON.stringify(firstEvent, null, 2));
+      console.log(`  First event keys: ${Object.keys(firstEvent).join(', ')}`);
+      console.log(`  Properties keys: ${firstEvent.properties ? Object.keys(firstEvent.properties).join(', ') : 'N/A'}`);

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@apps/api/scripts/test-export-api.ts`:
- Around line 111-121: The test currently prints the full event payload
(firstEvent) which can expose PII; remove or replace the console.log that
outputs the full event structure and instead log a redacted summary—e.g.,
Object.keys(firstEvent), Object.keys(firstEvent.properties || {}), or a
sanitized JSON with sensitive fields replaced—so update the code around the
firstEvent.properties check and the "First event structure" log to only emit
keys or a redacted summary instead of the full firstEvent object.

🧹 Nitpick comments (1)

apps/api/scripts/test-export-api.ts (1)

64-80: Add a request timeout to prevent hung CI runs.

A stalled endpoint will cause this test script to hang indefinitely. Using AbortController with a 15-second timeout improves reliability.

⏱️ Proposed implementation

   try {
-    const response = await fetch(url, {
-      method,
-      headers,
-    });
+    const controller = new AbortController();
+    const timeout = setTimeout(() => controller.abort(), 15_000);
+    const response = await fetch(url, {
+      method,
+      headers,
+      signal: controller.signal,
+    });
+    clearTimeout(timeout);