January 2005 - Development - lists.drupal.org

[drupal-devel] [feature] Handle full URLs in url() and l()
by mathias 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: jhriggs Updated by: mathias Status: patch Attachment: http://drupal.org/files/issues/external_urls.patch (838 bytes) Revisiting this issue one year later shows we're almost there. The only thing that's left is the ability to handle external urls when clean_urls is disabled. Once implemented, menu.module can happily accept external urls and not just Drupal paths, making the menu system much more attractive to end users. I'll update the menu.module docs if this patch (or an alternative solution) is accepted. mathias Previous comments: ------------------------------------------------------------------------ September 16, 2004 - 10:01 : jhriggs Attachment: http://drupal.org/files/issues/full_url.patch (1.97 KB) This little patch allows full URLs to be used for the url() and l() functions, giving modules a single interface for creating links. Currently, modules must use l() for drupal links and raw HTML for external links. This patch changes url() to handle -- basically pass through -- full URLs while still doing the proper handling of internal paths and works with clean URLs enabled or disabled. It also shortens (and hopefully simplifies) the url() code a bit. Some examples of calls that now work: ------------------------------------------------------------------------ September 30, 2004 - 06:09 : moshe weitzman +1. less HTML in code is a good thing for readability, if nothing else. -- View: http://drupal.org/node/10888 Edit: http://drupal.org/project/comments/add/10888

1 0

[drupal-devel] [feature] Testers needed: Node Type filtering in taxonomy on Drupal 4.5
by matteo 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: matteo Reported by: matteo Updated by: matteo Status: patch Personally, the proposed syntax is not important, but the functionality is VERY important, so, the actual code for this patch can be revised to adapt to a different syntax. I chose that syntax since now the syntax is: taxonomy/term/term_ids/level so if you want to add node types to that syntax, level must be explicitly set, while now it is not mandatory: taxonomy/term/term_ids/level/nodetypes As you can see from the code, it is very simple, but powerful, and compatible with the actual syntax. Once we decide for the syntax, I'm available, if you need, to write some code for it. Matteo matteo Previous comments: ------------------------------------------------------------------------ October 25, 2004 - 22:20 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_filter.patch (5.46 KB) The attached fix wants to add a new feature to taxonomy.module: the ability to filter data also on node types. This is a need when you have to share one vocabulary between multiple node types. One example is the vocabulary 'Geography', containing all Cities and Regions. You could have events, stories and other custom made node types, sharing this vocabulary. The need arises when you want to see only events of a certain region; if you filter only on region, you'll see also stories and the other custom node types. The new syntax is the following: taxonomy/term/term1,term2,term3(nodetype1,nodetype2)/........ to filter based on three terms (in OR each others) and on two different node types or taxonomy/term/term1+term2+term3(nodetype1,nodetype2)/........ to filter based on three terms (in AND each others) and on two different node types The rest of the syntax for taxonomy/term remains the same, and the patch works also for taxonomy feed. Node types are tested for validity. You can specify one or more node types; if you omit node types, normal term filtering is issued. Feedbacks and help testing this patch is needed; personally, I think this would improve temendously taxonomy functionalities, and hope this will be committed to Drupal core. Feel free to contact me by mail: m dot ferrari at tiscali dot it Matteo ------------------------------------------------------------------------ October 25, 2004 - 22:25 : moshe weitzman see http://drupal.org/node/5040 ------------------------------------------------------------------------ October 25, 2004 - 22:46 : matteo I agree with you that this is an old story, but nothing has been committed to 4.5, that introduced also a new taxonomy syntax. My patch has also some pluses: - it is specific for 4.5 and takes care of the new taxonomy syntax - it lets you specify multiple node types - syntax is clean Matteo ------------------------------------------------------------------------ January 19, 2005 - 23:05 : robclay Will this work with 4.5.2? If so - I am looking for exactly this funcionality! ------------------------------------------------------------------------ January 20, 2005 - 06:43 : matteo I'll work on a port for 4.5.2 hopefully next week... I'll keep you informed. Matteo ------------------------------------------------------------------------ January 24, 2005 - 00:19 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_452.patch (4.43 KB) Here is the patch that adds nodetype filtering capabilities to 4.5.2 taxonomy module. Syntax is backwards compatible, so if you plan not to use node type filtering, syntax does not change at all. (see previous postings in this thread) I would need extensive testing and feedback... and hope this functionality will be included into Drupal core. Matteo ------------------------------------------------------------------------ January 24, 2005 - 00:38 : Steven New features only get added to the HEAD branch... you'll have to update your patch. ------------------------------------------------------------------------ January 24, 2005 - 07:56 : matteo Ok, Steven, I'll rewrite the patch also for CVS.... Thanks Matteo ------------------------------------------------------------------------ January 24, 2005 - 10:16 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_CVS.patch (4.05 KB) Here is the patch for Drupal CVS. I noticed the great addition of node_rewrite_sql... very powerful... I'm starting extensive testing on the patch, and I really woul like it to be added to core taxonomy, because it becomes a must for several sites... Matteo ------------------------------------------------------------------------ January 24, 2005 - 15:43 : moshe weitzman I'd rather see this done universally (not just for taxonomy urls) using the new sql rewrite capability. specifically, some code will implement the rewrite_sql hook and look for a querystring param &ntype=blog,story and then inject the proper WHERE clause. this will be very useful for tracker, home page, taxonomy urls, etc. ------------------------------------------------------------------------ January 24, 2005 - 16:59 : Dries Before looking at the code (which violates Drupal's coding style), what do people think about the proposed URL scheme? Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? How about we change that to: taxonomy/all/$foo (or taxonomy/$foo) taxonomy/forum/$foo taxonomy/forum,book/$foo ... instead of the proposed URL scheme? It looks slightly more readable to me. (Does anyone know what 'synonyms' are for, and whether they could solve this problem? Really, I have no idea what these are for, even after having read the documentation.) ------------------------------------------------------------------------ January 24, 2005 - 17:15 : jvandyk Synonyms are useful for things like organisms, which have both a common name and a scientific name. That's the idea of synonyms, not necessarily the Drupal implementation of synonyms, which we plan to use but are not yet using. ------------------------------------------------------------------------ January 24, 2005 - 17:57 : Steven Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? I believe this is because when the URL changes were discussed, there was talk of a 'taxonomy/vocabulary' page which would display an overview of a vocabulary. But I think that role has already been filled by a contributions module. ------------------------------------------------------------------------ January 24, 2005 - 18:05 : Prometheus6 If taxonomy/$foo retrieves all node types then I think taxonomy/$foo/forum taxonomy/$foo/forum,book is the cleanest looking way to be specific. Give it a fixed position because it will always be there whereas the node type may or may not be. ------------------------------------------------------------------------ January 24, 2005 - 18:24 : Steven This could be hard to implement as $foo is a multi-var path. e.g. "taxonomy/terms/1" "taxonomy/terms/1/1" "taxonomy/terms/1/0/feed" etc. ------------------------------------------------------------------------ January 24, 2005 - 19:29 : Prometheus6 How about: taxonomy/$foo,$bar taxonomy/$foo,$bar/forum taxonomy/$foo,$bar/forum,book taxonomy/$foo,$bar/forum/feed -- View: http://drupal.org/node/12114 Edit: http://drupal.org/project/comments/add/12114

1 0

