Implementation of hook_db_rewrite_sql

could you please upload this as a patch.

*.module files uploaded as issues are not downloadable.

Keve,

This is great stuff!

I received one little error after uploading, patching, & creating mySQL table (this ocurred upon clicking on administer >> modules):

Parse error: parse error, unexpected T_CONSTANT_ENCAPSED_STRING, expecting ')' in /home/xaccountx/public_html/xdirectoryx/modules/taxonomy_access/taxonomy_access.module on line 55

Editing code in the patched taxonomy_access.module to the following cleared it up:

function taxonomy_access_menu()  {
  $access = user_access('administer users');

  $items[] = array('path' => 'admin/access/category', 'title' => t('category permissions'),
    'callback' => 'taxonomy_access_admin',
	'access' => $access,
	'callback arguments' => array(),
    'type' => MENU_LOCAL_TASK);
  return $items;
}

(The problem was just a missing comma, I believe)

One more little gotcha... the 'Create', 'Update', and 'Delete' filters (per taxonomy vocab) seem to be working wonderfully! However, the 'View' filter doesn't seem to be doing anything. My test users who are not granted view privileges for a couple of terms under a particular vocab can still see the nodes associated with the terms, as can anonymous users (for whom I've also unchecked view privileges for those terms).

I can't code to save my life... but I'll gladly try to help gather a little more debugging info on this & update my post here if I find anything.

Thanks for all the work you're putting into this!

Ok, little more info...

Regarding the 'View' issue, I posted... I can elaborate now that, having looked over the mySQL tables, it appears the correct grant assignment values are going in (0's & 1's where they are supposed to be for the proper roles/categories)... so at least a db insertion issue can probably be ruled out.

and oops! New issue regarding 'Update' filter...

if a user who does not have update privileges for a particular term clicks 'edit' on one of their nodes (whether associated with that term or otherwise), the following error occurs:

warning: array_key_exists(): The second argument should be either an array or an object in /home/xaccountx/public_html/xdirectoryx/modules/taxonomy.module on line 493.

Ok, s'all I got... for the time being, anyway. (Can you tell I'm eager to put this mod to use?! *smile*)

thanks keve. this is indeed great stuff. this is even greater for me because i was about to start the same job, and tada here comes your patch :-)

after applying your patch to the latest cvs, and adding the comma mentionned by guckie (may be you can't code, but at least you can debug, that's a good start :-), i have the exact opposite behavior. view permissions are working fine, while the others are ignored (i did not flushed my test tables). at least, update/delete/create working for guckie, and view working for me gives a total of a fully functionnal module and a bug to find :-)

guckie, did you test on 4.6.3 or cvs head ?

small additional thing, the settings page does not work with current cvs (form array elements in _settings() hook should be called #type,#default_value,...). this is a small issue, and i can post a patch to fix this plus the missing comma if needed.

i'll continue testing and will post here if i find anything new. thanks again for the good work !

Hello, syllance & thanks for bringing your expertise to the table here

To answer your question, I am working with cvs-HEAD (as of about 6 days ago) -- I tried rolling out the new cvs-HEAD a couple of days ago, but things started breaking (you know how it can go sometimes)... so I rolled it back approximately 6 days (wish I could pinpoint that exactly).. at any rate, I'm backing up so I can try rolling out brand new cvs-HEAD again in a bit

Now.. notes/updates on the little boo-boos I've found (boo-boos is a technical word, isn't it? *laugh*)

1) Making yet another correction to something I stated earlier...

Regarding this:

if a user who does not have update privileges for a particular term clicks 'edit' on one of their nodes (whether associated with that term or otherwise), the following error occurs:

warning: array_key_exists(): The second argument should be either an array or an object in /home/xaccountx/public_html/xdirectoryx/modules/taxonomy.module on line 493.

The error message actually crops up no matter which user is attempting to edit (including administrator) & no matter which node (whether it is term-related or not). Also, I've additionally noted that when the error message appears, the theme goes haywire, HOWEVER it is actually possible for the user to successfully continue making the edit.

