- Development - lists.drupal.org

[drupal-devel] [feature] "blog it" link lost in aggregator
by Prometheus6 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/10752 Post a follow up: http://drupal.org/project/comments/add/10752 Project: Drupal Version: 4.6.2 Component: aggregator.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Gunnar Langemark(a)www.langemark.com Updated by: Prometheus6 Status: patch (code needs review) Would adding the blog it link to the aggregator pages instead of the block be considered a new feature? I'm told new features are only added to CVS [1]. And how does one find out who maintains the aggregator module? I'm building a couple of modules that sit on top of the aggregator and have done a couple of things that would be generally useful to aggregator.module itself...like extending "blog it" to the node type of your choice and configuring which HTML tags are not stripped from the feed [1] http://drupal.org/node/27736#comment-37111 Prometheus6 Previous comments: ------------------------------------------------------------------------ Sun, 12 Sep 2004 07:51:48 +0000 : Gunnar Langemark(a)www.langemark.com I upgraded Langemarks Cafe and noticed that the "blog it" link from aggregator posts is no more. So I did a clean install of cvs - and it is still not there. I use it every day as it is an integrated part of my site maintenance routine. So for me it is critical - even though Drupal runs fine without. Sure hope this is a bug and not something which has been removed. ------------------------------------------------------------------------ Sun, 12 Sep 2004 23:44:20 +0000 : drumm I am on my slow computer and haven't done a full test (subscribe to feeds and confirm it works), but the permisison name recently changed from maintain blog to edit own blog. Make sure you have those all selected properly in admin >> users >> configure >> roles. Setting as a support request for now as I could not find any inconsistencies in the code yet. ------------------------------------------------------------------------ Fri, 18 Feb 2005 02:54:36 +0000 : tatonca Attachment: http://drupal.org/files/issues/aggregator.module_5.patch (1.11 KB) I hope this is the way to do this... There is a thread discussing this issue: http://drupal.org/node/10740 and I think Dries confirms that this was changed by design. I myself liked having the feature, so I added it back in on my site. Patch attached - hope folks find this useful... I thought it might be kewl to make this an option to be turned on or off in the aggregator admin settings - haven't got as far as figuring out how to do that yet ;) when I do I'll post that too... Since this was originally removed by design I've changed the status to feature request. I also tried to find another thread or issue but this seemed to be the only one... ~Tat~ ------------------------------------------------------------------------ Sat, 26 Mar 2005 07:14:04 +0000 : wal >From what I found, you have to be the original admin account to get the "blog it" icon. I even tried making an "admin" role and giving it all the permissions. The only way I see the icon is if I am logged in as the original admin account. Giving all the permissions to the "authenticated user" role didn't seem to be enough. I simply changed the original account to be my username. I know this isn't a real fix but since I am the only person using that specific install it fixes my issue. Good luck, --Bill ------------------------------------------------------------------------ Sat, 09 Apr 2005 17:22:49 +0000 : bloggator This worked for me: In your aggregator.module, replace everything from line 1111 to the end with: $output .= "<div class=\"news-item\">\n"; $output .= ' <div class="date">'. date('H:i', $item->timestamp) ."</div>\n"; $output .= " <div class=\"body\">\n"; $output .= " <div class=\"title\"><a href=\"$item->link\">$item->title</a> "; if ($user->uid && module_exist('blog') && user_access('edit own blog')) { if ($image = theme('image', 'misc/blog.png', t('blog it'), t('blog it'))) { $output .= '<span class="icon">'. l($image, 'node/add/blog', array('title' => t('Comment on this news item in your personal blog.'), 'class' => 'blog-it'), "iid=$item->iid") .'</span>'; $output .="</div>\n"; } } if ($item->description) { $output .= " <div class=\"description\">$item->description</div>\n"; } if ($item->ftitle && $item->fid) { $output .= " <div class=\"description\"> </div>\n"; $output .= ' <div class="source">'. t('Source') .': '. l($item->ftitle, "aggregator/sources/$item->fid") ."</div>\n"; } $result = db_query('SELECT c.title, c.cid FROM {aggregator_category_item} ci LEFT JOIN {aggregator_category} c ON ci.cid = c.cid WHERE ci.iid = %d ORDER BY c.title', $item->iid); $categories = array(); while ($category = db_fetch_object($result)) { $categories[] = l($category->title, 'aggregator/categories/'. $category->cid); } $output .= " </div>\n"; $output .= "</div>\n"; return $output; } ?> Best regards, Jeff ------------------------------------------------------------------------ Tue, 26 Apr 2005 22:58:57 +0000 : maui1 Hi Jeff, I tried your code patch for the aggregator module in 4.6 and it didn't get the blogIt and Post as Story links back. It did seem to slow access to aggregator down, but no links. Where was this design decision made, I would like to contribute my reasons for needing this functionality. ------------------------------------------------------------------------ Tue, 05 Jul 2005 04:11:13 +0000 : johnpg It's missing the $user declaration so it never gets past the "if" statements. Edit the theme_aggregator_page_item function so it looks like this. function theme_aggregator_page_item($item) { static $last; global $user; Then add if ($user->uid && module_exist('blog') && user_access('edit own blog')) { if ($image = theme('image', 'misc/blog.png', t('blog it'), t('blog it'))) { $output .= ''. l($image, 'node/add/blog', array('title' => t('Comment on this news item in your personal blog.'), 'class' => 'blog-it'), "iid=$item->iid", NULL, FALSE, TRUE) .''; } } Directly after this line: $output .= ' '. check_plain($item->title) .\" [2]\n"; This should really be back in the code as standard, very useful. John [2] http://drupal.org/'. check_url($item->link) .' ------------------------------------------------------------------------ Tue, 05 Jul 2005 04:12:50 +0000 : johnpg It's missing the $user declaration so it never gets past the "if" statements. Edit the theme_aggregator_page_item function so it looks like this. function theme_aggregator_page_item($item) { static $last; global $user; Then add if ($user->uid && module_exist('blog') && user_access('edit own blog')) { if ($image = theme('image', 'misc/blog.png', t('blog it'), t('blog it'))) { $output .= '<div class="icon">'. l($image, 'node/add/blog', array('title' => t('Comment on this news item in your personal blog.'), 'class' => 'blog-it'), "iid=$item->iid", NULL, FALSE, TRUE) .'</div>'; } } Directly after this line: $output .= ' <div class="title">'. check_plain($item->title) .\" [3]</div>\n"; This should really be back in the code as standard, very useful. John [3] http://drupal.org/'. check_url($item->link) .' ------------------------------------------------------------------------ Fri, 05 Aug 2005 17:06:24 +0000 : peacearena I get this working but with a strange problem. In the aggregator contents after the title, where the blog.png image should be, the actual code to the image file shows: <img src="misc/blog.png" alt="blog it" title="blog it" /> as a link. Yes the image exists, yes img span and div are allowed in that format. What am I doing wrong? I tried changing the path to the image, making it absolute, putting the image in the theme directory, but then nothing shows at all after the title. I hope someone can help me with this, I'd really like this feature. Using Drupal 4.6.2. Thanks. ------------------------------------------------------------------------ Sat, 06 Aug 2005 08:00:42 +0000 : Boris Mann Why the hell does this keep getting taken out?! Many people use it, it shouldn't be removed. If some developers somewhere don't like it, then add an admin setting to toggle this on or off. It really should be added to both 4.6 and 4.7. ------------------------------------------------------------------------ Sat, 06 Aug 2005 10:27:44 +0000 : Prometheus6 The link exists, but in the blocks created by the aggregator, not the page that displays the actual items. That's a somewhat less useful place. ------------------------------------------------------------------------ Sun, 07 Aug 2005 17:56:35 +0000 : maui1 I couldn't agree more. Placing the "Blogit" link in the block is almost worse than not having it. I only decide to blog an item or post it as a story based on reading the description, not from the title alone. Blogit and Post as Story were the two most used features in my Aggregator installation and users are screaming bloddy murder over their dissapearance. Please, Please add these features back to the main distribution and if necessary for some designers, have an admin check box to dissable it. ------------------------------------------------------------------------ Mon, 08 Aug 2005 15:00:47 +0000 : Bèr Kessels If there is a good reason to not have it in aggregator, then I will happily add it to node aggregator. IMO this feature is somewhat "off" in core. It does not really fit in our keep it simple, clean and lean mantra. IMO this is a very good feature for a contribution, like node aggregator.

