|
History
| Log In
|
|
|
|
[
Permlink
| « Hide
]
Petr Škoda - 07/Dec/07 04:18 PM
sending a fresh patch for review
I've applied the patch and I'm looking at it from the GUI perspective first.
Overall I think it's good, but it's a major change. I'm a bit worried about regressions ... feels like a 2.0 change to me right now ... The doublling up under Modules is a bit wierd. How about we just have Activities / Filters / Blocks folders and put a "Manage" item always first under all of them. I'll keep looking at it. I'm just going to report things here for a bit as I don't have much time.
admin/block.php?adminedit=off§ion=blocksettingsearch (turning off block editing) resulted in error: A required parameter (block) was missing Overall this is very much the direction we need to go in, I totally agree. I'm very tempted to jump it into 1.9, but being cautious.
Can you work on it a bit more, perhaps even in a branch? I do not know about any problems in current patch except:
1/ missing suport for plugin config table (used by auth plugns only) - I would propose to do it toether with settings changes in auth plugins 2/ missing language strings for localised titles of settings - the long names overlap the checkboxes, some strings could be reused 3/ quiz settings not converted - it uses a special feature to postprocess the settings, should be a new class IMHO sending fresh (not much tested yet) patch
* supports config_plugin * improved searching * first error selected * form values not reset when error found * auth page converted - auth plugins not converted yet (needs more work) * tree reordering (above comment) * minor bugfixes * other improvements and speedups fresh patch:
* config_plugin fully supported * auth modules partially converted * improved searching * bug fixing * error focus * performance improvements - partial tree loading - no settings for external pages and course page * added lang strings for blocvks and modules * more improvements * rewritten css in admin tree block - no more space;space; span br constructs there
* fixed bunch of incorrect links with hardcoded /admin/ * other minor changes Playing with this at the moment and I really like the improvements. The performance gain is staggering and it would be really great to see those improvements in 1.9 at least.
Frontpage as admin for example: before 1.2375 secs Included 124 files DB queries 107 after: 0.802977 secs Included 137 files DB queries 46 Guessing this is supposed to be dirroot ?
- require_once($CFG->dirroot.'/admin/mnet/adminlib.php'); + require_once("$CFG->wwwroot/$CFG->admin/mnet/adminlib.php"); Attaching the CLI patch for Petr to review for conflicts with this bug.
Absolutely a huge improvement and it seems pretty stable now - please put it in CVS! :-D +100
committed in cvs, please file new bug reports for regressions
This latest patch might cause http://tracker.moodle.org/browse/MDL-12713 for IE.
reopening, going to tweak the display of defaults, css, highlighting and searching in values and defaults...
reclosing, I hope it will work in IE6/7 as expected, please reopen if any problem found
reopening again - file location setting & length of text fields
Note that the problem posted in http://moodle.org/mod/forum/discuss.php?d=88002 where at least one version of the "attendance" block does not work in 1.9 is because this fix looks for a file "settings.php" in the block for configuration purposes, but settings.php is used for a different purpose in this third-party block.
We fixed this locally by changing our code for our customized block and renaming settings.php. However, in general, any block that happens to have a file named settings.php will break the admin tree. We did not report the problem at the time as it appears that the newer version of the attendance block does not have this file. But you may see other reports of this problem because of the use of settings.php for use by the admin block since this patch was made. In our case, it actually broke access to much of the admin block for administrators, so it was more difficult to track down than one part of the attendance block not being compatible. I searched the contrib before implementing this - there was no settings.php file. I am doing this quite often when I am tweaking the API, I would strongly encourage everybody to submit extensions to contrib so that we developers know what ppl are doing and have on their moodles. The unofficial policy always was: "If it is not in contrib it does not exist" ;-) I guess we should advertise this more, could you please have a look at our coding guide and fix it if necessary?
I was already planning some new diagnostic code as part of upgrade that show all non-core modules, blocks, filters and other extensions including missing plugins. Thanks for the info. Hey Petr,
I have a patch (that I needed internally) to add support for local/lib.php to add items to the admin menu, it would be aces for you to approve it & I'll commit it: http://paste.dollyfish.net.nz/8ff286 maybe plain include instead of function call to make it more like the rest, the code uses a lot of globals:
+ if (file_exists($CFG->dirroot.'/local/localsettings.php')) { + include($CFG->dirroot.'/local/localsettings.php'); + } |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Resolved
Major
admin tree improvements and bugfixing - meta
Closed
