Hi,

Module does not have any entry in permission table, I think should be at least one entry for allow view report (admin/reports/browscap), just because there is no way to delegate this option to other roles.

CommentFileSizeAuthor
#11 add-permissions-650688-11.patch2.39 KBdevin carlson
#8 add-permissions-650688-8.patch2.47 KBdevin carlson
#6 browscap_650588_6_hook_perm.patch2.8 KBsimon georges
#2 browscap_650588_2_hook_perm.patch2.8 KBsimon georges

Comments

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

I agree, I need this one too.

...subscribing

simon georges’s picture

simon georges commented
Status: Active » Needs review
StatusFileSize
new browscap_650588_2_hook_perm.patch2.8 KB

Patch provided.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Needs work

Looks mostly OK to me. However, given that the configuring can change an important function I suggest "administer browscap" instead of just "configure browscap."

What do you think?

simon georges’s picture

simon georges commented

Fine for me ;) Do you want me to re-roll a patch ?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Please do, yes.

simon georges’s picture

simon georges commented
Version: 6.x-1.1 » 6.x-1.x-dev
Status: Needs work » Needs review
StatusFileSize
new browscap_650588_6_hook_perm.patch2.8 KB

New patch, with "administer browscap", done on the latest 6.x-1.x-dev.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

#6 seems to work fine. Thanx ;)

devin carlson’s picture

devin carlson commented
Title: view report permission » Add permission to view reports
Version: 6.x-1.x-dev » 7.x-1.x-dev
Assigned: Unassigned » devin carlson
Issue tags: -granular permissions +needs backport to 6.x
StatusFileSize
new add-permissions-650688-8.patch2.47 KB

Moving to Browscap 7.x and then backporting.

The attached patch adds the permissions outlined in #6 to Browscap 7.x.

devin carlson’s picture

devin carlson commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs review » Needs work

Committed to 7.x-1.x.

Moving back to 6.x and marking as "needs work" to test the last patch for 6.x and ensure that it uses the same permission names as the 7.x patch.

devin carlson’s picture

devin carlson commented
Status: Needs work » Patch (to be ported)
devin carlson’s picture

devin carlson commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new add-permissions-650688-11.patch2.39 KB

Backport of #8.

devin carlson’s picture

devin carlson commented
Status: Needs review » Fixed

Committed to 6.x-1.x.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

Thanx Devin! One less patch to keep around and having to re-apply with each installation.

PS: ...welcome as a new co-maintainer of Browscap btw ;)

Status: Fixed » Closed (fixed)
Issue tags: -needs backport to 6.x

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +needs backport to 6.x

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.