Scheduled syncing updates #10748

rtibbles · 2023-05-25T00:19:17Z

Summary

Updates the scheduled syncing code to use the updated backend from Allow update and delete of scheduled tasks #10724 to:
- Update enqueue arguments
- Delete scheduled tasks
Also updates a lot of frontend logic to repopulate the data for the EditDeviceSyncSchedule component to allow properly editing the task based on existing enqueue arguments
Updates handling of date times in the frontend to better display and internationalize using vue-intl utilities
Updates the SyncFacilityModalGroup component to include handling of registration to KDP to avoid replicating logic across three different parts of the app
Integrates the updated SyncFacilityModalGroup into ManageSyncSchedule to leverage facility specific filtering and syncing to KDP
Adds support for syncing to KDP within the Sync schedule components

References

Fixes #10490

Reviewer guidance

Schedule, edit, and delete scheduled syncs using the UI in both the Device plugin and the Facilities plugin.

Screencast.from.05-24-2023.05.12.25.PM.webm

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

Allow editing and deleting of scheduled syncs.

…tyModalGroup.

github-actions · 2023-05-25T00:41:06Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.0a14.dev0_git.98.g3ba08341.pex
Windows Installer (EXE)	kolibri-0.16.0a14.dev0+git.98.g3ba08341-unsigned.exe
Debian Package	kolibri_0.16.0a14.dev0+git.98.g3ba08341-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.0a14.dev0+git.98.g3ba08341-0.3.0.dmg
Source Tarball	kolibri-0.16.0a14.dev0+git.98.g3ba08341.tar.gz
WHL file	kolibri-0.16.0a14.dev0+git.98.g3ba08341-py2.py3-none-any.whl

AllanOXDi · 2023-05-25T13:33:59Z

kolibri/plugins/device/assets/src/views/FacilitiesPage/facilityTasksQueue.js

+        const activeSyncTasks = this.facilityTasks.filter(
+          t =>
+            isSyncTask(t) &&
+            ((t.repeat !== null && !t.clearable) ||


None blocking comment: I am wondering if we could introduce intermediate variables with descriptive names to improve code readability. Something like

const isSync = isSyncTask(task); const isRepeatableNonClearable = task.repeat !== null && !task.clearable;

So that instead of using a complex logical expression, the return statement in the filter() method utilizes the intermediate variable

Yes, I think this functional style can be a bit hard to read and intermediary variables can help. I mostly didn't do that because it was like that when I got here! I'll leave for now, but I think worth thinking about when we do further reworking of the frontend task handling (which I have some overall ideas on how we can migrate to a dedicated composable).

AllanOXDi · 2023-05-25T13:46:32Z

packages/kolibri-common/components/SyncSchedule/EditDeviceSyncSchedule.vue

I am wondering why whenever I try to edit task scheduled for an hour it become inactive?

Uploading Screen Recording 2023-05-25 at 15.19.00.mov…

This is because the tasks scheduled on the hour immediately sync - and because of backend complexities, we can't edit currently running tasks. I think this could probably use some user experience improvements, but also wasn't quite sure the best way to manage it without string updates too.

LianaHarris360 · 2023-05-25T20:51:23Z

kolibri/core/assets/src/views/sync/ConfirmationRegisterModal.vue

@@ -90,6 +99,13 @@
            }
          });
      },
