Refactor/remove teststrategy middleware #795
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes the checkitemparams.php preselect-task middleware and makes it a normal method of the strategy class.
This removes the checkbreak pre-select task middleware and makes it a normal method of the strategy class.
Since we are calling the preselect tasks directly via run now and do not call the `process()` method in the abstract preselect_task class, the `get_required_context_keys()` method is not called anymore. Generally, this was not the best approach because its easy to use a context key that is not listed in the `get_required_context_keys()` method, so the actual code and the things defined there might diverge. A better approach would be to use a class instead of an array for the $context variable (e.g. preselect_context) and make sure it contains all required fields as properties.
davidszkiba
force-pushed
the
refactor/remove-teststrategy-middleware
branch
from
48787e4 to
f37d354
Compare
|
I've tested it but since it's a bigger change, I created a merge commit instead of rebasing so that in case something does not work as expected, it would be easy to revert the changes from this PR. |
mona-shakiba
pushed a commit
that referenced
this pull request
Feb 10, 2025
mona-shakiba
pushed a commit
that referenced
this pull request
Feb 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #791
Instead of using a set of middlewares to select the next question, the process is simplified a bit.
All required preselect tasks are now called in the strategy class. They can be overwritten by the subclasses of the different strategies.
For example, in strategy.php the preselect task
filterbytestinfois executed. If this is not needed for the classical CAT strategy, it can overwrite the call to this task to do nothing.Basically, this is implementing the template method design pattern.