Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx -Status: active +Status: patch In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. chx Previous comments: ------------------------------------------------------------------------ January 31, 2005 - 00:19 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ January 31, 2005 - 03:41 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. -- View: http://drupal.org/node/16452 Edit: http://drupal.org/project/comments/add/16452

Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 chx Previous comments: ------------------------------------------------------------------------ January 31, 2005 - 00:19 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ January 31, 2005 - 03:41 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ January 31, 2005 - 07:49 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ January 31, 2005 - 14:22 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ February 1, 2005 - 09:09 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) -- View: http://drupal.org/node/16452 Edit: http://drupal.org/project/comments/add/16452

Issue status update for http://drupal.org/node/16452 Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch tangent, this is not that easy. If you rewrite taxonomy queries, you get a chance to make a hook_db_rewrite_sql implementation which fires on $primary_key=='tid'. The only current implementation being node_db_rewrite_sql fires on 'nid'. chx Previous comments: ------------------------------------------------------------------------ January 31, 2005 - 00:19 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ January 31, 2005 - 03:41 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ January 31, 2005 - 07:49 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ January 31, 2005 - 14:22 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ February 1, 2005 - 09:09 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) ------------------------------------------------------------------------ February 1, 2005 - 11:07 : chx Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 ------------------------------------------------------------------------ March 1, 2005 - 10:08 : tangent This patch has no effect on term/node access. 1. All roles have been given the "access content" permission. 2. I have removed the "all" grant from the node_access table. 3. I have the "Categories" block visible and it is showing the terms and number of nodes which are created. 4. Browsing as an anonymous user I can see the same terms and node count in the categories block that I see as admin. 5. Clicking on a term (/taxonomy/term/2) shows all nodes (and their content) associated with that term. For example, an image node is displayed as a thumbnail and a page node is displayed with the teaser. 6. Clicking on the node title (/node/2) displays a "access denied" message. Node access is restricted when accessed via node.module but not via taxonomy.module. Printing out the sql that is generated after the rewrite in taxonomy_term_page(), for example, I don't see any joins added so I guessed that a db_rewrite_sql hook was required for this to be added since the primary in your patch is "tid".

Issue status update for http://drupal.org/node/16452 Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch I have made some documentation on hook_db_rewrite_sql though it does not showed up yet. chx Previous comments: ------------------------------------------------------------------------ January 31, 2005 - 00:19 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ January 31, 2005 - 03:41 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ January 31, 2005 - 07:49 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ January 31, 2005 - 14:22 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ February 1, 2005 - 09:09 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) ------------------------------------------------------------------------ February 1, 2005 - 11:07 : chx Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 ------------------------------------------------------------------------ March 1, 2005 - 10:08 : tangent This patch has no effect on term/node access. 1. All roles have been given the "access content" permission. 2. I have removed the "all" grant from the node_access table. 3. I have the "Categories" block visible and it is showing the terms and number of nodes which are created. 4. Browsing as an anonymous user I can see the same terms and node count in the categories block that I see as admin. 5. Clicking on a term (/taxonomy/term/2) shows all nodes (and their content) associated with that term. For example, an image node is displayed as a thumbnail and a page node is displayed with the teaser. 6. Clicking on the node title (/node/2) displays a "access denied" message. Node access is restricted when accessed via node.module but not via taxonomy.module. Printing out the sql that is generated after the rewrite in taxonomy_term_page(), for example, I don't see any joins added so I guessed that a db_rewrite_sql hook was required for this to be added since the primary in your patch is "tid". ------------------------------------------------------------------------ March 1, 2005 - 10:14 : chx tangent, this is not that easy. If you rewrite taxonomy queries, you get a chance to make a hook_db_rewrite_sql implementation which fires on $primary_key=='tid'. The only current implementation being node_db_rewrite_sql fires on 'nid'. ------------------------------------------------------------------------ March 1, 2005 - 10:20 : tangent I don't understand if you are saying that this is hard to implement or should be handled in another module. Are you saying that taxonomy_db_rewrite_sql() is required and needs to fire for 'tid' in order to restrict access as I thought?