+      cancel() {
+        if (this.alreadyRegistered && this.successOnAlreadyRegistered) {


If a facility tries to re-register to a project they are already registered to, should there be a check to see if it is already registered with the project before submission, which would change this data prop alreadyRegistered to true? Or is this line only meant to catch the error if the facility is found to already be registered?

I'm asking because the parts where alreadyRegistered is used in the template are never really shown and could be removed. I manipulated the data in Vue.js devtools to see this view.

This is only meant to catch the case where there is an error with registration due to the facility already being registered to KDP.

In the case where we are directly doing a registration action by using the Register option from the dot menu, this emits a 'cancel' event. In the case that I am using it here, I wanted it to instead emit a success event in this case, as it is fine to carry on and try to sync to KDP because it is already registered, so this logic is intended to be handling that.

pcenov · 2023-05-30T14:47:02Z

Hi @rtibbles here are some minor issues I was able to identify while testing the scheduled syncing:

When a sync is happening while one is at the Facilities page, the 'Last synced' value doesn't update unless I manually refresh the page:

KDP.Sync.mp4

The status of a KDP schedule is always displayed as 'Not connected' and there's always an error in the console which states: Cannot read properties of undefined (reading 'device name')

For some reason there are 2 tasks in the task manager which always show as incomplete (I'll let you know if I find specific steps to replicate that one):

Going to dismiss this for now until we get QA sign off!

rtibbles · 2023-05-30T15:29:34Z

Thanks Peter - I think issue 1 is an existing issue, so let's file that as a follow up.

For issue 2, I'll hide the connection status for KDP, and we can update that when we have better connection tracking for KDP once #10431 is resolved.

For issue 3, I'll tweak to make this less weird.

rtibbles · 2023-05-30T20:22:42Z

2 should now be fixed:

3 should now be better:

pcenov · 2023-05-31T11:52:14Z

@rtibbles I confirm that 2 and 3 are fixed now and have filed a follow-up issue for 1 here: #10759

I also noticed that if I lose my internet connection or I stop the server for a facility for which I have scheduled a sync, the line in the table completely disappears instead of seeing the 'Not connected' status:

2023-05-31_14-30-15.mp4

A somewhat similar issue can be observed at Facility > Data > Manage sync schedule where after editing the schedule the entry disappears:

2023-05-31_16-29-18.mp4

rtibbles · 2023-05-31T15:02:26Z

Thanks @pcenov - that's a good catch, I'll update to fix this.

rtibbles · 2023-06-01T19:15:41Z

I think this should now be fixed.

pcenov · 2023-06-02T15:10:20Z

Hi @rtibbles, I confirm that now if I stop the server of a facility for which I have a scheduled sync that is reflected correctly in the status which changes first to 'Not connected' and then back to 'Connected' once I've restarted the server. However if I turn off my wi-fi and then turn it on, the status remains 'Not connected' until I manually restart the server.

2023-06-02_17-54-09.mp4

Also this issue is still not fixed:

"A somewhat similar issue can be observed at Facility > Data > Manage sync schedule where after editing the schedule the entry disappears:"

2023-05-31_16-29-18.mp4

Lastly, one new very minor issue which we've missed so far - the 'Frequency' drop-down is incorrectly labeled 'Frequence'

I can of course file those as separate issues if you wish.

rtibbles · 2023-06-02T15:11:33Z

Lastly, one new very minor issue which we've missed so far - the 'Frequency' drop-down is incorrectly labeled 'Frequence'

This will need to be a separate issue, as we don't have the string to update this, I believe.

rtibbles · 2023-06-02T15:14:16Z

However if I turn off my wi-fi and then turn it on, the status remains 'Not connected' until I manually restart the server.

I think this should be filed as a follow up issue too - seems to be an issue with the connection status being properly updated when the network adapter state changes, so this is rather outside the remit of this PR!

rtibbles · 2023-06-02T15:14:50Z

"A somewhat similar issue can be observed at Facility > Data > Manage sync schedule where after editing the schedule the entry disappears:"

I had foolishly assumed that the previous issue that I fixed and this issue had the same root cause, I'll investigate further now.

rtibbles · 2023-06-07T02:07:53Z

@pcenov this issue:

"A somewhat similar issue can be observed at Facility > Data > Manage sync schedule where after editing the schedule the entry disappears:"

Was actually an issue with any navigation back from the sync edit page or the listing of all syncs page. Now all navigation actions should be working including:

Creating a new sync
Editing an existing sync
Deleting a sync
Pressing the X button when editing a sync
Cancelling a sync edit
Going back from the listing of syncs (should return to the correct Data page for the chosen facility)

pcenov · 2023-06-07T11:45:17Z

Thanks @rtibbles for further investigating and fixing it properly. No new issues observed while regression testing today, LGTM!

radinamatic

Manual QA passed, good to go! 👏🏽 💯

rtibbles added 6 commits May 24, 2023 17:00

Properly calculate next sync time for scheduled syncs.

1961cf5

Allow editing and deleting of scheduled syncs.

Move handling of registration while selecting KDP into the SyncFacili…

67eb9ee

…tyModalGroup.

Add handling of KDP syncing to manage schedule sync component.

122387b

Add handling for KDP syncing to edit device schedule.

2f439c0

Add better handling for scheduled tasks.

ab7bccf

Do existence check for nickname.

2cff899

rtibbles added the TODO: needs review Waiting for review label May 25, 2023

rtibbles added this to the Kolibri 0.16 Release: Automatic Syncing Updates milestone May 25, 2023

rtibbles requested review from nucleogenesis, LianaHarris360 and AllanOXDi May 25, 2023 00:19

github-actions bot added APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) SIZE: large labels May 25, 2023

AllanOXDi reviewed May 25, 2023

View reviewed changes

Update signature of composable function.

f84e3f9

AllanOXDi previously approved these changes May 25, 2023

View reviewed changes

LianaHarris360 reviewed May 25, 2023

View reviewed changes

pcenov added the gherkin update An issue that only requires updating Gherkin stories for a feature label May 29, 2023

rtibbles added 2 commits May 30, 2023 13:18

Don't show non-running scheduled tasks in the task panel UI.

a7bf6db

Don't show connection status for KDP.

a605c9f

pcenov mentioned this pull request May 31, 2023

Scheduled sync - The 'Last synced' value won't auto-update #10759

Closed

Add proper handling for showing scheduled tasks for unknown devices.

1768bce

This was referenced Jun 2, 2023

Connection status isn't being properly updated when the network adapter is reset #10777

Open

Scheduled sync - The 'Frequency' drop-down is labeled incorrectly #10778

Closed

Add route facility_ids to back routes to maintain context in navigation.

8224234

radinamatic approved these changes Jun 7, 2023

View reviewed changes

rtibbles merged commit 3e5861d into learningequality:develop Jun 7, 2023

rtibbles deleted the scheduled_syncing_updates branch June 7, 2023 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scheduled syncing updates #10748

Scheduled syncing updates #10748

rtibbles commented May 25, 2023 •

edited

Loading

github-actions bot commented May 25, 2023 •

edited

Loading

AllanOXDi May 25, 2023

rtibbles May 25, 2023

AllanOXDi May 25, 2023

rtibbles May 25, 2023

LianaHarris360 May 25, 2023

rtibbles May 25, 2023

pcenov commented May 30, 2023

rtibbles commented May 30, 2023

rtibbles commented May 30, 2023

pcenov commented May 31, 2023 •

edited

Loading

rtibbles commented May 31, 2023

rtibbles commented Jun 1, 2023

pcenov commented Jun 2, 2023

rtibbles commented Jun 2, 2023

rtibbles commented Jun 2, 2023

rtibbles commented Jun 2, 2023

rtibbles commented Jun 7, 2023

pcenov commented Jun 7, 2023

radinamatic left a comment

Scheduled syncing updates #10748

Scheduled syncing updates #10748

Conversation

rtibbles commented May 25, 2023 • edited Loading

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

github-actions bot commented May 25, 2023 • edited Loading

Build Artifacts

AllanOXDi May 25, 2023

Choose a reason for hiding this comment

rtibbles May 25, 2023

Choose a reason for hiding this comment

AllanOXDi May 25, 2023

Choose a reason for hiding this comment

rtibbles May 25, 2023

Choose a reason for hiding this comment

LianaHarris360 May 25, 2023

Choose a reason for hiding this comment

rtibbles May 25, 2023

Choose a reason for hiding this comment

pcenov commented May 30, 2023

rtibbles commented May 30, 2023

rtibbles commented May 30, 2023

pcenov commented May 31, 2023 • edited Loading

rtibbles commented May 31, 2023

rtibbles commented Jun 1, 2023

pcenov commented Jun 2, 2023

rtibbles commented Jun 2, 2023

rtibbles commented Jun 2, 2023

rtibbles commented Jun 2, 2023

rtibbles commented Jun 7, 2023

pcenov commented Jun 7, 2023

radinamatic left a comment

Choose a reason for hiding this comment

rtibbles commented May 25, 2023 •

edited

Loading

github-actions bot commented May 25, 2023 •

edited

Loading

pcenov commented May 31, 2023 •

edited

Loading