[drupal-devel] [feature] Improve functionality of block generation for book module
by andremolnar 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: book.module Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: andremolnar Status: patch Attachment: http://drupal.org/files/issues/book_module_7.patch (7.43 KB) Yet another patch: Brings this patch in line with - http://drupal.org/node/16074 (i.e. the column name change in {block} table) andre andremolnar Previous comments: ------------------------------------------------------------------------ December 9, 2004 - 06:12 : nysus Attachment: http://drupal.org/files/issues/book_19.patch (4.85 KB) Attached is a patch for the book module that does the following: 1) Allows book blocks to appear on any page at any time, not just when a node from the book is being viewed. 2) Allows multiple book blocks to appear on the same page. This functionality is achieved by the automatic creation of individual blocks for each book when the book is created. Simply enable the book's block to enjoy the benefits of 1 & 2 above. If the blocks are not enabled, the blocks will appear only when a node from that block is being viewed (the same way it works now). ------------------------------------------------------------------------ December 9, 2004 - 12:35 : andremolnar Attachment: http://drupal.org/files/issues/book_19_1.patch (4.84 KB) +1 This is great. A good many people have asked for something like this, and I think its a nice solution. But in the end this isn't up to me. One minor error in the patch... + $result = db_query('SELECT n.title, b.block, b.nid FROM {book} b INNER JOIN {node} n on n.nid = b.nid WHERE b.parent = 0'); b.block is not a valid field in this query. this updated patch removes reference to it. andre ------------------------------------------------------------------------ December 9, 2004 - 13:54 : nysus Glad you like it and thanks for fixing that up. It was left over from an older version of my patch. ------------------------------------------------------------------------ December 9, 2004 - 13:54 : andremolnar Steve, If I apply this patch, and I attempt to configure one of the newly created blocks. I noticed that for some reason the block.module is returning "true" at line 249 (of the block module)- and is creating a form for module-specific configuration. But all that shows up on the screen is the word "array". Can you trace this back to see why - and maybe update the patch. I can continue to test your changes - anything to help this patch make it into core. andre ------------------------------------------------------------------------ December 9, 2004 - 15:29 : nysus Hmmm...probably because I tested my patch on my version of Drupal, version 4.5.1. I'm having no problems. Are you using a cvs version of Drupal to test. If so, I'll set up cvs on my site and track this down. ------------------------------------------------------------------------ December 9, 2004 - 18:53 : drumm See http://drupal.org/node/12347 for information on how the block system has been updated. When I saw that the elseif ($op == 'view') was taken out I knew immedaitely that there was something weird about this patch. ------------------------------------------------------------------------ December 9, 2004 - 18:58 : nysus OK, thanks for the tip. Sorry for the confusion. Still kind of new to making open source contributions and it's easy for me to overlook some obvious stuff like this. I'll fix this up when I get a chance. ------------------------------------------------------------------------ December 9, 2004 - 19:06 : Dries Please do, because this being a new feature, it has no chance getting committed to the DRUPAL-4-5 branch. The DRUPAL-4-5 branch is in bugfix-mode. New features go into CVS HEAD. ------------------------------------------------------------------------ December 9, 2004 - 22:32 : Dries There is quite a bit of duplicated code in the patch. Maybe it can be simplified (using a function)? Either way, it is a little weird. I haven't tried the path, and I'm not sure I understood the description. It's somewhat vague. Is the book module exporting multiple blocks that are nearly identical, yet have different display behavior? If so, why not add a simple block configuration setting to the original book block? ------------------------------------------------------------------------ December 10, 2004 - 02:51 : nysus Dries, Yes, there is some code that can be factored out and some general cleaning up that can be done. It was a little tricky to write so I left it kind of ragged around the edges until I'm sure there are no bugs. It has worked very well on 4.5.1 but I obviously need to update it to work with cvs. I'll be on that soon. As far as what it does: 1) For every new book that a user creates, a new block is associated with it. So if you create "Book A", "Book B", "Book C", you will get three new blocks visible on the block administration page called "Book A", "Book B", "Book C". The original "Book Navigation" block is still there, too. The functionality of the "Book Navigation" block is not affected at all by this patch. 2) If Book A's block is enabled, the block containing its menu will appear not only when a node within Book A is viewed, but at all times (unless the user suppresses it on certain pages with the "path" feature). When any node is that is NOT in Book A is viewed, Book A's menu still appears but it is fully collapsed. When a node that DOES belong to Book A is viewed, Book A's Book menu expands accordingly. 3) The user can also enable Book B & C's block, and have their menus appear in a block at all times as well. 4) If none of the book's blocks are enabled by the user, the module will behave just as it did without the patch. That is, when the "Book Navigation" block is enabled, the only time any book menu will appear is when a book node is being viewed. 5) It's important to note (and this was the tricky part to write) that if both the "Book Navigation" block is enabled and "Book A" is enabled, they will play nice with each other and not do nasty things like create the same book menu twice. ------------------------------------------------------------------------ December 10, 2004 - 05:51 : nysus Attachment: http://drupal.org/files/issues/book_20.patch (5.37 KB) Andre, Attached is a new patch that will resolve the problem of the block-specific stuff showing on the block configuration form. Let me know if you spot any other bugs. If it looks good to you, I'll go to work making the code look leaner and prettier. ------------------------------------------------------------------------ December 10, 2004 - 07:26 : nysus Attachment: http://drupal.org/files/issues/book_21.patch (4 KB) Dries, Andre: Here is a new and improved streamlined version of the patch. Have a look if you get a chance. ------------------------------------------------------------------------ December 10, 2004 - 08:11 : nysus Attachment: http://drupal.org/files/issues/book_22.patch (4.15 KB) Found a bug in the last version that would cause the block to jump to a different location. I think this should do it. Everything appear to work well (famous last words). ------------------------------------------------------------------------ December 10, 2004 - 14:49 : andremolnar Steve: bugs appear to be gone, and I didn't run across any other errors. This is functioning exactly as described. everyone: I personally would encourage support for this functionality. Book is a powerful navigation building tool in a site, not only with its ability to move next and back through a hierarchy of pages - but also its ability to build the appropriate navigation block without further user intervention (unlike the admin features in the menu module or taxonomy). The most frequently cited complaint about the book module is its inflexibility when it comes to when and where the block shows up. I also frequently hear requests for the ability to show multiple book blocks at the same time. Up until now the best alternatives suggested required users to do a hack (e.g. build a custom block that calls such and such a hook). Most abandon their request at that point because its over their heads. With this patch all those requests are covered and more. Now all books can automatically have their own block and admins can easily decide when and where each of those individual blocks show up (left right, up down) and coupled with the new configuration features of the block module - its very easy for admins to decide on which individual pages a block will show up. I would be interested what other have to say about this feature. My only reservation (which is minor compared to the benefits of the functionality offered) is that there is no way to turn this functionality off. i.e. The default behaviour is to build individual blocks for each book. If there could be a way to toggle this feature on and off somewhere - it would be perfect. Still, AS IS - this is a major improvement and offers great flexibility to admins and site creators. andre ------------------------------------------------------------------------ December 10, 2004 - 15:04 : Anonymous Andre, Thanks for the feedback on the usefulness of this module. Glad I could pitch in and help. I agree about the inability to turn the feature on/off and I was thinking about that myself. I think it could easily be accomplished by creating a checkbox in the "book navigation" block individual configuration's settings. Call it "Enable individual book blocks." When enabled, the individual book blocks will appear on the block administration menu. One question: Where would the state of this checkbox get saved? Has Drupal moved away from serializing data in the data base? ------------------------------------------------------------------------ December 10, 2004 - 18:36 : Dries I'm afraid that 'Enable individual book blocks.' is not descriptive/clear at all. Are you suggesting a setting to toggle between 'show block on all pages' and 'show block on book pages only'? ------------------------------------------------------------------------ December 10, 2004 - 19:45 : nysus Dries, No. Andre and I suggest a setting within the "Book Navigation" cofiguration page, that would toggle whether or not individual books appear on the list of all blocks on the block administration page. Hence the name 'Enable individual book navigation blocks.' The help text for this checkbox might read something like: "By default, a book's navigation block is visible only when a page from that book is being viewed. Check this box if you want more control over where and when an book's navigation block is visible. You will then be able to control the book's navigation block location and visibility settings on the "admin/block" page." Hope this makes it pretty clear. ------------------------------------------------------------------------ December 10, 2004 - 20:08 : Dries I understand what you are trying to do, but not how you are trying to do it, or how the setting is supposed to work. I guess I'll have to try it when a new patch lands. ------------------------------------------------------------------------ December 11, 2004 - 09:52 : nysus Attachment: http://drupal.org/files/issues/book_23.patch (5.02 KB) Alright, fellas, I'm proud to unveil my crowning achievement in the open source development world (no big deal to most of you guys but pretty good for a hack like me). Thanks for all the input so far. It's been helpful. I've streamlined the heck out of it per Dries suggestion and I've created an option to turn this functionality on an off per Andre's suggestion. Does this look good to you guys? Anything else I have to fix or improve? Thanks. ------------------------------------------------------------------------ December 11, 2004 - 13:02 : Dries I tried the patch. It works great but to me, the 'Give books their own block' settings seems to be redundant. Why not export the current book navigation block, along with an additional block for each book? Looks a lot simpler to me. I think I spotted a bug: orphaned book pages (or possibly book pages that are unpublished) appear to be getting book navigation blocks. ------------------------------------------------------------------------ December 11, 2004 - 17:34 : nysus I'll see if I can fix the bug. Might be tricky. But I don't understand your recommendation to "export the current book navigation block, along with an additional block for each book". Can you expand on this thought? ------------------------------------------------------------------------ December 11, 2004 - 18:06 : nysus Dries, I am unable to duplicate the bug. I have three orphaned pages. I also tried unpublishing some pages. But as far as I can tell, the patch works as expected. ------------------------------------------------------------------------ December 11, 2004 - 19:00 : Dries If you can't reproduce the problem, chances are my node/book table is somewhat fubar. As for the configuration option. I suggest removing it and to always make these new blocks available on the /admin/block configuration screen. ------------------------------------------------------------------------ December 11, 2004 - 19:52 : moshe weitzman I am hoping that we maintain the option to keep the behavior where the appropriate book block only shows up when its book page viewed. this is a nice, tidy arrangement. ------------------------------------------------------------------------ December 11, 2004 - 20:03 : nysus Yes, if you don't enable any of the individual book blocks, the a book's block (i.e. navigation menu) will only appear when a page in a book is viewed. In other words, you have the option to have the book block act like this patch isn't even installed. ------------------------------------------------------------------------ December 11, 2004 - 20:54 : Dries I guess I'll have to try the patch again, because I don't understand why it works like this -- or at least, why it can't be made simpler. ------------------------------------------------------------------------ December 11, 2004 - 21:13 : nysus Can you be more specific? Why it works like what? ------------------------------------------------------------------------ December 11, 2004 - 21:24 : nysus Attachment: http://drupal.org/files/issues/book_24.patch (4.1 KB) This patch reverses a change made in the last patch which required a user to enable individual book block before they could enable any individual book blocks. ------------------------------------------------------------------------ December 11, 2004 - 21:41 : andremolnar As I mentioned earlier I am *fine* with either version of this patch as long as it makes it to core. But, as I said earlier, I clearly think the preference for admins would be to have the option to enable or disable this functionality. BTW - if this does make it in, I would be happy to create a Handbook page that describes the new features - something like "how to build robust site navigation using the book module". (on or after December 17th). andre ------------------------------------------------------------------------ December 12, 2004 - 12:11 : Anonymous I am not at all happy with these features. They are incosistant, confusing and should use exising methods and UIs. May main concern is the incosistancy: it is confusing, will require extra attention with each core code change, adds extra logic to the core, and is not re-useable. so here are my questions: 1) why do we not use the menu system and apis to build and adminster the trees? Saves code, does not add extra UIs, and gives users more power. 2) why do we need that extra showing logic? a book block should not get exeptional if clauses, it should use the existing path setting methods on block admin. extra logic is confusing for administrators (hey, i set the path so that the book-block should show up here, but it does not, why?) we should really not provide extra logic in the block hook, but should rather use default settings in block admin (the book could fill the bookblock sql cells with custom paths, for example) 3) we should avoid extra UIs. We already have far too much, and far too much different ones. Please rather improve the block admin, than add new separate interfaces. 4) why do book blocks need al these expeptions in the first place? If they are so exeptional, we could consider not using blocks, but something else, like in-line book layout (pages with the index etc) 5) why did you not choose for a general, standard, block gerneation API? that way modules, such as taxonomy, image gallery, weblinks, article, etc can reuse it and introduce block gernation. All that sayd, i like the idea of this functionality, but i fear for a great useability downfall if we start introducing all sorts of exeptions for all sorts of modules. Because now chances are very big that taxonomy, image gallery etc will need to introduce other UIs, other code, new methods and new documentation, if they too want some sort of better block handling. so a -1 from me. ------------------------------------------------------------------------ December 12, 2004 - 12:12 : Bèr Kessels ^^--- That was me (bèr kessels) forgot to log in..... ------------------------------------------------------------------------ December 12, 2004 - 12:34 : nysus I'm not going to pretend I can argue if my patch does or does not fit well into Drupal's larger architecture. My motivation for writing it is that I had an immediate need to create an easy way to make it easy for users to create menus. I'll let others decide whether or not the patch has merit from the big picture perspective. But if it doesn't, why not just use it for its immediate benefits and then throw it away when something better comes along? ------------------------------------------------------------------------ December 12, 2004 - 13:01 : Dries My summary is this: +1 on the functionality, -1 on the implementation. The code itself is good, but the usability/integration is not. ------------------------------------------------------------------------ December 12, 2004 - 13:13 : nysus When you say "usability" is that from the user's perspective or the code maintainers? I'm guessing it's the latter but I'm unsure. What about the idea of using the patch until a more permanent solution comes along? Yes, it's much better to live in a home with indoor plumbing but why not use the outhouse while you wait for a toilet to get installed? Or are there other considerations I'm overlooking that would make this a bad idea? ------------------------------------------------------------------------ December 12, 2004 - 13:20 : Goba nysus it is just generally against the Drupal philosophy to add improperly implemented functionality until something better comes along. There even used to be ocassions in Drupal releases, when some functionality was removed (not fixed) for a release, because its implementation was not adequate. ------------------------------------------------------------------------ December 12, 2004 - 13:55 : Dries Usability for the user. The extra setting on the block configuration page is both confusing and awkward. I don't understand why things must be configured/enabled that way (see my and Ber's previous comments on this issue). ------------------------------------------------------------------------ December 12, 2004 - 14:03 : nysus Well, just for the record, I reversed that functionality per your suggestion and uploaded the patch. The indiviudal book blocks now appear automatically. ------------------------------------------------------------------------ December 13, 2004 - 13:56 : Bèr Kessels Hi, I am sure you can make not only simpler, but better usefull for admins and users. All you need to do is use the menus for this. i.e. make a menu entry for each book page. For each book make a menu on level 0, without a parent. that way they become a seoprate menu, each with an own block. it saves code, makes things more consistent, and most important, uses drupal functionality where it should. ------------------------------------------------------------------------ December 13, 2004 - 14:50 : nysus Ber, Are you suggesting that for every single book page that a menu item be created? That's really not practical. That was my main motivation for writing this patch: to make it easy to put links, not necessarily related to one another, into a block. Any more pages than 10 and the sheer tedium of the job would prevent anyone from ever doing that. The menu.module is great, but adding new menu items is far from quick and painless. I just added about 10 to my menu for different taxonomies on my site and it wasn't fun. Plus, if you do as you suggest, there is also the problem of the book showing up twice. It will be generated by the menu and then it will be generated again by the book module which is programmed to design a block. You'd have to put some logic in the book.module _block hook to try to anticipate if a user has enabled a book in the menu. That wouldn't be pretty code. I'm all for putting automatic generation of book navigation blocks as part of the menu module. It does make more sense to have it there. But it forces me to ask the question: "Then why do we currently have code in the book module that generates a menu? Shouldn't that belong in the menu.module, too?" ------------------------------------------------------------------------ December 13, 2004 - 22:31 : killes(a)www.drop.org I think what Ber is trying to say is that you can write a contrib module that monitors the changes to the book table and creates menu items automatically. nodeapi is your friend. I would also prefer this solution. ------------------------------------------------------------------------ December 15, 2004 - 16:39 : Anonymous "Then why do we currently have code in the book module that generates a menu? Shouldn't that belong in the menu.module, too?" Because it's old code. It would be nice if book.module generated this block using its _menu hook, so that the admin would have a few options in terms of configuration. Plus, if you do as you suggest, there is also the problem of the book showing up twice. It will be generated by the menu and then it will be generated again by the book module which is programmed to design a block. No. The old code which manually builds a block in book.module would be removed. Book blocks would only be generated by the menu. This would also have the added benefit of allow the administrator to easily place a book in an appropriate spot in the menu tree, while still allowing the possibility of displaying it in a separate block. Because of menu caching, I don't expect a large performance hit for creating the menu items. That was my main motivation for writing this patch: to make it easy to put links, not necessarily related to one another, into a block. It sounds like instead of (mis)using book.module, your time would be better spent in a usability improvement to menu.module so it's easier to do this. ------------------------------------------------------------------------ December 16, 2004 - 06:55 : nysus Thanks for the feedback and input. I appreciate it. However, I would also appreciate if you took more care to avoid the condescending tone in your post: It sounds like instead of (mis)using book.module, your time would be better spent in a usability improvement to menu.module so it's easier to do this. It's really quite unnecessary and off-putting. Though it won't stop me from contributing to Drupal in the future, I'm sure others would be really turned off by such a patronizing comment and it could dissuade them. I'd like the Drupal community to be a welcoming and friendly place that will inspire people to contribute, not discourage them. Thanks. ------------------------------------------------------------------------ December 16, 2004 - 10:07 : Bèr Kessels nyesus, Please do not start /that/ discussion here. :) Drupal community is known fo being direct, maybe because of the big number of western-Europeans attending, maybe because of other reasons. No-one commented that you are wasting your time. But the commentor was telling you somthing likethis: "If you would follow the previous sugeestions, your added feature would be much better appreciated, and will probably work much better for you too". He/she was by no means telling you to stop your silly coding, or anything in that line. He/She only wanted to show you the obvious and better direction. We often deal with issues that add some feature, and a complete new UI, because the author does not like, or cannot use the existing UIs and features. This is nogt good, because if that same author would have spend his/her time on improving that existing functionality or UI (improving is not neccesarily the same as extending!!) that code and time would benefit all much better. Thats what the commentor tried to say. And so is it here: If you dislike the way the books handle the blocks, and if you do not want to use the menu, because you do not like its UI, then do not add another UI and more features, but rather merge these, and improve the parts (in the menu) you dislike. ------------------------------------------------------------------------ December 16, 2004 - 10:35 : Dries We can worry about the menu integration later. Let's focus on the new option's usability/interaction design first. ------------------------------------------------------------------------ December 16, 2004 - 14:51 : nysus I understand what the commenter was saying and like I said, I appreciate and understand it. I'm not upset and I'm not looking for an argument, I was just being direct as well. :) As part of the Drupal community (albeit a minor player), all I'm saying is that I would like to see folks not have a tin ear to the humanity in all of us. It will help make Drupal an even stronger community and attract even more talented developers. Human interaction is part of the development process. Whether we like it or not, we must cope and deal with it. ------------------------------------------------------------------------ December 18, 2004 - 09:54 : Dries I thought some more about this and am starting to believe that integration with the menu system should take priority. Here are common cool scenario's: I want to create a separate navigation block for the 'Drupal handbook'. I want to add a menu item called 'handbook' to the user block. That is, I want the book navigation to be part of the existing user navigation block. I want to add a menu item called 'handbook' to the top-level navigation menu. How would that work from a user's point of view? What do I have to click and where to configure this? ------------------------------------------------------------------------ December 21, 2004 - 18:47 : andremolnar I was actually thinking about the same thing last night (must have been something in the arctic air). 2. I want to add a menu item called 'handbook' to the user block. That is, I want the book navigation to be part of the existing user navigation block. 3. I want to add a menu item called 'handbook' to the top-level navigation menu. This is already possible (to a certain extent) with the current Book.module and Menu.module - A 'Books' menu item is created in the user navigation by having the Book.module enabled. Menu.module allows you to enable/disable this menu item. Menu.module also allows you to re-name this menu item. But, this only helps if you only intend to have 1 book (or else the name 'Handbook' is misleading if the user finds more than 1 book listed). - so this isn't good enough (or is it). 1. I want to create a separate navigation block for the 'Drupal handbook'. This is what I was trying to figure out. Not just this, but a different way to do what Nysus was attempting. i.e. create a menu block for each and every book that is created. There is a way to write code that would (re)build a 'custom menu' on every add/edit/update to a book page - or book outline update. Custom menu's automatically have a block created for them. But, this I think would be a misleading use of the 'custom' menu - as the menu would not be custom if they are a part of core. So, I would think that two new constants could be added to the menu.inc file - MENU_BOOK_MENU - and MENU_BOOK_ITEM - each would behave as custom menu's, but would be reserved for books. These menu types should NOT show up in the Menu.module administration - because the administration of the book_menu items would be done by administering the book itself (adding an item, removing an item, moving an item up/down in the hierarchy, assigning parents etc.). However, the blocks that the book_menus would create would show up in the block administration (so users could enable/disable each block - and decide where on the site they show up). The existing book block logic would be removed. So the logic would be: If a creates a book in the book administration page - the Book.module automatically creates a new MENU_BOOK_MENU Any time a user adds to or updates or delets a book page - the entire book menu is deleted and recreated based on the hierarchy defined by the book itself. The blocks for each book would show up in block administration. Any thoughts - I know that this doesn't exactly address points 1 and 2 - but it could act as a first step. Is this approach a bad idea? It would add special conditions for the proposed book menu's - but books would be a special case. Even if people don't like this solution, maybe it will give someone a better idea. I'd love to hear them. andre ------------------------------------------------------------------------ December 21, 2004 - 22:55 : Boris Mann +1 to this andre I had promised to put down my thoughts on this matter, as it relates to 1) primary and secondary links and how they are managed and 2) auto-generation of primary/secondary navigation based on book outlines So, for 1), we currently have functional-but-not-very-usable plain textfields to manage primary and secondary links. I would like to see menu.module used to control all navigation links, whether it is the navigation block or primary/secondary links. What is needed: a) default system menus labelled "primary" and "secondary" which would store; this is where modules could insert navigation b) support for full URLs (e.g. http://myexternaldomain.com) instead of just path 2) if we got 1 working, and andre does his book menus, this could get taken care of automatically. Basically, for brochureware/business/etc. sites that have static content, you could have a root book as one of the primary navigation links, and then the secondary links are generated automatically. ------------------------------------------------------------------------ December 22, 2004 - 21:02 : Dries I agree with Andre that the book module's integration with the menu system needs to be worked on. I support any effort that makes it easier to structure pages and that makes it easy to link pages/nodes from within a menu. However, I'm opposed to putting book module specific names in the menu system (eg. MENU_BOOK_MENU and MENU_BOOK_ITEM). I can imagine a handful of modules that want to maintain a menu tree (or part thereof) so I'm in favor of generic names. I'd have to read up on the menu system code, but last time I checked there was a MENU_MODIFIABLE_BY_ADMIN flag. You could choose not to set this flag for the menu items generated by the book module. Maybe it's already possible to implement to implement Andre's suggestion without having to modify/extend the menu system. Are you exploring this path? ------------------------------------------------------------------------ December 24, 2004 - 10:01 : andremolnar I've had a few spare hours to work on this. I've started to cobble together a solution - but in doing so I discovered a bug in the menu module (for which I will submit a seperate issue - if one doesn't already exist). The principal I suggested works. I added some code to the book module that creates custom menu's (MENU_CUSTOM_MENU with MENU_CUSTOM_ITEMs) for each Book that exists in a site. This is just as a proof of concept - I chose this menu type to start with because they automatically have a block created for them in admin/block (which is ultimatly the functionality we want). I ran a test and the menus are created as expected - the blocks are also created. But when I tested the menu blocks by enabling them I ran into a problem. It seems as though the menu system does not expand/explode sub menu items if the node type is book. I'm not sure why this is, and I haven't traced the source code yet - I thought I would ask if anyone has intimate knowledge of the menu system if they can point me in the right direction. No patch attached because until that problem is fixed this proposed change won't fly :( andre ------------------------------------------------------------------------ December 24, 2004 - 11:11 : Dries Just a wild guess: maybe it doesn't work because the book module's URI scheme is not hierarchical? ------------------------------------------------------------------------ January 7, 2005 - 10:45 : andremolnar Attachment: http://drupal.org/files/issues/book_module_1.patch (5.1 KB) I finally took some time out to do some work on this. As mentioned in a post to dev the problem I was having with menus not expanding/contracting properly was some crud in my database. Once that was cleared up my changes worked fine. I am attaching a patch for comments and for the brave willing to give it a test drive. History: This thread and http://drupal.org/node/15198 and http://drupal.org/node/15153 node/15198 has a patch that is required for this patch to work. What this does: Pretty straight forward. Any time a user adds/updates/deletes book or book pages (including via outline) - a function is called to create a new menu for each book. Any existing book menus are wiped out and then the new book menus are inserted - and the system menu is rebuilt to reflect the changes. The menus created consist of type MENU_MODULE_MENU and MENU_MODULE_ITEM. These menus show up in the menu/admin page so that admins can be aware of them, but the menu types are not editable via menu/admin (all changes are handled by the book module). Since the menus are in the menu table, the menu_block() hook handles the creation of the blocks for each of them. The blocks can then be administered in the usual ways via the block/admin interface. KNOWN ISSUE: (suggested solutions welcome) Since the menu table is updated on every change to books - the blocks associated with the menus are also recreated with default settings (i.e. disabled, and with no path or throttle settings) requiring a user to take an extra step and re-configure the book blocks for viewing on their site. I think this is unacceptable. For the casual user of the book module that only has a single book that doesn't change often, this would not be a big deal. But, if anyone wanted to make use of book module to handle dynamic site navigation this would create more work than it saves. Looking forward: If the block generation issue can be solved in a tidy way, this patch could allow users to use book to handle all their site navigation generation needs. Also, this patch could allow for the removal of a large chunk of code in the book module dedicated to building its own block via the block system. I left it there for now because I suppose there may be those out there that want to have book.module work the way book.module always worked (only show the book block when viewing a book page). Even so, since each book would have its own block, an admin could specify when and where the block shows via admin/block. I would appreciate feedback. If nothing else I hope this gives someone some new ideas. I'll continue to work on the block regeneration issue. andre ------------------------------------------------------------------------ January 7, 2005 - 10:47 : andremolnar Attachment: http://drupal.org/files/issues/book_module_2.patch (4.98 KB) sorry - here's a patch with prefered line breaks. andre ------------------------------------------------------------------------ January 7, 2005 - 16:47 : nysus Andre, I've been meaning to give this a throrough look when I get a chance. Hopefully this weekend. ---Steve ------------------------------------------------------------------------ January 20, 2005 - 02:02 : andremolnar Attachment: http://drupal.org/files/issues/book_module_3.patch (6.66 KB) Here is an updated patch. Same comments as followup 51 - except that the known issue has been resolved. This changes book module so that any action taken on a book, including adding new books or book pages will create a menu in the menu system for that book - and thus create blocks for those menus that can be administered in the usual ways. This is my first attempt at a major patch to core - comments are welcome I will be happy to update documentation once revisions to the code have been taken care of. andre ------------------------------------------------------------------------ January 20, 2005 - 02:53 : andremolnar Attachment: http://drupal.org/files/issues/book_module_4.patch (6.17 KB) Sorry - would be nice if I removed some debugging code - ;-) also previous patch also would have introduced a need for a change to the menu table that isn't required yet. This patch is a correct version andre ------------------------------------------------------------------------ January 20, 2005 - 19:16 : Dries The menu is recreated every time a book page is updated. I believe this is unwanted behavior because it requires the book block to be reconfigured upon every update. ------------------------------------------------------------------------ January 20, 2005 - 19:32 : andremolnar The menu is indeed re-created with every book page edit - because if the book page title changes the menu needs to reflect this change. The book block re-configuration is not required by the user. The code stores this information and re-sets the block settings. I will see if I can test for 'title change' before forcing the re-build of the menu - It would save a bit of processing power. andre ------------------------------------------------------------------------ January 20, 2005 - 20:21 : andremolnar Took another at this - and it turns out that I already have a check to see if title or weight change (rather - they already existed in the module and I used them). andre ------------------------------------------------------------------------ January 24, 2005 - 02:22 : andremolnar Attachment: http://drupal.org/files/issues/book_module_6.patch (7.42 KB) Feedback received indicated that the original block generation code in this module should be removed - since this patch hands block generation off to the menu and block system. This patch removes that code. BUT - It should be noted, that in order to have book blocks only show up on pages of node type book - an additional patch found at http://drupal.org/node/16074 is required. andre -- View: http://drupal.org/node/14120 Edit: http://drupal.org/project/comments/add/14120

