April 2005 - Development - lists.drupal.org

[drupal-devel] Re: CVS question: Multiple branch tags on a module
by Sheldon Rampton 14 Apr '05

14 Apr '05

I wrote: > > However, the 4.5 branch still doesn't appear as the "latest >> release" when I go to the project page: >> >> http://drupal.org/project/interwiki >> >> If people click on the "download latest release," they still get an >> old version of the module rather than the current version. What can > > I do to fix this? And Jim Riggs replied: >It looks like you just committed changes over an hour ago. There is >a script that runs on the server that packages the tar.gz files every >6 hours (I think). Wait a few hours and check it again. It will get >updated. OK, I waited and Jim was right. The tar.gz file now reflects my commit. The only thing now is that the project page still lists December 31, 2004 as the "latest release" date. This is a minor detail, of course. My main concern was making a current version available for download, and that's now accomplied. However, is there any way to get the latest release date changed on the project page? --Sheldon Rampton

3 2

[drupal-devel] [bug] Failure to respect node status upon editing (reverts to default)
by killes 14 Apr '05

14 Apr '05

Issue status update for http://drupal.org/node/7940 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: mathias Reported by: mathias Updated by: killes(a)www.drop.org Status: patch Some people think, that the node settings _should_ be reset after editing. There are good reasons for some of the settings to be reset, for exampel in case of the "promote" bit. I cannot see such reasons in case of the "comment" bit and would like to see that part of the patch included. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ May 21, 2004 - 04:00 : mathias How to recreate: 1. As admin, navigate to: administer » content » settings, and uncheck 'publish' for stories. 2. As admin, create a story and assign it to a user that can create and maintain their own stories, say this user's name is Mark. 3. Login as Mark and edit that story. 4. Instead of staying published, it reverts back to it's unpublished state. I think this behavior is a bug as these content settings are default settings and should only be applied during node creation (not editing). I know Drupal-contrib project owners get bitten with this everytime they edit their project pages. I'll work on a patch within the next day. ------------------------------------------------------------------------ May 21, 2004 - 16:59 : mathias Attachment: http://drupal.org/files/issues/node_defaults.patch (1.05 KB) Proposed solution: If a node is being updated: respect the following existing settings: 1. node status (published/unpublished) 2. node promotion (promoted on front page) 3. node static (static on front page) For any node, force the defaults for: 1. moderation 2. revisions With our current situation, say the admin makes 'promote to front page' and 'static on front page' the default settings for stories. Another user posts a story and bam! it's of course immediately visible. The admin decides he/she doesn't like that story and demotes it. Well as soon as that user edits his story, the default settings are re-assigned and his post is right back on the front page. ------------------------------------------------------------------------ August 3, 2004 - 20:38 : mathias Attachment: http://drupal.org/files/issues/node_defaults_0.patch (1.19 KB) When editing nodes, the following node properties should not be overwritten - node status (published/unpublished) - node promotion (promoted on front page) - node sticky (top of the list) Currently, if the author of a node doesn't have 'administer node' privileges, than the above settings will always revert to the default behavior of their node type group. This makes nodes disappear and node authors very very confused. ------------------------------------------------------------------------ August 3, 2004 - 20:56 : ccourtne Don't forget to take moderation into account. If a node is edited should it go back through moderation? Some sites may want the behavior of going back some sites may not. This is probably best as a toggle option on workflow default config screen. ------------------------------------------------------------------------ August 3, 2004 - 22:21 : mathias I don't think my patch interferes with the moderation features. If the default workflow for a type of nodes is set to 'moderate', than the moderation flag will always reset itself to 'moderation' upon editing. I did not alter this functionality. ------------------------------------------------------------------------ September 29, 2004 - 23:00 : mathias Attachment: http://drupal.org/files/issues/node_defaults_1.patch (1.64 KB) I was recently bitten by this bug again, so I'll try to describe the exact nature of the problem. If an user of a node doesn't have 'administer nodes' permission, than whenever they edit their own node the status, promotion, static, moderation and revision settings revert to their default state. This can cause: - A demoted post to once again be promoted. - A promoted post to be demoted. - A static post to disappear from the front page. All of these scenarios have happened to me simply by authors editing their own content. This patch allows all options except moderation and revision to stay with a node once it's created. Note: along a very similar line, the one-line 'node_validate does not respect group editing patch [1]' has been rolled into this as well. I'll take it out if need be. [1] http://drupal.org/node/11071 ------------------------------------------------------------------------ September 29, 2004 - 23:00 : mathias I was recently bitten by this bug again, so I'll try to describe the exact nature of the problem. If an user of a node doesn't have 'administer nodes' permission, than whenever they edit their own node the status, promotion, static, moderation and revision settings revert to their default state. This can cause: - A demoted post to once again be promoted. - A promoted post to be demoted. - A static post to disappear from the front page. All of these scenarios have happened to me simply by authors editing their own content. This patch allows all options except moderation and revision to stay with a node once it's created. Note: along a very similar line, the one-line 'node_validate does not respect group editing patch [2]' has been rolled into this as well. I'll take it out if need be. [2] http://drupal.org/node/11071 ------------------------------------------------------------------------ September 30, 2004 - 20:08 : Dries When someone edits a node that has been promoted, I do want to demote it from the main page. If not, the system is open for abuse. IMO, this is 'by design'. ------------------------------------------------------------------------ September 30, 2004 - 21:00 : Bèr Kessels Both Dries and Mathias have valid points here. If I add a post with loads of ugly stuff inside it, but an adminstrator decides not to delete it (because the post is still not too bad, or because of policy), but demote it, I can promote my spam next minute again by simply editing my content. This is bad. If I add a post with lots of good information, so that an administratr promotes it, next thing i can do is open it up and add loads of ugly spam to it. Buth are unwanted scenarios. The first is what we have now. The second is when this patch is committed. I think we need to seriously rething the workflow logic, maybe splitting workflow up into "add" and "edit". But maybe there are even better options? ------------------------------------------------------------------------ September 30, 2004 - 21:01 : Bèr Kessels Both Dries and Mathias have valid points here. If I add a post with loads of ugly stuff inside it, but an adminstrator decides not to delete it (because the post is still not too bad, or because of policy), but demote it, I can promote my spam next minute again by simply editing my content. This is bad. If I add a post with lots of good information, so that an administrator promotes it, next thing i can do is open it up and add loads of ugly spam to it. and then i have a promoted post with spam on that site. Both are unwanted scenarios. The first is what we have now. The second is when this patch is committed. I think we need to seriously rething the workflow logic, maybe splitting workflow up into "add" and "edit". But maybe there are even better options? ------------------------------------------------------------------------ September 30, 2004 - 21:02 : Bèr Kessels Both Dries and Mathias have valid points here. If I add a post with loads of ugly stuff inside it, but an adminstrator decides not to delete it (because the post is still not too bad, or because of policy), but demote it, I can promote my spam next minute again by simply editing my content. This is bad. If I add a post with lots of good information, so that an administrator promotes it, next thing i can do is open it up and add loads of ugly spam to it. and then i have a promoted post with spam on that site. Both are unwanted scenarios. The first is what we have now. The second is when this patch is committed. I think we need to seriously rething the workflow logic, maybe splitting workflow up into "add" and "edit". But maybe there are even better options? ------------------------------------------------------------------------ September 30, 2004 - 21:11 : Dries The first problem can be solved by blocking the user account, or by revoking the user's permissions. The second problem can probably be solved using the node-level permissions. Whenever I promote a post to the front page, I could revoke the user's edit rights. It would only work, if the modules don't by-pass the access rights, of course (I believe some do). ------------------------------------------------------------------------ October 3, 2004 - 22:28 : mathias Attachment: http://drupal.org/files/issues/node_defaults_2.patch (4.22 KB) Dries is right about using the power of node-level permissions for community editing. I think this patch would assist that goal. As it stands, node-level permissions don't work with community editing, at least in the sense we're used to. For one node_validate transfers ownership of the node to the current editing user which may not always be the author. This causes many problems since original authorship usually implies special privileges (e.g. 'edit own foo', delete). Second, sites have different trust levels for users. On some sites there's just a handful of trusted users that edit and maintain all the content. The current approach for community editing is to create a new role and give it the 'administer nodes' permission, which in my opinion makes the node form interface much more cluttered and gives users in that role full access to node properties. Full access is not always desired. Using a combination of node-level permissions and allowing a node to retain some or all of its options makes a clean, easy to use approach to maintainance. This new patch lets the admin control how node_validate handles node options during editing. It splits the default workflow screen into two parts: Creation settings, and Editing options. The creation settings is the default workflow we're all used to. The editing options allow the admin to say what happens to those node options when they are edited. I've created a node-level permissions module [3] based on JonBob's work that allows a user to choose which roles can view and/or edit their post. Unfortunately it needs this patch to allow node_validate to be a little less controlling. I'd like to contribute this module if it didn't need a patch. [3] http://www.asitis.org/tmp/ ------------------------------------------------------------------------ October 3, 2004 - 22:30 : mathias Attachment: http://drupal.org/files/issues/node_defaults_admin_content.png (22.39 KB) Screenshot of the new default 'node editing options' interface. ------------------------------------------------------------------------ October 4, 2004 - 16:16 : Bèr Kessels Mathias, First: using radio's for a Boolean is bad UI design IMO. It would be much better if you use checkboxes. Second: If you use radio's you can make the same matrix as above, for each node-type a reset foo flag. This will not only make the UI more consistent, but also improve usability by allowing per-node-type reset flag. Ber ------------------------------------------------------------------------ October 4, 2004 - 19:08 : Dries The settings GUI's help text is rather hard-core and will scare many user away. It will even frighten most Drupal developers. Also, I'm not sure I understood the screenshot correctly. The functionality introduced is much needed but the GUI and configuration parts needs work IMO. Keep working on it. ------------------------------------------------------------------------ October 14, 2004 - 22:55 : mathias Attachment: http://drupal.org/files/issues/node_defaults_3.patch (4.74 KB) As per Ber's feedback, I changed the interface to use checkboxes and allowed the reset flags to be set for each node-type. The result is a much more familiar and consistent experience. Dries: I revamped the help text, removing the scary bits and sticking to the bare essentials. The dialog concerning this patch has been excellent, and the result of which I feel is a sweet slice of code ready for core :) ------------------------------------------------------------------------ October 14, 2004 - 22:56 : mathias Attachment: http://drupal.org/files/issues/node_defaults_admin_content_0.png (18.68 KB) Latest screenshot of patch. ------------------------------------------------------------------------ October 28, 2004 - 21:08 : mathias Attachment: http://drupal.org/files/issues/node_defaults_4.patch (4.81 KB) Updated the help text to be slightly more descriptive, informing the reader that: Users with "administer nodes" permission can override the editing options. ------------------------------------------------------------------------ October 29, 2004 - 00:51 : Steven I'm not sure I like that admin interface. My brain tries to correlate the two tables, but the meanings are different (what is the value, keep the value?). Maybe it would be better just to have one checkbox per node type to allow you to either revert the published/frontpage/moderate/sticky fields on editing, or not. This should cover 99% of all use cases with an admin interface that is much easier to understand. ------------------------------------------------------------------------ October 30, 2004 - 20:29 : mathias Attachment: http://drupal.org/files/issues/node_defaults_5.patch (5.06 KB) It provides a consistant look and feel if the creation settings are the same as the editing settings since they both manipulate the same components of a node and you're just toggling states based on different actions. I think it is a bad idea to only give site admins an 'all or none' approach to override node options upon editing. Here are some examples why: - You want nodes to remember their 'sticky' setting, but unpublish themselves after editing. Once the node is approved, it goes right back to where it was. - You want nodes to remember their 'promote to front page' setting, but unpublish themselves after editing. Once the node is approved, it goes right back to where it was. - You want nodes to remember all their settings except moderation. The node is still published, but it goes into the queue to be rated again since the content has changed. - By default nodes are unpublished. Once it's been tagged as published it stays that way. Perhaps the moderation tag is reset. I've updated the code to hopefully make things even more clearer. Here's the latest screenshot [4]. [4] http://asitis.org/tmp/node_defaults_admin_content_1.png ------------------------------------------------------------------------ January 24, 2005 - 17:41 : moshe weitzman this patch fixes a fairly undesirable problem with our workflow. lets consider it along with other proposed changes to default workflow. ------------------------------------------------------------------------ March 13, 2005 - 23:04 : killes(a)www.drop.org Does not apply anymore. ------------------------------------------------------------------------ April 12, 2005 - 20:07 : r11r I'm still experiencing this problem with latest CVS (head and drupal-4-6) and in addition to the publishing options (Published, Promoted to front page, etc), it also affects the settings for comments. When a user edits the page, the settings revert to the default value for that content type, instead of what is currently set for the specific node. Example of this problem with respect to Comments: ( Create a user with permissions to edit own pages, but not administer comments. ) 1. user creates page (comments default to read/write) 2. admin edits page and turns off comments 3. user edits page and saves 4. comments are reset to read/write Help :) (Am willing to assist in fixing this if necessary) ------------------------------------------------------------------------ April 12, 2005 - 20:31 : killes(a)www.drop.org Ok, just some remarks from irc: 21:23 < killes> r11r: After looking a bit more at the code I think it is more of a feature request than a bug. 21:23 < killes> Not that I would mind some more flexibility, of course. 21:24 < r11r> killes: well, node settings magically resetting themselves could be considered a bug.. hehe 21:25 < killes> Well, it is not magically, it is "by design". ;) 21:25 < killes> i don't think we will get that into 4.6. But I woul dwork with you to get it into 4.7. 21:27 < killes> This needs to be part of a more general improved workflow thingie. 21:27 < killes> Did you try clousseau's workflow module? it is amazing. ------------------------------------------------------------------------ April 13, 2005 - 14:59 : r11r Haven't yet looked at workflow, but this issue can be fixed by simply unsetting the node properties for which the user doing the edit does not have permissions. i.e. in node.module (around line 1255) if (!$node->nid): -- load defaults from database settings else: -- unset node->status, moderate, promote, sticky, revision, created also in comment.module (around line 247, the nodeapi hooks) if (!user_access('administer nodes')): -- unset node->comment Because any properties that are not set in the node object do not get passed into the update sql statement, the node will retain its previous values after being edited by a user instead of reverting to the default values for the content-type. Can anyone think of problems with this approach for the time being? ------------------------------------------------------------------------ April 13, 2005 - 17:52 : r11r Change in comment.module, line 257 for head and 4.6, line 254 for 4.5. case 'validate': if (!user_access('administer nodes')) { if (!$node->nid) { // Force default for normal users: $node->comment = variable_get("comment_$node->type", 2); } else { // If the node is being updated, respect its previous settings unset($node->comment); } } break; ------------------------------------------------------------------------ April 13, 2005 - 18:02 : r11r Didn't really mean to change the title! ------------------------------------------------------------------------ April 13, 2005 - 18:05 : r11r Change in node.module, for head and 4.6, around line 1258 $node_options = variable_get('node_options_'. $node->type, array('status', 'promote')); if (!$node->nid) { // Force defaults in case people modify the form: $node->status = in_array('status', $node_options); $node->moderate = in_array('moderate', $node_options); $node->promote = in_array('promote', $node_options); $node->sticky = in_array('sticky', $node_options); } else { // If the node is being updated, respect is previous settings unset($node->status); unset($node->moderate); unset($node->promote); unset($node->sticky); } $node->revision = in_array('revision', $node_options); unset($node->created); For 4.5, the code is slightly different in node.module, around line 1095... if (!$node->nid) { // Force defaults in case people modify the form: $node->status = variable_get("node_status_$node->type", 1); $node->promote = variable_get("node_promote_$node->type", 1); $node->moderate = variable_get("node_moderate_$node->type", 0); $node->sticky = variable_get("node_sticky_$node->type", 0); } else { // If the node is being updated, respect is previous settings unset($node->status); unset($node->moderate); unset($node->promote); unset($node->sticky); } ------------------------------------------------------------------------ April 13, 2005 - 18:06 : r11r Attachment: http://drupal.org/files/issues/node.module_head.patch (1.38 KB) Patch for node.module in HEAD ------------------------------------------------------------------------ April 13, 2005 - 18:06 : r11r Attachment: http://drupal.org/files/issues/comment_head.module (76.29 KB) Patch for comment.module in HEAD ------------------------------------------------------------------------ April 13, 2005 - 18:07 : r11r Attachment: http://drupal.org/files/issues/comment.module_head.patch (976 bytes) Err, sorry too many files to pick from in the directory . :) Here's the patch again. (for comment.module, in HEAD) ------------------------------------------------------------------------ April 13, 2005 - 18:09 : r11r Attachment: http://drupal.org/files/issues/node.module_drupal-4-6.patch (1.39 KB) Patch for node.module in DRUPAL-4-6 ------------------------------------------------------------------------ April 13, 2005 - 18:09 : r11r Attachment: http://drupal.org/files/issues/comment.module_drupal-4-6.patch (988 bytes) Patch for comment.module in DRUPAL-4-6 ------------------------------------------------------------------------ April 13, 2005 - 18:10 : r11r Attachment: http://drupal.org/files/issues/node.module_drupal-4-5.patch (1.38 KB) Patch for node.module in DRUPAL-4-5 ------------------------------------------------------------------------ April 13, 2005 - 18:10 : r11r Attachment: http://drupal.org/files/issues/comment.module_drupal-4-5.patch (1010 bytes) Patch for comment.module in DRUPAL-4-5

1 0

[drupal-devel] Improve Drupal themability
by Stefan Nagtegaal 14 Apr '05

14 Apr '05

All, to improve theming of drupal I would like to introduce two new functions, which are theme_list() and theme_list_item().. function theme_list($items = array(), $id) { if (isset($items)) { $output .= '<ul id="'. $id .'">'; foreach ($items as $item) { $output .= theme('list_item', $item); } $output .= '</ul>'; } return $output; } function theme_list_item($item, $class = NULL) { return '<li class="'. $class .'">'. $item .'</li>'; } These two functions will help us, theme drupal. Every list is getting a unique id like $module-$method. For example we get: <ul id="book navigation"> <li class="leaf"><a href="#">...</a></li> <li class="leaf"><a href="#">...</a></li> <li class="leaf"><a href="#">...</a></li> </ul> With these two functions we can get all hardcoded lists (<ul><li>) out of drupal.. At the end, a lot of theme function will be deprecated. you can think about theme_status_messages(), theme_node_list(). Combining this with the modified theme_box()-approach of Ber Kessels (http://drupal.org/node/15332) we can theme almost anything we want and how we want, with less extraneous markup.. I would like to know what you guys think about this, and dicuss before having some major patches hitting the patch queue.. Stefan.

2 4

[drupal-devel] Caching and Opera
by Karoly Negyesi 14 Apr '05

14 Apr '05

I just chatted with an Opera person who fixed my Opera via VNC. I asked about caching woes. He pointed me that I could configure my Opera so it always checks for fresh pages (it only checks whether there is something new, so it's not THAT big a problem). I asked whether we could omit some headers so that Opera won't cache the page (like node edit page): "Once we didn't cache pages if the http headers told opera not to cache the page, but as many webservers are configured to set those headers without any good reason we had to remove that again. Many forums and dynamic pages send that header which made opera reload the page everytime. and _many_ users complained, so we disabled this behaviour for unsecure sites. So, in short, it's not possible, the users shall configure their Opera? Yes, you'll have to configure your opera that way. May I publish the last question and answer to it? That's public information, so yes." So here you are. This is the latest, official word. Regards NK

2 1

[drupal-devel] [bug] Blocks with no title kill Drupal
by killes 14 Apr '05

14 Apr '05

Issue status update for http://drupal.org/node/19965 Project: Drupal Version: cvs Component: block.module Category: bug reports -Priority: normal +Priority: critical Assigned to: Robin Monks Reported by: budda Updated by: killes(a)www.drop.org Status: patch Attachment: http://drupal.org/files/issues/block-fix.patch (673 bytes) The patch got apparently into core. It introduces a problem with php 5. $edit is not defined as an array if the form is called for the fiest time (default in switch). This patch fixes this. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ April 4, 2005 - 18:03 : budda When setting more than one block's title to '' (blank) Drupal dies, and doesn't catch the SQL query error nicely. The error is as follows: user error: Duplicate entry '' for key 2 query: UPDATE boxes SET title = '', body = ' ...<cut>... in /var/www/html/drupal-cvs/includes/database.mysql.inc on line 66. Using 4.6 build from 4/4/2005 The page ends with: warning: Cannot modify header information - headers already sent by (output started at /var/www/html/drupal-cvs/includes/common.inc:384) in /var/www/html/drupal-cvs/includes/common.inc on line 192. ------------------------------------------------------------------------ April 8, 2005 - 09:08 : robertDouglass There should be a check before the insert is attempted and if the block title already exists the user should be notified with form_error(). ------------------------------------------------------------------------ April 8, 2005 - 10:46 : Poolio this certainly doesn't solve the problem, but I've worked around this issue by using html tags in the title field for blocks that I don't want to show a title. ------------------------------------------------------------------------ April 9, 2005 - 01:31 : Robin Monks Attachment: http://drupal.org/files/issues/block.module.blank.title.patch (955 bytes) A small patch that checks to ensure the title does not already exit in the Blocks list. Also, if the field was left blank, in adds an HTML comment with the current date to the second to ensure that a duplicate title isn't created. This is my first patch, so I probably did something wrong, but I'm willing to fix it too. Robin Monks MozNetwork.org ------------------------------------------------------------------------ April 9, 2005 - 02:22 : Morbus Iff I'm, personally, fine with how you've fixed the blank block title. You've really solved two problems in one: duplicate titles (which is the cause of this Issue), and the desire to have blank (or no) titles (which is the reason for this Issue) for the end user. With that said, I'm not a fan of your error message (but, then again, anal wording is my specialty, so don't think I'm picking on you): I'd rename it to "That block title is already in use; please choose another." The goals here: The user may not remember all the previous titles he's used. Using "unique" starts forcing him to remember. We shouldn't do that. The user already knows he entered it - we don't have to repeat that to him. It's active, not passive (no use of "have"). Use a semi-colon, not a comma. It's much shorter and concise. ------------------------------------------------------------------------ April 9, 2005 - 07:27 : chx Attachment: http://drupal.org/files/issues/box_title.patch (1.49 KB) Why display any message at all? ------------------------------------------------------------------------ April 9, 2005 - 15:11 : Morbus Iff Displaying no message at all would only work if the user wasn't typing in ANY title whatsoever (since then we'd make one for them based on the date). But, if a user names a block "Users" one week, and then creates another block called "Users" three weeks later, than an whoopsidaisy error needs to be shown - we shouldn't modify the text randomly. ------------------------------------------------------------------------ April 9, 2005 - 16:10 : chx Well, let's discuss the main point: why shoiuld be the title unique? ------------------------------------------------------------------------ April 9, 2005 - 16:38 : Morbus Iff Are you asking why Drupal enforces them being unique, or why, visually and to the end-user, they should be unique? ------------------------------------------------------------------------ April 9, 2005 - 16:43 : chx I guess Drupal forces them 'cos for some reasons it is thought that for the users they should be unique. This is not the case, however. Yes, it's confusing if they are on the same page. But I can think on circumstances where they would not be on the same page, so the force from Drupal seems unnecessary. ------------------------------------------------------------------------ April 9, 2005 - 20:00 : robertDouglass agree with chx on the unique point. Like the functionality of adding html comments and timestamp (why timestamp? why not a block id?) if left blank if they must be uniqe. Would be in favor of making the visible title completely optional. Administrators have description to keep them separate on the admin screen. ------------------------------------------------------------------------ April 9, 2005 - 20:48 : Robin Monks Attachment: http://drupal.org/files/issues/drupal_0.patch (3.67 KB) This new patch no longer requires a unique value for title, and also checks to ensure the Block Description is not a duplicate or empty value. Big thanks go out to chx for assistance. Robin ------------------------------------------------------------------------ April 11, 2005 - 15:20 : Anonymous Shouldn't we add a test too to check info unicity ? ------------------------------------------------------------------------ April 11, 2005 - 15:38 : chx <?php if (empty($edit['info']) || db_num_rows(db_query("SELECT info FROM {boxes} WHERE info='%s'", $edit['info']))) { ?> To me, this seems like a check for info unicity... not to mention that the UNIQUE(info) DB constraint is not touched. ------------------------------------------------------------------------ April 11, 2005 - 15:46 : tostinni Ups, sorry for the anonymous and the message, I didn't saw this verification, I was only remembering block.module code. That's all fine for me. ------------------------------------------------------------------------ April 11, 2005 - 17:27 : njivy +1 to Robin Monks' patch. When I install Drupal, the first thing I always do is remove the /unique key/ constraint on block titles. I have not tested the other parts of Monks' patch, though, including the uniqueness constraint on block descriptions. ------------------------------------------------------------------------ April 11, 2005 - 20:04 : drumm Code looks okay. +1 "You must enter unique data into the block description field." doesn't sound like the friendliest of error messages. ------------------------------------------------------------------------ April 11, 2005 - 20:16 : Dries I'm OK with this patch but suggest that we look into improving the error message. Small nit: fix the spacing around '=' in the SQL query that checks for duplicates. ------------------------------------------------------------------------ April 12, 2005 - 00:40 : Robin Monks Attachment: http://drupal.org/files/issues/drupal_1.patch (3.97 KB) Here is a new patch with the changes and with the updates.inc file changed to reflect patchs applied after the original patch was made. Robin ------------------------------------------------------------------------ April 12, 2005 - 19:52 : Dries Committed to HEAD. Thanks. The patch did not apply against DRUPAL-4-6. Possibly needs to get backported. ------------------------------------------------------------------------ April 13, 2005 - 16:36 : Robin Monks Attachment: http://drupal.org/files/issues/4.6 blank title (3.63 KB) Here is the patch backported for 4.6. Robin MozNetwork.org ------------------------------------------------------------------------ April 13, 2005 - 16:45 : Robin Monks Attachment: http://drupal.org/files/issues/4_0.6 blank title (3.62 KB) That patch had a bug in it, new patch attached. Robin

1 0

[drupal-devel] [feature] Request New Password Security
by jacusah 14 Apr '05

14 Apr '05

Issue status update for http://drupal.org/node/18719 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: critical Assigned to: Anonymous Reported by: neofactor Updated by: jacusah Status: patch Will this patch be added to the core? jacusah Previous comments: ------------------------------------------------------------------------ March 11, 2005 - 04:52 : neofactor Problem: Any user can force another user's password to change... simply by selecting "request new password" and putting in their username. The user gets an email with the new.. but this feels like a violation to the user... and a pain. Solution? If someone requests a new password... Don't blindly change it... send an email that says...."Is this a real request authorized by you? Click here to confirm otherwise disregard this message" Please consider this critical for user by-in to Drupal as a secure system. I appreciate your consideration. http://drupal.org/node/18689 ------------------------------------------------------------------------ March 11, 2005 - 06:05 : neofactor I added some code to prevent the admin account from being reset... Please add as a patch. if ($account->uid == 1 { unset($account); form_set_error('name', t('Sorry. The username %name is not allowed to be changed.', array('%name' => '<em>'. $edit['name'] .'</em>'))); } // Just above this code on line 911: if ($account) { $from = variable_get('site_mail', ini_get('sendmail_from')); $pass = user_password(); ------------------------------------------------------------------------ March 11, 2005 - 06:08 : killes(a)www.drop.org Please only set to "patch" if you are actually submitting one. The solution you proposed is rather a workaround and will not be acceptable for core. ------------------------------------------------------------------------ March 11, 2005 - 10:21 : Deno I was in charge of a web site with >10000 paying customers for some time, and the "customers care" was constantly troubled with pasword change requests, until we switched to a system that worked in the following way: 1) "I forgot the password" function accepts either users email, or users login name, generates a random phrase, and adds an entry to "pasword_change_request" table. This entry consists of: - User ID (corresponding to email or login) - random phrase - datestamp User ID is unique key, and the entries in this table older than MAX_TIME are regularly purged by a cron job. 2) System sends an email with a link to "change forgotten password" page with this random phrase as argument to the user. That would be something like: https://drupal.org/user/cfp/d438rwiuodw894732ehdw 3) When user follows the link, "cfp" page checks if the random phrase exists in the table, allows the user to immediately change the password, and sends another email with "your password was changed on RIGHT_NOW ... " note to the user in question. - Password change automatically triggers deleting of the corresponding entry in pasword_change_request table. - Password change automatically logs-in the user as well. This way, users weren't bothered by accidental password changes, and they also understood the system better than the one with automatically generated passwords. For additional security, we also built some brakes that assured that the number of attempts to change the password, such as: - "An email with instructions was sent to you X minutes ago. In case you don't receive it within N hours, please contact the site administrator" - "According to our logs, you have visited the CFP page more than NMAX times within last 24 hours. For security reasons, you will be denied access to this page for the next 24 hours. Please contact the site administrator." hope this helps... ------------------------------------------------------------------------ March 30, 2005 - 22:01 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login.patch (6.52 KB) This patch for -cvs- addresses this issue. It generates an *only one use* URL to login into user account, instead of a new password. Then the user clicks on the URL, logs in, and may change his old password if desired. It is a only one use URL, and has some additional security measures, like a configurable time-out -can be confgured in admin/user/settings- and also, if somebody uses it or password is changed in the meanwhile, previously generated login URLs of this type won't work. About how it works, it creates a new url with: - user id, - timestamp - a hash of current timestamp, changed date for account, and the old password hashed With this patch, this wont work for admin accounts, which I think is better, you never know... It has a number of features like: - User can be ignoring these emails if somebody else is requesting access, a new password request wont change current password. However, the user will be warned when seeing the emails - It reveals no sensible information about the user account in the URL itself - the generated hash is not predictable at all if you dont know the old password, as it depends on user's old password - there's a configurable timeout - The URL's are only one use, as they depend on two timestamps, one of which will be changing in case this login is used -or in case user logs in for other means - Any 'new password' request is independent of others, so no one can be interfering with one user asking for his password back. Also doesn't require any new db field nor keeping track of requested passwords. ------------------------------------------------------------------------ March 30, 2005 - 22:41 : chx Very nice patch. Please consider for 4.6. ------------------------------------------------------------------------ March 31, 2005 - 00:16 : drumm Couldn't those nested if statements be made into one with a bit cleaner indentation? It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? The url is quite long, what could be done to make it shorter? ------------------------------------------------------------------------ March 31, 2005 - 01:50 : Jose A Reyero /Couldn't those nested if statements be made into one with a bit cleaner indentation?/ Maybe, but I had to split that if conditions in two parts because the $account var didn't get the value before the rest of the conditions -latest two- where evaluated. Someone can help with PHP here? /It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting?/ Agreed, will be polishing this a little bit, just wating for some more suggestions... For the duration, I'll change to 'hours', ok? Just don't like to restrict options using unneccessary dropdowns, it's only intended for administrators anyway. Maybe I add also a '0' for unlimited. Hardcoding it, would need anyway to be mentioned in the module help, so it's about the same overhead, both for coding and for the admin interface, and harcoding in two places in code is not good anyway. And I like having options, am I the only one? / The url is quite long, what could be done to make it shorter?/ Well, the url has to be long, it has neccessary params and an MD5 hash, which yes, could be cut someway, but that adding inneccessary code and also reducing security -though half an MD5 hash may be secure enough- but I see no point in that. ------------------------------------------------------------------------ March 31, 2005 - 03:18 : FactoryJoe(a)civicspacelabs.org "Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? " +1 for making this *not* configurable! C'mon, Drupal should have standards about security -- and if best practices say that, say, 8 hours, is a good amount of time for that URL to be active, then do that. Cut out the confusion and extra brain juice it takes to decide... "Gee, is 4 hours better than 5?" Additionally, there's a simple way to make this workflow better for the user. If you visit an outdated change password URL, you should be told, "Sorry, but that link you followed to change your password has expired. If you would like to reset your password, please fill out this form: [form] If you did not request to change your password, please notify [administrator's name w/ PM link]". This way, if you missed your change password deadline, you can request your password again right from the timeout page. If you didn't request the change, it will be obvious that someone else was trying to change it, and then you can contact the administrator and find out who it was that was messin' with your account. ------------------------------------------------------------------------ March 31, 2005 - 07:55 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_0.patch (4.72 KB) I cleaned up the patch a bit. I hope code style is OK. So may -- there are no checks against the arguments? There is no need. user_load uses %d for 'uid', so that's dealt with. Comparison by < and > means an implicit cast to integer, so $timestamp is also dealt with automatically. I do not think timeout needs to be configured. Please comment on not letting uid 1 using this one. I am not sure whether this is necessary. ------------------------------------------------------------------------ March 31, 2005 - 08:12 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_1.patch (6.21 KB) Implemented Chris' idea, so if you use a one time URl which is expired, you are sent to user/password with a message to ask for another. I changed a few messages and deleted the "not for admin" feature. Do we really want one gazillion support requests "I locked myself out of my site, user/password tells me not for admin, what now"? ------------------------------------------------------------------------ March 31, 2005 - 09:24 : chx One more point on uid 1. Yes, uid 1 can reset his pwd over the database. But then we would need to lock out administer users privileged users. Then administer nodes, 'cos of XSS. Then administer comments... then administer filters... OK, this is pointless. If someone can eavesdrop on your emails, everything is lost, I think. This is not something Drupal shall address. Maybe we could employ a security question -- secret answer pair. ------------------------------------------------------------------------ March 31, 2005 - 12:52 : Bèr Kessels Eventhough I am a less-settings advocate, I think this must remain a setting :). Why? Sites on Drupal differ a lot, and have a very differnet userbase. I run very low level sites, where it sometimes takes days before someone will read his/her mail. But I also know sites (drupal.org) where days would certainly be too long. Sorry Chris et al, I +1 on configurable duration. (hell, we really need some about:config alike screen in drupal) ------------------------------------------------------------------------ March 31, 2005 - 13:46 : Dries IMO, that setting has nothing to do with a site being "low level". If you request a new password, you probably want to login ASAP. I vote against a configuration option. Either way, this patch needs work: 1. We don't glue words together (eg. $hashedpass, $currenttime, $loginurl). Similarly, I suggest to change 'user/newpassword' to 'user/reset' or 'user/request-password'. 2. The patch exposes technical details to the user; terminology like "one time login URL" is likely to confuse many users. 3. The login URL is not clean URL safe, I think. 4. Please rename the variable $pass1 in user_pass_rehash(). 5. The code comments are often inconsistent. Some end with a punctuation, some don't. Some span two lines, others don't. On a related note, can't we reuse this system for the initial password? That is, instead of mailing a password, we mail an URL and force the user to enter a password. ------------------------------------------------------------------------ March 31, 2005 - 19:20 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login_2.patch (12.77 KB) Ok, I've started with chx's reviewed patch, and done most of Dries suggestions Main changes are: ----------------------- - Rewording, variable names, path, etc... as pointed out by Dries. Now it's user/reset. And yes, it was not clean-URL safe; it is now. - Allowed also for admin account, I dont like it though, but seem to be the only one :( - Used also for user registration, but I also kept old style login/password, think it can be some use and anyway, it's up to the site admin to remove it from the welcome e-mail. However it is only for 'no admin approval', see below about this.... - Added back in the config option for time-out, though reworked completely for usability. Default is '1 day' which I think should be reasonable. However, there's no time out for firts time users. About this, read below, please... ----------------------- Using this for first time login has some problems when admin aproval is required, as the hashed pass depends on 'changed' timestamp, and it changes when the admin approves an account. Actually, there's no way to know -or I didn't find it- when it is the first login for an approved account. For no admin approval, I just used 'changed' == 'created' as a criterium. I think we should be separating this changed date and last login date creating a new field in the user table, but that maybe for a future patch. That would also increase security, as the Administrator's info would be more accurate, and 'last login date' could be shown when user logs in, which is also security+ Related with this I've experienced problems, as administrator, when editing user accounts, then last login info for the users gets lost -is reset-... About the config option for time out, though I agree that enforcing security can be good in general, this is one of this cases where more security means less usability. And that decission should be left to Site administrators -each site has its own security requirements-. So I think the target here should be a 'reasonable' default, and then some flexibility for site admins. IMHO this latest patch is quite well balanced. So... everybody happy? ------------------------------------------------------------------------ March 31, 2005 - 21:43 : Dries "So ... everyone happy?" Not quite. The patch is an improvement but it uses tabs, incorrect spacing, incorrect placement of brackets, and does not use theme('placeholder'). If a new database column is required to implement this properly, please add one. This patch won't get committed until it is implemented properly. I still vote to remove the configuration option. We're getting there ... ------------------------------------------------------------------------ April 2, 2005 - 20:45 : factoryjoe I’m going to have to stick by my earlier assertion on this one, and, and, as with most usability issues, none of us are “correct” until we test it 50 times and see whether anyone actually uses the option. Just like with my folksonomy review [1], more options is *not always better* nor does it always help people *get things done*! To further expand this point, Jose says [2]: About the config option for time out, though I agree that enforcing security can be good in general, this is one of this cases where more security means less usability. And that decission should be left to Site administrators -each site has its own security requirements. " But this is a misuse of the term “usability”. What you’re talking about is *functionality*—since usability has more to do with focusing single-mindedly on the task at hand and reducing distraction than it does with providing extraneous options that might be useful for a fraction of all Drupal admins. To speak to Ber’s point [3], I cannot imagine a situation where, if I want to change my password because I’ve forgotten it, that it will take more than a few hours to get the email and fulfill the process. And even still, if it does — for whatever reason — take me a week to get around to resetting my password, then starting the process over again isn’t all that time consuming. Plus, I have the added sense of security that Drupal doesn’t let things like password resetting links stay active forever! So again, I agree that we need a reasonable default—24 hours is very generous for the singular task that we’re trying to serve here, which is, to my knowledge, to make the process for resetting your password easier and more secure. Lastly, about the patch itself: I noticed a couple <em> tags strewn around in lines 38, 43, 54 and 72 (as well as a <strong> on 54). Until we have a style guide for marking up usernames, should we really be using HTML like this? It seems to be that it would be better to use a span with a class of… say… “username” and let the themer determine whether the username should be bold, italicized, red, all caps and so on.*/ [1] http://drupal.org/node/19697#comment-25599 [2] http://drupal.org/node/18719#comment-25480 [3] http://drupal.org/node/18719#comment-25463 ------------------------------------------------------------------------ April 2, 2005 - 22:29 : factoryjoe One other quick question. How does this patch interact with distributed authentication? If I'm logged in with a remote account and ask to reset my password, what happens? ------------------------------------------------------------------------ April 3, 2005 - 11:22 : Dries factoryjoe: you can't reset your password when you are logged in. ------------------------------------------------------------------------ April 3, 2005 - 15:03 : Jose A Reyero About usability I've written this piece [4], so I really prefer to discuss it on the forums... That patch, I'll be cleaning it up a little bit, adding that last login field, and maybe go for that 'reasonable default', which anyway is not worth so much discussion.... [4] http://drupal.org/node/19918 ------------------------------------------------------------------------ April 5, 2005 - 02:20 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login_3.patch (20.26 KB) Ok, lets try to make people happy, which will be good for my karma, I hope so ;-) So, changes: - Added 'lastlogin' field to user table. Now there's one timestamp for when an account is edited and another one for login. Of course, I've also taken care of the user's admin page. - Some small detail like help text for e-mail fields and that. - Now it can be used also for accounts with admin approval. - And yes, removed config option, 1 day time out for password recovery, no time out for first login. By the way, I've also patched db files for PostgreSQL, please someone review it, I don't know too much about it. The default e-mails may need better wording. Specially the one for password recovery which just says 'You have requested a new password', but I dont want users get too worried in case someone else has requested that password and the old one that said something like 'This is your new login information..' which may suggest it has been sent by the admin maybe updating accounts..... Anyway, every site admin can configure that email... As an idea for the future or for someone else - I've had enough for now with logging in 100 times to try this-, now that we can know when it is the first login for an user, a nice welcome message or welcome screen would be great :-) Or if someone wants to go for better security *nix style, some message telling the user when last logged in wouldn't be bad... So... ? ------------------------------------------------------------------------ April 5, 2005 - 02:51 : factoryjoe +1 lastlogin time. Also, I just lost my admin password for WP and these are the emails I received at each stage of the process (email titles in bold): *[FactoryCity] Password Reset* "Someone has asked to reset a password for the login this site http://www.factorycity.net/blog [5] Login: admin To reset your password visit the following address, otherwise just ignore this email and nothing will happen. http://www.factorycity.net/blog/wp-login.php?action=resetpass&key=XXXXXf4xx… " *[FactoryCity] Your new password* " Login: admin Password: XxXxXxX http://www.factorycity.net/blog/wp-login.php " *[FactoryCity] Password Lost/Change* "Password Lost and Changed for user: admin " [5] http://www.factorycity.net/blog ------------------------------------------------------------------------ April 6, 2005 - 21:39 : Dries Good job. Some more remarks: - Don't glue words together. Please use 'login' instead of 'lastlogin'. - Write 'administrator' not 'admin'. - You capitalized some random 'subjects' (eg. those in the form's help text). ------------------------------------------------------------------------ April 7, 2005 - 18:30 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login_4.patch (20.28 KB) Replaced 'lastlogin' by 'login', 'admin' by 'administrator' Dries, about the "You capitalized some random 'subjects'", no clue what you are talking about... ?

1 0

[drupal-devel] [feature] View blocks by roles
by killes 14 Apr '05

14 Apr '05

Issue status update for http://drupal.org/node/12988 Project: Drupal Version: cvs Component: block.module Category: feature requests Priority: normal Assigned to: tostinni Reported by: tostinni Updated by: killes(a)www.drop.org Status: patch Ber: I am not sure how you do want to add visibility per role through a single column in the existing table. Can you elaborate? What I'd like to see would be block IDs that aren't only defined through the modules. Ie like the menu module handles them. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ November 15, 2004 - 23:35 : tostinni Attachment: http://drupal.org/files/issues/block_0_0.patch (5.45 KB) After the release of drumm's work about new block.module in CVS : Make block configuration understandable [1] and another comment : Set blocks to be hidden from anonymous users. [2], I decided to create a little patch to allow use of 'roles' for 'blocks'. So my patch provide a set of checkboxes in block configure (the new field drumm has setting up) that allow roles to see a block. Way it works : 1. If no roles set, block is visible by everybody. If some roles are set, only users with at least one of the roles can see the block. 2. Database : As I'm not sure this block will be included in core because a team is working on it (see Drupal 4.6 roadmap [3] - I will post a comment on civiclabs site), I just set up a new field 'roles' in blocks table. So when you modify the roles of a block, it just update this field with a concatenation of roles_id separated by comma. I'm conscient it's quite ugly, but if this patch find his public, I was thinking in creating a new table block_roles (as done for user_roles). Also update.inc isn't updated for the reasons mentioned above. Well I hope your comments. [1] http://drupal.org/node/11875 [2] http://drupal.org/node/12168 [3] http://drupal.org/node/12202 ------------------------------------------------------------------------ November 16, 2004 - 01:55 : drumm You use user_roles(1) which, if I understand that function correctly, removes the anonymous user role from the list. I think that this should be included. Use case: you want information for non-logged in users which is useless to those who are logged in. Watch your editor's use of tabs instead of spaces. It can and should be included. I haven't started work on this, and it saves me a lot of time if I have patches such as this to review. Why not have all the boxes checked by default to indicate visible to everyone instead of none checked? I would even support adding a validate step to make sure at least one is checked (probably belongs in another patch). I think another table is the way to go. Then you could even write the test for visibility by role to that query in block_list() instead of a loop in php. ------------------------------------------------------------------------ November 16, 2004 - 02:49 : tostinni Hi, Thanks for your feedback. Well a few mistakes (tabs, my editor is now configured, and user_roles(0)) I will correct in next patch. A little comment : for the moment I choosed NULL value for everybody if not, we face a few problems : - if the admin add a new role, it won't be asigned the right to view blocks with the "everybody" grant. - also if people import their blocks or if they apply this patch with a site containing blocks settings, they would be invisble unless explicitly allowed. That's why I considered this option, but if you prefer, I can select all checkboxes. For the validate step, I was thinking of it, but for the moment and the design, It was useless. Regarding of the table, the only issue I was contempling, was the lack of a block_id in blocks table, so I would have to create one. Are you ok with that ? ------------------------------------------------------------------------ March 13, 2005 - 21:37 : killes(a)www.drop.org New patch needed nased on drumm's feedback. ------------------------------------------------------------------------ April 12, 2005 - 01:02 : tostinni Attachment: http://drupal.org/files/issues/block_visibility.patch (8.09 KB) Hi, Sorry for the delay, here is the new patch based on current CVS. *Observations :* - I created a new table : block_visibility that store the visibility for each role / block When you update db scheme it would make visible blocks for all your availables roles, i.e. it changes nothing ;), just set up correctly permissions. - When you configure blocks, you will see a set of checkboxes to allow block to be visible regarding of the user's roles. If no role checked, nobody will see it. *Know bugs (by design ?) :* - When a custom block is created it receives no permissions, it means that it would be invisible unless you configure role in "configure" screen. I'm aware of this problem, but I still don't now how to deal with it. The major issue is that I can't predict which id will be inserted in 'boxes' table as it works with auto_increment / SERIAL columns instead of Drupal own sequence table. We can work around it, but I'm not sure it's a good solution to change this, it would cause a lot of troubles... Maybe look for a db_insert_id function ? Any ideas ? For the moment, the user have to grant permissions to role to be able to see custom block. *To do :* - Add visibility rehash in _block_rehash function. - Change help messages ? - Does my queries look nice ? (It works for sure, but is it a good way to write a join : FROM blocks b LEFT JOIN role r ON 1 = 1 ? ) - Others suggestions ? ------------------------------------------------------------------------ April 12, 2005 - 19:59 : drumm "When a custom block is created it receives no permissions, it means that it would be invisible unless you configure role in "configure" screen. " It should probably display the checkboxes for permissions on the block creation screen. All checkboxes check by default. "The major issue is that I can't predict which id will be inserted in 'boxes' table as it works with auto_increment / SERIAL columns instead of Drupal own sequence table. " It really should use Drupal's sequesnces table. You can safely ignore any auto increment or SERIAL. Just use db_next_id as usual. "- Does my queries look nice ? (It works for sure, but is it a good way to write a join : FROM blocks b LEFT JOIN role r ON 1 = 1 ? ) " I've never seen the INSERT ... SELECT syntax elsewhere in Drupal. This may mean that it is not Postgres-compatible or it is avaided for other reasons. A real join condition should be used, such as r.rid = v.rid. ------------------------------------------------------------------------ April 13, 2005 - 00:51 : tostinni "It should probably display the checkboxes for permissions on the block creation screen. All checkboxes check by default. " I was thinking of this issue, it's just that I didn't find out how to display it without redundancy between form's groups from block_box_form and from block_block, it's done now. "It really should use Drupal's sequesnces table. You can safely ignore any auto increment or SERIAL. Just use db_next_id as usual. " Ok, I will make the changes. "I've never seen the INSERT ... SELECT syntax elsewhere in Drupal. This may mean that it is not Postgres-compatible or it is avaided for other reasons. " Well INSERT ... SELECT syntax is part of SQL ANSI 92 and fully implemented in Postgre. I couldn't find since when, but at least since 6.4 version [4] of Postgre. So I think it's good enough. And maybe if no drupal developer ever used it, it's maybe they never needed it ;) Btw, if there's any limitation not to use it, who can tell me ? "A real join condition should be used, such as r.rid = v.rid. " I've done this on purpose : - If I'm modifying the block options and a new role has been added posterior to the block. Now I got a sort of inconsistance in my block_visibility table, because I miss this new role_id for the (module, delta) tuple. So when I'm reviewing/modifying my block configuration, I need to detect this inconsistence to be able to add this new (module, delta, rid) tuple in block_visibility. This is made doing a JOIN between role and blocks with this strange condition : 1=1 because they don't have a commun column. The other reason I've done it this way is because my MySQL documentation told me to prefer LEFT JOIN to RIGHT JOIN for compatibility. What do you think about it ? Should I use RIGHT JOIN without fear ? It would gives us : SELECT r.rid, v.visible FROM blocks b LEFT JOIN block_visibility v ON v.module = b.module AND v.delta = b.delta RIGHT OUTER JOIN role r ON r.rid = v.rid WHERE b.module = '%s' AND b.delta = '%s' and works the same way. Tomorrow I'll post my changes. [4] http://www.postgresql.org/docs/6.4/static/sql-insert.htm ------------------------------------------------------------------------ April 14, 2005 - 00:47 : tostinni Attachment: http://drupal.org/files/issues/block_visibility_0.patch (9.95 KB) Ok here is the new version including these changes : - based on latest CVS version - I created a new function _block_roles_save to handle saving roles - when a new custom block is created, it automatically selects all availables roles (user can change it) and save it - it also handles drupal sequence system for boxes table to allow insertion of the roles next to the custom block creation Still pending : make a _block_roles_rehash. Well I'm not very sure about this one, because it would generate a lot of queries : number of rows in blocks table x num in role _block_rehash works this way, but is it reasonnable ? If _block_rehash is needed not to display blocks that doesn't exists anymore, _block_roles_rehash is only here to clean the block_visibility table, but there won't be any problem with new or missing roles, block would just respect current roles. What should I do ? ------------------------------------------------------------------------ April 14, 2005 - 11:07 : Bèr Kessels Nice feauture. Nice interface, no clutter. But a big -1 on the way you use the database. This can be done much easier by adding a single column in the existing block table. Why introduce a new table and a lot of extra sql queries, update qeuries etc? Bèr

1 0

[drupal-devel] [feature] View blocks by roles
by Bèr Kessels 14 Apr '05

14 Apr '05

Issue status update for http://drupal.org/node/12988 Project: Drupal Version: cvs Component: block.module Category: feature requests Priority: normal Assigned to: tostinni Reported by: tostinni Updated by: Bèr Kessels Status: patch Nice feauture. Nice interface, no clutter. But a big -1 on the way you use the database. This can be done much easier by adding a single column in the existing block table. Why introduce a new table and a lot of extra sql queries, update qeuries etc? Bèr Bèr Kessels Previous comments: ------------------------------------------------------------------------ November 15, 2004 - 23:35 : tostinni Attachment: http://drupal.org/files/issues/block_0_0.patch (5.45 KB) After the release of drumm's work about new block.module in CVS : Make block configuration understandable [1] and another comment : Set blocks to be hidden from anonymous users. [2], I decided to create a little patch to allow use of 'roles' for 'blocks'. So my patch provide a set of checkboxes in block configure (the new field drumm has setting up) that allow roles to see a block. Way it works : 1. If no roles set, block is visible by everybody. If some roles are set, only users with at least one of the roles can see the block. 2. Database : As I'm not sure this block will be included in core because a team is working on it (see Drupal 4.6 roadmap [3] - I will post a comment on civiclabs site), I just set up a new field 'roles' in blocks table. So when you modify the roles of a block, it just update this field with a concatenation of roles_id separated by comma. I'm conscient it's quite ugly, but if this patch find his public, I was thinking in creating a new table block_roles (as done for user_roles). Also update.inc isn't updated for the reasons mentioned above. Well I hope your comments. [1] http://drupal.org/node/11875 [2] http://drupal.org/node/12168 [3] http://drupal.org/node/12202 ------------------------------------------------------------------------ November 16, 2004 - 01:55 : drumm You use user_roles(1) which, if I understand that function correctly, removes the anonymous user role from the list. I think that this should be included. Use case: you want information for non-logged in users which is useless to those who are logged in. Watch your editor's use of tabs instead of spaces. It can and should be included. I haven't started work on this, and it saves me a lot of time if I have patches such as this to review. Why not have all the boxes checked by default to indicate visible to everyone instead of none checked? I would even support adding a validate step to make sure at least one is checked (probably belongs in another patch). I think another table is the way to go. Then you could even write the test for visibility by role to that query in block_list() instead of a loop in php. ------------------------------------------------------------------------ November 16, 2004 - 02:49 : tostinni Hi, Thanks for your feedback. Well a few mistakes (tabs, my editor is now configured, and user_roles(0)) I will correct in next patch. A little comment : for the moment I choosed NULL value for everybody if not, we face a few problems : - if the admin add a new role, it won't be asigned the right to view blocks with the "everybody" grant. - also if people import their blocks or if they apply this patch with a site containing blocks settings, they would be invisble unless explicitly allowed. That's why I considered this option, but if you prefer, I can select all checkboxes. For the validate step, I was thinking of it, but for the moment and the design, It was useless. Regarding of the table, the only issue I was contempling, was the lack of a block_id in blocks table, so I would have to create one. Are you ok with that ? ------------------------------------------------------------------------ March 13, 2005 - 21:37 : killes(a)www.drop.org New patch needed nased on drumm's feedback. ------------------------------------------------------------------------ April 12, 2005 - 01:02 : tostinni Attachment: http://drupal.org/files/issues/block_visibility.patch (8.09 KB) Hi, Sorry for the delay, here is the new patch based on current CVS. *Observations :* - I created a new table : block_visibility that store the visibility for each role / block When you update db scheme it would make visible blocks for all your availables roles, i.e. it changes nothing ;), just set up correctly permissions. - When you configure blocks, you will see a set of checkboxes to allow block to be visible regarding of the user's roles. If no role checked, nobody will see it. *Know bugs (by design ?) :* - When a custom block is created it receives no permissions, it means that it would be invisible unless you configure role in "configure" screen. I'm aware of this problem, but I still don't now how to deal with it. The major issue is that I can't predict which id will be inserted in 'boxes' table as it works with auto_increment / SERIAL columns instead of Drupal own sequence table. We can work around it, but I'm not sure it's a good solution to change this, it would cause a lot of troubles... Maybe look for a db_insert_id function ? Any ideas ? For the moment, the user have to grant permissions to role to be able to see custom block. *To do :* - Add visibility rehash in _block_rehash function. - Change help messages ? - Does my queries look nice ? (It works for sure, but is it a good way to write a join : FROM blocks b LEFT JOIN role r ON 1 = 1 ? ) - Others suggestions ? ------------------------------------------------------------------------ April 12, 2005 - 19:59 : drumm "When a custom block is created it receives no permissions, it means that it would be invisible unless you configure role in "configure" screen. " It should probably display the checkboxes for permissions on the block creation screen. All checkboxes check by default. "The major issue is that I can't predict which id will be inserted in 'boxes' table as it works with auto_increment / SERIAL columns instead of Drupal own sequence table. " It really should use Drupal's sequesnces table. You can safely ignore any auto increment or SERIAL. Just use db_next_id as usual. "- Does my queries look nice ? (It works for sure, but is it a good way to write a join : FROM blocks b LEFT JOIN role r ON 1 = 1 ? ) " I've never seen the INSERT ... SELECT syntax elsewhere in Drupal. This may mean that it is not Postgres-compatible or it is avaided for other reasons. A real join condition should be used, such as r.rid = v.rid. ------------------------------------------------------------------------ April 13, 2005 - 00:51 : tostinni "It should probably display the checkboxes for permissions on the block creation screen. All checkboxes check by default. " I was thinking of this issue, it's just that I didn't find out how to display it without redundancy between form's groups from block_box_form and from block_block, it's done now. "It really should use Drupal's sequesnces table. You can safely ignore any auto increment or SERIAL. Just use db_next_id as usual. " Ok, I will make the changes. "I've never seen the INSERT ... SELECT syntax elsewhere in Drupal. This may mean that it is not Postgres-compatible or it is avaided for other reasons. " Well INSERT ... SELECT syntax is part of SQL ANSI 92 and fully implemented in Postgre. I couldn't find since when, but at least since 6.4 version [4] of Postgre. So I think it's good enough. And maybe if no drupal developer ever used it, it's maybe they never needed it ;) Btw, if there's any limitation not to use it, who can tell me ? "A real join condition should be used, such as r.rid = v.rid. " I've done this on purpose : - If I'm modifying the block options and a new role has been added posterior to the block. Now I got a sort of inconsistance in my block_visibility table, because I miss this new role_id for the (module, delta) tuple. So when I'm reviewing/modifying my block configuration, I need to detect this inconsistence to be able to add this new (module, delta, rid) tuple in block_visibility. This is made doing a JOIN between role and blocks with this strange condition : 1=1 because they don't have a commun column. The other reason I've done it this way is because my MySQL documentation told me to prefer LEFT JOIN to RIGHT JOIN for compatibility. What do you think about it ? Should I use RIGHT JOIN without fear ? It would gives us : SELECT r.rid, v.visible FROM blocks b LEFT JOIN block_visibility v ON v.module = b.module AND v.delta = b.delta RIGHT OUTER JOIN role r ON r.rid = v.rid WHERE b.module = '%s' AND b.delta = '%s' and works the same way. Tomorrow I'll post my changes. [4] http://www.postgresql.org/docs/6.4/static/sql-insert.htm ------------------------------------------------------------------------ April 14, 2005 - 00:47 : tostinni Attachment: http://drupal.org/files/issues/block_visibility_0.patch (9.95 KB) Ok here is the new version including these changes : - based on latest CVS version - I created a new function _block_roles_save to handle saving roles - when a new custom block is created, it automatically selects all availables roles (user can change it) and save it - it also handles drupal sequence system for boxes table to allow insertion of the roles next to the custom block creation Still pending : make a _block_roles_rehash. Well I'm not very sure about this one, because it would generate a lot of queries : number of rows in blocks table x num in role _block_rehash works this way, but is it reasonnable ? If _block_rehash is needed not to display blocks that doesn't exists anymore, _block_roles_rehash is only here to clean the block_visibility table, but there won't be any problem with new or missing roles, block would just respect current roles. What should I do ?

1 0

[drupal-devel] [feature] gzip compression
by Nick 14 Apr '05

14 Apr '05

Issue status update for http://drupal.org/node/20465 Project: Drupal Version: 4.5.2 Component: base system Category: feature requests Priority: minor Assigned to: Nick(a)drupal.ru Reported by: Nick(a)drupal.ru Updated by: Nick(a)drupal.ru Status: patch Attachment: http://drupal.org/files/issues/gzip.patch.tar.bz2 (6.7 KB) >From changelog for 4.5: "* added support for sending gzip compressed pages." But I don`t get how it swich on. Does it works? Searching drupal.org doesn`t help me. So, I`ve made a small patch. Nick(a)drupal.ru

3 2

[drupal-devel] Okay, I think I borked my CVS...what do I do now?
by Syscrusher 14 Apr '05

14 Apr '05

My original goal was to take the "hof" module from my sandbox/syscrusher/hof and put it into modules/hof for release, based on favorable test reports and no outstanding bugs. After reading the Contributor Guide, I *thought* what I needed to do was to checkout DRUPAL-4-6 branch of contributions, then put my module into there and tag it appropriately. When I did the CVS add, however, CVS reported that the per-directory sticky tag on the parent directory (modules) made the tag redundant. Okay, I thought, I'll just proceed to add the new files. That seemed to work, but they didn't show up on the CVS browser on drupal.org, which thought that the files didn't match any branch tag (nor head -- it said the files were there but didn't match even if I said "show all files"). So I did a CVS remove on that version (the original being still in my sandbox) and recommitted. I went back and re-read the docs, and concluded (apparently wrong) that I really needed to do an explicit "cvs tag -b DRUPAL-4-6" with the files existing physically under contributions/modules in my local copy of HEAD, and that this should make them appear in a checked-out 4.6 local copy. Apparently, I've thoroughly borked this, because I now can't get the files to show up in a 4.6 checkout, but can't get CVS to remove them so I can start over. I'm sorry to be dense here -- I *did* read the docs, but apparently I didn't do something correctly. Can someone suggest what I should do next? Scott -- ------------------------------------------------------------------------------- Scott Courtney Drupal user name: "syscrusher" http://drupal.org/user/9184 scott(a)4th.com Drupal projects: http://drupal.org/project/user/9184 Sandbox: http://cvs.drupal.org/viewcvs/drupal/contributions/sandbox/syscrusher

5 7