Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 2.0.5, 2.1.2, 2.2
-
Component/s: Questions
-
Labels:
-
Testing Instructions:
-
Affected Branches:MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
-
Fixed Branches:MOODLE_20_STABLE, MOODLE_21_STABLE
-
Pull from Repository:
-
Pull Master Branch:
-
Pull Master Diff URL:
-
Rank:18586
Description
qformat_default writefiles should be a public method not protected
Activity
Actually maybe we should keep this protected and make a higher level function public that takes all the files in a file area and exports them.
Yes. This (or something better) needs to be public so export methods in question types can use it. But, shall we sort out MDL-29059 first?
Yes. I copied the code from the method into one of my question types so I had access to the code in the question type plugin. I am waiting for us to resolve this so I can call the method on the format class instead and removed the copied code.
I am writing the code now for exporting the ddmarker questions as xml. We still have not resolved this. I think we just need to change the access from protected to public. The method name should have an underscore in it but I guess we just ignore this.
It seems that the second parameter of writefiles is not used anywhere and doesn't do anything. Are there format plug ins that might be calling this code on the parent class? If not maybe we could rename it and replace the second param with an indent level??
Sorry, Jamie, I forgot all about this. I think this change does the necessary. Could you give it a quick review.
I decided this really had to be fixed in all branches since 2.0, but then found that the changes would not cherry-pick, so had to do it three times.
Even though this looks like an API change, it is not really, because the old method was not public, even though it should have been.
You are right that there is no need for the second argument to write-files. However I left the encoding="base64" in the XML. Being explicit about the encoding there seems like a good idea (in case we find a better encoding later) however, there is no point in allowing the qtypes to specify the encoding when they call write_files.
I tested this code in 2.0, 2.1 and master branches. Looks good.
Integrated, thanks!
Works Great 
Thanks for fixing this, Tim.
And this has landed upstream, just on time for the upcoming new releases week. Thanks for it!
Ciao
Before making it a public method we should probably change it's name to write_files