1 0

[drupal-devel] [feature] Testers needed: Node Type filtering in taxonomy on Drupal 4.5
by Prometheus6 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: matteo Reported by: matteo Updated by: Prometheus6 Status: patch How about: taxonomy/$foo,$bar taxonomy/$foo,$bar/forum taxonomy/$foo,$bar/forum,book taxonomy/$foo,$bar/forum/feed Prometheus6 Previous comments: ------------------------------------------------------------------------ October 25, 2004 - 16:20 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_filter.patch (5.46 KB) The attached fix wants to add a new feature to taxonomy.module: the ability to filter data also on node types. This is a need when you have to share one vocabulary between multiple node types. One example is the vocabulary 'Geography', containing all Cities and Regions. You could have events, stories and other custom made node types, sharing this vocabulary. The need arises when you want to see only events of a certain region; if you filter only on region, you'll see also stories and the other custom node types. The new syntax is the following: taxonomy/term/term1,term2,term3(nodetype1,nodetype2)/........ to filter based on three terms (in OR each others) and on two different node types or taxonomy/term/term1+term2+term3(nodetype1,nodetype2)/........ to filter based on three terms (in AND each others) and on two different node types The rest of the syntax for taxonomy/term remains the same, and the patch works also for taxonomy feed. Node types are tested for validity. You can specify one or more node types; if you omit node types, normal term filtering is issued. Feedbacks and help testing this patch is needed; personally, I think this would improve temendously taxonomy functionalities, and hope this will be committed to Drupal core. Feel free to contact me by mail: m dot ferrari at tiscali dot it Matteo ------------------------------------------------------------------------ October 25, 2004 - 16:25 : moshe weitzman see http://drupal.org/node/5040 ------------------------------------------------------------------------ October 25, 2004 - 16:46 : matteo I agree with you that this is an old story, but nothing has been committed to 4.5, that introduced also a new taxonomy syntax. My patch has also some pluses: - it is specific for 4.5 and takes care of the new taxonomy syntax - it lets you specify multiple node types - syntax is clean Matteo ------------------------------------------------------------------------ January 19, 2005 - 17:05 : robclay Will this work with 4.5.2? If so - I am looking for exactly this funcionality! ------------------------------------------------------------------------ January 20, 2005 - 00:43 : matteo I'll work on a port for 4.5.2 hopefully next week... I'll keep you informed. Matteo ------------------------------------------------------------------------ January 23, 2005 - 18:19 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_452.patch (4.43 KB) Here is the patch that adds nodetype filtering capabilities to 4.5.2 taxonomy module. Syntax is backwards compatible, so if you plan not to use node type filtering, syntax does not change at all. (see previous postings in this thread) I would need extensive testing and feedback... and hope this functionality will be included into Drupal core. Matteo ------------------------------------------------------------------------ January 23, 2005 - 18:38 : Steven New features only get added to the HEAD branch... you'll have to update your patch. ------------------------------------------------------------------------ January 24, 2005 - 01:56 : matteo Ok, Steven, I'll rewrite the patch also for CVS.... Thanks Matteo ------------------------------------------------------------------------ January 24, 2005 - 04:16 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_CVS.patch (4.05 KB) Here is the patch for Drupal CVS. I noticed the great addition of node_rewrite_sql... very powerful... I'm starting extensive testing on the patch, and I really woul like it to be added to core taxonomy, because it becomes a must for several sites... Matteo ------------------------------------------------------------------------ January 24, 2005 - 09:43 : moshe weitzman I'd rather see this done universally (not just for taxonomy urls) using the new sql rewrite capability. specifically, some code will implement the rewrite_sql hook and look for a querystring param &ntype=blog,story and then inject the proper WHERE clause. this will be very useful for tracker, home page, taxonomy urls, etc. ------------------------------------------------------------------------ January 24, 2005 - 10:59 : Dries Before looking at the code (which violates Drupal's coding style), what do people think about the proposed URL scheme? Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? How about we change that to: taxonomy/all/$foo (or taxonomy/$foo) taxonomy/forum/$foo taxonomy/forum,book/$foo ... instead of the proposed URL scheme? It looks slightly more readable to me. (Does anyone know what 'synonyms' are for, and whether they could solve this problem? Really, I have no idea what these are for, even after having read the documentation.) ------------------------------------------------------------------------ January 24, 2005 - 11:15 : jvandyk Synonyms are useful for things like organisms, which have both a common name and a scientific name. That's the idea of synonyms, not necessarily the Drupal implementation of synonyms, which we plan to use but are not yet using. ------------------------------------------------------------------------ January 24, 2005 - 11:57 : Steven Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? I believe this is because when the URL changes were discussed, there was talk of a 'taxonomy/vocabulary' page which would display an overview of a vocabulary. But I think that role has already been filled by a contributions module. ------------------------------------------------------------------------ January 24, 2005 - 12:05 : Prometheus6 If taxonomy/$foo retrieves all node types then I think taxonomy/$foo/forum taxonomy/$foo/forum,book is the cleanest looking way to be specific. Give it a fixed position because it will always be there whereas the node type may or may not be. ------------------------------------------------------------------------ January 24, 2005 - 12:24 : Steven This could be hard to implement as $foo is a multi-var path. e.g. "taxonomy/terms/1" "taxonomy/terms/1/1" "taxonomy/terms/1/0/feed" etc. -- View: http://drupal.org/node/12114 Edit: http://drupal.org/project/comments/add/12114

