- Development - lists.drupal.org

[drupal-devel] [bug] Do not drop the complete accesslog on 4.6 update
by Goba 22 Jan '05

22 Jan '05

Project: Drupal Version: cvs Component: other Category: bug reports Priority: normal Assigned to: Goba Reported by: Goba Updated by: Goba Status: patch Attachment: http://drupal.org/files/issues/Drupal-do-not-drop-accesslog.patch (1.02 KB) The introduction of the access log id was made into an update, which drops the acceslog, and then recreates the table just to add a primary key. This is perfectly possible without loosing any data, and is documented in the MySQL docs. Patch attached. BTW the 'path' introduction before instead of using 'nid' was also buggy in the update, since it does not fill the 'path' values of the items, so all previous log items are invisible. That also needs to be fixed, but before it is fixed, I need input on the intended value of the 'path' field. In HEAD, statistics.module gets the path alias if available and stores that in the 'path' (line 77 currently). But then node/%d is matched with the path (line 174 currently), when a node access tracker is printed, so those nodes with path aliases are not working with it. It seems to me that the storage should be modified to not store the alias, but let it be computed when needed (eg. when printing links). But I am unsure, why was the alias stored. If this is fixed, we can get back to fixing the update to actually provide the 'path' value, before removing the 'nid', so that old accesslog items are still fully powering the site admin after the update. Goba -- View: http://drupal.org/node/15962 Edit: http://drupal.org/project/comments/add/15962

2 2

[drupal-devel] Contributions: missing support for table prefixing
by Dries Buytaert 22 Jan '05

22 Jan '05

The following modules have one or more queries that do not support table prefixing: $ find . -name "*.module" | xargs grep db_query | egrep -e INSERT -e UPDATE | grep -v '{' | awk -F ":" '{print $1}' | sort | uniq ./modules/auction/auction.module ./modules/friendlist/friendlist.module ./sandbox/breyten/modules/comment.module ./sandbox/breyten/modules/queue.module ./sandbox/dikini/binder/binder.module ./sandbox/eafarris/alt_homepage/althomepage.module ./sandbox/geek/catalog.module ./sandbox/gordon/dommenu/dommenu.module ./sandbox/jonbob/calendar/calendar.module ./sandbox/jonbob/navigation/navigation.module ./sandbox/killes/blocks/block.module ./sandbox/killes/image/image.module ./sandbox/kraichik/import.module.moveLinks2Bottom/import.module ./sandbox/marco/filter/old/filter.module ./sandbox/megagrunt/modules/enewsletter/enewsletter.module ./sandbox/paolino/import/import.module ./sandbox/remco/smileys.module ./sandbox/robert/comment.module ./sandbox/robert/user.module ./sandbox/simon/news/weblink.module ./sandbox/simon/weblink/weblink.module ./sandbox/simon/weblink/weblink_monitor.module ./sandbox/steph/installer/exemples/attached_file/attached_file.module ./sandbox/steph/modules/event/event.module ./sandbox/steph/modules/xstory/xstory.module ./sandbox/vinode/modules/quote/quote.module ./sandbox/walkah/translate_node/translate_node.module Please update your modules/sandbox. Thanks. -- Dries Buytaert :: http://www.buytaert.net/

1 0

[drupal-devel] [bug] QUICKFIX: utilize stat access right when printing view count
by Goba 22 Jan '05

22 Jan '05

Project: Drupal Version: cvs Component: statistics.module Category: bug reports Priority: normal Assigned to: Goba Reported by: Goba Updated by: Goba Status: patch Attachment: http://drupal.org/files/issues/Drupal-utilize-stat-access-right.patch (724 bytes) The stat counter display used to be restricted both by the statistics_display_counter global setting, and the current users access right to statistics. Since this access right is kept in the HEAD version of the module, I doubt it was not intended to only print out the view number to those with proper access rights. This simple patch adds the proper check. Goba -- View: http://drupal.org/node/15963 Edit: http://drupal.org/project/comments/add/15963

2 2

[drupal-devel] node/view deprecated (NODE)
by Dries Buytaert 22 Jan '05

22 Jan '05

