Coach > View learners - Missing 'Device status' and 'Last synced' values

[pcenov]

Observed behavior

The 'Device status' and 'Last synced' values are no longer displayed in Coach reports.

Expected behavior

The 'Device status' and 'Last synced' values should be displayed correctly.

Steps to reproduce the issue

1. On Device 1 setup a full facility
2. On Device 2 create a learn-only user for the facility on Device 1
3. Assign lessons/quizzes to the learner and complete them
4. As a coach go to Class home > View learners or Coach > Reports > View learner devices

Video

2023-04-28_16-55-26.mp4

Usage Details

Kolibri 0.16alfa13
Windows 10, Ubuntu - Chrome

[pcenov]

@radinamatic

[Pursottam6003]

Hii @pcenov Sir is it a backend issue or front end issue it looks like the front end is not able to display the data correctly and in the video I can able to see the console whether I guess it is showing array of objects of learners data?

Btw nicely identifying the bug 🫡

I will also reproduce this issue :)

[akolson]

HI @Pursottam6003. This is most likely a front-end issue, but first replicating the issue should clear this. Also, is this an issue you would like to take up?

[Pursottam6003]

HI @Pursottam6003. This is most likely a front-end issue, but first replicating the issue should clear this. Also, is this an issue you would like to take up?

Hii @akolson I would be happy to take this issue I tried to replicate this issue on my device but here what I did is created the users from the same device but here i am able to get the Last synced values

But I think I need to create a virtual machine to clearly understood the issue

[AllanOXDi]

Hey @pcenov I have been trying to replicate this bug but I am unable to. Are you able to dig into this further? Let me know.

[pcenov]

Hi @AllanOXDi could you please let me know on which build are you testing and perhaps share a video of what you are observing. I can confirm that I am able to replicate the issue in Kolibri 0.16.0a13.

[AllanOXDi]

I tested with 0.16.0a13 built.

[pcenov]

@AllanOXDi here's a screenshot of what I am observing at the moment, just to make sure we are talking about the same thing:

Let me know if you need further clarification.

[AllanOXDi]

As I found this hard to reproduce and work on . I suspect looking into this.fetchUserSyncStatus in the ClassLearnersListPage. To me it appears that , whatever is being returned from this endpoint is where the issue could be.

[akolson]

Hi @pcenov I was able to get it to show the status and last synced columns. I however suspect that this is closely related to the syncing issue experienced previously, but also a big possibility of the logic responsible for the display of the two columns is skewed. Here is a screenshot of what I was able to replicate.

As a next step to help resolve the bug, when you find some time, could you please post the response from /api/device/usersyncstatus api endpoint(Should be similar to response shown in right side of screenshot above)?

[pcenov]

Hi @akolson, I had a chance to take a closer look at this today. Looks like it's not just the device status and last sync that are not displayed, I am also not seeing anything in the 'Device activity' panel.

Here's a short video of what I am doing and observing (I've skipped some parts to make it shorter):

sync.mp4

Here's the api response:

Logs and databases for both devices:
Learner_device.zip
MAIN_device.zip

Let me know if you need additional info.

[akolson]

It looks like the sync_session_id field(in the UserSyncStatus model) is not being set on the main device. Thus the reason why all the properties that depend on the presence of the sync_session_id are null.

However, it is being set correctly on the learner device

cc @rtibbles @bjester

[bjester]

@akolson I believe this code should be setting it on the server-side https://github.com/learningequality/kolibri/blob/develop/kolibri/core/device/kolibri_plugin.py

[bjester]

I think it might be setting it for the LOD too. Perhaps something is clearing it?

[akolson]

Yes,, based on the db, it is being set on the LOD. The comment here suggests that it is being cleared

[bjester]

Hmm, but where's the actual code that deletes it? That might be a red herring, just an idea. The alternate possibility is that the hooks are broken

[akolson]

Yeah, still looking around

[akolson]