1 0

[drupal-devel] [feature] Testers needed: Node Type filtering in taxonomy on Drupal 4.5
by Steven 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: matteo Reported by: matteo Updated by: Steven Status: patch This could be hard to implement as $foo is a multi-var path. e.g. "taxonomy/terms/1" "taxonomy/terms/1/1" "taxonomy/terms/1/0/feed" etc. Steven Previous comments: ------------------------------------------------------------------------ October 25, 2004 - 22:20 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_filter.patch (5.46 KB) The attached fix wants to add a new feature to taxonomy.module: the ability to filter data also on node types. This is a need when you have to share one vocabulary between multiple node types. One example is the vocabulary 'Geography', containing all Cities and Regions. You could have events, stories and other custom made node types, sharing this vocabulary. The need arises when you want to see only events of a certain region; if you filter only on region, you'll see also stories and the other custom node types. The new syntax is the following: taxonomy/term/term1,term2,term3(nodetype1,nodetype2)/........ to filter based on three terms (in OR each others) and on two different node types or taxonomy/term/term1+term2+term3(nodetype1,nodetype2)/........ to filter based on three terms (in AND each others) and on two different node types The rest of the syntax for taxonomy/term remains the same, and the patch works also for taxonomy feed. Node types are tested for validity. You can specify one or more node types; if you omit node types, normal term filtering is issued. Feedbacks and help testing this patch is needed; personally, I think this would improve temendously taxonomy functionalities, and hope this will be committed to Drupal core. Feel free to contact me by mail: m dot ferrari at tiscali dot it Matteo ------------------------------------------------------------------------ October 25, 2004 - 22:25 : moshe weitzman see http://drupal.org/node/5040 ------------------------------------------------------------------------ October 25, 2004 - 22:46 : matteo I agree with you that this is an old story, but nothing has been committed to 4.5, that introduced also a new taxonomy syntax. My patch has also some pluses: - it is specific for 4.5 and takes care of the new taxonomy syntax - it lets you specify multiple node types - syntax is clean Matteo ------------------------------------------------------------------------ January 19, 2005 - 23:05 : robclay Will this work with 4.5.2? If so - I am looking for exactly this funcionality! ------------------------------------------------------------------------ January 20, 2005 - 06:43 : matteo I'll work on a port for 4.5.2 hopefully next week... I'll keep you informed. Matteo ------------------------------------------------------------------------ January 24, 2005 - 00:19 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_452.patch (4.43 KB) Here is the patch that adds nodetype filtering capabilities to 4.5.2 taxonomy module. Syntax is backwards compatible, so if you plan not to use node type filtering, syntax does not change at all. (see previous postings in this thread) I would need extensive testing and feedback... and hope this functionality will be included into Drupal core. Matteo ------------------------------------------------------------------------ January 24, 2005 - 00:38 : Steven New features only get added to the HEAD branch... you'll have to update your patch. ------------------------------------------------------------------------ January 24, 2005 - 07:56 : matteo Ok, Steven, I'll rewrite the patch also for CVS.... Thanks Matteo ------------------------------------------------------------------------ January 24, 2005 - 10:16 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_CVS.patch (4.05 KB) Here is the patch for Drupal CVS. I noticed the great addition of node_rewrite_sql... very powerful... I'm starting extensive testing on the patch, and I really woul like it to be added to core taxonomy, because it becomes a must for several sites... Matteo ------------------------------------------------------------------------ January 24, 2005 - 15:43 : moshe weitzman I'd rather see this done universally (not just for taxonomy urls) using the new sql rewrite capability. specifically, some code will implement the rewrite_sql hook and look for a querystring param &ntype=blog,story and then inject the proper WHERE clause. this will be very useful for tracker, home page, taxonomy urls, etc. ------------------------------------------------------------------------ January 24, 2005 - 16:59 : Dries Before looking at the code (which violates Drupal's coding style), what do people think about the proposed URL scheme? Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? How about we change that to: taxonomy/all/$foo (or taxonomy/$foo) taxonomy/forum/$foo taxonomy/forum,book/$foo ... instead of the proposed URL scheme? It looks slightly more readable to me. (Does anyone know what 'synonyms' are for, and whether they could solve this problem? Really, I have no idea what these are for, even after having read the documentation.) ------------------------------------------------------------------------ January 24, 2005 - 17:15 : jvandyk Synonyms are useful for things like organisms, which have both a common name and a scientific name. That's the idea of synonyms, not necessarily the Drupal implementation of synonyms, which we plan to use but are not yet using. ------------------------------------------------------------------------ January 24, 2005 - 17:57 : Steven Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? I believe this is because when the URL changes were discussed, there was talk of a 'taxonomy/vocabulary' page which would display an overview of a vocabulary. But I think that role has already been filled by a contributions module. ------------------------------------------------------------------------ January 24, 2005 - 18:05 : Prometheus6 If taxonomy/$foo retrieves all node types then I think taxonomy/$foo/forum taxonomy/$foo/forum,book is the cleanest looking way to be specific. Give it a fixed position because it will always be there whereas the node type may or may not be. -- View: http://drupal.org/node/12114 Edit: http://drupal.org/project/comments/add/12114

