Users who are in "No Institution" can't use skins

Bug #1249123 reported by Aaron Wells on 2013-11-07
6
This bug affects 1 person
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 :

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?

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 :

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 :
Changed in mahara:
assignee: nobody → Son Nguyen (ngson2000)
status: Confirmed → In Progress

Reviewed: https://reviews.mahara.org/2695
Committed: http://gitorious.org/mahara/mahara/commit/35a6a62ad98d9ff830291eb14815e8b0704b09c6
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 35a6a62ad98d9ff830291eb14815e8b0704b09c6
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: I4464867db90dc9fd34220b1d865fb5b3527ef168
Signed-off-by: Son Nguyen <email address hidden>

Robert Lyon (robertl-9) on 2013-11-11
Changed in mahara:
status: In Progress → Fix Committed
Aaron Wells (u-aaronw) wrote :

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 :
description: updated
Aaron Wells (u-aaronw) wrote :

I've spun off https://bugs.launchpad.net/mahara/+bug/1250302 for the separate issue of a site admin being unable to edit site skins if they belong to an institution with skins disabled.

Aaron Wells (u-aaronw) on 2013-11-12
Changed in mahara:
status: Fix Committed → In Progress
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2701
Committed: http://gitorious.org/mahara/mahara/commit/666df54987d91ea3522a8d7e9df36aff73950929
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 666df54987d91ea3522a8d7e9df36aff73950929
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 35a6a62ad98d9ff830291eb14815e8b0704b09c6.

Change-Id: I5902b77157fdb8df7c73dba27921c832c69b338b

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2702
Committed: http://gitorious.org/mahara/mahara/commit/d2531cc40f89b7cc889f6fc1671c0805f0019fce
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit d2531cc40f89b7cc889f6fc1671c0805f0019fce
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

Bug 1249123

Change-Id: I959a291d13a32132aa1cd791f1b8f094c8bfd5c4

Robert Lyon (robertl-9) on 2013-11-17
Changed in mahara:
status: In Progress → Fix Committed
Robert Lyon (robertl-9) wrote :
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2786
Committed: http://gitorious.org/mahara/mahara/commit/751d3e162e0fb821eeb893727c0ecb0553f6200e
Submitter: Son Nguyen (<email address hidden>)
Branch: 1.8_STABLE

commit 751d3e162e0fb821eeb893727c0ecb0553f6200e
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

Bug 1249123

Change-Id: I959a291d13a32132aa1cd791f1b8f094c8bfd5c4

Aaron Wells (u-aaronw) on 2013-12-18
Changed in mahara:
status: Fix Committed → Fix Released
Robert Lyon (robertl-9) on 2014-04-22
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers