-
Notifications
You must be signed in to change notification settings - Fork 714
Stage #9257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fix: agent window management and browser url tracking
* feat(activity-log): add sanitizeEntityForActivityLog helper for SQLite Add a new helper function that sanitizes entity data before activity logging to prevent SQLite's 'Too many parameter values' error (SQLITE_MAX_VARIABLE_NUMBER = 999). For SQLite/BetterSQLite3 only: - Replaces relation arrays (members, teams, etc.) with ID arrays - Keeps only primitive values and dates - Skips nested relation objects For PostgreSQL/MySQL: Returns the original entity unchanged. Fixes #9243 * fix(activity-log): sanitize data before logging to prevent SQLite error Apply sanitizeEntityForActivityLog to entity data in logActivity method before publishing the ActivityLogEvent. This prevents the 'RangeError: Too many parameter values' error when creating/updating tasks with multiple members/teams on BetterSQLite3. Fixes #9243 * feat(activity-log): add serializeActivityLogForSqlite helper function Add helper function to serialize JSON fields (data, updatedFields, previousValues, updatedValues, previousEntities, updatedEntities) to strings before database insert. This prevents SQLite's 'Too many parameter values' error (SQLITE_MAX_VARIABLE_NUMBER = 999) when creating activity logs with large nested objects. Fixes #9243 * fix(activity-log): serialize JSON fields before insert for SQLite Use serializeActivityLogForSqlite helper in create() method to stringify JSON fields before passing to TypeORM for SQLite/BetterSQLite3 databases. This fixes the 'RangeError: Too many parameter values were provided' error when creating tasks with multiple members/teams. Fixes #9243 * fix(activity-log): add updatedFields to SQLite deserialization Add missing 'updatedFields' to afterEntityLoad deserialization to ensure the field is properly parsed from JSON string when reading from SQLite. Fixes #9243
* feat: created the employee recent visits module, service and events * feat: get the employee recent visits based on options * fix: updated the employee recent visits module * feat: register recent visits for projects and tasks * fix: deepscan issue * fix: store all the employee visits and add return filters * fix: AI suggestions * fix: AI suggestions * fix: registered the employee recent visits module * fix: refactored the existing check using repository * fix: improved dto and service filters
Bumps [jws](https://github.com/brianloveswords/node-jws) from 3.2.2 to 3.2.3. - [Release notes](https://github.com/brianloveswords/node-jws/releases) - [Changelog](https://github.com/auth0/node-jws/blob/master/CHANGELOG.md) - [Commits](auth0/node-jws@v3.2.2...v3.2.3) --- updated-dependencies: - dependency-name: jws dependency-version: 3.2.3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [jws](https://github.com/brianloveswords/node-jws) from 3.2.2 to 3.2.3. - [Release notes](https://github.com/brianloveswords/node-jws/releases) - [Changelog](https://github.com/auth0/node-jws/blob/master/CHANGELOG.md) - [Commits](auth0/node-jws@v3.2.2...v3.2.3) --- updated-dependencies: - dependency-name: jws dependency-version: 3.2.3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [nodemailer](https://github.com/nodemailer/nodemailer) from 7.0.9 to 7.0.11. - [Release notes](https://github.com/nodemailer/nodemailer/releases) - [Changelog](https://github.com/nodemailer/nodemailer/blob/master/CHANGELOG.md) - [Commits](nodemailer/nodemailer@v7.0.9...v7.0.11) --- updated-dependencies: - dependency-name: nodemailer dependency-version: 7.0.11 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
…ler-7.0.11 chore(deps): bump nodemailer from 7.0.9 to 7.0.11
* feat(table-filters): add InviteStatusFilterComponent for invite status dropdown * feat(invites): add smart table filters for email, fullName, expireDate, and status columns * feat(invite): add backend support for LIKE filters on email/invitedByUser and isExpired toggle filter * fix(table-filters): improve InviteStatusFilterComponent - remove empty ngOnChanges, mark inviteStatuses as protected, handle null on filter clear * refactor(invites): use _getFilterFunction for fullName column filter * refactor(invite): reduce code duplication and add IsNull for non-expired filter - Extract role, projects, teams from restWhere to avoid duplicate filter application - Add helper functions buildBaseCondition and buildInvitedByUserCondition - Include IsNull() condition for non-expired filter to handle 'Never expire' invites - Add comments explaining boolean coercion from query params * refactor(table-filters): remove redundant constructor from InviteStatusFilterComponent * fix(invite): add defensive array handling for projects and teams filters Align with roleFilter pattern by handling both object with id property and direct array formats to prevent runtime errors from query params.
* fix: desktop integrated server failed to started * fix: desktop integrated server failed to started * Update apps/desktop/src/index.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Ruslan Konviser <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
|
|
Greptile OverviewGreptile SummaryThis PR merges multiple feature enhancements and bug fixes from the
The changes are well-structured with proper type safety improvements and defensive coding patterns. Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant AgentApp
participant WindowManager
participant ScreenCaptureWindow
participant NotificationUI
participant ActivityWindow
participant DesktopActivity
participant BackendAPI
participant EmployeeRecentVisitService
participant InviteService
Note over User,InviteService: Desktop Agent: Screen Capture & Activity Tracking
User->>AgentApp: Start tracking
AgentApp->>WindowManager: initScreenShotNotification()
WindowManager->>ScreenCaptureWindow: new ScreenCaptureWindow()
ScreenCaptureWindow->>ScreenCaptureWindow: loadURL()
loop Every tracking interval
AgentApp->>ActivityWindow: getActiveWindowAndSetDuration()
ActivityWindow->>DesktopActivity: getActiveWindow()
DesktopActivity-->>ActivityWindow: currentActiveWindow
ActivityWindow->>ActivityWindow: updateWindowActivities(APP)
alt Has URL
ActivityWindow->>ActivityWindow: updateWindowActivities(URL)
end
end
AgentApp->>WindowManager: showNotificationWindow(thumbUrl)
WindowManager->>WindowManager: isWindowReadyToShow() [recursive]
WindowManager->>NotificationUI: send('show_popup_screen_capture')
WindowManager->>NotificationUI: showInactive()
WindowManager->>WindowManager: setTimeout(hideNotificationWindow, 3000)
Note over User,InviteService: Backend: Employee Recent Visits
User->>BackendAPI: Visit project/task
BackendAPI->>EmployeeRecentVisitService: emitSaveEmployeeRecentVisitEvent()
EmployeeRecentVisitService->>EmployeeRecentVisitService: EventBus.publish()
EmployeeRecentVisitService->>EmployeeRecentVisitService: create(visitData)
alt Existing entry
EmployeeRecentVisitService->>EmployeeRecentVisitService: Update visitedAt
else New entry
EmployeeRecentVisitService->>EmployeeRecentVisitService: Create new entry
end
EmployeeRecentVisitService-->>BackendAPI: Recent visit saved
Note over User,InviteService: Backend: Enhanced Invite Filtering
User->>BackendAPI: GET /invites?email=john&isExpired=false
BackendAPI->>InviteService: findAllInvites(filters)
InviteService->>InviteService: buildWhereConditions()
alt Email filter
InviteService->>InviteService: Add LIKE email filter
end
alt Expiration filter
InviteService->>InviteService: Add expireDate >= now OR IS NULL
end
alt InvitedByUser filter
InviteService->>InviteService: OR (firstName LIKE, lastName LIKE)
end
InviteService-->>BackendAPI: Filtered invites
BackendAPI-->>User: Display invites
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
46 files reviewed, 2 comments
| async isWindowReadyToShow(window: BrowserWindow, windowType: WindowType) { | ||
| await delaySync(200); | ||
| if (!window?.webContents?.isLoading?.() && (windowType !== WindowType.notificationWindow || this.windowReadyStatus[windowType])) { | ||
| return true; | ||
| } | ||
| return this.isWindowReadyToShow(window, windowType); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: this recursive function could cause a stack overflow if the window never finishes loading or if windowReadyStatus[windowType] is never set to true
| async isWindowReadyToShow(window: BrowserWindow, windowType: WindowType) { | |
| await delaySync(200); | |
| if (!window?.webContents?.isLoading?.() && (windowType !== WindowType.notificationWindow || this.windowReadyStatus[windowType])) { | |
| return true; | |
| } | |
| return this.isWindowReadyToShow(window, windowType); | |
| } | |
| async isWindowReadyToShow(window: BrowserWindow, windowType: WindowType, maxRetries = 50): Promise<boolean> { | |
| if (maxRetries <= 0) { | |
| console.error(`Window ${windowType} failed to become ready after maximum retries`); | |
| return false; | |
| } | |
| await delaySync(200); | |
| if (!window?.webContents?.isLoading?.() && (windowType !== WindowType.notificationWindow || this.windowReadyStatus[windowType])) { | |
| return true; | |
| } | |
| return this.isWindowReadyToShow(window, windowType, maxRetries - 1); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/agent/src/main/window-manager.ts
Line: 284:290
Comment:
**logic:** this recursive function could cause a stack overflow if the window never finishes loading or if `windowReadyStatus[windowType]` is never set to true
```suggestion
async isWindowReadyToShow(window: BrowserWindow, windowType: WindowType, maxRetries = 50): Promise<boolean> {
if (maxRetries <= 0) {
console.error(`Window ${windowType} failed to become ready after maximum retries`);
return false;
}
await delaySync(200);
if (!window?.webContents?.isLoading?.() && (windowType !== WindowType.notificationWindow || this.windowReadyStatus[windowType])) {
return true;
}
return this.isWindowReadyToShow(window, windowType, maxRetries - 1);
}
```
How can I resolve this? If you propose a fix, please make it concise.| dashboardWindow: boolean; | ||
| }; | ||
| private static instance: AppWindow; | ||
| private autoHideTimeout: NodeJS.Timeout | null = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: inconsistent indentation - this line uses spaces while the rest of the class uses tabs
| private autoHideTimeout: NodeJS.Timeout | null = null; | |
| private autoHideTimeout: NodeJS.Timeout | null = null; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/agent/src/main/window-manager.ts
Line: 44:44
Comment:
**syntax:** inconsistent indentation - this line uses spaces while the rest of the class uses tabs
```suggestion
private autoHideTimeout: NodeJS.Timeout | null = null;
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 48 files
Prompt for AI agents (all 4 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/agent/src/main/window/screen-capture-window.ts">
<violation number="1" location="apps/agent/src/main/window/screen-capture-window.ts:17">
P3: Grammatically incorrect warning message. Consider rephrasing to: `'This method should not be called directly'` or `'Direct calls to this method are not supported'`.</violation>
</file>
<file name="packages/desktop-ui-lib/src/lib/agent-dashboard/services/logs.service.ts">
<violation number="1" location="packages/desktop-ui-lib/src/lib/agent-dashboard/services/logs.service.ts:96">
P2: Missing null/undefined check before iterating over `cacheLogs`. If the IPC call returns `null` or `undefined`, the `for...of` loop will throw a `TypeError`. Consider adding a guard clause or defaulting to an empty array.</violation>
</file>
<file name="packages/core/src/lib/employee-recent-visit/employee-recent-visit.controller.ts">
<violation number="1" location="packages/core/src/lib/employee-recent-visit/employee-recent-visit.controller.ts:10">
P2: `@Permissions()` is called without any arguments while `PermissionGuard` is in use. When no permissions are specified, `PermissionGuard` bypasses authorization checks entirely (returns `true`). Either specify the required permissions (e.g., `@Permissions(PermissionsEnum.SOME_PERMISSION)`) or remove `PermissionGuard` from the guards if no specific permission is needed.</violation>
</file>
<file name="packages/core/src/lib/activity-log/activity-log.subscriber.ts">
<violation number="1" location="packages/core/src/lib/activity-log/activity-log.subscriber.ts:119">
P2: Missing corresponding serialization for `updatedFields`. This field is now being deserialized in `afterEntityLoad`, but it's not included in the `serializeFields` call within `serializeDataForSQLite`. This asymmetry could cause issues when loading SQLite data that wasn't properly serialized. The `serializeDataForSQLite` method should also include `'updatedFields'` in its serialization array.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| } | ||
|
|
||
| override hide(): void { | ||
| console.warn('Prevent this method to directly call'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: Grammatically incorrect warning message. Consider rephrasing to: 'This method should not be called directly' or 'Direct calls to this method are not supported'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/src/main/window/screen-capture-window.ts, line 17:
<comment>Grammatically incorrect warning message. Consider rephrasing to: `'This method should not be called directly'` or `'Direct calls to this method are not supported'`.</comment>
<file context>
@@ -0,0 +1,19 @@
+ }
+
+ override hide(): void {
+ console.warn('Prevent this method to directly call');
+ }
+}
</file context>
| console.warn('Prevent this method to directly call'); | |
| console.warn('This method should not be called directly'); |
|
|
||
| async loadLogs() { | ||
| const cacheLogs: Record<string, any>[] = await this.electronService.ipcRenderer.invoke('load_logs'); | ||
| for (const logContent of cacheLogs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Missing null/undefined check before iterating over cacheLogs. If the IPC call returns null or undefined, the for...of loop will throw a TypeError. Consider adding a guard clause or defaulting to an empty array.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/desktop-ui-lib/src/lib/agent-dashboard/services/logs.service.ts, line 96:
<comment>Missing null/undefined check before iterating over `cacheLogs`. If the IPC call returns `null` or `undefined`, the `for...of` loop will throw a `TypeError`. Consider adding a guard clause or defaulting to an empty array.</comment>
<file context>
@@ -90,4 +90,11 @@ export class LogService {
+
+ async loadLogs() {
+ const cacheLogs: Record<string, any>[] = await this.electronService.ipcRenderer.invoke('load_logs');
+ for (const logContent of cacheLogs) {
+ this.handleLogStream(logContent);
+ }
</file context>
| for (const logContent of cacheLogs) { | |
| for (const logContent of cacheLogs ?? []) { |
| import { GetEmployeeRecentVisitsDTO } from './dto/get-employee-recent-visits.dto'; | ||
|
|
||
| @UseGuards(TenantPermissionGuard, PermissionGuard) | ||
| @Permissions() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: @Permissions() is called without any arguments while PermissionGuard is in use. When no permissions are specified, PermissionGuard bypasses authorization checks entirely (returns true). Either specify the required permissions (e.g., @Permissions(PermissionsEnum.SOME_PERMISSION)) or remove PermissionGuard from the guards if no specific permission is needed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/lib/employee-recent-visit/employee-recent-visit.controller.ts, line 10:
<comment>`@Permissions()` is called without any arguments while `PermissionGuard` is in use. When no permissions are specified, `PermissionGuard` bypasses authorization checks entirely (returns `true`). Either specify the required permissions (e.g., `@Permissions(PermissionsEnum.SOME_PERMISSION)`) or remove `PermissionGuard` from the guards if no specific permission is needed.</comment>
<file context>
@@ -0,0 +1,29 @@
+import { GetEmployeeRecentVisitsDTO } from './dto/get-employee-recent-visits.dto';
+
+@UseGuards(TenantPermissionGuard, PermissionGuard)
+@Permissions()
+@Controller('/employee-recent-visit')
+export class EmployeeRecentVisitController {
</file context>
| // Parse `updatedValues`, `previousValues`, `updatedEntities`, `previousEntities` if they are strings | ||
| // Parse `updatedFields`, `updatedValues`, `previousValues`, `updatedEntities`, `previousEntities` if they are strings | ||
| this.deserializeFields(entity, [ | ||
| 'updatedFields', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Missing corresponding serialization for updatedFields. This field is now being deserialized in afterEntityLoad, but it's not included in the serializeFields call within serializeDataForSQLite. This asymmetry could cause issues when loading SQLite data that wasn't properly serialized. The serializeDataForSQLite method should also include 'updatedFields' in its serialization array.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/lib/activity-log/activity-log.subscriber.ts, line 119:
<comment>Missing corresponding serialization for `updatedFields`. This field is now being deserialized in `afterEntityLoad`, but it's not included in the `serializeFields` call within `serializeDataForSQLite`. This asymmetry could cause issues when loading SQLite data that wasn't properly serialized. The `serializeDataForSQLite` method should also include `'updatedFields'` in its serialization array.</comment>
<file context>
@@ -114,8 +114,9 @@ export class ActivityLogSubscriber extends BaseEntityEventSubscriber<ActivityLog
- // Parse `updatedValues`, `previousValues`, `updatedEntities`, `previousEntities` if they are strings
+ // Parse `updatedFields`, `updatedValues`, `previousValues`, `updatedEntities`, `previousEntities` if they are strings
this.deserializeFields(entity, [
+ 'updatedFields',
'updatedValues',
'previousValues',
</file context>
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by cubic
Adds employee recent visits with an API and event-based tracking, improves agent window management and screen-capture flow, and fixes SQLite activity log insert errors. Also adds URL activity tracking and smarter invite filters.
New Features
Bug Fixes
Written for commit 15f7697. Summary will update automatically on new commits.