catalyst / moodle-MDL-72752

GNU General Public License v3.0
0 stars 2 forks source link

MDL-72321 core_question: Implement question filtering #63

Closed tuanngocnguyen closed 2 years ago

tuanngocnguyen commented 2 years ago

This implementation will introduce question filtering using the de-cupled participant filter. It makes changes to the api to implement ajax based filtering as well as the quiz to accept those filtering criteria.

Co-Authored-By: Safat Shahin safatshahin@catalyst-au.net Co-Authored-By: Marc-Alexandre Ghaly marc-alexandreghaly@catalyst-ca.net Co-Authored-By: Nathan Nguyen nathannguyen@catalyst-au.net

MDL-72321 core_question: Implement question filtering

This implementation will introduce question filtering using the de-cupled participant filter. It makes changes to the api to implement ajax based filtering as well as the quiz to accept those filtering criteria.

Co-Authored-By: Safat Shahin safatshahin@catalyst-au.net Co-Authored-By: Marc-Alexandre Ghaly marc-alexandreghaly@catalyst-ca.net Co-Authored-By: Nathan Nguyen nathannguyen@catalyst-au.net

MDL-72752 mod_quiz: fix pagination and sorting on add question from question bank

MDL-72752 mod_quiz: add custom view for random question

WIP filterable object

filter description

PLEASE DO NOT OPEN PULL REQUESTS VIA GITHUB

The moodle.git repository at Github is just a mirror of the official repository. We do not accept pull requests at Github.

See CONTRIBUTING.txt guidelines for how to contribute patches for Moodle. Thank you.

--

tuanngocnguyen commented 2 years ago

Hi @safatshahin ,

I have updated the MR with fixed issues from the previous review, unit test, history plugin and rebase. Please review and let me know if there is anything I need to fix Thanks

safatshahin commented 2 years ago

Hi @tuanngocnguyen The patch looks good with the changes so far, we need to think about the restore bit but that can be done differently. The big problem with this patch is issues with commits and MR changes. There have been a lot of changes in the filtering base, for ex, regression fix, behat fix, etc, whatever is fixed, these commits seems to remove them as a part of the MR as your commits don't have them. I can see it's almost impossible to track what you changed and what the Mr is merging as a part of the commit as the commits are mixed and hashes don't match. What you will have to do is, bring your commits specifically for these changes, rebase against the filter base branch and then do an MR. Cheers

safatshahin commented 2 years ago

@tuanngocnguyen The patch looks good, we also need some unit test coverage for backup and restore of random question filter conditions as well as some scenario-based test coverage. Those can be covered in a different git issue. Cheers