Details
-
Type:
Sub-task
-
Status:
Reopened
-
Priority:
Minor
-
Resolution: Unresolved
-
Affects Version/s: 1.9
-
Fix Version/s: STABLE backlog
-
Component/s: Questions
-
Labels:None
-
Affected Branches:MOODLE_19_STABLE
Description
Actually (as in older version) moving a question in another category is done by modifying the question->category parameter as in
line 486 of question/editlib.php
//move question
if (!set_field('question', 'category', $tocategory->id, 'id', $questionid)) {
error('Could not update category field');
}
Calculated question can use category shared dataitems sets and just changing the category is not sufficient.
I suggest that a moving_to_category() function being created in questiontype and that the default function could be the one used.
I let you create the function and I will work on a specific one for calculated question.
I suggest that the entire list i.e. $questionids be passed to the function because if you move all or part of the calculated questions that are in a category, the moving process is different.
Attachments
-
- calculatedquestiontype.php.patch
- 29/Nov/10 11:53 PM
- 18 kB
- Pierre Pichet
-
- calculatedsimplequestiontype.php.patch
- 29/Nov/10 11:53 PM
- 0.6 kB
- Pierre Pichet
-
$i18n.getText("admin.common.words.hide")
- movecategory.zip
- 25/Aug/10 12:21 AM
- 4 kB
- Pierre Pichet
-
- questiontype.php.patch 15 kB
- questionlib.php.patch 0.9 kB
-
- moving_associated_data_with_questions.patch.txt
- 09/May/08 9:50 PM
- 5 kB
- Tim Hunt
-
$i18n.getText("admin.common.words.hide")
- question_type_questiontype_diff.zip
- 06/Oct/07 11:43 PM
- 2 kB
- Pierre Pichet
- Download Zip
$i18n.getText("admin.common.words.show")- question_type_questiontype_diff.zip
- 06/Oct/07 11:43 PM
- 2 kB
- Pierre Pichet
-
- questionlib.php.patch
- 29/Nov/10 11:53 PM
- 3 kB
- Pierre Pichet
-
- questiontype.php.patch
- 29/Nov/10 11:53 PM
- 2 kB
- Pierre Pichet
-
- category_delete.jpg
- 87 kB
- 28/Oct/07 1:38 AM
-
- category_edit.jpg
- 86 kB
- 28/Oct/07 1:37 AM
Issue Links
| This issue will help resolve: | ||||
| MDL-14767 | Need to implement the bit in question_move_questions_to_category that handles datasets |
|
|
|
| MDL-6597 | Cloze questions sometimes get corrupted |
|
|
|
| MDL-14625 | Make multianswer questions more tolerant if one of the subquestions is missing |
|
|
|
| This issue will be resolved by: | ||||
| MDL-14762 | Moving files in answer->feedback when moving questions |
|
|
|
| MDL-25566 | Allowing multiple datasetdefs with identical name in the same category |
|
|
|
Activity
- All
- Comments
- History
- Activity
- Source
- Test Sessions
Should the default move_to_category also change the $question->category to the $tocategory->id , just in case?
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 ;
}
}
This is going to mean changes to quite a few places in the code. The category of a question is moved when :
- 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?
- 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.
The category shareable datasets will be much less usefull when I will finish (end of september) the new extended calculated that will allow multiple questions (or responses) in one question using the same datasets.
Given the various ways to migrate a question even when editing the question, perhaps the best answer is code the calculated questiontype so that it can recover from a category dataset migration.
The actual code allows the use in a quizz of a category dataset even if the question has been moved in another category.
The call being
function create_session_and_responses(&$question, &$state, $cmoptions, $attempt) {
// Find out how many datasets are available
global $CFG;
if(!$maxnumber = (int)get_field_sql(
"SELECT MIN(a.itemcount)
FROM {$CFG->prefix}question_dataset_definitions a,
{$CFG->prefix}question_datasets b
WHERE b.question = $question->id
AND a.id = b.datasetdefinition")) {
error("Couldn't get the specified dataset for a calculated " .
"question! (question: {$question->id}");
}
The problem is when you edit a calculated question because then the call is to see if there is a dataset with the same name in the category.
The call being
if ($sharedatasetdefs = get_records_select(
'question_dataset_definitions',
"type = '1'
AND name = '$dataset->name'
AND category = '$question->category'
ORDER BY id DESC;"
)) { // so there is at least one
$sharedatasetdef = array_shift($sharedatasetdefs);
if ( $sharedatasetdef->options == $datasetdef->options ){// identical so use it
$todo='useit' ;
$datasetdef =$sharedatasetdef ;
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).
Another reason to set a questiontype/move_to_category() function is that when you delete a category after moving questions to another category, in the case of calculated question the 'question_dataset_definitions->category points to a no more valid parameter.
Although this is the actual situation, it is not what it should be.
So let the questiontypes move themselves their questions and associated data.
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....
To introduce some extra questions for the user when moving a question is going to be a pain and a significant ammount of extra work I don't want to do. But actually there is no new problem in the code, right? This problem must have been there for calculated and dataset dependent questions for some time?
Yes this is a problem from the begining.
So there is no hurry in solving it and it should be done when editing the calculated questions because actually the questions can be used in quiz without problems.
On the other end, to get the moodle code use the class structure as much as possible, the moving (i.e. in most cases just changing the $question-category) should be done by a questiontype class function as suggested given the unknown structure of new plug-ins.
As you are more a professional programmer than me, it is up to you.
It is still not clear to me exactly what needs doing here, what the purpose of any changes would be.
Don't want to spend time on this until I can see what exactly it is needed for.
I am developping an extended version of the calculated question that will allow multianswers by using subquestions created as question records with the parent being the main question.
Actually as the moving of one question to another category is done at the category level by changing the category id in the question record, the main calculated question is the only one which will have its category id changed.
My proposal is to let the main question do the necessary categoryid changes to its subquestions.
I will create the necessary code modification, test it and submit it to Jamie.
The category id is not used by the main calculated question to retrieve its subquestions so my actual code is working even if the question is moved to another category.
However the main backup process work by selecting the questions on a category basis so it does not retrieve the subquestions if the question has been moved and the subquestions has not been moved also.
Doing the moving at the question level (or as questiontype function) assure that the subquestions will be at the rigth place and will be correctly backup and restored.
the default questiontype move_to_category() function will just do the actual change to the category_id.
This could be the case also for multianswer (cloze) question.
However actually the backup back all questions from all categories in one course so the problem does not appear but could if the main backup procedure is changed.
nevertless it is no a good code practice to have parameters like category id that either don't reflect the reality or in a bad case could indicate a category id that is no more valid.
I have ot check what happen when category are deleted.
I have test the following changes on my system (actual HEAD)
Adding a new function to question/type/questiontype.php
/**
- 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.
- 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; }
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.
Sorry there are two "diagnostic" lines to remove in the multianswer/questiontype
+ echo " question <pre>"; print_r($question);echo " </pre>";
+//exit;
Effectively these lines confirm that the $question object from editlib.php is not the complete question object but the question table record.
"It fails to deal with the case where questions are moved using the fields at the top of the question editing form."
There is no problem as this is solved (some supplementary check to do here) in the code of multianswer which reuse or create new question as subquestion in the database with the new category field.
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...
THIS IS A BUG at least for multiple answers (so with subquestions) (Cloze) question type
I create a Cloze question in a new category named "cloze (del)"
I move the question to another category using the normal move function when the questions are listed..
The category "cloze (del)" was empty
I go to the category edit page and effectively the "cloze (del)" shows as "cloze (del) (0) " i.e no questions. (fig category_edit.jpg)
I click on the delete (X) button and obtain the following message (fig category_delete.jpg)
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
Finally this is a bug, so I will continue on this as Cloze is also involved (MDL-6597) .
Tim and Jamie,
What is the best way to handle questions moving to another category if the questions have subquestions that should also be moved (i.e cloze question) or other data (calculated).?
I propose a new function so that this task is transferred to the question type that knows its requirements for moving from one category to another.?
What about files associated to the questions?.
Tim
can you comment on how to solve this "bug".
see http://moodle.org/mod/forum/discuss.php?d=93201#p411453
Having thought about this, I have concluded that it is actually unfortunate that it is not possible to move questions between categories just by changing the question.category field. I think that any data associated with a question should belong just to that question, and so we should not have direct links between question data and categories. Extra data should only link to questionid. However, there are at least three senses in which the world is not ideal here:
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.
However having a function in questionlib means that this limit B and C to the actual calculated questiontype and the actual multianswer type.
I have alreaday an almost finished project of a multianswer calculated question_type and new plug-in types could be of the multianswers question type.
Asking a question to move its components appears to me the natural way to do it.
"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.
So my proposal is that we create a move function for other question data excluding the files as they are part of another process.
"Perhaps I miss something but I did not find any search for find_file_links_from_html() in the answer->feedback."
more precisely
in multichoice/questiontype.php
function find_file_links($question, $courseid){
$urls = array();
// find links in the answers table.
$urls += question_find_file_links_from_html($question->options->correctfeedback, $courseid);
$urls += question_find_file_links_from_html($question->options->partiallycorrectfeedback, $courseid);
$urls += question_find_file_links_from_html($question->options->incorrectfeedback, $courseid);
foreach ($question->options->answers as $answer) {
$urls += question_find_file_links_from_html($answer->answer, $courseid);
}
//set all the values of the array to the question id
if ($urls){
$urls = array_combine(array_keys($urls), array_fill(0, count($urls), array($question->id)));
}
$urls = array_merge_recursive($urls, parent::find_file_links($question, $courseid));
return $urls;
}
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?
Pierre, about other question types: When I say I only intend to support multianswer and calculated, that is not really true. What I mean is that in the moving code, I will only support updating the category of datasets and other questions that are children of the one being moved. I assume that your new question types just use these to existing mechanisms, so they will work.
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.
"I will only support updating the category of datasets and other questions that are children of the one being moved."
It is OK for me as long as you create questiontype functions so that the moving is done at the questiontype level.
This is why I propose a move function for other question data without other specs that it should not move files.
The plugin structure should be as open as possible.
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.
"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."
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 ...
(The $answer->feedback bug was MDL-14762, which is now fixed.)
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.
My patch seems to work for multianswer questions, except that I reproduced MDL-14356 when testing 3a, and there was a silly error in /question/contextmoveq.php.
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.
Fisrt thing: Don't do anything for datasets now...
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 understand your needs to have stable code and plugin questions are one of the place from where bugs can happen.
And you had a bad experience with Netscape and dream that Moodle learn from the Firefox experience.
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...
The final decision is related about what restrictions questiontype should have in order to not compromise Moodle stability.
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.
"calculated questions remain useable even if the datasetdef category IS modified. "
Patches to questionlib.php and calculated/questiontype.php to move the category datasets.
if there are an identical dataset ( all parameters and items) in the movetocategory , it is used .
If the are no identical, the datasef is move in the movetocategory.
If there is a non identical dataset in the movetocategory with the same name, the dataset is move with a new name created by adding __n (number 1 to...) , the older name is changed in the question text and answers so the question remain useable (and quiz results remain valid).
If the question is return back in its original category with a __n append , the original category if not changed is used.
No more useful datasets are deleted.
Tim,
I am working on a somehow different code flow for the move_category_datasets($question) function in calculated/questiontype.php .
So better to wait .
I reopen this as I find a solution for moving category datasets.
These will be more useful in 2.0 so I set fix to 2.0.
the main idea when moving a calculated question is if there is a name conflict between a category dataset already in the category where the qeustion is moved then we add a posfix __1..(number) to the dataset name e.g. {c} becoming {c__1} and we change the {c} occurences in the question text and answers formulas so the question remain valid.
If we bring back the question in the original category where other question are using {c} then we revert to the {c} dataset as long as there was no change in the data items value.
This have been quite tested and can be put in HEAD.
Further work is necessary to implement this in the saving process as the actual code just test if the dataset definition parameter is identical before choosing to use an already existng category. If not identical the dataset is switch to private type.
/** This function move category datasets to the new question category
* If there is already a dataset category with the same name then a new
* name is created using a postfix __123 (a number) to differentiate with the
* dataset category already in the moving to category.
* In the process the original {param}name in the question text and answers text is changed, so
* the moved question remain valid.
* The code used arrays to store the generic wild name (i.e.{c}) and the added postfix index
* in the question and in the move to category.
* There are four main options
* 1.If there is no dataset name in the moveto category with the same generic name,
* then a copy of the dataset is created in the new category i.e. move_one_category_dataset()
* 2.If there is at least one dataset in the moveto category with the same generic name which
* has identical data then the question use this dataset.
* If the real name is different(i.e {c__2}) then the names are changed in the question text
* and anwers.
* 3.If there is no identical name dataset in the moveto category
* Then a copy of the dataset is created in the new category i.e. move_one_category_dataset()
* 4.There is a similar name dataset in the moveto category which has different data.
* In a do loop, the __1.. postfix is incremented until the resulting name in neither in the
* possible wild card in the question text and answers and neither in the moveto category.
* Then a copy of the dataset is created in the new category i.e. move_one_category_dataset()
* and the names are changed in the question text and anwers.
*
*/
function move_category_datasets($question){
/** This function move category datasets to the new question category
* If there is already a dataset category with the same name then a new
* name is created using a postfix __123 (a number) to differentiate with the
* dataset category already in the moving to category.
* In the process the original {param}name in the question text and answers text is changed, so
* the moved question remain valid.
* The code used arrays to store the generic wild name (i.e.{c}) and the added postfix index
* in the question and in the move to category.
* There are four main options
* 1.If there is no dataset name in the moveto category with the same generic name,
* then a copy of the dataset is created in the new category i.e. move_one_category_dataset()
* 2.If there is at least one dataset in the moveto category with the same generic name which
* has identical data then the question use this dataset.
* If the real name is different(i.e {c__2}) then the names are changed in the question text
* and anwers.
* 3.If there is no identical name dataset in the moveto category
* Then a copy of the dataset is created in the new category i.e. move_one_category_dataset()
* 4.There is a similar name dataset in the moveto category which has different data.
* In a do loop, the __1.. postfix is incremented until the resulting name in neither in the
* possible wild card in the question text and answers and neither in the moveto category.
* Then a copy of the dataset is created in the new category i.e. move_one_category_dataset()
* and the names are changed in the question text and anwers.
*
*/
function move_category_datasets($question){
Tim,
The proposed changes in moving datasets are mostly related to possible name conflict for category datasets if there are two identical name in the same category.
However if you move one question with a dataset i.e. {a} from category XX to category XXX the question will remain VALID in the quiz because the datasets are retrieve by the question_datasets table.
However a confusion could result in the way the dataset definitions are choosed in the datasetdefinitions_form.php using the array ("1-category_id-$name"used from function dataset_options)
So we don't apply these patchs and I will work on a better datasetdefinitions_form.php...
more later ( 3 AM in Montréal now)
Ti,
So I create a new issue ( improvement) MDL-25566 to modify the handling of category datasets so that the question can be moved without side effects and without a call to a new function..
Let you resolve this one .
The calculated questiontype needs the $questions array in order to check for all calculated qtype questions and see if the moving process include all the questions sharing the same dataset..
so the code should look like
foreach ($questions as $question){
// test if the question as not been moved by a preceding call .
if ($question->category != $tocategory->id) { $QTYPES[$question->qtype]->move_to_category($question,$tocategory->id,$questions); }
}
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 .