Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor teststrategies #791

Open
davidszkiba opened this issue Dec 23, 2024 · 1 comment
Open

Refactor teststrategies #791

davidszkiba opened this issue Dec 23, 2024 · 1 comment
Assignees
Labels

Comments

@davidszkiba
Copy link
Contributor

The organization of the question selection tasks (preselect-tasks) employed by the different test strategies via middlewares makes it unnecessarily complex. It can be made simpler by using template method.
It should be possible to simplify this step-by-step.

davidszkiba added a commit that referenced this issue Dec 23, 2024
This removes the checkitemparams.php preselect-task middleware and makes it a normal method of the strategy class.
davidszkiba added a commit that referenced this issue Dec 23, 2024
This removes the checkbreak pre-select task middleware and makes it a normal method of the strategy class.
davidszkiba added a commit that referenced this issue Jan 10, 2025
@ralferlebach
Copy link
Contributor

Yes! Yes! Yes! 😍

davidszkiba added a commit that referenced this issue Jan 16, 2025
This removes the checkitemparams.php preselect-task middleware and makes it a normal method of the strategy class.
davidszkiba added a commit that referenced this issue Jan 16, 2025
This removes the checkbreak pre-select task middleware and makes it a normal method of the strategy class.
davidszkiba added a commit that referenced this issue Jan 16, 2025
davidszkiba added a commit that referenced this issue Jan 16, 2025
davidszkiba added a commit that referenced this issue Jan 16, 2025
davidszkiba added a commit that referenced this issue Jan 17, 2025
davidszkiba added a commit that referenced this issue Jan 17, 2025
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 added a commit that referenced this issue Jan 17, 2025
@davidszkiba davidszkiba self-assigned this Jan 20, 2025
davidszkiba added a commit that referenced this issue Jan 24, 2025
This removes the checkitemparams.php preselect-task middleware and makes it a normal method of the strategy class.
davidszkiba added a commit that referenced this issue Jan 24, 2025
This removes the checkbreak pre-select task middleware and makes it a normal method of the strategy class.
davidszkiba added a commit that referenced this issue Jan 24, 2025
davidszkiba added a commit that referenced this issue Jan 24, 2025
davidszkiba added a commit that referenced this issue Jan 24, 2025
davidszkiba added a commit that referenced this issue Jan 24, 2025
davidszkiba added a commit that referenced this issue Jan 24, 2025
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 added a commit that referenced this issue Jan 24, 2025
mona-shakiba pushed a commit that referenced this issue Feb 10, 2025
This removes the checkitemparams.php preselect-task middleware and makes it a normal method of the strategy class.
mona-shakiba pushed a commit that referenced this issue Feb 10, 2025
This removes the checkbreak pre-select task middleware and makes it a normal method of the strategy class.
mona-shakiba pushed a commit that referenced this issue Feb 10, 2025
mona-shakiba pushed a commit that referenced this issue Feb 10, 2025
mona-shakiba pushed a commit that referenced this issue Feb 10, 2025
mona-shakiba pushed a commit that referenced this issue Feb 10, 2025
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.
mona-shakiba pushed a commit that referenced this issue Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants