- Development - lists.drupal.org

[drupal-devel] [bug] Author information block doesn't show values
by Thox 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/31585 Post a follow up: http://drupal.org/project/comments/add/31585 Project: Drupal Version: cvs Component: profile.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Thox Updated by: Thox Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/profile.module_3.patch (589 bytes) The author information block shows the field names, but not the values. This is a problem in the theme function using $value rather than $field->value. Simple patch attached. Thox

1 0

[drupal-devel] [task] Integrate primary links with menu navigation
by adrian 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/22215 Project: Drupal Version: 4.6.0 Component: theme system Category: tasks Priority: critical Assigned to: Anonymous Reported by: adrian Updated by: adrian Status: patch Attachment: http://drupal.org/files/issues/menu_patch.diff (7.58 KB) Here is the first version of the patch that uses the menu system to generate the primary and secondary navigational links. The navigation menu should by default be used as the source for the primary / secondary navigation (currently only in bluemarine, templates need to be modified to use this functionality). Furthermore, the css used sucks .. and we will need someone to write proper default css for us to make it at least presentable in the absence of theme css. The navlinks aren't really useful if there are more than 5 links in a menu, so admin/ looks utterly horrible. This code is copied and adjusted from Bér's primary links module, and Adrian Simmons cleaned up the CSS a bit. Bér didn't have any css with the module, so I had to rip it from his webschuur.com site. adrian

7 12

[drupal-devel] [bug] Blocks showing up when module disabled
by Uwe Hermann 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/13802 Post a follow up: http://drupal.org/project/comments/add/13802 Project: Drupal Version: cvs Component: module system Category: bug reports Priority: minor Assigned to: drumm Reported by: Jose A Reyero Updated by: Uwe Hermann -Status: patch (code needs work) +Status: patch (code needs review) Patch still applies, but I didn't test it. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Thu, 02 Dec 2004 13:19:31 +0000 : Jose A Reyero This only happens in the module administration page. To reproduce this behaviour: 1. enable a module and one block provided by it - I.e. 'archive' module 2. disable the module The block still shows up in the module administration page I find this more funny than annoying and it happens only in module admin page. Anyway, think it's and undesirable behaviour and should be addressed, maybe at the block management level. ------------------------------------------------------------------------ Thu, 16 Jun 2005 04:37:45 +0000 : deekayen I verified this in HEAD for the archive module. ------------------------------------------------------------------------ Thu, 11 Aug 2005 20:49:44 +0000 : tostinni This still apply to HEAD, the procedure to reproduce the bug is the right one. However, I don't understand exactly why the block keep appears on this page, and not on the others pages. The problem is that looking at the blocks table, "archive" block stay there even if it doesn't appear on other pages (on module page yes it does). When I enter a new page, saying home, archive block isn't there but is still on the db. The cache is flushed, because archive seems not to appear on the entry giving me the menu, but the current page keep the old cache entry (well I guess). Ok so I proposed a radical way to correct this : rehashing blocks after saving the module configuration. So I just add a _block_rehash(); after the menu_rebuild();, it's a little dirty but do the trick. If you have any suggestion about a better way to do this ;) <?php function system_listing_save($edit = array()) { $op = $_POST['op']; $edit = $_POST['edit']; if ($op == t('Save configuration')) { db_query("UPDATE {system} SET status = 0 WHERE type = '%s'", $edit['type']); foreach ($edit['status'] as $name => $status) { // Make certain that the default theme is enabled to avoid user error if (($edit['type'] == 'theme') && ($edit['theme_default'] == $name)) { $status = 1; } db_query("UPDATE {system} SET status = %d, throttle = %d WHERE type = '%s' AND name = '%s'", $status, $edit['throttle'][$name], $edit['type'], $name); } if ($edit['type'] == 'theme') { variable_set('theme_default', $edit['theme_default']); } menu_rebuild(); _block_rehash(); drupal_set_message(t('The configuration options have been saved.')); drupal_goto($_GET['q']); } } ?> ------------------------------------------------------------------------ Thu, 11 Aug 2005 22:17:26 +0000 : drumm I can work on this. ------------------------------------------------------------------------ Fri, 12 Aug 2005 00:20:14 +0000 : drumm Attachment: http://drupal.org/files/issues/system.module_5.diff (3.54 KB) I decided to implement a hook system for this. As a side effect: - system_listing_save() only does database things. The UI things such as drupal_set_message() are now separated into the calling function. - The status message for saving module is now: * The aggregator module has been disabled. * The archive module has been enabled. instead of "The configuration options have been saved." - The theme-specific things in system_listing_save() are not there anymore. - There is also a hook called when modules are enabled. I think I'll go write some documentation for the two new hooks now. ------------------------------------------------------------------------ Fri, 12 Aug 2005 00:48:40 +0000 : drumm Here is some hook documentation: <?php /** * This hook is called on every module for when each new module is enabled. The * module will be loaded at this point so the module's own hook will be called * and other modules may use module_invoke() to call functions in the module. * * @param $module * The name of the module which has just been enabled. */ hook_module_enabled($module) { } /** * This hook is called on every module for when each new module is disabled. * The module will still be loaded at this point so the module's own hook will * be called and other modules may use module_invoke() to call functions in the * module. * * @param $module * The name of the module which has just been disabled. T */ hook_module_disabled($module) { } ?> ------------------------------------------------------------------------ Wed, 07 Sep 2005 19:18:22 +0000 : drumm This patch is now fubar. Please feel free to resubmit the concept in update #2 by tostinni in patch form. ------------------------------------------------------------------------ Wed, 07 Sep 2005 20:17:08 +0000 : tostinni Sorry I didn't understood "This patch is now fubar." ? This patch has been abandonned ? ------------------------------------------------------------------------ Thu, 08 Sep 2005 00:18:24 +0000 : drumm Yes, I am not going to work on my approach currently. The patch was broken by the recent addition of multiple block regions. I may have time in the future. I recomend making a real patch file for the simpler approach as describe in update #2. ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:56:09 +0000 : tostinni Attachment: http://drupal.org/files/issues/system_block_rehash.patch (510 bytes) Ok here goes the patch.