1 0

[drupal-devel] [feature] Testers needed: Node Type filtering in taxonomy on Drupal 4.5
by Prometheus6 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: matteo Reported by: matteo Updated by: Prometheus6 Status: patch If taxonomy/$foo retrieves all node types then I think taxonomy/$foo/forum taxonomy/$foo/forum,book is the cleanest looking way to be specific. Give it a fixed position because it will always be there whereas the node type may or may not be. Prometheus6 Previous comments: ------------------------------------------------------------------------ October 25, 2004 - 16:20 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_filter.patch (5.46 KB) The attached fix wants to add a new feature to taxonomy.module: the ability to filter data also on node types. This is a need when you have to share one vocabulary between multiple node types. One example is the vocabulary 'Geography', containing all Cities and Regions. You could have events, stories and other custom made node types, sharing this vocabulary. The need arises when you want to see only events of a certain region; if you filter only on region, you'll see also stories and the other custom node types. The new syntax is the following: taxonomy/term/term1,term2,term3(nodetype1,nodetype2)/........ to filter based on three terms (in OR each others) and on two different node types or taxonomy/term/term1+term2+term3(nodetype1,nodetype2)/........ to filter based on three terms (in AND each others) and on two different node types The rest of the syntax for taxonomy/term remains the same, and the patch works also for taxonomy feed. Node types are tested for validity. You can specify one or more node types; if you omit node types, normal term filtering is issued. Feedbacks and help testing this patch is needed; personally, I think this would improve temendously taxonomy functionalities, and hope this will be committed to Drupal core. Feel free to contact me by mail: m dot ferrari at tiscali dot it Matteo ------------------------------------------------------------------------ October 25, 2004 - 16:25 : moshe weitzman see http://drupal.org/node/5040 ------------------------------------------------------------------------ October 25, 2004 - 16:46 : matteo I agree with you that this is an old story, but nothing has been committed to 4.5, that introduced also a new taxonomy syntax. My patch has also some pluses: - it is specific for 4.5 and takes care of the new taxonomy syntax - it lets you specify multiple node types - syntax is clean Matteo ------------------------------------------------------------------------ January 19, 2005 - 17:05 : robclay Will this work with 4.5.2? If so - I am looking for exactly this funcionality! ------------------------------------------------------------------------ January 20, 2005 - 00:43 : matteo I'll work on a port for 4.5.2 hopefully next week... I'll keep you informed. Matteo ------------------------------------------------------------------------ January 23, 2005 - 18:19 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_452.patch (4.43 KB) Here is the patch that adds nodetype filtering capabilities to 4.5.2 taxonomy module. Syntax is backwards compatible, so if you plan not to use node type filtering, syntax does not change at all. (see previous postings in this thread) I would need extensive testing and feedback... and hope this functionality will be included into Drupal core. Matteo ------------------------------------------------------------------------ January 23, 2005 - 18:38 : Steven New features only get added to the HEAD branch... you'll have to update your patch. ------------------------------------------------------------------------ January 24, 2005 - 01:56 : matteo Ok, Steven, I'll rewrite the patch also for CVS.... Thanks Matteo ------------------------------------------------------------------------ January 24, 2005 - 04:16 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_CVS.patch (4.05 KB) Here is the patch for Drupal CVS. I noticed the great addition of node_rewrite_sql... very powerful... I'm starting extensive testing on the patch, and I really woul like it to be added to core taxonomy, because it becomes a must for several sites... Matteo ------------------------------------------------------------------------ January 24, 2005 - 09:43 : moshe weitzman I'd rather see this done universally (not just for taxonomy urls) using the new sql rewrite capability. specifically, some code will implement the rewrite_sql hook and look for a querystring param &ntype=blog,story and then inject the proper WHERE clause. this will be very useful for tracker, home page, taxonomy urls, etc. ------------------------------------------------------------------------ January 24, 2005 - 10:59 : Dries Before looking at the code (which violates Drupal's coding style), what do people think about the proposed URL scheme? Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? How about we change that to: taxonomy/all/$foo (or taxonomy/$foo) taxonomy/forum/$foo taxonomy/forum,book/$foo ... instead of the proposed URL scheme? It looks slightly more readable to me. (Does anyone know what 'synonyms' are for, and whether they could solve this problem? Really, I have no idea what these are for, even after having read the documentation.) ------------------------------------------------------------------------ January 24, 2005 - 11:15 : jvandyk Synonyms are useful for things like organisms, which have both a common name and a scientific name. That's the idea of synonyms, not necessarily the Drupal implementation of synonyms, which we plan to use but are not yet using. ------------------------------------------------------------------------ January 24, 2005 - 11:57 : Steven Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? I believe this is because when the URL changes were discussed, there was talk of a 'taxonomy/vocabulary' page which would display an overview of a vocabulary. But I think that role has already been filled by a contributions module. -- View: http://drupal.org/node/12114 Edit: http://drupal.org/project/comments/add/12114

