StudentQuiz: Deleted orphaned question is not working as expected after MDL-78025 is implemented

[vuvanhieu143]

Hi @timhunt ,

This issue is ready for peer review. I have added two separate commits: 1) To fix this issue, as I have discussed with you, we will change the status of the question to draft when we disapprove of a question in StudentQuiz. 2) To update the legacy data.

[timhunt]

Automated tests:

1. CI fail looks real. Needs to be fixed.
2. We should add an additional commit to update the moodle-plugin-ci config, to run on all supported Moodle branches (a bit like https://github.com/moodleou/moodle-qtype_crossword/commit/aedc498c56598968bb2abfec66f1112016517792)

Main code

1. classes/local/studentquiz_question.php - change_state_visibility - the huge if statement here is not very readable. A better way to do this, both for simplicity of code and self-documentation is: Have an array like [self::STATE_DISAPPROVED => question_version_status::QUESTION_STATUS_DRAFT] which defines the logic ...
2. Then ... well, the change_state_visibility method is not even very helpful. Why is this a separate function? I would expect the work of udpating the state of the question in the question bank to be part of the funciotn that updates the studentquiz_question state, so it is impossible for the two things to get out-of-synch. Right, we have a huge chunk of duplciate logic between changestate.php and change_question_state in externallib.php That should be fixed.
3. And, the fact that both those places end up calling change_state_visibility twice in quick succession really shows this code is not well designed.
4. classes/question/bank/studentquiz_bank_view.php build_query - "qv.status <> '$state'", we shoudl be usign a placeholder to insert the value into the SQL.
5. ensure_studentquiz_question_status_is_always_ready - the coment here says "Make sure questions created/edited in StudentQuiz is always ready." but that is now incorrect.

Upgrade code:

1. If you are only doing a single $DB->execute, you don't need a transactoin.
2. The SQL is unnecessarily complicated. No need to have {question} and {question_versions} in the subquery (unless I am wrong). Just do UPDATE {question_versions} SET status = 'draft' WHERE questionbankentryid IN (...)
3. Then, there is no need to have two levels on nested subqueries.

[vuvanhieu143]

Hi @timhunt , Thank you for your feedback.

• For points 1,2,3. This change state code will be improved on 685932 since this issue is caused by out-of-sync data when we change the state.
• I have updated the code to use a placeholder
• I have added a commit to add MOODLE_402_STABLE with PHP 8.1 and I fix some warning issues.
• I have changed the name of the function to ensure_question_version_status_is_correct and change the description.
• I have updated the upgrade steps. Thank you, the query is more clean and optimized

[timhunt]

Thanks. Nice, and speedy, work.