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.
Users should only be able to access their own quetion preview. In the
past, for reasons I can no longer remember, this was enforced
using the session. It is much better to set the question_usage to belong
to the user's context.
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 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.
The standard themes do not use this for anything, but it makes it easier for themers to do cool stuff.
Also improve the API for getting the question state class.