1 0

[drupal-devel] [feature] Testers needed: Node Type filtering in taxonomy on Drupal 4.5
by Steven 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: matteo Reported by: matteo Updated by: Steven Status: patch Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? I believe this is because when the URL changes were discussed, there was talk of a 'taxonomy/vocabulary' page which would display an overview of a vocabulary. But I think that role has already been filled by a contributions module. Steven Previous comments: ------------------------------------------------------------------------ October 25, 2004 - 22:20 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_filter.patch (5.46 KB) The attached fix wants to add a new feature to taxonomy.module: the ability to filter data also on node types. This is a need when you have to share one vocabulary between multiple node types. One example is the vocabulary 'Geography', containing all Cities and Regions. You could have events, stories and other custom made node types, sharing this vocabulary. The need arises when you want to see only events of a certain region; if you filter only on region, you'll see also stories and the other custom node types. The new syntax is the following: taxonomy/term/term1,term2,term3(nodetype1,nodetype2)/........ to filter based on three terms (in OR each others) and on two different node types or taxonomy/term/term1+term2+term3(nodetype1,nodetype2)/........ to filter based on three terms (in AND each others) and on two different node types The rest of the syntax for taxonomy/term remains the same, and the patch works also for taxonomy feed. Node types are tested for validity. You can specify one or more node types; if you omit node types, normal term filtering is issued. Feedbacks and help testing this patch is needed; personally, I think this would improve temendously taxonomy functionalities, and hope this will be committed to Drupal core. Feel free to contact me by mail: m dot ferrari at tiscali dot it Matteo ------------------------------------------------------------------------ October 25, 2004 - 22:25 : moshe weitzman see http://drupal.org/node/5040 ------------------------------------------------------------------------ October 25, 2004 - 22:46 : matteo I agree with you that this is an old story, but nothing has been committed to 4.5, that introduced also a new taxonomy syntax. My patch has also some pluses: - it is specific for 4.5 and takes care of the new taxonomy syntax - it lets you specify multiple node types - syntax is clean Matteo ------------------------------------------------------------------------ January 19, 2005 - 23:05 : robclay Will this work with 4.5.2? If so - I am looking for exactly this funcionality! ------------------------------------------------------------------------ January 20, 2005 - 06:43 : matteo I'll work on a port for 4.5.2 hopefully next week... I'll keep you informed. Matteo ------------------------------------------------------------------------ January 24, 2005 - 00:19 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_452.patch (4.43 KB) Here is the patch that adds nodetype filtering capabilities to 4.5.2 taxonomy module. Syntax is backwards compatible, so if you plan not to use node type filtering, syntax does not change at all. (see previous postings in this thread) I would need extensive testing and feedback... and hope this functionality will be included into Drupal core. Matteo ------------------------------------------------------------------------ January 24, 2005 - 00:38 : Steven New features only get added to the HEAD branch... you'll have to update your patch. ------------------------------------------------------------------------ January 24, 2005 - 07:56 : matteo Ok, Steven, I'll rewrite the patch also for CVS.... Thanks Matteo ------------------------------------------------------------------------ January 24, 2005 - 10:16 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_CVS.patch (4.05 KB) Here is the patch for Drupal CVS. I noticed the great addition of node_rewrite_sql... very powerful... I'm starting extensive testing on the patch, and I really woul like it to be added to core taxonomy, because it becomes a must for several sites... Matteo ------------------------------------------------------------------------ January 24, 2005 - 15:43 : moshe weitzman I'd rather see this done universally (not just for taxonomy urls) using the new sql rewrite capability. specifically, some code will implement the rewrite_sql hook and look for a querystring param &ntype=blog,story and then inject the proper WHERE clause. this will be very useful for tracker, home page, taxonomy urls, etc. ------------------------------------------------------------------------ January 24, 2005 - 16:59 : Dries Before looking at the code (which violates Drupal's coding style), what do people think about the proposed URL scheme? Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? How about we change that to: taxonomy/all/$foo (or taxonomy/$foo) taxonomy/forum/$foo taxonomy/forum,book/$foo ... instead of the proposed URL scheme? It looks slightly more readable to me. (Does anyone know what 'synonyms' are for, and whether they could solve this problem? Really, I have no idea what these are for, even after having read the documentation.) ------------------------------------------------------------------------ January 24, 2005 - 17:15 : jvandyk Synonyms are useful for things like organisms, which have both a common name and a scientific name. That's the idea of synonyms, not necessarily the Drupal implementation of synonyms, which we plan to use but are not yet using. -- View: http://drupal.org/node/12114 Edit: http://drupal.org/project/comments/add/12114

