A site admin who belongs to an institution with skins disabled, gets an error trying to edit site skins

Bug #1250302 reported by Aaron Wells on 2013-11-12
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
High
Son Nguyen
1.10
High
Son Nguyen
1.8
High
Son Nguyen
1.9
High
Son Nguyen

Bug Description

To replicate:

1. Set $cfg->skins = true; in your config.php to enable skins
2. Disable skins for at least one institution, by using the institution configuration screen
3. Log in as a site admin who belongs to that institution
4. Navigate to "Configure site -> Skins".
5. Click on "Create skin"

Expected result: You should be able to create a skin
Actual result: You see a "feature not available" exception

Aaron Wells (u-aaronw) wrote :

I'm not sure what the best way to proceed with this is. The previous patch for Bug 1249123 resolved the issue by making it so that if you're an admin, "can_use_skins()" always returns true. This gets rid of the error messages, but it has the unfortunate side effect of also allowing site admins to use skins in their personal pages, regardless of the settings for the institution they belong to. I can easily see this causing problems when admins think that skins are turned on for all users, when in fact they're only turned on for the admins.

One alternative is to change every page so that it checks whether you're editing a site page before it calls can_use_skins(). But this is ugly and kind of error-prone.

A third option, is to parameterize can_use_skins(). Pass in a parameter that indicates whether you're working on a normal page or a site page, and use that to determine the answer. I think that's probably the way we should go.

Site admins are often not members of institutions and if skins are turned on in general, i.e. "No institution", they'll be able to use them. However, if site admins are members of institutions, the institution settings should be respected for their personal pages. Site pages could have skins, but then they aren't editing their personal pages, but a general page that doesn't bear their name.

Your last idea sounds good to me.

Son Nguyen (ngson2000) on 2013-11-19
Changed in mahara:
assignee: nobody → Son Nguyen (ngson2000)
Son Nguyen (ngson2000) on 2013-11-19
Changed in mahara:
status: New → In Progress
Aaron Wells (u-aaronw) on 2013-12-16
Changed in mahara:
milestone: 1.8.1 → 1.8.2

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

commit 0bc935fde9f55c1e594da48252618944b205a170
Author: Son Nguyen <email address hidden>
Date: Wed Nov 20 10:00:20 2013 +1300

Allow site admins to manage site skins (Bug 1250302)

and allow site pages to use site skins

Change-Id: Idf8a8a761c850ed609f78ed65175ee82c3370e2f
Signed-off-by: Son Nguyen <email address hidden>

Mahara Bot (dev-mahara) wrote :

Patch for "1.8_STABLE" branch: https://reviews.mahara.org/3276

Reviewed: https://reviews.mahara.org/3275
Committed: http://gitorious.org/mahara/mahara/commit/ea73fecbd4f5ec3839d9b396c4f672e6bd1f4af3
Submitter: Robert Lyon (<email address hidden>)
Branch: 1.9_STABLE

commit ea73fecbd4f5ec3839d9b396c4f672e6bd1f4af3
Author: Son Nguyen <email address hidden>
Date: Wed Nov 20 10:00:20 2013 +1300

Allow site admins to manage site skins (Bug 1250302)

and allow site pages to use site skins

Change-Id: Idf8a8a761c850ed609f78ed65175ee82c3370e2f
Signed-off-by: Son Nguyen <email address hidden>

Aaron Wells (u-aaronw) on 2014-10-20
Changed in mahara:
milestone: 1.10.0 → none
Aaron Wells (u-aaronw) on 2014-10-21
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