Moodle

Add Max category depth as a configuratble setting to the site settings

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9.3
  • Fix Version/s: None
  • Component/s: Course
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE

Description

We developed this patch for a client to allow them to set max category depth in the site settings rather than being forced to set it in config.php, which we do not allow access to.

Attachments

  1. Text File
    category_depth.patch
    30/Dec/08 9:12 AM
    3 kB
    Jason Hardin
  2. Text File
    MDL-17747.patch
    07/Jan/09 2:24 PM
    4 kB
    Dongsheng Cai
  3. Text File
    MDL-17747-2.patch
    08/Jan/09 4:55 PM
    4 kB
    Dongsheng Cai

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Michael Blake added a comment -

Dongsheng, please review this patch.

Show
Michael Blake added a comment - Dongsheng, please review this patch.
Hide
Permalink
Dongsheng Cai added a comment -

Verified, and patched, thanks Jason.

Show
Dongsheng Cai added a comment - Verified, and patched, thanks Jason.
Hide
Permalink
Tim Hunt added a comment -

I have no idea what this setting is, and the help text does not explain it to me.

Having looked at the code, I can now see, and 50 seems like a stupidly large default. Surely the default should be 0, meaning no limit. (Since this is now in the admin tree, the existing isset logic in course/lib.php is no longer any good.)

Also, if I was doing this, I would probably have fixed the name to be maxcategorydepth, so it followed the coding guidelines, even though that would have been a bigger change.

Rant over.

Jason, do you want to come up with an improved patch?

Show
Tim Hunt added a comment - I have no idea what this setting is, and the help text does not explain it to me. Having looked at the code, I can now see, and 50 seems like a stupidly large default. Surely the default should be 0, meaning no limit. (Since this is now in the admin tree, the existing isset logic in course/lib.php is no longer any good.) Also, if I was doing this, I would probably have fixed the name to be maxcategorydepth, so it followed the coding guidelines, even though that would have been a bigger change. Rant over. Jason, do you want to come up with an improved patch?
Hide
Permalink
Dongsheng Cai added a comment -

A new patch fixed the bugs Tim found out.

Show
Dongsheng Cai added a comment - A new patch fixed the bugs Tim found out.
Hide
Permalink
Tim Hunt added a comment -

isset($CFG->maxcategorydepth) && ($CFG->maxcategorydepth != 0)

would be better as

!empty($CFG->maxcategorydepth)

And sitemaxcategorydepthhelp - Oh, just noticed, all the other strings like this are called something like configsitemaxcategorydepth, anyway - I think

This specifies the maximum depth of nested categories shown when courses are listed on teh site front page.

Also, in the admin setting, the '0' option needs to be displayed as 'Unlimited' instead of 0.

Show
Tim Hunt added a comment - isset($CFG->maxcategorydepth) && ($CFG->maxcategorydepth != 0) would be better as !empty($CFG->maxcategorydepth) And sitemaxcategorydepthhelp - Oh, just noticed, all the other strings like this are called something like configsitemaxcategorydepth, anyway - I think This specifies the maximum depth of nested categories shown when courses are listed on teh site front page. Also, in the admin setting, the '0' option needs to be displayed as 'Unlimited' instead of 0.
Hide
Permalink
Dongsheng Cai added a comment -

Thanks Tim.
I attached a new patch here.

Show
Dongsheng Cai added a comment - Thanks Tim. I attached a new patch here.
Hide
Permalink
Dongsheng Cai added a comment -

committed, please review, thanks

Show
Dongsheng Cai added a comment - committed, please review, thanks
Hide
Permalink
Tim Hunt added a comment -

Tested and reviewed the code. Good work. Closing.

Show
Tim Hunt added a comment - Tested and reviewed the code. Good work. Closing.
Hide
Permalink
Jeffrey Silverman added a comment -

This is just my $0.02 on this: once a naming convention is suggested, it is probably a good idea to stick to it, as the change from the first patch to the second actually creates a regression for anyone who has implemented the first patch.

Note the underscores and lack thereof:

  • if (isset($CFG->max_category_depth) && ($depth >= $CFG->max_category_depth)) {
    + // maxcategorydepth == 0 meant no limit
    + if (!empty($CFG->maxcategorydepth) && $depth >= $CFG->maxcategorydepth) {

Thanks!

Show
Jeffrey Silverman added a comment - This is just my $0.02 on this: once a naming convention is suggested, it is probably a good idea to stick to it, as the change from the first patch to the second actually creates a regression for anyone who has implemented the first patch. Note the underscores and lack thereof:
  • if (isset($CFG->max_category_depth) && ($depth >= $CFG->max_category_depth)) { + // maxcategorydepth == 0 meant no limit + if (!empty($CFG->maxcategorydepth) && $depth >= $CFG->maxcategorydepth) {
Thanks!

People

Vote (6)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.