feat(backend): add snowflake data warehouse integration #129
Conversation
- Add SnowflakeService with connection pooling, query execution, and caching - Implement LockManager for distributed operations and resource coordination - Add Snowflake-related interfaces, enums, and constants to shared package - Add comprehensive architecture documentation for Snowflake integration - Configure environment variables for Snowflake connection - Add @snowflake-sdk/client dependency This integration enables server-side data warehouse queries with proper connection management, query optimization, and distributed locking. Signed-off-by: Asitha de Silva <[email protected]>
Signed-off-by: Asitha de Silva <[email protected]>
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds read-only Snowflake integration (env, SDK, constants, types), a pooled SnowflakeService with private-key/JWT auth and query deduplication via LockManager; new analytics HTTP endpoints and routes; frontend Segment analytics service and analytics-data fetches; docs; and UI/profile field renames (title → job_title). Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AnalyticsController
participant SnowflakeService
participant LockManager
participant Pool
participant SnowflakeDB
Client->>AnalyticsController: GET /api/analytics/...
activate AnalyticsController
AnalyticsController->>SnowflakeService: execute(sql, binds)
activate SnowflakeService
SnowflakeService->>SnowflakeService: validateReadOnlyQuery(sql)
SnowflakeService->>LockManager: hashQuery(sql, binds)
LockManager-->>SnowflakeService: key
rect rgba(200,220,255,0.18)
SnowflakeService->>LockManager: executeLocked(key, executor)
alt duplicate in-flight
LockManager-->>SnowflakeService: existing Promise (dedupe hit)
else new execution
LockManager->>SnowflakeService: run executor()
end
end
SnowflakeService->>SnowflakeService: ensurePool()
SnowflakeService->>Pool: acquire connection
Pool-->>SnowflakeService: connection
SnowflakeService->>SnowflakeDB: execute parameterized query
SnowflakeDB-->>SnowflakeService: rows & metadata
SnowflakeService->>Pool: release connection
SnowflakeService-->>AnalyticsController: result
deactivate SnowflakeService
AnalyticsController-->>Client: JSON response
deactivate AnalyticsController
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive Snowflake data warehouse integration for the LFX One platform, enabling read-only analytical queries with enterprise-grade features including connection pooling, distributed locking, and robust security controls.
Key changes:
- Snowflake service with connection pooling, query deduplication, and multi-layer SQL injection protection
- Lock manager supporting in-memory and future Redis-based distributed locking strategies
- Type-safe interfaces, enums, and constants for Snowflake operations in the shared package
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/snowflake.interface.ts |
Defines TypeScript interfaces for query results, options, pool stats, lock stats, and lock entries |
packages/shared/src/interfaces/index.ts |
Exports Snowflake interfaces |
packages/shared/src/enums/snowflake.enum.ts |
Defines lock strategy enum for memory and Redis-based locking |
packages/shared/src/enums/index.ts |
Exports Snowflake enums |
packages/shared/src/constants/snowflake.constant.ts |
Defines configuration constants for timeouts, pool sizing, and lock management |
packages/shared/src/constants/index.ts |
Exports Snowflake constants |
packages/shared/package.json |
Adds snowflake-sdk as peer dependency |
docs/architecture/backend/snowflake-integration.md |
Comprehensive documentation covering architecture, security, configuration, and best practices |
apps/lfx-one/src/server/utils/lock-manager.ts |
Implements query deduplication lock manager with memory strategy and Redis placeholders |
apps/lfx-one/src/server/services/snowflake.service.ts |
Core service implementing connection pooling, query execution, and security validation |
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html |
Adjusts scrollable container styling for meetings component |
apps/lfx-one/package.json |
Adds snowflake-sdk dependency to application |
apps/lfx-one/.gitignore |
Excludes RSA key files from version control |
apps/lfx-one/.env.example |
Adds Snowflake configuration variables with documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/shared/src/interfaces/snowflake.interface.ts (1)
85-89: Consider making the LockEntry interface more type-safe.The
promisefield usesPromise<any>which loses type safety. Consider makingLockEntrygeneric:-export interface LockEntry { - promise: Promise<any>; +export interface LockEntry<T = any> { + promise: Promise<T>; timestamp: number; waiters: number; }This maintains backward compatibility while allowing callers to specify the promise type when needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
apps/lfx-one/.env.example(1 hunks)apps/lfx-one/.gitignore(1 hunks)apps/lfx-one/package.json(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html(1 hunks)apps/lfx-one/src/server/services/snowflake.service.ts(1 hunks)apps/lfx-one/src/server/utils/lock-manager.ts(1 hunks)docs/architecture/backend/snowflake-integration.md(1 hunks)packages/shared/package.json(1 hunks)packages/shared/src/constants/index.ts(1 hunks)packages/shared/src/constants/snowflake.constant.ts(1 hunks)packages/shared/src/enums/index.ts(1 hunks)packages/shared/src/enums/snowflake.enum.ts(1 hunks)packages/shared/src/interfaces/index.ts(1 hunks)packages/shared/src/interfaces/snowflake.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use barrel exports; always use direct imports for standalone components
Files:
packages/shared/src/constants/index.tspackages/shared/src/interfaces/index.tspackages/shared/src/enums/index.ts
packages/shared/src/constants/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all reusable constants in the shared package at packages/shared/src/constants
Files:
packages/shared/src/constants/index.tspackages/shared/src/constants/snowflake.constant.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
packages/shared/src/constants/index.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/server/services/snowflake.service.tspackages/shared/src/enums/index.tspackages/shared/src/interfaces/snowflake.interface.tspackages/shared/src/enums/snowflake.enum.tspackages/shared/src/constants/snowflake.constant.tsapps/lfx-one/src/server/utils/lock-manager.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
packages/shared/src/constants/index.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/server/services/snowflake.service.tspackages/shared/src/enums/index.tspackages/shared/src/interfaces/snowflake.interface.tspackages/shared/src/enums/snowflake.enum.tspackages/shared/src/constants/snowflake.constant.tsapps/lfx-one/src/server/utils/lock-manager.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
packages/shared/src/constants/index.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/server/services/snowflake.service.tspackages/shared/src/enums/index.tspackages/shared/src/interfaces/snowflake.interface.tspackages/shared/src/enums/snowflake.enum.tspackages/shared/src/constants/snowflake.constant.tsapps/lfx-one/src/server/utils/lock-manager.ts
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all TypeScript interfaces in the shared package at packages/shared/src/interfaces
Files:
packages/shared/src/interfaces/index.tspackages/shared/src/interfaces/snowflake.interface.ts
packages/shared/src/enums/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all enums in the shared package at packages/shared/src/enums
Files:
packages/shared/src/enums/index.tspackages/shared/src/enums/snowflake.enum.ts
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html
🧬 Code graph analysis (2)
apps/lfx-one/src/server/services/snowflake.service.ts (3)
apps/lfx-one/src/server/utils/lock-manager.ts (1)
LockManager(20-245)packages/shared/src/interfaces/snowflake.interface.ts (4)
SnowflakeQueryOptions(20-30)SnowflakeQueryResult(10-14)SnowflakePoolStats(35-55)LockStats(60-80)packages/shared/src/constants/snowflake.constant.ts (1)
SNOWFLAKE_CONFIG(7-57)
apps/lfx-one/src/server/utils/lock-manager.ts (2)
packages/shared/src/interfaces/snowflake.interface.ts (2)
LockEntry(85-89)LockStats(60-80)packages/shared/src/constants/snowflake.constant.ts (1)
SNOWFLAKE_CONFIG(7-57)
🪛 dotenv-linter (4.0.0)
apps/lfx-one/.env.example
[warning] 42-42: [UnorderedKey] The SNOWFLAKE_DATABASE key should go before the SNOWFLAKE_USERNAME key
(UnorderedKey)
[warning] 44-44: [UnorderedKey] The SNOWFLAKE_ROLE key should go before the SNOWFLAKE_USERNAME key
(UnorderedKey)
🔇 Additional comments (27)
apps/lfx-one/.gitignore (1)
51-54: LGTM!Properly ignoring Snowflake private key files prevents accidental credential exposure in version control.
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html (1)
20-21: Unrelated UI change in Snowflake integration PR.This scroll behavior modification appears unrelated to Snowflake data warehouse integration (PR #129). The PR summary focuses on backend Snowflake services, yet this changes frontend meeting display behavior. Consider moving this to a separate PR for better change isolation and review clarity.
packages/shared/package.json (1)
57-58: Peer dependency correctly declares Snowflake SDK requirement.The peerDependency approach ensures consuming packages provide their own snowflake-sdk instance, preventing version conflicts. Ensure this version matches the dependency in apps/lfx-one/package.json for consistency.
packages/shared/src/enums/snowflake.enum.ts (1)
1-17: Well-structured enum with proper documentation and placement.The enum correctly follows the coding guideline to place all enums in the shared package. License header and JSDoc comments are present. Note that only MEMORY strategy is currently implemented in the codebase, with REDIS reserved for future distributed scenarios.
Based on coding guidelines.
packages/shared/src/enums/index.ts (1)
8-8: LGTM!The barrel export follows the existing pattern for enums in the shared package. The coding guideline against barrel exports specifically applies to standalone components, not enum modules.
packages/shared/src/interfaces/index.ts (1)
63-65: LGTM!The Snowflake interfaces export follows the established pattern for the shared package interfaces barrel. The coding guideline regarding barrel exports applies to standalone components, not interface modules.
packages/shared/src/constants/index.ts (1)
19-19: LGTM!The Snowflake constants export follows the established pattern and adheres to the coding guideline requiring reusable constants to be placed in the shared package.
Based on coding guidelines.
apps/lfx-one/package.json (1)
53-53: No action required—snowflake-sdk is already at the latest stable version.The latest stable version of snowflake-sdk is 2.3.1 (released October 9, 2025), and the package.json already specifies
^2.3.1. The dependency is current.apps/lfx-one/.env.example (1)
33-61: LGTM! Well-documented Snowflake configuration block.The configuration structure is clear with good inline documentation explaining both authentication methods and optional settings. The static analysis warnings about key ordering are false positives - alphabetical ordering is not required for environment variables.
packages/shared/src/constants/snowflake.constant.ts (1)
1-57: LGTM! Clean constant definitions with proper immutability.The configuration constants are well-documented and use
as constfor type-level immutability. The timeout and pool values are reasonable defaults for a production Snowflake integration.packages/shared/src/interfaces/snowflake.interface.ts (4)
1-14: LGTM! Well-designed result interface with proper type imports.The interface uses
typeimports from snowflake-sdk and provides generic type safety for query results. The optionalstatementHandleis useful for query tracking.
20-30: LGTM! Query options interface aligns with Snowflake SDK.The interface properly extends SDK capabilities while keeping the surface area minimal. The
fetchAsStringoption is essential for handling large numbers without precision loss.
35-55: LGTM! Comprehensive pool statistics interface.The interface captures all essential pool metrics for monitoring and debugging connection pool behavior.
60-80: LGTM! Well-structured lock statistics interface.The interface provides meaningful metrics for query deduplication monitoring, including the calculated deduplication rate.
docs/architecture/backend/snowflake-integration.md (1)
1-1015: LGTM! Comprehensive and well-structured documentation.The documentation provides excellent coverage of the Snowflake integration architecture, including:
- Clear architectural diagrams and patterns
- Detailed security considerations with multi-layer defense
- Practical code examples that align with the implementation
- Operational guidance for monitoring and troubleshooting
- Future enhancement roadmap for Redis-based locking
This will be valuable for both developers integrating with the service and operators maintaining it.
apps/lfx-one/src/server/services/snowflake.service.ts (4)
30-41: LGTM! Clean service initialization with configurable lock strategy.The constructor properly initializes the lock manager with the configured strategy, defaulting to memory-based locking for single-instance deployments.
51-126: LGTM! Well-structured query execution with comprehensive logging.The execute method properly:
- Validates read-only queries before execution
- Generates deterministic hashes for deduplication
- Uses the lock manager to prevent duplicate query execution
- Provides detailed structured logging for monitoring
The parameterized query approach effectively prevents SQL injection.
200-225: LGTM! Proper lazy initialization with race condition protection.The ensurePool method correctly handles concurrent calls by storing and reusing the poolPromise, preventing multiple pool creation attempts.
231-326: LGTM! Secure dual authentication strategy with comprehensive error handling.The createPool method:
- Supports both containerized (env var) and local development (file-based) authentication
- Properly handles private key formatting with crypto API
- Provides detailed logging for debugging authentication issues
- Falls back gracefully between authentication methods
The pool configuration correctly uses environment variables with defaults from SNOWFLAKE_CONFIG.
apps/lfx-one/src/server/utils/lock-manager.ts (8)
30-44: LGTM! Clean initialization with strategy pattern.The constructor properly initializes the memory-based locking strategy and starts the cleanup interval. The commented Redis scaffolding provides a clear path for future distributed locking.
54-65: LGTM! Strategy pattern provides clean extensibility.The executeLocked method correctly dispatches to the strategy-specific implementation, with clear error handling for unsupported strategies.
75-82: LGTM! Deterministic query hashing with normalization.The hash generation properly normalizes:
- Whitespace (replace multiple spaces with single space)
- Case sensitivity (lowercase)
- Date objects (via normalizeBinds)
This ensures identical queries produce the same hash for effective deduplication.
87-96: LGTM! Clean statistics calculation.The getStats method provides useful metrics including the calculated deduplication rate, with proper handling of the zero-division case.
101-115: LGTM! Proper resource cleanup.The shutdown method correctly clears the cleanup interval, empties the lock map, and logs completion. The commented Redis cleanup shows the pattern for future implementation.
121-170: LGTM! Correct implementation of the fetch-lock pattern.The executeLockedMemory method properly implements query deduplication:
- Returns existing promise for duplicate requests (line 138)
- Creates new promise and stores it for new requests (lines 144-151)
- Tracks hits/misses for metrics
- Cleans up locks with a buffer period after execution
The setTimeout in the finally block is appropriate here - it ensures the lock persists briefly after the query completes to handle racing requests.
210-220: LGTM! Proper bind parameter normalization.The normalizeBinds method correctly converts Date objects to ISO strings for deterministic hashing, ensuring that identical queries with Date parameters are properly deduplicated.
226-244: LGTM! Effective stale lock cleanup.The cleanup mechanism correctly:
- Runs periodically based on SNOWFLAKE_CONFIG.LOCK_CLEANUP_INTERVAL
- Calculates appropriate maxAge including query timeout and buffer
- Logs cleanup actions for debugging
This prevents memory leaks from locks that were never cleaned up due to errors or crashes.
- Add AnalyticsController with three endpoints: * GET /api/analytics/active-weeks-streak (52 weeks of activity data) * GET /api/analytics/pull-requests-merged (last 30 days) * GET /api/analytics/code-commits (last 30 days) - Implement proper error handling: * AuthenticationError (401) for missing user email * ResourceNotFoundError (404) for no analytics data * Consistent error propagation via next(error) - Use SQL window functions for accurate aggregations: * SUM(DAILY_COUNT) OVER () for totals across filtered results * DATEADD(DAY, -30, CURRENT_DATE()) for 30-day filtering - Add analytics data interfaces to shared package: * ActiveWeeksStreakRow/Response * UserPullRequestsRow/Response * UserCodeCommitsRow/Response - Update frontend dashboard with Chart.js visualizations: * Interactive tooltips with formatted dates and counts * Real-time data from Snowflake via analytics service - Update Snowflake integration documentation: * Focus on service architecture and best practices * Add examples for parameterized queries and lazy initialization * Document error handling patterns for callers - Update .env.example with analytics endpoint usage comments LFXV2-671 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Asitha de Silva <[email protected]>
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-129.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (11)
apps/lfx-one/.env.example (2)
47-55: Clarify private key handling; support base64 to avoid multiline env pitfalls.Env var name suggests API key, but Snowflake JWT needs a PEM private key. Recommend documenting that SNOWFLAKE_API_KEY expects PEM, or add SNOWFLAKE_PRIVATE_KEY_BASE64 with decoding in service. Also note passphrase handling for encrypted keys.
-# Method 1: Direct API Key (recommended for containers/Docker) -# SNOWFLAKE_API_KEY=your-private-key-here +# Method 1: Direct Private Key (recommended for containers/Docker) +# If using raw PEM, include literal newlines (\n) or use a multiline env file supported by your orchestrator. +# SNOWFLAKE_API_KEY=-----BEGIN PRIVATE KEY-----\nMIIEvgIBADANBgkq...\n-----END PRIVATE KEY-----\n +# Alternatively (preferred for containers), base64‑encode the PEM and set: +# SNOWFLAKE_PRIVATE_KEY_BASE64=base64-encoded-pemIf you adopt the base64 variant, update SnowflakeService to detect and decode it before passing to the SDK. I can provide the patch.
43-45: Satisfy dotenv-linter key-order warnings (non-functional).Reorder Snowflake keys alphabetically or per your convention to silence UnorderedKey warnings.
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (3)
70-85: Avoid string label matching; use a stable metric key/id.Matching on metric.label is brittle. Add a stable key (e.g., 'activeWeeksStreak' | 'pullRequestsMerged' | 'codeCommits') to constants and switch on it.
Example:
// In constants: add key: 'activeWeeksStreak' | 'pullRequestsMerged' | 'codeCommits' return baseMetrics.map(m => { switch (m.key) { case 'activeWeeksStreak': return this.transformActiveWeeksStreak(activeWeeksData); case 'pullRequestsMerged': return this.transformPullRequestsMerged(pullRequestsData); case 'codeCommits': return this.transformCodeCommits(codeCommitsDataValue); default: return m; } });
30-36: Handle observable errors to prevent UI crashes.toSignal will surface errors by default. Wrap source streams with catchError to return safe fallbacks.
-import { toSignal } from '@angular/core/rxjs-interop'; +import { toSignal } from '@angular/core/rxjs-interop'; +import { catchError, of } from 'rxjs'; // ... - private readonly activeWeeksStreakData = toSignal(this.analyticsService.getActiveWeeksStreak(), { + private readonly activeWeeksStreakData = toSignal( + this.analyticsService.getActiveWeeksStreak().pipe(catchError(() => of({ data: [], currentStreak: 0, totalWeeks: 0 }))), + , { initialValue: { data: [], currentStreak: 0, totalWeeks: 0, }, });Apply similar wrapping for pullRequestsMergedData and codeCommitsData.
Also applies to: 41-47, 52-58
112-123: Remove line-only dataset options from bar chart.Properties like tension and pointRadius are for line charts; they’re ignored on bar charts and add noise.
{ data: chartData.map((row) => row.IS_ACTIVE), borderColor: '#10b981', backgroundColor: 'rgba(16, 185, 129, 0.1)', - fill: true, - tension: 0.4, - borderWidth: 2, - pointRadius: 0, + borderWidth: 2 },packages/shared/src/interfaces/analytics-data.interface.ts (2)
14-18: Encode boolean as a discriminated numeric union.Make IS_ACTIVE explicit (0 | 1) for safer handling at compile time.
- IS_ACTIVE: number; + IS_ACTIVE: 0 | 1;
4-12: Consider introducing camelCase DTOs separate from Snowflake column casing.Keeping uppercase DB column names in shared interfaces couples UI to storage schema. Consider mapping to domain DTOs with camelCase in the controller layer.
Also applies to: 40-60, 81-101
apps/lfx-one/src/app/shared/services/analytics.service.ts (3)
4-7: Type error param and prepare for selective error handling/caching.
- Import HttpErrorResponse (for typed catchError) and, if you adopt the below refactor, throwError/timeout/shareReplay.
-import { HttpClient } from '@angular/common/http'; +import { HttpClient, HttpErrorResponse } from '@angular/common/http'; -import { catchError, Observable, of } from 'rxjs'; +import { catchError, Observable, of, throwError, timeout, shareReplay } from 'rxjs';
17-17: Minor: centralize base API path.Define a basePath to avoid repeating string literals and ease future changes.
export class AnalyticsService { - private readonly http = inject(HttpClient); + private readonly http = inject(HttpClient); + private readonly basePath = '/api/analytics';
24-33: Optional: use a LoggerService instead of console.error.Keeps logs consistent and configurable across environments.
Also applies to: 41-50, 58-67
apps/lfx-one/src/app/shared/services/segment.service.ts (1)
51-55: Queue page/events until analytics is ready to avoid dropsCurrently only identify() is queued. Page and custom events emitted before readiness are lost.
Apply:
@@ private scriptLoaded = false; private analyticsReady = false; private analytics?: LfxSegmentAnalytics; private identifyQueue: Array<{ user: unknown }> = []; + private eventQueue: Array<{ t: 'page' | 'track'; name: string; props?: Record<string, unknown> }> = []; @@ public trackPage(pageName: string, properties?: Record<string, unknown>): void { - if (!this.analyticsReady || !this.analytics) { - return; - } + if (!this.analyticsReady || !this.analytics) { + this.eventQueue.push({ t: 'page', name: pageName, props: properties }); + return; + } @@ public trackEvent(eventName: string, properties?: Record<string, unknown>): void { - if (!this.analyticsReady || !this.analytics) { - return; - } + if (!this.analyticsReady || !this.analytics) { + this.eventQueue.push({ t: 'track', name: eventName, props: properties }); + return; + } @@ await this.analytics.init(); this.analyticsReady = true; // Process any queued identify calls this.processIdentifyQueue(); + // Flush queued events + this.processEventQueue(); return; @@ private processIdentifyQueue(): void { while (this.identifyQueue.length > 0) { const item = this.identifyQueue.shift(); if (item) { this.identifyUser(item.user); } } } + + private processEventQueue(): void { + while (this.eventQueue.length > 0 && this.analytics) { + const ev = this.eventQueue.shift()!; + try { + if (ev.t === 'page') { + this.analytics.page(ev.name, ev.props); + } else { + this.analytics.track(ev.name, ev.props); + } + } catch (error) { + console.error('Error flushing queued Segment event:', error); + } + } + }Also applies to: 68-72, 146-149, 162-173
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/lfx-one/.env.example(1 hunks)apps/lfx-one/angular.json(1 hunks)apps/lfx-one/src/app/app.component.ts(3 hunks)apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts(3 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(1 hunks)apps/lfx-one/src/app/shared/services/segment.service.ts(1 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(1 hunks)apps/lfx-one/src/server/routes/analytics.route.ts(1 hunks)apps/lfx-one/src/server/server.ts(3 hunks)apps/lfx-one/src/server/services/snowflake.service.ts(1 hunks)docs/architecture/backend/snowflake-integration.md(1 hunks)packages/shared/src/interfaces/analytics-data.interface.ts(1 hunks)packages/shared/src/interfaces/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/architecture/backend/snowflake-integration.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/shared/src/interfaces/index.ts
- apps/lfx-one/src/server/services/snowflake.service.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/shared/services/segment.service.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/app.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/controllers/analytics.controller.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/shared/services/segment.service.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/app.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/controllers/analytics.controller.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/shared/services/segment.service.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/app.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/controllers/analytics.controller.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.ts
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all TypeScript interfaces in the shared package at packages/shared/src/interfaces
Files:
packages/shared/src/interfaces/analytics-data.interface.ts
🧬 Code graph analysis (6)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (3)
packages/shared/src/interfaces/components.interface.ts (1)
ProgressItemWithChart(321-330)packages/shared/src/constants/progress-metrics.constants.ts (2)
MAINTAINER_PROGRESS_METRICS(177-358)CORE_DEVELOPER_PROGRESS_METRICS(16-172)packages/shared/src/interfaces/analytics-data.interface.ts (3)
ActiveWeeksStreakResponse(23-38)UserPullRequestsResponse(64-79)UserCodeCommitsResponse(105-120)
apps/lfx-one/src/app/shared/services/segment.service.ts (1)
packages/shared/src/interfaces/segment.interface.ts (2)
LfxSegmentAnalyticsClass(63-66)LfxSegmentAnalytics(46-57)
apps/lfx-one/src/server/routes/analytics.route.ts (1)
apps/lfx-one/src/server/controllers/analytics.controller.ts (1)
AnalyticsController(22-228)
apps/lfx-one/src/app/app.component.ts (1)
packages/shared/src/interfaces/auth.interface.ts (1)
AuthContext(47-52)
apps/lfx-one/src/server/controllers/analytics.controller.ts (2)
apps/lfx-one/src/server/services/snowflake.service.ts (1)
SnowflakeService(25-342)packages/shared/src/interfaces/analytics-data.interface.ts (6)
ActiveWeeksStreakRow(8-18)ActiveWeeksStreakResponse(23-38)UserPullRequestsRow(44-59)UserPullRequestsResponse(64-79)UserCodeCommitsRow(85-100)UserCodeCommitsResponse(105-120)
apps/lfx-one/src/app/shared/services/analytics.service.ts (2)
apps/lfx-one/src/app/shared/services/segment.service.ts (1)
Injectable(23-192)packages/shared/src/interfaces/analytics-data.interface.ts (3)
ActiveWeeksStreakResponse(23-38)UserPullRequestsResponse(64-79)UserCodeCommitsResponse(105-120)
🪛 dotenv-linter (4.0.0)
apps/lfx-one/.env.example
[warning] 43-43: [UnorderedKey] The SNOWFLAKE_DATABASE key should go before the SNOWFLAKE_USERNAME key
(UnorderedKey)
[warning] 45-45: [UnorderedKey] The SNOWFLAKE_ROLE key should go before the SNOWFLAKE_USERNAME key
(UnorderedKey)
⏰ 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). (1)
- GitHub Check: build-and-push
🔇 Additional comments (10)
apps/lfx-one/.env.example (1)
74-77: Ensure NODE_TLS_REJECT_UNAUTHORIZED=0 is never enabled outside local.Keep this only in local/dev example files and CI containers that truly require it; never set in production environments.
apps/lfx-one/src/server/server.ts (2)
28-28: LGTM: analytics router wiring import.
209-209: LGTM: mount analytics routes after auth middleware.Protected under authMiddleware and consistent with other /api routes.
apps/lfx-one/src/server/routes/analytics.route.ts (1)
12-16: LGTM: concise routing with controller delegation.Handlers are thin and rely on controller; safe arrow functions avoid this-binding issues.
apps/lfx-one/src/server/controllers/analytics.controller.ts (2)
29-90: LGTM on parameterization and read-only access pattern.Good use of parameterized binds and explicit 30‑day/52‑week bounds; errors propagate via next(error).
Ensure SnowflakeService sets schema from env to align with Option A in the other comment.
Also applies to: 96-153, 159-216
43-51: I attempted to verify the hard-coded Snowflake database/schema references inapps/lfx-one/src/server/controllers/analytics.controller.ts, but the file is not accessible in the current environment.Manual verification required:
Please confirm:
- File location: Does
apps/lfx-one/src/server/controllers/analytics.controller.tsexist in your repository?- Code snippet accuracy: Are the three hard-coded schema references at the indicated lines exactly as shown in the review comment?
- Line ~43-51:
ANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONE.ACTIVE_WEEKS_STREAK- Line ~112-121:
ANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONE.USER_PULL_REQUESTS- Line ~175-184:
ANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONE.USER_CODE_COMMITS- Environment impact: Do these hard-coded paths break when deployed to non-DEV environments?
Once confirmed, the review comment's recommendations (Option A: use env-based schema configuration, or Option B: read from environment variables) should resolve the multi-environment deployment issue.
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
20-33: Review comment is incomplete; error-handling architecture missing.The problem is valid—all errors including 401s are masked by returning empty objects. However, the suggested refactor assumes error-response interceptors or guards exist to handle rethrown 401s, but none are present in the codebase. Rethrow 401 without an error handler will cause unhandled errors to propagate and potentially break request flows.
Additionally, the
basePathsuggestion doesn't align with codebase patterns—all services use hardcoded URLs (e.g.,'/api/analytics/...'), not base paths.Refactor is sound locally (handle errors, use constants, timeout, shareReplay), but incomplete architecturally. Recommend:
- First establish an error-response interceptor to catch and handle 401s globally
- Then apply the service-level refactoring with rethrow
- Use hardcoded URLs or inject base URL rather than
basePathpropertyPlease confirm:
- Whether 401 errors should be rethrown (requires global error handler first)
- Whether error-response interceptor should be added as prerequisite
- URL pattern preference (hardcoded vs. configuration-based)
apps/lfx-one/src/app/app.component.ts (2)
11-12: SegmentService wiring looks goodImport/injection swap is clean and aligns with service providedIn: 'root'. No issues.
Also applies to: 22-22
29-31: Init timing is SSR-safe via SegmentService.afterNextRenderCalling initialize() here is fine since the service defers work to afterNextRender.
If there’s any chance multiple roots call initialize(), consider idempotence guard in SegmentService (optional).
apps/lfx-one/src/app/shared/services/segment.service.ts (1)
38-44: Clarification needed: No consent management system currently exists in the codebaseThe recommendation to gate analytics on consent and DNT is valid, but the suggested implementation cannot be applied as-is. The code references
(window as any)?.LfxConsent?.analyticswhich does not exist in the codebase. Additionally, no consent storage mechanism (cookie, service, or otherwise) is currently implemented.Before implementing this change, you must:
- Establish a consent management system: Define where user consent will be stored and managed (e.g., cookie, localStorage, consent service, or Auth0 metadata)
- Document the consent property name: Specify what property or mechanism indicates analytics consent in your application
- Update the implementation: Once the consent system is defined, add the DNT/consent checks with the correct property path
The reviewer explicitly requested this verification. Work with your team to confirm the consent strategy before proceeding with the implementation.
- Add Authelia OIDC claims middleware for Auth0 compatibility - Transform standard OIDC claims to Auth0 format - Only activates for non-Auth0 users (issuer check) - Maps preferred_username, splits name into given/family - Modifies user object in-place (not replaceable) - Refactor profile controller to use NATS exclusively - Remove Supabase dependency for user profile data - Construct UserProfile from OIDC token claims - Use NATS as sole authoritative source for metadata - Email management methods still use Supabase - Change user metadata field from 'title' to 'job_title' - Update UserMetadata interface - Update frontend components (profile edit, layout, header) - Update backend validation logic - Ensure consistent field naming across all layers - Clean up debug logging - Remove console.log statement from server.ts - Simplify authelia userinfo fetch logic - Minor improvements - Remove unused 'open' dependency from angular.json - Fix header component formatting - Optimize auth-helper to use OIDC data directly Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-one/src/server/utils/auth-helper.ts (1)
10-10: Remove unnecessaryasyncdeclaration.The function no longer performs any asynchronous operations after removing the Authelia userinfo fetch. Keeping the
asynckeyword creates an unnecessary promise wrapper that impacts performance.Apply this diff:
-export async function getUsernameFromAuth(req: Request): Promise<string | null> { +export function getUsernameFromAuth(req: Request): string | null {Note: Ensure all call sites are updated to remove
awaitwhen calling this function.
♻️ Duplicate comments (1)
apps/lfx-one/src/server/server.ts (1)
164-164: Reduce Auth0 scope: remove update:current_user_metadata unless strictly required.The
update:current_user_metadatascope elevates privileges but is not used by the analytics endpoints introduced in this PR, which are read-only. This violates the principle of least privilege.As noted in a previous review, consider removing this scope:
- scope: 'openid email profile access:api offline_access update:current_user_metadata', + scope: 'openid email profile access:api offline_access',If this scope is genuinely required for other functionality, gate it behind an environment variable and document the specific use case.
🧹 Nitpick comments (6)
apps/lfx-one/src/server/middleware/authelia-claims.middleware.ts (3)
56-58: Avoid defaulting created_at/updated_at to “now”.Filling missing timestamps with now fabricates account/profile times and can mislead auditing and UX. Set only when present (diff above already reflects this).
36-37: Name parsing is naive; trim and split on whitespace, handle single or multi-part names.Minor robustness: trim, split on one-or-more whitespace, ignore empties.
Apply this diff:
- const nameParts = (autheliaUser['name'] || '').split(' '); + const nameParts = String(autheliaUser['name'] ?? '') + .trim() + .split(/\s+/) + .filter(Boolean);Also applies to: 46-53
39-41: Improve type safety for OIDC user mutation instead of casting to any.The cast to
anyat line 40 bypasses type safety. Sinceexpress-openid-connectprovides proper type definitions forreq.oidc(viaRequestContext), augmentExpress.Requestinapps/lfx-one/src/types/express.d.tsto include properly typedoidcproperty, or normalize claims tores.localsinstead of mutating the getter-returned user object in place.apps/lfx-one/src/app/shared/components/header/header.component.html (1)
37-41: Avoid dangling “at” when job_title or organization is missing.Render conditionally so “at” appears only when both exist.
Apply:
- <span class="text-xs text-gray-500" - >{{ userProfile()?.profile?.job_title }} at <span class="text-primary">{{ userProfile()?.profile?.organization }}</span></span - > + <span class="text-xs text-gray-500"> + @if (userProfile()?.profile?.job_title) { {{ userProfile()?.profile?.job_title }} } + @if (userProfile()?.profile?.job_title && userProfile()?.profile?.organization) { <span> at </span> } + @if (userProfile()?.profile?.organization) { <span class="text-primary">{{ userProfile()?.profile?.organization }}</span> } + </span>apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.ts (1)
86-92: Form, submit, and populate all aligned to job_title.All three touchpoints updated consistently. Optional: normalize country when computing isUSA to avoid whitespace/casing mismatches.
Apply:
- public readonly isUSA = computed(() => { - return this.selectedCountrySignal() === 'United States'; - }); + public readonly isUSA = computed(() => this.selectedCountrySignal().trim().toLowerCase() === 'united states');Also applies to: 142-148, 224-230
apps/lfx-one/src/server/controllers/profile.controller.ts (1)
36-49: Use AuthenticationError for missing auth to align semantics.When username/OIDC user is absent, prefer 401 AuthenticationError over ServiceValidationError for consistency and clearer API semantics. Optional but recommended.
Apply:
- const validationError = ServiceValidationError.forField('user_id', 'User authentication required', { + const validationError = new AuthenticationError('User authentication required', { operation: 'get_current_user_profile', service: 'profile_controller', path: req.path, }); ... - const validationError = ServiceValidationError.forField('user_id', 'User authentication data not available', { + const validationError = new AuthenticationError('User authentication data not available', { operation: 'get_current_user_profile', service: 'profile_controller', path: req.path, });If other controllers use AuthenticationError for similar cases, update here for consistency.
Also applies to: 51-64
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
apps/lfx-one/angular.json(1 hunks)apps/lfx-one/src/app/layouts/profile-layout/profile-layout.component.ts(1 hunks)apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.html(1 hunks)apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.ts(3 hunks)apps/lfx-one/src/app/shared/components/header/header.component.html(1 hunks)apps/lfx-one/src/app/shared/components/header/header.component.ts(1 hunks)apps/lfx-one/src/server/controllers/profile.controller.ts(3 hunks)apps/lfx-one/src/server/middleware/authelia-claims.middleware.ts(1 hunks)apps/lfx-one/src/server/server.ts(3 hunks)apps/lfx-one/src/server/services/user.service.ts(1 hunks)apps/lfx-one/src/server/utils/auth-helper.ts(1 hunks)packages/shared/src/interfaces/user-profile.interface.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/lfx-one/src/server/services/user.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/lfx-one/angular.json
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/app/layouts/profile-layout/profile-layout.component.tsapps/lfx-one/src/server/controllers/profile.controller.tspackages/shared/src/interfaces/user-profile.interface.tsapps/lfx-one/src/server/middleware/authelia-claims.middleware.tsapps/lfx-one/src/app/shared/components/header/header.component.tsapps/lfx-one/src/app/modules/profile/edit/profile-edit.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/utils/auth-helper.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/app/layouts/profile-layout/profile-layout.component.tsapps/lfx-one/src/app/shared/components/header/header.component.htmlapps/lfx-one/src/server/controllers/profile.controller.tsapps/lfx-one/src/app/modules/profile/edit/profile-edit.component.htmlpackages/shared/src/interfaces/user-profile.interface.tsapps/lfx-one/src/server/middleware/authelia-claims.middleware.tsapps/lfx-one/src/app/shared/components/header/header.component.tsapps/lfx-one/src/app/modules/profile/edit/profile-edit.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/utils/auth-helper.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/layouts/profile-layout/profile-layout.component.tsapps/lfx-one/src/server/controllers/profile.controller.tspackages/shared/src/interfaces/user-profile.interface.tsapps/lfx-one/src/server/middleware/authelia-claims.middleware.tsapps/lfx-one/src/app/shared/components/header/header.component.tsapps/lfx-one/src/app/modules/profile/edit/profile-edit.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/utils/auth-helper.ts
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/shared/components/header/header.component.htmlapps/lfx-one/src/app/modules/profile/edit/profile-edit.component.html
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all TypeScript interfaces in the shared package at packages/shared/src/interfaces
Files:
packages/shared/src/interfaces/user-profile.interface.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/server/controllers/profile.controller.ts (3)
apps/lfx-one/src/server/utils/auth-helper.ts (1)
getUsernameFromAuth(10-22)apps/lfx-one/src/server/errors/index.ts (1)
ServiceValidationError(7-7)packages/shared/src/interfaces/user-profile.interface.ts (3)
UserMetadata(118-133)UserProfile(4-12)CombinedProfile(17-20)
🔇 Additional comments (9)
apps/lfx-one/src/server/server.ts (2)
20-20: LGTM: Import follows established pattern.The import statement is clean and consistent with other router imports in this file.
209-209: LGTM: Router mounting is correctly positioned.The analytics router is properly mounted after authentication middleware and follows the established pattern for protected API routes.
apps/lfx-one/src/server/utils/auth-helper.ts (1)
16-16: Preferred_username extraction is properly supported.Verification confirms that:
- Authelia includes
preferred_usernameas a standard OIDC claim in its UserInfo response- The
authelia-claims.middleware.tsproperly transforms and preserves Authelia OIDC claims before this function is called, with fallback tosubifpreferred_usernameis unavailable- The function uses safe navigation (
?.) and returnsnullif the claim is missing, ensuring safe behaviorThe implementation is sound and tested through the existing middleware integration.
apps/lfx-one/src/server/middleware/authelia-claims.middleware.ts (3)
1-2: License header: LGTMMIT header present and compliant.
64-65: Flow control: LGTMAlways calls next(); safe no-op when not authenticated.
56-61: Remove theuser.sidassignment; 'sid' is a reserved OIDC claim and should not be attached to user claims.The security concern is valid: attaching server session IDs to
req.oidc.userviolates OpenID Connect specification (where 'sid' is reserved for the OP). While no direct serialization ofreq.oidc.userto responses or logs was found in the current codebase, the object is modified in place and could be accessed downstream, creating a defensive programming risk.Remove
user.sid = (req as any).sessionID || '';or store it separately inres.localsif absolutely required.The suggested style changes to
updated_atandcreated_at(usingifstatements instead of||defaults) are unrelated to the security issue and should be evaluated separately.apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.html (1)
96-106: Field rename aligned to shared interfaces.Binding to control="job_title" is consistent with the new UserMetadata.job_title. No issues.
apps/lfx-one/src/app/layouts/profile-layout/profile-layout.component.ts (1)
143-150: Subtitle source updated to job_title — looks good.Change is consistent with the field rename and null-safe.
packages/shared/src/interfaces/user-profile.interface.ts (1)
118-125: No verification issues found; migration is complete.The search confirmed that all references to user profile metadata already use
job_titleacross services, forms, and components. The singlecontrol="title"match is for meeting title (unrelated context). No legacytitlebindings tied to user metadata exist in the codebase.
apps/lfx-one/src/server/middleware/authelia-claims.middleware.ts
Outdated
Show resolved
Hide resolved
- Set Snowflake SDK log level to ERROR by default - Add SNOWFLAKE_LOG_LEVEL environment variable support - Prevent snowflake.log from being committed to repository Signed-off-by: Asitha de Silva <[email protected]>
Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.gitignore(1 hunks)apps/lfx-one/.gitignore(1 hunks)apps/lfx-one/src/server/services/snowflake.service.ts(1 hunks)apps/lfx-one/src/server/services/user.service.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/lfx-one/.gitignore
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/server/services/snowflake.service.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/server/services/snowflake.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/server/services/snowflake.service.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/server/services/snowflake.service.ts (3)
apps/lfx-one/src/server/utils/lock-manager.ts (1)
LockManager(20-245)packages/shared/src/interfaces/snowflake.interface.ts (4)
SnowflakeQueryOptions(20-30)SnowflakeQueryResult(10-14)SnowflakePoolStats(35-55)LockStats(60-80)packages/shared/src/constants/snowflake.constant.ts (1)
SNOWFLAKE_CONFIG(7-57)
⏰ 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). (1)
- GitHub Check: build-and-push
🔇 Additional comments (8)
apps/lfx-one/src/server/services/user.service.ts (1)
208-210: Field rename verified—all references updated correctly.The change from
metadata?.titletometadata?.job_titleis confirmed complete. The UserMetadata interface (packages/shared/src/interfaces/user-profile.interface.ts:122) has been properly updated with the new field, and no orphaned references to the oldtitlefield remain in the codebase.apps/lfx-one/src/server/services/snowflake.service.ts (7)
1-13: LGTM: License header and imports are properly structured.The required license header is present, and all imports are well-organized with clear separation between external dependencies, shared package imports, and local imports.
30-47: LGTM: Constructor properly initializes service components.The constructor correctly configures the Snowflake SDK logging, initializes the lock manager with the appropriate strategy, and provides useful initialization logging.
86-132: LGTM: Query execution and error handling are well implemented.The query execution logic properly wraps the Snowflake SDK callback in a Promise, includes comprehensive logging with query hash and duration metrics, and handles both success and error cases appropriately.
178-201: LGTM: Shutdown logic correctly implements timeout handling.The previous review concern about the timeout mechanism has been properly addressed. The code now uses
Promise.raceto correctly race the drain operation against a 30-second timeout, ensuring the promise either resolves when drain completes or rejects if it takes too long.
207-232: LGTM: Lazy initialization with proper thread safety.The
ensurePoolmethod correctly implements lazy initialization with thread safety by using a poolPromise to prevent concurrent pool creation attempts. Error handling properly resets the promise on failure.
304-347: LGTM: Read-only validation properly addresses CTE write operations.The previous concern about CTEs potentially containing write operations has been addressed. The validation now checks for write patterns anywhere in the query (line 307), not just at the start, using word boundaries to minimize false positives while ensuring comprehensive protection.
The addition of
EXECUTEandCALLpatterns (lines 320-321) further strengthens the validation.Note: While this approach may produce false positives (e.g., blocking queries with keywords in comments or string literals), this is acceptable for a read-only service where security is paramount. Database-level permissions should provide additional defense-in-depth.
150-166: Remove the type assertion suggestion—pool properties are already properly typed.The Pool class from generic-pool (the underlying pooling library) officially exposes
size,available,borrowed, andpendingas typed properties. The code insnowflake.service.tsat lines 161–164 correctly accesses these properties without needing anas anycast. The Snowflake SDK'sPooltype includes these properties, so no type safety issue exists.
| ACTIVITY_DATE, | ||
| DAILY_COUNT, | ||
| SUM(DAILY_COUNT) OVER () as TOTAL_COUNT | ||
| FROM ANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONE.USER_CODE_COMMITS |
There was a problem hiding this comment.
This one is already merged in Snowflake (ANALYTICS.PLATINUM_LFX_ONE.USER_CODE_COMMITS) — not sure if Jordan’s version is different.
🧹 Deployment RemovedThe deployment for PR #129 has been removed. |
* feat(backend): add snowflake data warehouse integration (LFXV2-671) - Add SnowflakeService with connection pooling, query execution, and caching - Implement LockManager for distributed operations and resource coordination - Add Snowflake-related interfaces, enums, and constants to shared package - Add comprehensive architecture documentation for Snowflake integration - Configure environment variables for Snowflake connection - Add @snowflake-sdk/client dependency This integration enables server-side data warehouse queries with proper connection management, query optimization, and distributed locking. Signed-off-by: Asitha de Silva <[email protected]> * feat(analytics): implement snowflake integration with error handling - Add AnalyticsController with three endpoints: * GET /api/analytics/active-weeks-streak (52 weeks of activity data) * GET /api/analytics/pull-requests-merged (last 30 days) * GET /api/analytics/code-commits (last 30 days) - Implement proper error handling: * AuthenticationError (401) for missing user email * ResourceNotFoundError (404) for no analytics data * Consistent error propagation via next(error) - Use SQL window functions for accurate aggregations: * SUM(DAILY_COUNT) OVER () for totals across filtered results * DATEADD(DAY, -30, CURRENT_DATE()) for 30-day filtering - Add analytics data interfaces to shared package: * ActiveWeeksStreakRow/Response * UserPullRequestsRow/Response * UserCodeCommitsRow/Response - Update frontend dashboard with Chart.js visualizations: * Interactive tooltips with formatted dates and counts * Real-time data from Snowflake via analytics service - Update Snowflake integration documentation: * Focus on service architecture and best practices * Add examples for parameterized queries and lazy initialization * Document error handling patterns for callers - Update .env.example with analytics endpoint usage comments LFXV2-671 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Asitha de Silva <[email protected]> * feat(auth): add authelia support and refactor profile to use nats - Add Authelia OIDC claims middleware for Auth0 compatibility - Transform standard OIDC claims to Auth0 format - Only activates for non-Auth0 users (issuer check) - Maps preferred_username, splits name into given/family - Modifies user object in-place (not replaceable) - Refactor profile controller to use NATS exclusively - Remove Supabase dependency for user profile data - Construct UserProfile from OIDC token claims - Use NATS as sole authoritative source for metadata - Email management methods still use Supabase - Change user metadata field from 'title' to 'job_title' - Update UserMetadata interface - Update frontend components (profile edit, layout, header) - Update backend validation logic - Ensure consistent field naming across all layers - Clean up debug logging - Remove console.log statement from server.ts - Simplify authelia userinfo fetch logic - Minor improvements - Remove unused 'open' dependency from angular.json - Fix header component formatting - Optimize auth-helper to use OIDC data directly Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]> * build: configure snowflake sdk logging and add to gitignore - Set Snowflake SDK log level to ERROR by default - Add SNOWFLAKE_LOG_LEVEL environment variable support - Prevent snowflake.log from being committed to repository Signed-off-by: Asitha de Silva <[email protected]> * fix(server): remove middleware Signed-off-by: Asitha de Silva <[email protected]> --------- Signed-off-by: Asitha de Silva <[email protected]> Co-authored-by: Claude <[email protected]>
* feat(backend): add snowflake data warehouse integration (LFXV2-671) - Add SnowflakeService with connection pooling, query execution, and caching - Implement LockManager for distributed operations and resource coordination - Add Snowflake-related interfaces, enums, and constants to shared package - Add comprehensive architecture documentation for Snowflake integration - Configure environment variables for Snowflake connection - Add @snowflake-sdk/client dependency This integration enables server-side data warehouse queries with proper connection management, query optimization, and distributed locking. Signed-off-by: Asitha de Silva <[email protected]> * feat(analytics): implement snowflake integration with error handling - Add AnalyticsController with three endpoints: * GET /api/analytics/active-weeks-streak (52 weeks of activity data) * GET /api/analytics/pull-requests-merged (last 30 days) * GET /api/analytics/code-commits (last 30 days) - Implement proper error handling: * AuthenticationError (401) for missing user email * ResourceNotFoundError (404) for no analytics data * Consistent error propagation via next(error) - Use SQL window functions for accurate aggregations: * SUM(DAILY_COUNT) OVER () for totals across filtered results * DATEADD(DAY, -30, CURRENT_DATE()) for 30-day filtering - Add analytics data interfaces to shared package: * ActiveWeeksStreakRow/Response * UserPullRequestsRow/Response * UserCodeCommitsRow/Response - Update frontend dashboard with Chart.js visualizations: * Interactive tooltips with formatted dates and counts * Real-time data from Snowflake via analytics service - Update Snowflake integration documentation: * Focus on service architecture and best practices * Add examples for parameterized queries and lazy initialization * Document error handling patterns for callers - Update .env.example with analytics endpoint usage comments LFXV2-671 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Asitha de Silva <[email protected]> * feat(auth): add authelia support and refactor profile to use nats - Add Authelia OIDC claims middleware for Auth0 compatibility - Transform standard OIDC claims to Auth0 format - Only activates for non-Auth0 users (issuer check) - Maps preferred_username, splits name into given/family - Modifies user object in-place (not replaceable) - Refactor profile controller to use NATS exclusively - Remove Supabase dependency for user profile data - Construct UserProfile from OIDC token claims - Use NATS as sole authoritative source for metadata - Email management methods still use Supabase - Change user metadata field from 'title' to 'job_title' - Update UserMetadata interface - Update frontend components (profile edit, layout, header) - Update backend validation logic - Ensure consistent field naming across all layers - Clean up debug logging - Remove console.log statement from server.ts - Simplify authelia userinfo fetch logic - Minor improvements - Remove unused 'open' dependency from angular.json - Fix header component formatting - Optimize auth-helper to use OIDC data directly Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]> * build: configure snowflake sdk logging and add to gitignore - Set Snowflake SDK log level to ERROR by default - Add SNOWFLAKE_LOG_LEVEL environment variable support - Prevent snowflake.log from being committed to repository Signed-off-by: Asitha de Silva <[email protected]> * fix(server): remove middleware Signed-off-by: Asitha de Silva <[email protected]> --------- Signed-off-by: Asitha de Silva <[email protected]> Co-authored-by: Claude <[email protected]>
* feat(backend): add snowflake data warehouse integration (LFXV2-671) - Add SnowflakeService with connection pooling, query execution, and caching - Implement LockManager for distributed operations and resource coordination - Add Snowflake-related interfaces, enums, and constants to shared package - Add comprehensive architecture documentation for Snowflake integration - Configure environment variables for Snowflake connection - Add @snowflake-sdk/client dependency This integration enables server-side data warehouse queries with proper connection management, query optimization, and distributed locking. Signed-off-by: Asitha de Silva <[email protected]> * feat(analytics): implement snowflake integration with error handling - Add AnalyticsController with three endpoints: * GET /api/analytics/active-weeks-streak (52 weeks of activity data) * GET /api/analytics/pull-requests-merged (last 30 days) * GET /api/analytics/code-commits (last 30 days) - Implement proper error handling: * AuthenticationError (401) for missing user email * ResourceNotFoundError (404) for no analytics data * Consistent error propagation via next(error) - Use SQL window functions for accurate aggregations: * SUM(DAILY_COUNT) OVER () for totals across filtered results * DATEADD(DAY, -30, CURRENT_DATE()) for 30-day filtering - Add analytics data interfaces to shared package: * ActiveWeeksStreakRow/Response * UserPullRequestsRow/Response * UserCodeCommitsRow/Response - Update frontend dashboard with Chart.js visualizations: * Interactive tooltips with formatted dates and counts * Real-time data from Snowflake via analytics service - Update Snowflake integration documentation: * Focus on service architecture and best practices * Add examples for parameterized queries and lazy initialization * Document error handling patterns for callers - Update .env.example with analytics endpoint usage comments LFXV2-671 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Asitha de Silva <[email protected]> * feat(auth): add authelia support and refactor profile to use nats - Add Authelia OIDC claims middleware for Auth0 compatibility - Transform standard OIDC claims to Auth0 format - Only activates for non-Auth0 users (issuer check) - Maps preferred_username, splits name into given/family - Modifies user object in-place (not replaceable) - Refactor profile controller to use NATS exclusively - Remove Supabase dependency for user profile data - Construct UserProfile from OIDC token claims - Use NATS as sole authoritative source for metadata - Email management methods still use Supabase - Change user metadata field from 'title' to 'job_title' - Update UserMetadata interface - Update frontend components (profile edit, layout, header) - Update backend validation logic - Ensure consistent field naming across all layers - Clean up debug logging - Remove console.log statement from server.ts - Simplify authelia userinfo fetch logic - Minor improvements - Remove unused 'open' dependency from angular.json - Fix header component formatting - Optimize auth-helper to use OIDC data directly Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]> * build: configure snowflake sdk logging and add to gitignore - Set Snowflake SDK log level to ERROR by default - Add SNOWFLAKE_LOG_LEVEL environment variable support - Prevent snowflake.log from being committed to repository Signed-off-by: Asitha de Silva <[email protected]> * fix(server): remove middleware Signed-off-by: Asitha de Silva <[email protected]> --------- Signed-off-by: Asitha de Silva <[email protected]> Co-authored-by: Claude <[email protected]>
* feat(backend): add snowflake data warehouse integration (LFXV2-671) - Add SnowflakeService with connection pooling, query execution, and caching - Implement LockManager for distributed operations and resource coordination - Add Snowflake-related interfaces, enums, and constants to shared package - Add comprehensive architecture documentation for Snowflake integration - Configure environment variables for Snowflake connection - Add @snowflake-sdk/client dependency This integration enables server-side data warehouse queries with proper connection management, query optimization, and distributed locking. Signed-off-by: Asitha de Silva <[email protected]> * feat(analytics): implement snowflake integration with error handling - Add AnalyticsController with three endpoints: * GET /api/analytics/active-weeks-streak (52 weeks of activity data) * GET /api/analytics/pull-requests-merged (last 30 days) * GET /api/analytics/code-commits (last 30 days) - Implement proper error handling: * AuthenticationError (401) for missing user email * ResourceNotFoundError (404) for no analytics data * Consistent error propagation via next(error) - Use SQL window functions for accurate aggregations: * SUM(DAILY_COUNT) OVER () for totals across filtered results * DATEADD(DAY, -30, CURRENT_DATE()) for 30-day filtering - Add analytics data interfaces to shared package: * ActiveWeeksStreakRow/Response * UserPullRequestsRow/Response * UserCodeCommitsRow/Response - Update frontend dashboard with Chart.js visualizations: * Interactive tooltips with formatted dates and counts * Real-time data from Snowflake via analytics service - Update Snowflake integration documentation: * Focus on service architecture and best practices * Add examples for parameterized queries and lazy initialization * Document error handling patterns for callers - Update .env.example with analytics endpoint usage comments LFXV2-671 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Asitha de Silva <[email protected]> * feat(auth): add authelia support and refactor profile to use nats - Add Authelia OIDC claims middleware for Auth0 compatibility - Transform standard OIDC claims to Auth0 format - Only activates for non-Auth0 users (issuer check) - Maps preferred_username, splits name into given/family - Modifies user object in-place (not replaceable) - Refactor profile controller to use NATS exclusively - Remove Supabase dependency for user profile data - Construct UserProfile from OIDC token claims - Use NATS as sole authoritative source for metadata - Email management methods still use Supabase - Change user metadata field from 'title' to 'job_title' - Update UserMetadata interface - Update frontend components (profile edit, layout, header) - Update backend validation logic - Ensure consistent field naming across all layers - Clean up debug logging - Remove console.log statement from server.ts - Simplify authelia userinfo fetch logic - Minor improvements - Remove unused 'open' dependency from angular.json - Fix header component formatting - Optimize auth-helper to use OIDC data directly Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]> * build: configure snowflake sdk logging and add to gitignore - Set Snowflake SDK log level to ERROR by default - Add SNOWFLAKE_LOG_LEVEL environment variable support - Prevent snowflake.log from being committed to repository Signed-off-by: Asitha de Silva <[email protected]> * fix(server): remove middleware Signed-off-by: Asitha de Silva <[email protected]> --------- Signed-off-by: Asitha de Silva <[email protected]> Co-authored-by: Claude <[email protected]>
* feat(backend): add snowflake data warehouse integration (LFXV2-671) - Add SnowflakeService with connection pooling, query execution, and caching - Implement LockManager for distributed operations and resource coordination - Add Snowflake-related interfaces, enums, and constants to shared package - Add comprehensive architecture documentation for Snowflake integration - Configure environment variables for Snowflake connection - Add @snowflake-sdk/client dependency This integration enables server-side data warehouse queries with proper connection management, query optimization, and distributed locking. Signed-off-by: Asitha de Silva <[email protected]> * feat(analytics): implement snowflake integration with error handling - Add AnalyticsController with three endpoints: * GET /api/analytics/active-weeks-streak (52 weeks of activity data) * GET /api/analytics/pull-requests-merged (last 30 days) * GET /api/analytics/code-commits (last 30 days) - Implement proper error handling: * AuthenticationError (401) for missing user email * ResourceNotFoundError (404) for no analytics data * Consistent error propagation via next(error) - Use SQL window functions for accurate aggregations: * SUM(DAILY_COUNT) OVER () for totals across filtered results * DATEADD(DAY, -30, CURRENT_DATE()) for 30-day filtering - Add analytics data interfaces to shared package: * ActiveWeeksStreakRow/Response * UserPullRequestsRow/Response * UserCodeCommitsRow/Response - Update frontend dashboard with Chart.js visualizations: * Interactive tooltips with formatted dates and counts * Real-time data from Snowflake via analytics service - Update Snowflake integration documentation: * Focus on service architecture and best practices * Add examples for parameterized queries and lazy initialization * Document error handling patterns for callers - Update .env.example with analytics endpoint usage comments LFXV2-671 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Asitha de Silva <[email protected]> * feat(auth): add authelia support and refactor profile to use nats - Add Authelia OIDC claims middleware for Auth0 compatibility - Transform standard OIDC claims to Auth0 format - Only activates for non-Auth0 users (issuer check) - Maps preferred_username, splits name into given/family - Modifies user object in-place (not replaceable) - Refactor profile controller to use NATS exclusively - Remove Supabase dependency for user profile data - Construct UserProfile from OIDC token claims - Use NATS as sole authoritative source for metadata - Email management methods still use Supabase - Change user metadata field from 'title' to 'job_title' - Update UserMetadata interface - Update frontend components (profile edit, layout, header) - Update backend validation logic - Ensure consistent field naming across all layers - Clean up debug logging - Remove console.log statement from server.ts - Simplify authelia userinfo fetch logic - Minor improvements - Remove unused 'open' dependency from angular.json - Fix header component formatting - Optimize auth-helper to use OIDC data directly Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]> * build: configure snowflake sdk logging and add to gitignore - Set Snowflake SDK log level to ERROR by default - Add SNOWFLAKE_LOG_LEVEL environment variable support - Prevent snowflake.log from being committed to repository Signed-off-by: Asitha de Silva <[email protected]> * fix(server): remove middleware Signed-off-by: Asitha de Silva <[email protected]> --------- Signed-off-by: Asitha de Silva <[email protected]> Co-authored-by: Claude <[email protected]> Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
2 participants
Summary
This PR adds Snowflake data warehouse integration for analytics data retrieval and implements Authelia authentication support with NATS-based profile management.
Changes
Snowflake Integration
Authentication & Authorization
/meetingand/public/apiendpointsProfile Management Refactoring
job_titlefield across frontend and backend (reverted fromtitle)job_title:Environment Configuration
Testing
Dependencies
Deployment Notes
Generated with Claude Code