Fix section checking

[tuanngocnguyen]

Fix for

#135

[PhMemmel]

Thank you for raising the issue and providing a fix. I think we reached a point where all of this, especially dealing with the restriction hooks require to be covered by detailed unit tests, especially covering the cases with and without present hooks. Would you mind adding some?

[tuanngocnguyen]

Hi @PhMemmel,

Thanks for your comment, I have added unit test for filter_sections_different_course hook. The test without hood should already covered in test_mass_duplicate_modules_to_course.

[tuanngocnguyen]

Hi @PhMemmel ,

The patch has been tested on my local dev, would you please review once you have time?
Thanks

[PhMemmel]

Thank you very much for your work.

• The changes make sense to me
• Code is well documented
• I also tested manually and could not find a regression
• Coverage is good, thank you for adding the test

Just one minor annotation: If I'm not overlooking anything you could make the tests a bit less complicated/more readable by using $this->redirectHook(...) in the unit test method. By using this you do not need to create and inckude any fixture files but instead just define the hook callback function as private function in the test class or even anonymous callback function directly in the test method itself. This at least has shown very helpful to me when I wrote unit tests for hooks.

[PhMemmel]

Could you also please rebase your branch to the latest master? I merged the updated GHA workflow config so pipelines are green (at least for Moodle 5).

[tuanngocnguyen]

Hi @PhMemmel ,

Thanks for your review and suggestion.

I have updated the patch with redirectHook, and removed fixtures.
I have also rebased my branch.

[PhMemmel]

Thank you very much for your contribution!

[PhMemmel]

for documentation purposes: failing main branch GHA not related to this PR

[tuanngocnguyen]

Thanks @PhMemmel