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.

Sign up for GitHub

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

Jump to bottom

Difference in class activity notifications after full facility import #11782

Closed
radinamatic opened this issue Jan 25, 2024 · 4 comments · Fixed by #12386
Closed

Difference in class activity notifications after full facility import #11782

radinamatic opened this issue Jan 25, 2024 · 4 comments · Fixed by #12386
Assignees
@AlexVelezLl
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) bug Behavior is wrong or broken DEV: frontend P1 - important Priority: High impact on UX
Milestone
Kolibri 0.17: Pla...

Comments

@radinamatic
Copy link
Member

Observed behavior

In a recent test I noticed some differences between class activity notifications displayed between two devices after the full facility import. Today I could not replicate the 'need help' status disappearance after full facility import from Win11 (0.16b10) to Win7 (0.16b13), howver, the class activity notifications continue not to display on the source (Win11) device anymore. The last syncing activity on that Win11 server was 5-6 days ago.

2024-01-24_22-11-19

Errors and logs

Logs and dbs for both servers.

Expected behavior

Consistent display of class activity on both devices

User-facing consequences

Confusing and inconsistent information presented to the coach.

Steps to reproduce

Import the full facility (with some class interactions and notifications) from one server to another.

Context

  • Kolibri version: Windows 11 (0.16b10) and Windows 7 (0.16b13)
@radinamatic radinamatic added bug Behavior is wrong or broken P0 - critical Priority: Release blocker or regression APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Jan 25, 2024
@marcellamaki marcellamaki added P1 - important Priority: High impact on UX and removed P0 - critical Priority: Release blocker or regression labels Feb 1, 2024
@marcellamaki
Copy link
Member

As discussed during the planning meeting, we'll downgrade to P1, but if we can possibly fit it in before 0.16.0 we will

@rtibbles
Copy link
Member

rtibbles commented Feb 1, 2024

Looking at the notification databases, this is not an issue with the notifications being properly generated, but rather that our code at some point is implicitly relying on the auto-incrementing integer id order to be indicative of the timestamp ordering for the notification. When we were generating the notifications just after the activity, this assumption held true, but now with the bulk creation after sync, it is no longer the case.

This can be fixed by ensuring that we always order the LearnerProgressNotifications by their timestamp field, not the id field.

@rtibbles rtibbles self-assigned this Feb 13, 2024
@marcellamaki
Copy link
Member

Possibly related issue noted by @jredrejo in the 0.17.0-alpha0 bug bash:

Notifications don’t appear in the coach panel after failing 7 responses in a row

@rtibbles rtibbles assigned AlexVelezLl and unassigned rtibbles Jul 1, 2024
@AlexVelezLl AlexVelezLl mentioned this issue Jul 2, 2024
Merged
9 tasks
@rtibbles
Copy link
Member

rtibbles commented Aug 8, 2024

Fixed in #12386

@rtibbles rtibbles closed this as completed Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AlexVelezLl AlexVelezLl

Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) bug Behavior is wrong or broken DEV: frontend P1 - important Priority: High impact on UX
Projects
None yet
Milestone
Kolibri 0.17: Planned Patch 1
Development

Successfully merging a pull request may close this issue.

Fix list activity notifications AlexVelezLl/kolibri
4 participants
@radinamatic @rtibbles @marcellamaki @AlexVelezLl