Hi @pcenov! So we dug out the pr that introduced these changes, thanks to @marcellamaki 🙂 . Just wondering if you could test test against the artifacts of this pr when you find some time. (PS: The pr is about a 13 months old so its possible a lot has changed and it may not work.)

[pcenov]

Hi @akolson I am getting a Failed to generate URL to download artifact. error when I attempt to download any of the build assets from #9438.

[rtibbles]

They've probably expired - I think the default retention period for artifacts is 3 months.

[akolson]

@rtibbles is there a way to rerun the build?

[bjester]

We reproduced this again in the bug hunt. @akolson I can help figure out what's going wrong.

[bjester]

1. Unless I'm misremembering, this active=True filter doesn't seem to make sense. A completed transfer should be active=False and an errored transfer likely has active=True, so no query filter against that should be used. Perhaps the variable naming are inconsistent with its intention but it could use more clarity.
2. The last_synced annotation is tracking that last activity of a LOD sync session, which will likely always have some value and isn't representative of a completed sync. Additionally, if something fails and the sync is garbage collected, a new sync session will be created.
3. The above pieces of information are important for map_status which also appears that there is a scenario where it could produce no status!

[bjester]

an errored transfer likely has active=True

that is until the sync session is garbage collected.

[rtibbles]

Unless I'm misremembering, this active=True filter doesn't seem to make sense. A completed transfer should be active=False and an errored transfer likely has active=True, so no query filter against that should be used. Perhaps the variable naming are inconsistent with its intention but it could use more clarity.

Looking at the map_status function, the reason we are doing the active=True filter is because we are only using this information when a sync is in progress or has errored, which seems to accord with your description of when this would be the case.

The last_synced annotation is tracking that last activity of a LOD sync session, which will likely always have some value and isn't representative of a completed sync. Additionally, if something fails and the sync is garbage collected, a new sync session will be created.

I think this is where the null values are likely coming from - if the sync has been garbage collected, then the sync session associated with the sync status is null, and so the last_synced value is null. As a result we fail to catch any of the conditionals in map_status and return None, rather than NOT_RECENTLY_SYNCED.

[rtibbles]

This helps a little to actually show a status when there is no sync session, but is not the root of the problem. The root appears to be that morango never actually starts the initializing stage, as far as I can tell.

I think this is because the LocalSessionContext defaults to the INITIALIZING stage, and therefore the session controller doesn't think there's been a state change when it fires the middleware, and so never fires the started signal handlers.

See https://github.com/learningequality/morango/blob/release-v0.7.x/morango/sync/context.py#L234 and https://github.com/learningequality/morango/blob/release-v0.7.x/morango/sync/controller.py#L250

I am guessing the salient difference when the sync is being run on the LODs, because they are running the management command is to do with these lines of code, which appear to be causing the initializing started signals to be fired: https://github.com/learningequality/kolibri/blob/release-v0.16.x/kolibri/core/auth/management/utils.py#L560

[rtibbles]

Seems like the solution here may be to add equivalent code into the TransferSessionViewset create method in morango, in order to ensure that the initializing started signals are always fired.

[rtibbles]

This issue will only be completely solved once learningequality/morango#201 is merged, and a new version of morango is released and kolibri updated with it.

[bjester]

The firing of the initializing.started signal was a hack implemented in Kolibri to get around the issue of the signals firing within the transaction. Morango never fired that signal itself, because it was inconsistent with other signals on what data was available-- the initialization stage creates the sync session so a started signal wouldn't have the same information as all other signals.

My mistake here was forgetting to preserve the attachment to the completed signal, and only attach to the started signal in the sync command. We wouldn't want to fire the signal in the viewset like learningequality/morango#201 because that creates a discrepancy between client and server side signal firing. In short, the signal firing discrepancy originated in Kolibri from my hack, so I think it's best to contain the hacks to Kolibri until we have better flexibility in morango (that flexibility would delete the changes made in the morango PR anyway)

[rtibbles]

After a bit of a tag team effort here, this has been resolved by @bjester in #11268