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

Fix order in which notification updates are commited to state #11374

Merged

Conversation

MisRob
Copy link
Member

@MisRob MisRob commented Oct 9, 2023

Summary

Commit updates to state sorted by timestamp.

References

Fixes #11302 where "Completed" update was commited before "Started" update.

Reviewer guidance

Start a quiz on a full device. Take the quiz on LoD device. Wait for sync to happen and confirm that #11302 is fixed.

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

@MisRob MisRob requested review from rtibbles and pcenov October 9, 2023 10:49
@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) SIZE: small and removed DEV: frontend labels Oct 9, 2023
Commit updates to state sorted by timestamp.
Fixes learningequality#11302
where "Completed" update was commited before "Started" update.
@MisRob MisRob force-pushed the fix-notifications-order branch from 42cc256 to 17225f9 Compare October 9, 2023 10:55
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming that the problem is only for the order in which recently received notifications arrive, then this looks like the right fix - if manual QA confirms the fix, this is good to go.

@MisRob
Copy link
Member Author

MisRob commented Oct 10, 2023

Assuming that the problem is only for the order in which recently received notifications arrive

I haven't observed any other problems but didn't really dig deep into other places

Copy link
Member

@pcenov pcenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented as specified, no regression issues observed.

@rtibbles rtibbles merged commit e67339e into learningequality:release-v0.16.x Oct 10, 2023
34 checks passed
@MisRob MisRob deleted the fix-notifications-order branch February 29, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) SIZE: small TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Status not reported simultaneously between Quizzes and Activity
3 participants