1 0

[drupal-devel] [feature] "blog it" link lost in aggregator
by Bèr Kessels 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/10752 Post a follow up: http://drupal.org/project/comments/add/10752 Project: Drupal Version: 4.6.2 Component: aggregator.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Gunnar Langemark(a)www.langemark.com Updated by: Bèr Kessels Status: patch (code needs review) If there is a good reason to not have it in aggregator, then I will happily add it to node aggregator. IMO this feature is somewhat "off" in core. It does not really fit in our keep it simple, clean and lean mantra. IMO this is a very good feature for a contribution, like node aggregator. Bèr Kessels Previous comments: ------------------------------------------------------------------------ Sun, 12 Sep 2004 07:51:48 +0000 : Gunnar Langemark(a)www.langemark.com I upgraded Langemarks Cafe and noticed that the "blog it" link from aggregator posts is no more. So I did a clean install of cvs - and it is still not there. I use it every day as it is an integrated part of my site maintenance routine. So for me it is critical - even though Drupal runs fine without. Sure hope this is a bug and not something which has been removed. ------------------------------------------------------------------------ Sun, 12 Sep 2004 23:44:20 +0000 : drumm I am on my slow computer and haven't done a full test (subscribe to feeds and confirm it works), but the permisison name recently changed from maintain blog to edit own blog. Make sure you have those all selected properly in admin >> users >> configure >> roles. Setting as a support request for now as I could not find any inconsistencies in the code yet. ------------------------------------------------------------------------ Fri, 18 Feb 2005 02:54:36 +0000 : tatonca Attachment: http://drupal.org/files/issues/aggregator.module_5.patch (1.11 KB) I hope this is the way to do this... There is a thread discussing this issue: http://drupal.org/node/10740 and I think Dries confirms that this was changed by design. I myself liked having the feature, so I added it back in on my site. Patch attached - hope folks find this useful... I thought it might be kewl to make this an option to be turned on or off in the aggregator admin settings - haven't got as far as figuring out how to do that yet ;) when I do I'll post that too... Since this was originally removed by design I've changed the status to feature request. I also tried to find another thread or issue but this seemed to be the only one... ~Tat~ ------------------------------------------------------------------------ Sat, 26 Mar 2005 07:14:04 +0000 : wal >From what I found, you have to be the original admin account to get the "blog it" icon. I even tried making an "admin" role and giving it all the permissions. The only way I see the icon is if I am logged in as the original admin account. Giving all the permissions to the "authenticated user" role didn't seem to be enough. I simply changed the original account to be my username. I know this isn't a real fix but since I am the only person using that specific install it fixes my issue. Good luck, --Bill ------------------------------------------------------------------------ Sat, 09 Apr 2005 17:22:49 +0000 : bloggator This worked for me: In your aggregator.module, replace everything from line 1111 to the end with: $output .= "<div class=\"news-item\">\n"; $output .= ' <div class="date">'. date('H:i', $item->timestamp) ."</div>\n"; $output .= " <div class=\"body\">\n"; $output .= " <div class=\"title\"><a href=\"$item->link\">$item->title</a> "; if ($user->uid && module_exist('blog') && user_access('edit own blog')) { if ($image = theme('image', 'misc/blog.png', t('blog it'), t('blog it'))) { $output .= '<span class="icon">'. l($image, 'node/add/blog', array('title' => t('Comment on this news item in your personal blog.'), 'class' => 'blog-it'), "iid=$item->iid") .'</span>'; $output .="</div>\n"; } } if ($item->description) { $output .= " <div class=\"description\">$item->description</div>\n"; } if ($item->ftitle && $item->fid) { $output .= " <div class=\"description\"> </div>\n"; $output .= ' <div class="source">'. t('Source') .': '. l($item->ftitle, "aggregator/sources/$item->fid") ."</div>\n"; } $result = db_query('SELECT c.title, c.cid FROM {aggregator_category_item} ci LEFT JOIN {aggregator_category} c ON ci.cid = c.cid WHERE ci.iid = %d ORDER BY c.title', $item->iid); $categories = array(); while ($category = db_fetch_object($result)) { $categories[] = l($category->title, 'aggregator/categories/'. $category->cid); } $output .= " </div>\n"; $output .= "</div>\n"; return $output; } ?> Best regards, Jeff ------------------------------------------------------------------------ Tue, 26 Apr 2005 22:58:57 +0000 : maui1 Hi Jeff, I tried your code patch for the aggregator module in 4.6 and it didn't get the blogIt and Post as Story links back. It did seem to slow access to aggregator down, but no links. Where was this design decision made, I would like to contribute my reasons for needing this functionality. ------------------------------------------------------------------------ Tue, 05 Jul 2005 04:11:13 +0000 : johnpg It's missing the $user declaration so it never gets past the "if" statements. Edit the theme_aggregator_page_item function so it looks like this. function theme_aggregator_page_item($item) { static $last; global $user; Then add if ($user->uid && module_exist('blog') && user_access('edit own blog')) { if ($image = theme('image', 'misc/blog.png', t('blog it'), t('blog it'))) { $output .= ''. l($image, 'node/add/blog', array('title' => t('Comment on this news item in your personal blog.'), 'class' => 'blog-it'), "iid=$item->iid", NULL, FALSE, TRUE) .''; } } Directly after this line: $output .= ' '. check_plain($item->title) .\" [1]\n"; This should really be back in the code as standard, very useful. John [1] http://drupal.org/'. check_url($item->link) .' ------------------------------------------------------------------------ Tue, 05 Jul 2005 04:12:50 +0000 : johnpg It's missing the $user declaration so it never gets past the "if" statements. Edit the theme_aggregator_page_item function so it looks like this. function theme_aggregator_page_item($item) { static $last; global $user; Then add if ($user->uid && module_exist('blog') && user_access('edit own blog')) { if ($image = theme('image', 'misc/blog.png', t('blog it'), t('blog it'))) { $output .= '<div class="icon">'. l($image, 'node/add/blog', array('title' => t('Comment on this news item in your personal blog.'), 'class' => 'blog-it'), "iid=$item->iid", NULL, FALSE, TRUE) .'</div>'; } } Directly after this line: $output .= ' <div class="title">'. check_plain($item->title) .\" [2]</div>\n"; This should really be back in the code as standard, very useful. John [2] http://drupal.org/'. check_url($item->link) .' ------------------------------------------------------------------------ Fri, 05 Aug 2005 17:06:24 +0000 : peacearena I get this working but with a strange problem. In the aggregator contents after the title, where the blog.png image should be, the actual code to the image file shows: <img src="misc/blog.png" alt="blog it" title="blog it" /> as a link. Yes the image exists, yes img span and div are allowed in that format. What am I doing wrong? I tried changing the path to the image, making it absolute, putting the image in the theme directory, but then nothing shows at all after the title. I hope someone can help me with this, I'd really like this feature. Using Drupal 4.6.2. Thanks. ------------------------------------------------------------------------ Sat, 06 Aug 2005 08:00:42 +0000 : Boris Mann Why the hell does this keep getting taken out?! Many people use it, it shouldn't be removed. If some developers somewhere don't like it, then add an admin setting to toggle this on or off. It really should be added to both 4.6 and 4.7. ------------------------------------------------------------------------ Sat, 06 Aug 2005 10:27:44 +0000 : Prometheus6 The link exists, but in the blocks created by the aggregator, not the page that displays the actual items. That's a somewhat less useful place. ------------------------------------------------------------------------ Sun, 07 Aug 2005 17:56:35 +0000 : maui1 I couldn't agree more. Placing the "Blogit" link in the block is almost worse than not having it. I only decide to blog an item or post it as a story based on reading the description, not from the title alone. Blogit and Post as Story were the two most used features in my Aggregator installation and users are screaming bloddy murder over their dissapearance. Please, Please add these features back to the main distribution and if necessary for some designers, have an admin check box to dissable it.

