feat(dashboards): add data copilot integration #167
Conversation
Add self-contained Data Copilot component with drawer UI for AI-powered data insights. Component integrates with board member dashboard and automatically pulls organization/project context. - Create DataCopilotComponent with PrimeNG drawer - Implement signal-based viewChild and SSR-safe iframe creation - Add responsive drawer (full width mobile, 3/4 width desktop) - Integrate with board member dashboard - Auto-inject organization/project from context services - Add iframe cleanup on drawer close LFXV2-768 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
|
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 a new standalone DataCopilotComponent, inserts it into the Board Member Dashboard template and component imports, adjusts the dashboard header layout, updates the copilot trigger label, and implements lazy creation/destruction of an iframe populated with org/project context in a right-side drawer. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Dashboard as BoardMemberDashboard
participant DataCopilot as DataCopilotComponent
participant Drawer as Drawer UI
participant Iframe as iframe Element
User->>Dashboard: Load dashboard
Dashboard->>DataCopilot: Render component
DataCopilot->>DataCopilot: Resolve org/project context
User->>DataCopilot: Click "Ask LFX Lens"
DataCopilot->>Drawer: visible = true
Drawer->>Drawer: Open
rect `#E6F2FF`
DataCopilot->>DataCopilot: afterNextRender -> createIframe()
DataCopilot->>Iframe: append iframe with src(context)
end
Drawer-->>User: Show iframe content
User->>Drawer: Close
Drawer->>DataCopilot: onHide()
DataCopilot->>DataCopilot: destroyIframe()
DataCopilot->>Drawer: visible = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR adds a Data Copilot AI assistant feature to the board member dashboard by implementing a self-contained component with PrimeNG drawer integration and iframe-based embedding.
- Implements a new DataCopilotComponent with signal-based state management and SSR-safe iframe creation
- Integrates the Data Copilot button into the board member dashboard header
- Uses Angular 19 patterns including signal-based viewChild and computed signals for reactive context injection
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts |
New DataCopilot component with drawer state management, iframe lifecycle handling, and automatic organization/project context injection |
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html |
Component template with trigger button and PrimeNG drawer containing iframe container |
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.scss |
Styles for iframe container with flexbox layout |
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts |
Imports DataCopilotComponent for use in dashboard |
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html |
Adds DataCopilot button to dashboard header with updated layout |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts (2)
101-101: Consider making the iframe URL configurable.The iframe URL is hardcoded to
https://lfx-data-copilot.onrender.com/embed. For better maintainability and environment flexibility, consider moving this to an environment configuration or injectable config service.// In environment.ts or config service export const environment = { dataCopilotUrl: 'https://lfx-data-copilot.onrender.com/embed', // ... }; // In component import { environment } from '@env/environment'; // ... iframe.src = `${environment.dataCopilotUrl}?${params.toString()}`;
87-89: Silent failure when container is unavailable.The guard returns silently if the container is not available. While this prevents errors, it could lead to a confusing user experience where the drawer opens but remains empty with no indication of why.
Consider adding console logging or error handling:
private createIframe(): void { const container = this.iframeContainer(); if (this.iframeCreated || !container?.nativeElement) { + if (!container?.nativeElement) { + console.warn('DataCopilot: iframe container not available'); + } return; }
📜 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 (5)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts(2 hunks)apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html(1 hunks)apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.scss(1 hunks)apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts (1)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
Component(21-94)
⏰ 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: Agent
🔇 Additional comments (6)
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.scss (1)
4-10: LGTM!The iframe container styling is appropriate and minimal. The full width/height with flex layout and hidden overflow will properly contain the embedded iframe.
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html (1)
7-10: LGTM!The Data Copilot component is cleanly integrated into the dashboard header. The
justify-betweenclass properly spaces the foundation title and the copilot button.apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html (1)
5-11: LGTM!The button configuration is clear and appropriate. The icon, label, and size work well for the intended use case.
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
7-7: LGTM!The DataCopilotComponent is properly imported and registered in the component's imports array. The integration follows Angular 19 standalone component patterns correctly.
Also applies to: 23-31
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts (2)
18-33: LGTM!The component properties are well-structured. The signal-based viewChild, computed context values, and visibility signal follow Angular 19 patterns correctly. The optional chaining in later code properly handles cases where the container may not be available.
71-80: LGTM!The iframe cleanup logic is correct. It removes all child nodes and resets the tracking flag, properly managing resources when the drawer closes.
However, verify that component destruction is also handled. If the component is destroyed while the drawer is open, the iframe should also be cleaned up:
export class DataCopilotComponent implements OnDestroy { // ... existing code ... public ngOnDestroy(): void { this.destroyIframe(); } }
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts
Show resolved
Hide resolved
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-167.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
- Remove unused afterNextRender constructor logic that never executed - Replace fragile setTimeout with drawer onShow lifecycle event - Fix redundant state updates by using one-way binding with explicit event handlers - Add null-safe navigation to computed signals for account/project data LFXV2-768 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
Signed-off-by: Asitha de Silva <[email protected]>
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: 1
♻️ Duplicate comments (2)
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts (2)
86-111: Critical: Missing requiredapiKeyparameter.The PR objectives explicitly state: "Require an apiKey input parameter; construct iframe URL with query parameters: apiKey, organization_id, organization_name, project_slug, project_name."
The
URLSearchParamsconstruction (lines 95-100) omits the requiredapiKeyparameter, and there's no@Input()to receive it.Add the apiKey input and include it in the URL:
+ @Input({ required: true }) public apiKey!: string; private createIframe(): void { const container = this.iframeContainer(); if (this.iframeCreated || !container?.nativeElement) { return; } const iframe = document.createElement('iframe'); // Construct the iframe src with query parameters const params = new URLSearchParams({ + apiKey: this.apiKey, organization_id: this.organizationId(), organization_name: this.organizationName(), project_slug: this.projectSlug(), project_name: this.projectName(), }); iframe.src = `https://lfx-data-copilot.onrender.com/embed?${params.toString()}`;Then update the board-member dashboard template to bind the apiKey:
<lfx-data-copilot [apiKey]="yourApiKeyValue"></lfx-data-copilot>
102-106: Critical: Add iframe sandbox security restrictions.The iframe embeds external content from
https://lfx-data-copilot.onrender.com/embedwithout any security restrictions. This poses security risks such as:
- Unrestricted script execution
- Access to same-origin resources
- Form submission capabilities
- Top-level navigation
Add a
sandboxattribute with minimal required permissions:iframe.src = `https://lfx-data-copilot.onrender.com/embed?${params.toString()}`; iframe.width = '100%'; iframe.height = '100%'; iframe.style.border = 'none'; iframe.title = 'LFX Data Copilot'; + iframe.setAttribute('sandbox', 'allow-scripts allow-same-origin');Adjust the sandbox permissions based on the actual requirements of the embedded content. Consider adding:
allow-formsif form submission is neededallow-popupsif the iframe needs to open new windows- Review and restrict to the minimum necessary permissions
🧹 Nitpick comments (3)
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts (3)
50-57: Remove redundantvisible.set(true)call.Line 51 is unnecessary because the
onShowcallback only fires after the drawer is already visible (i.e., whenvisibleis alreadytrue). The drawer state is managed by PrimeNG, and this explicit set is redundant.Apply this diff:
protected onShow(): void { - this.visible.set(true); - // Create iframe when drawer is fully visible if (!this.iframeCreated) { this.createIframe(); } }
62-67: Consider removing redundantvisible.set(false)call.Similar to
onShow, line 63 may be redundant sinceonHidefires when the drawer is already hidden. However, setting it explicitly ensures signal consistency if other code depends on thevisiblesignal state.
102-102: Consider using environment configuration for the iframe URL.The iframe URL
https://lfx-data-copilot.onrender.com/embedis hardcoded, making it difficult to use different endpoints for development, staging, and production environments.Move the URL to an environment configuration file:
In
environment.ts:export const environment = { production: false, dataCopilotUrl: 'https://lfx-data-copilot.onrender.com/embed' };Then inject and use it:
private readonly environment = inject(ENVIRONMENT); // or however environments are injected iframe.src = `${this.environment.dataCopilotUrl}?${params.toString()}`;
📜 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 (2)
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html(1 hunks)apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html
🧰 Additional context used
🧬 Code graph analysis (1)
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts (1)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
Component(21-94)
⏰ 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 (3)
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts (3)
43-45: LGTM: Clean separation of concerns.The
openDrawermethod properly delegates to the drawer's event system. Iframe creation is correctly handled in theonShowcallback (lines 50-57), eliminating the race condition flagged in previous reviews.
72-81: LGTM: Proper iframe cleanup.The cleanup logic correctly removes all child nodes and resets the
iframeCreatedflag, ensuring proper resource management when the drawer closes.
26-26: Feature flag is properly used.The
isLfxLensEnabledfield is used in the template (data-copilot.component.html:5) to conditionally render content with@if (isLfxLensEnabled()). No issues found.
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.ts
Show resolved
Hide resolved
🧹 Deployment RemovedThe deployment for PR #167 has been removed. |
Summary
Implementation Details
Related
JIRA: LFXV2-768
Generated with Claude Code