This is very similar to MDL-77090, but at the time, I missed that this
also needed to be handled. (Question metadata is, I think, only used
by the quiz 'Try another question like this one' feature.)
The behaviour-specific data was getting corrupted when the regrade
recreated the first step, because $oldstep->get_behaviour_data() was
stripping off the leading '-' characters from the names, and they were
not being added back.
This commit implements the necessary core hooks to ensure we only
allow a regrade of a quetion attempt to take place if the new and old
versions of the question are sufficiently similar.
It will be followed by commits to each question type where the
new method needs to be implemented.
Automated tests will be included in the first of those (mulitple choice)
becuse we need a question type that implements the hooks to test
the core changes.
As well as fixing the bug, I also rewrote the test to use
data providers, which should lead to more useful failure messages.
And, I moved the magic number we used as the float tolerence to
be a named constant.
That is, we don't call apply_attempt_state as soon as a
the data is loaded from the database. Instead, we wait
and only call it if really needed.
This should (especially after the next commit) be a performance win
during quizzes, particularly for people using advanced question
types liks STACK, or people making extensive use of the
'Try another question like this one' feature.
When the teacher is upgrading a previously entered grade, we re-display
exactly what they typed before if possible, rather than displaying with
a set number of decimal places.
* A method to change the max mark for one question_attempt in the usage
* A method to replace one question in a usage with another, moving the
old question_attempt to the end.
* Methods to set and get metadata (string name value pairs) for each
question_attempt in the usage. This gets stored in the first step in a
way that should not interfere with anything else.
Variant has two purposes. First to determine which version of the
question the student will see. Hence it is used to set up the state
of the quetsion when the question is started. Then the internal state of
the question is saved in the first step.
Once that has been done, the variant number is purely informative, and
just used to break down the statistics.
In some cases (the one I have in mind is qtype_opaque) then there is a
complex randomisation process, which may lead to several inital variant
numbers acutally giving the same version of the question. In this case
it is nice if the question can update the stored variant number, to make
the statistics more meaningful.
If the validation failed, so the manual grading page was re-displayed
with a validation error, then any comment that had just been typed in
was getting lost. This fixes that.
Before this, autosave was only working to save data when a student went
back in to continue an attempt. If the student, having crashed out,
never went back in and continued the attempt, their auto-saved responses
were not used when the attempt was automatically finished. That was a
rather bad oversight, which should now be fixed.
This parallels question_attempt->minfraction, which allows the
fractional mark to go below zere.
This is needed to allow the certainty-base marking behaviours to work
better.
When doing Each attempt builds on last, we need to copy any response
files into a draft file area, and then re-save them.
While writing the unit test for this, I had to deal with a todo in the
question engine so that questions with files in the response could be
unit-tested.
I also found an fixed a bug with qtype_essay_question::is_same_response
and fixed some notices in the existing essay/manual graded unit tests.
At the moment, when attempt is built on the last one, "not yet answered"
message is shown, which confuses many people. This patch modifies the state to
"complete" for attempt based on previous and modifies the output string.
Many thanks to Tim Hunt for guiding me through quiz infrastructure and some code
suggestions.
1. Autosave works in some ways just like a normal save. We ultimately
call $behaviour->process_save() to do the work, and create a new step to
hold the data.
2. However, we come in through a completely different route through the
API, starting with separate auto-save methods. This keeps the auto-save
changes mostly separate, and so reduced the chance of breaking existing
working code.
3. When the time comes to store the auto-save step in the database, we
save it using a negative sequence number.
This is a clever trick that not only distinguises these steps, but also
avoids unique key errors when an auto-save and a real action happen
simultaneously. (There are unit tests for these tricky edge cases.)
4. When we load the data back from the database, most of the time the
auto-save steps are loaded back as if they were a real save, and so the
auto-saved data is used when the question is then rendered.
5. However, before we process another action, we remove the auto-saved
step, so it does not appear in the final history.
Comment format (FORMAT_...) was correctly being processed when the
manual grading happened as the result of a form submission. It was only
when done using the question_usage or question_attempt API method that
there was no way to specify the format. (Although I think the only place
this API as used was in the unit tests.)
Note that question_attempt::manual_grade API had to change, but I don't
think that is a real API change. Calling code should be using
question_usage::question_attempt, which is backwards compatible.
Note that now, if you don't pass format, then no error is generated, but
a developer debugging message is generated.
In the case where either a question_attempt had not steps, or a
question_usage had not question_attempts, the load_from_records methods
could get stuck in an infinite loop.
This fix ensures that does not happen, with unit tests to verify it. At
the same time, I noticed an error in the existing tests, which this
patch fixes.
The code to upgrade attempts from before Moodle 2.0 to 2.1 created
attempt data that was not exactly the same as a new attempt created in
2.1+. This did not matter very much - revew and the quiz reports all
worked OK - but it broke on re-grade.
These changes detect the problem data in the re-grade code, an apply a
work-around so that the re-grade gives the correct result.
The problem was mostly that, in the past, we did not worry if
question_attempt_step.id changed during regrade (because we deleted the
old step row and inserted a new one). However, now that steps can have
associated files, we can't be that slack, becuase the step id is used as
the file itemid.
So, now, we have to update the existing rows during a regrade. We do
this by having the question engine tell the question_engine_unit_of_work
that the step has first been deleted, and then added back. Then we make
the unit-of-work spot that delete + add = update.
This also means that during regrading, we have to pass around some extra
ids so that new steps know the id of the step they are replacing.
Naturally, this requires some quite trickly logic, so I finally got
around to writing unit tests for question_engine_unit_of_work, which is
a good thing.
Along the way I also got around to renaming
question_attempt->set_number_in_usage, which got missed out when
everthing else was renamed to slot ages ago.
Finally, while working on this code, I noticed and fixed some PHPdoc
comments.