1 0

[drupal-devel] [feature] Patch To Add User Profile Search
by jjeff 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/28325 Post a follow up: http://drupal.org/project/comments/add/28325 Project: Drupal Version: cvs Component: profile.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: jjeff Updated by: jjeff Status: patch (code needs review) Attachment: http://drupal.org/files/issues/profilesearch.patch (1.88 KB) Here's a patch that adds a search function for user profiles. Users with "access user profiles" privileges can search all public profile fields, while users with 'administer users' privileges can search both private and public profile fields. Search results display username (as a link) along with a snippet of the user's profile. -Jeff Robbins jjeff

3 3

[drupal-devel] Hosting company for inclusion in http://drupal.org/services/hosting
by Phillip Berry 08 Aug '05

08 Aug '05

Hello, If possible we would like our company Bliss Webhosting to be included on http://drupal.org/services/hosting. Obviously we fully support Drupal. Our url is www.blisswebhosting.com Could the wording read as follows; "Bliss Webhosting is an Australian webhosting company that like do do things differently. Our custom control-panel "Blisscontrol", specifically allows one-click installation and upgrading of Drupal and other popular applications. Choose between a predefined plan, starting from $14.95AUD p/m or a custom plan where after specifying your exact needs will show you an instant monthly price. There is no setup fees and after signing up, accounts are immediately configured and enabled." Could you also link the name "Blisscontrol" to http://www.blisswebhosting.com/blisscontrol.html Kind Regards Phillip Berry Bliss Webhosting