I've been looking at the line 493 error for awhile & getting nowhere with it. It seems to me that it has something to do with this function on line 555:
function _taxonomy_access_update_db($current_rids = NULL, $old_rids = NULL) {
(maybe the fact that $old_rids doesn't get defined as an array variable until line 609?) *shrug* I'll bet it's right under my nose. I'll also bet that one of you guys can spot it in no time.

2) Just a tiny quirk I encountered:

---- after removing update rights for a user for a particular term, received a 'page not found' message when administrator account tried accessing 'administer >> access control >> categories >> edit term' ----- (manually inserting 1's in term_access.update of the db cleared that problem up & administrator regained access to the category permissions admin interface) --- i reproduced this bug a second time just to be sure

Ok, will keep plugging away after a break.

Thanks again, you guys!

Keve,

My apologies. You are right.

Errors 1) & 2) in my post must have been occurring between my keyboard and my chair. After upgrading to the Oct 15 cvs-HEAD, I can't reproduce them either.

Also, the 'View' filter now applies as long as 'Promoted to front page' is not set for the term-restricted node.

Thanks again for this module!

keve,
Are you 'allowed' to check this module into CVS yet?
It would be great to get your latest changes (to the 4.6.3 version and this one, also) into CVS for the world to use (w/o patching).
-Steve

keve,
Please see this: http://drupal.org/cvs-account
and this: http://lists.drupal.org/archives/drupal-devel/2005-10/msg00092.html
That's all I know!
-Steve

As far as I know you don't need a CVS log-in for that. You make a patch for CVS, upload an issue to the right Component with version marked as CVS and Status set to Patch. But of cause it can also be overlooked that way.

forget my previous email. i'm running a fresh cvs with latest taxonomy_access patch, and everything is working fine.

simple tests were done, and i'll do some additionnal consistency checks with previous implementation, but i could not reproduced any problem i had. permissions are working as expected, if we except special behavior of "page not found" instead of "access denied".

thanks

I am so appreciative of this module, Keve! With current CVS versions, I'm experiencing no problems whatsoever with it! Nice job!

I am curious about a few things..

Like, Syllance, I would be interested in seeing an 'access denied' message rather than 'page not found' if that's feasible.

Also, how could we prevent RSS feeds from containing nodes of restricted terms? Would it be possible to incorporate a couple more operational functions and treat them as roles, possibly? I'm thinking 'feed' and 'upload' would be extremely useful permission options. (Ultimately, I think it would be wonderful to see term-based permissions included in Drupal core... but...) until such a thing happens... what are your opinions on including other term-related (and not role-based) permissions in the taxonomy-access-control module? Would it be workable?

my comment for "page not found" was not supposed to be an issue :-)

this is the behavior of the original implementation anyway (see http://drupal.org/node/23979). i understand why it's there and i'm fine with it as soon as the permissions are working, which is the case.

did not checked for RSS access, but i was about to do some more testing with initial implementation, so i'll double check.

goal for this patch is to move taxonomy_access implementation to _db_rewrite_sql hook. this is done, freeing taxonomy_access of any patch need, and it makes the module works with 4.7/head. i suggest discussing other matters on other threads, and let keve concentrate on getting this patch into CVS :-)

for evolution, i guess this might comes soon (http://drupal.org/node/33177).

thanks

Is there any chance of making this improved module work for 4.6.3? I am building a site that uses taxonomy heavily and I am desperately looking for a solution to limit access to specific terms. My client won't wait until 4.7, which seems at least a month away by conservative estimates.

after some additionnal testing, i don't experience any RSS access problem, and everything is still working fine. i'll have a more detailed look during the week end and will compare patch/patchless implementations on dedicated tests sandbox.

the patch is however broken with the very latest cvs. the 'category persmissions' tab does not work anymore (function form() not found). it still use the old form api, so i guess an upgrade to new api would help. unless someone else is working on it, i'll be glad to provide the upgrade in a couple days.

i'm wondering about the latest news and future of taxonomy_access. keve, are you now the official maintainer ? and are your patches scheduled to hit CVS ? or will there be some changes to them ?

thanks

I am also wondering... about the latest news and future of taxonomy_access. keve, are you now the official maintainer? and are your patches scheduled to hit CVS?

thanks for the update keve. i hope you'll soon have your cvs access, so your patches gets added.

except for potential bug fixes, is the current _rewrite_sql implementation the one that will make it in CVS ? or is there any scheduled changes/updates ?

congratulations for being the new taxonomy_access guru :-)

keve, I see that you have checked in some updates in CVS!! Great!

Just one question... the top of this thread says that it is no longer necessary to patch taxonomy.module. But I see that you checked in a new version of taxonomy.patch in HEAD. And the INSTALL.txt file in HEAD still says that the patch is necessary. Can you clear this up for me?

Thanks!

-Steve

Very good - thanks!