Moodle

Navigation - defaulthomepage for user preference not functional for admin user

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0.5, 2.1.2, 2.2
  • Fix Version/s: 2.0.6, 2.1.3
  • Component/s: Navigation
  • Labels:
  • Testing Instructions:
    Hide

    1. Log in as an admin
    2. Set defaulthomepage to `User preference` (Settings > Site administration > Appearance > Navigation)
    3. In the navigation browse to `My home`
    4. In settings expand My profile settings and check that there is a `Make this my default home page` link
    5. Click that link
    6. Browse to any course
    7. Check that `My home` is now the first item in the navbar and that it links to /my/

    Show
    1. Log in as an admin 2. Set defaulthomepage to `User preference` (Settings > Site administration > Appearance > Navigation) 3. In the navigation browse to `My home` 4. In settings expand My profile settings and check that there is a `Make this my default home page` link 5. Click that link 6. Browse to any course 7. Check that `My home` is now the first item in the navbar and that it links to /my/
  • Difficulty:
    Easy
  • 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:
    wip-MDL-27278-master
  • Pull Master Diff URL:

Description

I'm not quite sure which component to choose but navigation seems like a good place to start. This issue is created in response to Ray Lawrence's question in the forums (http://moodle.org/mod/forum/discuss.php?d=172992) about how to set the user preference for the user home page. It does not seem possible for the site admin to be able to do this because of the current logic.

To re-create the issue:

1) Login as site admin and set defaulthomepage to 'User preference' (via Settings -> Appearance -> Navigation)
2) The intended functionality is that a link is added if the current page is not the user's preferred home page to allow it to be switched via the optional setdefaulthome param; however, the logic is by-passed because of the use of an else if in the case of the user with the site config option. As a result, under Settings -> My profile settings the admin does not see the expected 'Make this my default home page' link.

As a result of this bug, the admin does not have the ability to select their default home page user preference as other users can and it thus makes it seem like hidden functionality. I would propose simply removing the else if and making it its own separate if statement. I do not see any concerns as it just allows site admins the ability to have that code executed.

Peace - Anthony

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Anthony Borrow added a comment -

Possible patch at https://github.com/arborrow/moodle/compare/MOODLE_20_STABLE...MDL-27278

Show
Anthony Borrow added a comment - Possible patch at https://github.com/arborrow/moodle/compare/MOODLE_20_STABLE...MDL-27278
Hide
Permalink
Petr Škoda added a comment -

This might be related to the fact that originally admins could not go to "My moodle" page for performance reasons because it would process all site courses. Now we have enrolments, so technically it could be supported now, we would have to make 100% sure this would not be causing performance problems.

However nobody should be using admin accounts for normal work in Moodle, it should be used only for server configuration and administration...

Show
Petr Škoda added a comment - This might be related to the fact that originally admins could not go to "My moodle" page for performance reasons because it would process all site courses. Now we have enrolments, so technically it could be supported now, we would have to make 100% sure this would not be causing performance problems. However nobody should be using admin accounts for normal work in Moodle, it should be used only for server configuration and administration...
Hide
Permalink
Martin Dougiamas added a comment -

Regardless, my +1 to make the admin setting work the same as normal users.

Show
Martin Dougiamas added a comment - Regardless, my +1 to make the admin setting work the same as normal users.
Hide
Permalink
Ray Lawrence added a comment -

OK, thanks for moving this over Anthony.

If I'm understanding this correctly the setting referred to is for users who are non-admins. I tested with a non-admin account and couldn't find where to set my preference (if allowed).

Also, we seem to have lost the ability for admins to force (i.e. restrict to) non-admins to the "my Moodle" page. I'm not a fan of that approach but I've seen it used on many sites - probably an unecessary migration headache....

Site admin accounts being used for "normal work". Agreed, they shouldn't be. So.... perhaps controversially... shouldn't that acct only have site config permissions? Should I open a new issue or is that suggestion fatally flawed in a way I can't see?

Show
Ray Lawrence added a comment - OK, thanks for moving this over Anthony. If I'm understanding this correctly the setting referred to is for users who are non-admins. I tested with a non-admin account and couldn't find where to set my preference (if allowed). Also, we seem to have lost the ability for admins to force (i.e. restrict to) non-admins to the "my Moodle" page. I'm not a fan of that approach but I've seen it used on many sites - probably an unecessary migration headache.... Site admin accounts being used for "normal work". Agreed, they shouldn't be. So.... perhaps controversially... shouldn't that acct only have site config permissions? Should I open a new issue or is that suggestion fatally flawed in a way I can't see?
Hide
Permalink
Anthony Borrow added a comment -

Ray,

The setting should be, at least IMHO, for all users as it is a user preference. Even site admins have preferences and I expect them to work. I also think that it is possible to force users to the my page unless I am missing something. The option to set the defaulthomepage to My Moodle should still work but I have not specifically tested it. If it is not working, let me know as that may be a separate bug (I am deliberately trying to keep this issue simple and straightforward). Regardless of how the site admin account is or should be used - respecting settings and having consistent behavior to other users I think is important. I'm not sure that your suggestion is flawed, I just don't understand the rationale behind it.

Peace - Anthony