Issue status update for http://drupal.org/node/16452 Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: tangent Status: patch +1 for me too. The access functionality that this patch enabled is essential in my opinion and should be in 4.6. Without it, the currently implemented node access functionality is worthless. I think the final patch should include a db_rewrite_sql hook to complete the functionality though. tangent Previous comments: ------------------------------------------------------------------------ January 30, 2005 - 18:19 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ January 30, 2005 - 21:41 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ January 31, 2005 - 01:49 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ January 31, 2005 - 08:22 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ February 1, 2005 - 03:09 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) ------------------------------------------------------------------------ February 1, 2005 - 05:07 : chx Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 ------------------------------------------------------------------------ March 1, 2005 - 04:08 : tangent This patch has no effect on term/node access. 1. All roles have been given the "access content" permission. 2. I have removed the "all" grant from the node_access table. 3. I have the "Categories" block visible and it is showing the terms and number of nodes which are created. 4. Browsing as an anonymous user I can see the same terms and node count in the categories block that I see as admin. 5. Clicking on a term (/taxonomy/term/2) shows all nodes (and their content) associated with that term. For example, an image node is displayed as a thumbnail and a page node is displayed with the teaser. 6. Clicking on the node title (/node/2) displays a "access denied" message. Node access is restricted when accessed via node.module but not via taxonomy.module. Printing out the sql that is generated after the rewrite in taxonomy_term_page(), for example, I don't see any joins added so I guessed that a db_rewrite_sql hook was required for this to be added since the primary in your patch is "tid". ------------------------------------------------------------------------ March 1, 2005 - 04:14 : chx tangent, this is not that easy. If you rewrite taxonomy queries, you get a chance to make a hook_db_rewrite_sql implementation which fires on $primary_key=='tid'. The only current implementation being node_db_rewrite_sql fires on 'nid'. ------------------------------------------------------------------------ March 1, 2005 - 04:20 : tangent I don't understand if you are saying that this is hard to implement or should be handled in another module. Are you saying that taxonomy_db_rewrite_sql() is required and needs to fire for 'tid' in order to restrict access as I thought? ------------------------------------------------------------------------ March 1, 2005 - 06:55 : chx I have made some documentation on hook_db_rewrite_sql though it does not showed up yet. ------------------------------------------------------------------------ March 1, 2005 - 14:06 : moshe weitzman I'd like to see this patch in core. By running taxonomy queries through db_rewrite_sql() we can achieve two key enhancements: - access modules can filter out terms to which the current user has no access. this would complete our implemetnation of private forums. jose mentioned this earlier. - taxonomy node listings and feeds could be filtered by node type using an upcoming simple module which recognizes '&type=event,story' style querystrings as a filter. this will work on taxo lists, home page list, tracker, etc. there are likely more uses.

