Skip to content
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

[Fix] Activity Watch integration issue #8223

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

adkif
Copy link
Contributor

@adkif adkif commented Sep 17, 2024

PR

Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.


@adkif adkif requested a review from evereq September 17, 2024 11:29
Copy link

nx-cloud bot commented Sep 17, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 773f4fb. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


🟥 Failed Commands
nx build gauzy -c=production --prod --verbose
✅ Successfully ran 5 targets

Sent with 💌 from NxCloud.

@@ -1703,34 +1703,24 @@ export class TimeTrackerComponent implements OnInit, AfterViewInit {
this.electronService.ipcRenderer.send('stop-capture-screen');

if (this._startMode === TimerStartMode.MANUAL) {
console.log('Stopping timer');
const timer = await this.electronService.ipcRenderer.invoke('STOP_TIMER', config);
console.log('Taking screen capture');
Copy link
Member

Choose a reason for hiding this comment

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

@adkif please cover (as it was before!) everything inside this IF statement into

asyncScheduler.schedule(async () => {
  console.log('Taking screen capture');
  const activities = await this.electronService.ipcRenderer.invoke('TAKE_SCREEN_CAPTURE', config);
  console.log('Sending activities');
  await this.sendActivities(activities);
}, 1000);

We discussed it before few times... We do NOT want to wait till that process finish, it can go in background (and can even potentially fail), we want to STOP timer right after user click "STOP" and do all other ops in the background. In the future we will have to put this as "work item" in the tasks QUEUE, but for now simple schedule with 1 sec delay is good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evereq This code causes issues with activities when it sends after the timer stops. The timer needs to reset and close some processes. The best approach is to send the last activities and then stop. A major problem was uploading screenshot images, so I used asapScheduler to prevent blocking during activity sending.

Copy link
Member

@evereq evereq Sep 17, 2024

Choose a reason for hiding this comment

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

@adkif It takes considerable amount of time to send those activities and to make a screen capture! I do NOT want to do it in the main thread and block timer stop! And it WAS working VERY good after I added that asyncScheduler.schedule, is it causing issues for AW!?? I really don't understand why we need to remove this

Copy link
Member

Choose a reason for hiding this comment

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

@adkif in addition, if something fails, it does NOT stop timer! I.e. it shows button as not active, but timer not stop! (if we remove that asyncScheduler.schedule). So again, I would like to KEEP it, unless you know what exactly it broke and NO other way to fix that!

@adkif adkif requested a review from evereq September 17, 2024 17:41
}

const timeSlotId = resActivities.id;
asapScheduler.schedule(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evereq evereq merged commit 8e876ce into fix/offline-sync-and-permission Sep 17, 2024
15 of 16 checks passed
@evereq evereq deleted the fix/activity-watch branch September 17, 2024 19:36
evereq pushed a commit that referenced this pull request Sep 17, 2024
* feat: add hasAllPermissions and hasAnyPermissions utility function

* fix: toggleApiStop method adding source and refactor

* fix: added optional chaining operator to access `status.lastLog.id`

* [Fix] Activity Watch integration issue (#8223)

* feat: add timeout to upload Images method time tracker service

* fix: speed up timer start/stop
evereq added a commit that referenced this pull request Sep 17, 2024
* [Fix] Remove nested event listeners (#8211)

* refactor: PowerManagerDetectInactivity, DesktopOsInactivityHandler, IntervalDAO, and TimeTrackerComponent classes; modify event listeners, method calls, and conditional statements; remove ipcMain and _clearIntervals references.

* fix: missing await

* fix: theme switch

* fix: add .titlebar-space class with margin-top: 15px

* refactor: rename `sort` to `isSortable` in Smart Table columns

* fix: email template utils query optimization

* fix: email templates rendering migrations

* fix(cspell): typo spelling :-)

* revert: fix: email templates rendering migrations

* fix: favorite entity missing column indexing

* fix: favorite entity missing column indexing (sqlite & mysql)

* MySQL syntax error on Time Tracking Dashboard reload

* [Fix] Offline sync and permission (#8222)

* feat: add hasAllPermissions and hasAnyPermissions utility function

* fix: toggleApiStop method adding source and refactor

* fix: added optional chaining operator to access `status.lastLog.id`

* [Fix] Activity Watch integration issue (#8223)

* feat: add timeout to upload Images method time tracker service

* fix: speed up timer start/stop

---------

Co-authored-by: Kifungo A <[email protected]>
Co-authored-by: Rahul R. <[email protected]>
Co-authored-by: samuelmbabhazi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants