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

Add Adhoc Task for Each Course Module #164

Merged
merged 4 commits into from
Sep 24, 2024

Conversation

rajandangi
Copy link
Contributor

This update introduces an adhoc task for each course module.

  • Added a new file classes/task/reengagement_adhoc_task.php to handle the adhoc task logic.
  • Moved portion of code form lib.php to integrate the adhoc task functionality.

@rajandangi rajandangi requested a review from danmarsden August 27, 2024 00:06
@rajandangi rajandangi force-pushed the add/course_module/addhoc_task branch from 7a29509 to 65e5fe5 Compare August 27, 2024 07:43
@rajandangi
Copy link
Contributor Author

rajandangi commented Aug 29, 2024

The CI failed due to deprecated 404 Behat tests, which aren't related to this change. However, I've addressed the 404 test issue in this branch. If you prefer, we can create a separate branch specifically for the 404 fix, including the updated Behat test."

@petersistrom
Copy link
Contributor

Hi @danmarsden
I've reviewed and tested @rajandangi's PR and it looks good to me, no new code was added it's just splitting the scheduled task into adhoc tasks for each course module. Just checking in with you to make sure that in principle it is something you are happy to have merged in?

@danmarsden
Copy link
Member

no objections to the concept - if someone has carefully peer reviewed the patch and made sure it does pretty much the same thing - of course this needs to land in the latest branch "first" before the 400_STABLE branch is merged in but happy for you to hit the merge button on your end if that's done.

Refactor reengagement_adhoc_task.php to remove unnecessary check for empty startusers array

Fix typo in reengagement_adhoc_task.php comment
Fix: ci codechecker errors
@rajandangi rajandangi force-pushed the add/course_module/addhoc_task branch from 8035bd0 to 68d7f81 Compare September 19, 2024 09:43
@rajandangi
Copy link
Contributor Author

Thanks, @danmarsden , for creating the new MOODLE_404_STABLE branch.

Since we now have a dedicated branch for 404, all the tests are passing on this PR. Please check and confirm.

@rajandangi
Copy link
Contributor Author

rajandangi commented Sep 20, 2024

no objections to the concept - if someone has carefully peer reviewed the patch and made sure it does pretty much the same thing - of course this needs to land in the latest branch "first" before the 400_STABLE branch is merged in but happy for you to hit the merge button on your end if that's done.

I’ve created another PR #174 for the latest branch by cherry-picking the commit that splits the scheduled task into an adhoc_task for each course module. Please check and confirm.

@petersistrom petersistrom merged commit ff91925 into MOODLE_400_STABLE Sep 24, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants