search

Syxton / moodle-block_massaction

THE OFFICIAL Mass Actions block for Moodle 3.9 and beyond
9 stars 14 forks source link

Duplicate to course (new section) #59

Closed Syxton closed 2 years ago

Syxton commented 2 years ago

@PhMemmel What do you think about this addition?

PhMemmel commented 2 years ago

@Syxton Nice idea! Will test and review soon.

Syxton commented 2 years ago

@PhMemmel Thank you for the great feedback. I have used your advice and also restructured a some now that I understand a bit better on what is going on. Please look at the latest changes and tell me what you think.

PhMemmel commented 2 years ago

Yes, code looks much cleaner now while being more stable, good job. There's just one point I directly added as review comment in the code which we should fix together with this newly added feature.

PhMemmel commented 2 years ago

I just found by accident that there is a core function course_create_sections_if_missing($courseid, $sectionnum). We should definitely make use of this instead of counting and adding sections ourselves in case we are importing to same sections the modules originally had. This function however does not solve the things with the orphaned sections, so we still have to keep this part of the code

Syxton commented 2 years ago

@PhMemmel Ok, I think I'm ready for another review. No rush.

PhMemmel commented 2 years ago

Very nice improvement of the code, good job! I did not manually test every single case, but in general it seems to work, we also have good test coverage (gj BTW ;-)), so we should be good to go with this. Thanks a lot! Very nice feature addition and improvement of my initial code!

Mergeable from my side