Issue Details (XML | Word | Printable)

Key: MDL-19731
Type: Sub-task Sub-task
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: David Mudrak
Reporter: David Mudrak
Votes: 0
Watchers: 0
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle
MDL-19077

have multiple renderers belonging to one plugin

Created: 07/Jul/09 03:35 PM   Updated: 08/Jul/09 09:47 PM
Return to search
Component/s: Lib
Affects Version/s: 2.0
Fix Version/s: 2.0

File Attachments: 1. Text File 0001-MDL-19731-get_renderer-accepts-optional-subtype.patch (14 kB)


Participants: David Mudrak and Tim Hunt
Security Level: None
QA Assignee: Tim Hunt
Resolved date: 08/Jul/09
Affected Branches: MOODLE_20_STABLE
Fixed Branches: MOODLE_20_STABLE


 Description  « Hide
It appears to be useful to allow an extra word in the plugin renderer class name, like moodle_mod_workshop_subthing_renderer - so you can have multiple renderers belonging to one plugin.

See http://moodle.org/mod/cvsadmin/view.php?conversationid=2705 for the background

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Tim Hunt made changes - 07/Jul/09 03:37 PM
Field Original Value New Value
Issue Type Improvement [ 4 ] Sub-task [ 5 ]
Parent MDL-19077 [ 31801 ]
David Mudrak added a comment - 07/Jul/09 06:46 PM
Fixed URL

David Mudrak made changes - 07/Jul/09 06:46 PM
Description It appears to be useful to allow an extra word in the plugin renderer class name, like moodle_mod_workshop_subthing_renderer - so you can have multiple renderers belonging to one plugin.

See http://moodle.org/mod/cvsadmin/view.php?conversationid=2706 for the background
It appears to be useful to allow an extra word in the plugin renderer class name, like moodle_mod_workshop_subthing_renderer - so you can have multiple renderers belonging to one plugin.

See http://moodle.org/mod/cvsadmin/view.php?conversationid=2705 for the background
David Mudrak added a comment - 07/Jul/09 08:12 PM
See the attached 0001 patch for the proposed implementation

If you are using git, you can run
git am <<the patch filename>>
or - if it fails - try
git am -3 <<the patch filename>>

Non-gitters will just remove some lines from the top of the patch file to get pure diff


David Mudrak made changes - 07/Jul/09 08:12 PM
Tim Hunt added a comment - 07/Jul/09 10:10 PM
That looks good.

I assume you have run the unit tests, and they all pass. Please comment. Thanks.


David Mudrak added a comment - 08/Jul/09 05:55 AM
Yes, the tests passed. I modified some of the current tests a bit - I know I should not do that but I felt I am doing right thing, hopefully. Also, I added some more to test the variations of calls with the new parameter. The new parameter works for my needs.
To be reviewed more carefully: how I integrated this into the templateable renderers as I am not very confident in this area. Thanks in advance.

Tim Hunt added a comment - 08/Jul/09 10:32 AM
Doh! that was an interesting typo. I mean to say Please commit.

However, you are right template_renderer_factory needs a bit more work. In get_renderer, the $searchpaths array needs to

$path = $rootpath . '/' . $module;
if (!is_null($subtype)) { $path .= '/' . $subtype; }
if (is_dir($path)) { $searchpaths[] = $path; }

Is probably a good way to do it.

Anyway, template renderer is only an experimental proof-of-concept. No need to be too concerned with it now.


David Mudrak added a comment - 08/Jul/09 03:40 PM
Well, this was my original intention to keep all templates in one directory. But you are right, subdirectories templates should respect the subtypes.

p.s. I like the typo. Maybe we can start using "Fix a lot" instead "Thanks a lot" and so on


David Mudrak committed 3 files to 'Moodle CVS' - 08/Jul/09 05:37 PM
MDL-19731 get_renderer() accepts optional subtype

The initial patch has been reviewed at the issue page. This commit also
fixes template_renderer_factory::get_renderer() and sets up folders for
subtypes there.
MODIFY lib/simpletest/testoutputlib.php   Rev. 1.8    (+72 -11 lines)
MODIFY lib/deprecatedlib.php   Rev. 1.123    (+2 -2 lines)
MODIFY lib/outputlib.php   Rev. 1.27    (+28 -14 lines)
David Mudrak added a comment - 08/Jul/09 05:39 PM
Committed. I have fixed the template_renderer_factory::get_renderer() so it sets up folders for subtypes. This involved a little extension of the unit test.

David Mudrak made changes - 08/Jul/09 05:39 PM
Fix Version/s 2.0 [ 10122 ]
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Tim Hunt added a comment - 08/Jul/09 09:47 PM
Great. Thanks.

Tim Hunt made changes - 08/Jul/09 09:47 PM
Status Resolved [ 5 ] Closed [ 6 ]