search

catalyst / moodle-mod_reengagement

Allows timed release of content and emails users to remind them to complete a course activity
23 stars 39 forks source link

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

Open Urpokarhu1 opened 4 months ago

Urpokarhu1 commented 4 months ago

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 https://github.com/catalyst/moodle-mod_reengagement/issues/143

Attached is a video of the problem.

  1. Create re-engagement with settings
  1. Save and display
  2. Notice how the time isn't 10 minutes in the future but some random time.

https://github.com/catalyst/moodle-mod_reengagement/assets/53904029/565c3956-d5ac-48b6-b76c-ad739c07d96e

danmarsden commented 4 months ago

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 commented 2 months ago

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 commented 2 months ago

Great to hear :)