1 0

[drupal-devel] [feature] Single step account creation and login
by Uwe Hermann 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/4942 Post a follow up: http://drupal.org/project/comments/add/4942 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: normal Assigned to: amanuel Reported by: mathias Updated by: Uwe Hermann Status: patch (code needs review) I agree with killes here. Asking the user to choose a password usually results in _very_ insecure passwords. Give them random passwords per default in order to keep most of the accounts secure. If a user then changes the password to his pet's name, that's his problem... Uwe Hermann Previous comments: ------------------------------------------------------------------------ Sat, 03 Jan 2004 23:49:54 +0000 : mathias Under special circumstances, i would like a new user to be able to choose their own password and be automatically logged into the site without needing to check their email. I feel this is necessary for my ecommerce module, where after anonymous users are ready to "Proceed to checkout", they need to create an user account to continue. It is tedious for the customer if they have to stop, check their email, log into the site and then resume the checkout process. I'm game to code this (if this has potential to be part of the core), i just need some feedback on the best way to tweak user_register. ------------------------------------------------------------------------ Sun, 04 Jan 2004 06:18:07 +0000 : moshe weitzman +1 for this. I don't think you will get much objection to this much needed usability enhancement. ------------------------------------------------------------------------ Thu, 08 Jan 2004 05:48:40 +0000 : mathias Attachment: http://drupal.org/files/issues/user.module-quick_acount.patch (7.11 KB) This is a prototype patch to make sure i'm going in the right direction. I feel this patch is about 95% complete, the outstanding issue being page redirection after quick-account creation. It introduces and new option under the user admin settings "Public registrations" called "Visitors can create accounts and sign in immediately.", allowing visitors to submit their own passwords upon account creation. This is important in an ecommerce site where a new customer wants to the checkout process to be as easy and seamless as possible. ------------------------------------------------------------------------ Thu, 08 Jan 2004 10:22:09 +0000 : moshe weitzman i read through the patch. looks good to me. a few notes - we should still send a password via email to users who choose their ow password. with some text tweaking, we should be able to send the same welcome email to 'generated password' registratants, and 'user specified password' registrants. - I think we need a setting for minimum length of a password. the password textfield should inform users of this requirement. - you don't actually relinquish control after saying "/* Let the developer control where the user is redirected. */". i assume this part isn't finished yet. nice work. ------------------------------------------------------------------------ Thu, 08 Jan 2004 13:46:07 +0000 : flevour Are there any anti-bot checks around, e.g. randomly generated images that contain text or numbers to insert in a box? Congrats for your work :p // flevour ------------------------------------------------------------------------ Thu, 08 Jan 2004 13:54:19 +0000 : Bèr Kessels I had another idea. that would be to filter all emailadresses (in content too) into a link to the feedback module. thus http://www.mysite.org/feedback/mailto/me/mysite.org the feedback can then print a form that can send the message to me(a)mysite.org. Would this be a good feature or not? Ber ------------------------------------------------------------------------ Thu, 08 Jan 2004 13:56:36 +0000 : Bèr Kessels sorry folks. placed this in the wrong box. Was updating another feature, and reading this one (to see if it was the same one) i then, by accident, filled replyed here. :( Ber ------------------------------------------------------------------------ Thu, 08 Jan 2004 16:49:56 +0000 : Dries Showing an image with random generated text that a user is supposed to copy, makes your website inaccessible for visually impaired: they can't be read by a screen reader. ------------------------------------------------------------------------ Thu, 08 Jan 2004 17:07:02 +0000 : mathias Responding to Moshe's comments: - we should still send a password via email to users who choose their own password. with some text tweaking, we should be able to send the same welcome email to 'generated password' registratants, and 'user specified password' registrants. That is a good point. I'll update the patch. - I think we need a setting for minimum length of a password. the password textfield should inform users of this requirement. Agreed. In my patch i checked to make sure the password was at least six characters long, but this should be an element that can be tweaked by the admin. It might be best to make this a global password length system variable. - you don't actually relinquish control after saying "/* Let the developer control where the user is redirected. */". i assume this part isn't finished yet. Yep, that's the part i'm still working on. Thanks for the critical eyes. ------------------------------------------------------------------------ Wed, 14 Jan 2004 04:31:06 +0000 : mathias Attachment: http://drupal.org/files/issues/user_0.module-quick_acount.patch (7.95 KB) The patch has been updated and is ready for final review, and commit. ------------------------------------------------------------------------ Wed, 14 Jan 2004 22:37:53 +0000 : Dries Not sure. Wouldn't it make more sense (and result in better/less code) to let the user *always* choose his password and to introduce a admin setting to control whether e-mail addresses should be validated by e-mail? ------------------------------------------------------------------------ Sun, 15 Feb 2004 17:02:38 +0000 : moshe weitzman moving out of patch queue until a cleaner implementation is submitted ... this feature is still quite valuable. ------------------------------------------------------------------------ Fri, 05 Nov 2004 19:21:49 +0000 : Nick Nassar I agree with Dries that it makes a lot of sense to always let the user choose her password. It's a pain to copy and paste in a randomly generated password, then change it. Hash link based verification is much easier. That's really a seperate issue from an option to disable verification. ------------------------------------------------------------------------ Fri, 22 Jul 2005 22:11:42 +0000 : amanuel Attachment: http://drupal.org/files/issues/user4.6.2.patch (4.25 KB) Following Dries's suggestion, I have implemented a "Enable Email Verification" option to user.module. The attached patch does the job. With this patch the system by default will ask for a password. If Email Verification is turned on in the settings, the system will send the password via email. $edit['destination'] is carried so as to allow the user to return where they were (shopping cart etc.) Any comments? Amanuel ------------------------------------------------------------------------ Sat, 23 Jul 2005 05:48:08 +0000 : mathias I want users to be able to enter their own passwords upon account creation, but this patch still needs some work. - The email verification checkbox in user admin settings is confusing. I'm assuming that it applies to any of the selected registration options? However when I select that only site admins can create new user accounts, the accounts I create don't get any emails sent for the user to verify. - When a user signs up and enters his/her own password I think they should be logged in automatically rather than taken to a screen asking them to click the login button. - User entered passwords aren't validated. We should check to make sure they're at least six characters and verify the password strength level to some degree (i.e. same characters, all lowercase letters.) ------------------------------------------------------------------------ Fri, 29 Jul 2005 05:09:30 +0000 : Steven I agree with Dries. Random-generated passwords are hard to use. We already have optional hash-link functionality on signup, so I think we should always use it. ------------------------------------------------------------------------ Sun, 07 Aug 2005 17:02:45 +0000 : killes(a)www.drop.org I actually disagree with Dries and Steven. I let firefox maintain all my passwords and couldn't care less what my actual pw for any Drupal based website is. If we let the user provide a password then I at least woudl want to havd Drupal suggest one for me.

1 0

[drupal-devel] [bug] missing vid field in the forum table
by crunchywelch 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/30344 Post a follow up: http://drupal.org/project/comments/add/30344 Project: Drupal Version: cvs Component: database system Category: bug reports Priority: critical Assigned to: Anonymous Reported by: crunchywelch Updated by: crunchywelch Status: patch (code needs review) Attachment: http://drupal.org/files/issues/database.mysql_6.patch (1.07 KB) The database.* files do not contain the vid field in the forum table, causing forum module to break when submitting forum topics. Also, database.pgsql contains an old field called 'shadow' in the forum table. These patches fix this problem. crunchywelch

3 6

[drupal-devel] [bug] PostgreSQL Socket Connection
by iamtheari 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/26836 Post a follow up: http://drupal.org/project/comments/add/26836 Project: Drupal Version: 4.6.2 Component: postgresql database Category: bug reports Priority: normal Assigned to: Anonymous Reported by: iamtheari Updated by: iamtheari Status: patch Attachment: http://drupal.org/files/issues/drupal-pgsql-socket.patch (917 bytes) Drupal does not allow for PostgreSQL connections via unix socket. I have written a patch that allows it to do so. The effect of this patch is that a hostname of 'localhost' in a pgsql:// $db_url will use Unix sockets unless a port it set in the URL. A hostname of '127.0.0.1' will use TCP/IP as before. Examples: Unix socket: pgsql://user:pass@localhost/db TCP/IP: pgsql://user:pass@localhost:4444/db TCP/IP: pgsql://user:pass@127.0.0.1/db TCP/IP: pgsql://user:pass@127.0.0.1:4444/db The patch is attached to this bug report. From a top-level Drupal directory, type 'patch -p0 < /path/to/drupal-pgsql-socket.patch' to apply it (omit the single-quotes). iamtheari

5 13

[drupal-devel] [feature] Themeable profile list patch for profile.module
by Uwe Hermann 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/20479 Post a follow up: http://drupal.org/project/comments/add/20479 Project: Drupal Version: cvs Component: profile.module Category: feature requests Priority: normal Assigned to: vfilby Reported by: vfilby Updated by: Uwe Hermann -Status: patch (code needs review) +Status: patch (code needs work) Patch doesn't apply any more. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Thu, 14 Apr 2005 03:16:04 +0000 : vfilby Attachment: http://drupal.org/files/issues/profile-themeable-user-list.patch (2.08 KB) Here is a patch for the profile module that provides a way to theme user lists. I simply took the last part of the giant if in profile_browse, moved it to a new function called theme_profile_list and mde profile_browse call the new function. ------------------------------------------------------------------------ Thu, 30 Jun 2005 04:52:17 +0000 : moshe weitzman looka useful ... please provide unified diff format. as per http://drupal.org/handbook ------------------------------------------------------------------------ Thu, 07 Jul 2005 10:39:32 +0000 : robertDouglass It doesn't really look suitable for theming, though, due to all the database logic. To make this really a theme function, you'd have to prepare the data into some variables that you can get by calling function A, and make a theme function B that outputs the variables. Things that can't be in theme functions include SQL and user access logic. ------------------------------------------------------------------------ Thu, 04 Aug 2005 03:58:09 +0000 : ejort Attachment: http://drupal.org/files/issues/profile.module.theme_profile_list.patch (2.17 KB) Hi, This is a patch providing the same thing, but without taking the database code into the theme_profile_list function. I've had this running on one of my sites [1] for a while and found it necessary to output a table listing member contact details (note: there aren't many profile fields accessible by anonymous users in that example, but you get the idea). Unfortunately I never got around to separating it from my other profile.module modifications until now. Let me know what you think. Cheers, Eric [1] http://www.renegades.com.au/?q=members/contact-lists/all-men

1 0

