StudentQuiz: There are some issues of Add completion criteria #732619 #474
Conversation
|
Hi @timhunt, |
| */ | ||
| public static function update_state(stdClass $course, $cm, ?int $userid = null): void { | ||
| public static function update_state(stdClass $course, $cm, ?int $userid = null): bool { |
There was a problem hiding this comment.
I don't think it is a good idea to change the return type just for unit testing. Your unit test should verify the real outcome, not some artificial thing.
| */ | ||
| public static function update_state(stdClass $course, $cm, ?int $userid = null): void { | ||
| public static function update_state(stdClass $course, $cm, ?int $userid = null): bool { |
There was a problem hiding this comment.
I think update_state is not a very accurate name for what this function does. Would trigger_completion_state_update be more accurate?
tests/custom_completion_test.php
Outdated
| $cm = cm_info::create(get_coursemodule_from_id('studentquiz', $studentquiz->cmid)); | ||
| $completioninfo = new \completion_info($course); | ||
| $customcompletion = new custom_completion($cm, $user->id, $completioninfo->get_core_completion_state($cm, $user->id)); | ||
| $status = $customcompletion::update_state($course, $cm, $user->id); |
There was a problem hiding this comment.
This is a static method. The call should be custom_completion::update_state, with no need to create an instance.
tests/custom_completion_test.php
Outdated
| false, | ||
| ], | ||
| 'Students can manually mark the activity as completed' => [ | ||
| 1, |
There was a problem hiding this comment.
Why are you using magic numbers here, and not the constants like COMPLETION_TRACKING_MANUAL ?
BruceGoodGuy
force-pushed
the
wip732619
branch
from
1e4f577 to
43e361d
Compare
|
Hi @timhunt , Thanks for your feedback. In this commit, I have made two changes:
|
|
Thanks. Merged. |
No description provided.