Development

Download

development@drupal.org

January 2005

54 participants
341 discussions

[drupal-devel] [feature] Node type filter on node admin page
by Steven 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node.module Category: feature requests Priority: normal Assigned to: Steven Reported by: Anonymous Updated by: Steven Status: patch Simple. There aren't any guidelines on theme box usage :P. Plus, theming the admin interface is probably not a big priority. Steven Previous comments: ------------------------------------------------------------------------ August 23, 2004 - 17:09 : Anonymous Attachment: http://drupal.org/files/issues/node_admin_filter.patch (3.8 KB) Here's a patch for the current CVS version which adds the option to filter nodes by type on the node admin page. Having built a few sites recently which make heavy use of flexinodes, the ability to filter out particular node types would have been extremely useful, so I hope this makes it to 4.5.0. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ September 4, 2004 - 00:38 : Bèr Kessels I think we should really come up with a good solution for this screen. In the past, I went looking for such a solution and found that the various MP3 library tools might come to help. They deal with large amounts of metadata, some of wch are important, other are not. I summarise here what the UI could be like: ** The menu-based (tree) approach: Most mp3 collections use this idea. In a tree like structure on the side, you can make selections. The list in the main area will follow this selection. (Itunes: http://www.apple.com/itunes/playlists.html , Juk: http://developer.kde.org/~wheeler/images/juk-2.0/juk-2.png, finf/freeamp: http://www.zinf.org/images/zinf_mymusic_shot.png) In drupal that would mean a menu-based filtering method. I once made a patch for that, but that was rejected: http://drupal.org/node/view/7766 The good thing here, is that it is virtually endless scalable. So in theory, flexinode coud add menu items to show only nodes where foo is bar. | - nodetype | |- blog | |= Flexinode | | |=foo | | | |=bar | |- page ** The multiple selectlist UI: Another moethod is a extended method of what we have now: select items from select boxes and press submit. Above the main area one will find some selection UI where one can in- and exlude sertain criteria. (rhythmbox: http://www.rhythmbox.org/screenshots/Screenshot-Rhythmbox.png, and winamp http://www.winamp.com/player/walkthrough.php) Note that both still show some form of tree navigation. In drupal that would mean multiple selects in three columns (rhytmbox and winamp show only two artist and album) For drupal we need three of thos blocks: one for node-types, one for node-settings (promoted, etc) and one for taxonomy terms. The node settings should be logical too: If a criteria is selected it it true, if not: false. That way we can also get rod of the annoying logic: "view posts that are not promoted" and the likes. In both solutions the "perform batch action" needs to move somewhere else. I would plead for moving it to the bottom of the list. A lot of webmail cleints do it that way. The reason being simple: the steps you take to perform the update are: 1) select criteria (find nodes) 2) checkmark the chages (we need columns of checkboxes) 3) perform the update. Logically the way you go through the UI is left->right, top-> down (not taking into consideration japanese/hebrew/arabs etc) So all in all: i think indeed the UI needs serious work. I am willing to take this one me, bu I do need some consensus first. I am not willing to spend hours on a new UI (like the menu-based approach) to find the patch being rejected.Therefore I like some discussion up fron, rather than the " code first see later" approach. Regards, Ber ------------------------------------------------------------------------ September 5, 2004 - 23:42 : Bèr Kessels Attachment: http://drupal.org/files/issues/new_UI_proposal.png (175.31 KB) Here is a gimped mockup from what option 2) could look like. WE could add a fouth box with users too, but I am not sure how that would go with permissions and the like. I need to do some thinking about selecatibility by users first. ------------------------------------------------------------------------ September 6, 2004 - 00:01 : Bèr Kessels Sorry for the spam: but I forgot to mention that there are a few issues: The "perform action dropdown" should list stuff as: promote, demote, etc. But that list needs to be generated with the selection above in mind. So if you choose " published" the action list should not have "publish these nodes" Also the action list could contain "recategorise" wich would need another screen after the one in the "ugly screendump-gimp-mockup", to be able to choose the categories in wich the selection should be placed. And of course the delete these items with a confimation dialog. (http://drupal.org/node/view/7743#comment-11624) ------------------------------------------------------------------------ September 6, 2004 - 14:32 : Bèr Kessels Attachment: http://drupal.org/files/issues/node_admin.html (29.95 KB) After a long discussion with Steven And gerhard, I designed the page in HTML, to let you all try it in some way. Beware: the html is an ugly mock, It is meant as scetch, not as valid html. Some notes on the page: "View all nodes that are:" contains three differnet possibilities for searching. The first two (above the blank line) are there for simplicity, but they might confuse users, because they need a lot of text. The second idea uses a checkbox to enable the select next to it. The third one uses some ajvascript to improve useablity. We should of course choose one of these three methods. "View all nodes that are classified in:" There are quite some people that have questions with whether or not we should include this selection aat all. Here is why I think it should definately go Because most sites have only one purpose and thus a lot of nodes of one type in one state. For example a personal blog site: 99% of the nodes are blogs. 99% are promoted. Only 1 or 2 are not promoted, not published or of type page. Or take a photo-album-site. All posts will be images, the main difference is the classification (the galleries) I placed them in. "any/all" There must be a way to choose whether the sected options should be an AND or an OR. I tried three different methods, each block has one method. We should choose one. Please give me feedback, for I still have to decide whether or not to make this a contributed module or start lobby-ing to get it in core. ------------------------------------------------------------------------ September 13, 2004 - 11:50 : moshe weitzman looking good, and really useful. a few small comments: - consider upping the number of nodes that appear on this page. it is nearly pointless to use paging on this sort of UI - i like the second idea of an 'enabled' checkbox, without the javascript. i think you can use 2 checkboxes, where the second one is called 'publish', 'promote', and 'has comments' - I prefer the horizontal configuration of the 'Require: all any' radio buttons - don't repeat 'the selected posts' in the choices for the bottom dropdown. perhaps use a title for that form_select() instead. - the last choice in that dropdown is 'reclassify'. i don't see how this screen could support that. instead, use my taxonomy_multi_edit.module from contrib :) i suggest making this a contrib module at first. it can be a tab off of the current page. thats how taxonomy_multi_edit.module appears. ------------------------------------------------------------------------ September 13, 2004 - 13:03 : Bèr Kessels About that reclassify: That was exactly my intention: using the taxonomy_muliti_edit module for that. All I wanted to do is pass a list of nids to your module and open a screen using that module with the previously selected nodes! About the other comments: Yes upping the number of nodes might be a good idea. Anyone any idea what number would still be usefull. 10.000 nodes (on drupal.org) in one list is not a good idea, IMO, so some paging is needed. indeed: repeatinf words in dropdowns is considered "bad UI design" thank you for pointing it out. Regards, Ber ------------------------------------------------------------------------ September 28, 2004 - 13:37 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type.patch (1.96 KB) here is an updated version of this patch, syncing it with CVS HEAD. Also, i did change the functionality somewhat: previous versions of the patch asked the modules what node types were out there (via hook_node_name()). This version asks the node table what types are in there (via SELECT DISTINCT type FROM {node}). I find this more useful, because it's entirely possible that, over time, modules will be removed from the system, yet they still have rows in the node table. This method gives a more accurate view of the types of nodes on the system. The new UI looks excellent, but this patch does not address it. ------------------------------------------------------------------------ September 28, 2004 - 19:05 : drumm -1 I applied and tested and it did not work at all for me. The type options selection only contains 's' and 'W', selecting one does not work. The list of types seems to come from the DB which is probably an unnecessary query. ------------------------------------------------------------------------ September 29, 2004 - 12:51 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type_1.patch (2.31 KB) Here's a version of that patch that should work. Sorry. I think that getting the types from the node table is necessary. The list of installed nodes is not the authority on what types of nodes are in the db; we should ask the db directly. It's a small query on one admin page; the result is that the dropdown has in it the most accurate information available. That's worth it, imo. ------------------------------------------------------------------------ September 29, 2004 - 12:52 : eafarris s/list of installed nodes/list of installed modules/ . sheesh. teach me to post before my tea. ------------------------------------------------------------------------ October 21, 2004 - 18:15 : Bèr Kessels Attachment: http://drupal.org/files/issues/node.html (29.33 KB) I do not know where it weant, but my proposal had a HTML mockup. Attaching it here to see what i meant. ------------------------------------------------------------------------ October 23, 2004 - 13:14 : com2 Nice filter options, but what I really miss it the possibility to select all checkboxes. Really boring job when you have to click the whole list one by one. ------------------------------------------------------------------------ October 24, 2004 - 10:41 : Bèr Kessels I am not a big fan of those options in forms. They more often break things than that thy fix them. Rahter use a bookmarklet r so for that: http://www.squarefree.com/bookmarklets/forms.html Ber ------------------------------------------------------------------------ October 25, 2004 - 01:03 : Steven Attachment: http://drupal.org/files/issues/node_admin_ucd.html (29.83 KB) Based on some more IRC discussion, I came up with this (attachment). It is based on an earlier mockup. It does use Javascript and doesn't degrade without script (yet), but I think it's worth considering. It can be implemented quite cleanly. The idea would be to have a filter like this initially, and after clicking 'Go', to show the current filter as well as the form again, allowing you to refine your search. I haven't tested browser compatibility, but it should work on most browsers. The only ones I know for sure won't work are IE4 and NS4, which are old enough to ignore IMO: after all, this is admin only, and we can put some stricter browser requirements on that. Recent versions of other browsers should work (it only does a simple visibility toggle). I like it because it's not an airplane cockpit, but still offers complex filtering options through refinement. It is also quite similar to the search/filtering UI in e.g. mail applications. The question is of course, how to degrade it for people without Javascript. My suggestion would be to simply show all filter options, and put a radiobutton in front of each: ( ) Where type is [ blog | v ] ( ) Where published status is ( ) published ( ) unpublished ( ) Where category is [ term1 | v ] ... It has more visual clutter, but the same principle of use. ------------------------------------------------------------------------ October 25, 2004 - 01:13 : Steven In fact, I just realized that the radio-button version can be coded to send exactly the same form data as the scripted version, which makes it more interesting. ------------------------------------------------------------------------ October 29, 2004 - 00:52 : Steven If you guys approve of my idea, I'll start coding. Assigning to myself for now. ------------------------------------------------------------------------ November 1, 2004 - 17:35 : Bèr Kessels I like this idea much more than my previous approaches. Gave it some thought and came with the follwing behaviour: * I think we need to discuss or at least design the tabular data too: It makes 0 sense to show the "types" column, if you chose "type = image" * I think we need to introduce a hook to add info to that tabulat view. Images can output thumbnails, for example * In tabular view, we should make: title a textfield, type and author a select and satus a select too. * I think we should introduce another range of options: "where content contains" [ ] and "where title contains" [ ] * I would like to get some feedback on how many drupal-using people are firmly against javascript, with a reason why. I bet that this is < 1% ! Bèr ------------------------------------------------------------------------ November 1, 2004 - 17:42 : Steven We don't have an anti-JS policy in Drupal. It's just that until recently, doing cross browser JS was a chore and making it accessible and degradable would be a disaster. This form can be implemented cleanly and will degrade without problems. Anyone who opposes it because it is JS can bugger off ;). ------------------------------------------------------------------------ November 3, 2004 - 19:24 : com2 There is a more radical way around JS. You could forget the table idea alltogether and present the nodes in Listbox. Then selecting becomes much easier with Ctrl-Click and Shift-Click ------------------------------------------------------------------------ November 4, 2004 - 00:02 : Bèr Kessels Did you have a look at the previous applied UI improvements? Did you consider the pre's and cons of those earlier posts? It seams you are repeating passed issues here, please reply with miore detailed issues here, Bèr ------------------------------------------------------------------------ December 3, 2004 - 10:49 : Steven Attachment: http://drupal.org/files/issues/node-admin.patch (13.1 KB) Here's a patch which implements the JS approach of refining the selection. ------------------------------------------------------------------------ December 3, 2004 - 10:50 : Steven Attachment: http://drupal.org/files/issues/node_admin.png (34.38 KB) And here's a screenshot of how it looks. ------------------------------------------------------------------------ December 3, 2004 - 19:23 : FactoryJoe(a)civicspacelabs.org This is looking better and better. I'm going to do a UI review of this a post any additional thoughts or comments. Chris ------------------------------------------------------------------------ December 3, 2004 - 21:50 : Anonymous Ack, I tested this using the bluebeach theme, but there's a small CSS bug with bluemarine, where the header for the operations form is stuck to the right of the filter form. I'll make an updated patch later. In the meantime, comments are much appreciated. ------------------------------------------------------------------------ December 3, 2004 - 23:15 : killes(a)www.drop.org I actually prefer the non-JS menu over the JS menu. Not (only) because of the JS but because I prefer radiobuttons over pulldown menus. The argument that the non-JS approach uses more vertical space can be mitigated by redirecting to a #results anchor. ------------------------------------------------------------------------ January 15, 2005 - 06:32 : Steven Attachment: http://drupal.org/files/issues/node.admin.patch (14.41 KB) Here's an updated patch... I have exams in a week, so I don't have time to finish up the Javascript version, so I removed it (it can be added later easily). However, the non-JS version is still very usable. Changes since last time: - Uses semantic XHTML (definition list, and yes, they are for more than just definitions, even w3c agrees). - Merges the various node flags (promoted, published, ...) into one "status" dropdown. This keeps the whole selector shorter, and is better UI-wise IMO. - Adds drupal_gotos, they were missing. - Updates node admin watchog messages to properly display the type. The code used t($node->type) which is a big no-no. hook_node_name() is now invoked instead. - Removes some dead code. - I rolled Ber's node mass deletion patch in as it affects the same piece of code. Updating either patch for the other separately would be a waste of time. I also added some more improvements to Bèr's code, such as removing the incorrect help from the confirmation page, cleaning up the code style a bit, not loading every node just to display its title and displaying a single drupal_set_message() after the deleting rather than one per node. The old issue is here: http://drupal.org/node/7743 ------------------------------------------------------------------------ January 15, 2005 - 06:32 : Steven Attachment: http://drupal.org/files/issues/node admin.png (30.19 KB) ------------------------------------------------------------------------ January 15, 2005 - 09:55 : Bèr Kessels Thanks for integrating the batch delete patch, Steven! ------------------------------------------------------------------------ January 15, 2005 - 13:47 : Dries Attachment: http://drupal.org/files/issues/node-filter-wrap.png (34.55 KB) It looks good but: 1. It makes the UI looks cluttered. I suggest visually grouping the varios form elements so it becomes easier for they eye to skip over them. 2. It is too wide. Try using the Chameleon theme with a sidebar on the right at 1024x768 (eg. my laptop's resolution) and you'll find that the form elements wrap, and wrap in an awkward order. See screenshot. Maybe we should group the form elements using from_group() and make "Show only items where" the form_group()'s title. It would save space. ------------------------------------------------------------------------ January 15, 2005 - 16:14 : Bèr Kessels Steven, Is there a reason why you do not use theme_box, but hardcode HTML in your forms? If there is none, I will spend some time monday to fix it. Bèr -- View: http://drupal.org/node/10296 Edit: http://drupal.org/project/comments/add/10296

1 0

[drupal-devel] [feature] Centralize node queries somehow
by chx 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/node_rewrite_sql_10.patch (43.99 KB) Yes, that's true. Default value of query added to helper _node_rewrite_sql. chx Previous comments: ------------------------------------------------------------------------ December 23, 2004 - 22:25 : chx Attachment: http://drupal.org/files/issues/node_builder.patch (5.56 KB) Although I have posted this to the devel list, as Steve pointed out, putting patches to sandbox is not the best way to do things. So I open this thread. I have measured the time for build a node query in this is about 0.22-0.24ms (on an Athlon 933 MHz machine). ------------------------------------------------------------------------ December 23, 2004 - 23:33 : Dries I'm not likely to commit this. It's not conform with Drupal's coding conventions, but more importantly, a node query builder doesn't solve any real problems. It just adds a different way of doing things without offering a significant advantage. ------------------------------------------------------------------------ December 23, 2004 - 23:51 : chx Attachment: http://drupal.org/files/issues/node_builder_0.patch (5.67 KB) At this moment there are direct queries into the node table everywhere. After node_access_*_sql calls were invented, a lot of queries needed to be changed, and I think we will find more to be inserted. If someone does sg. else with the queries, permission, language etc. he needs to patch 60+ quieries in core alone. This is not a good thing. That's why I am proposing a central node query builder. As for code standards, code-style.pl node.module returns 17 errors, none of them comes from my patch. However, I should admit, there were a few spaces missing from the database.inc patch. So I resubmit. ------------------------------------------------------------------------ December 23, 2004 - 23:56 : chx Attachment: http://drupal.org/files/issues/node_builder_1.patch (5.67 KB) OK, I was wrong, there is a space missing in one of the rewritten queries. But I doubt this matters too much, as this is only a proposal and I think there will be a lot of revisions before it gets to the core. ------------------------------------------------------------------------ December 24, 2004 - 00:26 : drumm The blank lines between the comments and function starts will confuse the documentation parser on drupaldocs.org. I'm not sure if I support this quite yet. Reducing the number of queries and whatnot is great, but this patch doesn't do that quite yet. I'd only want this if the new system is more readable and easy to write. I'm not sure if this is true. As for code-style.pl, I don't think this is actually used much and it doesn't catch everything. Such as, most operators (=, =>, +, etc) like a space on either side of themselves. I'd support removing code-style.pl and throwing it in contributions somewhere. (If this needs further discussion or action, please file another issue, lets keep this focused on this patch). I'm going to wait for a few more revisions of this before I weigh in a +1 or -1. ------------------------------------------------------------------------ December 24, 2004 - 00:58 : Jose A Reyero I can really think of a number of advantages of this, and though not really for committing it yet, think is very good to see some discussion on the issue and some concrete implementations. When implementing i18n I run into this problem, of having to patch lots of queries -actually too many- in about all the modules doing any kind of node listing. This far, I'm stuck at this point. Then I realized it was just the same for node permissions, and will be the same again for whatever new functionality you want to add in the future which affects node listings. I think actually that direct access to 'node' table should be avoided when possible out of the node.module. And the only way to achieve this is to have some kind of query builder. And also think that chx's approach is very good. it provides some upper layer at the 'node' level, which can use some object level semantics while relying on a db layer query builder, which could be used later for other objects -users, taxonomy terms...- Just think of all the work that this could have saved when implementing permissions system, which I think is ugly and dirty anyway because of all that node_join, node_where... In the long run, adding such complex logic as sql code, patching and re-patching queries is bad, very bad, making any new change one higher level of complexity and driving us away from a OO model, which I think we should tend to. It's not using PHP objects. It is thinking of nodes as 'objects' and coding according to it. Oh, yes, sorry, this is growing too long to write it here :-). But we could talk also about db portability... ------------------------------------------------------------------------ December 24, 2004 - 01:14 : moshe weitzman I should add that the Organic Groups module could benefit from this node listing SQL builder. I have awkwardly worked around the problem for now but I don't like it. I don't know if this patch is good or not, but I do lean toward the functionality that it offers. Remember that any developer is free not to use the query builder and issue direct SQL as needed. If the query builder is making life hard, simply don't use, just like today. ------------------------------------------------------------------------ December 24, 2004 - 10:35 : Bèr Kessels I +1 for this functionality (i have no time to comment on coding style etc). It will not only improve maintainability, but will allow a far easier implementation of the i18n. i18n currently has a lot of patches, simpley to add logic to all those node sql queries referred to. ------------------------------------------------------------------------ December 24, 2004 - 13:20 : chx Attachment: http://drupal.org/files/issues/node_builder_2.patch (1.63 KB) OK, maybe the former approach was too complex. How about this? This requires a lot less change to the queries and no change to database.inc and ten times faster. As usual, a sample query is rewritten and an example of the proposed hook is provided. ------------------------------------------------------------------------ December 25, 2004 - 00:09 : chx Final thoughts for this day. If you like the last version, there is a possibility to automate the whole thing by adding the following three lines to the beginning of db_prefix_tables: Of course, there is very small chance of a loop here, so this would require more thought, but I think at the end this would be a good thing. ------------------------------------------------------------------------ December 27, 2004 - 12:55 : Dries I like the second approach better as it keep things readable and gives me a bit more control over the order in which things are written. Looking for more feedback from the others. Did you try updating more queries to make sure it works as intended in all cases? ------------------------------------------------------------------------ December 27, 2004 - 19:48 : chx Attachment: http://drupal.org/files/issues/node_builder_3.patch (1.23 KB) Yes, I tried with many different queries. And I have found one bug -- forgot the underscore in the regexp following FORM so I have rewritten the whole regexp. It is now a lot simpler :) And node_query is now reentrant, so if a function in the process of hook_sql calls node_query, it won't fall into an infinite loop. This situation did not occur but it's better to forestall such things. ------------------------------------------------------------------------ December 28, 2004 - 05:48 : moshe weitzman This patch is a step forward. I thought of another feature which becomes easier with the proposed hook_sql(). think of filtering like freshmeat.net. At freshmeat, you can say that you only want to see projects where OS=Windows and License=GPL (for example). A Drupal equivalent is 'only show me nodes related to Democrats and Poverty'. This sort if global node filtering is very hard in Drupal today. With this hook, it becomes easy. ------------------------------------------------------------------------ December 28, 2004 - 12:37 : Dries It's starting to look good (and you're getting more support)! Have you read the following threads: http://lists.drupal.org/archives/drupal-devel/2004-11/msg00198.html and http://lists.drupal.org/archives/drupal-devel/2004-11/msg00230.html? It's an open issue related to your work. If you start joining tables, you might have to add DISTINCT()s. When you are not joining tables, it would be nice if we'd not pay the cost of a DISTINCT(). It would be good if you could update the node queries in core to take advantage of it. It's not necessary while prototyping though. It would be good if the il8n team could take a closer look at this patch by trying to use it. It would be good if you could add some PHPDoc. ------------------------------------------------------------------------ January 4, 2005 - 07:55 : chx Here is a sum of changes in this new revision: Following Dries' advice, I read those threads and now DISTINCT(nid) is used only if there is something joined. Whether the access_grants system is used or not, is determined at the module configuration screen, so for efficiency reasons, I have patched system.module. I the very first version of my patch (you know, with arrays) I passed on the whole query so hook_sql implementations could easily determine whether they are interested in this query or not. In a very cool brainstorming with Jose, we found out, that passing the whole query is not a good thing, hints are enough. So when I will patch the forum queries, I will define a NODE_SQL_FORUM hint, and a filter which does something cool with forums (I have heard something about private forums not implemented -- I maybe wrong here) may do so. We now have a separate helper function to call hook_sql which is cool, 'cos with it the last node_access_*_sql calls which were lurking in node_search could be elinimated. ------------------------------------------------------------------------ January 4, 2005 - 07:56 : chx Attachment: http://drupal.org/files/issues/central_node_query.patch (6.46 KB) Funny thing. Forgot to attach the patch. ------------------------------------------------------------------------ January 6, 2005 - 00:46 : chx Another day, another revision. As this patch now has nothing to do with queries, but it is about rewriting node SQL statements, I have renamed it node_rewrite_sql, the helper _node_rewrite_sql and there is hook_node_rewrite_sql. hook_node_rewrite_sql expected return value has changed: it can return an array('join'=> ..., 'where'=>..., 'distinct'=> ) so it may determine whether DISTINCT is needed or not. Neither index is mandatory. That hook_node_rewrite_sql impementations will decide whether they are asking for DISTINCT or not, is -- I think -- a good opportunity to optimize for speed. For eg. I doubt every node access mechanism requires DISTINCT. One last note: hook return value can be also can be a string with a WHERE statement for the sake of simplicity. node_rewrite_sql now works with multiline queries. ------------------------------------------------------------------------ January 6, 2005 - 00:46 : chx Attachment: http://drupal.org/files/issues/central_node_query_0.patch (7.43 KB) I can't believe I once again did not attach the patch ------------------------------------------------------------------------ January 6, 2005 - 17:22 : chx Attachment: http://drupal.org/files/issues/central_node_query_1.patch (6.65 KB) Moshe, thanks for your patience and wisdom. Extract is thrown out, list is used. node_node_rewrite_sql looks better now. access_grants_present is determined in run time, once per page. ------------------------------------------------------------------------ January 6, 2005 - 21:35 : Dries The hint-stuff looks a bit hairy to me. It would be nice if this could be avoided but I see what it is necessary. The semaphore stuff is vague too. I don't see how you'd be able to get into an endless loop. It might actually be useful to turn node_access_grants_present() into a more generic module.inc function. Maybe something along the lines of module_implements($hook), which would return a list of modules implementing the specified $hook. I suggest you update more core modules. If you do, I'll measure the performance gain against a copy of drupal.org's database. ------------------------------------------------------------------------ January 6, 2005 - 22:47 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql.patch (15.72 KB) The hint stuff is something is pretty useful: hook implementations can fine tune themselves when to fire and when not. Good for features and performance, too. module_implements is done. Semaphore -- we will have tons of node_rewrite_sql calls in the core. Now someone writes a hook_node_rewrite_sql implementation which calls a function which in turn calls another one which calls node_rewrite_sql . I would rather not be the one who debugs such a scenario. Thanks for the offer! forum.module is now patched. Others will follow tomorrow. ------------------------------------------------------------------------ January 6, 2005 - 23:21 : Jose A Reyero I think this looks good enough, so I'm starting to test and use this for i18n. If this get into the node module, I don't think I will need any node.module patch anymore... A pair of performance hints: - As this node_rewrite_sql is going to be called several times for each page, we can cache the list of modules implementing the hook in a static variable. - We can even cache the whole result for each 'hint' value, so the whole hook will only be called once or twice for each page. Some suggested 'hint' values to be defined: NODE_REWRITE_SQL_LIST -> This one for simple generic list of nodes for the main page, could be default. NODE_REWRITE_SQL_ADMIN_LIST -> List of nodes for administration pages NODE_REWRITE_SQL_RELATED_LIST -> List of objects related to nodes, but no nodes themselves -like comments-, node table may have to be joined... NODE_REWRITE_SQL_BLOCK_LIST -> Listing for a block NODE_REWRITE_SQL_NO_LANGUAGE -> No need to add language conditions. This could hapen when you need a listing for other languages, or mixed languages, for translators or for administrators.. I would need something like this last one for i18n, just to have the permissions working and some flexibility with language conditions. And also the one you already have in place ...._SEARCH. All this will allow for options like search, admin, list current/all languages... ------------------------------------------------------------------------ January 7, 2005 - 17:30 : Jose A Reyero Attachment: http://drupal.org/files/issues/central_node_query_node_module.patch (7.07 KB) I found some problems when using the patch. The where conditions are not merged well if there are more than once, and also conditions need some parenthesis around. So I replaced the $where and $join strings in _node_rewrite_sql by arrays which are imploded at the end. Also added some 'hint' definitions. Patches for i18n module are coming next. This works like charm for node listings, comment listings, searches, etc... Btw, I also think 'semaphores' are not really needed. ------------------------------------------------------------------------ January 7, 2005 - 19:04 : Dries I don't see how the hint system can be portable: are contributed modules supposed to define their own hits? ------------------------------------------------------------------------ January 7, 2005 - 22:54 : chx The hints -- I was in a hurry yesterday evening, so I have forgot to define a NODE_REWRITE_SQL_FORUM in the forum.module patch and also forgot to use the hints. So I think core node types will have their hint defined. I have a vague idea about how contrib modules could define their own hint type: we shall have a NODE_REWRITE_SQL_HINT_MAX, and every hint define should like this -- beware this is not working! This is just an idea. Well, if defines does not work, maybe a global node_rewrite_sql_hints array should be introduced...? ------------------------------------------------------------------------ January 8, 2005 - 09:54 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_0.patch (16.69 KB) Here is a list of changes: Complete rethought of hint system. Uses array of strings instead of defines. There are a number of examples. the new comment.module patch shows a good usage example for the third parameter of node_rewrite_sql which is at the moment called node_alias. It's really not a good name, it should be "the alias of the table which holds the nid field for this query". BTW. there should be an index on comment.nid. Incorporated Jose's "replaced the $where and $join strings in _node_rewrite_sql by arrays which are imploded at the end". I was a fool to lose these, the first version (which used array) used a proper implode for WHEREs. ------------------------------------------------------------------------ January 8, 2005 - 11:27 : Dries I have two fundamental problems with it this patch: The loop detection mechanism has to go. The hints are difficult to understand and I'm not convinced they are good/needed. Does the patch contain a good example of a node_rewrite_sql function/hook that uses these hints? I can't find one. I won't like these hints until I'm convinced they are needed and that they represent the best possible solution. Convince me. Could you extend the documentation about the hint-system? Currently it reads "An array of hint strings about the query, passed on to hook_sql handlers.". Clearly, that doesn't help me at all. The documentation should answer the following questions: (1) what are the hints for, (2) when should I use them (or not use them) and (3) what does hints look like (what is their format/syntax)? Note that I can't compare performance unless more queries have been updated. Thanks chx. ------------------------------------------------------------------------ January 8, 2005 - 11:30 : Dries Why does the hints in the comment module include 'related_list'. This is confusing. ------------------------------------------------------------------------ January 8, 2005 - 12:52 : Jose A Reyero Well, it seems we have different views about the 'hints'. At this point, the node_rewrite has two main uses: permissions and language conditions (i18n). I don't know whether they could have any use for permission system, but for i18n, some extra information about the rewritten queries will be needed: - Most of the queries will just select fields in the node table. Here, only a where condition is needed, but language condition may be added or not depending on the kind of query: _LIST would be a list for the main page, so only in this case, decissions can be made depending on the path. For some pages, you may want a list of nodes for all languages and for some others, only current language. _ADMIN_LIST is a list of nodes for administration. An option will be added in i18n for 'administering all languages together' or 'administer each language separately', or even a language field could be added in the search form. So language conditions will be merged diferently _BLOCK_LIST is a list of nodes for blocks. I'm thinking here of a global option like 'show mixed languages in blocks'. Anyway, this is a list for a block and you cannot have any logic depending on path here. _SEARCH, would be only for searching, and in this case you may decide to search 'all languages' or only the current one. This can be a global option or some extra field in search form. When doing text searches, searching all languages together will make a lot of sense, but also restricting search by language. About _RELATED_LIST, this is the case of comments, is a list of objects which are not nodes themselves, but related objects in a different table. Here, the node table itself may have to be joined as it is not yet in the query. When dealing with the translation system, the language conditions will be added or not by the module requesting the query itself, so no further language conditions will be needed. Queries will have to be run anyway through 'node_rewrite_sql' to have permissions in place. That's what the _NO_LANGUAGE hint is about. So this is my proposed use for the 'hint' thing, though maybe the hints themselves could be better defined or elaborated, but this is the main idea. I mean no specific hints for modules but generic ones for 'kind of listing' you are requesting. Maybe for the permission system, it could be also some use, as you could decide you want to show a list of objects to anyone, but deny access to the objects themselves, something like 'subscribe or register if you want to read the article'. So permissions here will apply differently depending on what you are doing. We can think also about more creative use of permissions like 'you can read latest articles' but old ones will be available only for selected people, or the opposite 'only old content available for everybody'. Or even 'you can read articles but not comments'. This kind of permissions would need also some 'hints'. ------------------------------------------------------------------------ January 8, 2005 - 13:17 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_1.patch (26.89 KB) More queries rewritten. Loop detection gone. Will gather my thoughts on hint system later, but I do hope this amount of queries will be enough for a performance measurement. Now only blogapi, blog, book calls node_access_*_sql, all others are done. ------------------------------------------------------------------------ January 8, 2005 - 13:20 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_2.patch (26.88 KB) Forgot to save taxonomy.module before rolling the patch, so a DISTINCT remained :( ------------------------------------------------------------------------ January 8, 2005 - 13:38 : chx I see Jose has written a lot about hints, thanks! "Well, it seems we have different views about the 'hints'." -- "we" probably means Dries vs. "Jose and I" 'cos I agree with Jose, but I see some more uses, namely what I have said before on making a filter on forums. If you want to that, you need to know which queries are coming from the forum.module. What shall we do? Pass on the whole query, and let the filter proccess the query to decide whether now it is the forum block it hunts for? I doubt. Let's give it a hint it's a forum and a block query. Some kind of extensionable hint mechanism is necessary, 'cos what about a contrib module which filters another contrib node, say flexinode? ------------------------------------------------------------------------ January 9, 2005 - 15:05 : Dries Could we drop the hint system if we'd choose not to support the il8n module for now? ------------------------------------------------------------------------ January 9, 2005 - 16:54 : Jose A Reyero :-( Ok, let's try to think positively. Could we at least pass the query along so i18n -or whatever module tries to implement this hook- could support itself? ------------------------------------------------------------------------ January 9, 2005 - 18:32 : chx Well, passing along the query would mean no problems, but I wonder whether a better documented hint system would be better? I had no problems with letting the loop detection system go it was just a safety belt for some distant future possibility. But the hint system is -- in my point -- is needed. So, here are the valid hint strings: node, forum, comment etc.name of the module from the query originates, like forum, comment, flexinode etc. genericthis would be be for generic listings, like node_default_page, forum_get_forums etc. relatednode table is not joined so if you want to use other fields from node table than nid you shall join node table yourself. However, if you have your table which is like JOIN {mytable} ON mt.nid=$nid_alias.nid that's fine. adminsomething meant only for administrators. blocknodes in a block searchsomething to do with a search ------------------------------------------------------------------------ January 9, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_3.patch (28.31 KB) Killes said I did not had -F^f in the last version. ------------------------------------------------------------------------ January 9, 2005 - 22:37 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_4.patch (28.77 KB) Automatic version as discussed and args for Killes. ------------------------------------------------------------------------ January 9, 2005 - 22:55 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_5.patch (28.31 KB) Hm. It seems we have agreed on no automatization. However, arbitrary arguments are kept for Killes. ------------------------------------------------------------------------ January 10, 2005 - 21:02 : Dries The last version of this patch improved the performance of my test site's main page by 257%. That is, my machine can serve 2.57 times as much pages in the same time interval. Out of curiousity, I investigated this some more and it turns out this can be tracked down to the useless DISTINCT's which this patch removes. Unless someone objects to this patch or the approach taken, I'm going to commit this shortly. Please review this patch carefully as it marks an important change. ------------------------------------------------------------------------ January 10, 2005 - 22:39 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_6.patch (44.18 KB) More DISTINCTs cleared, now there are no more node_access_*_sql calls in the core modules. This may be a final version. Or we can find a few more to rewrite. I've searched for {node} selects, dropped those which had nid = %d as I thought those do not need rewrite (I may be wrong) and dropped also those that are rewritten, dropped those that are indexing or cron. Here are the remaining queries, which may need rewriting: ------------------------------------------------------------------------ January 10, 2005 - 23:02 : Chris Johnson This patch seems like both a Good Thing and a Bad Thing to me. The idea of centralizing node queries is a good one, I think. It seems to be the original motivation for this patch. It allows better SQL optmization and hence performance. It puts all the code in one place for better future maintenance and enhancement. On the other hand, constructing complex SQL statements from programmatic logic is full of peril. The task quickly becomes very large and complex. Manipulating the SQL language elements via code means it is harder to see when one has made a syntactic mistake. Most such errors will be fatal. As the SQL construction code becomes more complex with development, the number of paths through it become huge and impossible to exhaustively test. Some day, someone will pass parameters that cause fatal SQL syntax to be generated in a "production" system. It also obscures SQL syntax differences between supported databases. It's more likely to introduce usage that works with MySQL but fails with Postgres. I've long been in favor of unifying and centralizing all of the database access in Drupal, to the greatest extent which does not adversely affect performance. So in this way, I think this patch is a big step in that general direction. But I've also been involved in maintaining a search engine which used this technique to create the SQL statements on the fly. It became extremely complex over time as one, then two, then three, etc. new possibilities were added (e.g. new usage, new search criteria). The more complex it became, the more regression errors we had and the harder to debug it became. It got to the point where making one small addition would result in 3 new bugs. It worked most of the time, and when it worked, it was fast. But it failed fatally in a significant number of ways. I'm not sure whether to vote +3 or -4 on this one. It might be good; it might be terrible. ------------------------------------------------------------------------ January 10, 2005 - 23:19 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_7.patch (44.18 KB) Chris, I understand your concerns, but this method -- unlike the original -- tries to keep things as simple as possible. You can only JOIN tables and add WHERE conditions. We are not creating complex SQL on the fly, I tried that originally, just read comment #1 for the warm welcome by Dries: "I'm not likely to commit this". So do not worry, while I may be a little hotheaded, Dries keeps order with an iron hand. And the actual rewrite mechanism is also very simple. Ps. Two typos corrected in this version. ------------------------------------------------------------------------ January 13, 2005 - 23:20 : Dries I'm going to test and benchmark the latest version of this patch over the weekend. If all goes well, I'll commit this to HEAD as no concrete alternatives have been proposed. (I can't think of a better approach and believe that this patch does a good job balancing different values/requirements.) It would be much appreciated if those who maintain node-level permission modules could take a look at this patch to evaluate its usefulness/performance. If you maintain a module that queries the node table you might want to look at this as well -- after all, you'll have to update your module if this hits core. (Note that the PHPdoc code in the patch is not up-to-date.) ------------------------------------------------------------------------ January 14, 2005 - 13:47 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_8.patch (43.97 KB) PHPdoc is upgraded, I hope now it is up to date. ------------------------------------------------------------------------ January 15, 2005 - 09:39 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_9.patch (43.98 KB) module_implementations had a minor, but performance eating bug, it's caching was broken. ------------------------------------------------------------------------ January 15, 2005 - 13:44 : Jose A Reyero The function _node_rewrite_sql shoult have a default value for the $query param, as it is called at least once with no arguments. You get a 'warning' otherwise. -- View: http://drupal.org/node/14731 Edit: http://drupal.org/project/comments/add/14731

1 0

[drupal-devel] [feature] Node type filter on node admin page
by Bèr Kessels 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node.module Category: feature requests Priority: normal Assigned to: Steven Reported by: Anonymous Updated by: Bèr Kessels Status: patch Steven, Is there a reason why you do not use theme_box, but hardcode HTML in your forms? If there is none, I will spend some time monday to fix it. Bèr Bèr Kessels Previous comments: ------------------------------------------------------------------------ August 23, 2004 - 17:09 : Anonymous Attachment: http://drupal.org/files/issues/node_admin_filter.patch (3.8 KB) Here's a patch for the current CVS version which adds the option to filter nodes by type on the node admin page. Having built a few sites recently which make heavy use of flexinodes, the ability to filter out particular node types would have been extremely useful, so I hope this makes it to 4.5.0. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ September 4, 2004 - 00:38 : Bèr Kessels I think we should really come up with a good solution for this screen. In the past, I went looking for such a solution and found that the various MP3 library tools might come to help. They deal with large amounts of metadata, some of wch are important, other are not. I summarise here what the UI could be like: ** The menu-based (tree) approach: Most mp3 collections use this idea. In a tree like structure on the side, you can make selections. The list in the main area will follow this selection. (Itunes: http://www.apple.com/itunes/playlists.html , Juk: http://developer.kde.org/~wheeler/images/juk-2.0/juk-2.png, finf/freeamp: http://www.zinf.org/images/zinf_mymusic_shot.png) In drupal that would mean a menu-based filtering method. I once made a patch for that, but that was rejected: http://drupal.org/node/view/7766 The good thing here, is that it is virtually endless scalable. So in theory, flexinode coud add menu items to show only nodes where foo is bar. | - nodetype | |- blog | |= Flexinode | | |=foo | | | |=bar | |- page ** The multiple selectlist UI: Another moethod is a extended method of what we have now: select items from select boxes and press submit. Above the main area one will find some selection UI where one can in- and exlude sertain criteria. (rhythmbox: http://www.rhythmbox.org/screenshots/Screenshot-Rhythmbox.png, and winamp http://www.winamp.com/player/walkthrough.php) Note that both still show some form of tree navigation. In drupal that would mean multiple selects in three columns (rhytmbox and winamp show only two artist and album) For drupal we need three of thos blocks: one for node-types, one for node-settings (promoted, etc) and one for taxonomy terms. The node settings should be logical too: If a criteria is selected it it true, if not: false. That way we can also get rod of the annoying logic: "view posts that are not promoted" and the likes. In both solutions the "perform batch action" needs to move somewhere else. I would plead for moving it to the bottom of the list. A lot of webmail cleints do it that way. The reason being simple: the steps you take to perform the update are: 1) select criteria (find nodes) 2) checkmark the chages (we need columns of checkboxes) 3) perform the update. Logically the way you go through the UI is left->right, top-> down (not taking into consideration japanese/hebrew/arabs etc) So all in all: i think indeed the UI needs serious work. I am willing to take this one me, bu I do need some consensus first. I am not willing to spend hours on a new UI (like the menu-based approach) to find the patch being rejected.Therefore I like some discussion up fron, rather than the " code first see later" approach. Regards, Ber ------------------------------------------------------------------------ September 5, 2004 - 23:42 : Bèr Kessels Attachment: http://drupal.org/files/issues/new_UI_proposal.png (175.31 KB) Here is a gimped mockup from what option 2) could look like. WE could add a fouth box with users too, but I am not sure how that would go with permissions and the like. I need to do some thinking about selecatibility by users first. ------------------------------------------------------------------------ September 6, 2004 - 00:01 : Bèr Kessels Sorry for the spam: but I forgot to mention that there are a few issues: The "perform action dropdown" should list stuff as: promote, demote, etc. But that list needs to be generated with the selection above in mind. So if you choose " published" the action list should not have "publish these nodes" Also the action list could contain "recategorise" wich would need another screen after the one in the "ugly screendump-gimp-mockup", to be able to choose the categories in wich the selection should be placed. And of course the delete these items with a confimation dialog. (http://drupal.org/node/view/7743#comment-11624) ------------------------------------------------------------------------ September 6, 2004 - 14:32 : Bèr Kessels Attachment: http://drupal.org/files/issues/node_admin.html (29.95 KB) After a long discussion with Steven And gerhard, I designed the page in HTML, to let you all try it in some way. Beware: the html is an ugly mock, It is meant as scetch, not as valid html. Some notes on the page: "View all nodes that are:" contains three differnet possibilities for searching. The first two (above the blank line) are there for simplicity, but they might confuse users, because they need a lot of text. The second idea uses a checkbox to enable the select next to it. The third one uses some ajvascript to improve useablity. We should of course choose one of these three methods. "View all nodes that are classified in:" There are quite some people that have questions with whether or not we should include this selection aat all. Here is why I think it should definately go Because most sites have only one purpose and thus a lot of nodes of one type in one state. For example a personal blog site: 99% of the nodes are blogs. 99% are promoted. Only 1 or 2 are not promoted, not published or of type page. Or take a photo-album-site. All posts will be images, the main difference is the classification (the galleries) I placed them in. "any/all" There must be a way to choose whether the sected options should be an AND or an OR. I tried three different methods, each block has one method. We should choose one. Please give me feedback, for I still have to decide whether or not to make this a contributed module or start lobby-ing to get it in core. ------------------------------------------------------------------------ September 13, 2004 - 11:50 : moshe weitzman looking good, and really useful. a few small comments: - consider upping the number of nodes that appear on this page. it is nearly pointless to use paging on this sort of UI - i like the second idea of an 'enabled' checkbox, without the javascript. i think you can use 2 checkboxes, where the second one is called 'publish', 'promote', and 'has comments' - I prefer the horizontal configuration of the 'Require: all any' radio buttons - don't repeat 'the selected posts' in the choices for the bottom dropdown. perhaps use a title for that form_select() instead. - the last choice in that dropdown is 'reclassify'. i don't see how this screen could support that. instead, use my taxonomy_multi_edit.module from contrib :) i suggest making this a contrib module at first. it can be a tab off of the current page. thats how taxonomy_multi_edit.module appears. ------------------------------------------------------------------------ September 13, 2004 - 13:03 : Bèr Kessels About that reclassify: That was exactly my intention: using the taxonomy_muliti_edit module for that. All I wanted to do is pass a list of nids to your module and open a screen using that module with the previously selected nodes! About the other comments: Yes upping the number of nodes might be a good idea. Anyone any idea what number would still be usefull. 10.000 nodes (on drupal.org) in one list is not a good idea, IMO, so some paging is needed. indeed: repeatinf words in dropdowns is considered "bad UI design" thank you for pointing it out. Regards, Ber ------------------------------------------------------------------------ September 28, 2004 - 13:37 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type.patch (1.96 KB) here is an updated version of this patch, syncing it with CVS HEAD. Also, i did change the functionality somewhat: previous versions of the patch asked the modules what node types were out there (via hook_node_name()). This version asks the node table what types are in there (via SELECT DISTINCT type FROM {node}). I find this more useful, because it's entirely possible that, over time, modules will be removed from the system, yet they still have rows in the node table. This method gives a more accurate view of the types of nodes on the system. The new UI looks excellent, but this patch does not address it. ------------------------------------------------------------------------ September 28, 2004 - 19:05 : drumm -1 I applied and tested and it did not work at all for me. The type options selection only contains 's' and 'W', selecting one does not work. The list of types seems to come from the DB which is probably an unnecessary query. ------------------------------------------------------------------------ September 29, 2004 - 12:51 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type_1.patch (2.31 KB) Here's a version of that patch that should work. Sorry. I think that getting the types from the node table is necessary. The list of installed nodes is not the authority on what types of nodes are in the db; we should ask the db directly. It's a small query on one admin page; the result is that the dropdown has in it the most accurate information available. That's worth it, imo. ------------------------------------------------------------------------ September 29, 2004 - 12:52 : eafarris s/list of installed nodes/list of installed modules/ . sheesh. teach me to post before my tea. ------------------------------------------------------------------------ October 21, 2004 - 18:15 : Bèr Kessels Attachment: http://drupal.org/files/issues/node.html (29.33 KB) I do not know where it weant, but my proposal had a HTML mockup. Attaching it here to see what i meant. ------------------------------------------------------------------------ October 23, 2004 - 13:14 : com2 Nice filter options, but what I really miss it the possibility to select all checkboxes. Really boring job when you have to click the whole list one by one. ------------------------------------------------------------------------ October 24, 2004 - 10:41 : Bèr Kessels I am not a big fan of those options in forms. They more often break things than that thy fix them. Rahter use a bookmarklet r so for that: http://www.squarefree.com/bookmarklets/forms.html Ber ------------------------------------------------------------------------ October 25, 2004 - 01:03 : Steven Attachment: http://drupal.org/files/issues/node_admin_ucd.html (29.83 KB) Based on some more IRC discussion, I came up with this (attachment). It is based on an earlier mockup. It does use Javascript and doesn't degrade without script (yet), but I think it's worth considering. It can be implemented quite cleanly. The idea would be to have a filter like this initially, and after clicking 'Go', to show the current filter as well as the form again, allowing you to refine your search. I haven't tested browser compatibility, but it should work on most browsers. The only ones I know for sure won't work are IE4 and NS4, which are old enough to ignore IMO: after all, this is admin only, and we can put some stricter browser requirements on that. Recent versions of other browsers should work (it only does a simple visibility toggle). I like it because it's not an airplane cockpit, but still offers complex filtering options through refinement. It is also quite similar to the search/filtering UI in e.g. mail applications. The question is of course, how to degrade it for people without Javascript. My suggestion would be to simply show all filter options, and put a radiobutton in front of each: ( ) Where type is [ blog | v ] ( ) Where published status is ( ) published ( ) unpublished ( ) Where category is [ term1 | v ] ... It has more visual clutter, but the same principle of use. ------------------------------------------------------------------------ October 25, 2004 - 01:13 : Steven In fact, I just realized that the radio-button version can be coded to send exactly the same form data as the scripted version, which makes it more interesting. ------------------------------------------------------------------------ October 29, 2004 - 00:52 : Steven If you guys approve of my idea, I'll start coding. Assigning to myself for now. ------------------------------------------------------------------------ November 1, 2004 - 17:35 : Bèr Kessels I like this idea much more than my previous approaches. Gave it some thought and came with the follwing behaviour: * I think we need to discuss or at least design the tabular data too: It makes 0 sense to show the "types" column, if you chose "type = image" * I think we need to introduce a hook to add info to that tabulat view. Images can output thumbnails, for example * In tabular view, we should make: title a textfield, type and author a select and satus a select too. * I think we should introduce another range of options: "where content contains" [ ] and "where title contains" [ ] * I would like to get some feedback on how many drupal-using people are firmly against javascript, with a reason why. I bet that this is < 1% ! Bèr ------------------------------------------------------------------------ November 1, 2004 - 17:42 : Steven We don't have an anti-JS policy in Drupal. It's just that until recently, doing cross browser JS was a chore and making it accessible and degradable would be a disaster. This form can be implemented cleanly and will degrade without problems. Anyone who opposes it because it is JS can bugger off ;). ------------------------------------------------------------------------ November 3, 2004 - 19:24 : com2 There is a more radical way around JS. You could forget the table idea alltogether and present the nodes in Listbox. Then selecting becomes much easier with Ctrl-Click and Shift-Click ------------------------------------------------------------------------ November 4, 2004 - 00:02 : Bèr Kessels Did you have a look at the previous applied UI improvements? Did you consider the pre's and cons of those earlier posts? It seams you are repeating passed issues here, please reply with miore detailed issues here, Bèr ------------------------------------------------------------------------ December 3, 2004 - 10:49 : Steven Attachment: http://drupal.org/files/issues/node-admin.patch (13.1 KB) Here's a patch which implements the JS approach of refining the selection. ------------------------------------------------------------------------ December 3, 2004 - 10:50 : Steven Attachment: http://drupal.org/files/issues/node_admin.png (34.38 KB) And here's a screenshot of how it looks. ------------------------------------------------------------------------ December 3, 2004 - 19:23 : FactoryJoe(a)civicspacelabs.org This is looking better and better. I'm going to do a UI review of this a post any additional thoughts or comments. Chris ------------------------------------------------------------------------ December 3, 2004 - 21:50 : Anonymous Ack, I tested this using the bluebeach theme, but there's a small CSS bug with bluemarine, where the header for the operations form is stuck to the right of the filter form. I'll make an updated patch later. In the meantime, comments are much appreciated. ------------------------------------------------------------------------ December 3, 2004 - 23:15 : killes(a)www.drop.org I actually prefer the non-JS menu over the JS menu. Not (only) because of the JS but because I prefer radiobuttons over pulldown menus. The argument that the non-JS approach uses more vertical space can be mitigated by redirecting to a #results anchor. ------------------------------------------------------------------------ January 15, 2005 - 06:32 : Steven Attachment: http://drupal.org/files/issues/node.admin.patch (14.41 KB) Here's an updated patch... I have exams in a week, so I don't have time to finish up the Javascript version, so I removed it (it can be added later easily). However, the non-JS version is still very usable. Changes since last time: - Uses semantic XHTML (definition list, and yes, they are for more than just definitions, even w3c agrees). - Merges the various node flags (promoted, published, ...) into one "status" dropdown. This keeps the whole selector shorter, and is better UI-wise IMO. - Adds drupal_gotos, they were missing. - Updates node admin watchog messages to properly display the type. The code used t($node->type) which is a big no-no. hook_node_name() is now invoked instead. - Removes some dead code. - I rolled Ber's node mass deletion patch in as it affects the same piece of code. Updating either patch for the other separately would be a waste of time. I also added some more improvements to Bèr's code, such as removing the incorrect help from the confirmation page, cleaning up the code style a bit, not loading every node just to display its title and displaying a single drupal_set_message() after the deleting rather than one per node. The old issue is here: http://drupal.org/node/7743 ------------------------------------------------------------------------ January 15, 2005 - 06:32 : Steven Attachment: http://drupal.org/files/issues/node admin.png (30.19 KB) ------------------------------------------------------------------------ January 15, 2005 - 09:55 : Bèr Kessels Thanks for integrating the batch delete patch, Steven! ------------------------------------------------------------------------ January 15, 2005 - 13:47 : Dries Attachment: http://drupal.org/files/issues/node-filter-wrap.png (34.55 KB) It looks good but: 1. It makes the UI looks cluttered. I suggest visually grouping the varios form elements so it becomes easier for they eye to skip over them. 2. It is too wide. Try using the Chameleon theme with a sidebar on the right at 1024x768 (eg. my laptop's resolution) and you'll find that the form elements wrap, and wrap in an awkward order. See screenshot. Maybe we should group the form elements using from_group() and make "Show only items where" the form_group()'s title. It would save space. -- View: http://drupal.org/node/10296 Edit: http://drupal.org/project/comments/add/10296

1 0

[drupal-devel] cvs log messages
by no-reply＠zind.net 15 Jan '05

15 Jan '05

File: /translations/drupal-pot/aggregator-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/archive-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/block-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/blog-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/blogapi-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/book-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/comment-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/common-inc.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/drupal-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/file-inc.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/filter-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/forum-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/general.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/locale-inc.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/locale-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/menu-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/node-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/path-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/ping-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/poll-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/profile-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/queue-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/search-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/statistics-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/story-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/system-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/taxonomy-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/throttle-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/upload-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/user-module.pot 1.2.2.3 DRUPAL-4-5 /translations/drupal-pot/watchdog-module.pot 1.2.2.3 DRUPAL-4-5 Date: January 15, 2005 - 12:33 User: goba Translation template update for 4.5.2 ---------------------------------------------------------------------- File: /CHANGELOG.txt 1.58 Date: January 15, 2005 - 09:09 User: dries - Updated the CHANGELOG.txt ---------------------------------------------------------------------- File: /CHANGELOG.txt 1.41.2.3 DRUPAL-4-5 Date: January 15, 2005 - 09:09 User: dries - Updated the CHANGELOG.txt ---------------------------------------------------------------------- File: /modules/search.module 1.112 Date: January 15, 2005 - 09:03 User: dries - Stripped white-space. ---------------------------------------------------------------------- File: /database/database.mysql 1.153.2.3 DRUPAL-4-5 /database/database.pgsql 1.94.2.2 DRUPAL-4-5 Date: January 15, 2005 - 08:52 User: dries - Corrected the 'update_start' date. ---------------------------------------------------------------------- File: /modules/story.module 1.164 Date: January 15, 2005 - 08:01 User: dries - Patch #15264 by Ber et al.: improved the story module's help text. ---------------------------------------------------------------------- File: /modules/htmlarea/htmlarea.module 1.66 Date: January 15, 2005 - 07:14 User: gordon - added modifications so that htmlarea fork from http://code.gogo.co.nz/products/htmlarea_fork.html will work correctly. You just need to check the new field to include the "linebreak" in the toolbar. ---------------------------------------------------------------------- File: /modules/mail_archive/CHANGELOG.txt 1.5 /modules/mail_archive/mail_archive.module 1.6 /modules/mail_archive/README.txt 1.6 Date: January 15, 2005 - 03:24 User: jeremy - mail_archive.module: o determine body type using encoding field, not type field o allow 'access mail archive' access to "next" and "previous" links o fix 'next message' and 'previous message' links to really link to next and previous message o added mouse-over descriptions to all menu items ---------------------------------------------------------------------- File: /modules/wiki/README.txt 1.2 Date: January 15, 2005 - 02:18 User: veridicus Added my name as current maintainer and removed old TODOs ---------------------------------------------------------------------- File: /modules/wiki/wiki.module 1.12 Date: January 15, 2005 - 02:17 User: veridicus Major changes to use Drupal 4.5 API and add a few new formatting features ---------------------------------------------------------------------- File: /modules/wiki/CHANGELOG NONE /modules/wiki/INSTALL NONE Date: January 15, 2005 - 02:16 User: veridicus Renamed with .txt extension ---------------------------------------------------------------------- File: /modules/wiki/CHANGELOG.txt 1.1 /modules/wiki/INSTALL.txt 1.1 Date: January 15, 2005 - 02:15 User: veridicus Renamed with .txt extension ---------------------------------------------------------------------- File: /modules/remindme/po/remindme-module.pot 1.1 /modules/remindme/po/remindme.pot NONE Date: January 15, 2005 - 02:13 User: killes PO update. ---------------------------------------------------------------------- File: /modules/remindme/po/remindme.pot 1.4 /modules/remindme/README.txt 1.2 /modules/remindme/remindme.module 1.21 Date: January 15, 2005 - 02:09 User: killes Updated to Drupal 4.5 ---------------------------------------------------------------------- File: /sandbox/killes/speed-drupal/modules/block/block.inc 1.1 /sandbox/killes/speed-drupal/modules/block/block.module 1.1 Date: January 15, 2005 - 01:48 User: killes Hack'n slash ---------------------------------------------------------------------- File: /modules/filestore2/filestore2.module 1.24 Date: January 14, 2005 - 23:43 User: gordon - fix bug #15571 - now users require "access connect" to see blocks ---------------------------------------------------------------------- File: /sandbox/killes/speed-drupal/modules/filter/filter.inc 1.1 /sandbox/killes/speed-drupal/modules/filter/filter.module 1.1 Date: January 14, 2005 - 23:39 User: killes Another one bites the dust. ---------------------------------------------------------------------- File: /modules/ldap_integration/ldap_integration.module 1.2.2.3 DRUPAL-4-5 Date: January 14, 2005 - 23:21 User: pablobm Removed ugly output generated after failed logins ---------------------------------------------------------------------- File: /modules/htmlarea/patch/node.patch 1.1.2.4 DRUPAL-4-5 Date: January 14, 2005 - 23:18 User: gordon - update node patch for latest stable version ---------------------------------------------------------------------- File: /sandbox/killes/speed-drupal/modules/statistics/statistics.module 1.2 Date: January 14, 2005 - 22:55 User: killes bugfix ---------------------------------------------------------------------- File: /sandbox/killes/speed-drupal/modules/system/system.module 1.2 Date: January 14, 2005 - 22:55 User: killes bugfix ---------------------------------------------------------------------- File: /modules/ecommerce/file/CHANGELOG.txt 1.4 /modules/ecommerce/file/file.module 1.15 /modules/ecommerce/paypal/CHANGELOG.txt 1.3 /modules/ecommerce/paypal/paypal.module 1.13 /modules/ecommerce/store/CHANGELOG.txt 1.5 /modules/ecommerce/store/store.module 1.31 Date: January 14, 2005 - 22:37 User: mathias * Allow admin and user's with 'administer store' permission to view the 'my files' page and order history of other users without needing to login as that user. Once the user id is know, the admin can navigate to store/myfiles/uid and store/history/uid respectively. * For PayPal transactions, when the payment status is completed and the transaction consists of only non-shippable items, set the transaction workflow to complete. This allows product such as file downloads to be available immediately to the user. Authorize.net module needs to be updated as well. * Allow user to browse expired file downloads they've purchased. * Parcel products consisting of file downloads were not showing on the 'my files' page. Fixed. ---------------------------------------------------------------------- File: /modules/ecommerce/file/CHANGELOG.txt 1.1.2.3 DRUPAL-4-5 /modules/ecommerce/file/file.module 1.9.2.5 DRUPAL-4-5 /modules/ecommerce/paypal/CHANGELOG.txt 1.1.2.2 DRUPAL-4-5 /modules/ecommerce/paypal/paypal.module 1.11.2.2 DRUPAL-4-5 /modules/ecommerce/store/CHANGELOG.txt 1.1.2.4 DRUPAL-4-5 /modules/ecommerce/store/store.module 1.22.2.7 DRUPAL-4-5 Date: January 14, 2005 - 22:37 User: mathias * Allow admin and user's with 'administer store' permission to view the 'my files' page and order history of other users without needing to login as that user. Once the user id is know, the admin can navigate to store/myfiles/uid and store/history/uid respectively. * For PayPal transactions, when the payment status is completed and the transaction consists of only non-shippable items, set the transaction workflow to complete. This allows product such as file downloads to be available immediately to the user. Authorize.net module needs to be updated as well. * Allow user to browse expired file downloads they've purchased. * Parcel products consisting of file downloads were not showing on the 'my files' page. Fixed. ---------------------------------------------------------------------- File: /themes/manji/obsidian/style.css 1.2 Date: January 14, 2005 - 21:31 User: chrisada fixed navigation menu placement ---------------------------------------------------------------------- File: /sandbox/killes/speed-drupal/modules/system/system.inc 1.1 /sandbox/killes/speed-drupal/modules/system/system.module 1.1 Date: January 14, 2005 - 19:01 User: killes Split up system.module Roughly 1:2 ---------------------------------------------------------------------- File: /includes/module.inc 1.64 Date: January 14, 2005 - 17:30 User: unconed Remove left-overs from admin.module. ---------------------------------------------------------------------- File: /database/database.mysql 1.165 /database/database.pgsql 1.101 /database/updates.inc 1.80 Date: January 14, 2005 - 17:29 User: unconed Remove left-overs from admin.module. ---------------------------------------------------------------------- File: /sandbox/junyor/import/blogger.inc 1.2 Date: January 14, 2005 - 16:41 User: junyor Fixed bug ---------------------------------------------------------------------- File: /sandbox/dikini/binder/binder.module 1.3 Date: January 14, 2005 - 16:40 User: dikini changed the local menues to go under edit ---------------------------------------------------------------------- File: /modules/taxonomy_otf/taxonomy_otf.module 1.8 Date: January 14, 2005 - 15:47 User: njivy Bug #15591: Fixed the form-within-a-form error. ---------------------------------------------------------------------- File: /CHANGELOG.txt 1.57 DRUPAL-4-5 /INSTALL.txt 1.10 DRUPAL-4-5 /modules/tracker.module 1.100.2.1 DRUPAL-4-5 Date: January 14, 2005 - 15:43 User: dries - Patch #15500 by Morbus: ignore unpublished comments when determining last_post. (This matches the behavior in HEAD.) ---------------------------------------------------------------------- File: /sandbox/killes/speed-drupal/modules/statistics/statistics.inc 1.1 /sandbox/killes/speed-drupal/modules/statistics/statistics.module 1.1 /sandbox/killes/speed-drupal/modules/user/user.inc 1.2 Date: January 14, 2005 - 15:23 User: killes Split up statistics.module http://drupal.org/node/15566 ---------------------------------------------------------------------- File: /modules/admin.module NONE /modules/system.module 1.191 Date: January 14, 2005 - 15:21 User: dries - Patch #15570 by Drumm: integrated the admin and system module. Renamed the callback as per Goba's suggestion. ---------------------------------------------------------------------- File: /modules/user.module 1.430 Date: January 14, 2005 - 15:15 User: dries - Patch #15566 by drumm: must specify a userneme or password error happens when it shouldn't. ---------------------------------------------------------------------- File: /themes/manji/obsidian/images/bg.gif 1.1 /themes/manji/obsidian/images/bold.png 1.1 /themes/manji/obsidian/images/bottom-whole.gif 1.1 /themes/manji/obsidian/images/bottomgradient.jpg 1.1 /themes/manji/obsidian/images/close.png 1.1 /themes/manji/obsidian/images/comments.gif 1.1 /themes/manji/obsidian/images/commentsdown.gif 1.1 /themes/manji/obsidian/images/commentsgradient.jpg 1.1 /themes/manji/obsidian/images/end.jpg 1.1 /themes/manji/obsidian/images/gradient.gif 1.1 /themes/manji/obsidian/images/gradient.png 1.1 /themes/manji/obsidian/images/header_left.gif 1.1 /themes/manji/obsidian/images/header_right.gif 1.1 /themes/manji/obsidian/images/header_whole.gif 1.1 /themes/manji/obsidian/images/icon.jpg 1.1 /themes/manji/obsidian/images/italic.png 1.1 /themes/manji/obsidian/images/link.png 1.1 /themes/manji/obsidian/images/masthead.jpg 1.1 /themes/manji/obsidian/images/navgradient.jpg 1.1 /themes/manji/obsidian/images/topbutton.gif 1.1 Date: January 14, 2005 - 15:05 User: chrisada Added Obsidian Style for Manji Template ---------------------------------------------------------------------- File: /themes/manji/obsidian/screenshot.png 1.1 /themes/manji/obsidian/style.css 1.1 Date: January 14, 2005 - 15:04 User: chrisada Added Obsidian style for Manji Template ---------------------------------------------------------------------- File: /themes/manji/page.tpl.php 1.2 /themes/manji/style.css 1.2 Date: January 14, 2005 - 15:01 User: chrisada Small update to XHTML to allow for additional style (obsidian) ---------------------------------------------------------------------- File: /sandbox/killes/speed-drupal/modules/user/user.inc 1.1 /sandbox/killes/speed-drupal/modules/user/user.module 1.1 Date: January 14, 2005 - 14:51 User: killes Trying to make Drupal faster. See http://drupal.org/node/15572 ---------------------------------------------------------------------- File: /sandbox/killes/de.po NONE Date: January 14, 2005 - 14:37 User: killes ----------------------------------------------------------------------

1 0

[drupal-devel] [feature] Node type filter on node admin page
by Dries 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node.module Category: feature requests Priority: normal Assigned to: Steven Reported by: Anonymous Updated by: Dries Status: patch Attachment: http://drupal.org/files/issues/node-filter-wrap.png (34.55 KB) It looks good but: 1. It makes the UI looks cluttered. I suggest visually grouping the varios form elements so it becomes easier for they eye to skip over them. 2. It is too wide. Try using the Chameleon theme with a sidebar on the right at 1024x768 (eg. my laptop's resolution) and you'll find that the form elements wrap, and wrap in an awkward order. See screenshot. Maybe we should group the form elements using from_group() and make "Show only items where" the form_group()'s title. It would save space. Dries Previous comments: ------------------------------------------------------------------------ August 23, 2004 - 18:09 : Anonymous Attachment: http://drupal.org/files/issues/node_admin_filter.patch (3.8 KB) Here's a patch for the current CVS version which adds the option to filter nodes by type on the node admin page. Having built a few sites recently which make heavy use of flexinodes, the ability to filter out particular node types would have been extremely useful, so I hope this makes it to 4.5.0. ------------------------------------------------------------------------ August 23, 2004 - 22:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ August 23, 2004 - 22:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ September 4, 2004 - 01:38 : Bèr Kessels I think we should really come up with a good solution for this screen. In the past, I went looking for such a solution and found that the various MP3 library tools might come to help. They deal with large amounts of metadata, some of wch are important, other are not. I summarise here what the UI could be like: ** The menu-based (tree) approach: Most mp3 collections use this idea. In a tree like structure on the side, you can make selections. The list in the main area will follow this selection. (Itunes: http://www.apple.com/itunes/playlists.html , Juk: http://developer.kde.org/~wheeler/images/juk-2.0/juk-2.png, finf/freeamp: http://www.zinf.org/images/zinf_mymusic_shot.png) In drupal that would mean a menu-based filtering method. I once made a patch for that, but that was rejected: http://drupal.org/node/view/7766 The good thing here, is that it is virtually endless scalable. So in theory, flexinode coud add menu items to show only nodes where foo is bar. | - nodetype | |- blog | |= Flexinode | | |=foo | | | |=bar | |- page ** The multiple selectlist UI: Another moethod is a extended method of what we have now: select items from select boxes and press submit. Above the main area one will find some selection UI where one can in- and exlude sertain criteria. (rhythmbox: http://www.rhythmbox.org/screenshots/Screenshot-Rhythmbox.png, and winamp http://www.winamp.com/player/walkthrough.php) Note that both still show some form of tree navigation. In drupal that would mean multiple selects in three columns (rhytmbox and winamp show only two artist and album) For drupal we need three of thos blocks: one for node-types, one for node-settings (promoted, etc) and one for taxonomy terms. The node settings should be logical too: If a criteria is selected it it true, if not: false. That way we can also get rod of the annoying logic: "view posts that are not promoted" and the likes. In both solutions the "perform batch action" needs to move somewhere else. I would plead for moving it to the bottom of the list. A lot of webmail cleints do it that way. The reason being simple: the steps you take to perform the update are: 1) select criteria (find nodes) 2) checkmark the chages (we need columns of checkboxes) 3) perform the update. Logically the way you go through the UI is left->right, top-> down (not taking into consideration japanese/hebrew/arabs etc) So all in all: i think indeed the UI needs serious work. I am willing to take this one me, bu I do need some consensus first. I am not willing to spend hours on a new UI (like the menu-based approach) to find the patch being rejected.Therefore I like some discussion up fron, rather than the " code first see later" approach. Regards, Ber ------------------------------------------------------------------------ September 6, 2004 - 00:42 : Bèr Kessels Attachment: http://drupal.org/files/issues/new_UI_proposal.png (175.31 KB) Here is a gimped mockup from what option 2) could look like. WE could add a fouth box with users too, but I am not sure how that would go with permissions and the like. I need to do some thinking about selecatibility by users first. ------------------------------------------------------------------------ September 6, 2004 - 01:01 : Bèr Kessels Sorry for the spam: but I forgot to mention that there are a few issues: The "perform action dropdown" should list stuff as: promote, demote, etc. But that list needs to be generated with the selection above in mind. So if you choose " published" the action list should not have "publish these nodes" Also the action list could contain "recategorise" wich would need another screen after the one in the "ugly screendump-gimp-mockup", to be able to choose the categories in wich the selection should be placed. And of course the delete these items with a confimation dialog. (http://drupal.org/node/view/7743#comment-11624) ------------------------------------------------------------------------ September 6, 2004 - 15:32 : Bèr Kessels Attachment: http://drupal.org/files/issues/node_admin.html (29.95 KB) After a long discussion with Steven And gerhard, I designed the page in HTML, to let you all try it in some way. Beware: the html is an ugly mock, It is meant as scetch, not as valid html. Some notes on the page: "View all nodes that are:" contains three differnet possibilities for searching. The first two (above the blank line) are there for simplicity, but they might confuse users, because they need a lot of text. The second idea uses a checkbox to enable the select next to it. The third one uses some ajvascript to improve useablity. We should of course choose one of these three methods. "View all nodes that are classified in:" There are quite some people that have questions with whether or not we should include this selection aat all. Here is why I think it should definately go Because most sites have only one purpose and thus a lot of nodes of one type in one state. For example a personal blog site: 99% of the nodes are blogs. 99% are promoted. Only 1 or 2 are not promoted, not published or of type page. Or take a photo-album-site. All posts will be images, the main difference is the classification (the galleries) I placed them in. "any/all" There must be a way to choose whether the sected options should be an AND or an OR. I tried three different methods, each block has one method. We should choose one. Please give me feedback, for I still have to decide whether or not to make this a contributed module or start lobby-ing to get it in core. ------------------------------------------------------------------------ September 13, 2004 - 12:50 : moshe weitzman looking good, and really useful. a few small comments: - consider upping the number of nodes that appear on this page. it is nearly pointless to use paging on this sort of UI - i like the second idea of an 'enabled' checkbox, without the javascript. i think you can use 2 checkboxes, where the second one is called 'publish', 'promote', and 'has comments' - I prefer the horizontal configuration of the 'Require: all any' radio buttons - don't repeat 'the selected posts' in the choices for the bottom dropdown. perhaps use a title for that form_select() instead. - the last choice in that dropdown is 'reclassify'. i don't see how this screen could support that. instead, use my taxonomy_multi_edit.module from contrib :) i suggest making this a contrib module at first. it can be a tab off of the current page. thats how taxonomy_multi_edit.module appears. ------------------------------------------------------------------------ September 13, 2004 - 14:03 : Bèr Kessels About that reclassify: That was exactly my intention: using the taxonomy_muliti_edit module for that. All I wanted to do is pass a list of nids to your module and open a screen using that module with the previously selected nodes! About the other comments: Yes upping the number of nodes might be a good idea. Anyone any idea what number would still be usefull. 10.000 nodes (on drupal.org) in one list is not a good idea, IMO, so some paging is needed. indeed: repeatinf words in dropdowns is considered "bad UI design" thank you for pointing it out. Regards, Ber ------------------------------------------------------------------------ September 28, 2004 - 14:37 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type.patch (1.96 KB) here is an updated version of this patch, syncing it with CVS HEAD. Also, i did change the functionality somewhat: previous versions of the patch asked the modules what node types were out there (via hook_node_name()). This version asks the node table what types are in there (via SELECT DISTINCT type FROM {node}). I find this more useful, because it's entirely possible that, over time, modules will be removed from the system, yet they still have rows in the node table. This method gives a more accurate view of the types of nodes on the system. The new UI looks excellent, but this patch does not address it. ------------------------------------------------------------------------ September 28, 2004 - 20:05 : drumm -1 I applied and tested and it did not work at all for me. The type options selection only contains 's' and 'W', selecting one does not work. The list of types seems to come from the DB which is probably an unnecessary query. ------------------------------------------------------------------------ September 29, 2004 - 13:51 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type_1.patch (2.31 KB) Here's a version of that patch that should work. Sorry. I think that getting the types from the node table is necessary. The list of installed nodes is not the authority on what types of nodes are in the db; we should ask the db directly. It's a small query on one admin page; the result is that the dropdown has in it the most accurate information available. That's worth it, imo. ------------------------------------------------------------------------ September 29, 2004 - 13:52 : eafarris s/list of installed nodes/list of installed modules/ . sheesh. teach me to post before my tea. ------------------------------------------------------------------------ October 21, 2004 - 19:15 : Bèr Kessels Attachment: http://drupal.org/files/issues/node.html (29.33 KB) I do not know where it weant, but my proposal had a HTML mockup. Attaching it here to see what i meant. ------------------------------------------------------------------------ October 23, 2004 - 14:14 : com2 Nice filter options, but what I really miss it the possibility to select all checkboxes. Really boring job when you have to click the whole list one by one. ------------------------------------------------------------------------ October 24, 2004 - 11:41 : Bèr Kessels I am not a big fan of those options in forms. They more often break things than that thy fix them. Rahter use a bookmarklet r so for that: http://www.squarefree.com/bookmarklets/forms.html Ber ------------------------------------------------------------------------ October 25, 2004 - 02:03 : Steven Attachment: http://drupal.org/files/issues/node_admin_ucd.html (29.83 KB) Based on some more IRC discussion, I came up with this (attachment). It is based on an earlier mockup. It does use Javascript and doesn't degrade without script (yet), but I think it's worth considering. It can be implemented quite cleanly. The idea would be to have a filter like this initially, and after clicking 'Go', to show the current filter as well as the form again, allowing you to refine your search. I haven't tested browser compatibility, but it should work on most browsers. The only ones I know for sure won't work are IE4 and NS4, which are old enough to ignore IMO: after all, this is admin only, and we can put some stricter browser requirements on that. Recent versions of other browsers should work (it only does a simple visibility toggle). I like it because it's not an airplane cockpit, but still offers complex filtering options through refinement. It is also quite similar to the search/filtering UI in e.g. mail applications. The question is of course, how to degrade it for people without Javascript. My suggestion would be to simply show all filter options, and put a radiobutton in front of each: ( ) Where type is [ blog | v ] ( ) Where published status is ( ) published ( ) unpublished ( ) Where category is [ term1 | v ] ... It has more visual clutter, but the same principle of use. ------------------------------------------------------------------------ October 25, 2004 - 02:13 : Steven In fact, I just realized that the radio-button version can be coded to send exactly the same form data as the scripted version, which makes it more interesting. ------------------------------------------------------------------------ October 29, 2004 - 01:52 : Steven If you guys approve of my idea, I'll start coding. Assigning to myself for now. ------------------------------------------------------------------------ November 1, 2004 - 18:35 : Bèr Kessels I like this idea much more than my previous approaches. Gave it some thought and came with the follwing behaviour: * I think we need to discuss or at least design the tabular data too: It makes 0 sense to show the "types" column, if you chose "type = image" * I think we need to introduce a hook to add info to that tabulat view. Images can output thumbnails, for example * In tabular view, we should make: title a textfield, type and author a select and satus a select too. * I think we should introduce another range of options: "where content contains" [ ] and "where title contains" [ ] * I would like to get some feedback on how many drupal-using people are firmly against javascript, with a reason why. I bet that this is < 1% ! Bèr ------------------------------------------------------------------------ November 1, 2004 - 18:42 : Steven We don't have an anti-JS policy in Drupal. It's just that until recently, doing cross browser JS was a chore and making it accessible and degradable would be a disaster. This form can be implemented cleanly and will degrade without problems. Anyone who opposes it because it is JS can bugger off ;). ------------------------------------------------------------------------ November 3, 2004 - 20:24 : com2 There is a more radical way around JS. You could forget the table idea alltogether and present the nodes in Listbox. Then selecting becomes much easier with Ctrl-Click and Shift-Click ------------------------------------------------------------------------ November 4, 2004 - 01:02 : Bèr Kessels Did you have a look at the previous applied UI improvements? Did you consider the pre's and cons of those earlier posts? It seams you are repeating passed issues here, please reply with miore detailed issues here, Bèr ------------------------------------------------------------------------ December 3, 2004 - 11:49 : Steven Attachment: http://drupal.org/files/issues/node-admin.patch (13.1 KB) Here's a patch which implements the JS approach of refining the selection. ------------------------------------------------------------------------ December 3, 2004 - 11:50 : Steven Attachment: http://drupal.org/files/issues/node_admin.png (34.38 KB) And here's a screenshot of how it looks. ------------------------------------------------------------------------ December 3, 2004 - 20:23 : FactoryJoe(a)civicspacelabs.org This is looking better and better. I'm going to do a UI review of this a post any additional thoughts or comments. Chris ------------------------------------------------------------------------ December 3, 2004 - 22:50 : Anonymous Ack, I tested this using the bluebeach theme, but there's a small CSS bug with bluemarine, where the header for the operations form is stuck to the right of the filter form. I'll make an updated patch later. In the meantime, comments are much appreciated. ------------------------------------------------------------------------ December 4, 2004 - 00:15 : killes(a)www.drop.org I actually prefer the non-JS menu over the JS menu. Not (only) because of the JS but because I prefer radiobuttons over pulldown menus. The argument that the non-JS approach uses more vertical space can be mitigated by redirecting to a #results anchor. ------------------------------------------------------------------------ January 15, 2005 - 07:32 : Steven Attachment: http://drupal.org/files/issues/node.admin.patch (14.41 KB) Here's an updated patch... I have exams in a week, so I don't have time to finish up the Javascript version, so I removed it (it can be added later easily). However, the non-JS version is still very usable. Changes since last time: - Uses semantic XHTML (definition list, and yes, they are for more than just definitions, even w3c agrees). - Merges the various node flags (promoted, published, ...) into one "status" dropdown. This keeps the whole selector shorter, and is better UI-wise IMO. - Adds drupal_gotos, they were missing. - Updates node admin watchog messages to properly display the type. The code used t($node->type) which is a big no-no. hook_node_name() is now invoked instead. - Removes some dead code. - I rolled Ber's node mass deletion patch in as it affects the same piece of code. Updating either patch for the other separately would be a waste of time. I also added some more improvements to Bèr's code, such as removing the incorrect help from the confirmation page, cleaning up the code style a bit, not loading every node just to display its title and displaying a single drupal_set_message() after the deleting rather than one per node. The old issue is here: http://drupal.org/node/7743 ------------------------------------------------------------------------ January 15, 2005 - 07:32 : Steven Attachment: http://drupal.org/files/issues/node admin.png (30.19 KB) ------------------------------------------------------------------------ January 15, 2005 - 10:55 : Bèr Kessels Thanks for integrating the batch delete patch, Steven! -- View: http://drupal.org/node/10296 Edit: http://drupal.org/project/comments/add/10296

1 0

[drupal-devel] [feature] Centralize node queries somehow
by Jose A Reyero 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: Jose A Reyero Status: patch The function _node_rewrite_sql shoult have a default value for the $query param, as it is called at least once with no arguments. You get a 'warning' otherwise. Jose A Reyero Previous comments: ------------------------------------------------------------------------ December 23, 2004 - 22:25 : chx Attachment: http://drupal.org/files/issues/node_builder.patch (5.56 KB) Although I have posted this to the devel list, as Steve pointed out, putting patches to sandbox is not the best way to do things. So I open this thread. I have measured the time for build a node query in this is about 0.22-0.24ms (on an Athlon 933 MHz machine). ------------------------------------------------------------------------ December 23, 2004 - 23:33 : Dries I'm not likely to commit this. It's not conform with Drupal's coding conventions, but more importantly, a node query builder doesn't solve any real problems. It just adds a different way of doing things without offering a significant advantage. ------------------------------------------------------------------------ December 23, 2004 - 23:51 : chx Attachment: http://drupal.org/files/issues/node_builder_0.patch (5.67 KB) At this moment there are direct queries into the node table everywhere. After node_access_*_sql calls were invented, a lot of queries needed to be changed, and I think we will find more to be inserted. If someone does sg. else with the queries, permission, language etc. he needs to patch 60+ quieries in core alone. This is not a good thing. That's why I am proposing a central node query builder. As for code standards, code-style.pl node.module returns 17 errors, none of them comes from my patch. However, I should admit, there were a few spaces missing from the database.inc patch. So I resubmit. ------------------------------------------------------------------------ December 23, 2004 - 23:56 : chx Attachment: http://drupal.org/files/issues/node_builder_1.patch (5.67 KB) OK, I was wrong, there is a space missing in one of the rewritten queries. But I doubt this matters too much, as this is only a proposal and I think there will be a lot of revisions before it gets to the core. ------------------------------------------------------------------------ December 24, 2004 - 00:26 : drumm The blank lines between the comments and function starts will confuse the documentation parser on drupaldocs.org. I'm not sure if I support this quite yet. Reducing the number of queries and whatnot is great, but this patch doesn't do that quite yet. I'd only want this if the new system is more readable and easy to write. I'm not sure if this is true. As for code-style.pl, I don't think this is actually used much and it doesn't catch everything. Such as, most operators (=, =>, +, etc) like a space on either side of themselves. I'd support removing code-style.pl and throwing it in contributions somewhere. (If this needs further discussion or action, please file another issue, lets keep this focused on this patch). I'm going to wait for a few more revisions of this before I weigh in a +1 or -1. ------------------------------------------------------------------------ December 24, 2004 - 00:58 : Jose A Reyero I can really think of a number of advantages of this, and though not really for committing it yet, think is very good to see some discussion on the issue and some concrete implementations. When implementing i18n I run into this problem, of having to patch lots of queries -actually too many- in about all the modules doing any kind of node listing. This far, I'm stuck at this point. Then I realized it was just the same for node permissions, and will be the same again for whatever new functionality you want to add in the future which affects node listings. I think actually that direct access to 'node' table should be avoided when possible out of the node.module. And the only way to achieve this is to have some kind of query builder. And also think that chx's approach is very good. it provides some upper layer at the 'node' level, which can use some object level semantics while relying on a db layer query builder, which could be used later for other objects -users, taxonomy terms...- Just think of all the work that this could have saved when implementing permissions system, which I think is ugly and dirty anyway because of all that node_join, node_where... In the long run, adding such complex logic as sql code, patching and re-patching queries is bad, very bad, making any new change one higher level of complexity and driving us away from a OO model, which I think we should tend to. It's not using PHP objects. It is thinking of nodes as 'objects' and coding according to it. Oh, yes, sorry, this is growing too long to write it here :-). But we could talk also about db portability... ------------------------------------------------------------------------ December 24, 2004 - 01:14 : moshe weitzman I should add that the Organic Groups module could benefit from this node listing SQL builder. I have awkwardly worked around the problem for now but I don't like it. I don't know if this patch is good or not, but I do lean toward the functionality that it offers. Remember that any developer is free not to use the query builder and issue direct SQL as needed. If the query builder is making life hard, simply don't use, just like today. ------------------------------------------------------------------------ December 24, 2004 - 10:35 : Bèr Kessels I +1 for this functionality (i have no time to comment on coding style etc). It will not only improve maintainability, but will allow a far easier implementation of the i18n. i18n currently has a lot of patches, simpley to add logic to all those node sql queries referred to. ------------------------------------------------------------------------ December 24, 2004 - 13:20 : chx Attachment: http://drupal.org/files/issues/node_builder_2.patch (1.63 KB) OK, maybe the former approach was too complex. How about this? This requires a lot less change to the queries and no change to database.inc and ten times faster. As usual, a sample query is rewritten and an example of the proposed hook is provided. ------------------------------------------------------------------------ December 25, 2004 - 00:09 : chx Final thoughts for this day. If you like the last version, there is a possibility to automate the whole thing by adding the following three lines to the beginning of db_prefix_tables: Of course, there is very small chance of a loop here, so this would require more thought, but I think at the end this would be a good thing. ------------------------------------------------------------------------ December 27, 2004 - 12:55 : Dries I like the second approach better as it keep things readable and gives me a bit more control over the order in which things are written. Looking for more feedback from the others. Did you try updating more queries to make sure it works as intended in all cases? ------------------------------------------------------------------------ December 27, 2004 - 19:48 : chx Attachment: http://drupal.org/files/issues/node_builder_3.patch (1.23 KB) Yes, I tried with many different queries. And I have found one bug -- forgot the underscore in the regexp following FORM so I have rewritten the whole regexp. It is now a lot simpler :) And node_query is now reentrant, so if a function in the process of hook_sql calls node_query, it won't fall into an infinite loop. This situation did not occur but it's better to forestall such things. ------------------------------------------------------------------------ December 28, 2004 - 05:48 : moshe weitzman This patch is a step forward. I thought of another feature which becomes easier with the proposed hook_sql(). think of filtering like freshmeat.net. At freshmeat, you can say that you only want to see projects where OS=Windows and License=GPL (for example). A Drupal equivalent is 'only show me nodes related to Democrats and Poverty'. This sort if global node filtering is very hard in Drupal today. With this hook, it becomes easy. ------------------------------------------------------------------------ December 28, 2004 - 12:37 : Dries It's starting to look good (and you're getting more support)! Have you read the following threads: http://lists.drupal.org/archives/drupal-devel/2004-11/msg00198.html and http://lists.drupal.org/archives/drupal-devel/2004-11/msg00230.html? It's an open issue related to your work. If you start joining tables, you might have to add DISTINCT()s. When you are not joining tables, it would be nice if we'd not pay the cost of a DISTINCT(). It would be good if you could update the node queries in core to take advantage of it. It's not necessary while prototyping though. It would be good if the il8n team could take a closer look at this patch by trying to use it. It would be good if you could add some PHPDoc. ------------------------------------------------------------------------ January 4, 2005 - 07:55 : chx Here is a sum of changes in this new revision: Following Dries' advice, I read those threads and now DISTINCT(nid) is used only if there is something joined. Whether the access_grants system is used or not, is determined at the module configuration screen, so for efficiency reasons, I have patched system.module. I the very first version of my patch (you know, with arrays) I passed on the whole query so hook_sql implementations could easily determine whether they are interested in this query or not. In a very cool brainstorming with Jose, we found out, that passing the whole query is not a good thing, hints are enough. So when I will patch the forum queries, I will define a NODE_SQL_FORUM hint, and a filter which does something cool with forums (I have heard something about private forums not implemented -- I maybe wrong here) may do so. We now have a separate helper function to call hook_sql which is cool, 'cos with it the last node_access_*_sql calls which were lurking in node_search could be elinimated. ------------------------------------------------------------------------ January 4, 2005 - 07:56 : chx Attachment: http://drupal.org/files/issues/central_node_query.patch (6.46 KB) Funny thing. Forgot to attach the patch. ------------------------------------------------------------------------ January 6, 2005 - 00:46 : chx Another day, another revision. As this patch now has nothing to do with queries, but it is about rewriting node SQL statements, I have renamed it node_rewrite_sql, the helper _node_rewrite_sql and there is hook_node_rewrite_sql. hook_node_rewrite_sql expected return value has changed: it can return an array('join'=> ..., 'where'=>..., 'distinct'=> ) so it may determine whether DISTINCT is needed or not. Neither index is mandatory. That hook_node_rewrite_sql impementations will decide whether they are asking for DISTINCT or not, is -- I think -- a good opportunity to optimize for speed. For eg. I doubt every node access mechanism requires DISTINCT. One last note: hook return value can be also can be a string with a WHERE statement for the sake of simplicity. node_rewrite_sql now works with multiline queries. ------------------------------------------------------------------------ January 6, 2005 - 00:46 : chx Attachment: http://drupal.org/files/issues/central_node_query_0.patch (7.43 KB) I can't believe I once again did not attach the patch ------------------------------------------------------------------------ January 6, 2005 - 17:22 : chx Attachment: http://drupal.org/files/issues/central_node_query_1.patch (6.65 KB) Moshe, thanks for your patience and wisdom. Extract is thrown out, list is used. node_node_rewrite_sql looks better now. access_grants_present is determined in run time, once per page. ------------------------------------------------------------------------ January 6, 2005 - 21:35 : Dries The hint-stuff looks a bit hairy to me. It would be nice if this could be avoided but I see what it is necessary. The semaphore stuff is vague too. I don't see how you'd be able to get into an endless loop. It might actually be useful to turn node_access_grants_present() into a more generic module.inc function. Maybe something along the lines of module_implements($hook), which would return a list of modules implementing the specified $hook. I suggest you update more core modules. If you do, I'll measure the performance gain against a copy of drupal.org's database. ------------------------------------------------------------------------ January 6, 2005 - 22:47 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql.patch (15.72 KB) The hint stuff is something is pretty useful: hook implementations can fine tune themselves when to fire and when not. Good for features and performance, too. module_implements is done. Semaphore -- we will have tons of node_rewrite_sql calls in the core. Now someone writes a hook_node_rewrite_sql implementation which calls a function which in turn calls another one which calls node_rewrite_sql . I would rather not be the one who debugs such a scenario. Thanks for the offer! forum.module is now patched. Others will follow tomorrow. ------------------------------------------------------------------------ January 6, 2005 - 23:21 : Jose A Reyero I think this looks good enough, so I'm starting to test and use this for i18n. If this get into the node module, I don't think I will need any node.module patch anymore... A pair of performance hints: - As this node_rewrite_sql is going to be called several times for each page, we can cache the list of modules implementing the hook in a static variable. - We can even cache the whole result for each 'hint' value, so the whole hook will only be called once or twice for each page. Some suggested 'hint' values to be defined: NODE_REWRITE_SQL_LIST -> This one for simple generic list of nodes for the main page, could be default. NODE_REWRITE_SQL_ADMIN_LIST -> List of nodes for administration pages NODE_REWRITE_SQL_RELATED_LIST -> List of objects related to nodes, but no nodes themselves -like comments-, node table may have to be joined... NODE_REWRITE_SQL_BLOCK_LIST -> Listing for a block NODE_REWRITE_SQL_NO_LANGUAGE -> No need to add language conditions. This could hapen when you need a listing for other languages, or mixed languages, for translators or for administrators.. I would need something like this last one for i18n, just to have the permissions working and some flexibility with language conditions. And also the one you already have in place ...._SEARCH. All this will allow for options like search, admin, list current/all languages... ------------------------------------------------------------------------ January 7, 2005 - 17:30 : Jose A Reyero Attachment: http://drupal.org/files/issues/central_node_query_node_module.patch (7.07 KB) I found some problems when using the patch. The where conditions are not merged well if there are more than once, and also conditions need some parenthesis around. So I replaced the $where and $join strings in _node_rewrite_sql by arrays which are imploded at the end. Also added some 'hint' definitions. Patches for i18n module are coming next. This works like charm for node listings, comment listings, searches, etc... Btw, I also think 'semaphores' are not really needed. ------------------------------------------------------------------------ January 7, 2005 - 19:04 : Dries I don't see how the hint system can be portable: are contributed modules supposed to define their own hits? ------------------------------------------------------------------------ January 7, 2005 - 22:54 : chx The hints -- I was in a hurry yesterday evening, so I have forgot to define a NODE_REWRITE_SQL_FORUM in the forum.module patch and also forgot to use the hints. So I think core node types will have their hint defined. I have a vague idea about how contrib modules could define their own hint type: we shall have a NODE_REWRITE_SQL_HINT_MAX, and every hint define should like this -- beware this is not working! This is just an idea. Well, if defines does not work, maybe a global node_rewrite_sql_hints array should be introduced...? ------------------------------------------------------------------------ January 8, 2005 - 09:54 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_0.patch (16.69 KB) Here is a list of changes: Complete rethought of hint system. Uses array of strings instead of defines. There are a number of examples. the new comment.module patch shows a good usage example for the third parameter of node_rewrite_sql which is at the moment called node_alias. It's really not a good name, it should be "the alias of the table which holds the nid field for this query". BTW. there should be an index on comment.nid. Incorporated Jose's "replaced the $where and $join strings in _node_rewrite_sql by arrays which are imploded at the end". I was a fool to lose these, the first version (which used array) used a proper implode for WHEREs. ------------------------------------------------------------------------ January 8, 2005 - 11:27 : Dries I have two fundamental problems with it this patch: The loop detection mechanism has to go. The hints are difficult to understand and I'm not convinced they are good/needed. Does the patch contain a good example of a node_rewrite_sql function/hook that uses these hints? I can't find one. I won't like these hints until I'm convinced they are needed and that they represent the best possible solution. Convince me. Could you extend the documentation about the hint-system? Currently it reads "An array of hint strings about the query, passed on to hook_sql handlers.". Clearly, that doesn't help me at all. The documentation should answer the following questions: (1) what are the hints for, (2) when should I use them (or not use them) and (3) what does hints look like (what is their format/syntax)? Note that I can't compare performance unless more queries have been updated. Thanks chx. ------------------------------------------------------------------------ January 8, 2005 - 11:30 : Dries Why does the hints in the comment module include 'related_list'. This is confusing. ------------------------------------------------------------------------ January 8, 2005 - 12:52 : Jose A Reyero Well, it seems we have different views about the 'hints'. At this point, the node_rewrite has two main uses: permissions and language conditions (i18n). I don't know whether they could have any use for permission system, but for i18n, some extra information about the rewritten queries will be needed: - Most of the queries will just select fields in the node table. Here, only a where condition is needed, but language condition may be added or not depending on the kind of query: _LIST would be a list for the main page, so only in this case, decissions can be made depending on the path. For some pages, you may want a list of nodes for all languages and for some others, only current language. _ADMIN_LIST is a list of nodes for administration. An option will be added in i18n for 'administering all languages together' or 'administer each language separately', or even a language field could be added in the search form. So language conditions will be merged diferently _BLOCK_LIST is a list of nodes for blocks. I'm thinking here of a global option like 'show mixed languages in blocks'. Anyway, this is a list for a block and you cannot have any logic depending on path here. _SEARCH, would be only for searching, and in this case you may decide to search 'all languages' or only the current one. This can be a global option or some extra field in search form. When doing text searches, searching all languages together will make a lot of sense, but also restricting search by language. About _RELATED_LIST, this is the case of comments, is a list of objects which are not nodes themselves, but related objects in a different table. Here, the node table itself may have to be joined as it is not yet in the query. When dealing with the translation system, the language conditions will be added or not by the module requesting the query itself, so no further language conditions will be needed. Queries will have to be run anyway through 'node_rewrite_sql' to have permissions in place. That's what the _NO_LANGUAGE hint is about. So this is my proposed use for the 'hint' thing, though maybe the hints themselves could be better defined or elaborated, but this is the main idea. I mean no specific hints for modules but generic ones for 'kind of listing' you are requesting. Maybe for the permission system, it could be also some use, as you could decide you want to show a list of objects to anyone, but deny access to the objects themselves, something like 'subscribe or register if you want to read the article'. So permissions here will apply differently depending on what you are doing. We can think also about more creative use of permissions like 'you can read latest articles' but old ones will be available only for selected people, or the opposite 'only old content available for everybody'. Or even 'you can read articles but not comments'. This kind of permissions would need also some 'hints'. ------------------------------------------------------------------------ January 8, 2005 - 13:17 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_1.patch (26.89 KB) More queries rewritten. Loop detection gone. Will gather my thoughts on hint system later, but I do hope this amount of queries will be enough for a performance measurement. Now only blogapi, blog, book calls node_access_*_sql, all others are done. ------------------------------------------------------------------------ January 8, 2005 - 13:20 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_2.patch (26.88 KB) Forgot to save taxonomy.module before rolling the patch, so a DISTINCT remained :( ------------------------------------------------------------------------ January 8, 2005 - 13:38 : chx I see Jose has written a lot about hints, thanks! "Well, it seems we have different views about the 'hints'." -- "we" probably means Dries vs. "Jose and I" 'cos I agree with Jose, but I see some more uses, namely what I have said before on making a filter on forums. If you want to that, you need to know which queries are coming from the forum.module. What shall we do? Pass on the whole query, and let the filter proccess the query to decide whether now it is the forum block it hunts for? I doubt. Let's give it a hint it's a forum and a block query. Some kind of extensionable hint mechanism is necessary, 'cos what about a contrib module which filters another contrib node, say flexinode? ------------------------------------------------------------------------ January 9, 2005 - 15:05 : Dries Could we drop the hint system if we'd choose not to support the il8n module for now? ------------------------------------------------------------------------ January 9, 2005 - 16:54 : Jose A Reyero :-( Ok, let's try to think positively. Could we at least pass the query along so i18n -or whatever module tries to implement this hook- could support itself? ------------------------------------------------------------------------ January 9, 2005 - 18:32 : chx Well, passing along the query would mean no problems, but I wonder whether a better documented hint system would be better? I had no problems with letting the loop detection system go it was just a safety belt for some distant future possibility. But the hint system is -- in my point -- is needed. So, here are the valid hint strings: node, forum, comment etc.name of the module from the query originates, like forum, comment, flexinode etc. genericthis would be be for generic listings, like node_default_page, forum_get_forums etc. relatednode table is not joined so if you want to use other fields from node table than nid you shall join node table yourself. However, if you have your table which is like JOIN {mytable} ON mt.nid=$nid_alias.nid that's fine. adminsomething meant only for administrators. blocknodes in a block searchsomething to do with a search ------------------------------------------------------------------------ January 9, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_3.patch (28.31 KB) Killes said I did not had -F^f in the last version. ------------------------------------------------------------------------ January 9, 2005 - 22:37 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_4.patch (28.77 KB) Automatic version as discussed and args for Killes. ------------------------------------------------------------------------ January 9, 2005 - 22:55 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_5.patch (28.31 KB) Hm. It seems we have agreed on no automatization. However, arbitrary arguments are kept for Killes. ------------------------------------------------------------------------ January 10, 2005 - 21:02 : Dries The last version of this patch improved the performance of my test site's main page by 257%. That is, my machine can serve 2.57 times as much pages in the same time interval. Out of curiousity, I investigated this some more and it turns out this can be tracked down to the useless DISTINCT's which this patch removes. Unless someone objects to this patch or the approach taken, I'm going to commit this shortly. Please review this patch carefully as it marks an important change. ------------------------------------------------------------------------ January 10, 2005 - 22:39 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_6.patch (44.18 KB) More DISTINCTs cleared, now there are no more node_access_*_sql calls in the core modules. This may be a final version. Or we can find a few more to rewrite. I've searched for {node} selects, dropped those which had nid = %d as I thought those do not need rewrite (I may be wrong) and dropped also those that are rewritten, dropped those that are indexing or cron. Here are the remaining queries, which may need rewriting: ------------------------------------------------------------------------ January 10, 2005 - 23:02 : Chris Johnson This patch seems like both a Good Thing and a Bad Thing to me. The idea of centralizing node queries is a good one, I think. It seems to be the original motivation for this patch. It allows better SQL optmization and hence performance. It puts all the code in one place for better future maintenance and enhancement. On the other hand, constructing complex SQL statements from programmatic logic is full of peril. The task quickly becomes very large and complex. Manipulating the SQL language elements via code means it is harder to see when one has made a syntactic mistake. Most such errors will be fatal. As the SQL construction code becomes more complex with development, the number of paths through it become huge and impossible to exhaustively test. Some day, someone will pass parameters that cause fatal SQL syntax to be generated in a "production" system. It also obscures SQL syntax differences between supported databases. It's more likely to introduce usage that works with MySQL but fails with Postgres. I've long been in favor of unifying and centralizing all of the database access in Drupal, to the greatest extent which does not adversely affect performance. So in this way, I think this patch is a big step in that general direction. But I've also been involved in maintaining a search engine which used this technique to create the SQL statements on the fly. It became extremely complex over time as one, then two, then three, etc. new possibilities were added (e.g. new usage, new search criteria). The more complex it became, the more regression errors we had and the harder to debug it became. It got to the point where making one small addition would result in 3 new bugs. It worked most of the time, and when it worked, it was fast. But it failed fatally in a significant number of ways. I'm not sure whether to vote +3 or -4 on this one. It might be good; it might be terrible. ------------------------------------------------------------------------ January 10, 2005 - 23:19 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_7.patch (44.18 KB) Chris, I understand your concerns, but this method -- unlike the original -- tries to keep things as simple as possible. You can only JOIN tables and add WHERE conditions. We are not creating complex SQL on the fly, I tried that originally, just read comment #1 for the warm welcome by Dries: "I'm not likely to commit this". So do not worry, while I may be a little hotheaded, Dries keeps order with an iron hand. And the actual rewrite mechanism is also very simple. Ps. Two typos corrected in this version. ------------------------------------------------------------------------ January 13, 2005 - 23:20 : Dries I'm going to test and benchmark the latest version of this patch over the weekend. If all goes well, I'll commit this to HEAD as no concrete alternatives have been proposed. (I can't think of a better approach and believe that this patch does a good job balancing different values/requirements.) It would be much appreciated if those who maintain node-level permission modules could take a look at this patch to evaluate its usefulness/performance. If you maintain a module that queries the node table you might want to look at this as well -- after all, you'll have to update your module if this hits core. (Note that the PHPdoc code in the patch is not up-to-date.) ------------------------------------------------------------------------ January 14, 2005 - 13:47 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_8.patch (43.97 KB) PHPdoc is upgraded, I hope now it is up to date. ------------------------------------------------------------------------ January 15, 2005 - 09:39 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_9.patch (43.98 KB) module_implementations had a minor, but performance eating bug, it's caching was broken. -- View: http://drupal.org/node/14731 Edit: http://drupal.org/project/comments/add/14731

1 0

[drupal-devel] [feature] Node type filter on node admin page
by Bèr Kessels 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node.module Category: feature requests Priority: normal Assigned to: Steven Reported by: Anonymous Updated by: Bèr Kessels Status: patch Thanks for integrating the batch delete patch, Steven! Bèr Kessels Previous comments: ------------------------------------------------------------------------ August 23, 2004 - 17:09 : Anonymous Attachment: http://drupal.org/files/issues/node_admin_filter.patch (3.8 KB) Here's a patch for the current CVS version which adds the option to filter nodes by type on the node admin page. Having built a few sites recently which make heavy use of flexinodes, the ability to filter out particular node types would have been extremely useful, so I hope this makes it to 4.5.0. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ September 4, 2004 - 00:38 : Bèr Kessels I think we should really come up with a good solution for this screen. In the past, I went looking for such a solution and found that the various MP3 library tools might come to help. They deal with large amounts of metadata, some of wch are important, other are not. I summarise here what the UI could be like: ** The menu-based (tree) approach: Most mp3 collections use this idea. In a tree like structure on the side, you can make selections. The list in the main area will follow this selection. (Itunes: http://www.apple.com/itunes/playlists.html , Juk: http://developer.kde.org/~wheeler/images/juk-2.0/juk-2.png, finf/freeamp: http://www.zinf.org/images/zinf_mymusic_shot.png) In drupal that would mean a menu-based filtering method. I once made a patch for that, but that was rejected: http://drupal.org/node/view/7766 The good thing here, is that it is virtually endless scalable. So in theory, flexinode coud add menu items to show only nodes where foo is bar. | - nodetype | |- blog | |= Flexinode | | |=foo | | | |=bar | |- page ** The multiple selectlist UI: Another moethod is a extended method of what we have now: select items from select boxes and press submit. Above the main area one will find some selection UI where one can in- and exlude sertain criteria. (rhythmbox: http://www.rhythmbox.org/screenshots/Screenshot-Rhythmbox.png, and winamp http://www.winamp.com/player/walkthrough.php) Note that both still show some form of tree navigation. In drupal that would mean multiple selects in three columns (rhytmbox and winamp show only two artist and album) For drupal we need three of thos blocks: one for node-types, one for node-settings (promoted, etc) and one for taxonomy terms. The node settings should be logical too: If a criteria is selected it it true, if not: false. That way we can also get rod of the annoying logic: "view posts that are not promoted" and the likes. In both solutions the "perform batch action" needs to move somewhere else. I would plead for moving it to the bottom of the list. A lot of webmail cleints do it that way. The reason being simple: the steps you take to perform the update are: 1) select criteria (find nodes) 2) checkmark the chages (we need columns of checkboxes) 3) perform the update. Logically the way you go through the UI is left->right, top-> down (not taking into consideration japanese/hebrew/arabs etc) So all in all: i think indeed the UI needs serious work. I am willing to take this one me, bu I do need some consensus first. I am not willing to spend hours on a new UI (like the menu-based approach) to find the patch being rejected.Therefore I like some discussion up fron, rather than the " code first see later" approach. Regards, Ber ------------------------------------------------------------------------ September 5, 2004 - 23:42 : Bèr Kessels Attachment: http://drupal.org/files/issues/new_UI_proposal.png (175.31 KB) Here is a gimped mockup from what option 2) could look like. WE could add a fouth box with users too, but I am not sure how that would go with permissions and the like. I need to do some thinking about selecatibility by users first. ------------------------------------------------------------------------ September 6, 2004 - 00:01 : Bèr Kessels Sorry for the spam: but I forgot to mention that there are a few issues: The "perform action dropdown" should list stuff as: promote, demote, etc. But that list needs to be generated with the selection above in mind. So if you choose " published" the action list should not have "publish these nodes" Also the action list could contain "recategorise" wich would need another screen after the one in the "ugly screendump-gimp-mockup", to be able to choose the categories in wich the selection should be placed. And of course the delete these items with a confimation dialog. (http://drupal.org/node/view/7743#comment-11624) ------------------------------------------------------------------------ September 6, 2004 - 14:32 : Bèr Kessels Attachment: http://drupal.org/files/issues/node_admin.html (29.95 KB) After a long discussion with Steven And gerhard, I designed the page in HTML, to let you all try it in some way. Beware: the html is an ugly mock, It is meant as scetch, not as valid html. Some notes on the page: "View all nodes that are:" contains three differnet possibilities for searching. The first two (above the blank line) are there for simplicity, but they might confuse users, because they need a lot of text. The second idea uses a checkbox to enable the select next to it. The third one uses some ajvascript to improve useablity. We should of course choose one of these three methods. "View all nodes that are classified in:" There are quite some people that have questions with whether or not we should include this selection aat all. Here is why I think it should definately go Because most sites have only one purpose and thus a lot of nodes of one type in one state. For example a personal blog site: 99% of the nodes are blogs. 99% are promoted. Only 1 or 2 are not promoted, not published or of type page. Or take a photo-album-site. All posts will be images, the main difference is the classification (the galleries) I placed them in. "any/all" There must be a way to choose whether the sected options should be an AND or an OR. I tried three different methods, each block has one method. We should choose one. Please give me feedback, for I still have to decide whether or not to make this a contributed module or start lobby-ing to get it in core. ------------------------------------------------------------------------ September 13, 2004 - 11:50 : moshe weitzman looking good, and really useful. a few small comments: - consider upping the number of nodes that appear on this page. it is nearly pointless to use paging on this sort of UI - i like the second idea of an 'enabled' checkbox, without the javascript. i think you can use 2 checkboxes, where the second one is called 'publish', 'promote', and 'has comments' - I prefer the horizontal configuration of the 'Require: all any' radio buttons - don't repeat 'the selected posts' in the choices for the bottom dropdown. perhaps use a title for that form_select() instead. - the last choice in that dropdown is 'reclassify'. i don't see how this screen could support that. instead, use my taxonomy_multi_edit.module from contrib :) i suggest making this a contrib module at first. it can be a tab off of the current page. thats how taxonomy_multi_edit.module appears. ------------------------------------------------------------------------ September 13, 2004 - 13:03 : Bèr Kessels About that reclassify: That was exactly my intention: using the taxonomy_muliti_edit module for that. All I wanted to do is pass a list of nids to your module and open a screen using that module with the previously selected nodes! About the other comments: Yes upping the number of nodes might be a good idea. Anyone any idea what number would still be usefull. 10.000 nodes (on drupal.org) in one list is not a good idea, IMO, so some paging is needed. indeed: repeatinf words in dropdowns is considered "bad UI design" thank you for pointing it out. Regards, Ber ------------------------------------------------------------------------ September 28, 2004 - 13:37 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type.patch (1.96 KB) here is an updated version of this patch, syncing it with CVS HEAD. Also, i did change the functionality somewhat: previous versions of the patch asked the modules what node types were out there (via hook_node_name()). This version asks the node table what types are in there (via SELECT DISTINCT type FROM {node}). I find this more useful, because it's entirely possible that, over time, modules will be removed from the system, yet they still have rows in the node table. This method gives a more accurate view of the types of nodes on the system. The new UI looks excellent, but this patch does not address it. ------------------------------------------------------------------------ September 28, 2004 - 19:05 : drumm -1 I applied and tested and it did not work at all for me. The type options selection only contains 's' and 'W', selecting one does not work. The list of types seems to come from the DB which is probably an unnecessary query. ------------------------------------------------------------------------ September 29, 2004 - 12:51 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type_1.patch (2.31 KB) Here's a version of that patch that should work. Sorry. I think that getting the types from the node table is necessary. The list of installed nodes is not the authority on what types of nodes are in the db; we should ask the db directly. It's a small query on one admin page; the result is that the dropdown has in it the most accurate information available. That's worth it, imo. ------------------------------------------------------------------------ September 29, 2004 - 12:52 : eafarris s/list of installed nodes/list of installed modules/ . sheesh. teach me to post before my tea. ------------------------------------------------------------------------ October 21, 2004 - 18:15 : Bèr Kessels Attachment: http://drupal.org/files/issues/node.html (29.33 KB) I do not know where it weant, but my proposal had a HTML mockup. Attaching it here to see what i meant. ------------------------------------------------------------------------ October 23, 2004 - 13:14 : com2 Nice filter options, but what I really miss it the possibility to select all checkboxes. Really boring job when you have to click the whole list one by one. ------------------------------------------------------------------------ October 24, 2004 - 10:41 : Bèr Kessels I am not a big fan of those options in forms. They more often break things than that thy fix them. Rahter use a bookmarklet r so for that: http://www.squarefree.com/bookmarklets/forms.html Ber ------------------------------------------------------------------------ October 25, 2004 - 01:03 : Steven Attachment: http://drupal.org/files/issues/node_admin_ucd.html (29.83 KB) Based on some more IRC discussion, I came up with this (attachment). It is based on an earlier mockup. It does use Javascript and doesn't degrade without script (yet), but I think it's worth considering. It can be implemented quite cleanly. The idea would be to have a filter like this initially, and after clicking 'Go', to show the current filter as well as the form again, allowing you to refine your search. I haven't tested browser compatibility, but it should work on most browsers. The only ones I know for sure won't work are IE4 and NS4, which are old enough to ignore IMO: after all, this is admin only, and we can put some stricter browser requirements on that. Recent versions of other browsers should work (it only does a simple visibility toggle). I like it because it's not an airplane cockpit, but still offers complex filtering options through refinement. It is also quite similar to the search/filtering UI in e.g. mail applications. The question is of course, how to degrade it for people without Javascript. My suggestion would be to simply show all filter options, and put a radiobutton in front of each: ( ) Where type is [ blog | v ] ( ) Where published status is ( ) published ( ) unpublished ( ) Where category is [ term1 | v ] ... It has more visual clutter, but the same principle of use. ------------------------------------------------------------------------ October 25, 2004 - 01:13 : Steven In fact, I just realized that the radio-button version can be coded to send exactly the same form data as the scripted version, which makes it more interesting. ------------------------------------------------------------------------ October 29, 2004 - 00:52 : Steven If you guys approve of my idea, I'll start coding. Assigning to myself for now. ------------------------------------------------------------------------ November 1, 2004 - 17:35 : Bèr Kessels I like this idea much more than my previous approaches. Gave it some thought and came with the follwing behaviour: * I think we need to discuss or at least design the tabular data too: It makes 0 sense to show the "types" column, if you chose "type = image" * I think we need to introduce a hook to add info to that tabulat view. Images can output thumbnails, for example * In tabular view, we should make: title a textfield, type and author a select and satus a select too. * I think we should introduce another range of options: "where content contains" [ ] and "where title contains" [ ] * I would like to get some feedback on how many drupal-using people are firmly against javascript, with a reason why. I bet that this is < 1% ! Bèr ------------------------------------------------------------------------ November 1, 2004 - 17:42 : Steven We don't have an anti-JS policy in Drupal. It's just that until recently, doing cross browser JS was a chore and making it accessible and degradable would be a disaster. This form can be implemented cleanly and will degrade without problems. Anyone who opposes it because it is JS can bugger off ;). ------------------------------------------------------------------------ November 3, 2004 - 19:24 : com2 There is a more radical way around JS. You could forget the table idea alltogether and present the nodes in Listbox. Then selecting becomes much easier with Ctrl-Click and Shift-Click ------------------------------------------------------------------------ November 4, 2004 - 00:02 : Bèr Kessels Did you have a look at the previous applied UI improvements? Did you consider the pre's and cons of those earlier posts? It seams you are repeating passed issues here, please reply with miore detailed issues here, Bèr ------------------------------------------------------------------------ December 3, 2004 - 10:49 : Steven Attachment: http://drupal.org/files/issues/node-admin.patch (13.1 KB) Here's a patch which implements the JS approach of refining the selection. ------------------------------------------------------------------------ December 3, 2004 - 10:50 : Steven Attachment: http://drupal.org/files/issues/node_admin.png (34.38 KB) And here's a screenshot of how it looks. ------------------------------------------------------------------------ December 3, 2004 - 19:23 : FactoryJoe(a)civicspacelabs.org This is looking better and better. I'm going to do a UI review of this a post any additional thoughts or comments. Chris ------------------------------------------------------------------------ December 3, 2004 - 21:50 : Anonymous Ack, I tested this using the bluebeach theme, but there's a small CSS bug with bluemarine, where the header for the operations form is stuck to the right of the filter form. I'll make an updated patch later. In the meantime, comments are much appreciated. ------------------------------------------------------------------------ December 3, 2004 - 23:15 : killes(a)www.drop.org I actually prefer the non-JS menu over the JS menu. Not (only) because of the JS but because I prefer radiobuttons over pulldown menus. The argument that the non-JS approach uses more vertical space can be mitigated by redirecting to a #results anchor. ------------------------------------------------------------------------ January 15, 2005 - 06:32 : Steven Attachment: http://drupal.org/files/issues/node.admin.patch (14.41 KB) Here's an updated patch... I have exams in a week, so I don't have time to finish up the Javascript version, so I removed it (it can be added later easily). However, the non-JS version is still very usable. Changes since last time: - Uses semantic XHTML (definition list, and yes, they are for more than just definitions, even w3c agrees). - Merges the various node flags (promoted, published, ...) into one "status" dropdown. This keeps the whole selector shorter, and is better UI-wise IMO. - Adds drupal_gotos, they were missing. - Updates node admin watchog messages to properly display the type. The code used t($node->type) which is a big no-no. hook_node_name() is now invoked instead. - Removes some dead code. - I rolled Ber's node mass deletion patch in as it affects the same piece of code. Updating either patch for the other separately would be a waste of time. I also added some more improvements to Bèr's code, such as removing the incorrect help from the confirmation page, cleaning up the code style a bit, not loading every node just to display its title and displaying a single drupal_set_message() after the deleting rather than one per node. The old issue is here: http://drupal.org/node/7743 ------------------------------------------------------------------------ January 15, 2005 - 06:32 : Steven Attachment: http://drupal.org/files/issues/node admin.png (30.19 KB) -- View: http://drupal.org/node/10296 Edit: http://drupal.org/project/comments/add/10296

1 0

[drupal-devel] [feature] Centralize node queries somehow
by chx 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/node_rewrite_sql_9.patch (43.98 KB) module_implementations had a minor, but performance eating bug, it's caching was broken. chx Previous comments: ------------------------------------------------------------------------ December 23, 2004 - 22:25 : chx Attachment: http://drupal.org/files/issues/node_builder.patch (5.56 KB) Although I have posted this to the devel list, as Steve pointed out, putting patches to sandbox is not the best way to do things. So I open this thread. I have measured the time for build a node query in this is about 0.22-0.24ms (on an Athlon 933 MHz machine). ------------------------------------------------------------------------ December 23, 2004 - 23:33 : Dries I'm not likely to commit this. It's not conform with Drupal's coding conventions, but more importantly, a node query builder doesn't solve any real problems. It just adds a different way of doing things without offering a significant advantage. ------------------------------------------------------------------------ December 23, 2004 - 23:51 : chx Attachment: http://drupal.org/files/issues/node_builder_0.patch (5.67 KB) At this moment there are direct queries into the node table everywhere. After node_access_*_sql calls were invented, a lot of queries needed to be changed, and I think we will find more to be inserted. If someone does sg. else with the queries, permission, language etc. he needs to patch 60+ quieries in core alone. This is not a good thing. That's why I am proposing a central node query builder. As for code standards, code-style.pl node.module returns 17 errors, none of them comes from my patch. However, I should admit, there were a few spaces missing from the database.inc patch. So I resubmit. ------------------------------------------------------------------------ December 23, 2004 - 23:56 : chx Attachment: http://drupal.org/files/issues/node_builder_1.patch (5.67 KB) OK, I was wrong, there is a space missing in one of the rewritten queries. But I doubt this matters too much, as this is only a proposal and I think there will be a lot of revisions before it gets to the core. ------------------------------------------------------------------------ December 24, 2004 - 00:26 : drumm The blank lines between the comments and function starts will confuse the documentation parser on drupaldocs.org. I'm not sure if I support this quite yet. Reducing the number of queries and whatnot is great, but this patch doesn't do that quite yet. I'd only want this if the new system is more readable and easy to write. I'm not sure if this is true. As for code-style.pl, I don't think this is actually used much and it doesn't catch everything. Such as, most operators (=, =>, +, etc) like a space on either side of themselves. I'd support removing code-style.pl and throwing it in contributions somewhere. (If this needs further discussion or action, please file another issue, lets keep this focused on this patch). I'm going to wait for a few more revisions of this before I weigh in a +1 or -1. ------------------------------------------------------------------------ December 24, 2004 - 00:58 : Jose A Reyero I can really think of a number of advantages of this, and though not really for committing it yet, think is very good to see some discussion on the issue and some concrete implementations. When implementing i18n I run into this problem, of having to patch lots of queries -actually too many- in about all the modules doing any kind of node listing. This far, I'm stuck at this point. Then I realized it was just the same for node permissions, and will be the same again for whatever new functionality you want to add in the future which affects node listings. I think actually that direct access to 'node' table should be avoided when possible out of the node.module. And the only way to achieve this is to have some kind of query builder. And also think that chx's approach is very good. it provides some upper layer at the 'node' level, which can use some object level semantics while relying on a db layer query builder, which could be used later for other objects -users, taxonomy terms...- Just think of all the work that this could have saved when implementing permissions system, which I think is ugly and dirty anyway because of all that node_join, node_where... In the long run, adding such complex logic as sql code, patching and re-patching queries is bad, very bad, making any new change one higher level of complexity and driving us away from a OO model, which I think we should tend to. It's not using PHP objects. It is thinking of nodes as 'objects' and coding according to it. Oh, yes, sorry, this is growing too long to write it here :-). But we could talk also about db portability... ------------------------------------------------------------------------ December 24, 2004 - 01:14 : moshe weitzman I should add that the Organic Groups module could benefit from this node listing SQL builder. I have awkwardly worked around the problem for now but I don't like it. I don't know if this patch is good or not, but I do lean toward the functionality that it offers. Remember that any developer is free not to use the query builder and issue direct SQL as needed. If the query builder is making life hard, simply don't use, just like today. ------------------------------------------------------------------------ December 24, 2004 - 10:35 : Bèr Kessels I +1 for this functionality (i have no time to comment on coding style etc). It will not only improve maintainability, but will allow a far easier implementation of the i18n. i18n currently has a lot of patches, simpley to add logic to all those node sql queries referred to. ------------------------------------------------------------------------ December 24, 2004 - 13:20 : chx Attachment: http://drupal.org/files/issues/node_builder_2.patch (1.63 KB) OK, maybe the former approach was too complex. How about this? This requires a lot less change to the queries and no change to database.inc and ten times faster. As usual, a sample query is rewritten and an example of the proposed hook is provided. ------------------------------------------------------------------------ December 25, 2004 - 00:09 : chx Final thoughts for this day. If you like the last version, there is a possibility to automate the whole thing by adding the following three lines to the beginning of db_prefix_tables: Of course, there is very small chance of a loop here, so this would require more thought, but I think at the end this would be a good thing. ------------------------------------------------------------------------ December 27, 2004 - 12:55 : Dries I like the second approach better as it keep things readable and gives me a bit more control over the order in which things are written. Looking for more feedback from the others. Did you try updating more queries to make sure it works as intended in all cases? ------------------------------------------------------------------------ December 27, 2004 - 19:48 : chx Attachment: http://drupal.org/files/issues/node_builder_3.patch (1.23 KB) Yes, I tried with many different queries. And I have found one bug -- forgot the underscore in the regexp following FORM so I have rewritten the whole regexp. It is now a lot simpler :) And node_query is now reentrant, so if a function in the process of hook_sql calls node_query, it won't fall into an infinite loop. This situation did not occur but it's better to forestall such things. ------------------------------------------------------------------------ December 28, 2004 - 05:48 : moshe weitzman This patch is a step forward. I thought of another feature which becomes easier with the proposed hook_sql(). think of filtering like freshmeat.net. At freshmeat, you can say that you only want to see projects where OS=Windows and License=GPL (for example). A Drupal equivalent is 'only show me nodes related to Democrats and Poverty'. This sort if global node filtering is very hard in Drupal today. With this hook, it becomes easy. ------------------------------------------------------------------------ December 28, 2004 - 12:37 : Dries It's starting to look good (and you're getting more support)! Have you read the following threads: http://lists.drupal.org/archives/drupal-devel/2004-11/msg00198.html and http://lists.drupal.org/archives/drupal-devel/2004-11/msg00230.html? It's an open issue related to your work. If you start joining tables, you might have to add DISTINCT()s. When you are not joining tables, it would be nice if we'd not pay the cost of a DISTINCT(). It would be good if you could update the node queries in core to take advantage of it. It's not necessary while prototyping though. It would be good if the il8n team could take a closer look at this patch by trying to use it. It would be good if you could add some PHPDoc. ------------------------------------------------------------------------ January 4, 2005 - 07:55 : chx Here is a sum of changes in this new revision: Following Dries' advice, I read those threads and now DISTINCT(nid) is used only if there is something joined. Whether the access_grants system is used or not, is determined at the module configuration screen, so for efficiency reasons, I have patched system.module. I the very first version of my patch (you know, with arrays) I passed on the whole query so hook_sql implementations could easily determine whether they are interested in this query or not. In a very cool brainstorming with Jose, we found out, that passing the whole query is not a good thing, hints are enough. So when I will patch the forum queries, I will define a NODE_SQL_FORUM hint, and a filter which does something cool with forums (I have heard something about private forums not implemented -- I maybe wrong here) may do so. We now have a separate helper function to call hook_sql which is cool, 'cos with it the last node_access_*_sql calls which were lurking in node_search could be elinimated. ------------------------------------------------------------------------ January 4, 2005 - 07:56 : chx Attachment: http://drupal.org/files/issues/central_node_query.patch (6.46 KB) Funny thing. Forgot to attach the patch. ------------------------------------------------------------------------ January 6, 2005 - 00:46 : chx Another day, another revision. As this patch now has nothing to do with queries, but it is about rewriting node SQL statements, I have renamed it node_rewrite_sql, the helper _node_rewrite_sql and there is hook_node_rewrite_sql. hook_node_rewrite_sql expected return value has changed: it can return an array('join'=> ..., 'where'=>..., 'distinct'=> ) so it may determine whether DISTINCT is needed or not. Neither index is mandatory. That hook_node_rewrite_sql impementations will decide whether they are asking for DISTINCT or not, is -- I think -- a good opportunity to optimize for speed. For eg. I doubt every node access mechanism requires DISTINCT. One last note: hook return value can be also can be a string with a WHERE statement for the sake of simplicity. node_rewrite_sql now works with multiline queries. ------------------------------------------------------------------------ January 6, 2005 - 00:46 : chx Attachment: http://drupal.org/files/issues/central_node_query_0.patch (7.43 KB) I can't believe I once again did not attach the patch ------------------------------------------------------------------------ January 6, 2005 - 17:22 : chx Attachment: http://drupal.org/files/issues/central_node_query_1.patch (6.65 KB) Moshe, thanks for your patience and wisdom. Extract is thrown out, list is used. node_node_rewrite_sql looks better now. access_grants_present is determined in run time, once per page. ------------------------------------------------------------------------ January 6, 2005 - 21:35 : Dries The hint-stuff looks a bit hairy to me. It would be nice if this could be avoided but I see what it is necessary. The semaphore stuff is vague too. I don't see how you'd be able to get into an endless loop. It might actually be useful to turn node_access_grants_present() into a more generic module.inc function. Maybe something along the lines of module_implements($hook), which would return a list of modules implementing the specified $hook. I suggest you update more core modules. If you do, I'll measure the performance gain against a copy of drupal.org's database. ------------------------------------------------------------------------ January 6, 2005 - 22:47 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql.patch (15.72 KB) The hint stuff is something is pretty useful: hook implementations can fine tune themselves when to fire and when not. Good for features and performance, too. module_implements is done. Semaphore -- we will have tons of node_rewrite_sql calls in the core. Now someone writes a hook_node_rewrite_sql implementation which calls a function which in turn calls another one which calls node_rewrite_sql . I would rather not be the one who debugs such a scenario. Thanks for the offer! forum.module is now patched. Others will follow tomorrow. ------------------------------------------------------------------------ January 6, 2005 - 23:21 : Jose A Reyero I think this looks good enough, so I'm starting to test and use this for i18n. If this get into the node module, I don't think I will need any node.module patch anymore... A pair of performance hints: - As this node_rewrite_sql is going to be called several times for each page, we can cache the list of modules implementing the hook in a static variable. - We can even cache the whole result for each 'hint' value, so the whole hook will only be called once or twice for each page. Some suggested 'hint' values to be defined: NODE_REWRITE_SQL_LIST -> This one for simple generic list of nodes for the main page, could be default. NODE_REWRITE_SQL_ADMIN_LIST -> List of nodes for administration pages NODE_REWRITE_SQL_RELATED_LIST -> List of objects related to nodes, but no nodes themselves -like comments-, node table may have to be joined... NODE_REWRITE_SQL_BLOCK_LIST -> Listing for a block NODE_REWRITE_SQL_NO_LANGUAGE -> No need to add language conditions. This could hapen when you need a listing for other languages, or mixed languages, for translators or for administrators.. I would need something like this last one for i18n, just to have the permissions working and some flexibility with language conditions. And also the one you already have in place ...._SEARCH. All this will allow for options like search, admin, list current/all languages... ------------------------------------------------------------------------ January 7, 2005 - 17:30 : Jose A Reyero Attachment: http://drupal.org/files/issues/central_node_query_node_module.patch (7.07 KB) I found some problems when using the patch. The where conditions are not merged well if there are more than once, and also conditions need some parenthesis around. So I replaced the $where and $join strings in _node_rewrite_sql by arrays which are imploded at the end. Also added some 'hint' definitions. Patches for i18n module are coming next. This works like charm for node listings, comment listings, searches, etc... Btw, I also think 'semaphores' are not really needed. ------------------------------------------------------------------------ January 7, 2005 - 19:04 : Dries I don't see how the hint system can be portable: are contributed modules supposed to define their own hits? ------------------------------------------------------------------------ January 7, 2005 - 22:54 : chx The hints -- I was in a hurry yesterday evening, so I have forgot to define a NODE_REWRITE_SQL_FORUM in the forum.module patch and also forgot to use the hints. So I think core node types will have their hint defined. I have a vague idea about how contrib modules could define their own hint type: we shall have a NODE_REWRITE_SQL_HINT_MAX, and every hint define should like this -- beware this is not working! This is just an idea. Well, if defines does not work, maybe a global node_rewrite_sql_hints array should be introduced...? ------------------------------------------------------------------------ January 8, 2005 - 09:54 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_0.patch (16.69 KB) Here is a list of changes: Complete rethought of hint system. Uses array of strings instead of defines. There are a number of examples. the new comment.module patch shows a good usage example for the third parameter of node_rewrite_sql which is at the moment called node_alias. It's really not a good name, it should be "the alias of the table which holds the nid field for this query". BTW. there should be an index on comment.nid. Incorporated Jose's "replaced the $where and $join strings in _node_rewrite_sql by arrays which are imploded at the end". I was a fool to lose these, the first version (which used array) used a proper implode for WHEREs. ------------------------------------------------------------------------ January 8, 2005 - 11:27 : Dries I have two fundamental problems with it this patch: The loop detection mechanism has to go. The hints are difficult to understand and I'm not convinced they are good/needed. Does the patch contain a good example of a node_rewrite_sql function/hook that uses these hints? I can't find one. I won't like these hints until I'm convinced they are needed and that they represent the best possible solution. Convince me. Could you extend the documentation about the hint-system? Currently it reads "An array of hint strings about the query, passed on to hook_sql handlers.". Clearly, that doesn't help me at all. The documentation should answer the following questions: (1) what are the hints for, (2) when should I use them (or not use them) and (3) what does hints look like (what is their format/syntax)? Note that I can't compare performance unless more queries have been updated. Thanks chx. ------------------------------------------------------------------------ January 8, 2005 - 11:30 : Dries Why does the hints in the comment module include 'related_list'. This is confusing. ------------------------------------------------------------------------ January 8, 2005 - 12:52 : Jose A Reyero Well, it seems we have different views about the 'hints'. At this point, the node_rewrite has two main uses: permissions and language conditions (i18n). I don't know whether they could have any use for permission system, but for i18n, some extra information about the rewritten queries will be needed: - Most of the queries will just select fields in the node table. Here, only a where condition is needed, but language condition may be added or not depending on the kind of query: _LIST would be a list for the main page, so only in this case, decissions can be made depending on the path. For some pages, you may want a list of nodes for all languages and for some others, only current language. _ADMIN_LIST is a list of nodes for administration. An option will be added in i18n for 'administering all languages together' or 'administer each language separately', or even a language field could be added in the search form. So language conditions will be merged diferently _BLOCK_LIST is a list of nodes for blocks. I'm thinking here of a global option like 'show mixed languages in blocks'. Anyway, this is a list for a block and you cannot have any logic depending on path here. _SEARCH, would be only for searching, and in this case you may decide to search 'all languages' or only the current one. This can be a global option or some extra field in search form. When doing text searches, searching all languages together will make a lot of sense, but also restricting search by language. About _RELATED_LIST, this is the case of comments, is a list of objects which are not nodes themselves, but related objects in a different table. Here, the node table itself may have to be joined as it is not yet in the query. When dealing with the translation system, the language conditions will be added or not by the module requesting the query itself, so no further language conditions will be needed. Queries will have to be run anyway through 'node_rewrite_sql' to have permissions in place. That's what the _NO_LANGUAGE hint is about. So this is my proposed use for the 'hint' thing, though maybe the hints themselves could be better defined or elaborated, but this is the main idea. I mean no specific hints for modules but generic ones for 'kind of listing' you are requesting. Maybe for the permission system, it could be also some use, as you could decide you want to show a list of objects to anyone, but deny access to the objects themselves, something like 'subscribe or register if you want to read the article'. So permissions here will apply differently depending on what you are doing. We can think also about more creative use of permissions like 'you can read latest articles' but old ones will be available only for selected people, or the opposite 'only old content available for everybody'. Or even 'you can read articles but not comments'. This kind of permissions would need also some 'hints'. ------------------------------------------------------------------------ January 8, 2005 - 13:17 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_1.patch (26.89 KB) More queries rewritten. Loop detection gone. Will gather my thoughts on hint system later, but I do hope this amount of queries will be enough for a performance measurement. Now only blogapi, blog, book calls node_access_*_sql, all others are done. ------------------------------------------------------------------------ January 8, 2005 - 13:20 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_2.patch (26.88 KB) Forgot to save taxonomy.module before rolling the patch, so a DISTINCT remained :( ------------------------------------------------------------------------ January 8, 2005 - 13:38 : chx I see Jose has written a lot about hints, thanks! "Well, it seems we have different views about the 'hints'." -- "we" probably means Dries vs. "Jose and I" 'cos I agree with Jose, but I see some more uses, namely what I have said before on making a filter on forums. If you want to that, you need to know which queries are coming from the forum.module. What shall we do? Pass on the whole query, and let the filter proccess the query to decide whether now it is the forum block it hunts for? I doubt. Let's give it a hint it's a forum and a block query. Some kind of extensionable hint mechanism is necessary, 'cos what about a contrib module which filters another contrib node, say flexinode? ------------------------------------------------------------------------ January 9, 2005 - 15:05 : Dries Could we drop the hint system if we'd choose not to support the il8n module for now? ------------------------------------------------------------------------ January 9, 2005 - 16:54 : Jose A Reyero :-( Ok, let's try to think positively. Could we at least pass the query along so i18n -or whatever module tries to implement this hook- could support itself? ------------------------------------------------------------------------ January 9, 2005 - 18:32 : chx Well, passing along the query would mean no problems, but I wonder whether a better documented hint system would be better? I had no problems with letting the loop detection system go it was just a safety belt for some distant future possibility. But the hint system is -- in my point -- is needed. So, here are the valid hint strings: node, forum, comment etc.name of the module from the query originates, like forum, comment, flexinode etc. genericthis would be be for generic listings, like node_default_page, forum_get_forums etc. relatednode table is not joined so if you want to use other fields from node table than nid you shall join node table yourself. However, if you have your table which is like JOIN {mytable} ON mt.nid=$nid_alias.nid that's fine. adminsomething meant only for administrators. blocknodes in a block searchsomething to do with a search ------------------------------------------------------------------------ January 9, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_3.patch (28.31 KB) Killes said I did not had -F^f in the last version. ------------------------------------------------------------------------ January 9, 2005 - 22:37 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_4.patch (28.77 KB) Automatic version as discussed and args for Killes. ------------------------------------------------------------------------ January 9, 2005 - 22:55 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_5.patch (28.31 KB) Hm. It seems we have agreed on no automatization. However, arbitrary arguments are kept for Killes. ------------------------------------------------------------------------ January 10, 2005 - 21:02 : Dries The last version of this patch improved the performance of my test site's main page by 257%. That is, my machine can serve 2.57 times as much pages in the same time interval. Out of curiousity, I investigated this some more and it turns out this can be tracked down to the useless DISTINCT's which this patch removes. Unless someone objects to this patch or the approach taken, I'm going to commit this shortly. Please review this patch carefully as it marks an important change. ------------------------------------------------------------------------ January 10, 2005 - 22:39 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_6.patch (44.18 KB) More DISTINCTs cleared, now there are no more node_access_*_sql calls in the core modules. This may be a final version. Or we can find a few more to rewrite. I've searched for {node} selects, dropped those which had nid = %d as I thought those do not need rewrite (I may be wrong) and dropped also those that are rewritten, dropped those that are indexing or cron. Here are the remaining queries, which may need rewriting: ------------------------------------------------------------------------ January 10, 2005 - 23:02 : Chris Johnson This patch seems like both a Good Thing and a Bad Thing to me. The idea of centralizing node queries is a good one, I think. It seems to be the original motivation for this patch. It allows better SQL optmization and hence performance. It puts all the code in one place for better future maintenance and enhancement. On the other hand, constructing complex SQL statements from programmatic logic is full of peril. The task quickly becomes very large and complex. Manipulating the SQL language elements via code means it is harder to see when one has made a syntactic mistake. Most such errors will be fatal. As the SQL construction code becomes more complex with development, the number of paths through it become huge and impossible to exhaustively test. Some day, someone will pass parameters that cause fatal SQL syntax to be generated in a "production" system. It also obscures SQL syntax differences between supported databases. It's more likely to introduce usage that works with MySQL but fails with Postgres. I've long been in favor of unifying and centralizing all of the database access in Drupal, to the greatest extent which does not adversely affect performance. So in this way, I think this patch is a big step in that general direction. But I've also been involved in maintaining a search engine which used this technique to create the SQL statements on the fly. It became extremely complex over time as one, then two, then three, etc. new possibilities were added (e.g. new usage, new search criteria). The more complex it became, the more regression errors we had and the harder to debug it became. It got to the point where making one small addition would result in 3 new bugs. It worked most of the time, and when it worked, it was fast. But it failed fatally in a significant number of ways. I'm not sure whether to vote +3 or -4 on this one. It might be good; it might be terrible. ------------------------------------------------------------------------ January 10, 2005 - 23:19 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_7.patch (44.18 KB) Chris, I understand your concerns, but this method -- unlike the original -- tries to keep things as simple as possible. You can only JOIN tables and add WHERE conditions. We are not creating complex SQL on the fly, I tried that originally, just read comment #1 for the warm welcome by Dries: "I'm not likely to commit this". So do not worry, while I may be a little hotheaded, Dries keeps order with an iron hand. And the actual rewrite mechanism is also very simple. Ps. Two typos corrected in this version. ------------------------------------------------------------------------ January 13, 2005 - 23:20 : Dries I'm going to test and benchmark the latest version of this patch over the weekend. If all goes well, I'll commit this to HEAD as no concrete alternatives have been proposed. (I can't think of a better approach and believe that this patch does a good job balancing different values/requirements.) It would be much appreciated if those who maintain node-level permission modules could take a look at this patch to evaluate its usefulness/performance. If you maintain a module that queries the node table you might want to look at this as well -- after all, you'll have to update your module if this hits core. (Note that the PHPdoc code in the patch is not up-to-date.) ------------------------------------------------------------------------ January 14, 2005 - 13:47 : chx Attachment: http://drupal.org/files/issues/node_rewrite_sql_8.patch (43.97 KB) PHPdoc is upgraded, I hope now it is up to date. -- View: http://drupal.org/node/14731 Edit: http://drupal.org/project/comments/add/14731

1 0

[drupal-devel] [feature] Node type filter on node admin page
by Steven 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node.module Category: feature requests Priority: normal Assigned to: Steven Reported by: Anonymous Updated by: Steven Status: patch Attachment: http://drupal.org/files/issues/node admin.png (30.19 KB) Steven Previous comments: ------------------------------------------------------------------------ August 23, 2004 - 17:09 : Anonymous Attachment: http://drupal.org/files/issues/node_admin_filter.patch (3.8 KB) Here's a patch for the current CVS version which adds the option to filter nodes by type on the node admin page. Having built a few sites recently which make heavy use of flexinodes, the ability to filter out particular node types would have been extremely useful, so I hope this makes it to 4.5.0. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ September 4, 2004 - 00:38 : Bèr Kessels I think we should really come up with a good solution for this screen. In the past, I went looking for such a solution and found that the various MP3 library tools might come to help. They deal with large amounts of metadata, some of wch are important, other are not. I summarise here what the UI could be like: ** The menu-based (tree) approach: Most mp3 collections use this idea. In a tree like structure on the side, you can make selections. The list in the main area will follow this selection. (Itunes: http://www.apple.com/itunes/playlists.html , Juk: http://developer.kde.org/~wheeler/images/juk-2.0/juk-2.png, finf/freeamp: http://www.zinf.org/images/zinf_mymusic_shot.png) In drupal that would mean a menu-based filtering method. I once made a patch for that, but that was rejected: http://drupal.org/node/view/7766 The good thing here, is that it is virtually endless scalable. So in theory, flexinode coud add menu items to show only nodes where foo is bar. | - nodetype | |- blog | |= Flexinode | | |=foo | | | |=bar | |- page ** The multiple selectlist UI: Another moethod is a extended method of what we have now: select items from select boxes and press submit. Above the main area one will find some selection UI where one can in- and exlude sertain criteria. (rhythmbox: http://www.rhythmbox.org/screenshots/Screenshot-Rhythmbox.png, and winamp http://www.winamp.com/player/walkthrough.php) Note that both still show some form of tree navigation. In drupal that would mean multiple selects in three columns (rhytmbox and winamp show only two artist and album) For drupal we need three of thos blocks: one for node-types, one for node-settings (promoted, etc) and one for taxonomy terms. The node settings should be logical too: If a criteria is selected it it true, if not: false. That way we can also get rod of the annoying logic: "view posts that are not promoted" and the likes. In both solutions the "perform batch action" needs to move somewhere else. I would plead for moving it to the bottom of the list. A lot of webmail cleints do it that way. The reason being simple: the steps you take to perform the update are: 1) select criteria (find nodes) 2) checkmark the chages (we need columns of checkboxes) 3) perform the update. Logically the way you go through the UI is left->right, top-> down (not taking into consideration japanese/hebrew/arabs etc) So all in all: i think indeed the UI needs serious work. I am willing to take this one me, bu I do need some consensus first. I am not willing to spend hours on a new UI (like the menu-based approach) to find the patch being rejected.Therefore I like some discussion up fron, rather than the " code first see later" approach. Regards, Ber ------------------------------------------------------------------------ September 5, 2004 - 23:42 : Bèr Kessels Attachment: http://drupal.org/files/issues/new_UI_proposal.png (175.31 KB) Here is a gimped mockup from what option 2) could look like. WE could add a fouth box with users too, but I am not sure how that would go with permissions and the like. I need to do some thinking about selecatibility by users first. ------------------------------------------------------------------------ September 6, 2004 - 00:01 : Bèr Kessels Sorry for the spam: but I forgot to mention that there are a few issues: The "perform action dropdown" should list stuff as: promote, demote, etc. But that list needs to be generated with the selection above in mind. So if you choose " published" the action list should not have "publish these nodes" Also the action list could contain "recategorise" wich would need another screen after the one in the "ugly screendump-gimp-mockup", to be able to choose the categories in wich the selection should be placed. And of course the delete these items with a confimation dialog. (http://drupal.org/node/view/7743#comment-11624) ------------------------------------------------------------------------ September 6, 2004 - 14:32 : Bèr Kessels Attachment: http://drupal.org/files/issues/node_admin.html (29.95 KB) After a long discussion with Steven And gerhard, I designed the page in HTML, to let you all try it in some way. Beware: the html is an ugly mock, It is meant as scetch, not as valid html. Some notes on the page: "View all nodes that are:" contains three differnet possibilities for searching. The first two (above the blank line) are there for simplicity, but they might confuse users, because they need a lot of text. The second idea uses a checkbox to enable the select next to it. The third one uses some ajvascript to improve useablity. We should of course choose one of these three methods. "View all nodes that are classified in:" There are quite some people that have questions with whether or not we should include this selection aat all. Here is why I think it should definately go Because most sites have only one purpose and thus a lot of nodes of one type in one state. For example a personal blog site: 99% of the nodes are blogs. 99% are promoted. Only 1 or 2 are not promoted, not published or of type page. Or take a photo-album-site. All posts will be images, the main difference is the classification (the galleries) I placed them in. "any/all" There must be a way to choose whether the sected options should be an AND or an OR. I tried three different methods, each block has one method. We should choose one. Please give me feedback, for I still have to decide whether or not to make this a contributed module or start lobby-ing to get it in core. ------------------------------------------------------------------------ September 13, 2004 - 11:50 : moshe weitzman looking good, and really useful. a few small comments: - consider upping the number of nodes that appear on this page. it is nearly pointless to use paging on this sort of UI - i like the second idea of an 'enabled' checkbox, without the javascript. i think you can use 2 checkboxes, where the second one is called 'publish', 'promote', and 'has comments' - I prefer the horizontal configuration of the 'Require: all any' radio buttons - don't repeat 'the selected posts' in the choices for the bottom dropdown. perhaps use a title for that form_select() instead. - the last choice in that dropdown is 'reclassify'. i don't see how this screen could support that. instead, use my taxonomy_multi_edit.module from contrib :) i suggest making this a contrib module at first. it can be a tab off of the current page. thats how taxonomy_multi_edit.module appears. ------------------------------------------------------------------------ September 13, 2004 - 13:03 : Bèr Kessels About that reclassify: That was exactly my intention: using the taxonomy_muliti_edit module for that. All I wanted to do is pass a list of nids to your module and open a screen using that module with the previously selected nodes! About the other comments: Yes upping the number of nodes might be a good idea. Anyone any idea what number would still be usefull. 10.000 nodes (on drupal.org) in one list is not a good idea, IMO, so some paging is needed. indeed: repeatinf words in dropdowns is considered "bad UI design" thank you for pointing it out. Regards, Ber ------------------------------------------------------------------------ September 28, 2004 - 13:37 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type.patch (1.96 KB) here is an updated version of this patch, syncing it with CVS HEAD. Also, i did change the functionality somewhat: previous versions of the patch asked the modules what node types were out there (via hook_node_name()). This version asks the node table what types are in there (via SELECT DISTINCT type FROM {node}). I find this more useful, because it's entirely possible that, over time, modules will be removed from the system, yet they still have rows in the node table. This method gives a more accurate view of the types of nodes on the system. The new UI looks excellent, but this patch does not address it. ------------------------------------------------------------------------ September 28, 2004 - 19:05 : drumm -1 I applied and tested and it did not work at all for me. The type options selection only contains 's' and 'W', selecting one does not work. The list of types seems to come from the DB which is probably an unnecessary query. ------------------------------------------------------------------------ September 29, 2004 - 12:51 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type_1.patch (2.31 KB) Here's a version of that patch that should work. Sorry. I think that getting the types from the node table is necessary. The list of installed nodes is not the authority on what types of nodes are in the db; we should ask the db directly. It's a small query on one admin page; the result is that the dropdown has in it the most accurate information available. That's worth it, imo. ------------------------------------------------------------------------ September 29, 2004 - 12:52 : eafarris s/list of installed nodes/list of installed modules/ . sheesh. teach me to post before my tea. ------------------------------------------------------------------------ October 21, 2004 - 18:15 : Bèr Kessels Attachment: http://drupal.org/files/issues/node.html (29.33 KB) I do not know where it weant, but my proposal had a HTML mockup. Attaching it here to see what i meant. ------------------------------------------------------------------------ October 23, 2004 - 13:14 : com2 Nice filter options, but what I really miss it the possibility to select all checkboxes. Really boring job when you have to click the whole list one by one. ------------------------------------------------------------------------ October 24, 2004 - 10:41 : Bèr Kessels I am not a big fan of those options in forms. They more often break things than that thy fix them. Rahter use a bookmarklet r so for that: http://www.squarefree.com/bookmarklets/forms.html Ber ------------------------------------------------------------------------ October 25, 2004 - 01:03 : Steven Attachment: http://drupal.org/files/issues/node_admin_ucd.html (29.83 KB) Based on some more IRC discussion, I came up with this (attachment). It is based on an earlier mockup. It does use Javascript and doesn't degrade without script (yet), but I think it's worth considering. It can be implemented quite cleanly. The idea would be to have a filter like this initially, and after clicking 'Go', to show the current filter as well as the form again, allowing you to refine your search. I haven't tested browser compatibility, but it should work on most browsers. The only ones I know for sure won't work are IE4 and NS4, which are old enough to ignore IMO: after all, this is admin only, and we can put some stricter browser requirements on that. Recent versions of other browsers should work (it only does a simple visibility toggle). I like it because it's not an airplane cockpit, but still offers complex filtering options through refinement. It is also quite similar to the search/filtering UI in e.g. mail applications. The question is of course, how to degrade it for people without Javascript. My suggestion would be to simply show all filter options, and put a radiobutton in front of each: ( ) Where type is [ blog | v ] ( ) Where published status is ( ) published ( ) unpublished ( ) Where category is [ term1 | v ] ... It has more visual clutter, but the same principle of use. ------------------------------------------------------------------------ October 25, 2004 - 01:13 : Steven In fact, I just realized that the radio-button version can be coded to send exactly the same form data as the scripted version, which makes it more interesting. ------------------------------------------------------------------------ October 29, 2004 - 00:52 : Steven If you guys approve of my idea, I'll start coding. Assigning to myself for now. ------------------------------------------------------------------------ November 1, 2004 - 17:35 : Bèr Kessels I like this idea much more than my previous approaches. Gave it some thought and came with the follwing behaviour: * I think we need to discuss or at least design the tabular data too: It makes 0 sense to show the "types" column, if you chose "type = image" * I think we need to introduce a hook to add info to that tabulat view. Images can output thumbnails, for example * In tabular view, we should make: title a textfield, type and author a select and satus a select too. * I think we should introduce another range of options: "where content contains" [ ] and "where title contains" [ ] * I would like to get some feedback on how many drupal-using people are firmly against javascript, with a reason why. I bet that this is < 1% ! Bèr ------------------------------------------------------------------------ November 1, 2004 - 17:42 : Steven We don't have an anti-JS policy in Drupal. It's just that until recently, doing cross browser JS was a chore and making it accessible and degradable would be a disaster. This form can be implemented cleanly and will degrade without problems. Anyone who opposes it because it is JS can bugger off ;). ------------------------------------------------------------------------ November 3, 2004 - 19:24 : com2 There is a more radical way around JS. You could forget the table idea alltogether and present the nodes in Listbox. Then selecting becomes much easier with Ctrl-Click and Shift-Click ------------------------------------------------------------------------ November 4, 2004 - 00:02 : Bèr Kessels Did you have a look at the previous applied UI improvements? Did you consider the pre's and cons of those earlier posts? It seams you are repeating passed issues here, please reply with miore detailed issues here, Bèr ------------------------------------------------------------------------ December 3, 2004 - 10:49 : Steven Attachment: http://drupal.org/files/issues/node-admin.patch (13.1 KB) Here's a patch which implements the JS approach of refining the selection. ------------------------------------------------------------------------ December 3, 2004 - 10:50 : Steven Attachment: http://drupal.org/files/issues/node_admin.png (34.38 KB) And here's a screenshot of how it looks. ------------------------------------------------------------------------ December 3, 2004 - 19:23 : FactoryJoe(a)civicspacelabs.org This is looking better and better. I'm going to do a UI review of this a post any additional thoughts or comments. Chris ------------------------------------------------------------------------ December 3, 2004 - 21:50 : Anonymous Ack, I tested this using the bluebeach theme, but there's a small CSS bug with bluemarine, where the header for the operations form is stuck to the right of the filter form. I'll make an updated patch later. In the meantime, comments are much appreciated. ------------------------------------------------------------------------ December 3, 2004 - 23:15 : killes(a)www.drop.org I actually prefer the non-JS menu over the JS menu. Not (only) because of the JS but because I prefer radiobuttons over pulldown menus. The argument that the non-JS approach uses more vertical space can be mitigated by redirecting to a #results anchor. ------------------------------------------------------------------------ January 15, 2005 - 06:32 : Steven Attachment: http://drupal.org/files/issues/node.admin.patch (14.41 KB) Here's an updated patch... I have exams in a week, so I don't have time to finish up the Javascript version, so I removed it (it can be added later easily). However, the non-JS version is still very usable. Changes since last time: - Uses semantic XHTML (definition list, and yes, they are for more than just definitions, even w3c agrees). - Merges the various node flags (promoted, published, ...) into one "status" dropdown. This keeps the whole selector shorter, and is better UI-wise IMO. - Adds drupal_gotos, they were missing. - Updates node admin watchog messages to properly display the type. The code used t($node->type) which is a big no-no. hook_node_name() is now invoked instead. - Removes some dead code. - I rolled Ber's node mass deletion patch in as it affects the same piece of code. Updating either patch for the other separately would be a waste of time. I also added some more improvements to Bèr's code, such as removing the incorrect help from the confirmation page, cleaning up the code style a bit, not loading every node just to display its title and displaying a single drupal_set_message() after the deleting rather than one per node. The old issue is here: http://drupal.org/node/7743 -- View: http://drupal.org/node/10296 Edit: http://drupal.org/project/comments/add/10296

1 0

[drupal-devel] [feature] Node type filter on node admin page
by Steven 15 Jan '05

15 Jan '05

Project: Drupal Version: cvs Component: node.module Category: feature requests Priority: normal Assigned to: Steven Reported by: Anonymous Updated by: Steven Status: patch Attachment: http://drupal.org/files/issues/node.admin.patch (14.41 KB) Here's an updated patch... I have exams in a week, so I don't have time to finish up the Javascript version, so I removed it (it can be added later easily). However, the non-JS version is still very usable. Changes since last time: - Uses semantic XHTML (definition list, and yes, they are for more than just definitions, even w3c agrees). - Merges the various node flags (promoted, published, ...) into one "status" dropdown. This keeps the whole selector shorter, and is better UI-wise IMO. - Adds drupal_gotos, they were missing. - Updates node admin watchog messages to properly display the type. The code used t($node->type) which is a big no-no. hook_node_name() is now invoked instead. - Removes some dead code. - I rolled Ber's node mass deletion patch in as it affects the same piece of code. Updating either patch for the other separately would be a waste of time. I also added some more improvements to Bèr's code, such as removing the incorrect help from the confirmation page, cleaning up the code style a bit, not loading every node just to display its title and displaying a single drupal_set_message() after the deleting rather than one per node. The old issue is here: http://drupal.org/node/7743 Steven Previous comments: ------------------------------------------------------------------------ August 23, 2004 - 17:09 : Anonymous Attachment: http://drupal.org/files/issues/node_admin_filter.patch (3.8 KB) Here's a patch for the current CVS version which adds the option to filter nodes by type on the node admin page. Having built a few sites recently which make heavy use of flexinodes, the ability to filter out particular node types would have been extremely useful, so I hope this makes it to 4.5.0. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ August 23, 2004 - 21:06 : leafish_dylan This would be a great feature. ------------------------------------------------------------------------ September 4, 2004 - 00:38 : Bèr Kessels I think we should really come up with a good solution for this screen. In the past, I went looking for such a solution and found that the various MP3 library tools might come to help. They deal with large amounts of metadata, some of wch are important, other are not. I summarise here what the UI could be like: ** The menu-based (tree) approach: Most mp3 collections use this idea. In a tree like structure on the side, you can make selections. The list in the main area will follow this selection. (Itunes: http://www.apple.com/itunes/playlists.html , Juk: http://developer.kde.org/~wheeler/images/juk-2.0/juk-2.png, finf/freeamp: http://www.zinf.org/images/zinf_mymusic_shot.png) In drupal that would mean a menu-based filtering method. I once made a patch for that, but that was rejected: http://drupal.org/node/view/7766 The good thing here, is that it is virtually endless scalable. So in theory, flexinode coud add menu items to show only nodes where foo is bar. | - nodetype | |- blog | |= Flexinode | | |=foo | | | |=bar | |- page ** The multiple selectlist UI: Another moethod is a extended method of what we have now: select items from select boxes and press submit. Above the main area one will find some selection UI where one can in- and exlude sertain criteria. (rhythmbox: http://www.rhythmbox.org/screenshots/Screenshot-Rhythmbox.png, and winamp http://www.winamp.com/player/walkthrough.php) Note that both still show some form of tree navigation. In drupal that would mean multiple selects in three columns (rhytmbox and winamp show only two artist and album) For drupal we need three of thos blocks: one for node-types, one for node-settings (promoted, etc) and one for taxonomy terms. The node settings should be logical too: If a criteria is selected it it true, if not: false. That way we can also get rod of the annoying logic: "view posts that are not promoted" and the likes. In both solutions the "perform batch action" needs to move somewhere else. I would plead for moving it to the bottom of the list. A lot of webmail cleints do it that way. The reason being simple: the steps you take to perform the update are: 1) select criteria (find nodes) 2) checkmark the chages (we need columns of checkboxes) 3) perform the update. Logically the way you go through the UI is left->right, top-> down (not taking into consideration japanese/hebrew/arabs etc) So all in all: i think indeed the UI needs serious work. I am willing to take this one me, bu I do need some consensus first. I am not willing to spend hours on a new UI (like the menu-based approach) to find the patch being rejected.Therefore I like some discussion up fron, rather than the " code first see later" approach. Regards, Ber ------------------------------------------------------------------------ September 5, 2004 - 23:42 : Bèr Kessels Attachment: http://drupal.org/files/issues/new_UI_proposal.png (175.31 KB) Here is a gimped mockup from what option 2) could look like. WE could add a fouth box with users too, but I am not sure how that would go with permissions and the like. I need to do some thinking about selecatibility by users first. ------------------------------------------------------------------------ September 6, 2004 - 00:01 : Bèr Kessels Sorry for the spam: but I forgot to mention that there are a few issues: The "perform action dropdown" should list stuff as: promote, demote, etc. But that list needs to be generated with the selection above in mind. So if you choose " published" the action list should not have "publish these nodes" Also the action list could contain "recategorise" wich would need another screen after the one in the "ugly screendump-gimp-mockup", to be able to choose the categories in wich the selection should be placed. And of course the delete these items with a confimation dialog. (http://drupal.org/node/view/7743#comment-11624) ------------------------------------------------------------------------ September 6, 2004 - 14:32 : Bèr Kessels Attachment: http://drupal.org/files/issues/node_admin.html (29.95 KB) After a long discussion with Steven And gerhard, I designed the page in HTML, to let you all try it in some way. Beware: the html is an ugly mock, It is meant as scetch, not as valid html. Some notes on the page: "View all nodes that are:" contains three differnet possibilities for searching. The first two (above the blank line) are there for simplicity, but they might confuse users, because they need a lot of text. The second idea uses a checkbox to enable the select next to it. The third one uses some ajvascript to improve useablity. We should of course choose one of these three methods. "View all nodes that are classified in:" There are quite some people that have questions with whether or not we should include this selection aat all. Here is why I think it should definately go Because most sites have only one purpose and thus a lot of nodes of one type in one state. For example a personal blog site: 99% of the nodes are blogs. 99% are promoted. Only 1 or 2 are not promoted, not published or of type page. Or take a photo-album-site. All posts will be images, the main difference is the classification (the galleries) I placed them in. "any/all" There must be a way to choose whether the sected options should be an AND or an OR. I tried three different methods, each block has one method. We should choose one. Please give me feedback, for I still have to decide whether or not to make this a contributed module or start lobby-ing to get it in core. ------------------------------------------------------------------------ September 13, 2004 - 11:50 : moshe weitzman looking good, and really useful. a few small comments: - consider upping the number of nodes that appear on this page. it is nearly pointless to use paging on this sort of UI - i like the second idea of an 'enabled' checkbox, without the javascript. i think you can use 2 checkboxes, where the second one is called 'publish', 'promote', and 'has comments' - I prefer the horizontal configuration of the 'Require: all any' radio buttons - don't repeat 'the selected posts' in the choices for the bottom dropdown. perhaps use a title for that form_select() instead. - the last choice in that dropdown is 'reclassify'. i don't see how this screen could support that. instead, use my taxonomy_multi_edit.module from contrib :) i suggest making this a contrib module at first. it can be a tab off of the current page. thats how taxonomy_multi_edit.module appears. ------------------------------------------------------------------------ September 13, 2004 - 13:03 : Bèr Kessels About that reclassify: That was exactly my intention: using the taxonomy_muliti_edit module for that. All I wanted to do is pass a list of nids to your module and open a screen using that module with the previously selected nodes! About the other comments: Yes upping the number of nodes might be a good idea. Anyone any idea what number would still be usefull. 10.000 nodes (on drupal.org) in one list is not a good idea, IMO, so some paging is needed. indeed: repeatinf words in dropdowns is considered "bad UI design" thank you for pointing it out. Regards, Ber ------------------------------------------------------------------------ September 28, 2004 - 13:37 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type.patch (1.96 KB) here is an updated version of this patch, syncing it with CVS HEAD. Also, i did change the functionality somewhat: previous versions of the patch asked the modules what node types were out there (via hook_node_name()). This version asks the node table what types are in there (via SELECT DISTINCT type FROM {node}). I find this more useful, because it's entirely possible that, over time, modules will be removed from the system, yet they still have rows in the node table. This method gives a more accurate view of the types of nodes on the system. The new UI looks excellent, but this patch does not address it. ------------------------------------------------------------------------ September 28, 2004 - 19:05 : drumm -1 I applied and tested and it did not work at all for me. The type options selection only contains 's' and 'W', selecting one does not work. The list of types seems to come from the DB which is probably an unnecessary query. ------------------------------------------------------------------------ September 29, 2004 - 12:51 : eafarris Attachment: http://drupal.org/files/issues/admin_node_filter_by_type_1.patch (2.31 KB) Here's a version of that patch that should work. Sorry. I think that getting the types from the node table is necessary. The list of installed nodes is not the authority on what types of nodes are in the db; we should ask the db directly. It's a small query on one admin page; the result is that the dropdown has in it the most accurate information available. That's worth it, imo. ------------------------------------------------------------------------ September 29, 2004 - 12:52 : eafarris s/list of installed nodes/list of installed modules/ . sheesh. teach me to post before my tea. ------------------------------------------------------------------------ October 21, 2004 - 18:15 : Bèr Kessels Attachment: http://drupal.org/files/issues/node.html (29.33 KB) I do not know where it weant, but my proposal had a HTML mockup. Attaching it here to see what i meant. ------------------------------------------------------------------------ October 23, 2004 - 13:14 : com2 Nice filter options, but what I really miss it the possibility to select all checkboxes. Really boring job when you have to click the whole list one by one. ------------------------------------------------------------------------ October 24, 2004 - 10:41 : Bèr Kessels I am not a big fan of those options in forms. They more often break things than that thy fix them. Rahter use a bookmarklet r so for that: http://www.squarefree.com/bookmarklets/forms.html Ber ------------------------------------------------------------------------ October 25, 2004 - 01:03 : Steven Attachment: http://drupal.org/files/issues/node_admin_ucd.html (29.83 KB) Based on some more IRC discussion, I came up with this (attachment). It is based on an earlier mockup. It does use Javascript and doesn't degrade without script (yet), but I think it's worth considering. It can be implemented quite cleanly. The idea would be to have a filter like this initially, and after clicking 'Go', to show the current filter as well as the form again, allowing you to refine your search. I haven't tested browser compatibility, but it should work on most browsers. The only ones I know for sure won't work are IE4 and NS4, which are old enough to ignore IMO: after all, this is admin only, and we can put some stricter browser requirements on that. Recent versions of other browsers should work (it only does a simple visibility toggle). I like it because it's not an airplane cockpit, but still offers complex filtering options through refinement. It is also quite similar to the search/filtering UI in e.g. mail applications. The question is of course, how to degrade it for people without Javascript. My suggestion would be to simply show all filter options, and put a radiobutton in front of each: ( ) Where type is [ blog | v ] ( ) Where published status is ( ) published ( ) unpublished ( ) Where category is [ term1 | v ] ... It has more visual clutter, but the same principle of use. ------------------------------------------------------------------------ October 25, 2004 - 01:13 : Steven In fact, I just realized that the radio-button version can be coded to send exactly the same form data as the scripted version, which makes it more interesting. ------------------------------------------------------------------------ October 29, 2004 - 00:52 : Steven If you guys approve of my idea, I'll start coding. Assigning to myself for now. ------------------------------------------------------------------------ November 1, 2004 - 17:35 : Bèr Kessels I like this idea much more than my previous approaches. Gave it some thought and came with the follwing behaviour: * I think we need to discuss or at least design the tabular data too: It makes 0 sense to show the "types" column, if you chose "type = image" * I think we need to introduce a hook to add info to that tabulat view. Images can output thumbnails, for example * In tabular view, we should make: title a textfield, type and author a select and satus a select too. * I think we should introduce another range of options: "where content contains" [ ] and "where title contains" [ ] * I would like to get some feedback on how many drupal-using people are firmly against javascript, with a reason why. I bet that this is < 1% ! Bèr ------------------------------------------------------------------------ November 1, 2004 - 17:42 : Steven We don't have an anti-JS policy in Drupal. It's just that until recently, doing cross browser JS was a chore and making it accessible and degradable would be a disaster. This form can be implemented cleanly and will degrade without problems. Anyone who opposes it because it is JS can bugger off ;). ------------------------------------------------------------------------ November 3, 2004 - 19:24 : com2 There is a more radical way around JS. You could forget the table idea alltogether and present the nodes in Listbox. Then selecting becomes much easier with Ctrl-Click and Shift-Click ------------------------------------------------------------------------ November 4, 2004 - 00:02 : Bèr Kessels Did you have a look at the previous applied UI improvements? Did you consider the pre's and cons of those earlier posts? It seams you are repeating passed issues here, please reply with miore detailed issues here, Bèr ------------------------------------------------------------------------ December 3, 2004 - 10:49 : Steven Attachment: http://drupal.org/files/issues/node-admin.patch (13.1 KB) Here's a patch which implements the JS approach of refining the selection. ------------------------------------------------------------------------ December 3, 2004 - 10:50 : Steven Attachment: http://drupal.org/files/issues/node_admin.png (34.38 KB) And here's a screenshot of how it looks. ------------------------------------------------------------------------ December 3, 2004 - 19:23 : FactoryJoe(a)civicspacelabs.org This is looking better and better. I'm going to do a UI review of this a post any additional thoughts or comments. Chris ------------------------------------------------------------------------ December 3, 2004 - 21:50 : Anonymous Ack, I tested this using the bluebeach theme, but there's a small CSS bug with bluemarine, where the header for the operations form is stuck to the right of the filter form. I'll make an updated patch later. In the meantime, comments are much appreciated. ------------------------------------------------------------------------ December 3, 2004 - 23:15 : killes(a)www.drop.org I actually prefer the non-JS menu over the JS menu. Not (only) because of the JS but because I prefer radiobuttons over pulldown menus. The argument that the non-JS approach uses more vertical space can be mitigated by redirecting to a #results anchor. -- View: http://drupal.org/node/10296 Edit: http://drupal.org/project/comments/add/10296

1 0

Jump to page: