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

Notify time shows random time not the next message send time #151

Open
Urpokarhu1 opened this issue Apr 17, 2024 · 3 comments
Open

Notify time shows random time not the next message send time #151

Urpokarhu1 opened this issue Apr 17, 2024 · 3 comments
Labels

Comments

@Urpokarhu1
Copy link

Urpokarhu1 commented Apr 17, 2024

Hello,

tested with version mod_reengagement 2023020804. First notify time isn't correct, it shows some random time not the the next message will be sent.

This might be related to that notify times are universal and shared between courses and re-engagement activities #143

Attached is a video of the problem.

  1. Create re-engagement with settings
  • Notify user: after delay
  • Notification delay: 10 minutes
  • Reminder count: 1
  • Reengagement duration: 1 week.
  1. Save and display
  2. Notice how the time isn't 10 minutes in the future but some random time.
timebug.mp4
@Urpokarhu1 Urpokarhu1 changed the title Notify time show current time, not next message send time Notify time shows current time, not the next message send time Apr 17, 2024
@Urpokarhu1 Urpokarhu1 changed the title Notify time shows current time, not the next message send time Notify time shows arbitrary time not the next message send time Apr 17, 2024
@Urpokarhu1 Urpokarhu1 changed the title Notify time shows arbitrary time not the next message send time Notify time shows random time not the next message send time Apr 17, 2024
@danmarsden danmarsden added the bug label Apr 17, 2024
@danmarsden
Copy link
Member

Thanks @Urpokarhu1 - yeah there's defnitely some improvements that need to be made with that report - hopefully one day it will annoy someone enough to either fire us a pull request, or one of our existing clients will ask for us to clean it up!

@weilai-irl
Copy link

Hi @Urpokarhu1,

I have made a PR #155, which mainly improves scheduled task performance, but also contains the fix to this issue.

The root cause of the issue is, when a student access the plugin view.php page, the completion time / email time displayed is fetched by finding a random in progress record of the user, without limiting the in-progress record to the current mod_reengagement activity only.

@danmarsden, please review the PR. It's a good strategy to wait for PR after all 👍

Regards,
Lai

@Urpokarhu1
Copy link
Author

Great to hear :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants