Patch will probably fail with my luck, but here's what it looks like:

Index: location.module
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/location/location.module,v
retrieving revision 1.222.2.20
diff -u -r1.222.2.20 location.module
--- location.module	3 Mar 2009 20:07:19 -0000	1.222.2.20
+++ location.module	13 Oct 2009 11:36:05 -0000
@@ -86,7 +86,7 @@
  */
 function location_perm() {
   return array(
-    'submit latitude/longitude',
+    'submit latitude/longitude', 'view user list', 'view location list'
   );
 }
 
Index: location.views_default.inc
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/location/location.views_default.inc,v
retrieving revision 1.5
diff -u -r1.5 location.views_default.inc
--- location.views_default.inc	22 Oct 2008 17:28:48 -0000	1.5
+++ location.views_default.inc	13 Oct 2009 11:39:52 -0000
@@ -169,6 +169,10 @@
     'default' => '-1',
   ));
   $handler = $view->new_display('page', 'Page', 'page_1');
+  $handler->override_option('access', array(
+    'type' => 'perm',
+    'perm' => 'view location list',
+  ));
   $handler->override_option('path', 'location/views');
   $handler->override_option('menu', array(
     'type' => 'normal',
@@ -601,6 +605,10 @@
     'default' => '-1',
   ));
   $handler = $view->new_display('page', 'Page', 'page_1');
+  $handler->override_option('access', array(
+    'type' => 'perm',
+    'perm' => 'view user list',
+  ));
   $handler->override_option('path', 'location/users');
   $handler->override_option('menu', array(
     'type' => 'normal',
CommentFileSizeAuthor
#3 location_perms_change_1.patch1.11 KBhutch
location_perms_change.patch1.61 KBjerdiggity

Comments

jerdiggity’s picture

jerdiggity commented
Category: task » feature
yesct’s picture

yesct commented
Issue tags: +location permissions

tagging.

In case someone is looking at this, who has never reviewed a patch before:
http://drupal.org/patch/review
(And you dont have to do a complete review, even just doing a partial review is helpful. For example, just try and apply the patch and report back if it cleanly applies: http://drupal.org/patch/apply Or do a code style review. Or just apply it, and see if the permissions work, etc.)

hutch’s picture

hutch commented
StatusFileSize
new location_perms_change_1.patch1.11 KB

I've tested this patch on current location module, April 6 2010

patch -p0 < location_perms_change_0.patch
(Stripping trailing CRs from patch.)
patching file location.module
Hunk #1 succeeded at 95 (offset 9 lines).
(Stripping trailing CRs from patch.)
patching file location.views_default.inc

So it went in OK, a bit bumpy but patch is smart ;-)

Tested it and the perms work.
Ran Code review over the location module, plenty to do there but none of it related to this patch.

Attached is a re-roll of the patch using unix diff.

This is a good and sensible addition to this module.

yesct’s picture

yesct commented
Status: Needs review » Reviewed & tested by the community

I dont understand permissions and how they are controlled in code. Is it really this easy to add in new permissions?

hutch’s picture

hutch commented

Yes.
see http://api.drupal.org/api/function/hook_perm/6

Views has its own way of managing permissions as you can see in the rest of the patch, elsewhere you would use something like:

if (user_access('view user list')) {
  # do something
}

The Menu system has its own system for managing access permissions, very powerful.

yesct’s picture

yesct commented
Issue tags: +location bdragon consider commit

tagging

jerdiggity’s picture

jerdiggity commented
Title: Patch to make restrict access to location/users » Patch to provide access permissions/restrictions to locations

Yikes I must have been half-asleep when I originally submitted this issue. And then again two months later. ;)
(Changing title...)

hutch’s picture

hutch commented
Version: 6.x-3.1-rc1 » 6.x-3.x-dev

Just a reminder to the maintainer to consider this patch, the one in#3 works fine on this morning's snapshot

rooby’s picture

rooby commented
Status: Reviewed & tested by the community » Fixed

Committed to the D6 & D7 branches with a couple of minor changes:
* Naming of permissions
* Position of perms in default view array
* Also added perm for location directory view

yesct’s picture

yesct commented

http://drupal.org/cvs?commit=366560
and
http://drupal.org/cvs?commit=366566

:)

Status: Fixed » Closed (fixed)

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

agileware’s picture

agileware commented
Status: Closed (fixed) » Active
Issue tags: -location bdragon consider commit +location

I must have been fully asleep when I committed this.

I think this should be rolled back because views permissions are not fixed.
Users can edit the permissions in their views and make it so these permissions don't do what they say they do.

Why not just assign permissions by role to the views yourself?

Might need to also add an update to make sure we keep peoples permissions if they are currently using these.
It just needs to check the roles that have these permissions and add them to the views permissions.

rooby’s picture

rooby commented

Shit that was me on a different computer. Gotta check the login :(

jerdiggity’s picture

jerdiggity commented

All this patch was supposed to do was add two additional perms to location.module which could then be toggled/overridden as needed. If it's rolled back, hook_perm() in location.module will go back to consisting only of

function location_perm() {
  return array('submit latitude/longitude');
}

so I'm not sure what the benefit would be if it were rolled back... Unless I'm missing something (wouldn't be the first time), in which case, My bad. ;-)
 
Am I missing something?

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Issue summary: View changes
Status: Active » Closed (outdated)

Closing old D6 issues as D6 is end of life