[drupal-devel] [bug] blocked users still have profile link (which doesn't work)
by Uwe Hermann 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/30134 Post a follow up: http://drupal.org/project/comments/add/30134 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: moshe weitzman Updated by: Uwe Hermann -Status: patch (code needs review) +Status: patch (code needs work) Patch applies, but needs more work IMHO. The "Submitted by" link is still there if the user is blocked. The same is true for the content list in "admin/node", the link to disabled users is still there. There are probably more occurences... Uwe Hermann Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 20:47:18 +0000 : moshe weitzman a blocked user has a regular profile link issued by theme('username'). the problem is that if you follow the link, you get 'page not found' error. ------------------------------------------------------------------------ Wed, 31 Aug 2005 20:53:37 +0000 : killes(a)www.drop.org In which way would a fix be desirable: a) not display a link b) make profile display work? ------------------------------------------------------------------------ Wed, 31 Aug 2005 20:58:27 +0000 : moshe weitzman a) - one of the main reasons for blocking a user is that they post inappropriate content in their profile ------------------------------------------------------------------------ Wed, 31 Aug 2005 21:06:10 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/username.patch (628 bytes) Makes sense, care to try this patch?

1 0

[drupal-devel] [feature] Localize date formats
by Uwe Hermann 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/11623 Post a follow up: http://drupal.org/project/comments/add/11623 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: pancho Reported by: Goba Updated by: Uwe Hermann -Status: patch (code needs review) +Status: patch (code needs work) Doesn't apply anymore. Also, please provide _one_ patch (not one for each file you change). Uwe Hermann Previous comments: ------------------------------------------------------------------------ Fri, 15 Oct 2004 18:52:24 +0000 : Goba Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats.patch (1.99 KB) Currently if I set a date format for the site, then it will be used with all interface translations. Different languages however have different needs when presenting dates. The Hungarian date format does not allow the day name to be the first and it calls for a dot between the hour and minute (yes, a dot not a colon). This means that Drupal ships with no suitable date format for Hungarians. Also none of the Hungarian date formats will be suitable on an English interface, while none of the English date formats are acceptable on a Hungarian interface on a bilingual site. The only option is to make date formats translateable. Since these are dynamic t() calls, extractor.php needs to be extended to include the date formats shipped with drupal in the pot output, so translators will get them (much like we do for some numbers and day names). I will take care of extending extractor.php, once this patch gets applied. ------------------------------------------------------------------------ Fri, 15 Oct 2004 19:00:27 +0000 : Goba Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats_0.patch (1.99 KB) Well, the t() needs to be called on the date format set by the admin, so I needed to fix three t() calls. Updated patch attached. ------------------------------------------------------------------------ Fri, 15 Oct 2004 19:01:56 +0000 : killes(a)www.drop.org *g* I recall that there was a time when you didn't like this approach. Anyway, I fully endorse it and would like to see it in 4.5. There is also no suitable data format for German in core. And yes, that's a bug not a feature request. ------------------------------------------------------------------------ Fri, 15 Oct 2004 19:05:33 +0000 : Goba Yes, I didn't like it, because these are dynamic t()s. But since the selected format always needs to be presented in the actual language, it needs to be dynamic by nature. We have some of these dynamic t()s already, handled specially in extractor.php. ------------------------------------------------------------------------ Fri, 15 Oct 2004 23:07:29 +0000 : Steven I like this idea a lot, but I don't like the simplistic "wrap it all in t" feature. There are instances where format_date output is intended for script output. Changing those date formats would mean a bug in the code or UI (for example the way a node date is specified and parsed). Only those strings that are intended for user consumption should be translated. That's why I think we need a more specialized date handling component. It would deal with stuff like variations ('show hour/hide hour'), long/short variations (we need to review the usage of small/medium/long), as well as localizing them. We would need to revise the formats shipped with Drupal, but this gets tricky. For example would we only ship US-style AM/PM date formats with core? Should we only have M/D/Y ordering, or also D/M/Y ordering? If we move the time format settings to locale (where they do below, in principle) we might get a drop in usability: moving from a simple selection box, to requiring the whole localisation framework 'just to switch from 12 hours to 24 hours'. In some parts of the world, there is only one format for dates (e.g. chinese/japanese 2004年10月16日 1時00分). For them, the only variations are long vs short (whether to show the year, the time, etc), in other parts of the world all bets are off and people use what they like. This would mean that if we provide a default set of formats, and let them be translated through PO files, some languages might require more items than provided and some languages would have to use duplicates to fill all available options. On top of that, there is no guarantee that format #3 in French matches format #3 in German, so you'd need to be able to specify this per language. Ideally, we would let a language define any number of date formats, and let the user choose one of those per language. We could also provide a custom option where you enter your own date format, for the realy picky ones. The question remains: what do we do for sites without locale enabled? We can't go to requiring locale just to switch the date on a non-localized site. In that case, perhaps we could do this: - Have locale.module deal with date formats when it is enabled. Have a per-language format setting, selected from a language-specific, po-defined date format list. - Have system.module implement a 'localization' admin menu item if locale.module is /not/ enabled. This page carries the old selector, which acts like a simplified version of locale's format selector, with the list we current have in Drupal (i.e. variations of 24/12hours, d/m/y, y/m/d, m/d/y, ...). To prevent confusion, we add a link to system.module's version of the localization page, which instructs users to enable locale.module for full localization abilities). I know it sounds a bit weird (we certainly don't do this anywhere else), but it would be consistent in terms of UI (the date format is "localization" no matter how you look at it) and keep both non-localized site admins and localized site admins happy, without sacrificing too much ease of set up. ------------------------------------------------------------------------ Sat, 16 Oct 2004 09:34:09 +0000 : Goba Steven, you say: "/There are instances where format_date output is intended for script output. Changing those date formats would mean a bug in the code or UI/ " But the current approach is to translate the month and day names in every case, so format_date **already** assumes that the date it outputs is for human consumption and not for machines. So changing the order of the fields in user date formats should not hurt. Sure it might not be a useability dream, but it does not hurt. By the way, adjusting date formats based on user language is only a small piece of localisation not covered by Drupal core yet. I need to use one mission statement and site name in all languages, and even my primary and secondary links need to have the same text in all site languages, since they are not translateable. If we are about to find a generic solution for locale covered settings, we need to take these into account too. ------------------------------------------------------------------------ Fri, 29 Oct 2004 00:10:44 +0000 : Steven ""so format_date *already* assumes that the date it outputs is for human consumption and not for machines." " What I mean is changing the placement of the date components (day, month, year). For numerical dates which are parsed by a program, this will introduce bugs. I'd much rather have a cleaner date format selection method integrated with locale. ------------------------------------------------------------------------ Fri, 29 Oct 2004 13:19:01 +0000 : Goba Steven, the custom date formats are not translated, only those selected by the admin. And the admin can select date types with spelled out month/day names at any level, so they are not machine parsable anyway in the current Drupal version. **This patch does not introduce bugs.** I admit it does not look good, but there is no system for locale dependant admin settings yet. ------------------------------------------------------------------------ Fri, 29 Oct 2004 15:05:28 +0000 : Steven I'm saying we do need such a system. With your patch, the format that the admin selects in language A will not necessarily match the one in language B. We should go for locale-defined formats with a locale-neutral list to fallback on. ------------------------------------------------------------------------ Sun, 13 Mar 2005 14:55:22 +0000 : pancho Attachment: http://drupal.org/files/issues/dateformat.patch.zip (14.91 KB) ************************ DATEFORMAT-PATCH PACKAGE ************************ by pancho(a)suenderhauf.net 2005-03-13 03:07 This 'DateFormat' patch package only works with the latest CVS version of Drupal 4.60 RC. This 'DateFormat' patch package applies the following changes: 1. It introduces a more flexible system of standard date and time formats which helps both users and developers while staying backwards-compatible. We often see special date formats be used in modules, which is only second best, as these can neither be adapted by the user nor localized. More flexibility allows the developers to use the built-in format functions wherever possible. Changes in detail: - The standard date formats used to be combined date&time-formats. This is now split into separate date formats and time formats, to allow dates and times to be rendered in a more flexible way. - Still it's backward compatible: the combined formats - while deprecated - still exist. - A "Tiny" format is added to the "Small", "Medium" and "Large" formats. 2. It enables Drupal to show e.g. German and Hungarian date formats (with a colon as separator), which was _not_ possible before. 3. It prepares both the Drupal system and module developers for the upcoming locale_dateformat module. This module will fully localize date and time formats according to the localization of the current user. The locale_dateformat module will be available within the next weeks. I'm always happy about suggestions and criticism, if constructive. You can either write a comment on www.cvs.net or an email to pancho(a)suenderhauf.net. Bye, Pancho ------------- INSTALLATION: ------------- The package contains two patches for the latest CVS versions of Drupal 4.60 RC, both of which should be applied: 1. common.inc (patch for v.1.429) 2. system.module (patch for v.1.197) There are no changes in database structure. ------------------------------------------------------------------------ Sun, 13 Mar 2005 14:59:33 +0000 : pancho P.S. What the hell did I write about www.cvs.net? No, what I meant was of course www.drupal.org (here), where you can write a comment! ------------------------------------------------------------------------ Sun, 13 Mar 2005 15:08:35 +0000 : pancho I feel better if I refile the thread as a feature request. The only thing you could describe as a bug was that German & Hungarian date formats were not supported. The rest are definitely feature improvements. ------------------------------------------------------------------------ Sun, 13 Mar 2005 16:38:02 +0000 : Steven - Localizing date formats seems like something that should be solved properly in core once and tied into locale.module. We need a solution which does not pose problems with mixing languages, and which does not require us to keep adding more and more date formats to the list (e.g. what about asian date formats? They don't have spaces.) Your "if (!module_exist('locale_dateformat'))" solution is unacceptable for core. - What's the point of the 'tiny' date format if it's not used anywhere? Where would it be used? - Please don't post patches as zips, this is annoying. Just make one global patch file instead of separate ones. Multiple files can be posted with multiple follow ups. - You need to see how this affects the date usage around Drupal, e.g. the profile date field formatting, which depends on the date format being in a specific format. - Code style: check spaces around quotes. Also, avoid trailing spaces on lines, this is untidy. - This issue is not "critical" IMO. It is a new feature request, and as such should probably wait until after 4.6, as 4.6 is in feature freeze. - The apostrophes around the separator item seem a bit unnecessary. ------------------------------------------------------------------------ Mon, 14 Mar 2005 17:08:34 +0000 : pancho */First of all: This patch works! Even it may be not yet ready to be adopted in the core, it is good for individual use./* To Steven: Okay, Steven, I was asking for criticism... :) With some of the points you make I can agree: - This feature should be completely and properly rewritten and not just fixed to have a good solution for a longer period and not only for now. - The "if (!module_exist('locale_dateformat'))" solution is not good code style at least for core, as well as the spaces. I didn't take enough care about that, as I wanted to present a solution now, before the release of 4.6. - Sorry about the zipped patches. I didn't expect it to bother anyone. But on the other hand: - It's not about adding more and more date formats. You will agree that the German web is a big enough market which should not be left aside anymore. Therefore the feature request /is critical/ from my perspective at least. And the solution I presented doesn't blow up the date format lists, in the contrary it makes them easier to use. - How can you complain about the tiny version not to be used. If it doesn't exist it /can't be used/. - The apostrophees do make sense, because the separator can also be a single space. Nobody would identify that in the menu without apostrophees. So you were critizising some points which I will fix in a new version of the patch as soon as possible. But you didn't say anything about the split of date and time formats. To me it makes sense, as there have already been more people asking for "just date" or "just time". While I already started the development of a "locale_dateformat.module", I will put this on hold, until we figured out a good solution altogether. Anyway, I'm ready to help developing this feature within "locale.module" if I'm being asked. Otherwise developing a separate module would at least offer the so needed functionality /within this century/ ;-) Pancho ------------------------------------------------------------------------ Fri, 18 Mar 2005 07:50:14 +0000 : Steven Firstly: "Even it may be not yet ready to be adopted in the core, it is good for individual use. " The patch queue is really part of the Drupal development process. It does not help us much to work on patches that are not intended for core. "It's not about adding more and more date formats. You will agree that the German web is a big enough market which should not be left aside anymore. Therefore the feature request is critical from my perspective at least. And the solution I presented doesn't blow up the date format lists, in the contrary it makes them easier to use. " I'm just saying, that solutions like the German date format (which has a special notation for the day numbers) or Asian formats (which have no spaces and reversed order) should really be handled through locale IMO, where date formats are picked per language, and can be added manually or through .po files. "How can you complain about the tiny version not to be used. If it doesn't exist it can't be used. " I imagine you introduced this format for a reason, no? Adding new features without knowing exactly where they should be used is silly. If you add a new feature in a patch, you should put in the work to see that this feature is actually used. I didn't really talk about splitting date/time formats for now, as it is mostly a usability thing. I'd much rather wait for a solution integrated with locale before I comment on it. ------------------------------------------------------------------------ Wed, 27 Jul 2005 16:32:19 +0000 : Bèr Kessels +1 for the idea. +1 for the implementation. The idea is simple: just pull the format string trough the locales. No new interfaces (+1), no new configuration options (+1) and no additional cruft. po files will then take care of the real format. However, I see one issue: we should remove the dopdowns in /admin/settings. The patch still applies.

1 0

[drupal-devel] [feature] Comment approval queue a pain to use
by Uwe Hermann 19 Sep '05

19 Sep '05

Issue status update for http://drupal.org/node/28595 Post a follow up: http://drupal.org/project/comments/add/28595 Project: Drupal Version: cvs Component: comment.module Category: feature requests Priority: normal Assigned to: mrowe Reported by: robertDouglass Updated by: Uwe Hermann Status: patch (code needs review) Attachment: http://drupal.org/files/issues/comment_bulk_approval_2.patch (2.45 KB) Rerolled patch and fixed a few issues: Using ' instead of " now where possible, made "Save changes" translatable, use %d more often, indent code using 2 spaces (not 4). The functionality works as expected and is quite an improvement over the current stuff. +1 from me. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Wed, 10 Aug 2005 08:29:14 +0000 : robertDouglass When using the comment approval queue the workflow for approving comments is awful. You have to click administer->comments->approval queue to get a list of comments awaiting approval. Then you have to click edit on one of the comments, click the collabsible field-set for the publishing controls, click to published and save the comment. Then repeat the process for the next comment. I'd say that this workflow is a significant deterrent to anyone using this functionality. To make it better, there would be checkboxes next to each comment in the table on the approval queue page, and a set of operators analogous to the content overview page. Better yet would be if each comment in the table was an AJAX link to the comment's body and info which would expand right there in the table itself, since it is usually impossible just from the comment title to know whether to publish or not. If one could click the various comments open, right there in the table, then mass publish or delete, we'd have a real approval queue. Perhaps the spam module could tie into the actions that can be taken on comments so a mass "mark as spam" could be done too. ------------------------------------------------------------------------ Wed, 14 Sep 2005 13:32:47 +0000 : mrowe Attachment: http://drupal.org/files/issues/comment_bulk_approval.patch (2.24 KB) Here's a patch (against HEAD) that gives you the cheap-and-dirty approach (checkboxes that allow bulk approval/unapproval of comments). I'll leave the gold-plated AJAX solution to someone with more time. :) ------------------------------------------------------------------------ Wed, 14 Sep 2005 21:17:21 +0000 : Dries - Don't use tabs. - SQL queries might be vularnable to SQL injection attacks. Use db_query()'s %s and %d directives. ------------------------------------------------------------------------ Thu, 15 Sep 2005 02:24:17 +0000 : mrowe Attachment: http://drupal.org/files/issues/comment_bulk_approval_0.patch (2.38 KB) Ok, I think I got rid of all the tabs this time... I've had to use db_escape_string, rather than the sprintf params to db_query, but it should have the same effect. ------------------------------------------------------------------------ Thu, 15 Sep 2005 03:28:50 +0000 : moshe weitzman minor nits: we use an extra line in our else clauses. this should be two lines: } else { also, you can almost always avoid using db_escape_string() directly. you will need to do a str_repeat('%d, ', count($unpublish)) in the SQL(for unpublish query) and then pass a an array as the last argument to db_query. This is the seldom used 'pass all values in an array' feature of db_query: http://drupaldocs.org/api/head/function/db_query ------------------------------------------------------------------------ Thu, 15 Sep 2005 11:10:20 +0000 : mrowe Attachment: http://drupal.org/files/issues/comment_bulk_approval_1.patch (2.43 KB) Oh, so you're saying I should have read the style guide... ;) I've fixed the brace position, and I've redone the db_query as you suggested. Thanks! ------------------------------------------------------------------------ Sun, 18 Sep 2005 11:36:02 +0000 : Dries Code needs more small style fixes. Haven't tried the patch, nor did I try to look at the big picture. However, I agree that the comment approval workflow sucks. How do other systems deal with this? What does their UI and interaction design look like?

1 0