1 0

[drupal-devel] [bug] Failure to respect node status upon editing (reverts to default)
by moshe weitzman 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: mathias Reported by: mathias Updated by: moshe weitzman Status: patch this patch fixes a fairly undesirable problem with our workflow. lets consider it along with other proposed changes to default workflow. moshe weitzman Previous comments: ------------------------------------------------------------------------ May 20, 2004 - 22: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 - 10: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 - 14: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 - 14: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 - 16: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 - 17: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 - 17: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 - 14: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 - 15: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 - 15: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 - 15: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 - 15: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 - 16: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 - 16: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 - 10: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 - 13: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 - 16: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 - 16:56 : mathias Attachment: http://drupal.org/files/issues/node_defaults_admin_content_0.png (18.68 KB) Latest screenshot of patch. ------------------------------------------------------------------------ October 28, 2004 - 15: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 28, 2004 - 18: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 - 14: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 -- View: http://drupal.org/node/7940 Edit: http://drupal.org/project/comments/add/7940

1 0

[drupal-devel] [feature] Testers needed: Node Type filtering in taxonomy on Drupal 4.5
by jvandyk 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: matteo Reported by: matteo Updated by: jvandyk Status: patch Synonyms are useful for things like organisms, which have both a common name and a scientific name. That's the idea of synonyms, not necessarily the Drupal implementation of synonyms, which we plan to use but are not yet using. jvandyk Previous comments: ------------------------------------------------------------------------ October 25, 2004 - 21:20 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_filter.patch (5.46 KB) The attached fix wants to add a new feature to taxonomy.module: the ability to filter data also on node types. This is a need when you have to share one vocabulary between multiple node types. One example is the vocabulary 'Geography', containing all Cities and Regions. You could have events, stories and other custom made node types, sharing this vocabulary. The need arises when you want to see only events of a certain region; if you filter only on region, you'll see also stories and the other custom node types. The new syntax is the following: taxonomy/term/term1,term2,term3(nodetype1,nodetype2)/........ to filter based on three terms (in OR each others) and on two different node types or taxonomy/term/term1+term2+term3(nodetype1,nodetype2)/........ to filter based on three terms (in AND each others) and on two different node types The rest of the syntax for taxonomy/term remains the same, and the patch works also for taxonomy feed. Node types are tested for validity. You can specify one or more node types; if you omit node types, normal term filtering is issued. Feedbacks and help testing this patch is needed; personally, I think this would improve temendously taxonomy functionalities, and hope this will be committed to Drupal core. Feel free to contact me by mail: m dot ferrari at tiscali dot it Matteo ------------------------------------------------------------------------ October 25, 2004 - 21:25 : moshe weitzman see http://drupal.org/node/5040 ------------------------------------------------------------------------ October 25, 2004 - 21:46 : matteo I agree with you that this is an old story, but nothing has been committed to 4.5, that introduced also a new taxonomy syntax. My patch has also some pluses: - it is specific for 4.5 and takes care of the new taxonomy syntax - it lets you specify multiple node types - syntax is clean Matteo ------------------------------------------------------------------------ January 19, 2005 - 22:05 : robclay Will this work with 4.5.2? If so - I am looking for exactly this funcionality! ------------------------------------------------------------------------ January 20, 2005 - 05:43 : matteo I'll work on a port for 4.5.2 hopefully next week... I'll keep you informed. Matteo ------------------------------------------------------------------------ January 23, 2005 - 23:19 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_452.patch (4.43 KB) Here is the patch that adds nodetype filtering capabilities to 4.5.2 taxonomy module. Syntax is backwards compatible, so if you plan not to use node type filtering, syntax does not change at all. (see previous postings in this thread) I would need extensive testing and feedback... and hope this functionality will be included into Drupal core. Matteo ------------------------------------------------------------------------ January 23, 2005 - 23:38 : Steven New features only get added to the HEAD branch... you'll have to update your patch. ------------------------------------------------------------------------ January 24, 2005 - 06:56 : matteo Ok, Steven, I'll rewrite the patch also for CVS.... Thanks Matteo ------------------------------------------------------------------------ January 24, 2005 - 09:16 : matteo Attachment: http://drupal.org/files/issues/taxonomy_nodetype_CVS.patch (4.05 KB) Here is the patch for Drupal CVS. I noticed the great addition of node_rewrite_sql... very powerful... I'm starting extensive testing on the patch, and I really woul like it to be added to core taxonomy, because it becomes a must for several sites... Matteo ------------------------------------------------------------------------ January 24, 2005 - 14:43 : moshe weitzman I'd rather see this done universally (not just for taxonomy urls) using the new sql rewrite capability. specifically, some code will implement the rewrite_sql hook and look for a querystring param &ntype=blog,story and then inject the proper WHERE clause. this will be very useful for tracker, home page, taxonomy urls, etc. ------------------------------------------------------------------------ January 24, 2005 - 15:59 : Dries Before looking at the code (which violates Drupal's coding style), what do people think about the proposed URL scheme? Can anyone tell me why we use 'taxonomy/term/$foo' and not just 'taxonomy/$foo'? How about we change that to: taxonomy/all/$foo (or taxonomy/$foo) taxonomy/forum/$foo taxonomy/forum,book/$foo ... instead of the proposed URL scheme? It looks slightly more readable to me. (Does anyone know what 'synonyms' are for, and whether they could solve this problem? Really, I have no idea what these are for, even after having read the documentation.) -- View: http://drupal.org/node/12114 Edit: http://drupal.org/project/comments/add/12114

1 0

[drupal-devel] [feature] Create extra form elements for comment form
by Dries 24 Jan '05

24 Jan '05

Project: Drupal Version: cvs Component: comment.module Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: Dries Status: patch I'd vote against a contentapi at this point: I don't think it adds real value. I'm OK with a comment API and a node API (which IMO should be called _node() not _nodeapi()). Dries Previous comments: ------------------------------------------------------------------------ November 26, 2004 - 22: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 - 22:34 : nysus Attachment: http://drupal.org/files/issues/comment_8.patch (278 bytes) Oops, would help to attach the patch, eh? ------------------------------------------------------------------------ November 26, 2004 - 23: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 - 23: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 27, 2004 - 00: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 27, 2004 - 00: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 27, 2004 - 00:06 : nysus Right, let's see what others think and go from there. Anyone? ------------------------------------------------------------------------ November 27, 2004 - 00: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 - 01: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 - 01: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 - 05: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 - 06: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 - 10: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 - 19: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 - 08: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 ------------------------------------------------------------------------ January 22, 2005 - 11:44 : stefan nagtegaal @ 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.. -- View: http://drupal.org/node/13539 Edit: http://drupal.org/project/comments/add/13539

1 0