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

Improve polling on the home page to more accurately and quickly display information related to sync statuses #11883

Open
marcellamaki opened this issue Feb 15, 2024 · 20 comments
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 SIZE: medium TAG: ux update Improved user-facing feature

Comments

@marcellamaki
Copy link
Member

Observed in connection with several different issues during the 0.16 sync session on Feb 14 2024

Observed behavior

  • "Resources are missing or not supported" banner sometimes displays for "too long", meaning that it does not disappear on its own without a manual refresh (or navigation away from the page and back), which can be confusing for users. It also means that banner that displays a waiting to sync or actively syncing status is not necessarily refreshing and displaying in the way it was originally intended to, which was to provide feedback for these intermediary stages
  • There is also no indication of which lessons or quizzes have missing resources - is it all? is it one?
  • There is no indication of whether or not the missing resources are waiting for another sync

Desired behavior/Acceptance Criteria

  • Fix polling on the home page (either this was lost entirely, inadvertently, or there is a bug somewhere, or, we need to adjust the frequency) - the goal is to ensure that the polling is frequent enough to provide meaningful updates to the user, without spamming the server unnecessarily. Once assigned, please suggest an approach in the issue comments for discussion before proceeding to implement.
  • Add the "warning" icon (a yellow triangle with an "!" centered) to the lesson or quiz card that can be used to indicate a missing the resource, and use the KCircularLoader to indicate that a sync is happening. So, we want to: 1) fix the banner display, and 2) have a consistent display, aligned to the banner state, on the specific card or cards that shows if resources are missing or waiting to sync
  • Learners should not be able to click into a lesson or quiz that has no available resources
  1. Steps to reproduce

  2. 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
  3. Assign some resources to the Learner
  4. In order to cause a "failure" or missing resources, disconnecting the facility device and the LOD manually (stopping the facility server for instance) may be the best approach to reproduce
  5. Observe the display of the missing resources
  6. Restart and reconnect the devices to the shared LAN
  7. Observe that the missing resource display does not disappear until page refresh

Context

0.16.x

@marcellamaki marcellamaki added the P1 - important Priority: High impact on UX label Feb 15, 2024
@marcellamaki marcellamaki added TAG: ux update Improved user-facing feature APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend help wanted Open source contributors welcome SIZE: medium labels Feb 15, 2024
@Technmad
Copy link

@marcellamaki Can i give it a shot

@AlexVelezLl
Copy link
Member

Hi @Technmad! Sure, I'll assign this issue to you :). Thanks for your interest in contributing to Kolibri 👐

@Technmad
Copy link

@AlexVelezLl Thanks for opportunity, I'll start right away

@Janarthana2992
Copy link

can I work on this issue, as a GSOC 24' applicant? @AlexVelezLl

@AlexVelezLl
Copy link
Member

Hi @Technmad! are you still working on this? If @Technmad is no longer working on this, we can assign it to you @Janarthana2992. 👐

@AlexVelezLl
Copy link
Member

Hello, since @Technmad did not respond, I will assign this issue to you, @Janarthana2992. Thank you very much for volunteering. Let us know if you have any questions.

@rtibbles
Copy link
Member

rtibbles commented Nov 8, 2024

Unassigning as no updates in over 6 months.

@Jeevansm25
Copy link

This issue is still open?

@AlexVelezLl
Copy link
Member

Hi @Jeevansm25! Yes, this issue is still open. I just assigned you another issue so lets wait for now before I assign you this. Once you have a PR for the other issue you can ping us to assign you this one 🤗.

@Jeevansm25
Copy link

Hi @Jeevansm25! Yes, this issue is still open. I just assigned you another issue so lets wait for now before I assign you this. Once you have a PR for the other issue you can ping us to assign you this one 🤗.

Thank you for assigning me the other issue 😊. I'll work on it and submit a PR soon. Once it's done, I'll reach out to you for this issue. Thanks again for your guidance and support! 🙌

@yashhash2
Copy link
Contributor

@AlexVelezLl Can you assign this issue to me as @Jeevansm25 is already on other issue.

@AlexVelezLl
Copy link
Member

