superbaloo [superbaloo] [#538850]

Hi !

I recently (1 year ago) discovered Drupal through an internship at the Open
Web Solutions company (http://ows.fr).

As I really do like the drupal way of thinking which make it one of the most
powerful CMS over the world, I was missing a feature that could permit me to
have a fine-cut control over the user access : an access control based on the
drupal taxonomy, so i did create my own module to do that.

At first sight, you may think it's just another fork of already available module
on drupal.org like taxonomy_access or tac_lite. First one is a taxonomy control
access based on roles, second one is a taxonomy control access based on users. But
both of those modules miss the - according to me essential - inheritance notion :
if you have access to one term, you don't automatically have access to the children
nodes. My module does take care about inheritance and this way permit a powerfull
user access control.

Description:
For this description, I will try to illustrate the module with the following term
scheme:
1 (root) -> 2 -> 4
-> 5 -> 7
-> 3 -> 6

The basic idea of inheritance is : if you have access to one term then you may
access all children terms.

So I provided 4 different grant access for this module:
- grant_view
Which controls whether the user may view nodes associated to this term
- grant_update
Which controls whether the user may add nodes associated to this term or edit
them
- grant_delete
Which controls whether the user may delete nodes associated to this term
- grant_admin
Which is the most particular grant out of the four.

The last grant access (grant_admin) permits a user to "delegate" control of an access
over a term to another user : if a lambda use has the grant_admin over the term 2, he
may grant view to another user to terms 2, 4, 5 or 7.

So now you are familiar with the possible grant access, if the user lambda have
grant_view on the term 1, then he may view nodes associated with the term 1, 2, 3, 4,
5, 6 and/or 7. The only record in the database about the grant access is about the
term 1. To reach this goal in a reasonnable amount of time and with simple queries, I
used SQL functions in the DBMS. Availability of those functions is controlled each
time the module is invoked by the hook_init which permit not to break the module even
if the database is transfered to another dbms and if the user has forgot to backup
sql functions :).

Yet another feature of this module is the ability to restrain access to a subtree
while he still have access to the other part. For example, if i set grant_view = 1 to
the term 1 to the user lambda and grant_view = -1 to the term 2, then the user may view
terms 1, 3 and 6 while access to terms 2, 4, 5 and 7 would be denied. The default value
is 0 and it's the value which permits inheritance.

This module also provides a UI to administer the module, views and views handler for
specific fields. It is also tested with simpletest, some work on code coverage has been done.

This module could have multiple applications :
- An e-commerce website where a department supervisor may edit nodes associated to
his department or delegate rights to his employees
- An news website where the person in charge of the sport section may only create
articles in his own section or may delegate access to the golf area to one specific
person.

The module could be found
here : http://github.com/baloo/taxonomy-access-user/tarball/master

Thank you in Advance for accepting my CVS account request.

Comment	File	Size	Author
#33	taxonomy-access-user.tar_.gz	13.08 KB	superbaloo
#31	taxonomy_access_user.tar_.gz	11.72 KB	superbaloo
#31	taxonomy_access_user.zip	17.99 KB	superbaloo
#29	taxonomy_access_user.tgz	11.71 KB	superbaloo
#25	taxonomy_access_user.tar_.gz	11.7 KB	superbaloo
#25	taxonomy_access_user.zip	17.96 KB	superbaloo
#24	taxonomy_access_user.tar_.gz	11.73 KB	superbaloo
#24	taxonomy_access_user.zip	17.96 KB	superbaloo
#19	taxonomy_access_user.tar_.gz	11.87 KB	superbaloo
#19	taxonomy_access_user.zip	18.12 KB	superbaloo
#15	taxonomy_access_user.tar_.gz	11.83 KB	superbaloo
#15	taxonomy_access_user.zip	18.1 KB	superbaloo
#13	taxonomy-access-user.tar_.gz	13.46 KB	superbaloo
#13	taxonomy-access-user.zip	22.24 KB	superbaloo
#11	baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544.tar_.gz	12.82 KB	superbaloo
#6	baloo-taxonomy-access-user-a001db98e4c74e5c50abb5e4a225daeadd7545f5.tar_.gz	12.81 KB	superbaloo
#4	baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd.tar_.gz	12.77 KB	superbaloo

Comments

Comment #1

superbaloo commented 3 August 2009 at 16:23

BTW, the module is sponsored by Open Web Solutions (http://ows.fr - http://drupal.org/profile/companies/Open%20web%20solutions)

Comment #2

ultimateboy commented 3 August 2009 at 21:06

Status:

Postponed (maintainer needs more info)

» Needs review

Link to tarball is in original post. This is ready for review.

Comment #3

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 4 August 2009 at 01:27

Status:

Needs review

» Needs work

http://github.com/baloo/taxonomy-access-user/tarball/master downloads a tarball archive that contains just two empty folders, and a README.

Comment #4

superbaloo commented 4 August 2009 at 05:47

Status:

Needs work

» Needs review

Status	File	Size
new	baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd.tar_.gz	12.77 KB

?! It's working with me, but anyway here is the tgz as attachement ;)

% tar tzvf baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd.tar.gz
drwxrwxr-x root/root         0 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/
-rw-rw-r-- root/root         5 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/.gitignore
-rw-rw-r-- root/root      2916 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/README
drwxrwxr-x root/root         0 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/
drwxrwxr-x root/root         0 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/includes/
drwxrwxr-x root/root         0 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/includes/views/
drwxrwxr-x root/root         0 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/includes/views/handlers/
-rw-rw-r-- root/root      1131 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/includes/views/handlers/views_handler_field_multiple_values.inc
-rw-rw-r-- root/root      1121 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/includes/views/handlers/views_handler_field_taxonomy_access_user_link_delete.inc
-rw-rw-r-- root/root      1113 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/includes/views/handlers/views_handler_field_taxonomy_access_user_link_edit.inc
-rw-rw-r-- root/root      2986 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/includes/views/taxonomy_access_user_views.views.inc
-rw-rw-r-- root/root      8141 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/includes/views/taxonomy_access_user_views.views_default.inc
-rw-rw-r-- root/root     10355 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/taxonomy_access_user.admin.inc
-rw-rw-r-- root/root      5623 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/taxonomy_access_user.inc
-rw-rw-r-- root/root       157 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/taxonomy_access_user.info
-rw-rw-r-- root/root      2254 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/taxonomy_access_user.install
-rw-rw-r-- root/root     15829 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/taxonomy_access_user.module
-rw-rw-r-- root/root      5290 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/taxonomy_access_user.test
-rw-rw-r-- root/root       187 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/taxonomy_access_user_views.info
-rw-rw-r-- root/root      1609 2009-08-03 18:17 baloo-taxonomy-access-user-f39c8876d8708ac7c8365324e7c736a4ebf242cd/taxonomy_access_user/taxonomy_access_user_views.module

If you prefer git ...

git clone git://github.com/baloo/taxonomy-access-user.git

would do the work too ;)

Comment #5

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 4 August 2009 at 12:37

Status:

Needs review

» Needs work

  foreach ($form as $key => $value) {
    // Get keys like tid:<tid>:0
    if (drupal_substr($key, 0, 4) === 'tid:' &&
      _taxonomy_access_user_access(taxonomy_get_term($form[$key]['#term']['tid']), array('grant_admin'))) {

It is better to use element_children(), which filters out all the keys starting with "#" (you are not looking for them). This would speed up the search for the keys.

  if (!is_null($access) && !is_array($access)) {
    trigger_error('$access argument is neither NULL nor array');
    return NULL;
  }

The code should call watchdog(), and then return to the caller.

/**
 * Implementation of hook_enable().
 * Installs the sql functions
 */
function taxonomy_access_user_enable() {
  module_load_include('inc', 'taxonomy_access_user');
  taxonomy_access_user_sql_uninstall();
  taxonomy_access_user_sql_install();
}

hook_enable() should be in the installation file.

  $items['taxonomy/access/%taxonomy_vocabulary'] = array(
    'title' => 'List terms',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('taxonomy_overview_terms', 2),
    'access callback' => 'taxonomy_access_user_vocabulary_admin',
    'access arguments' => array(2),
    'file' => 'taxonomy.admin.inc',
    'file path' => drupal_get_path('module', 'taxonomy'),
    'type' => MENU_CALLBACK,
  );

I am not sure what the purpose of that menu callback is. May you explain it?

```
  if ($account === NULL)
    $account = $user;
 
  if ($account === NULL)
    return FALSE;
 
```
The global variable is normally initialized; even when the user is anonymous, the variable is not null.
```
  $result = db_query_range('SELECT 1 FROM {term_data} td
    WHERE td.vid = %d AND grant_admin(tid,%d) = 1',
    $vocabulary->vid,
    $account->uid, 0, 1);
```
There is a easier way to count the number of rows matching a criteria (and it would not require db_query_range(). Also, I am not sure about grant_admin(tid, %d). That is not a syntax accepted from all the database engines; you should use a generic SQL syntax, so the module works in every database engines Drupal support (already Drupal 7 supports a new database engine, and more could be added in future).

  if (db_fetch_array($result))
    return TRUE;
 
  return FALSE;

Follow the coding standards.

```
  global $user, $db_prefix;
```
$db_prefix is never used from a module.

  // Else verify with the database
  $result = db_query('SELECT '. $db_prefix .'_grant_admin(tid,%d) FROM {term_access_user} WHERE tauid = %d', $user->uid, $tauid->tauid);

The syntax of the query is not standard, or not accepted by all the database engine versions.

The module you propose extend something done by an already existing module. In these cases, Drupal suggests to join forces with the maintainer of that module.

Comment #6

superbaloo commented 4 August 2009 at 14:16

Status:

Needs work

» Needs review

Status	File	Size
new	baloo-taxonomy-access-user-a001db98e4c74e5c50abb5e4a225daeadd7545f5.tar_.gz	12.81 KB

OK
OK
OK
OK This element permit user that don't have the "administer taxonomy" permission
to access taxonomy term list and display a nice form to edit permissions of a
vocabulary. They may access the list only if they have at least one grant_admin
on a term of the list.
OK didn't know
Yes, grant_admin is an sql function. I did choose a sql function because queries
are _much_ simplier to write and _much_ faster to execute :).
About functions support in DBMS:
- First, it's a drupal 6 module and both mysql and pgsql are supported
- Second, most DBMS support sql functions, the work in taxonomy_access_user.inc
  can be extended to sqlite (@see http://php.net/manual/en/function.sqlite-create-function.php),
  oracle that is very similar to mysql syntax (or probably the opposite ! :p), mssql, ...
  (the only one not supporting functions is mysql 4.x)
brace are optional according to the coding standards ... but OK (probably missed somes)
OK the db_query did the work well :)
supported by mysql (>=5.0) and pgsql ... yes in drupal 6.x we lose mysql 4.x support ...

As i said in the readme file, there are two other modules providing taxonomy access control :
- tac_lite,
- taxonomy_access

both of them don't provide the inheritence notion.
The first one don't use the database (that's the purpose of the "lite"),
the second one is role-based. If you really want me to merge with one of the existing modules,
i will have to convince the tac_lite maintainer that using database is okay or convince the taxonomy_access
module maintainer to convert his role-based access control module to user-based access control ...

Comment #7

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 4 August 2009 at 19:30

This is what the Drupal coding standards report:

You are strongly encouraged to always use curly braces even in situations where they are technically optional. Having them increases readability and decreases the likelihood of logic errors being introduced when new lines are added.

It doesn't say the braces are optional; it say you are strongly encouraged to use them.

OK This element permit user that don't have the "administer taxonomy" permission
to access taxonomy term list and display a nice form to edit permissions of a
vocabulary. They may access the list only if they have at least one grant_admin
on a term of the list.

If the user doesn't have permission to administer taxonomy, why should s/he be allowed to change the taxonomy permissions?
Also, the code is showing the taxonomy page at another URL; It would be better to alter the taxonomy page, not to duplicate it.

Comment #8

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 4 August 2009 at 19:45

Status:

Needs review

» Needs work

Comment #9

superbaloo commented 4 August 2009 at 20:06

Status:

Needs work

» Needs review

Ok,
this element is just a fork of the menu element that gives /?q=admin/content/taxonomy/1,
this page is defined like this in the taxonomy.module

<?php
  $items['admin/content/taxonomy/%taxonomy_vocabulary'] = array(
    'title' => 'List terms',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('taxonomy_overview_terms', 3),
    'access arguments' => array('administer taxonomy'),
    'type' => MENU_CALLBACK,
    'file' => 'taxonomy.admin.inc',
  );
?>

The menu here check if the user have "administer taxonomy". User that may have grant_admin on a term, may not have the "administer taxonomy" permission, but i want them to still administer terms they may admin.

The only way i've found was to reimplement a another page with my own access callback that check whether the user may admin one term of the taxonomy or not ... Is it incorrect ? what is the "drupal way" to do this ?

implement it like this ?

<?php
  $items['admin/content/taxonomy/%taxonomy_vocabulary'] = array(
    'title' => 'List terms',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('taxonomy_overview_terms', 2),
    'access callback' => 'taxonomy_access_user_vocabulary_admin',
    'access arguments' => array(2),
    'file' => 'taxonomy.admin.inc',
    'file path' => drupal_get_path('module', 'taxonomy'),
    'type' => MENU_CALLBACK,
  );
?>

Comment #10

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 4 August 2009 at 20:37

Status:

Needs review

» Needs work

When you need to alter a menu callback defined by another module, it's enough to implement hook_menu_alter().

Comment #11

superbaloo commented 5 August 2009 at 08:26

Status:

Needs work

» Needs review

Status	File	Size
new	baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544.tar_.gz	12.82 KB

Here are the updates

Comment #12

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 5 August 2009 at 09:54

Status:

Needs review

» Needs work

The tarball archive you attach keep to show me only two directories, and a README.

Comment #13

superbaloo commented 5 August 2009 at 10:05

Status:

Needs work

» Needs review

Status	File	Size
new	taxonomy-access-user.zip	22.24 KB
new	taxonomy-access-user.tar_.gz	13.46 KB

(11:58:31) [superbaloo @ desktop-ows] [~/Downloads]
% wget http://drupal.org/files/issues/baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544.tar_.gz
--2009-08-05 11:58:34--  http://drupal.org/files/issues/baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544.tar_.gz
Résolution de drupal.org... 140.211.166.6
Connexion vers drupal.org|140.211.166.6|:80...connecté.
requête HTTP transmise, en attente de la réponse...200 OK
Longueur: 13132 (13K) [application/x-gzip]
Saving to: `baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544.tar_.gz'

100%[==========================================================================================================================================================================================================================================>] 13 132      24,9K/s   in 0,5s    

2009-08-05 11:58:35 (24,9 KB/s) - « baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544.tar_.gz » sauvegardé [13132/13132]

(11:58:35) [superbaloo @ desktop-ows] [~/Downloads]
% tar tzvf baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544.tar_.gz                           
drwxrwxr-x root/root         0 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/
-rw-rw-r-- root/root         5 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/.gitignore
-rw-rw-r-- root/root      2916 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/README
drwxrwxr-x root/root         0 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/
drwxrwxr-x root/root         0 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/
drwxrwxr-x root/root         0 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/
drwxrwxr-x root/root         0 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/
-rw-rw-r-- root/root      1147 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_multiple_values.inc
-rw-rw-r-- root/root      1121 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_taxonomy_access_user_link_delete.inc
-rw-rw-r-- root/root      1113 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_taxonomy_access_user_link_edit.inc
-rw-rw-r-- root/root      2986 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/taxonomy_access_user_views.views.inc
-rw-rw-r-- root/root      8141 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/taxonomy_access_user_views.views_default.inc
-rw-rw-r-- root/root     10405 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.admin.inc
-rw-rw-r-- root/root      5501 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.inc
-rw-rw-r-- root/root       157 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.info
-rw-rw-r-- root/root      2502 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.install
-rw-rw-r-- root/root     15794 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.module
-rw-rw-r-- root/root      5314 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.test
-rw-rw-r-- root/root       187 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user_views.info
-rw-rw-r-- root/root      1609 2009-08-05 10:21 baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user_views.module

anyway ... here is another tarball and a zip file

(12:00:12) [superbaloo @ desktop-ows] [~/Downloads]
% tar zcvf taxonomy-access-user.tar.gz baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/README
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/.gitignore
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.admin.inc
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user_views.info
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.info
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.test
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.module
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_taxonomy_access_user_link_edit.inc
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_multiple_values.inc
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_taxonomy_access_user_link_delete.inc
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/taxonomy_access_user_views.views.inc
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/taxonomy_access_user_views.views_default.inc
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user_views.module
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.inc
baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.install
(12:00:47) [superbaloo @ desktop-ows] [~/Downloads]
% zip -r taxonomy-access-user.zip baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/
updating: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/ (stored 0%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/README (deflated 57%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/.gitignore (stored 0%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/ (stored 0%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.admin.inc (deflated 77%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user_views.info (deflated 34%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.info (deflated 26%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.test (deflated 74%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.module (deflated 75%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/ (stored 0%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/ (stored 0%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/ (stored 0%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_taxonomy_access_user_link_edit.inc (deflated 54%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_multiple_values.inc (deflated 55%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/handlers/views_handler_field_taxonomy_access_user_link_delete.inc (deflated 54%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/taxonomy_access_user_views.views.inc (deflated 78%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/includes/views/taxonomy_access_user_views.views_default.inc (deflated 84%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user_views.module (deflated 66%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.inc (deflated 71%)
  adding: baloo-taxonomy-access-user-86ca5262f25ce4b6c7168aca437844d2eda5e544/taxonomy_access_user/taxonomy_access_user.install (deflated 77%)

Comment #14

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 5 August 2009 at 11:51

Status:

Needs review

» Needs work

This time the archives worked; maybe the problem is simply the long name.

  if ($user !== NULL && $user->uid == $tauid->uid) {
    drupal_set_message(t('You are about editing your own access to this term'), 'warning');
  }

The user object is always initialized.

    form_set_error(
      'user',
      t(
        'User already assigned to this term, click !link_here to edit it',
        array(
          '!link_here' => l(
            t('here'),
            'taxonomy/access/edit/'. $row->tauid,
            array(
              'attributes' => array(
                'class' => 'popups-form-reload',
              ),
              'query' => drupal_get_destination(),
            )),
        )));

Together t() you use url(), not l(). That is also reported in the documentation for t().

  $term_access_user = array(
    'tauid' => $form_state['values']['term_access_user'],
    'uid' => $user->uid,
    'grant_view' => $form_state['values']['grant_view'],
    'grant_update' => $form_state['values']['grant_update'],
    'grant_delete' => $form_state['values']['grant_delete'],
    'grant_admin' => $form_state['values']['grant_admin'],
  );

  drupal_write_record('term_access_user', $term_access_user, array('tauid'));

The code doesn't work when the record being written in the database doesn't exist. That is because the last parameter is used only when you are updating a table record; if you are writing a new record, you don't need to pass the last parameter.

The module saves in its own database table the user name, and the user ID. The data are redundant (from the user ID you can get the user name). Drupal allows users with the right permission to change their user name; in such cases, it's perfectly useless to save the user name (but then, the unique user identifier that will never change is already saved).

  $user = user_load(array('name' => $form_state['values']['user']));

  if (!$user) {
    form_set_error('user', t('User does not exist'));
  }

  if (!$user = user_load(array('name' => $form_state['values']['user']))) {
    form_set_error('user', t('User does not exist'));
  }

function taxonomy_access_user_form_alter(&$form, $form_state, $form_id) {
  // If the form has taxonomy
  if (isset($form['taxonomy'])) {
    foreach ($form['taxonomy'] as $vid => $taxonomy) {

      // Sanity check
      if (is_array($taxonomy) && isset($taxonomy['#type'])) {

        // If the taxonomy is value
        if ($taxonomy['#type'] === 'value') {

          //Verify the vid is handled by the module
          if (array_search($vid, variable_get('taxonomy_access_user_vids', array())) !== FALSE) {
            $term  = taxonomy_get_term($taxonomy['#value']);

            // Verify the user may add/edit the node
            if (taxonomy_access_user_control('access', $term, array('grant_update')) === FALSE) {

              // Return access denied
              drupal_access_denied();
              die();
            }
          }
        }
      }
    }
  }
}

The code should check if the form it is acting on is the one it is supposed to work on; that is the reason of passing $form_id.

```
  watchdog('taxonomy access user', '$access argument is neither NULL nor array');
```
The error message doesn't help much, and it gives details that are not useful for who reads the error message. "Wrong parameters passed to taxonomy_access_user_control()." should be more helpful; at least they would know there is something wrong in the code, and they could report that there is something wrong in a call to taxonomy_access_user_control(). It is then a kind of message that is not normally given by any modules. I would rather remove the checking for the parameters, in such cases.
```
  @drupal_write_record('term_access_user', $term_access_user);
```
The code doesn't check if the record is trying to write already exist. If the module is writing data it should not be writing, then something must be corrected in the module; if then the module must write a record if that doesn't already exist, then it should check if the record exists, before to write it.

function taxonomy_access_user_menu_alter(&$items) {
  if (!isset($items['admin/content/taxonomy/%taxonomy_vocabulary'])) {
    return NULL;
  }

  // This element permit user that don't have the "adminiter taxonomy" permission
  // to access taxonomy term list and display a nice for to edit permissions of a
  // vocabulary
  $items['admin/content/taxonomy/%taxonomy_vocabulary']['access callback'] = 'taxonomy_access_user_vocabulary_admin';
  $items['admin/content/taxonomy/%taxonomy_vocabulary']['access arguments'] = array(3);

}

function taxonomy_access_user_menu_alter(&$items) {
  if (isset($items['admin/content/taxonomy/%taxonomy_vocabulary'])) {
    // This element permit user that don't have the "adminiter taxonomy" permission
    // to access taxonomy term list and display a nice for to edit permissions of a
    // vocabulary
    $items['admin/content/taxonomy/%taxonomy_vocabulary']['access callback'] = 'taxonomy_access_user_vocabulary_admin';
    $items['admin/content/taxonomy/%taxonomy_vocabulary']['access arguments'] = array(3);
  }
}

I am not still sure what you do with a SQL function cannot be done differently. I would understand the use of a SQL function if there are not other ways to do what you are trying to do; still, I would put the compatibility at first place.
taxonomy_access_user_sql_uninstall(), and taxonomy_access_user_sql_install() should be placed in the installation file, as they are only used during the installation/uninstallation of the module.

  if (array_search($access, $ops, TRUE) === FALSE)
    return FALSE;

See the coding standards.

  if ($result == '1') {
    return TRUE;
  }
  else {
    return FALSE;
  }

  return $result == '1';

Or simply (if the code calling that function is not strictly expecting a boolean, which is true most of the times)

  return $result;

    if ($value == 1) {
      if (!_taxonomy_access_user_access_tid_one_access($tid, $key, $account)) {
        return FALSE;
      }
    }

    if ($value == 1 && !_taxonomy_access_user_access_tid_one_access($tid, $key, $account)) {
      return FALSE;
    }

    if ($value && !_taxonomy_access_user_access_tid_one_access($tid, $key, $account)) {
      return FALSE;
    }

The first IF-statement should be used if the function could return also -1; the second IF-statement should be used if the function can just return 1 and 0 (or a positive number, and zero).

  else{
    $tids = array(
      $terms->tid
    );

  else{
    $tids = array($terms->tid);

Use more than one line if the code line is longer than 80 characters.

    $result = _taxonomy_access_user_access_tid($tid, $access, $account);
    if ($result !== TRUE) {
      return FALSE;
    }

    if (!_taxonomy_access_user_access_tid($tid, $access, $account)) {
      return FALSE;
    }

There is no need to use a variable for the result of a function, when the result is just used in the IF-statement.

Comment #15

superbaloo commented 5 August 2009 at 15:08

Status:

Needs work

» Needs review

Status	File	Size
new	taxonomy_access_user.zip	18.1 KB
new	taxonomy_access_user.tar_.gz	11.83 KB

Ok i will try to do it like this now
OK
OK
OK
What makes you think that ? look at the term_access_user schema, there is no place to store a username ... it's only ids
OK
OK

<?php
function taxonomy_access_user_form_alter(&$form, $form_state, $form_id) {
  // If the form is a node form
  if (substr($form_id, -10, 10) !== '_node_form') {
    return NULL;
  }



  // If the form has taxonomy
  if (isset($form['taxonomy'])) {
    foreach ($form['taxonomy'] as $vid => $taxonomy) {

      // Sanity check
      if (is_array($taxonomy) && isset($taxonomy['#type'])) {

        // If the taxonomy is value
        if ($taxonomy['#type'] === 'value') {

          //Verify the vid is handled by the module
          if (array_search($vid, variable_get('taxonomy_access_user_vids', array())) !== FALSE) {
            $term  = taxonomy_get_term($taxonomy['#value']);

            // Verify the user may add/edit the node
            if (taxonomy_access_user_control('access', $term, array('grant_update')) === FALSE) {

              // Return access denied
              drupal_access_denied();
              die();
            }
          }
        }
      }
    }
  }
}
?>

<?php
    if (!$tauid = db_result(db_query_range('SELECT tauid FROM {term_access_user} WHERE tid = %d AND uid = %d', $term->tid, $account->uid, 0, 1))) {
      drupal_write_record('term_access_user', $term_access_user);
    }
    else {
      $term_access_user['tauid'] = $tauid;
      drupal_write_record('term_access_user', $term_access_user, array('tauid'));
    }
?>

OK
I just can't imagine the hook_db_rewrite_sql() without this sql function
Pros:
- Perfomance
- Simplicity
Cons:
- Mysql 4.x compatibility
For me the compatibility with mysql 4.x doesn't justify the complexity increase. (Personnal choice)
No, it is used in hook_init() too that check if the functions are available or not. I've done this like this because the drupal webmaster may dump his database without functions and then crash the module
OK, missed this one
OK, first solution prefered
OK
OK
OK

Comment #16

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 5 August 2009 at 15:40

What makes you think that ? look at the term_access_user schema, there is no place to store a username ... it's only ids

It doesn't save the user name because when you know the user ID (uid), you can find the user name with user_load(). If you save the user name, and then you present that on the settings page for taxonomy access, you would shown a wrong user name, if the users are allowed to change their own user names (as it happens on Drupal.org, in example). The only value that doesn't change for a user is his uid.

Installation tasks are only executed during the installation/updating of the module; hook_init() should not do such tasks. Check any modules, and you will see that none of the modules you see do that.

I am not sure that using a SQL function in the module simplified the code; as I can see it, you added more code, and you didn't simplified it. I am not sure about the performance either.

Comment #17

superbaloo commented 5 August 2009 at 15:56

The module saves in its own database table the user name, and the user ID. The data are redundant (from the user ID you can get the user name). Drupal allows users with the right permission to change their user name; in such cases, it's perfectly useless to save the user name (but then, the unique user identifier that will never change is already saved).

First, I agree that the module should not store the username but only the user ID but you said i was saving the user name in the module database table while there is no field where to do that. So my question was "where do you think the code is trying to store the username in the database?"

Second, if I don't do the check in the hook_init() what is the best place to do so ? At least do you agree i should do the check at drupal startup ?

Third, the question of the SQL function would probably be an eternal question around the module, ... My choice was to code it with the sql function and this module is now up and running for almost a month without problems. I'm not against removing this sql function and doing it only in php. If someone submit a patch, has an way to do it, ... that's okay but i'd just don't know how to do it in a simple way :) .

Comment #18

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 5 August 2009 at 17:01

Status:

Needs review

» Needs work

  // Tests the user
  $account = user_load(array('name' => $form_state['values']['user']));
  if (!$account = user_load(array('name' => $form_state['values']['user']))) {
    form_set_error('user', t('User does not exist'));
  }

You load the user object twice.

  if (is_array($tids) === FALSE) {
    $tids = array($tids);
  }

There is no need to use the ===; then the code can be simply written as

  if (!is_array($tids)) {
    $tids = array($tids);
  }

  $result = db_query("SELECT
    tid,
    {grant_view}(tid, %d) as grant_view,
    {grant_update}(tid, %d) as grant_update,
    {grant_delete}(tid, %d) as grant_delete,
    {grant_admin}(tid, %d) as grant_admin
    FROM {term_data}
    WHERE
      tid IN ('%s')",
    $account->uid,
    $account->uid,
    $account->uid,
    $account->uid,

    implode($tids, '\',\''));

You should use the function db_placeholders().

      if (is_array($taxo))
        $taxo[$tid]->$name = $value;
      if (is_object($taxo))
        $taxo->$name = $value;

See the coding standards.

  // If the form is a node form
  if (substr($form_id, -10, 10) !== '_node_form') {
    return NULL;
  }

To verify if the form is a node edit form, the code is a little more complicated.

  if (isset($form['type']) && isset($form['#node']) && $form['type']['#value'] .'_node_form' == $form_id) {

  if (taxonomy_access_user_control('access', $term, array('grant_update')) === FALSE) {

  if (!taxonomy_access_user_control('access', $term, array('grant_update'))) {

  db_result(db_query_range('SELECT tauid FROM {term_access_user} WHERE tid = %d AND uid = %d', $term->tid, $account->uid, 0, 1));

  db_result(db_query('SELECT tauid FROM {term_access_user} WHERE tid = %d AND uid = %d', $term->tid, $account->uid));

There is no need to use db_query_range() as you are calling db_query() only once, in this case.

    return $term_access_user['tauid'];
    break;

It's enough to use the return statement.

                'grant_view' => (isset($done_grants['grant_view']) === FALSE && $row->grant_view == 1)?1:0,
                'grant_update' => (isset($done_grants['grant_update']) === FALSE && $row->grant_update == 1)?1:0,
                'grant_delete' => (isset($done_grants['grant_delete']) === FALSE && $row->grant_delete == 1)?1:0,

See the Drupal coding standards.

  'grant_view' => (!isset($done_grants['grant_view']) && $row->grant_view == 1) ? 1 : 0

          if (count($done_grants) === 3) {
            break;
          }

          if (count($done_grants) == 3) {
            break;
          }

What is the difference between the two code blocks?

          if (count($myterms) === 0) {
            break;
          }

  if ($account === NULL) {
    $account = $user;
  }

  if (!isset($account)) {
    $account = $user;
  }

  $result = db_query_range('SELECT 1 FROM {term_data} td
    WHERE td.vid = %d AND grant_admin(tid,%d) = 1',
    $vocabulary->vid,
    $account->uid, 0, 1);

  return (bool) db_fetch_array($result);

  return (boolean) db_result(db_query('SELECT COUNT(*) FROM {term_data} td
    WHERE td.vid = %d AND grant_admin(tid,%d) = 1',
    $vocabulary->vid,
    $account->uid));

Comment #19

superbaloo commented 5 August 2009 at 17:31

Status	File	Size
new	taxonomy_access_user.zip	18.12 KB
new	taxonomy_access_user.tar_.gz	11.87 KB

OK
OK
OK
OK
OK, thanks for the snippet
OK
OK
Yes ! ^^
OK
i don't see your point, === should be a little (very little in fact) faster than ==, but that's a detail ! :)
same ?
why ? $account would always be setted ... NULL or an instance of stdClass but setted, i can use is_null if you want but i just don't get your point. (It used in a function($account = NULL))
OK

Comment #20

superbaloo commented 5 August 2009 at 17:32

Status:

Needs work

» Needs review

forgot the need review

Comment #21

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 5 August 2009 at 18:42

10. i don't see your point, === should be a little (very little in fact) faster than ==, but that's a detail ! :)

Actually, === could not be faster; the strictly equal operator first verify that the two values are of the same type, and then verify that the two values are equal. In the code I reported, count() always return an integer, and there is no reason to strictly compare the returned value with 3.

12. why ? $account would always be setted ... NULL or an instance of stdClass but setted, i can use is_null if you want but i just don't get your point. (It used in a function($account = NULL))

Try to execute the following code:

  $var = NULL;
  $bool = isset($var);
  
  var_dump($bool);

The result will be a bool(false); it is also reported in the PHP documentation, which says

isset() will return FALSE if testing a variable that has been set to NULL.

Comment #22

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 5 August 2009 at 18:52

Status:

Needs review

» Needs work

The code still needs to be changed; some of the observations I made before are still valid.
In particular, the installation code must be executed only inside hook_install(), and hook_update_NN(), or from functions called from those hooks. Check the Drupal core code, or any third-party modules code; you will never see the implementation of hook_enable() what the implementation of that hook is doing in your module.

Comment #23

superbaloo commented 5 August 2009 at 19:34

Status:

Needs review

» Needs work

For the === vs == debate :

(21:25:25) [superbaloo @ baloo] [~/temps]
% cat test.php
<?php



$i = 10000;

$start = 0;
$end = 0;


$start = microtime();


while($i--) {
  $bool = ($i === 6666);
}


$end = microtime();

var_dump($end - $start);


$i = 10000;

$start = microtime();

while($i--) {
  $bool = ($i == 6666);
}


$end = microtime();

var_dump($end - $start);


(21:25:28) [superbaloo @ baloo] [~/temps]
% php test.php
float(0.00109)
float(0.00144)

(21:34:16) [superbaloo @ baloo] [~]
% php -v
PHP 5.2.10 with Suhosin-Patch 0.9.7 (cli) (built: Jul 14 2009 11:56:54) 
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.2.0, Copyright (c) 1998-2009 Zend Technologies
    with eAccelerator v0.9.5.3, Copyright (c) 2004-2006 eAccelerator, by eAccelerator

=== is a little bit faster ...

Ok for other modifications :) I will post the corrected code :)

Comment #24

superbaloo commented 5 August 2009 at 19:33

Status:

Needs work

» Needs review

Status	File	Size
new	taxonomy_access_user.zip	17.96 KB
new	taxonomy_access_user.tar_.gz	11.73 KB

Corrected code here :)

Comment #25

superbaloo commented 6 August 2009 at 08:26

Status:

Needs work

» Needs review

Status	File	Size
new	taxonomy_access_user.zip	17.96 KB
new	taxonomy_access_user.tar_.gz	11.7 KB

forgot a ), this version should be better :p

Comment #26

superbaloo commented 10 August 2009 at 07:43

is the code ready ?

Comment #27

superbaloo commented 12 August 2009 at 11:08

@KiamLaLuno : ping ?

Please, at least, respond to say "ok, i don't have the time to review this module right now" if you don't have the time ...

Comment #28

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 12 August 2009 at 11:24

@superbaloo: There are other people who can review your code.

Comment #29

superbaloo commented 12 August 2009 at 14:49

Status	File	Size
new	taxonomy_access_user.tgz	11.71 KB

minor corrections

Comment #30

AjK commented 14 August 2009 at 20:01

Status:	Needs review	» Needs work
Issue tags:		+Module review

                drupal_access_denied();
                die();

Modules shouldn't die. drupal_access_denied() is enough.

      watchdog('taxonomy access user', 'Wrong parameters passed to taxonomy_access_user_control() : $access argument required');

t() ?

Regarding ===, count() always returns an int and your constant is an int so I fail to see why a type check is needed.

  return (bool) db_result(db_query('SELECT COUNT(*) FROM {term_data} td
    WHERE td.vid = %d AND grant_admin(tid,%d) = 1',
    $vocabulary->vid,
    $account->uid, 0, 1));

Normally when querying the term_data table one would wrap the SQL in db_rewrite_sql(). For your module, being an access control module, you may have done this deliberately. However, as a reviewer, I'd like to see a comment as to why it isn't wrapped in db_rewrite_sql() to save me the bother of tracing the code path and so I understand what your code is doing. This is a common mistake but, as I said, in your case you may be not doing it for a specific reason.

Comment #31

superbaloo commented 24 August 2009 at 07:29

Status:

Needs work

» Needs review

Status	File	Size
new	taxonomy_access_user.zip	17.99 KB
new	taxonomy_access_user.tar_.gz	11.72 KB

Sorry for being late, i was off last week :)

Ok
Sorry forgot the t function
Yes, i agree, == should be enough and is right logically but using === seems to be a little bit faster than using == so i used to add a third = when it's possible :). Okay, if it's really a problem for you, i could consider removing the third =. (@see http://drupal.org/node/538850#comment-1892862)
Ok, i added the db_rewrite_sql, it could be usefull if the drupal is using more than one taxonomy based access control :)

Comment #32

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 29 August 2009 at 15:10

Status:

Needs review

» Needs work

When I try to set the permission of a taxonomy term, I get the following error messages:

notice: Undefined variable: tauid in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 89.
notice: Trying to get property of non-object in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 89.
notice: Undefined variable: tauid in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 97.
notice: Trying to get property of non-object in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 97.
notice: Trying to get property of non-object in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 104.
notice: Undefined property: stdClass::$grant_view in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 195.
notice: Undefined property: stdClass::$grant_update in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 195.
notice: Undefined property: stdClass::$grant_delete in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 195.
notice: Undefined property: stdClass::$grant_admin in /Applications/MAMP/htdocs/dr61/sites/all/modules/taxonomy_access_user/taxonomy_access_user.admin.inc on line 195.

The form that allows to set the access permissions uses a series of radios fields, when it could use checkboxes; the setting page would be more compact. The edit permission page could use a table where the columns are the permissions, and the rows are the users.
The page to change the permissions set for a user is not redirect to any page; it should redirect the user to the page that lists the permissions given to the taxonomy term (taxonomy/access/edit/term/<tid>).

Comment #33

superbaloo commented 4 September 2009 at 11:39

Status	File	Size
new	taxonomy-access-user.tar_.gz	13.08 KB

ok so:

fixed this :) shitty copy/paste
in fact i need radios because sometimes i have 3 options (not displayed in the root term as you may not inherit)
fixed
I also get back to an earlier version (before #30) because watchdog parameters may not be passed through the t() function. It's done after (as indicated in the documentation)