2 1

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by Jeremy 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/28420 Post a follow up: http://drupal.org/project/comments/add/28420 Project: Drupal -Version: 4.6.2 +Version: cvs Component: comment.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Jeremy(a)kerneltrap.org Updated by: Jeremy(a)kerneltrap.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/comment.module_12.patch (1.4 KB) Here's a second version of the patch that doesn't require any manual configuration. Jeremy(a)kerneltrap.org Previous comments: ------------------------------------------------------------------------ Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy(a)kerneltrap.org Setting "Preview comment" to "Required" does not strictly require that the comment be previewed first. This is being abused by spammers to quickly and efficiently post spam comments. I discovered this after I added a new feature to my new spam module [1] to auto-blacklist spammer IP addresses, allowing me to block comment spammers when they preview a comment and thus preventing them from ever inserting their spam into my database. I configured my comment module to "require" comment previews, and yet found that the comments were slipping past my filter. I finally realized what the spammer is doing is setting $_POST['op'] to 'Post comment', effectively bypassing the preview phase. I'm currently looking for a clean solution to this. At the moment the only idea I have is to generate a token at the preview phase, and validate the token at the post phase. Unfortunately the token would have to be stored in the databse between the preview and the post, which adds overhead. Alternatively, I've considered using a time-based hash which would constantly update depending on the time of day. This could easily be validated without storing anything in the database. If too long has gone between the preview and the post, an additional preview step would be required... The down side here is that the time-based hash would be publically available, and thus the spammer could easily duplicate it in their script. A private key could solve for that, but increases the complexity as it adds a configuration step. I have the feeling I'm missing a simpler, cleaner solution. Suggestions? [1] http://kerneltrap.org/jeremy/drupal/spam/ ------------------------------------------------------------------------ Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman even if you get this fixed, won't these bots just add a preview step? this 'preview required' feature is designed to maintain high quality submissions by forcing users to proof read. it isn;t designed for security. i think you want to hook into comment_validate(). just add a hook here - there is already a hook_comment() waiting for you to add an operation. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:49:30 +0000 : Eaton I posted a patch a few days ago (http://drupal.org/node/28255) that adds validation and form construction hooks for comments. It's similar to the one that the captcha module uses, though it adds comment form_pre and form_post hooks instead of a single comment form hook. ------------------------------------------------------------------------ Mon, 08 Aug 2005 13:30:34 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB) > even if you get this fixed, won't these bots just add a preview step? Eventually, yes, but it drastically changes their ability to fling spam at a site. As is, they simply have a script that shoots data out at high speed without having to wait for messages to return from the server. It is the server that is doing all the work, thus making it simple for a spammer to DoS a site. If "preview required" really meant "preview required", they would be forced to first automate clicking "preview", and then wait for a response before clicking "submit". This requires more resources on their side, and allows us to add delays after clicking "preview" (if we detect that they are a spammer) further using their resources. > this 'preview required' feature is designed to maintain high quality > submissions by forcing users to proof read. it isn;t designed for security. Regardless of the intention, I was misled to believe that configuring my site to require previews would require that all comments were first previewed. As a site administrator, I would prefer to know that "required" really means "required". > i think you want to hook into comment_validate(). just add a hook here - > there is already a hook_comment() waiting for you to add an operation. Yes and no. Ultimately yes that will work and will allow my spam module to prevent the spam from ever being posted. But it still leaves the greatest burden on the web server, instead of on the spammer. The spammer can still use a very simple script that only pushes data, and thus can generate spam at an unbelievable rapid rate. Here is an example patch to enforce "preview required". It's one idea, I'm sure there are better ones.

1 0

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by Jeremy 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/28420 Post a follow up: http://drupal.org/project/comments/add/28420 Project: Drupal Version: 4.6.2 Component: comment.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Jeremy(a)kerneltrap.org Updated by: Jeremy(a)kerneltrap.org -Status: active +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB) > even if you get this fixed, won't these bots just add a preview step? Eventually, yes, but it drastically changes their ability to fling spam at a site. As is, they simply have a script that shoots data out at high speed without having to wait for messages to return from the server. It is the server that is doing all the work, thus making it simple for a spammer to DoS a site. If "preview required" really meant "preview required", they would be forced to first automate clicking "preview", and then wait for a response before clicking "submit". This requires more resources on their side, and allows us to add delays after clicking "preview" (if we detect that they are a spammer) further using their resources. > this 'preview required' feature is designed to maintain high quality > submissions by forcing users to proof read. it isn;t designed for security. Regardless of the intention, I was misled to believe that configuring my site to require previews would require that all comments were first previewed. As a site administrator, I would prefer to know that "required" really means "required". > i think you want to hook into comment_validate(). just add a hook here - > there is already a hook_comment() waiting for you to add an operation. Yes and no. Ultimately yes that will work and will allow my spam module to prevent the spam from ever being posted. But it still leaves the greatest burden on the web server, instead of on the spammer. The spammer can still use a very simple script that only pushes data, and thus can generate spam at an unbelievable rapid rate. Here is an example patch to enforce "preview required". It's one idea, I'm sure there are better ones. Jeremy(a)kerneltrap.org Previous comments: ------------------------------------------------------------------------ Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy(a)kerneltrap.org Setting "Preview comment" to "Required" does not strictly require that the comment be previewed first. This is being abused by spammers to quickly and efficiently post spam comments. I discovered this after I added a new feature to my new spam module [1] to auto-blacklist spammer IP addresses, allowing me to block comment spammers when they preview a comment and thus preventing them from ever inserting their spam into my database. I configured my comment module to "require" comment previews, and yet found that the comments were slipping past my filter. I finally realized what the spammer is doing is setting $_POST['op'] to 'Post comment', effectively bypassing the preview phase. I'm currently looking for a clean solution to this. At the moment the only idea I have is to generate a token at the preview phase, and validate the token at the post phase. Unfortunately the token would have to be stored in the databse between the preview and the post, which adds overhead. Alternatively, I've considered using a time-based hash which would constantly update depending on the time of day. This could easily be validated without storing anything in the database. If too long has gone between the preview and the post, an additional preview step would be required... The down side here is that the time-based hash would be publically available, and thus the spammer could easily duplicate it in their script. A private key could solve for that, but increases the complexity as it adds a configuration step. I have the feeling I'm missing a simpler, cleaner solution. Suggestions? [1] http://kerneltrap.org/jeremy/drupal/spam/ ------------------------------------------------------------------------ Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman even if you get this fixed, won't these bots just add a preview step? this 'preview required' feature is designed to maintain high quality submissions by forcing users to proof read. it isn;t designed for security. i think you want to hook into comment_validate(). just add a hook here - there is already a hook_comment() waiting for you to add an operation. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:49:30 +0000 : Eaton I posted a patch a few days ago (http://drupal.org/node/28255) that adds validation and form construction hooks for comments. It's similar to the one that the captcha module uses, though it adds comment form_pre and form_post hooks instead of a single comment form hook.

1 0

[drupal-devel] [task] Extend db_query()
by Cvbge 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/17656 Post a follow up: http://drupal.org/project/comments/add/17656 Project: Drupal Version: cvs Component: database system Category: tasks Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: Cvbge Status: patch (code needs review) I'll be talking about postgres db only. The patch won't work as expected in some cases. Let's say we have a text/char/other character column and we want to insert a text that looks like a number. The _db_argument_type() will recognize the value as a numeric/number etc and will use $key = %d format. Even not considering printf() numeric conversion this can lead to incorrect value inserted into db. Examples of such problematic texts are: 01 (leading 0), 2e2 (scientific notation), probably also text with trailing/leading white spaces. When inserting such data into db without quotes the postgres db will think this is a number and will change 010 to 10, 2e2 to 200 . Here is an example: piotr=> CREATE TABLE t(c CHAR(10)); piotr=> INSERT INTO t VALUES (01); piotr=> INSERT INTO t VALUES (2e2); piotr=> INSERT INTO t VALUES (-01); piotr=> INSERT INTO t VALUES (000); piotr=> SELECT * from t; c ------------ 1 200 -1 0 (4 rows) I think it's safe to use '%s' everywhere. I could not find it writtent directly in postgres documentation (although I remember reading it somewhere some time ago). Postgres will convert the string to correct integer/numeric/bool value. Cvbge Previous comments: ------------------------------------------------------------------------ Mon, 21 Feb 2005 12:48:30 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query.patch (2.16 KB) We should make our database abstraction layer more robust and ensure that module authors can use it without string manipulations inside the query. Several queries use implode() to get their arguments into the query. This is undesirable as we rely on the module author to check the keys and values of such arrays for exploitation attempts. I have created the attached patch which shouldbe able to allow us to not use implode anymore. A minor problem is that all inserted values will be treated as strings. This might be a problem with PostgreSQL at least. However, the same strategy is already used in Drupal core without any complaints I know of. Summary: This patch will alow us to simplify some code in node.module, user.module, taxonomy.module and probably others. ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:03:58 +0000 : killes(a)www.drop.org It's a patch. ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:19:13 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query_0.patch (2.07 KB) Squeezed out two lines of code after consultation with Karoly. Adds only 10 loc (plus some docs). ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:23:08 +0000 : chx Do I need to say +1? ------------------------------------------------------------------------ Thu, 03 Mar 2005 00:15:10 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query_1.patch (2.69 KB) After some discussion with Adrian at Drupal Con we found out that we do not know why node_save currently works with pgsql. It currently assumes that all db columns are strings. It seems to work but we should not rely on it. Here is a patch that checks for the type of field that is inserted. It needs testing. ------------------------------------------------------------------------ Tue, 26 Jul 2005 01:17:04 +0000 : drumm +1 for making this into an API. I've seen too many hacked together query builders in Drupal and Contrib. I have not tested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:30:29 +0000 : Bèr Kessels untested. a big +1 for the feature ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:41:52 +0000 : killes(a)www.drop.org the patch still applies. the new patch here updates node_save to use it. Untested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:42:24 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/node-%a.patch (926 bytes) the patch still applies. the new patch here updates node_save to use it. Untested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:45:17 +0000 : Cvbge "After some discussion with Adrian at Drupal Con we found out that we do not know why node_save currently works with pgsql. " Well, it does not work. Real life exampless of not-working include flexinode (my experience) and forum module (as someone reported). Probably also others. The bug occurs when a normal user (but with necessarily rights) adds a node and he has no controls (the 'moderated', 'sticky', 'published' etc). The, in node_load() $node->sticky, $node->moderated (and maybe others) are set to FALSE (or TRUE, but in this case it works). When doing printf("%s", FALSE) the FALSE is change to empty string. The sticky and moderated db fields are numeric and postgresql do not accept '' (empty string) as a value of integer type. The result is for example such error: warning: pg_query(): Query failed: ERROR: invalid input syntax for integer: "" in ..../includes/database.pgsql.inc on line 45. user error: query: INSERT INTO node (title, uid, type, teaser, status, moderate, promote, sticky, body, comment, created, changed, nid) VALUES('xx', '2', 'flexinode-1', '<div class="flexinode-body flexinode-1"><div class="flexinode-image-3"><div class="form-item"> <label>Zdjęcie:</label><br /> <img alt="xx" src="..../pliki/" /><br />Get original file (28KB) [1] </div> </div></div>', '1', '', '1', '', '<div class="flexinode-body flexinode-1"><div class="flexinode-image-3"><div class="form-item"> <label>Zdjęcie:</label><br /> <img alt="xx" src="..../pliki/" /><br />Get original file (28KB) [2] </div> < in ..../includes/database.pgsql.inc on line 62. [1] http://drupal.org/..../pliki//tmp/male.jpg [2] http://drupal.org/..../pliki//tmp/male.jpg ------------------------------------------------------------------------ Wed, 27 Jul 2005 13:03:14 +0000 : Cvbge Attachment: http://drupal.org/files/issues/node.module_5.diff (500 bytes) Here's a quick fix for 4.6 ------------------------------------------------------------------------ Wed, 27 Jul 2005 13:10:54 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/node-a.patch (1.8 KB) the last patch wasn't good. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:19:14 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_api_taxonomy.patch (3.5 KB) Here is an additional patch for taxonomy.module it removes the two custom functions that are used for inserts and updates. Needs testing. @Cvbge: shoduld we have a better test than just is_numeric in _db_argument_type to fix the pgsql problems? We could also try to pass $value by reference and cast it to int. this function is only used for updates/inserts so we can afford an additional check. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:37:25 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_query.patch (2.74 KB) Here's a revised version fo the original patch. _db_argument_type now gets the $value argument by reference and we check for bools too. numerics and bools are cast to int. ------------------------------------------------------------------------ Mon, 08 Aug 2005 10:34:36 +0000 : Bèr Kessels I cannot say anything about performance for I do not know how to benchmark this. But I tried it, And rewrote some queries in a custom module, to use it, and like it a lot. The code becomes cleaner, and better readable. But above all, i see this idea going into a very interesting and good direction: that of 'more' query builders in core'. a +1!