Hi @yashhash2! For sure! I will assign this to you. Please let us know if you have any question :)

@MisRob
Copy link
Member

MisRob commented Dec 17, 2024

@yashhash2
Copy link
Contributor

To address the issue of missing resources on lesson and quiz cards, the following approach is proposed:

showMissingResource Function: This function will check whether an individual lesson or quiz has a missing_resource flag. If the flag is present, the function will return true, indicating that the warning icon should be displayed for that specific card.

Polling for Missing Resources: A polling mechanism will be implemented to check for missing resources every 5 seconds. This polling behavior will update the missing_resource flag for each lesson and quiz, ensuring that the status of resources is regularly refreshed.

Conditional Rendering on Cards:

A yellow warning icon (⚠️) will be displayed in the top-right corner of the lesson or quiz card to indicate when resources are missing.
A circular loader (KCircularLoader) will appear at the center of the card when the resource is in the process of syncing.
Card Styling: Necessary styles will be applied to position the warning icon and loader correctly within the individual lesson or quiz cards, ensuring a visually consistent and clear display.

Disabling Card Interaction: If a resource is missing for a particular card, that card will be visually disabled by applying the pointer-events: none CSS style. This will prevent user interaction with the card until the resources are available.

This approach will ensure that users receive clear and timely feedback regarding missing or syncing resources on lesson and quiz cards. It will also provide a more intuitive user experience by disabling interaction with cards that lack resources. Let me know if you have any suggestions or feedback on this approach before proceeding with implementation.

@marcellamaki
Copy link
Member Author

Hi @yashhash2 - overall this sounds like a good summary of the approach needed. Before you get started, I'd like to request that you investigate a little bit more the previous polling implementation on the home page and do some troubleshooting. Then, share a little bit about what you find here and we can decide together a bit more on the specifics of implementation.

You can use git history of the files that compose the home page to see if polling is still happening, if it's happening sometimes, or if it's not happening at all. There may be some commits after we implemented it the first time that broke it, or it may just be not polling frequently enough. Building this skill of investigating the underlying causes of bugs in a codebase is a very useful one, and this is a great opportunity to do so. Taking a little bit of extra time early will help you uncover the root cause of the issue and make the best decision about the approach moving forward.

After that, we can discuss further what decisions would need to be made for the frontend components, although it seems like you're on the right track. :)

Let me know if you have any questions!

@yashhash2
Copy link
Contributor

Hi @marcellamaki,

I was reviewing the history of components related to the homepage and noticed a change made about a year ago under the commit titled "Rehydrate the home page when syncing has completed to refresh missing resources." This change may be contributing to the issue where the "Resources are missing or not supported" banner persists for too long without disappearing automatically.

Potential Cause

The change involved using get() from @vueuse/core within the isSyncing function instead of directly accessing .value. I believe this could be causing delays in reactivity updates. Here’s a breakdown of the potential issues:

Reactivity Problems with get()

  • get(status) abstracts direct access to .value. If get() doesn’t respond properly to reactive changes, it may delay the update cycle.
  • Vue's reactivity system is optimized for .value access with refs or computed properties. Abstraction with get() can sometimes cause unexpected reactivity delays.

Subtle Differences in Behavior

  • get() may introduce additional evaluation steps, causing status or syncDownloadsInProgress to lag behind real-time changes. As a result, isSyncing could continue returning true even when the sync process is complete.

Watch Effectiveness

  • The watch(isSyncing, (newVal, oldVal) => { ... }) function depends on isSyncing updating correctly. If get() causes delays, the syncComplete event may not be emitted promptly, leading to the banner persisting longer than expected.

Hypothesis

Using get() may unintentionally hinder reactivity, delaying the detection of sync completion and, consequently, the automatic dismissal of the banner. Reverting to direct .value access should improve reactivity and resolve the issue.

Suggested Change

Refactor isSyncing by replacing get() with direct .value access:

const isSyncing = computed(() => {
  return status.value === SyncStatus.QUEUED || 
         status.value === SyncStatus.SYNCING || 
         syncDownloadsInProgress.value;
});

Additional Note