Show
Anthony Borrow added a comment - Ray, The setting should be, at least IMHO, for all users as it is a user preference. Even site admins have preferences and I expect them to work. I also think that it is possible to force users to the my page unless I am missing something. The option to set the defaulthomepage to My Moodle should still work but I have not specifically tested it. If it is not working, let me know as that may be a separate bug (I am deliberately trying to keep this issue simple and straightforward). Regardless of how the site admin account is or should be used - respecting settings and having consistent behavior to other users I think is important. I'm not sure that your suggestion is flawed, I just don't understand the rationale behind it. Peace - Anthony
Hide
Permalink
Sam Hemelryk added a comment -

Hi guys,

Indeed this sort of user preference should be aplicable to all users admin included.
I've created a patch for this now that tidies up the bit of code in question and is now up for peer-review.

Cheers
Sam

Show
Sam Hemelryk added a comment - Hi guys, Indeed this sort of user preference should be aplicable to all users admin included. I've created a patch for this now that tidies up the bit of code in question and is now up for peer-review. Cheers Sam
Hide
Permalink
Sam Hemelryk added a comment -

Putting this up for integration this week.

Show
Sam Hemelryk added a comment - Putting this up for integration this week.
Hide
Permalink
Andrew Davis added a comment - - edited

Had a quick look. The changes looks sensible enough. Mostly looks like a bit of a reshuffle to remove repeated has_capability() calls which probably should have been done as part of a separate issue. tisk tisk

Show
Andrew Davis added a comment - - edited Had a quick look. The changes looks sensible enough. Mostly looks like a bit of a reshuffle to remove repeated has_capability() calls which probably should have been done as part of a separate issue. tisk tisk
Hide
Permalink
Aparup Banerjee added a comment - - edited

integrated and ready for testing.

Thanks for the sanity check Andrew.

ps: main change is right before redirect($CFG->wwwroot .'/my/');

Show
Aparup Banerjee added a comment - - edited integrated and ready for testing. Thanks for the sanity check Andrew. ps: main change is right before redirect($CFG->wwwroot .'/my/');
Hide
Permalink
Rossiani Wijaya added a comment -

This is working great.

Test passed.

Show
Rossiani Wijaya added a comment - This is working great. Test passed.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Many thanks for the hard work developing and testing this. It has been spread to cvs and git upstream repositories.

Closing, ciao

Show
Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work developing and testing this. It has been spread to cvs and git upstream repositories. Closing, ciao
Hide
Permalink
Shane Elliott added a comment -

Line 61 of index.php is a killer as it changes an acceptable behaviour for some sites eg default home page is "my" but users can still navigate to site home (via navigation block) if they choose.

The changes now make the "Site Home" link (in navigation block) redundant as it always redirects to "My" (for those without moodle/site:config capability).

Is there any work-around for this scenario?

Show
Shane Elliott added a comment - Line 61 of index.php is a killer as it changes an acceptable behaviour for some sites eg default home page is "my" but users can still navigate to site home (via navigation block) if they choose. The changes now make the "Site Home" link (in navigation block) redundant as it always redirects to "My" (for those without moodle/site:config capability). Is there any work-around for this scenario?
Hide
Permalink
Sam Hemelryk added a comment -

Hi Shane,

I've just been looking into this, I don't think this change has had that effect.
To be sure I reset my master branch back to bdfb2b7 (the commit before my changes) and found that logged in as a student if default home page was /my/ I would always be redirected to /my/ if I arrived at / unless an optional param 'redirect=0' is provided at the same time. The navigation does this (the home page link on the navigation should have ?redirect=0).

The redirection issue did ring a bell though. I had a quick search and found MDL-23024 which is about adding back the mymoodleredirect setting so that we use that [consistently] rather than the ugly somewhat hacky redirect get param.
Adding this setting back, perhaps as forcehomepage would allow us to force the redirect only if that setting were true.

I'm thinking that the solution you are looking for is likely that, does that sound about right?

Cheers
Sam

Show
Sam Hemelryk added a comment - Hi Shane, I've just been looking into this, I don't think this change has had that effect. To be sure I reset my master branch back to bdfb2b7 (the commit before my changes) and found that logged in as a student if default home page was /my/ I would always be redirected to /my/ if I arrived at / unless an optional param 'redirect=0' is provided at the same time. The navigation does this (the home page link on the navigation should have ?redirect=0). The redirection issue did ring a bell though. I had a quick search and found MDL-23024 which is about adding back the mymoodleredirect setting so that we use that [consistently] rather than the ugly somewhat hacky redirect get param. Adding this setting back, perhaps as forcehomepage would allow us to force the redirect only if that setting were true. I'm thinking that the solution you are looking for is likely that, does that sound about right? Cheers Sam
Hide
Permalink
Shane Elliott added a comment -

Hi Sam,

The problem is this line:

} else if ($CFG->defaulthomepage == HOMEPAGE_MY && (optional_param('redirect', true, PARAM_BOOL) || !$hassiteconfig)) {

You are correct about the previous behaviour. The problem now is that "?redirect=0" no longer works for the average user as "!$hassiteconfig" evaluates to true forcing the redirect to /my

Cheers,
Shane.

Show
Shane Elliott added a comment - Hi Sam, The problem is this line: } else if ($CFG->defaulthomepage == HOMEPAGE_MY && (optional_param('redirect', true, PARAM_BOOL) || !$hassiteconfig)) { You are correct about the previous behaviour. The problem now is that "?redirect=0" no longer works for the average user as "!$hassiteconfig" evaluates to true forcing the redirect to /my Cheers, Shane.

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: