Moodle in English: modfile.php request for review

modfile.php request for review

by Petr Skoda - Friday, August 4, 2006, 3:01 PM

Hi!

I would like to ask other developers to review attached version of modfile.php and sample conversions of Glossary and Quiz.

Objectives:

Separate module files from course files (new modfiles_location() function)
Move access control and security stuff from file.php into modules (new xxx_modfile_callback() in module's lib.php)
Improve security (sesskey can be used also for student uploaded files - linking prevention)
Virtual files (module can override file location, generate file content on the fly or redirect)

List of changes:

Added modfile.php
Glossary attachments handled by modfile.php
Glossary export uses modfile.php too
Glossary backup/restore uses modfile_location() to find the files; backup file structure is not changed
Quiz - images in shared questions are now visible
Some other minor tweaks and needed changes

The patch can only be applied to latest HEAD revision. I would like to commit is ASAP if approved, so that I can continue with the rest of modules.

Thanks,
skodak

modfile5.patch

Re: modfile.php request for review

by Tim Hunt - Monday, August 7, 2006, 11:36 PM

Review comments (you asked for it

Will your approach solve the published question image problem?

I'm sorry, I've just realised that this approach is not going to work for questions because, as of Moodle 1.6, questions do not belong to any particular module. They can be used by any module that wants to use them. It is just historical reasons that at the moment only the quiz module uses them.

The logic is: a question belongs to a course, and an image belongs to a question. Any module in the same course can use that question, and hence use the image, but the module does not own the questions it uses. For published questions, any module in any course can use the question, and hence use the image.

So trying to solve the question image sharing problem using a function mod/xxx/lip.php/xxx_modfile_callback() for any value of xxx is going to require some nasty hacks, as you found.

The only way to get a non-hacky solution, is to expand the meaning of the 'mod' in modfile so it applies to other things than modules. That would be like lang files, where as well as a quiz.php lang file for the quiz module, you have a question.php lang file for the question bank. For our particular problem, you would need modfile to treat 'question' as the name of a module.

Unfortunately, the previous paragraph is not a very practical suggestion, because modfile.php is necessarily very module specific, and needs to use things like the course_modules table to check who can access what.

I'm sorry that I did not realise that this approach was not going to work before you wrote all the code to change all the question types.

Comments on the Quiz code

In mod/quiz/lib.php

Please add a comment above the defintion of function quiz_modfile_callback.

Given the previous section, I have not looked at the rest of these changes in detail yet.

Comments on the file library core code

In lib/filelib.php:

it should use 403 instead of 404 for forbidden errors.
In send_file(), I would take the opportunity to remove the ? : operators to make the code more readable.
Why have you removed the TODO, but not the @ signs before each header call? I can't think of any good reasons why these calls might give errors, and if they do, I think we should find out about them and fix the problem, rather than ignore the error message. Do you know why the @s are necessary. If you think it is too risky to remove them now, then I think we should remove them on HEAD immediately after we branch for 1.7.

In file.php

"// security: block serving of files handled by modfile.php". In the if statement, shouldn't you list quiz as well as glossary?
"// security: force download of all attachments submitted by students". I don't like the way this is done as a big 'if' statment. I would change this code (and similarly for the other security policy if statements that take a list of modules) to

$force_download_for = array('forum', 'assignment', 'data', ...);
if ((count($args) >= 3)
        and (strtolower($args[1]) == 'moddata')
        and in_array(strtolower($args[2]), $force_download_for)) {

(I would also move the definition of all the policy arrays to the top of the file, so they are all there together, with comments explaining them.)

In modfile.php

Should you add a comment to the switch at the end, explaining why it does not need break statements?

In lib/weblib.php

Why is get_file_argument() here, not in moodlelib.php (with require_param etc.) for filelib.php? (I know, this is not changed by your patch, but I think we should move it as a separate checkin.)

In backup/restorelib.php

I think (but I am not sure) it is bad that you have had to make the first argument of restore_create_sections pass by reference, just to get one tiny bit of data back that is used in restore_get_new_cm, which is currently only used in restoring the glossary. $restore is not modified by any of the other restore_xxx functions, so I think you are opening Pandora's box here.

I don't understand why glossary/restorelib.php doesn't just have all the necessary information to hand so it can easily make up the fake $cm object and call modfiles_location() itself. That seems like a much more modular solution to me.

On the other hand, looking at how much code you were able to eliminate in glossary/restorelib.php, perhaps I don't understand this well enough to comment. I just feel that there must be a better solution than the one you adopted.

Comments on the Glossary code

I have not reviewed this.

Re: modfile.php request for review

by Tim Hunt - Tuesday, August 8, 2006, 12:18 AM

How can we solve the images in published questions problem?

At the moment we are in a mess because the two separate features 'allow questions to be shared by other courses' and 'allow questions to display an image' were implemented separately, and both seem to have been implemented with insufficient thought for the consequences.

Publish questions

I think the one that is fundamentally wrong is the publish questions feature.

It is a great feature. There should be a way to share questions between courses, but hacking it in as something that is specific to questions is wrong, which is why it does not play nicely when combined with other features like course files.

The reason it is fundamentally wrong is that Moodle already has a mechanism for sharing stuff between courses. Namely the metacourses system. Stuff that belongs to SITE (course 1) can be accessed everywhere, and stuff that belongs to a metacourse can be accessed by any child courses.

Why don't we solve all our problems by removing the publish question feature, and instead making questions and categories from metacourses and the site available in child courses? (They are not currently accessible, which I think is a bug.)

Then, to be nice, we should add some user-interface to make it possible to move questions/question categories from a course to a metacourse, or the site, and vice-versa. (The existing move question to another category UI may actually be good enough. Also, of course, there would be permission checks here.)

Add image to question

This feature also sucks because the question text is edited in the rich text editor like every other bit of Moodle content, and the rich-text editor lets you add an image (or as many images as you like) to the question text.

Of course, the rich text editor is not great becuase it is not well integrated with the course files area. There is no easy way to upload in new image while editing some rich text. (This is a bug with the existing question image feature too, where you can select one previously uploaded image from a dropdown). And there is no easy way to add an already uploaded image - you have to manually construct the URL.

But I think we should be trying to fix the general problem of adding images to rich text course content, not worrying about some question-specific hack.

Comments? Or have I gone completely mad?

Re: modfile.php request for review

by Tim Hunt - Tuesday, August 8, 2006, 1:18 AM

Hmm. Seems that I may not understand how metacourses work. Drat! I thought I was on to something.

Re: modfile.php request for review

by Ray Lawrence - Tuesday, August 8, 2006, 1:29 AM

Or have I gone completely mad? Not yet, I suspect, but this could hasten the process.

On the publish questions issue:

Normally only Admins have editing access on the site.
If I have understood your proposal correctly there would be a requirement to create an additional course, a metacourse, for each course (or set of what would then be child courses) where if was felt desirable to publish a category. This could certainly make a mess of the site with a lot of otherwise empty metacourses.

I'm not a developer so the following may by plain silly, but is there another route from which this could be approached?

There is import /export functionality for questions which is logical and easy to understand. Is there any mileage in, rather than publishing questions, simply exporting them? At present the exported file is course specific (unless manually imported in another course). Might it be possible to utilise something like the "import" functionality to, erm, import exported questions from one course into another or from a single site "repository" of exported questions. There are issues with updating of shared questions with this approach which are not present in the current "publish" model. In addition there would need to be a revisd set of rules concerning the courses one could access to import from as it would need to be wider than for resources or modules. Feel free to refer me to the initial half dozen words in this post if appropriate...

Re: modfile.php request for review

by Tim Hunt - Tuesday, August 8, 2006, 5:35 AM

Using import/export is not really good enough. Sure, it lets you achieve the derired result, but only with a lot of hassle. Actually sharing the questions is better. For instace, if there is a mistake in a question that you have 'shared' several times using import and export, then fixing the problem is a pain. If you have a proper sharing mechanism, you only have to fix the problem once.

So I think that some sort of question sharing mechanism is definitely needed. We just need to find a model for doing it that fits with the rest of Moodle, so we can actally make it work in a way that is secure, and which plays nicely with all the other Moodle feature like backup/restore.

The thing about ony admins editing site becomes less of a problem in 1.7, when we have proper roles and permissions.

The bit about having to create lots of extra metacourses just to hold questions. Well, that is a disadvantage, but in exchange you get more control about which other courses you share your questions with. For example, perhaps the business school staff want to share questions between the business school courses, but they don't want any other faculty using their questions. Currenly, this sort of flexibility is not supported. it would be a good feature to offer, if it was feasible to implement.

Re: modfile.php request for review

by Ray Lawrence - Tuesday, August 8, 2006, 4:20 PM

Thanks Tim,

It was simply a passing thought. From your later responses in this discussion, it appears that this make need a rethink from the bottom up. Best wishes in your endeavours in this area.

Re: modfile.php request for review

by Petr Skoda - Tuesday, August 8, 2006, 1:41 AM

Or have I gone completely mad?

You are my hero - I do not know any other developer that would be willing to work on this part of Moodle

Re: modfile.php request for review

by Petr Skoda - Tuesday, August 8, 2006, 1:30 AM

Will your approach solve the published question image problem?
I'm sorry, I've just realised that this approach is not going to work for questions because, as of Moodle 1.6, questions do not belong to any particular module. They can be used by any module that wants to use them. It is just historical reasons that at the moment only the quiz module uses them. The logic is: a question belongs to a course, and an image belongs to a question. Any module in the same course can use that question, and hence use the image, but the module does not own the questions it uses. For published questions, any module in any course can use the question, and hence use the image.

Separation of questions from modules was IMHO really bad idea in the same way as was storing module files directly in course files instead of in moddata (you can not find out which files belong to resources). The reason is we can not "package" and export or share module instances among courses.

It would be IMO much easier to create a hidden course with quizes that share questions with normal quizes, lessons, etc. The sharing of GUI code in current quiz/questions implementation seems quite hacky - code in question/ directory must not reference quiz or lesson.

So trying to solve the question image sharing problem using a function mod/xxx/lip.php/xxx_modfile_callback() for any value of xxx is going to require some nasty hacks, as you found. The only way to get a non-hacky solution, is to expand the meaning of the 'mod' in modfile so it applies to other things than modules. That would be like lang files, where as well as a quiz.php lang file for the quiz module, you have a question.php lang file for the question bank. For our particular problem, you would need modfile to treat 'question' as the name of a module.Unfortunately, the previous paragraph is not a very practical suggestion, because modfile.php is necessarily very module specific, and needs to use things like the course_modules table to check who can access what.

modfile.php can be used for displaying of quizes because it can fetch the image from anywhere. It can not be used for question editing outside of quiz or lesson. We should not use file.php too, because it was meant to be serving public files == nothing secret. Coursefiles area should be IMO separated from module file area, we should not be storing question images there or we end up with new hacks in file.php.

We might as well create question image area in dataroot/questionfiles/questionid/ and questionfile.php and use it for teacher editing and modfile.php for quiz/lesson viewing. Then rewrite image plugin in htmlarea or tinymce to offer only files from it. We could relink the absolute links from quizfile.php to modfile.php on the fly when viewing quiz.

It is all a bit complex, but because questions do not fit current Moodle design there will be no easy solution...

Comments on the file library core code
In lib/filelib.php:it should use 403 instead of 404 for forbidden errors.
In send_file(), I would take the opportunity to remove the ? : operators to make the code more readable.

It is IMO correct according to the spec at http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

I like the ? : operator

Why have you removed the TODO, but not the @ signs before each header call? I can't think of any good reasons why these calls might give errors, and if they do, I think we should find out about them and fix the problem, rather than ignore the error message. Do you know why the @s are necessary. If you think it is too risky to remove them now, then I think we should remove them on HEAD immediately after we branch for 1.7.
In file.php

The problem with headers without @ is that they may throw weird errors on some server other than IIS and Apache. If we want to remove them, we should do it in print_header() instead of here. In anycase they are caused most often by trailing whitespace somewhere (config.php or library functions). There is only one place that checks it properly - health center.

"// security: block serving of files handled by modfile.php". In the if statement, shouldn't you list quiz as well as glossary?

Quiz is not using moddata area, no reason to block any files in file.php.

In backup/restorelib.php I think (but I am not sure) it is bad that you have had to make the first argument of restore_create_sections pass by reference, just to get one tiny bit of data back that is used in restore_get_new_cm, which is currently only used in restoring the glossary. $restore is not modified by any of the other restore_xxx functions, so I think you are opening Pandora's box here.

Hmm, I could either change the backup restore temporary table structure or pass by reference and store the temporary data in $restore. IMHO we might eliminate the temporary tables completely :-? We will have to ask our backup guru Eloy

I agree with the rest of comments

Re: modfile.php request for review

by Tim Hunt - Tuesday, August 8, 2006, 6:21 AM

Separation of questions from modules was IMHO really bad idea in the same way as was storing module files directly in course files instead of in moddata (you can not find out which files belong to resources). The reason is we can not "package" and export or share module instances among courses.

It would be IMO much easier to create a hidden course with quizes that share questions with normal quizes, lessons, etc. The sharing of GUI code in current quiz/questions implementation seems quite hacky - code in question/ directory must not reference quiz or lesson.

I disagree. Separating out questions so that the same set of question types and question instances could be used wherever it makes sense was a great move.

There are two ways in which using a computer is much better than using a text book. One is that it lets students work collaboratively, and Moodle is great there, with forums and wikis and so on.

The other way is in iteractivity. If you try to make the student think with a self assessment question in a book, all you can do for feedback is to print the answer at the back of the book, which means that that the student has to faff around turning pages, and the feedback they get does not adapt to what their answer was. Software can do a much better job of challenging the student, but at the moment, what Moodle has to offer in this regard is rather feeble. In the 21st centuary we we should be doing much better than multiple choice questions.

So we must invest time in making lots of much better question types that really let the teachers challenge their students. And we should be able to use those questions wherever it makes sense. Not just in formative and summative tests (quizzes), and in lessons, but everywhere!

And if we are using questions everywere, they can't belong to one particular module, and the code as to be shared. The concept of a separate question bank makes perfect sense. (Is it currenly implemented in the best way? That is a separeate question.)

Now sure, from a development point of view, having things that are shared in lots of different places makes life a lot more complicated. Well, welcome to Moodle. The codebase is full of things that are horribly complicated because Moodle is definitely not optimised to make life easy for the developers. It is optimised to provide the features that teachers and students want, and if that makes life more difficult for the developers, well tought. As a developer this often causes me to swear and curse, but ultimately, I can't argue with it. It is the right philosophy. It is our job to build the learning environment that is most effective for teaching, however hard that is.

So I think we have to find a way to let stuff be shared around the place, while still allowing module instances and so on to be packaged up and moved around using backup and restore. And, no, I don't have a suggestion for how to do it.

I'm afraid that I have spotted another potential problem with your modfile proposal. If we go with it, then we can't even share images. I think it is a prefectly reasonable requirement from a teacher that they can upload an image to their course once, and then use that image in a "Compose a web page" resource, and in a quiz question, and in a blog post. With your modfiles proposal, where every file is directly owned by a module instance, they will definitely have to upload the file three times, which sucks.

So perhaps we should forget worrying about question banks for now, and instead tackle the easier 'image bank'? ('image gallery' might be a better name). Which ties in to what you said about improving the UI for including images in the HTML editor in all sorts of content when using the rich text area.

Sorry for making this more complicated, but I think it is important we get this right, and get it right from the teacher's point of view. And I know that about 6 weeks before the Moodle 1.7 release may not be the optimal time to be having this debate.

Re: modfile.php request for review

by Petr Skoda - Tuesday, August 8, 2006, 2:14 PM

I'm afraid that I have spotted another potential problem with your modfile proposal. If we go with it, then we can't even share images. I think it is a prefectly reasonable requirement from a teacher that they can upload an image to their course once, and then use that image in a "Compose a web page" resource, and in a quiz question, and in a blog post. With your modfiles proposal, where every file is directly owned by a module instance, they will definitely have to upload the file three times, which sucks.

That was one of my goals to prevent direct linking to shared images (or other files) from course files! We should not encourage it because modules can not reliably track those linked files and on-the-fly relinking is not always possible. The only way I see is to improve the image plugin in visual editor and create a new file manager. They would not need to upload file 3x, we could add button to make a copy into resource from his/her teacher file area or we could even create a sort of symbolic link. All this would be possible with modfile.php because it is not doing anything itself - instead modules decide what to do. It gives developers freedom to do anything with module files, but they must add some tweaks to html editor or elsewhere to make it usable and user friendly.

Compose a web page is good for small site but when you want to for example copy section to another course you hit several problems:

you have to copy all course files or use dark magic to find needed files
absolute inter-activity links are broken because you would have to track the origin of each activity and do relinking in all of them after each move (I did that already for book import and it was not nice)

Now if the modules owned the files and we used on-the-fly activity links from filters only all would work just fine.

Now back to questions. If there were no global course questions and instead several types of modules would be able to 'provide' questions using some predefined interface everything would be much more flexible and easier to implement because we could keep the modularity. We would be sharing the questions on higher than database level. Source modules would simply register on site level and share the questions with consumer modules, consumers could either copy or link to questions.

We could for example create question module that would be sucking questions from another server and sharing them with other local modules (question repository connector - implemented as student invisible module placed in any course). We could use existing modules as sources (Hotpot? or Scorm?), just add an interface and here we go.

I am convinced that course level questions are not good solution. We can change them now (because they do not work anyway) or suffer with them for several years.

Re: modfile.php request for review

by Tim Hunt - Tuesday, August 8, 2006, 7:19 PM

I'm going to make two replies. First about questions, then later about files.

I am half convinced by what you say.

I am a bit suprised that you have previously said that things being shared makes like difficult, and now you are suggesting that we let all modules share questions with other modules!!! I think that is a recipe for ending up with a big mess.

But I do agree with you that having the questions bank code sitting in a top level /question/ folder is an anomaly. It means that we have two whole new concepts in the code that do not relate to anything else, namely question categories and questions. And this anomly is not really necessary.

I think it works if we make a new module type 'questionbank'. Then what are currenly question categories become instances of the questionbank module.

Then all question instances belong to a particular questionbank instance, which belongs to a particular course, and the questiontypes are sub-plugins of the questionbank module, a bit like resource types are sub-plugins of the resource module.

This way, we don't have to have all modules sharing questions with other modules. We just have one module providing questions to all the rest. I am still convinced that it is right to bave a central place to collect all the question type code, with a single questiontype API, rather than letting lots of different modules each have their own questiontype API.

This works for publishing questions, becuase under new roles an permission, an instance of a module is one of the contexts. So for a particular question category, you can be quite fexible about which users are allowed to edit or use questions in that category.

The only thing we loose in terms of current functionality is the ability to have hierarchical categories. Instead you just have a flat list of questionbanks in a course. I think that is a very small loss, so we just live with it.

The difficulty with this is, as you say, knowing which questions are used where. So, for example, when you backup an instance of the quiz module, knowing which question instances (or perhaps entire questionbank instances) to include in the backup file. But I think we have to get round this somehow. For example, where a module uses questions, insist that it implements a function to return a list of questions that it uses, and then make the backup system intelligent enough to call that function. (Actually, we could do it more generally, and let each module instance declare a list of other module instances it uses, for backup/export purposes.)

There is also the practical problem that this is yet another big refactoring of the question code; so we have to move all the code and break CVS history again; and rename all the tables again; and making question categories into module instances would break the backup file format in a big way.

But this feels like the right solution to me, so if we can really thrash it out and become convinced it is right, it is probably worth the pain. What do you thing?

Re: modfile.php request for review

by Tim Hunt - Wednesday, August 9, 2006, 1:41 AM

Now my response about serving files:

I think the reason we are having problems is that at the moment we are trying to do most of the processing when the user requests the file, and at that stage, the only information we have available to us is

The session of the person making the request
The path of the file they are asking for

The problem is, this is not really enough information. It is more information it seems at first, because there is a lot of information burried in the path names we use, but it is still not enough. For instance, as you have explained, it is very important to know where a file came from, in order to know how much to trust it. That information is almost completely gone by the time we come to serve the file, although sometimes we can guess it from the file path if we were lucky.

I don't see why we should live with this lack of information. Attached to Moodle we have a huge database designed for storing information. So let us use it to store the information about each file that we need in order to serve it securely and safely.

Of course, storing stuff in the database about each file means we have to deal with the possibility that the files on the disc and the information in the database may get out of synch. But actually, that is not the end of the world. There are two cases:

The database refers to a file that does not exist on disc.
There is a file on disc but there is no entry in the database.

We can ignore both these problems until someone requests the affected file (or backs up the affected course). Then we need to apply a work-around.

In case 1, when we notice the problem, we log that it happened because it indicates a probably bug in the code somewhere, then we delete the database entry, and act as if the file never existed (e.g. show a 404 error).

In case 2, again we log the problem so the bug that caused it can be investigated. Then we add a database entry about the file with the safest possible settings (forcedownload, etc.) (or possibly use the existing code that deduces reasonable download options from the path), then we use the file according to those settings.

If we take this approach, then we need a new file library to handle all the operations in the moodledata area and keep the datbase up to date. But using the fallback above we don't have to break compatibility with existing modules at first. By letting modules control what is stored in the database about each file, we can give them the control they need, and the modles can apply that control when the file is uploaded and all the necessary information is to hand. At the same time, the fact we can explicitly record where each file came from (the user id, or may be their role), we can be certain to apply the right security setting.

This may be a slightly over-engineered solution, but it is looking like we need this much control.

P.S. I had a word with Thanh about repository APIs/JSR-170, since the whole point about repositories is that you can store files with bits of metadata, and what I am proposing is storing the bits of metadata that we need about each file. While the JSR-170 stuff looks quite cool, it will not be usable to replace the moodledata folder for any time soon, so it is not a solution to this problem now.

P.P.S. I must add that I am really enjoying this debate. These are difficult questions, and it is really great that you are trying to solve them. I hope I am being helpful. If at any stage I stop being helpful, please feel free to ignore me.

Re: modfile.php request for review

by Petr Skoda - Saturday, August 12, 2006, 5:08 PM

Storing of advanced file attributes in database is good idea discussed here several times before. The only problem is that it should not be made mandatory in all modules and moodle core. Most modules do not need that, they can use hard coded access control shared for all its files. Moreover roles in 1.7 will add some more information that would be available when processing modfile requests.

Modfile.php does allow both simple hard coded and advanced approaches at the same time - it is only a file serving interface for modules. Sure - modfile is not suitable for all current modules, but IMHO we should blame the modules for being badly designed instead:

resources:
We should call them 'links' because they are just linking to content from course files or elsewhere. You can not use them off-line, convert them to another format, search them. We can not use modfile in them now, but we can create new type that stores the data properly in moddata directory and do proper access control.

questions and quiz:
If we move questions back into some QuestionBank module lots of current problems would be solved easily and the modfile.php could be used there too.

The design goals of question sharing should IMHO be:

allow sharing of questions between modules
allow simple and advanced implementations - for small and laaarge sites
optional versioning
optional off-line export with solution
optional sharing of questions among courses
optional support for question repositories

1. sharing of questions is definitely needed - the problem seems to be that it can not be now achieved easily without extensive changes in Moodle core, problems: where to store images and how to access them; backup/restore; upgrade problems - modules are tied to specific moodle core version

2. There is not interface for accessing questions, all except question types is hard coded. There is no minimal implementation and you can not replace the questions with more advanced implementation.

3. 4. 5. The features are either there or not - it is hard to change or replace anything according to your institution requirements.

Here is my imagination:

Questions are stored in modules called question banks. They need not store the data itself they could fetch the questions from elsewhere.

Question banks would reqister in Moodle core to announce its presence in the course. This mechanism should be more generic to allow cooperation of other modules too. The registration method would need ($courseid, $cmid, $type) where $type is the type of service offered. Quiz would simply search for any module offering service type 'questions' and throw error when no module found in the course (and probably ask for creation of new QuestionBank in course).

To make implementation simple quiz and other consuming modules would communicate with question banks only within the same course. Question banks could be stored in special hidden course section or be only hidden from students.

There could be several different question banks in each course, each question would be identified by $cmid of question bank and $questionid that would be unique only in the scope of its question bank. Category would be used only for easier question selection, one question could be in several different categories.

Each question bank would have to provide basic interface with some optional functions:

get description of one or several question - title, max score, type
get online representation of question (for quiz view) - part of html form
process answer ($_POST) and return serialized answer for storage
calculate score from serialized answer
get online representation of answered question from serialized answer (quiz review)
optional - edit question (not all question are editable)
optional - get html only offline representation with solution in javascript (for export as content package)
optional - versioning interface
... some more functions ...

When constructing quiz it's questions would be registered in question bank which could optionally implement versioning. Quizes need not know about question versions, question banks could decide because they would know when the question was added to the quiz.

Quiz backup/restore code would be quite simple, it would just require the backup of Question Banks and store only $cmid of question bank and $questionid. Each question bank could handle backup/restore differently - simple types would just dump all question and all images. The ones with sharing could do really complex work to manage it right.

It might sound convincing, but how would we do it in real life? Patching of current broken code is not possible - it sort of works, we can not start breaking it again, but we would need people to test it.

Theoretical roadmap:

create NewQuiz and QuestionBank totally separate from Moodle core and current code (without backup/restore for now) - only minimal set of features
work on features, backup/restore
when it has enough features, copy database data from current implementation
make it stable, do mandatory conversion, use question banks from other modules
deprecate old implementation, start adding new types of question banks

In any case it would be lots and lots of work for several months, but it could give the developers some freedom to innovate without looking back.

Sharing questions (and images) across courses

by Fig Newton - Sunday, August 27, 2006, 2:18 AM

I'm sorry to jump into the discussion, but I thought I should inject the perspective of a teacher.

I've been using moodle for three years and the ability to share questions (with images) BETWEEN courses has been crucial. I teach several levels of chemistry and physics and have many hundreds of questions, most with images. Any one single question could be used amoung several courses. For example, fundamental questions on mechanics are often suitable for both lower and advanced courses in physics, where more challenging questions would be used for homework in the advance course used only as extra credit in a lower level course.

Up to this point, I have used a hidden course of published questions. The changes in 1.6 are so detrimental to the sharing of questions that I will most likely have to downgrade back to 1.5.4 (with its associated bugs and less robust feature set). Eliminating the ability to share questions between courses several limits the utility of moodle to allow teachers to collaborate betweens courses.

A quick fix would be helpful (such as being able to select an image stored in the Site file areas, currently the drop down menu only displays images within the course where the question is authored).

I do applaud the goal of being able to use questions across modules, but not at the cost of losing the ability to share questions between courses. Perhaps the concept of the metacourse can be expanded to create a repository of shared resources. This ubercourse would not be acccessible by students. Administators could assign which courses can access the resources in the repository. Not to be overly ambitious, but sharing department surveys, teacher feedback forms, web links, glossaries and other resources across courses would also be very useful.

Fig Newton

Re: modfile.php request for review

by N Hansen - Tuesday, August 8, 2006, 11:42 PM

Re: modfile.php request for review

by Petr Skoda - Saturday, August 12, 2006, 1:53 AM

This problem unfortunately already exists in the database module. The image field only allows for uploading images, not selecting already existing images in the course files.

It would be wrong to envourage linking to course files in database module, because students must not browse the course files and it would also cause export/import troubles. Teachers should not use course files as their private storage because files are public and they are often not able to find the old/useless files and delete them. Coursefiles is one big chaos today we should let modules do the work for us

The basic rule for modules should be to keep its files in its moddata directory and let them create and delete all files. There should be no need to use the file manager for module files. Instead of adding absolute links into HTML we should rely more on filters to add all links on the fly.

Solution is simple - modify upload manager to allow copying of files from user's private file storage on server instead of uploading. The only problem is we do not have any such private file area in the standard Moodle distribution yet

Re: modfile.php request for review

by Ger Tielemans - Sunday, August 27, 2006, 7:46 PM

Why not give Moodle a real CMS, with only direct (flie) access for teachers and Moodle itself when a file is "pasted" in a course?

Then the teacher has the choice between pasting files stored in his course and global files uploaded and stored in the CMS?

Do I miss a point?

Re: modfile.php request for review

by Tim Hunt - Tuesday, August 29, 2006, 1:41 AM

Do I miss a point?

Yes, doing that would be a lot of work, so it has not happened yet. It is obviously a good idea, it;s just a lot easier said than done.

Re: modfile.php request for review

by Tim Hunt - Tuesday, August 29, 2006, 1:41 AM

Do I miss a point?

Yes, doing that would be a lot of work, so it has not happened yet. It is obviously a good idea, it;s just a lot easier said than done.

Testing and QA

modfile.php request for review

Will your approach solve the published question image problem?

Comments on the Quiz code

Comments on the file library core code

Comments on the Glossary code

Publish questions

Add image to question