The following files still use 'node/view/x' which is long deprecated in favor of 'node/x/view'. modules/admnotify/admnotify.module modules/authorise/patches/tree.patch modules/automember/INSTALL modules/clipper/INSTALL modules/clipper/clipper.module modules/collimator/INSTALL modules/commentrss/commentrss.module modules/emailpage/emailpage.module modules/filestore/filestore.module modules/groups/patches/event.patch modules/groups/patches/poll.patch modules/image_browse/image_browse.module modules/jsdomenu/jsdomenu.module modules/lanparty/lanparty.module modules/macrotags/macros.inc modules/moblog/moblog.module modules/navlinks/navlinks.module modules/navtable/navtable.module modules/node_aggregator/BUGS modules/node_aggregator/feed.module modules/node_aggregator/item.module modules/node_image/node_image.module modules/popup/popup.module modules/project/update-project.php modules/review/review.module modules/shamu/shamu.module modules/sidebar/CHANGELOG modules/sidebar/sidebar.module modules/spam/contributed/spam_tokens/jlusterorg.mysql modules/tag/tag.module modules/tec/tec.module modules/term_access/patches/poll.patch modules/trackback/CHANGELOG modules/translation/translation.module modules/wallpaper/wallpaper.module modules/webform/webform.module modules/wiki/CHANGELOG.txt sandbox/adrian/modules/weblink.module sandbox/adrian/themes/chameleon/chameleon.theme sandbox/almaw/for_review/0035.remove.teaser.from.db.patch sandbox/ber/multilingual/multilingual.module sandbox/ber/nodeadmin/node.module sandbox/ber/related/related.module sandbox/eafarris/alt_homepage/althomepage.module sandbox/garym/patches/rss-lite.patch sandbox/garym/themes/iguana/iguana.theme sandbox/gordon/patches/02_event_multi_location.patch sandbox/gordon/patches/event.patch sandbox/gordon/patches/ip-to-country.patch sandbox/irwin/permissionscontrol/old/access_control_requirements_specification.lyx sandbox/irwin/permissionscontrol/old/access_control_requirements_specification.txt sandbox/irwin/permissionscontrol/old/accesscontrol_requirements.txt sandbox/irwin/permissionscontrol/srs/htmlsrc/drupal_permissionscontrol_srs.html sandbox/irwin/permissionscontrol/srs/paperbib.bib sandbox/jareyero/image/image.module sandbox/jareyero/image/imagegallery.module sandbox/javanaut/attached_node/attached_node.module sandbox/jeremy/4.4.0/kerneltrap/database/url_alias.dump sandbox/jeremy/4.4.0/kerneltrap/statistics.module.patch sandbox/jluster/purple_numbers/purple_numbers.hack sandbox/jonbob/calendar/calendar.module sandbox/jseng/drupal4blog/.htaccess sandbox/jseng/drupal4blog/database/updates.inc sandbox/jseng/drupal4blog/includes/theme.inc sandbox/jseng/drupal4blog/modules/atom.module sandbox/jseng/drupal4blog/modules/block.module sandbox/jseng/drupal4blog/modules/blog.module sandbox/jseng/drupal4blog/modules/blogapi.module sandbox/jseng/drupal4blog/modules/book.module sandbox/jseng/drupal4blog/modules/comment.module sandbox/jseng/drupal4blog/modules/forum.module sandbox/jseng/drupal4blog/modules/node.module sandbox/jseng/drupal4blog/modules/page.module sandbox/jseng/drupal4blog/modules/path.module sandbox/jseng/drupal4blog/modules/poll.module sandbox/jseng/drupal4blog/modules/search.module sandbox/jseng/drupal4blog/modules/statistics.module sandbox/jseng/drupal4blog/modules/subscriptions.module sandbox/jseng/drupal4blog/modules/tag.module sandbox/jseng/drupal4blog/modules/taxonomy.module sandbox/jseng/drupal4blog/modules/title.module sandbox/jseng/drupal4blog/modules/trackback.module sandbox/jseng/drupal4blog/modules/tracker.module sandbox/jseng/drupal4blog/modules/user.module sandbox/jseng/drupal4blog/themes/chameleon/chameleon.theme sandbox/jseng/drupal4blog/themes/xtemplate/xtemplate.theme sandbox/junyor/comment.module sandbox/junyor/import/CHANGELOG sandbox/junyor/import/README sandbox/junyor/notify/CHANGELOG sandbox/killes/catalog/catalog.module sandbox/killes/contexthelp/contexthelp.module sandbox/killes/event.module sandbox/killes/field_image.inc sandbox/killes/flexinode.module sandbox/marco/fileapi/filestore.module sandbox/marco/fileapi/filestore.module.patch sandbox/marco/fileapi/pgallery.module sandbox/mathias/modules/print/print.module sandbox/matteo/custom done types/announce.module sandbox/matteo/custom done types/biography.module sandbox/matteo/custom done types/choir.module sandbox/matteo/custom done types/group.module sandbox/matteo/custom done types/media.module sandbox/matteo/custom done types/resource.module sandbox/matteo/custom done types/review.module sandbox/matteo/modules/Drupal CVS/Newsletter/newsletter.module sandbox/matteo/modules/Drupal CVS/print/print.module sandbox/matteo/modules/Drupal CVS/refer/refer.module sandbox/matteo/modules/Drupal CVS/taxonomy/taxonomy.module sandbox/matteo/modules/Drupal CVS/taxonomy_html/taxonomy.module sandbox/matteo/modules/Drupal CVS/taxonomy_html/taxonomy_html.module sandbox/megagrunt/xtemplate/default/xtemplate.template sandbox/megagrunt/xtemplate/pushbutton/xtemplate.template sandbox/moshe/clients/spoons.module sandbox/nedjo/modules/mapbuilder/mapbuilder.module sandbox/nedjo/themes/greengarden/xtemplate.theme sandbox/pablobm/modules/comment.module sandbox/simon/mnogosearch/HOWTO sandbox/simon/news/weblink.module sandbox/simon/weblink/weblink.module sandbox/skip/forms/forms.module sandbox/skip/navlinks/navlinks.module sandbox/stefan/image/image.module sandbox/stefan/image/imagegallery.module sandbox/steph/modules/event/event.module sandbox/tdobes/node_weight/README.txt sandbox/tdobes/node_weight/node_weight-43.patch sandbox/tdobes/node_weight/node_weight-44.patch sandbox/vinode/modules/quote/quote.module themes/gworks/gworks.theme themes/gworks/xtemplate.theme themes/marvin_2k/marvin_2k.theme themes/polder/polder.theme translations/ar/block-module.po translations/ar/path-module.po translations/nl/path-module.po translations/pl/path-module.po tricks/blocks/mostrecentcomments.php tricks/mt2drupal/mt-to-drupal.cgi Please take a moment or two to update your modules/themes/sandbox. Thanks. -- Dries Buytaert :: http://www.buytaert.net/