Issue status update for http://drupal.org/node/16452 Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: stefan nagtegaal Status: patch Is this considered for 4.6 or 4.7?? I like it! stefan nagtegaal Previous comments: ------------------------------------------------------------------------ January 31, 2005 - 00:19 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ January 31, 2005 - 03:41 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ January 31, 2005 - 07:49 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ January 31, 2005 - 14:22 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ February 1, 2005 - 09:09 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) ------------------------------------------------------------------------ February 1, 2005 - 11:07 : chx Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 ------------------------------------------------------------------------ March 1, 2005 - 10:08 : tangent This patch has no effect on term/node access. 1. All roles have been given the "access content" permission. 2. I have removed the "all" grant from the node_access table. 3. I have the "Categories" block visible and it is showing the terms and number of nodes which are created. 4. Browsing as an anonymous user I can see the same terms and node count in the categories block that I see as admin. 5. Clicking on a term (/taxonomy/term/2) shows all nodes (and their content) associated with that term. For example, an image node is displayed as a thumbnail and a page node is displayed with the teaser. 6. Clicking on the node title (/node/2) displays a "access denied" message. Node access is restricted when accessed via node.module but not via taxonomy.module. Printing out the sql that is generated after the rewrite in taxonomy_term_page(), for example, I don't see any joins added so I guessed that a db_rewrite_sql hook was required for this to be added since the primary in your patch is "tid". ------------------------------------------------------------------------ March 1, 2005 - 10:14 : chx tangent, this is not that easy. If you rewrite taxonomy queries, you get a chance to make a hook_db_rewrite_sql implementation which fires on $primary_key=='tid'. The only current implementation being node_db_rewrite_sql fires on 'nid'. ------------------------------------------------------------------------ March 1, 2005 - 10:20 : tangent I don't understand if you are saying that this is hard to implement or should be handled in another module. Are you saying that taxonomy_db_rewrite_sql() is required and needs to fire for 'tid' in order to restrict access as I thought? ------------------------------------------------------------------------ March 1, 2005 - 12:55 : chx I have made some documentation on hook_db_rewrite_sql though it does not showed up yet. ------------------------------------------------------------------------ March 1, 2005 - 20:06 : moshe weitzman I'd like to see this patch in core. By running taxonomy queries through db_rewrite_sql() we can achieve two key enhancements: - access modules can filter out terms to which the current user has no access. this would complete our implemetnation of private forums. jose mentioned this earlier. - taxonomy node listings and feeds could be filtered by node type using an upcoming simple module which recognizes '&type=event,story' style querystrings as a filter. this will work on taxo lists, home page list, tracker, etc. there are likely more uses. ------------------------------------------------------------------------ March 1, 2005 - 20:16 : tangent +1 for me too. The access functionality that this patch enabled is essential in my opinion and should be in 4.6. Without it, the currently implemented node access functionality is worthless. I think the final patch should include a db_rewrite_sql hook to complete the functionality though. ------------------------------------------------------------------------ March 5, 2005 - 17:43 : Bèr Kessels +1 from me too. But I would like to see some benchmarking with a database with loads of terms. If someone volunteers I have a DB with approx 300 terms and the same amount of nodes in them. Just give me a private call (berkessels curlythingy gmx dot net) and I might forward it (if i know you ;).