1 0

[drupal-devel] [bug] Users with 'maintain books' or 'edit own book pages' permission cannot delete book pages
by Bèr Kessels 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/21559 Post a follow up: http://drupal.org/project/comments/add/21559 Project: Drupal Version: cvs Component: book.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: clydefrog Updated by: Bèr Kessels Status: patch (code needs review) I would say this is a "by design". Maybe we need to rename the two book permissions into: "create books" and "edit own book pages". None of the other nodes can be deleted by the roles with "edit own Foo" permissions, nor should the books. Bèr Kessels Previous comments: ------------------------------------------------------------------------ Fri, 29 Apr 2005 04:32:19 +0000 : clydefrog Attachment: http://drupal.org/files/issues/delete_own_book_pages.patch (626 bytes) AFAICT, only 'administer nodes' permission allows deletion of book pages. This patch allows users with 'maintain books' or 'edit own book pages' permission to delete book pages. The logic doesn't check if the node has updates pending. Should it? ------------------------------------------------------------------------ Fri, 29 Apr 2005 14:10:22 +0000 : clydefrog It's a patch, duh. ------------------------------------------------------------------------ Fri, 29 Apr 2005 14:21:27 +0000 : rivena If maintain books is a permission that overlaps other permissions, it should say so. For example, /maintain books (create, edit, and delete all book pages)/. While I agree that someone who has been given maintainer status should be able to delete pages, I wonder if 'delete own pages' should be made into a seperate permission. I could see a situation where I would not want someone to be able to delete their own pages. Do these permissions affect things that are not book pages but have been put into books? Anisa. It's /Golden Week/! ------------------------------------------------------------------------ Sat, 30 Apr 2005 03:43:17 +0000 : clydefrog I think it's clear that "maintain books" implies the ability to edit and delete book pages. No other node module provides a "delete own " permission, so it would be inconsistent for book.module to provide it. Can you provide some examples to justify the inconsistency? These permissions do not affect nodes which aren't book pages but are in a book. Those nodes are controlled by their respective type-specific permissions, but only users with "maintain books" permission can add or remove those nodes from books. I believe this is the correct behavior. ------------------------------------------------------------------------ Sat, 30 Apr 2005 11:03:47 +0000 : rivena Hmm... But, you just said that until this patch, it *didn't* include the delete pages permission. Right? So, now you are changing it's behavior, but leaving the wording the same. How will people know? Or does it not matter, because you presume they didn't really know it didn't include delete in the first place? I don't know that I can defend my idea of having a seperate delete own pages permission against a standard of consistency. If all the other edit own permissions include the delete pages permission, then I have no quibbles. :) Except it would be more userfriendly if I knew what the permissions meant before I granted them to someone, without having to look at the code. But I do realize that is completely out of the scope of the patch. :) Where do all these little changes *go*, anyway? The changelog just says things like... refractored the search module. Anisa. ------------------------------------------------------------------------ Sat, 30 Apr 2005 16:04:14 +0000 : clydefrog I believe that "maintain books" permission implies the ability to delete book pages, so this patch fixes that bug. Other node modules include deletion in the "edit own" permission. For example, page.module: <?php if ($op == 'update' || $op == 'delete') { if (user_access('edit own pages') && ($user->uid == $node->uid)) { return TRUE; } } ?> I agree that it would be better if permissions were clearer about what they implied. There was a discussion started by killes [1] a while ago: "One problem with Drupal is the way it handles user permissions. The permissions are attached to the user object but nobody really knows what they do. To find out what a particular permission allows you to do, you often need to have a look at the code. " I don't know if it ever lead to any code. I think the changelog is meant to list the changes since the previous version without too much technical detail. If you want to see every last change, take a look at the cvs messages [2]. [1] http://lists.drupal.org/archives/drupal-devel/2005-04/msg00704.html [2] http://drupal.org/project/cvs/3060 ------------------------------------------------------------------------ Mon, 23 May 2005 17:33:18 +0000 : rivena I've been looking at the book module permissions recently, and did a bit of testing. I wanted to know what was the minimum permissions needed to create a new book. I assumed based on this discussion, and a similar discussion on the drupal support list that this was 'maintain books' and 'administer nodes'. I ran into a problem, then added 'access nodes' to this. So now my user has no other permissions other than 'maintain books', 'administer nodes' and 'access nodes'. You'd think this is more than sufficient to do something simple like create a new book, but in fact, with just these permissions, you do not have access to create a book page. You can edit other pages, and move them into a new book. Once you have create pages permission, you appear to finally have all the permissions needed, edit own pages is not needed. So, since, the point of this patch was that not having delete permission was odd, might I suggest adding create permission to maintain books? ;p Anisa.

1 0

[drupal-devel] [task] Extend db_query()
by Bèr Kessels 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/17656 Post a follow up: http://drupal.org/project/comments/add/17656 Project: Drupal Version: cvs Component: database system Category: tasks Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: Bèr Kessels Status: patch (code needs review) I cannot say anything about performance for I do not know how to benchmark this. But I tried it, And rewrote some queries in a custom module, to use it, and like it a lot. The code becomes cleaner, and better readable. But above all, i see this idea going into a very interesting and good direction: that of 'more' query builders in core'. a +1! Bèr Kessels Previous comments: ------------------------------------------------------------------------ Mon, 21 Feb 2005 12:48:30 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query.patch (2.16 KB) We should make our database abstraction layer more robust and ensure that module authors can use it without string manipulations inside the query. Several queries use implode() to get their arguments into the query. This is undesirable as we rely on the module author to check the keys and values of such arrays for exploitation attempts. I have created the attached patch which shouldbe able to allow us to not use implode anymore. A minor problem is that all inserted values will be treated as strings. This might be a problem with PostgreSQL at least. However, the same strategy is already used in Drupal core without any complaints I know of. Summary: This patch will alow us to simplify some code in node.module, user.module, taxonomy.module and probably others. ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:03:58 +0000 : killes(a)www.drop.org It's a patch. ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:19:13 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query_0.patch (2.07 KB) Squeezed out two lines of code after consultation with Karoly. Adds only 10 loc (plus some docs). ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:23:08 +0000 : chx Do I need to say +1? ------------------------------------------------------------------------ Thu, 03 Mar 2005 00:15:10 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query_1.patch (2.69 KB) After some discussion with Adrian at Drupal Con we found out that we do not know why node_save currently works with pgsql. It currently assumes that all db columns are strings. It seems to work but we should not rely on it. Here is a patch that checks for the type of field that is inserted. It needs testing. ------------------------------------------------------------------------ Tue, 26 Jul 2005 01:17:04 +0000 : drumm +1 for making this into an API. I've seen too many hacked together query builders in Drupal and Contrib. I have not tested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:30:29 +0000 : Bèr Kessels untested. a big +1 for the feature ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:41:52 +0000 : killes(a)www.drop.org the patch still applies. the new patch here updates node_save to use it. Untested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:42:24 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/node-%a.patch (926 bytes) the patch still applies. the new patch here updates node_save to use it. Untested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:45:17 +0000 : Cvbge "After some discussion with Adrian at Drupal Con we found out that we do not know why node_save currently works with pgsql. " Well, it does not work. Real life exampless of not-working include flexinode (my experience) and forum module (as someone reported). Probably also others. The bug occurs when a normal user (but with necessarily rights) adds a node and he has no controls (the 'moderated', 'sticky', 'published' etc). The, in node_load() $node->sticky, $node->moderated (and maybe others) are set to FALSE (or TRUE, but in this case it works). When doing printf("%s", FALSE) the FALSE is change to empty string. The sticky and moderated db fields are numeric and postgresql do not accept '' (empty string) as a value of integer type. The result is for example such error: warning: pg_query(): Query failed: ERROR: invalid input syntax for integer: "" in ..../includes/database.pgsql.inc on line 45. user error: query: INSERT INTO node (title, uid, type, teaser, status, moderate, promote, sticky, body, comment, created, changed, nid) VALUES('xx', '2', 'flexinode-1', '<div class="flexinode-body flexinode-1"><div class="flexinode-image-3"><div class="form-item"> <label>Zdjęcie:</label><br /> <img alt="xx" src="..../pliki/" /><br />Get original file (28KB) [1] </div> </div></div>', '1', '', '1', '', '<div class="flexinode-body flexinode-1"><div class="flexinode-image-3"><div class="form-item"> <label>Zdjęcie:</label><br /> <img alt="xx" src="..../pliki/" /><br />Get original file (28KB) [2] </div> < in ..../includes/database.pgsql.inc on line 62. [1] http://drupal.org/..../pliki//tmp/male.jpg [2] http://drupal.org/..../pliki//tmp/male.jpg ------------------------------------------------------------------------ Wed, 27 Jul 2005 13:03:14 +0000 : Cvbge Attachment: http://drupal.org/files/issues/node.module_5.diff (500 bytes) Here's a quick fix for 4.6 ------------------------------------------------------------------------ Wed, 27 Jul 2005 13:10:54 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/node-a.patch (1.8 KB) the last patch wasn't good. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:19:14 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_api_taxonomy.patch (3.5 KB) Here is an additional patch for taxonomy.module it removes the two custom functions that are used for inserts and updates. Needs testing. @Cvbge: shoduld we have a better test than just is_numeric in _db_argument_type to fix the pgsql problems? We could also try to pass $value by reference and cast it to int. this function is only used for updates/inserts so we can afford an additional check. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:37:25 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_query.patch (2.74 KB) Here's a revised version fo the original patch. _db_argument_type now gets the $value argument by reference and we check for bools too. numerics and bools are cast to int.