1 0

[drupal-devel] user/view deprecated (USER)
by Dries Buytaert 22 Jan '05

22 Jan '05

The following files still use 'user/view/x' which is long deprecated in favor of 'user/x/view': modules/affiliate/affiliate.module modules/auction/auction.module modules/friendlist/friendlist.module modules/lanparty/lanparty.module modules/rsvp/rsvp.module modules/style/style.module modules/syndication/syndication.module modules/taggame/taggame.module modules/trip_search/trip_search.module sandbox/adrian/themes/chameleon/chameleon.theme sandbox/garym/themes/marvin_2k/marvin_2k.patch sandbox/geek/tag/tag.module sandbox/jseng/drupal4blog/includes/common.inc sandbox/jseng/drupal4blog/modules/subscriptions.module sandbox/jseng/drupal4blog/modules/user.module sandbox/jseng/drupal4blog/themes/xtemplate/xtemplate.theme sandbox/marco/fileapi/fileapi.TUTORIAL sandbox/marco/fileapi/fsadmin.module sandbox/marco/fileapi/profile.module.diff sandbox/matteo/modules/Drupal CVS/language_switch/common.inc sandbox/nedjo/modules/search_enhancements/trip_search.module themes/gworks/xtemplate.theme themes/ivy/ivy.theme themes/polder/polder.inc Please take a moment or two to update your modules/themes/sandbox. Thanks. -- Dries Buytaert :: http://www.buytaert.net/

1 0

[drupal-devel] [feature] Create extra form elements for comment form
by stefan nagtegaal 22 Jan '05

22 Jan '05

Project: Drupal Version: cvs Component: comment.module Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: stefan nagtegaal Status: patch @ TDobes: The other issue is marked as 'duplicate', while here is the most interesting discussion going on about contentapi() vs. nodeapi()/commentapi().. Dries, what is you opinion about this? What needs to be done to get a patch like this in core? - Should we code the proposed contentapi()? - Another implementation? I really would like this in before we freeze HEAD, so i'm willing to cooperate in this.. stefan nagtegaal Previous comments: ------------------------------------------------------------------------ November 26, 2004 - 21:33 : nysus Like node forms which allow modules to use the nodeapi hook to prepend and append form elements to the node form, this simple change lets modules do the same for the comment form. Like node form, it also relies on the nodeapi hook. I plan to use it for an improvement to the subscriptions module that I have written which allows users to subscribe to individual comments without having to subscribe to an entire node. This change to the core was recommended by Moshe Weitzam found here: http://lists.drupal.org/archives/drupal-devel/2004-11/msg00256.html Thanks for the tip, Moshe! ------------------------------------------------------------------------ November 26, 2004 - 21:34 : nysus Attachment: http://drupal.org/files/issues/comment_8.patch (278 bytes) Oops, would help to attach the patch, eh? ------------------------------------------------------------------------ November 26, 2004 - 22:04 : TDobes A few things: 1. Please use the unified diff format for making patches -- see http://drupal.org/patch 2. I believe the suggestion was to add these features to hook_comment [1], not hook_nodeapi. nodeapi is just for nodes... comment is just for comments. 3. (not really related to yoru patch) Wouldn't it make more sense to have a hook_commentapi rather than a hook_comment? It seems like this would be more consistent with the naming of hook_nodeapi. 4. Many of the nodeapi $op values [2] could also apply to comments. Adding these to a commentapi could really open a lot of nice possibilities for modules. [1] http://drupaldocs.org/api/head/function/hook_comment [2] http://drupaldocs.org/api/head/function/hook_nodeapi ------------------------------------------------------------------------ November 26, 2004 - 22:36 : nysus TDobes, I don't know if you read the thread from the previous discussion, but there I pointed out that I didn't think the hook_comment seemed like the right function to modify. As far as creating a commentapi function, I thought about that. But I wondered if adding all the code necessary to support it was really worth it. You can use the pre-existing nodeapi to accomplish the same exact thing. And you won't have to create a new commentapi function in the comment.module and you won't have to create a new function in each of the modules that wants to use it. All you have to do is slap another case in the module's _nodeapi hook. So, you've got this in the comment.module's theme_comment_form() function: // Append extra comment form elements $form .= implode('', node_invoke_nodeapi($edit, 'comment form post')); And then in the subscription.module, you've got the following in the subscription_nodeapi() function: I understand your concern about maintaining naming conventions. Perhaps the nodeapi hook should be given a new name since it can have functionality beyond nodes. PS: Yeah, I know about patching with -u switch, just forgot. ------------------------------------------------------------------------ November 26, 2004 - 23:01 : TDobes I did read the thread that you linked to. At the moment (before your patch), hook_nodeapi is ONLY concerned with nodes. I believe it should stay that way. It is not that much more work for module authors to add a _comment (or _commentapi if we rename it) function to their module. On the other hand, it would be exceptionally confusing for people new to Drupal if we placed comment functions in a node hook. If we do decide to combine both hooks, I agree with you that nodeapi should be renamed. However, for the time being, I do not feel this is necessary. So... I agree with you that this would be a nice feature. However, I'd prefer a larger, more logical patch (adding to hook_comment) to a small and confusing patch (sticking this in hook_nodeapi, where it doesn't belong, IMO). But that's just my opinion... we'll have to see what others think. ------------------------------------------------------------------------ November 26, 2004 - 23:03 : nysus TDobes, OK, after rereading what you wrote and thinking about this some more, I think I may see a small advantage to having a separate hook_comment function. You are saying you could use the same op code string for both the hook_comment and nodeapi, right? But how about if the op code string were appended in some standardized way so the module _nodeapi hook could still be used? For example, you could prepend 'comment' to each of the codes for each operation that dealt with comments. So you'd have 'add' for when a node is added use 'comment add' for when a comment is added. 'insert' would be used for nodes and 'comment insert' for when comments are inserted. I guess it all boils down to the larger question I raised earler: Do we really need a separate _comment hook? ------------------------------------------------------------------------ November 26, 2004 - 23:06 : nysus Right, let's see what others think and go from there. Anyone? ------------------------------------------------------------------------ November 26, 2004 - 23:19 : Steven Hook_comment /is/ comment API. Renaming it to hook_commentapi seems like a good consistency idea. Putting all this inside hook_nodeapi is unacceptable and unclean. ------------------------------------------------------------------------ November 27, 2004 - 00:10 : moshe weitzman the best idea proposed by jhriggs was to combine nodeapi, commentapi, and user hooks into one contentapi hook. see http://lists.drupal.org/archives/drupal-devel/2004-07/msg00614.html in that msg, jhriggs prefers a single contentapi() hook over multiple $op specific hooks. Thats my slight preference as well. ------------------------------------------------------------------------ November 27, 2004 - 00:23 : Steven The contentapi puts us back in the discussion of "what is content?". Are users content? What about user profiles? etc. I don't think a generalized contentapi hook will help us though. A module will rarely affect nodes, comments and users at the same time. Almost all modules will implement contentapi and have a if ($type == 'node') or if ($type == 'comment') at the beginning. This makes the code more complicated than it should. It is also bad from a performance point of view to call every module which does something with nodes, comments or users when an operation is performed on any of them. ------------------------------------------------------------------------ November 27, 2004 - 04:51 : nysus After a little more consideration and after reading some of the comments here, I'll just recode this so that it uses the hook_comment. It just doesn't make sense to try to push through a fundamental change to Drupal's API to accomodate this small patch. ------------------------------------------------------------------------ November 27, 2004 - 05:37 : nysus Attachment: http://drupal.org/files/issues/comment_9.patch (876 bytes) OK, here's a second revision that works with hook_comment() instead. ------------------------------------------------------------------------ November 27, 2004 - 09:45 : Dries "Wouldn't it make more sense to have a hook_commentapi rather than a hook_comment? It seems like this would be more consistent with the naming of hook_nodeapi." No, but it would make sense to rename hook_nodeapi to hook_node. ------------------------------------------------------------------------ November 29, 2004 - 18:01 : jhriggs I still like the _contentapi() idea. I have discussed it several times with Kjartan, and although we have a few differences in opinion as to exactly how it should be coded, we both agree that it would be useful functionality. It does raise the "What is content?" question as Steven points out. Perhaps another name would be more appropriate. I do see tremendous benefit in having a single entry-point for this type of extensibility, though. Say for example that I want to append some type of disclaimer or a verification step (a la captcha or a similar module I wrote awhile back) in every submission form, be it node, comment, poll vote, user registration, profile, etc. I would only have to handle it in one place. Every time I receive a 'form pre' or 'form post', I can insert my content...or I can handle certain cases differently if desired (based on the type of content). Additionally, any module would be able to benefit from the logic. Instead of having somewhat overlapping _nodeapi(), _user(), and _comment() functionality like we do now (or will have even more of with the patch proposed here), we would have one invocation function that any module can use for any type of content that they define (i.e. user in user module, profile in profile.module, node in node.module, comment in comment.module, poll vote form in poll.module, etc.). Finally, as I have also discussed with Kjartan, functionality like this would mean that project.module would not have to use its own comment system (lots of duplicated code). It would be able to use the enhanced form-insertion and verification available through the hook. If there is sufficient interest, I will revive/rework the patch that I have started several times... ------------------------------------------------------------------------ December 24, 2004 - 07:48 : TDobes There's a similar discussion going on in another issue [3]... one of these should be marked as a duplicate. [3] http://drupal.org/node/14708 -- View: http://drupal.org/node/13539 Edit: http://drupal.org/project/comments/add/13539

1 0

[drupal-devel] [bug] Reintroduce translateable watchdog types
by Goba 22 Jan '05

22 Jan '05

Project: Drupal Version: cvs Component: watchdog.module Category: bug reports Priority: normal Assigned to: Goba Reported by: Goba Updated by: Goba Status: patch Attachment: http://drupal.org/files/issues/Drupal-reintroduce-translated-watchdog-types… (1.67 KB) This used to be in watchdog module, but it seems it was gone due to some reogranization. The times are printed localized, the watchdog messages are translated, all other stuff is printed in the locale, so it is obvious to translate watchdog type names too. The attached simple patch fixes this. Extractor already extracts watchdog type names for translation, as this used to be translated on the interface. Goba -- View: http://drupal.org/node/15957 Edit: http://drupal.org/project/comments/add/15957

2 1

[drupal-devel] [task] Better default workflow
by stefan nagtegaal 22 Jan '05

22 Jan '05

Project: Drupal Version: cvs Component: node system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: stefan nagtegaal Status: patch Wow, i really like this patch! This implementation is very clean and looks consistant with the way the input format's are configured. (Which is imo one of the best UI we have.) +1 for this.. stefan nagtegaal Previous comments: ------------------------------------------------------------------------ December 10, 2004 - 03:27 : drumm Attachment: http://drupal.org/files/issues/types1.png (8.05 KB) This is a screenshot of the page replacing the 'default workflow.' ------------------------------------------------------------------------ December 10, 2004 - 03:29 : drumm Attachment: http://drupal.org/files/issues/types2.png (18.41 KB) This is what you get by clicking 'configure.' ------------------------------------------------------------------------ December 10, 2004 - 04:01 : drumm Attachment: http://drupal.org/files/issues/default_workflow.diff (10.79 KB) The default workflow page quickly becomes usuable as modules are added. I think it works much better broken up into one page per content type. This is a very basic patch which simply moves around the UI and does not change any functionality. I see lots of potential in following this path: * The submission guidelines would make sense on this page. Have the node module put those in, no need to have it duplicated in all the modues. * This page could be moved around to be a top level admin menu with local tasks 'list' and 'add new type' for making lightweight types on the fly instead of the very lightweight modules we have now. * Permissions for who can create nodes or edit their own might make sense on or next to these pages. * The various elements of the create content form, or even the node view, could be reordered with weights. This requires modules implementing hook_nodeapi('settings') to return strings containing the form elements instead of associative arrays. ------------------------------------------------------------------------ December 10, 2004 - 04:09 : moshe weitzman Perhaps we could preserve the overview of all node types and settings in read only table view. then an edit click would lead to the form you show. i think the overview of all settings at once is useful sometimes. ------------------------------------------------------------------------ December 10, 2004 - 09:35 : Dries This is a step in the right direction but the patch needs to evolve a bit before it is commit-worthy. For one, I would use form_group() rather than a form_item-hack. For consistency, table headers should start with a capital letter. Furthermore, I support the idea of creating a more prominent workflow-menu. Moving the story and forum settings to this page, might make sense, although I'm not sure people expect to find the 'submission guidelines' or 'forum structure settings' under 'workflow'. I guess it would work. Either way, this patch has potential so keep working on it. ------------------------------------------------------------------------ December 10, 2004 - 16:57 : tostinni Is this : Configure Workflow by roles [1] could be a way of improvement to ? Good ideas for the patch, thanks. [1] http://drupal.org/node/13129 ------------------------------------------------------------------------ December 16, 2004 - 23:22 : Dries The statistics module's "Display node counter" setting could be part of the workflow system (that or it should be added to the theme's "Toggle display" settings). Similarly, the "Display post information on" setting on the global theme configuration page might be better of being part of the workflow system. ------------------------------------------------------------------------ December 17, 2004 - 10:55 : Bèr Kessels Display and rendering setings should always be part of the theme settings, and preferably only the global settings. they have nothing to do with workflow, and will confuse people if they are "hidden"there. Similar to the "permissions" for upload, that are (were?) hidden there, since uploading has little to do with "allowing uploads". ------------------------------------------------------------------------ January 21, 2005 - 01:26 : drumm Attachment: http://drupal.org/files/issues/default_workflow_0.diff (16.22 KB) Here is a cleaned up patch which: - Capitalizes the column headers - Uses form_checkboxes() instead of the form_item() hack Because I wanted to use form_checkboxes() rather than a form_group() I had to make those 5 variables into one array. There is an update to set the one variable per node type to the values of the five old ones, and delete the five. I have not benchmarked the difference of one array per node type in the variables table vs. five booleans (ints) per node type, but I expect it might be faster. I do have more ideas for what should be done once this first step is finished. - I think 'content types' should be a menu item in the navigation menu, not a local task. This will let us put a 'add content type' tab in the logical place when/if it is time to do that. Also this page is buried now. - I think that the submission guidelines and minimum words should go on these pages. - I think form_group() should be used once the form grows. We could stick what we have now in a 'Workflow' fieldset. - I think that the 'settings' $op of hook_nodeapi() should be rethought and maybe put in another hook. - I think that node create/edit permissions could go on these pages. - I think that this form should have a 'save' $op and use system_settings_save() because that limits what modules can do to using variables. - I think that there should be a 'validate' $op so modules can do validation. - I think that the five options should be rethought. However, these are all separate issues which should have their own patches and debate and many will have to wait until 4.7. But this gets the door open for modules and ideas in 4.6. I'm willing to put more in the overview table, but I would like input on what it should look like to avoid width problems and be as usable as possible moving forward. Perhaps that should wait for another patch? ------------------------------------------------------------------------ January 21, 2005 - 01:32 : drumm BTW, I consider the added 'else' in system.module to be a bugfix. I'm not exactly sure why system_settings_save() does a drupal_goto(), but it helps make the form act a lot more predictably if it does the same type of returning or goto for whichever button is pressed. -- View: http://drupal.org/node/14170 Edit: http://drupal.org/project/comments/add/14170

1 0

[drupal-devel] [bug] "Add new comment" no longer shows up in $links?
by Dries 22 Jan '05

22 Jan '05

Project: Drupal Version: cvs Component: poll.module Category: bug reports Priority: normal Assigned to: Goba Reported by: Morbus Iff Updated by: Dries Status: patch This update seems to bring all core modules in line. However, several contributed modules behave differently or have not been updated accordingly. I compiled a list of modules that use $main instead of $teaser: $ grep -r "\$main" * | grep "_view(&" | awk -F ":" '{print $1}' modules/event/event.module modules/image/image.module modules/filestore2/filestore2.module modules/bookreview/bookreview.module modules/lanparty/lanparty.module modules/survey/survey.module modules/mail_archive/mail_archive.module sandbox/geek/catalog.module sandbox/gordon/patches/02_event_multi_location.patch sandbox/jseng/drupal4blog/modules/poll.module sandbox/killes/event.module sandbox/killes/catalog/catalog.module sandbox/matteo/custom done types/announce.module sandbox/matteo/custom done types/biography.module sandbox/matteo/custom done types/choir.module sandbox/matteo/custom done types/group.module sandbox/matteo/custom done types/media.module sandbox/matteo/custom done types/resource.module sandbox/matteo/custom done types/review.module sandbox/matteo/modules/Drupal CVS/feedback/feedback.module sandbox/steph/modules/event/event.module sandbox/steph/modules/xstory/xstory.module sandbox/walkah/image/45/image.module sandbox/walkah/image/image.module I don't recall when this change happened (from 4.4 to 4.5 I think) but it might be worth adding a note to the upgrade guide. Dries Previous comments: ------------------------------------------------------------------------ December 30, 2004 - 16:41 : Morbus Iff I recently upgraded from 4.4.2 to 4.5.1, and have noticed that the poll block no longer displays the "add new comment" or "# comments" link. This is (was) important to me, as I usually ask for comments whenever someone chooses the negative option. Any idea where and why this went? You can still get to the comment form in a roundabout way ("other polls", choose your poll, whammo), but that's far too many clicks and intellect points. ------------------------------------------------------------------------ January 18, 2005 - 17:50 : rooey I had the same problem going from 4.4 to 4.5 - i still see it in 4.5.2 :( There is *some* info on comment issues here: http://drupal.org/node/11366 I didn't find any of them solved this particular issue tho. ------------------------------------------------------------------------ January 18, 2005 - 19:28 : Junyor So, not even the "add new comment" link is showing up? I just loaded the poll module and that shows fine. However, the current comment count doesn't show correctly in the block. This is what I've seen so far: 1) poll_block calls poll_view 2) poll_view calls poll_view_voting, which outputs the poll choices 3) for blocks, poll_view calls link_node, which calls hook_link('node') in each module. For all other poll displays, the theme does the call to link_node (via node_view) I'm guessing poll_view is where the problem is. I'll keep looking at it. ------------------------------------------------------------------------ January 18, 2005 - 19:41 : Junyor Changing the call to link_node on line 438 in poll.module (in poll_view) to: ------------------------------------------------------------------------ January 18, 2005 - 19:51 : Junyor Trying again with correct PHP tags. :) Changing the call to link_node on line 438 in poll.module (in poll_view) to: seems to solve the problem. I'm trying to look now at the history of link_node to see why that fixed it. That's the same parameters that the call in theme_node uses. ------------------------------------------------------------------------ January 18, 2005 - 19:58 : Junyor Attachment: http://drupal.org/files/issues/junyor.poll.patch (762 bytes) The change to theme_node was done as part of http://drupal.org/node/9287. Other than theme_node, poll_view is the only function that calls link_node directly. Attached is a patch to fix the function call in poll_view to send the correct parameters. ------------------------------------------------------------------------ January 18, 2005 - 20:21 : Morbus Iff Confirmed Junyor's patch. Excellent (note: take my confirmation with a grain of salt. One of my previous patches caused a grievous bug for 4.5.2 tracker.module, and I should not be trusted anymore <g>). ------------------------------------------------------------------------ January 18, 2005 - 20:22 : Morbus Iff (His patch is currently in use at gamegrene.com, running 4.5.1). ------------------------------------------------------------------------ January 18, 2005 - 20:23 : Jeremy(a)kerneltrap.org Seems to work like a charm -- thanks. :) ------------------------------------------------------------------------ January 18, 2005 - 20:35 : moshe weitzman please submit a patch for HEAD as well (if needed). link_node() no longer exists so this patch is no help for HEAD ------------------------------------------------------------------------ January 18, 2005 - 20:36 : Junyor Note: The attached patch is a good solution for 4.5.2. For HEAD, all hook_link functions need to have their parameter list updated to make sure they're using $teaser and not $main. Same goes for node_link. Additionally, we should think about adding a theme function to handle hook_block links (if one doesn't already exist). ------------------------------------------------------------------------ January 18, 2005 - 20:42 : Junyor @moshe: I don't think a patch for HEAD to fix this bug will be needed, as the erroneous function call was removed. Has anyone tested the poll module on HEAD to see if this problem exists there? ------------------------------------------------------------------------ January 20, 2005 - 21:39 : Dries Committed to HEAD. ------------------------------------------------------------------------ January 22, 2005 - 02:35 : Goba Attachment: http://drupal.org/files/issues/Drupal-fix-poll-links.patch (1.72 KB) Dries actually comitted the fix to 4.5, but this is still buggy in HEAD. A patch attached which fixes the behaviour. Simply 1 needs to be passed on the teaser value to get the proper comment (and other type of links), since the poll block is kind of a teaser. This patch also renames $main to $teaser, where appropriate to follow latest Drupal conventions. Without this rename, poll_view() gets a $teaser parameter, and then tries to use $main. This might also apply to 4.5, but since the value of $teaser is never used from then on, it is not much of a problem, just some unneccessary code. I have not removed it from poll_view_results() and poll_view_voting(), to keep the familiary looking param list, although $teaser is not utilized there. Note: someone might check the Drupal code for $main, since as this patch shows it was renamed to $teaser at some places, but not all. -- View: http://drupal.org/node/14936 Edit: http://drupal.org/project/comments/add/14936

1 0

[drupal-devel] [bug] "Add new comment" no longer shows up in $links?
by Goba 22 Jan '05

22 Jan '05

Project: Drupal -Version: 4.5.1 +Version: cvs Component: poll.module Category: bug reports Priority: normal -Assigned to: Junyor +Assigned to: Goba Reported by: Morbus Iff Updated by: Goba -Status: fixed +Status: patch Attachment: http://drupal.org/files/issues/Drupal-fix-poll-links.patch (1.72 KB) Dries actually comitted the fix to 4.5, but this is still buggy in HEAD. A patch attached which fixes the behaviour. Simply 1 needs to be passed on the teaser value to get the proper comment (and other type of links), since the poll block is kind of a teaser. This patch also renames $main to $teaser, where appropriate to follow latest Drupal conventions. Without this rename, poll_view() gets a $teaser parameter, and then tries to use $main. This might also apply to 4.5, but since the value of $teaser is never used from then on, it is not much of a problem, just some unneccessary code. I have not removed it from poll_view_results() and poll_view_voting(), to keep the familiary looking param list, although $teaser is not utilized there. Note: someone might check the Drupal code for $main, since as this patch shows it was renamed to $teaser at some places, but not all. Goba Previous comments: ------------------------------------------------------------------------ December 30, 2004 - 15:41 : Morbus Iff I recently upgraded from 4.4.2 to 4.5.1, and have noticed that the poll block no longer displays the "add new comment" or "# comments" link. This is (was) important to me, as I usually ask for comments whenever someone chooses the negative option. Any idea where and why this went? You can still get to the comment form in a roundabout way ("other polls", choose your poll, whammo), but that's far too many clicks and intellect points. ------------------------------------------------------------------------ January 18, 2005 - 16:50 : rooey I had the same problem going from 4.4 to 4.5 - i still see it in 4.5.2 :( There is *some* info on comment issues here: http://drupal.org/node/11366 I didn't find any of them solved this particular issue tho. ------------------------------------------------------------------------ January 18, 2005 - 18:28 : Junyor So, not even the "add new comment" link is showing up? I just loaded the poll module and that shows fine. However, the current comment count doesn't show correctly in the block. This is what I've seen so far: 1) poll_block calls poll_view 2) poll_view calls poll_view_voting, which outputs the poll choices 3) for blocks, poll_view calls link_node, which calls hook_link('node') in each module. For all other poll displays, the theme does the call to link_node (via node_view) I'm guessing poll_view is where the problem is. I'll keep looking at it. ------------------------------------------------------------------------ January 18, 2005 - 18:41 : Junyor Changing the call to link_node on line 438 in poll.module (in poll_view) to: ------------------------------------------------------------------------ January 18, 2005 - 18:51 : Junyor Trying again with correct PHP tags. :) Changing the call to link_node on line 438 in poll.module (in poll_view) to: seems to solve the problem. I'm trying to look now at the history of link_node to see why that fixed it. That's the same parameters that the call in theme_node uses. ------------------------------------------------------------------------ January 18, 2005 - 18:58 : Junyor Attachment: http://drupal.org/files/issues/junyor.poll.patch (762 bytes) The change to theme_node was done as part of http://drupal.org/node/9287. Other than theme_node, poll_view is the only function that calls link_node directly. Attached is a patch to fix the function call in poll_view to send the correct parameters. ------------------------------------------------------------------------ January 18, 2005 - 19:21 : Morbus Iff Confirmed Junyor's patch. Excellent (note: take my confirmation with a grain of salt. One of my previous patches caused a grievous bug for 4.5.2 tracker.module, and I should not be trusted anymore <g>). ------------------------------------------------------------------------ January 18, 2005 - 19:22 : Morbus Iff (His patch is currently in use at gamegrene.com, running 4.5.1). ------------------------------------------------------------------------ January 18, 2005 - 19:23 : Jeremy(a)kerneltrap.org Seems to work like a charm -- thanks. :) ------------------------------------------------------------------------ January 18, 2005 - 19:35 : moshe weitzman please submit a patch for HEAD as well (if needed). link_node() no longer exists so this patch is no help for HEAD ------------------------------------------------------------------------ January 18, 2005 - 19:36 : Junyor Note: The attached patch is a good solution for 4.5.2. For HEAD, all hook_link functions need to have their parameter list updated to make sure they're using $teaser and not $main. Same goes for node_link. Additionally, we should think about adding a theme function to handle hook_block links (if one doesn't already exist). ------------------------------------------------------------------------ January 18, 2005 - 19:42 : Junyor @moshe: I don't think a patch for HEAD to fix this bug will be needed, as the erroneous function call was removed. Has anyone tested the poll module on HEAD to see if this problem exists there? ------------------------------------------------------------------------ January 20, 2005 - 20:39 : Dries Committed to HEAD. -- View: http://drupal.org/node/14936 Edit: http://drupal.org/project/comments/add/14936

1 0