Issue status update for http://drupal.org/node/16452 Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/mini_taxo_rewrite.patch (1.41 KB) This time I think I'd go with small steps. This patch is -- I think -- might be enough for tid rewrites. chx Previous comments: ------------------------------------------------------------------------ January 31, 2005 - 00:19 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ January 31, 2005 - 03:41 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ January 31, 2005 - 07:49 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ January 31, 2005 - 14:22 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ February 1, 2005 - 09:09 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) ------------------------------------------------------------------------ February 1, 2005 - 11:07 : chx Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 ------------------------------------------------------------------------ March 1, 2005 - 10:08 : tangent This patch has no effect on term/node access. 1. All roles have been given the "access content" permission. 2. I have removed the "all" grant from the node_access table. 3. I have the "Categories" block visible and it is showing the terms and number of nodes which are created. 4. Browsing as an anonymous user I can see the same terms and node count in the categories block that I see as admin. 5. Clicking on a term (/taxonomy/term/2) shows all nodes (and their content) associated with that term. For example, an image node is displayed as a thumbnail and a page node is displayed with the teaser. 6. Clicking on the node title (/node/2) displays a "access denied" message. Node access is restricted when accessed via node.module but not via taxonomy.module. Printing out the sql that is generated after the rewrite in taxonomy_term_page(), for example, I don't see any joins added so I guessed that a db_rewrite_sql hook was required for this to be added since the primary in your patch is "tid". ------------------------------------------------------------------------ March 1, 2005 - 10:14 : chx tangent, this is not that easy. If you rewrite taxonomy queries, you get a chance to make a hook_db_rewrite_sql implementation which fires on $primary_key=='tid'. The only current implementation being node_db_rewrite_sql fires on 'nid'. ------------------------------------------------------------------------ March 1, 2005 - 10:20 : tangent I don't understand if you are saying that this is hard to implement or should be handled in another module. Are you saying that taxonomy_db_rewrite_sql() is required and needs to fire for 'tid' in order to restrict access as I thought? ------------------------------------------------------------------------ March 1, 2005 - 12:55 : chx I have made some documentation on hook_db_rewrite_sql though it does not showed up yet. ------------------------------------------------------------------------ March 1, 2005 - 20:06 : moshe weitzman I'd like to see this patch in core. By running taxonomy queries through db_rewrite_sql() we can achieve two key enhancements: - access modules can filter out terms to which the current user has no access. this would complete our implemetnation of private forums. jose mentioned this earlier. - taxonomy node listings and feeds could be filtered by node type using an upcoming simple module which recognizes '&type=event,story' style querystrings as a filter. this will work on taxo lists, home page list, tracker, etc. there are likely more uses. ------------------------------------------------------------------------ March 1, 2005 - 20:16 : tangent +1 for me too. The access functionality that this patch enabled is essential in my opinion and should be in 4.6. Without it, the currently implemented node access functionality is worthless. I think the final patch should include a db_rewrite_sql hook to complete the functionality though. ------------------------------------------------------------------------ March 5, 2005 - 17:43 : Bèr Kessels +1 from me too. But I would like to see some benchmarking with a database with loads of terms. If someone volunteers I have a DB with approx 300 terms and the same amount of nodes in them. Just give me a private call (berkessels curlythingy gmx dot net) and I might forward it (if i know you ;). ------------------------------------------------------------------------ April 3, 2005 - 20:54 : stefan nagtegaal Is this considered for 4.6 or 4.7?? I like it! ------------------------------------------------------------------------ April 3, 2005 - 21:02 : chx Do not commit this patch, it is buggy, term_page needs a few more lines (see the privateterms taxonomy patch in my sandbox for code). Will reroll soon.