1 0

[drupal-devel] [task] Extend db_query()
by killes 08 Aug '05

08 Aug '05

Issue status update for http://drupal.org/node/17656 Post a follow up: http://drupal.org/project/comments/add/17656 Project: Drupal Version: cvs Component: database system Category: tasks Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: killes(a)www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/db_query.patch (2.74 KB) Here's a revised version fo the original patch. _db_argument_type now gets the $value argument by reference and we check for bools too. numerics and bools are cast to int. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Mon, 21 Feb 2005 12:48:30 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query.patch (2.16 KB) We should make our database abstraction layer more robust and ensure that module authors can use it without string manipulations inside the query. Several queries use implode() to get their arguments into the query. This is undesirable as we rely on the module author to check the keys and values of such arrays for exploitation attempts. I have created the attached patch which shouldbe able to allow us to not use implode anymore. A minor problem is that all inserted values will be treated as strings. This might be a problem with PostgreSQL at least. However, the same strategy is already used in Drupal core without any complaints I know of. Summary: This patch will alow us to simplify some code in node.module, user.module, taxonomy.module and probably others. ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:03:58 +0000 : killes(a)www.drop.org It's a patch. ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:19:13 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query_0.patch (2.07 KB) Squeezed out two lines of code after consultation with Karoly. Adds only 10 loc (plus some docs). ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:23:08 +0000 : chx Do I need to say +1? ------------------------------------------------------------------------ Thu, 03 Mar 2005 00:15:10 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query_1.patch (2.69 KB) After some discussion with Adrian at Drupal Con we found out that we do not know why node_save currently works with pgsql. It currently assumes that all db columns are strings. It seems to work but we should not rely on it. Here is a patch that checks for the type of field that is inserted. It needs testing. ------------------------------------------------------------------------ Tue, 26 Jul 2005 01:17:04 +0000 : drumm +1 for making this into an API. I've seen too many hacked together query builders in Drupal and Contrib. I have not tested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:30:29 +0000 : Bèr Kessels untested. a big +1 for the feature ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:41:52 +0000 : killes(a)www.drop.org the patch still applies. the new patch here updates node_save to use it. Untested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:42:24 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/node-%a.patch (926 bytes) the patch still applies. the new patch here updates node_save to use it. Untested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:45:17 +0000 : Cvbge "After some discussion with Adrian at Drupal Con we found out that we do not know why node_save currently works with pgsql. " Well, it does not work. Real life exampless of not-working include flexinode (my experience) and forum module (as someone reported). Probably also others. The bug occurs when a normal user (but with necessarily rights) adds a node and he has no controls (the 'moderated', 'sticky', 'published' etc). The, in node_load() $node->sticky, $node->moderated (and maybe others) are set to FALSE (or TRUE, but in this case it works). When doing printf("%s", FALSE) the FALSE is change to empty string. The sticky and moderated db fields are numeric and postgresql do not accept '' (empty string) as a value of integer type. The result is for example such error: warning: pg_query(): Query failed: ERROR: invalid input syntax for integer: "" in ..../includes/database.pgsql.inc on line 45. user error: query: INSERT INTO node (title, uid, type, teaser, status, moderate, promote, sticky, body, comment, created, changed, nid) VALUES('xx', '2', 'flexinode-1', '<div class="flexinode-body flexinode-1"><div class="flexinode-image-3"><div class="form-item"> <label>Zdjęcie:</label><br /> <img alt="xx" src="..../pliki/" /><br />Get original file (28KB) [1] </div> </div></div>', '1', '', '1', '', '<div class="flexinode-body flexinode-1"><div class="flexinode-image-3"><div class="form-item"> <label>Zdjęcie:</label><br /> <img alt="xx" src="..../pliki/" /><br />Get original file (28KB) [2] </div> < in ..../includes/database.pgsql.inc on line 62. [1] http://drupal.org/..../pliki//tmp/male.jpg [2] http://drupal.org/..../pliki//tmp/male.jpg ------------------------------------------------------------------------ Wed, 27 Jul 2005 13:03:14 +0000 : Cvbge Attachment: http://drupal.org/files/issues/node.module_5.diff (500 bytes) Here's a quick fix for 4.6 ------------------------------------------------------------------------ Wed, 27 Jul 2005 13:10:54 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/node-a.patch (1.8 KB) the last patch wasn't good. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:19:14 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_api_taxonomy.patch (3.5 KB) Here is an additional patch for taxonomy.module it removes the two custom functions that are used for inserts and updates. Needs testing. @Cvbge: shoduld we have a better test than just is_numeric in _db_argument_type to fix the pgsql problems? We could also try to pass $value by reference and cast it to int. this function is only used for updates/inserts so we can afford an additional check.

1 0