Users who are in "No Institution" can't use skins
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| Mahara |
High
|
Son Nguyen | ||
| 1.8 |
High
|
Unassigned | ||
| 1.9 |
High
|
Son Nguyen |
Bug Description
To replicate:
1. Set $cfg->skins = true;
2. Go to the configuration screen for the default institution ("No Institution")
3. Note that unlike other institutions, there is no skins checkbox displayed
4. Click "Submit" on the configuration screen
5. Log in as a user in the default institution
6. Navigate to their Portfolio section
Expected result: They should have a "Portfolio -> Skins" option.
Actual result: They do not.
The problem is that, although we set institution.skins to 1 by default during installation and upgrade, the absence of the skins checkbox on the default institution's config screen means that it gets set to 0 the first time you save anything on that screen.
Looking at the code, I realize that the reason the skins checkbox is hidden form the default institution was not on purpose, but because the checkbox made the most logical sense placed adjacent to the other theming-related settings, and those all happen to be settings that can't be overridden for the default institution.
Aaron Wells (u-aaronw) wrote : | #1 |
Kristina Hoeppner (kris-hoeppner) wrote : | #2 |
We don't hide other institution settings from "No institution". "No institution" is the site and thus, the settings in "Config site" apply. However, for skins the thing is that there is no checkbox in "Config site" and that's why it's not working I guess.
Aaron Wells (u-aaronw) wrote : | #3 |
Well, by "hiding" I mean that there are quite a few settings that show up on the Institution config page for other institutions, but which are not shown for "No Institution".
It sounds like you are saying, that a setting should be hidden from this list, only if the purpose of that setting is to allow the Institution to override one of the site default settings.
In that case we should probably expose the skins checkbox for the "No Institution" institution. Or alternately we could just set it to default to being the same as $cfg->skins. But potentially in a multitenanted site an admin might want the No Institution users to not have skins, so it's probably useful to expose the checkbox.
Son Nguyen (ngson2000) wrote : | #4 |
Changed in mahara: | |
assignee: | nobody → Son Nguyen (ngson2000) |
status: | Confirmed → In Progress |
Reviewed: https:/
Committed: http://
Submitter: Robert Lyon (<email address hidden>)
Branch: master
commit 35a6a62ad98d9ff
Author: Son Nguyen <email address hidden>
Date: Tue Nov 12 10:11:37 2013 +1300
Enable 'No institution' users to use skins (Bug 1249123)
if it is activated in config.php
The skins setting is added to Site config page as well
Change-Id: I4464867db90dc9
Signed-off-by: Son Nguyen <email address hidden>
Changed in mahara: | |
status: | In Progress → Fix Committed |
Aaron Wells (u-aaronw) wrote : | #6 |
I appreciate the quick action on this, but unfortunately Patch 2696 doesn't resolve this in the way we need to. That patch does this:
1. Adds skins to the admin web UI. This is no good, though, because we specifically left it out of the admin web UI because the feature is still experimental and may be a security risk.
2. Gives all users in "No Institution" the ability to use skins. However, since this is an institution-level config, it would be more useful for admins to be able to decide whether the "No Institution" can use skins just like any other institution.
As described above, all we need to do for this bug is to make it so that the skins checkbox is exposed on the configuration screen for "No Institution". So I'm going to revert the patch and implement it that way.
Aaron Wells (u-aaronw) wrote : | #8 |
I've spun off https:/
Changed in mahara: | |
status: | Fix Committed → In Progress |
Mahara Bot (dev-mahara) wrote : | #9 |
Reviewed: https:/
Committed: http://
Submitter: Robert Lyon (<email address hidden>)
Branch: master
commit 666df54987d91ea
Author: Aaron Wells <email address hidden>
Date: Tue Nov 12 16:45:16 2013 +1300
Revert "Enable 'No institution' users to use skins (Bug 1249123) if it is activated in config.php"
This reverts commit 35a6a62ad98d9ff
Change-Id: I5902b77157fdb8
Mahara Bot (dev-mahara) wrote : | #10 |
Reviewed: https:/
Committed: http://
Submitter: Robert Lyon (<email address hidden>)
Branch: master
commit d2531cc40f89b7c
Author: Aaron Wells <email address hidden>
Date: Tue Nov 12 16:55:58 2013 +1300
Display the skins checkbox for the default institution config page
Change-Id: I959a291d13a321
Changed in mahara: | |
status: | In Progress → Fix Committed |
Robert Lyon (robertl-9) wrote : | #11 |
patch for v1.8
https:/
Mahara Bot (dev-mahara) wrote : | #12 |
Reviewed: https:/
Committed: http://
Submitter: Son Nguyen (<email address hidden>)
Branch: 1.8_STABLE
commit 751d3e162e0fb82
Author: Aaron Wells <email address hidden>
Date: Tue Nov 12 16:55:58 2013 +1300
Display the skins checkbox for the default institution config page
Change-Id: I959a291d13a321
Changed in mahara: | |
status: | Fix Committed → Fix Released |
Changed in mahara: | |
status: | Fix Committed → Fix Released |
I think we should probably move this setting into the configurable area for "No Institution". On the other hand, it may be worth revisiting what the rationale was for hiding so many other options from "No Institution". Perhaps that same rationale applies to skins?