The default move_to_category
function move_to_category($question,$tocategory->id,&$questions){
//move question
if (!set_field('question', 'category', $tocategory->id, 'id', $question->id)) {
error('Could not update category field');
}
Should the default return the error message?
The planned calculated question move_to_category() will be able to test for the presence of other calculated questions and eventually move all of them in the first call setting their $question->category to the $tocategory->id in the $questions array so there will be no more calls for these questions.
It could help if the preceeding
$sql = "SELECT q.*, c.contextid FROM {$CFG->prefix}question q, {$CFG->prefix}question_categories c WHERE q.id IN ($questionidlist) AND c.id = q.category";
if (!$questions = get_records_sql($sql)){
print_error('questiondoesnotexist', 'question', $pageurl->out());
include ORDER by q.qtype .

function move_to_category(&$question,$tocategory->id,&$questions){
//move question
if (!set_field('question', 'category', $tocategory->id, 'id', $question->id)) { error('Could not update category field'); } else { // set to the new category // either $question->category = $tocategory->id ; // or $questions[$question->id]->category = $tocategory->id ; }
}

• A user edits a question and changes the category without changing the context.
• A user moves a number of questions to another category / context using the 'Move to >>' form which is part of the question list in the question bank.
• A user uses the new move question actions icon to move a question.
• A user deletes a category and questions contained are moved to another category selected by the user.

I'm assuming if a whole cat and it's questions are moved in the category editing page then no special processing is needed by calculated question.

You just want to do some extra processing and db operations Pierre?? You don't need to be able to print any forms or buttons before moving the question and you don't want the option to refuse to move the questions?

The actual coding is to use the actual dataset if identical or create a new one (this could happen if the question has been moved) or use a private one if there is already a shareable dataset with the same name but a different definition.
At the time being I don't know how to settle the problem (should we drop shareable datasets?).
When the new extended functions calculated question will work, this could be settled.

I do think however that on the long term, given the new plug-in types, the modification of a question parameter should be done by a call to a questiontype function even if at the time being the default function do the same thing that ryour code is doing.
So having a fom that confirm the moving as in the delete process will be a good thing and a better one if the questiontypes could display specific messages.
As the calculated questiontype is the only one that do have possible problems when moving from a category, it's specific move_to category() function will not be available in the next weeks so we can wait (but not close the bug).

However is the moving process should be controlled at the upper level. ?
For doing this we will need more than one function.
A first one is_question_movable_to_category()
The moving process (the one you are coding) should display each question with a select yes or no to confirm the move with potentially a message from the questiontypes to tell the user of the potential problem when moving the question.
In the case of calculated question the issue is more complicated.
case 1: all the calculated questions from the category are selected
A :and there is NO identical shareable dataset in the new category.So let the questiontype move everything (questions and datasets)
B: there is a similar dataset (same name) but with other definition parameters (Min, MAX etc).
1: Yes the user accepts to use the existing category
2: the user want to substitute the existing category with the one moving
3: the user want the conversion of the existing category in a private category for each of the questions moved.
case 2: only some question are moved
C: the user want to add the other questions or not
then the precedding A and B applied.

All these complicated issues for a known questiontype illustrate the potentially difficulties with the unknown new plug-in questiontypes.
Finally(or perhaps) the best way is to let each questiontype managed the moving of its questiontype questions and set a parameter (using the $questions object?) so that the moving process knows which questions were moved
(and if the category can be deleted...)

Eliminating these shareable datasets and allowing only private datasets and everything is more simple. But this is another debate that we could set when the new extended calculated will be completed....

Don't want to spend time on this until I can see what exactly it is needed for.

• Move question to another category
  *
• The default function set the question fiel parent to the new category id.
• Question types with complex structure could set other parameters to
• reflect the category change i.e. for multianswer qtype the subquestion question records
  *
• @param object $question The question that is moved
• @param integer $tocategoryid The id of the new category
  */
  function move_question_to_category($question, $tocategoryid)
  Unknown macro: { if (!set_field('question', 'category', $tocategoryid, 'id', $question->id)) { return false; // error('Could not update category field'); }
  return true;
  }
  
  changing some lines in editlib.php (around 449) and global QTYPES line 415
  if (!$checkforfiles){
  foreach ($questions as $question){
  if(! $QTYPES[$question->qtype]->move_question_to_category($question, $tocategoryid)){ error('Could not update category field'); }
  }
  redirect($returnurl);
  It works correctly for normal questions.
  I had the following somewhere in multianswer/questiontype.php
  
  function move_question_to_category($question, $tocategoryid){
  global $QTYPES;
  if (!set_field('question', 'category', $tocategoryid, 'id', $question->id)) { return false; // error('Could not update category field'); } }
  
  // Get relevant data indexed by positionkey from the multianswers table
  if (!$sequence = get_field('question_multianswer', 'sequence', 'question', $question->id)) { notify('Error: Cloze question '.$question->id.' is missing question options!'); return false; }

$wrappedquestions = get_records_list('question', 'id', $sequence, 'id ASC');
foreach ($wrappedquestions as $subquestion){
if(! $QTYPES[$subquestion->qtype]->move_question_to_category($subquestion, $tocategoryid)){ return false; }
}
return true;
}
I works correclty and the category field of the question records of the subquestions were correctly modified.
So the database question table category field follow the moving from one category to another.

Tim and jamie I am waiting for your approval to CVS the changes.

Hopefully I will have time to think aobut this properly on Monday or Tuesday.

Tim
Here are the diff created by turtoise.
The main purpose is that the moving being done at the questiotype level because some actual questiontypes ( m,ultianswer, calculated soon) are storing subquestion in the question table and the categoryid field sould be valid for each question records.
Furthermore this will allow also calculated question change other category fields ).
Also it allows plug-ins to be more versatile.
I don't know where the error message should be put ideally, so I ask your comment on this.

Normally, you create a single patch file containing all the changes in all files. That is much easier to work with that a zip file with lots of separate patches.

Is there actually a bug at the moment with moving multianswer questions? or is this just a code cleanup?

This is not a bug as is because in the code, the subquestions are identified by their id independent from the category field.

There is no bug in the actual backup and restore as they backup all the questions in the course categories.

With the new code of Jamie multianswer questions could migrate almost anywhere on a moodle site i.e in different category, in different courses but their subquestions won't follow because they will conserve their old category id.
Here the backup and restore will fail as it is base on the course categories.

I am not sure also of what happen when you delete a category.

My proposal is just to be sure that the question->category is always a valid parameter.
I do understand that as a pro in coding you dislike non valid parameters but as an overloaded solving bugs pro, you want to be sure that you are not creating a new bug...

Effectively the subquestions remain in the original category.
They should move transparently for the user.
questions can be moved using the normal move function for which I submit a code proposal
or when editing them if the question saving process do it rigth see
http://moodle.org/mod/forum/discuss.php?d=83258

A. The fact that questions can refer to images and other files. Fortunately we have a robust solution to this in 1.9.

B. multianswer questions, where some of the question data is subquestions which also live in the mdl_question table, and so have their own question.category fields.

C. calculated questions, where datasets can be linked to categories.

So, B. and C. are the problems we need to solve.

Jamie, in his comment http://tracker.moodle.org/browse/MDL-10899?focusedCommentId=33593#action_33593 identifies 4 ways that questions can be moved:

1. A user edits a question and changes the category without changing the context.
2. A user moves a number of questions to another category / context using the 'Move to >>' form which is part of the question list in the question bank.
a. and there are no files to move.
b. and there are files to move.
3. A user uses the new move question actions icon to move a question.
a. and there are no files to move.
b. and there are files to move.
4. A user deletes a category and questions contained are moved to another category selected by the user.

1. and 3a. currently go through the save_question method of the question type which gives the question types full control, and this currently does the right thing for all existing question types.

2a. is done by line 450 of /question/editlib.php. This needs to be changed to handle B. and C.

2b and 3b are done by /question/contextmoveq.php, around line 163. This needs to be changed in a similar way.

4. is done by line 355 of /question/category_class.php and also needs to be changed.

In the near future, after MDL-14378 is fixed, there may also by a 5th case, however, this can probably use the same code as 4.

Anyway, that is three similar areas of code that need to change, so that says to me we need a function or method.

Because I don't think questions should have data linked to the category, I don't think we should have a move_question_to_category method on the question types. Instead we should just have a function in questionlib that deals with cases B. and C. That does mean question-type specific code in questionlib, but I think that is the lesser of two evils.

"The fact that questions can refer to images and other files. Fortunately we have a robust solution to this in 1.9. "
You have a robust solution because you create a similar questiontype function question_find_file_links_from_html().

Why was is good for files moving is not good for other question data?

What I think we need for future development and actual plug-in, is simply a function that have the initial category and the final category set with all roles or permissions already done. For most questions, this will be effectively only change the category field in the question table. For others it can be other data but effectively not moving files as this is already done with question_find_file_links_from_html.

"Fortunately we have a robust solution to this in 1.9"
a robust solution is as robust as its components.
Perhaps I miss something but I did not find any search for find_file_links_from_html() in the answer->feedback.

should be modified by something like

foreach ($question->options->answers as $answer) { $urls += question_find_file_links_from_html($answer->answer, $courseid); $urls += question_find_file_links_from_html($answer->feedback, $courseid); }
and similarly in function replace_file_links()

and other questiontypes...

a new bug or a new task?

As for multichoice, multichoice->find_file_links calls parent::find_file_links, and that does

if ($this->has_html_answers() && isset($question->options->answers)){
foreach ($question->options->answers as $answerkey => $answer){
$thisurls= question_find_file_links_from_html($answer->answer, $courseid);

so all is well there.

Actually because
.1 calculated question has no control about the moving process
2, the user should be able to move or create a single question even if it use category shared data
3. I want that the process to be transparent to the user

the dataitemdefinitionss are automatically created when the user finished the first page of the editing process.
If it is a new question the datasets are created as private,
if the user is modifying a question and
the old question has category level dataset that does not exist in the new category , it is created automatically.
or if there is an already existing dataset with the same name and the same definition (limit etc.), then it is used
or if there is an already existing dataset with the same name but different definition then a new private dataset is created.

At the second page the user can change these choices the available category datasets are always displayed either in the first page or the second one.
When the question is used , the dataitems are located without reference to the category so the questions are always working (as long as the user has created them in the third page..).
So dataset moving is not a real problem.
These modifications has been merged down to 1.6ffor many months wthout any problems.

So better not to change this, unless you agree to create a supplementary step in the moving process allowint to choose which calcualted question should be moved or not.

The subquestion moving is not a problem if the subquestions remain in the database as the sequence store the subquestions id.
Moving them when the question move, will eliminate all risks of deleting them and also will eliminate a quite surprising message to the user that there are questions in an apparently empty category peculiarly if he as set to show the hidden questions.

if ($this->has_html_answers() && isset($question->options->answers)){
foreach ($question->options->answers as $answerkey => $answer){
$thisurls= question_find_file_links_from_html($answer->answer, $courseid);

so all is well there."

Soory, I don't undrestand how question_find_file_links_from_html($answer->answer, $courseid) will find the links in $answer->feedback

function question_find_file_links_from_html($html, $courseid){
global $CFG;
$baseurl = question_file_links_base_url($courseid);
$searchfor = '!'.
'(<\s*(a|img)\s[^>]*(href|src)\s*=\s*")'.$baseurl.'([^"]*)"'.
'|'.
'(<\s*(a|img)\s[^>]*(href|src)\s*=\s*\')'.$baseurl.'([^\']*)\''.
'!i';
$matches = array();
$no = preg_match_all($searchfor, $html, $matches);
if ($no){
$rawurls = array_filter(array_merge($matches[5], $matches[10]));//array_filter removes empty elements
//remove any links that point somewhere they shouldn't
foreach (array_keys($rawurls) as $rawurlkey){
if (!$cleanedurl = question_url_check($rawurls[$rawurlkey])){ unset($rawurls[$rawurlkey]); } else { $rawurls[$rawurlkey] = $cleanedurl; }

}
$urls = array_flip($rawurls);// array_flip removes duplicate files
// and when we merge arrays will continue to automatically remove duplicates
} else { $urls = array(); }
return $urls;
}
the $html parameter does not contains the $answer->feedback ...

Got to go now. I'll answer your other point later.

Back to the moving questions issue. Pierre, you are setting out good arguments, which is making me think and justify my position, but I still think I am right. Let me try to explain my reasoning further.

You say: "The plugin structure should be as open as possible." and I disagree. In Moodle, there is a division of responsibility between the question bank and the individual question types. So I think the correct statement is "Question type plugins should have the flexibility to do anything within their area of responsibility." And I think organising questions into categories definitely belongs to the question bank, not to individual questions.

So I think it is up to the question bank to solve problems A. B. and C. It is only A. that involves asking the question types to list and update the links they contain. The question bank is quite capable of handling child questions and datasets on its own.

I have just updated http://docs.moodle.org/en/Development:Quiz_database_structure to include the dataset tables. I hope I have got the right links. I really don't like the link from dataset_definitions to question_categories. I think it should be like numerical units, which are private to the question. (That does not stop you having an option in the user interface for copying datasets from other questions in the category.) Anyway, that is history. We still need to cope with dataset as they are now, and the questionbank can do that in a function.

I have nearly finished writing the code. I will attach it soon for review.

Also tested for ordinary question types like shortanswer and description.

And Jamie just gave it a quick review and could not spot any problems, so I am going to check it in.

I have just created MDL-14767 for the dataset issue.

The actual coding of calculated question handle this as the best compromise between history and actuality.

And the datasets being something specific to calculated question although there was a provision to handle things different from numbres, the move function should be maintained at the questiontype level even if it was only to respect class coding.

The datasets category option was designed because teachers can create a series of number and ask different mathematical questions on these numbers.

Remember that the teacher can either let random genration defined the numbers or he can set them individually (i.e I create volumes of solution following the real flaks available in the laboratory).

I am creating the cloze calculated questiontype because it is more realistic to ask for the mean and standard deviation of the same datasets.
Other question types could want to share values at the category level.
So this is why I think that internal (database) category specific parameters should be handled at the questiontype level.

Teachers have somewhat specific needs that are surprising.
For example in WebCT I hacked the quiz code to synchcronize data between the different questions in a quiz and in the recent quiz forum post there was a hack that use a description type question to handle a javascript to change the cloze display...

I think that with the roles and new category code, a lot of parameters are defined for questions and this could not move so much in the future unless...

Is it the real time to limit what questions can and cannot do in Moodle?

In some aspect yes but this should not compromise the future.

You want to restrict the use of category but already there are questions like random short answer matching that use the category. I could think of linking calculated questions. etc.

As in society, freedom is the best solution until someone break the rules...

Actually moving dataset should be left when the user edit the question because he can manage the name conflict as he knows more about the parameters.
So you can forget B because the calculated questions remain useable even if the datasetdef category is not modified.
On reflexion, the edit_calculated_question_form.php could be improved to a better handling.
For example we could also warn the user that the dasetdef category is not the same as the question category and show him which questions are using this dasetdef . etc.

About C It will be quite easy to change the category id of all questions with have the parent_id set to the question id that is moving and this can be done at the questionlib level.

So actual calculated question even the multianswers proposal and the actual multianswers can be managed with your solution.

My proposal was more about openness as a principle and yours about openness and code stability.

If your proposal becomes a problem for a valuable plug-in new questiontype, we could see how to manage this.

Can we agree to a wait and see policy...

In the mean time solve C and put B on the bottom of your TODO list.

I will explore probably after the summer holidays, how to improve the dataset handling for example when they are used by questions in a quiz.