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

Learner’s relative time since last sync doesn’t update as time passes #11880

Closed
marcellamaki opened this issue Feb 15, 2024 · 4 comments
Closed
Assignees
Labels
APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend help wanted Open source contributors welcome P1 - important Priority: High impact on UX

Comments

@marcellamaki
Copy link
Member

Spotted by @bjester in 0.16 sync session Feb 14 2024

Observed behavior

Unless the sync status changes, the relative time since last sync doesn’t change over time as time passes. It seems like there is a bug or inconsistency in our <SyncStatusDisplay/> component's use of relative time.

Errors and logs

no-relative-time

Expected behavior

While we are already referencing the relative time, we need to fix it if it not working, to update the sync status in between changes in the sync status itself. Another possibility is that we need to change the polling within the side panel, to provide more consistent updates to calculate the relative time, and call the function that calculates it more often. However, we do not want to be polling. If that seems to be the better fix after further investigation, there should be some proposal raised in the issue comments for team discussion before proceeding.

User-facing consequences

Confusion about when the last sync happened due to slow/inaccurate UI status.

Steps to reproduce

  1. Set up two Kolibris - one as a Learn Only Device (LOD) and one as a full facility. The LOD should be set up to sync back the main facility
  2. Assign some resources to the Learner
  3. Observe the accuracy of sync status and the calculation of relative time
  4. Repeat if necessary

Notes:

  • opening and closing the side panel does impact the calculation - see the side panel code and sync status indicator code for more
  • testing by mocking a slower connection (i.e. Slow 3G) may be helpful in ensuring that the sync happens slowly enough that there is time to observe and troubleshoot

Context

Kolibri
release-v0.16.x

@marcellamaki marcellamaki added P1 - important Priority: High impact on UX APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend help wanted Open source contributors welcome labels Feb 15, 2024
@rtibbles rtibbles self-assigned this Feb 27, 2024
@rtibbles rtibbles assigned AlexVelezLl and unassigned rtibbles Apr 25, 2024
@AlexVelezLl
Copy link
Member

AlexVelezLl commented Apr 26, 2024

Hi @marcellamaki @rtibbles. This error is just because we are not updating the "now" reference in the component, as we just initialize it in the data object, but never update it again:

Thats why when we calculate the relativeTime

const relativeTime = this.$formatRelative(this.lastSynced, { now: this.now });
, we have an outdated value of now. A simple solution could be just removing the this.now state (we dont use it for anything else) and instead call the now() method each time the method is excecuted: const nowTime = now(); so the now is always updated.

Let me know if there is any issue with this solution. We can also add an setInterval to update the "now" value each X time in case the pollUserSyncStatus is paused.

@rtibbles
Copy link
Member

I think given the example above, it seems like there may be long periods where even if the pollUserSyncStatus is not paused, the this.lastSynced value remains constant, which means that the computed prop that it is being used in would not be recalculated.

So I think the advantage of keeping this.now as data is that we could use a setInterval to update it periodically, which would then trigger a recalculation of the computed prop.

This is what the ElapsedTime component does. One possibility would be to extract the logic from ElapsedTime component and turn it into a reusable composable to use in this situation, or you can just copy the logic to do the setInterval.

@AlexVelezLl
Copy link
Member

AlexVelezLl commented Apr 26, 2024

the this.lastSynced value remains constant, which means that the computed prop that it is being used in would not be recalculated.

For some reason I tried it and the computed prop was being recalculated each 10 seconds even if the status and lastSynced was the same 😅. But yes, the setInterval seems more reliable just in case. I will do it!

@AlexVelezLl
Copy link
Member

Fixed in #12110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend help wanted Open source contributors welcome P1 - important Priority: High impact on UX
Projects
None yet
Development

No branches or pull requests

3 participants