Currently, I am unable to fully replicate the resource-missing issue during testing. Even after stopping the facility server, resources remain available at the LOD. However, when resources are hidden from learners, the banner does appear. If there are alternative approaches to consistently trigger the banner for testing, I would appreciate your insights.

Looking forward to your thoughts on this hypothesis.

Thank you,
Yash Kumar Singh

@MisRob
Copy link
Member

MisRob commented Jan 16, 2025

Hi @yashhash2, thanks for looking into it.

Abstraction with get() can sometimes cause unexpected reactivity

Is this something you've observed yourself or is there some source describing it as a general Vue issue? From your comment, it's not clear to me whether you think that using .value will help or if you are already confident about it.

Refactor isSyncing by replacing get() with direct .value access

Regarding using .value, I don't see a problem with it if there's clarity that it's really the cause

Currently, I am unable to fully replicate the resource-missing issue during testing. Even after stopping the facility server, resources remain available at the LOD. However, when resources are hidden from learners, the banner does appear. If there are alternative approaches to consistently trigger the banner for testing, I would appreciate your insights.

Hm yes syncing issues can generally be a bit tricky to reproduce, sorry for the trouble. I wonder if finding the relevant places on frontend and tweaking them to mock backend behaviors that are needed would help.

@yashhash2
Copy link
Contributor

Hi @MisRob,

Thank you for your thoughtful response and questions.

Observations and Context

Regarding the potential impact of using get() on reactivity, my initial hypothesis was based on how @vueuse/core abstracts reactive values. While get() provides consistency and readability, it introduces an abstraction layer over direct .value access. This indirection could theoretically contribute to delayed reactivity updates.

However, after manually testing by removing get(), I observed that this is not the root cause of the issue. I apologize for the delay, as I spent time reviewing the timeline of related commits.

Updated Findings

Through further exploration, I discovered that a manual page refresh is required even to reflect assigned resources, even when new resources are available. This behavior suggests that the homepage currently lacks automatic polling to regularly fetch updated data. The absence of polling prevents the "Resources are missing or not supported" banner from disappearing without user intervention.

Proposed Approach

To address this, I propose implementing a manual polling mechanism using setInterval() to fetch data at regular intervals, ensuring the homepage reflects up-to-date resource availability dynamically. Before proceeding, I would appreciate your input on this approach or any alternative strategies you recommend. Additionally, if there are any relevant resources or examples that demonstrate how polling is implemented within Kolibri, I would be grateful for the guidance.

Looking forward to your feedback.

Best regards,
Yash Kumar Singh

@MisRob
Copy link
Member

MisRob commented Jan 21, 2025

Hi @yashhash2, yes, fixing the polling is what we wish to do.

As per the issue description

Fix polling on the home page (either this was lost entirely, inadvertently, or there is a bug somewhere, or, we need to adjust the frequency) - the goal is to ensure that the polling is frequent enough to provide meaningful updates to the user, without spamming the server unnecessarily. Once assigned, please suggest an approach in the issue comments for discussion before proceeding to implement.

the next step for you would be to search through the code and find out whether there's some polling already implemented but it's broken, or if we lost it completely and it needs to be re-implemented. Please search code carefully.

@yashhash2
Copy link
Contributor

Hi @MisRob,

I noticed that polling is actively occurring on the page via useUserSyncStatus. However, the issue with the banner not disappearing seems to stem from the ResourceSyncingUiAlert component. Specifically, the @syncComplete="hydrateHomePage" event is not being emitted, which prevents hydrateHomePage from being triggered.

In my case, I logged the status value and found that it is consistently "NOT_RECENTLY_SYNCED". As a result, the isSyncing value in the watch function remains false at all times, and the syncComplete event is never emitted.

For reference, please find attached the screenshot of the logs:

Image

To resolve this, we may need to address the syncing issue or adjust the conditions for emitting the syncComplete event to ensure that hydrateHomePage is triggered successfully.

Let me know your thoughts.

Best regards,
Yash Kumar Singh

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 SIZE: medium TAG: ux update Improved user-facing feature
Projects
None yet
Development

No branches or pull requests

8 participants