Issue status update for http://drupal.org/node/16452 Post a follow up: http://drupal.org/project/comments/add/16452 Project: Drupal -Version: 4.6.0 +Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx -Status: active +Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/tid_vid_rewrite.patch (4.31 KB) This is long overdue. chx Previous comments: ------------------------------------------------------------------------ Sun, 30 Jan 2005 23:19:27 +0000 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ Mon, 31 Jan 2005 02:41:46 +0000 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ Mon, 31 Jan 2005 06:49:02 +0000 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ Mon, 31 Jan 2005 13:22:47 +0000 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ Tue, 01 Feb 2005 08:09:25 +0000 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) ------------------------------------------------------------------------ Tue, 01 Feb 2005 10:07:16 +0000 : chx Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:08:30 +0000 : tangent This patch has no effect on term/node access. 1. All roles have been given the "access content" permission. 2. I have removed the "all" grant from the node_access table. 3. I have the "Categories" block visible and it is showing the terms and number of nodes which are created. 4. Browsing as an anonymous user I can see the same terms and node count in the categories block that I see as admin. 5. Clicking on a term (/taxonomy/term/2) shows all nodes (and their content) associated with that term. For example, an image node is displayed as a thumbnail and a page node is displayed with the teaser. 6. Clicking on the node title (/node/2) displays a "access denied" message. Node access is restricted when accessed via node.module but not via taxonomy.module. Printing out the sql that is generated after the rewrite in taxonomy_term_page(), for example, I don't see any joins added so I guessed that a db_rewrite_sql hook was required for this to be added since the primary in your patch is "tid". ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:14:25 +0000 : chx tangent, this is not that easy. If you rewrite taxonomy queries, you get a chance to make a hook_db_rewrite_sql implementation which fires on $primary_key=='tid'. The only current implementation being node_db_rewrite_sql fires on 'nid'. ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:20:28 +0000 : tangent I don't understand if you are saying that this is hard to implement or should be handled in another module. Are you saying that taxonomy_db_rewrite_sql() is required and needs to fire for 'tid' in order to restrict access as I thought? ------------------------------------------------------------------------ Tue, 01 Mar 2005 11:55:53 +0000 : chx I have made some documentation on hook_db_rewrite_sql though it does not showed up yet. ------------------------------------------------------------------------ Tue, 01 Mar 2005 19:06:25 +0000 : moshe weitzman I'd like to see this patch in core. By running taxonomy queries through db_rewrite_sql() we can achieve two key enhancements: - access modules can filter out terms to which the current user has no access. this would complete our implemetnation of private forums. jose mentioned this earlier. - taxonomy node listings and feeds could be filtered by node type using an upcoming simple module which recognizes '&type=event,story' style querystrings as a filter. this will work on taxo lists, home page list, tracker, etc. there are likely more uses. ------------------------------------------------------------------------ Tue, 01 Mar 2005 19:16:54 +0000 : tangent +1 for me too. The access functionality that this patch enabled is essential in my opinion and should be in 4.6. Without it, the currently implemented node access functionality is worthless. I think the final patch should include a db_rewrite_sql hook to complete the functionality though. ------------------------------------------------------------------------ Sat, 05 Mar 2005 16:43:10 +0000 : Bèr Kessels +1 from me too. But I would like to see some benchmarking with a database with loads of terms. If someone volunteers I have a DB with approx 300 terms and the same amount of nodes in them. Just give me a private call (berkessels curlythingy gmx dot net) and I might forward it (if i know you ;). ------------------------------------------------------------------------ Sun, 03 Apr 2005 19:54:24 +0000 : stefan nagtegaal Is this considered for 4.6 or 4.7?? I like it! ------------------------------------------------------------------------ Sun, 03 Apr 2005 20:02:40 +0000 : chx Do not commit this patch, it is buggy, term_page needs a few more lines (see the privateterms taxonomy patch in my sandbox for code). Will reroll soon. ------------------------------------------------------------------------ Thu, 28 Apr 2005 21:19:58 +0000 : chx Attachment: http://drupal.org/files/issues/mini_taxo_rewrite.patch (1.41 KB) This time I think I'd go with small steps. This patch is -- I think -- might be enough for tid rewrites. ------------------------------------------------------------------------ Sat, 07 May 2005 18:19:48 +0000 : moshe weitzman this simple change should help us fix the remaining issue with private forums; the dropdown where you pick which forum to post into still shows inaccessible forums. ------------------------------------------------------------------------ Sat, 21 May 2005 21:33:12 +0000 : Dries Committed to HEAD. ------------------------------------------------------------------------ Sat, 20 Aug 2005 12:01:23 +0000 : Jose A Reyero Attachment: http://drupal.org/files/issues/taxonomy_query_rewrite_plus.patch (4.04 KB) I think there are a few more queries that should be rewritten. Patch updated for HEAD.

Issue status update for http://drupal.org/node/16452 Post a follow up: http://drupal.org/project/comments/add/16452 Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch (ready to be committed) This is assigned to me, so I'll be the one who answers: if we want to a proper taxonomy access rights module -- taxonomy_access is popular -- then these are needed. The small was enough for forums but not full taxonomy. chx Previous comments: ------------------------------------------------------------------------ Sun, 30 Jan 2005 23:19:27 +0000 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ Mon, 31 Jan 2005 02:41:46 +0000 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ Mon, 31 Jan 2005 06:49:02 +0000 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ Mon, 31 Jan 2005 13:22:47 +0000 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ Tue, 01 Feb 2005 08:09:25 +0000 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) ------------------------------------------------------------------------ Tue, 01 Feb 2005 10:07:16 +0000 : chx Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:08:30 +0000 : tangent This patch has no effect on term/node access. 1. All roles have been given the "access content" permission. 2. I have removed the "all" grant from the node_access table. 3. I have the "Categories" block visible and it is showing the terms and number of nodes which are created. 4. Browsing as an anonymous user I can see the same terms and node count in the categories block that I see as admin. 5. Clicking on a term (/taxonomy/term/2) shows all nodes (and their content) associated with that term. For example, an image node is displayed as a thumbnail and a page node is displayed with the teaser. 6. Clicking on the node title (/node/2) displays a "access denied" message. Node access is restricted when accessed via node.module but not via taxonomy.module. Printing out the sql that is generated after the rewrite in taxonomy_term_page(), for example, I don't see any joins added so I guessed that a db_rewrite_sql hook was required for this to be added since the primary in your patch is "tid". ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:14:25 +0000 : chx tangent, this is not that easy. If you rewrite taxonomy queries, you get a chance to make a hook_db_rewrite_sql implementation which fires on $primary_key=='tid'. The only current implementation being node_db_rewrite_sql fires on 'nid'. ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:20:28 +0000 : tangent I don't understand if you are saying that this is hard to implement or should be handled in another module. Are you saying that taxonomy_db_rewrite_sql() is required and needs to fire for 'tid' in order to restrict access as I thought? ------------------------------------------------------------------------ Tue, 01 Mar 2005 11:55:53 +0000 : chx I have made some documentation on hook_db_rewrite_sql though it does not showed up yet. ------------------------------------------------------------------------ Tue, 01 Mar 2005 19:06:25 +0000 : moshe weitzman I'd like to see this patch in core. By running taxonomy queries through db_rewrite_sql() we can achieve two key enhancements: - access modules can filter out terms to which the current user has no access. this would complete our implemetnation of private forums. jose mentioned this earlier. - taxonomy node listings and feeds could be filtered by node type using an upcoming simple module which recognizes '&type=event,story' style querystrings as a filter. this will work on taxo lists, home page list, tracker, etc. there are likely more uses. ------------------------------------------------------------------------ Tue, 01 Mar 2005 19:16:54 +0000 : tangent +1 for me too. The access functionality that this patch enabled is essential in my opinion and should be in 4.6. Without it, the currently implemented node access functionality is worthless. I think the final patch should include a db_rewrite_sql hook to complete the functionality though. ------------------------------------------------------------------------ Sat, 05 Mar 2005 16:43:10 +0000 : Bèr Kessels +1 from me too. But I would like to see some benchmarking with a database with loads of terms. If someone volunteers I have a DB with approx 300 terms and the same amount of nodes in them. Just give me a private call (berkessels curlythingy gmx dot net) and I might forward it (if i know you ;). ------------------------------------------------------------------------ Sun, 03 Apr 2005 19:54:24 +0000 : stefan nagtegaal Is this considered for 4.6 or 4.7?? I like it! ------------------------------------------------------------------------ Sun, 03 Apr 2005 20:02:40 +0000 : chx Do not commit this patch, it is buggy, term_page needs a few more lines (see the privateterms taxonomy patch in my sandbox for code). Will reroll soon. ------------------------------------------------------------------------ Thu, 28 Apr 2005 21:19:58 +0000 : chx Attachment: http://drupal.org/files/issues/mini_taxo_rewrite.patch (1.41 KB) This time I think I'd go with small steps. This patch is -- I think -- might be enough for tid rewrites. ------------------------------------------------------------------------ Sat, 07 May 2005 18:19:48 +0000 : moshe weitzman this simple change should help us fix the remaining issue with private forums; the dropdown where you pick which forum to post into still shows inaccessible forums. ------------------------------------------------------------------------ Sat, 21 May 2005 21:33:12 +0000 : Dries Committed to HEAD. ------------------------------------------------------------------------ Sat, 20 Aug 2005 12:01:23 +0000 : Jose A Reyero Attachment: http://drupal.org/files/issues/taxonomy_query_rewrite_plus.patch (4.04 KB) I think there are a few more queries that should be rewritten. Patch updated for HEAD. ------------------------------------------------------------------------ Sat, 20 Aug 2005 20:59:51 +0000 : chx Attachment: http://drupal.org/files/issues/tid_vid_rewrite.patch (4.31 KB) This is long overdue. ------------------------------------------------------------------------ Sat, 20 Aug 2005 21:05:18 +0000 : moshe weitzman jose - it would be nice if we had an example of the new rewrites might be used.

Issue status update for http://drupal.org/node/16452 Post a follow up: http://drupal.org/project/comments/add/16452 Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests -Priority: normal +Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (ready to be committed) I asked moshe aboutwhat's a 'critical feature request' and he says: "i guess if it a feature that many people want or that will enable developers to build many features that people want". To quote a well-respected developer "From what I have seen, a taxonomy-based permission scheme is more popular than one using node types" in http://lists.drupal.org/archives/drupal-devel/2005-04/msg00490.html so I think the second half of the above sentence is in effect. chx Previous comments: ------------------------------------------------------------------------ Sun, 30 Jan 2005 23:19:27 +0000 : chx Attachment: http://drupal.org/files/issues/taxonomy_db_rewrite.patch (6.71 KB) I'd like to present a use for db_rewrite_sql: taxonomy rewrites. If this goes through, it will be possible to create an access mechanism, languages and whatever for taxonomy. This is a very small change, I think, much smaller than the node_rewrite_sql megpatch was 'cos at the moment nothing changes. There are no DISTINCTs deleted. But this opens a door for future things. Also, this only affects taxonomy.module, 'cos no other module deals with term_data and vocabulary tables (well, forum.module does, but those queries does not need a rewrite). ------------------------------------------------------------------------ Mon, 31 Jan 2005 02:41:46 +0000 : moshe weitzman please, let's identify some use cases before we run more queries through the regex and hooks implied by db_qeury_sql. There might be a good reason to do this, but you have to convince us. ------------------------------------------------------------------------ Mon, 31 Jan 2005 06:49:02 +0000 : chx In my latest version of db_rewrite_sql, which Dries has not yet commited, regexs are not executed if not necessary. That version was created well before Moshe's post, and I'm pretty sure it will pass, 'cos it is not a feature but a bugfix patch actually. Until there are no implementations of hook_db_rewrite_sql which cares for primary_field=='vid' or 'tid' , the performance hit is minimal, no regexps. ------------------------------------------------------------------------ Mon, 31 Jan 2005 13:22:47 +0000 : Jose A Reyero Use cases: - Permissions for taxonomy terms/vocabs, some tables and conditions must be joined for taxonomy queries - Multilanguage pages/taxonomy (i18n), additional conditions needed also for taxonomy ------------------------------------------------------------------------ Tue, 01 Feb 2005 08:09:25 +0000 : robertDouglass +1 for being able to put user/role based permissions on vocabularies. I see this as extremely important (can we do menus too?) ------------------------------------------------------------------------ Tue, 01 Feb 2005 10:07:16 +0000 : chx Yes, we can do menus [1] too. Good idea. [1] http://drupal.org/node/16542 ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:08:30 +0000 : tangent This patch has no effect on term/node access. 1. All roles have been given the "access content" permission. 2. I have removed the "all" grant from the node_access table. 3. I have the "Categories" block visible and it is showing the terms and number of nodes which are created. 4. Browsing as an anonymous user I can see the same terms and node count in the categories block that I see as admin. 5. Clicking on a term (/taxonomy/term/2) shows all nodes (and their content) associated with that term. For example, an image node is displayed as a thumbnail and a page node is displayed with the teaser. 6. Clicking on the node title (/node/2) displays a "access denied" message. Node access is restricted when accessed via node.module but not via taxonomy.module. Printing out the sql that is generated after the rewrite in taxonomy_term_page(), for example, I don't see any joins added so I guessed that a db_rewrite_sql hook was required for this to be added since the primary in your patch is "tid". ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:14:25 +0000 : chx tangent, this is not that easy. If you rewrite taxonomy queries, you get a chance to make a hook_db_rewrite_sql implementation which fires on $primary_key=='tid'. The only current implementation being node_db_rewrite_sql fires on 'nid'. ------------------------------------------------------------------------ Tue, 01 Mar 2005 09:20:28 +0000 : tangent I don't understand if you are saying that this is hard to implement or should be handled in another module. Are you saying that taxonomy_db_rewrite_sql() is required and needs to fire for 'tid' in order to restrict access as I thought? ------------------------------------------------------------------------ Tue, 01 Mar 2005 11:55:53 +0000 : chx I have made some documentation on hook_db_rewrite_sql though it does not showed up yet. ------------------------------------------------------------------------ Tue, 01 Mar 2005 19:06:25 +0000 : moshe weitzman I'd like to see this patch in core. By running taxonomy queries through db_rewrite_sql() we can achieve two key enhancements: - access modules can filter out terms to which the current user has no access. this would complete our implemetnation of private forums. jose mentioned this earlier. - taxonomy node listings and feeds could be filtered by node type using an upcoming simple module which recognizes '&type=event,story' style querystrings as a filter. this will work on taxo lists, home page list, tracker, etc. there are likely more uses. ------------------------------------------------------------------------ Tue, 01 Mar 2005 19:16:54 +0000 : tangent +1 for me too. The access functionality that this patch enabled is essential in my opinion and should be in 4.6. Without it, the currently implemented node access functionality is worthless. I think the final patch should include a db_rewrite_sql hook to complete the functionality though. ------------------------------------------------------------------------ Sat, 05 Mar 2005 16:43:10 +0000 : Bèr Kessels +1 from me too. But I would like to see some benchmarking with a database with loads of terms. If someone volunteers I have a DB with approx 300 terms and the same amount of nodes in them. Just give me a private call (berkessels curlythingy gmx dot net) and I might forward it (if i know you ;). ------------------------------------------------------------------------ Sun, 03 Apr 2005 19:54:24 +0000 : stefan nagtegaal Is this considered for 4.6 or 4.7?? I like it! ------------------------------------------------------------------------ Sun, 03 Apr 2005 20:02:40 +0000 : chx Do not commit this patch, it is buggy, term_page needs a few more lines (see the privateterms taxonomy patch in my sandbox for code). Will reroll soon. ------------------------------------------------------------------------ Thu, 28 Apr 2005 21:19:58 +0000 : chx Attachment: http://drupal.org/files/issues/mini_taxo_rewrite.patch (1.41 KB) This time I think I'd go with small steps. This patch is -- I think -- might be enough for tid rewrites. ------------------------------------------------------------------------ Sat, 07 May 2005 18:19:48 +0000 : moshe weitzman this simple change should help us fix the remaining issue with private forums; the dropdown where you pick which forum to post into still shows inaccessible forums. ------------------------------------------------------------------------ Sat, 21 May 2005 21:33:12 +0000 : Dries Committed to HEAD. ------------------------------------------------------------------------ Sat, 20 Aug 2005 12:01:23 +0000 : Jose A Reyero Attachment: http://drupal.org/files/issues/taxonomy_query_rewrite_plus.patch (4.04 KB) I think there are a few more queries that should be rewritten. Patch updated for HEAD. ------------------------------------------------------------------------ Sat, 20 Aug 2005 20:59:51 +0000 : chx Attachment: http://drupal.org/files/issues/tid_vid_rewrite.patch (4.31 KB) This is long overdue. ------------------------------------------------------------------------ Sat, 20 Aug 2005 21:05:18 +0000 : moshe weitzman jose - it would be nice if we had an example of the new rewrites might be used. ------------------------------------------------------------------------ Sat, 20 Aug 2005 21:11:01 +0000 : chx This is assigned to me, so I'll be the one who answers: if we want to a proper taxonomy access rights module -- taxonomy_access is popular -- then these are needed. The small was enough for forums but not full taxonomy. ------------------------------------------------------------------------ Sat, 20 Aug 2005 21:34:09 +0000 : Jose A Reyero Hi, This is already used by i18n module, and this patch was included as a part of v4.6 of the module. Also, I've just updated i18n-HEAD and you can give it a try.