- Development - lists.drupal.org

[drupal-devel] no 4.6 for Drupal Docs?
by Anisa 25 Apr '05

25 Apr '05

Feel free to yell at me for not searching, but this seemed like an odd omission. Head has already opened, right? Anisa.

1 0

[drupal-devel] [bug] prefix.sh need fixes with postgresql
by patryk 25 Apr '05

25 Apr '05

Issue status update for http://drupal.org/node/21246 Project: Drupal Version: 4.6.0 Component: other Category: bug reports Priority: normal Assigned to: patryk Reported by: patryk Updated by: patryk Status: patch Attachment: http://drupal.org/files/issues/drupal-cvs-prefix-pgsql-fix.diff (557 bytes) prefix.sh does not work correctly with PostgreSQL database scheme. patryk

2 2

Re: [drupal-devel] DrupalForge?
by puregin 25 Apr '05

25 Apr '05

On 25 Apr 2005, at 12:53 AM, Djun Kim wrote: > > On 25 Apr 2005, at 12:29 AM, Jeremy Epstein wrote: >> > >> mamboserver.com -> apple.com >> drupal.org -> lego.com >> >> Take a look at the pairs of sites side-by-side, and you'll see what I >> mean. The similarities are mainly in the choice of colour. Maybe the >> Drupal community should look into making little plastic toys, just as >> a fundraiser on the side? :-). Clean, modular, accessible, semantic, >> usable, and [free... err, I mean...] reasonably priced plastic toys, >> of course. > > Lego is almost as famous for its lawyering as for its plastic. I > think > we should be careful ;) > > -- > puregin >

1 0

[drupal-devel] [feature] Improve search link ranking
by Steven 25 Apr '05

25 Apr '05

Issue status update for http://drupal.org/node/21152 Project: Drupal Version: cvs Component: search.module Category: feature requests Priority: normal Assigned to: Steven Reported by: Steven Updated by: Steven Status: patch Attachment: http://drupal.org/files/issues/search.link.ranking.patch (2.87 KB) Existing system Search.module currently tracks links to nodes, and indexes the words in the link's text as part of the target node. Thus, if someone links to the magic quotes FAQ with <a>more info about magic quotes</a> then the target node will get a higher ranking for a query containing any of the words "more info about magic quotes". However, there are two problems with this: 1) Most people don't provide a good caption for their link ("click here" or even just the URL). 2) Sometimes there are a bunch of names for a particular subject and the user used a less common name (e.g. teaser, excerpt, summary). This means that the link hint of "this page is important" often does not come into play simply because the search query doesn't match any linked keywords. The search query goes like this: SELECT i.sid, i.type, SUM(i.score / t.total) AS score FROM search_index i INNER JOIN search_total t ON i.word = t.word WHERE (...keyword matching...) GROUP BY i.sid, i.type ORDER BY score DESC Thus it selects all matching keywords for the query, groups them by source (a sid/type pair, e.g. "node" + nid) and sums the (relative) scores per source. For example when searching for "Drupal 4.6.0" one might get: #wordsidfromsidtypescoretotalsum (per sid/type)#1drupal1NULLnode6106/10 + 2/5 + 1/10 = 1.1#24601NULLnode25#3drupal13node110#4drupal2NULLnode3102/10 = 0.2 #3 is a link from node 3 to node 1 with the word "drupal". The other rows represent words that appeared literally in the result node. Note that these are the rows before grouping and summation. MySQL will only return two rows, where the first row combines #1,#2,#3 (node 1 with score 1.1) and the second row is only for #4 (node 2 with score 0.2). Changes This patch improves the results by assigning a de-facto score boost to nodes which are linked to. Only the node itself still has to match one of the given keywords, not the link texts. The node receives a boost proportional to how many times it has been linked to. It fits well within the existing search.module idea of trying to ensure the top results are relevant rather than focusing on making all results relevant. It does not obsolete the old mechanism, but complements it. Implementation-wise, this is a bit tricky. Issues The new, generic link boost should however only be applied once per item, not once per keyword. And if an item does not match any keywords, we don't want it to show up in the results (even if it has links pointing to it). There are two approaches to this: - Store the link boost among the regular words with an empty word as a special marker. When selecting the results, we assume the empty word is implicitly part of the search query, but we need to only keep those sid/type pairs that have at least one non-empty word in them. The only way I can think of is to require that each indexed item has a matching row (with a 0 score if there are no links). Then we can do a COUNT(*) per item and require that each sid/type pair has at least two matching rows (one link boost + one real word). This would of course be quite wasteful as there are nowhere near as much links as there are nodes. SELECT i.sid, i.type, COUNT(*) AS count, SUM(i.score / t.total) AS score FROM search_index i INNER JOIN search_total t ON i.word = t.word WHERE (i.word = '') OR (...keyword matching...) GROUP BY i.sid, i.type ORDER BY score DESC HAVING count > 1 - Store the link boost in a separate table to join on. As far as I know it's not possible to do one join per GROUP BY item, so I'd have to use a MAX() statement to reduce the link boost which is fetched per keyword to one score per sid/type pair. If we don't want an entry in search_links per item, then we have to use an if() statement to see if the join succeeded (0.02 is the score boost per link): SELECT i.type, i.sid, (SUM(i.score/t.count) + MAX(if(j.sid IS NULL, 0, j.score))*0.02) AS score FROM search_index i INNER JOIN search_total t ON i.word = t.word LEFT JOIN search_links j ON i.sid = j.sid AND i.type = j.type WHERE (...keyword matching..) GROUP BY i.type, i.sid ORDER BY score DESC In this patch I implemented a hybrid option: the link boosts are stored in the search_index table with an empty word, like in option 1. But like option 2, we use a join (on search_index itself) to fetch the boost and we don't store link counts for items without links. SELECT i.type, i.sid, (SUM(i.score/t.count) + MAX(if(j.word IS NULL, 0, j.score))*0.02) AS score FROM search_index i INNER JOIN search_total t ON i.word = t.word LEFT JOIN search_index j ON i.sid = j.sid AND j.word = '' WHERE (...keyword matching...) GROUP BY i.type, i.sid ORDER BY score DESC The reason I used the same table is because the links table would otherwise simply have the same DB structure as the search_index table. The word column is keyed, so restricting to j.word = "" should be fast. While this patch works, I'm not too sure about the implementation. I'm not an SQL guru, so feedback is certainly needed on all aspects. Results I can't really try it out on Drupal.org because it requires re-indexing, but I do have some clues that it would provide an improvement: of all the linked keywords in the search index now, 1977 are for the word "http", which almost certainly means a literal URL. Thus these records are almost never used, unless someone actually searched for "http", even though they tell us something useful (the linked nodes were interesting). Steven

2 1

[drupal-devel] [task] Consistent return 'passive voice' in form_set_error() and drupal_set_message()
by puregin 25 Apr '05

25 Apr '05

Issue status update for http://drupal.org/node/15595 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: stefan nagtegaal Reported by: bertboerland(a)www.drop.org Updated by: puregin Status: patch Attachment: http://drupal.org/files/issues/drupal_set_message_new.patch (50.98 KB) The attached patch updates Stefan's earlier patch with a few corrections and tweaks to messages. The bulk of these are minor typos, corrections to noun/verb number agreement, consistent use of 'could not' vs. 'can not', etc. In includes/locale.inc, I have made the tone of messages slightly more formal by replacing '... file is broken, because...' in messages with '... file contains an error: ...'. While visiting the affected files, I also made minor fixes or improvements to other messages. I have re-written portions of the help text for locale.module. Please review and comment! :) puregin Previous comments: ------------------------------------------------------------------------ January 14, 2005 - 05:59 : bertboerland(a)www.drop.org *My* drupal not *Yours*! I think that when one addresses the user or the administrator of a site, it is best to do this outside in instead of inside out. So when in the documentation the coder is referring to the site of the administrator, this site should be called "your site" (/inside out/) but "my site" (/outside in/). There are numours examples where the text in referring to "your"where "my"would be apopriate [1] There used to be some "your" in the userinterface for the enduser as well, but that has been solved in 4.5. Before that you had "my account", but "track your postings". This has now all beens replaced by "my"and "mine". If everybody agrees, lets also corect this for the administrator as well; my administrator, not yours ;-) [1] examples: admin/logs/referrers "...that point to your web site." admin/settings "General configuration options for your site" ------------------------------------------------------------------------ April 19, 2005 - 08:36 : stefan nagtegaal We should consistently use the 'passive voice', when returning messages using form_set_error() and drupal_set_message(). ------------------------------------------------------------------------ April 22, 2005 - 04:05 : stefan nagtegaal Attachment: http://drupal.org/files/issues/drupal_set_message consistency part 1 of 2 (51.16 KB) first patch of 2 to make the drupal_set_message(), form_set_error() and watchdog() messages more consistent. please review/comment and apply.. ------------------------------------------------------------------------ April 22, 2005 - 04:05 : stefan nagtegaal Setting status to Patch. ------------------------------------------------------------------------ April 23, 2005 - 00:49 : puregin A big -1 from me on replacing "you" with "me" :) Regarding the use of the first person - I find it extremely awkward and confusing to see usage such as 'My Account', "Here are my recent posts", etc. From my perspective as a native speaker of English, I find that this kind of usage violates conventional "conversational context". To clarify, if I am communicating with another person *via* the medium of a program then I expect that person to follow the normal conventions of conversation, in particular, to say "me, my, I" when referring to him/herself, and "you, your, etc." when referring to me. If I am communicating with a program, I *also* expect it to follow conversational conventions. Particularly obsequious programs may always refer to themselves in an indirect second person: "This unworthy program was unable to execute your most excellent command, and will henceforth terminate itself" I have to say I really don't like this kind of interaction. I like a program that tells me straight up "I can't find my configuration files! Sorry, I'm giving up!" People like, understand, and are extremely good at direct conversational interaction. It's intuitively clear who is speaking and what the relation to the listener is. I might tolerate a page with title "My Account", but really, a message such as 'This page shows my site-wide referrer statistics. I can optionally view just the "external referrers" or the "internal referrers". Referrers are web pages, both local and on other sites, that point to your my site.' is just plain confusing and unclear. To quote Sendmail: "I refuse to talk to myself" Please don't inflict this gratuitous use of first person on us! Djun ------------------------------------------------------------------------ April 23, 2005 - 05:28 : rivena Have to agree with Djun. You're going to run into a lot of problems changing all the you's to me's. It'd be creepy if my drupal install started talking about itself. Anisa. ------------------------------------------------------------------------ April 24, 2005 - 04:57 : Dries I checked a couple other sites like Amazon.com, Apple.com, NYT and they all use 'your'. I suggest we use 'your' and that we document this guideline. The exception is Yahoo!, who seems to use 'My Yahoo!'. ------------------------------------------------------------------------ April 24, 2005 - 10:30 : Bèr Kessels AFAIK its as follows: A site that offers services, will use " yours". Community sites will therefore use " your Profile". A site that is used by a single user, should use "my". Its My site, if I own that blog. But if I am but a part from a large community, they offer "your blog". Ber

1 0

[drupal-devel] [bug] Wrong GROUP BY statement crashes PostgreSQL
by patryk 25 Apr '05

25 Apr '05

Issue status update for http://drupal.org/node/21249 Project: Drupal Version: 4.6.0 Component: locale.module Category: bug reports Priority: critical Assigned to: patryk Reported by: patryk Updated by: patryk Status: patch Attachment: http://drupal.org/files/issues/drupal-cvs-locale_inc-pgsql-fix.diff (848 bytes) Wrong GROUP BY statement crashes PostgreSQL in locale.inc when exporting POT. patryk

1 0

[drupal-devel] Login problems
by Maykel Moya 25 Apr '05

25 Apr '05

I can authenticate on my drupal as admin (uid = 1) from almost every url. But, after authenticated, I can see the list of roles/permissions on admin/access. If I update something, after 'Save' drupal says Access denied. Then I cannot relogin while the path is still admin/access. In the log what says is access denied to path/access to Anonymous. Is like the Drupal. Regards, maykel

1 0

[drupal-devel] Security in Drupal vs. Wordpress, Postnuke, Mambo
by Boris Mann 25 Apr '05

25 Apr '05

Along with the new security contact form, etc., this blog post is interesting: http://sibowo.blogspot.com/2005/04/drupal-vs-wordpress-vs-postnuke- vs.html I can off course interpret the graphs, but I don't speak the language. Looks like Secunia generated those graphs automatically. -- Boris Mann http://www.bryght.com Vancouver 778-896-2747 / San Francisco 415-367-3595 IM boris_mann(a)jabber.org / SKYPE borismann

3 2

[drupal-devel] [bug] Put revisions in their own table
by killes 24 Apr '05

24 Apr '05

Issue status update for http://drupal.org/node/7582 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: killes(a)www.drop.org Status: patch Attachment: http://drupal.org/files/issues/revisions_25.patch (60.38 KB) Updated to cvs. Dries: Based on some remarks in #drupal this is the last update I am going to do. Apply it or won't fix it. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ May 5, 2004 - 17:25 : killes(a)www.drop.org Currently all node revisions are stored in a serialized field in node.table and retrieved for _each_ page view although they are rarely needed. However, we have agreed that serializing data is bad and that we should try to keep the memory foot print pf Drupal small. Therefore I propose to create a separate revisions table which would be in principle identical to the node table, only that it could have several old copies of the same node. Extra data added by other modules could be added in a serialized field unless we find a better solution. ------------------------------------------------------------------------ May 5, 2004 - 18:06 : jhriggs I too think the serialized approach is less than desirable, but here's an alternative. This would likely take some considerable rework in core and contrib, but the following is how we handle similar types of situations in our databases at work. It is more elegant that a separate table, and avoids the (almost exact) duplication of a table. Instead of separate tables, keep all revisions of nodes in the node table as follows: * add field: active (0/1 or Y/N) * add field: revision * every revision of a node is stored in the node table; however, only one revision can be active at any given time * nid can no longer be unique -- primary/unique key becomes (nid, active) * any time a node is loaded, updated (without revision), etc., the active version is used. Thoughts? ------------------------------------------------------------------------ May 5, 2004 - 18:57 : killes(a)www.drop.org I am not opposed to your scheme, but I want to stress the following: * Duplicating a table's structure is not bad (IMHO) as long as the content is different. * having two tables will allow us to have a rather small node table. This is (maybe) a performance gain. ------------------------------------------------------------------------ May 5, 2004 - 19:37 : jhriggs I don't necessarily think that duplicating a table's structure is _bad_. It just seems to be wasteful and a pain to maintain. (Every change to the node table is made twice...easy to do, but also easy to miss perhaps.) As for performance, as long as nid and the active indicator are indexed, there shouldn't be any performance loss. Also, archiving an old version when making a new revision will be much simpler: just change the active indicator rather than copying an entire node to another table (and ensuring everything gets copied...again a potential maintainance issue). To be honest, I would just like to see the serialized data go away, regarless of what approach is taken. ------------------------------------------------------------------------ July 30, 2004 - 20:49 : Nick Nassar Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_07-30-2004.p… (10.47 KB) I'm interested in using Drupal for a large scale wiki-type project. In order to do this, I need revisions to be in their own table. Attached is a patch to do just that. Most of the changes are pretty self explanatory. Spreading out node data across two tables meant that I had to add database functions to do locking/transactions. Without this, race conditions in which the database becomes corrupted are possible. ------------------------------------------------------------------------ July 30, 2004 - 20:54 : Nick Nassar Oh yeah... The patch is a diff against Drupal CVS ------------------------------------------------------------------------ July 31, 2004 - 01:00 : Anonymous Gerhard speaking. Nick, thanks a lot for your nice patch! It saves me a great deal of labour. I looked through it and immediately liked it. You not only put the old revisions into a new table but also the current one. Do you have an estimate how much more expensive the additional join is? Besides a few minor coding style issues I found a major one: Just a few hours before you uploaded your patch JonBob's node access patch hit core. That means your patch won't apply anymore as all the queries you change have been changed. Can I bug you to update your patch? ------------------------------------------------------------------------ July 31, 2004 - 02:11 : Anonymous Also I think that your upgrade path loses existing revisions. ------------------------------------------------------------------------ July 31, 2004 - 03:39 : drumm I think this is the proper way to do things. No columns are duplicated, there is no serialized data, and only the fields that are logically revised are stored. Nothing jumped out at me as a way to have my node modules be able to keep a table of revisions of additional fields. I'm guessing this could be done within the confines of _insert and _update. Assuming the upgrade path works and modules can extend it I give it a +1. ------------------------------------------------------------------------ July 31, 2004 - 15:40 : Nick Nassar It figures that just as I finish a big patch, another patch comes along and breaks it. Oh well, it should be a pretty easy to fix. I'll work on it. Fixing the upgrade path to keep revisions should be fairly painless. I found another issue that needs to be fixed before this patch gets merged. There format of a node needs to be stored for each revision. Otherwise, for modules that store a format for the nodes, such as page and book, if you write one revision in PHP and the next in HTML, the PHP revision will be displayed as HTML. This is part of a larger issue of how node modules should store revisions of additional fields. I think each module that wants to do this should create another table with (nid, revid) as the primary key. Just as when they want to add fields to a node they create another table with nid as the primary key. As far as performance goes, for sites that make heavy use of revisions, an extra join on primary keys is going to be a lot faster than grabbing all of the revisions from that database everytime. We would need to run benchmarks to determine is the overall difference in speed is for an average site is a gain or a loss. I'm guessing it's very minor either way. ------------------------------------------------------------------------ August 23, 2004 - 14:55 : Nick Nassar Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_08-23-2004.p… (10.92 KB) Here's an updated patch against CVS that puts revisions in their own table, provides an upgrade path, and fixes the format related bugs in the last patch. Hopefully, this can make it into CVS as soon as the freeze is over. ------------------------------------------------------------------------ August 23, 2004 - 15:10 : moshe weitzman Interesting patch ... drumm's question is still outstanding. how do modules store revisions of their fields? Are they expected to manage this on their own? Thats not how it works today. As an aside, i am seeing profile_ fields in my node.revisions column. One could argue that those need not be saved. They pertain to the node author, not to the node itself. ------------------------------------------------------------------------ August 23, 2004 - 17:14 : Nick Nassar Having modules be responsible for storing revisions of their own fields is a side-effect of storing revision data in tables. There's really no way around it. However, revisions generally don't make sense for node types that don't have PHP/HTML content, such as polls. I think it's going to be a pretty rare scenario for a new node type to want another field to change per-revision, so it's a pretty good trade-off. Storing fields that shouldn't be part of revisions, such as the profile_ fields, is a side-effect of storing revisions as serialized objects. Applying this patch will free up that wasted space. :) ------------------------------------------------------------------------ August 23, 2004 - 18:20 : Anonymous There should be a hook that let's the module choose whether it supports history. This way a module author can prevent the user from doing something that may break his module or just cause undefined behavior. If the module doesn't support history then don't let the user/admin choose to add history to nodes of that type. Craig ------------------------------------------------------------------------ August 23, 2004 - 20:23 : Nick Nassar I agree, there should be an API change to make specifying support for revisions easier. In the interests of keeping patches small and keeping to one change per patch, I think the API change should be a separate issue. A sort of ad-hoc API to decide whether or not a module supports revisions by default already exists. Instead of having a hook, modules set the default value of the "Create new revision" field in the edit form. The admin can change this option in admin/node/configure/defaults. This patch doesn't change that. Revisions are broken for node types that have their own database structure, like polls, even when storing them as serialized objects. This patch doesn't change that, either. ------------------------------------------------------------------------ October 26, 2004 - 03:35 : moshe weitzman I'm guessing that someone is going to have to demonstrate that this patch performs as well as current drupal before it gets comitted. i think this patch is a few benchmarks from being comitted. ------------------------------------------------------------------------ October 27, 2004 - 02:04 : Nick Nassar Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004.p… (11 KB) I ran some really unscientific benchmarks, and it looks like this patch has a negligible affect on performance. I used apache bench and the database from theregular.org, which doesn't contain any revisions (worst case scenario for this patch) and contains several hundred nodes. Both the patched and unpatched versions hovered between 2.36 and 2.38 requests per second. The command I used was: ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2' http://192.168.0.100/ An updated patch that should apply to CVS is attached. ------------------------------------------------------------------------ October 27, 2004 - 02:05 : Nick Nassar I ran some really unscientific benchmarks, and it looks like this patch has a negligible affect on performance. I used apache bench and the database from theregular.org, which doesn't contain any revisions (worst case scenario for this patch) and contains several hundred nodes. Both the patched and unpatched versions hovered between 2.36 and 2.38 requests per second. The command I used was: ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2' http://192.168.0.100/ An updated patch that should apply to CVS is attached. ------------------------------------------------------------------------ October 27, 2004 - 02:05 : Nick Nassar Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004.p… (11 KB) I ran some really unscientific benchmarks, and it looks like this patch has a negligible affect on performance. I used apache bench and the database from theregular.org, which doesn't contain any revisions (worst case scenario for this patch) and contains several hundred nodes. Both the patched and unpatched versions hovered between 2.36 and 2.38 requests per second. The command I used was: ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2' http://192.168.0.100/ An updated patch that should apply to CVS is attached. ------------------------------------------------------------------------ November 15, 2004 - 06:05 : elias1884 please overthink the revision system default workflow as well. don't look at the revision system as an isolated system but as a part of the whole workflow system! if you combine revisions with the moderation queue the most logic default workflow would be like that: auth user creates node (revision #0) admin approves the node (status = 1, moderation = 0) => node publicly available auth user finds typo and changes node (revision #1, status = 0, moderation = 1) ------- what happens at that point at the moment is, that the node is not accessible anymore at all until the new revision is approved by admin. of course the new revision should not go online until reviewed and approved, this is absolutely correct, but there is no reason to not take the old revision offline, since it was already approved and should therefore be online until the new revision is approved. it is not practical if a node disappears only because the author corrected a typo. ------- admin approves the node (status = 1, moderation = 0) eventhough I first thought a plain boolean active field would not be capable of providing that functionality if finally came to the conclusion, that it can. The only thing to do is to not set that bit, when a new revision is created, but when it is approved (in case moderation is activated under default workflow). Every revision should have its own moderation, status and active field and on approval they are set like this (status=1, moderation=0, active=Y). When you wanna rollback to an old revision, you can chose between all revisions that already have the moderation bit set back to 0 again and the published to 1. There should be an extra permission for rollback! another concern that I have about the default workflow is, that users can't see the content, they have just created, when moderation is enabled. Eventhough, there is a big fat "submission accepted" presented after submissions, unexperienced users tend to question the information those stupid tincans give them, if they can't find their content afterwards. Many users are really lazy bastards and they don't even read the status messages. The best feedback about whether his story was submitted successfully or not of course is, if he can find the story somewhere on the site, maybe with a status message on top of it, mentioning, that the content is currently not publicly available since it has not been approved yet. there should be a my content section under my account, like somebody is trying to do with the workspace module I guess. so my suggestion is to make (status=0, moderation=1) still available for the creator under a my content section somewhere! ------------------------------------------------------------------------ November 24, 2004 - 05:21 : Nick Nassar I agree. The current workflow for moderation queues and revisions needs to change, but this patch isn't the place for it. The patch is already too big, and it only does the backend stuff. Instead of adding more to this patch and making it take even longer to get into core, would you mind creating a new issue for your UI suggestions, so the those changes can be added as a separate patch? Thanks, Nick ------------------------------------------------------------------------ December 11, 2004 - 13:26 : Dries This patch is _much_ needed so I'd love to see someone revive it. In order for this patch to be accepted, the following needs to be done: Update this patch to CVS HEAD. Rename revid to vid. Rename node_rev to node_revisions. Rename node_rev.changed to node_revisions.timestamp. Rename $rnode to $revision. Fix the coding style to match Drupal's: proper spacing, single quotes where possible, proper variable names. Benchmark this patch with a large database with enough revisions. I'd be happy to benchmark this on my local copy of the drupal.org database. The book.log field should probably move to the node_revisions table. This can be done in a separate patch. Investigate whether transactions are well-supported. ------------------------------------------------------------------------ December 13, 2004 - 01:25 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004-r… (11.02 KB) I've worked a bit on the patch (coding style issues as mentioned by Dries). One thing I noticed is that the patch uses REPLACE. IIRC this needs to be chagned to "UPDATE, if fail INSERT" for pgsql compatibility. Nick, are you still interested in working on that patch? I'd like to know how it works on your site and work on getting it into core. ------------------------------------------------------------------------ December 13, 2004 - 13:33 : Dries Gerhard: your patch does not apply. ------------------------------------------------------------------------ December 13, 2004 - 18:10 : killes(a)www.drop.org Yes, I know, that was the same version as I mailed to you earlier. ------------------------------------------------------------------------ December 13, 2004 - 22:02 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions.patch (52.96 KB) Ok, upüdated the patch to cvs. ------------------------------------------------------------------------ December 14, 2004 - 09:58 : Dries Some more comments: db_begin_transaction() and db_end_transaction() do not belong in database.inc, but in database.mysql.inc and database.pgsql.inc respectively. The node module calls node_revisionsision_list() which is not defined. (Fxed that on my local copy.) Do db_begin_transaction() and db_end_transaction() deprecate Jeremy's table locking patch? The upgrade path assigns the wrong user ID to each revision. The upgrade path assigns the wrong date to each revision (that or a node's revision page shows the wrong usernames/dates). The coding style needs a bit of work, but we can worry about that later. ------------------------------------------------------------------------ December 14, 2004 - 18:34 : Nick Nassar If you need any help getting those things fixed, just let me know. ------------------------------------------------------------------------ December 14, 2004 - 18:50 : Nick Nassar How this relates to Jeremy's node locking patch: There was lots of discussion, and node locking was decided against because from an end user point of view you never want a node to be locked. He's now advocating for a much simpler patch that warns users if their changes will overwrite someone elses. That patch still has a race condition, which might be fixed using db_begin_transaction(). http://drupal.org/node/6025 ------------------------------------------------------------------------ December 14, 2004 - 23:26 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_0.patch (55.96 KB) Here is an updated patch that tries to address Dries concerns. ------------------------------------------------------------------------ December 15, 2004 - 09:32 : Dries Attachment: http://drupal.org/files/issues/revisions-bug.png (76.06 KB) It didn't fix the aforementioned bugs. See attached screenshot. ------------------------------------------------------------------------ January 6, 2005 - 21:15 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_1.patch (51.77 KB) Ok, here is a new version. Dries and myself worked hart at it, so please have a look. what is still missing - database upgrades for the core modules with an own table - contrib modules need an upgrade too. - do we need nid and vid in both the node and the node_revisions table? - the amount of sql queries means a good stress testing for large databases. ------------------------------------------------------------------------ January 19, 2005 - 22:43 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_2.patch (49.49 KB) Here is an updated patch. We discussed to keep the current title in node module and also in the revisiosn table. This is content duplication but will save many joins as many queries only need the title of a node. Discussion is welcome. ------------------------------------------------------------------------ January 20, 2005 - 00:33 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_3.patch (29.93 KB) I've implemented the aforementioned solution. This makes the patch much smaller. The patch now also removes taxonomy_node_has_term() which wasn't used anywhere. I'd really apprciate if some people could test drive the patch. It will be another huge improvement for 4.6. ------------------------------------------------------------------------ January 20, 2005 - 01:05 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_4.patch (30 KB) Another revision. Steven didn't like my literal $node->vid in queries. ------------------------------------------------------------------------ January 20, 2005 - 02:10 : killes(a)www.drop.org - database upgrades for the core modules with an own table - contrib modules need an upgrade too. - do we need nid and vid in both the node and the node_revisions table? - the amount of sql queries means a good stress testing for large databases. These issues are still open, btw. Especially the first one needs to be tackled. ------------------------------------------------------------------------ January 25, 2005 - 21:11 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_5.patch (51.13 KB) Here is a patch that has the database tables updated for forum, book, and page module. ------------------------------------------------------------------------ January 29, 2005 - 23:55 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_6.patch (49.18 KB) Yet another update to keep it working with head. The patch now also removes the table definitons for the page table. ------------------------------------------------------------------------ January 29, 2005 - 23:57 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_7.patch (55.69 KB) Sorry, that was the old version, this is the right one. ------------------------------------------------------------------------ January 31, 2005 - 20:55 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_8.patch (55.71 KB) Updated once more. ------------------------------------------------------------------------ January 31, 2005 - 21:52 : Dries Anyone to help review/test this? ------------------------------------------------------------------------ January 31, 2005 - 22:22 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_9.patch (49.29 KB) Updated again, the update functions occurred twice. Thanks Bart. ------------------------------------------------------------------------ February 2, 2005 - 01:27 : killes(a)www.drop.org Don't know if the db I am using is corrupted or what. I still do have some didficulties. The latest patch is attached. ------------------------------------------------------------------------ February 2, 2005 - 01:27 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_10.patch (49.67 KB) I am probably slowly going mad ... ------------------------------------------------------------------------ February 2, 2005 - 02:54 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_11.patch (48.95 KB) The update issue still needs investigating. This patch is updated for cvs. ------------------------------------------------------------------------ February 2, 2005 - 21:20 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_12.patch (49.83 KB) Ok, here is a new version. I've solved my troubles with book.module. There are still some issues with forum module. Possibly due to inconsistent database. ------------------------------------------------------------------------ February 2, 2005 - 22:31 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_13.patch (49.83 KB) Turns out the drupal.org database had indeed some quirks. Please run this query in your oldest db and tell me the result: select nid,type from node where type like '%/%'; If you get a non-zero result we might need to add another security update. The patch could use still more testing, though. ------------------------------------------------------------------------ February 3, 2005 - 02:16 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_14.patch (49 KB) Ok, we are getting somewhere. At a first glance the update is working. There is a problem remaining: the revisions tab will be shown whether the node has revisions or not. Not sure we can/need to fix this. People with a drupal.org account can log in at http://killes.drupaldevs.org/revision/ and poke around. Your permissions will be the same as on drupal.org. Feel free to vreak everything but don't forget to file complaints here. (Note: this is only a pruned version of the drupal.org database with all project nodes and nodes with nids > 7000 dropped). ------------------------------------------------------------------------ February 3, 2005 - 05:19 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_15.patch (52.39 KB) There was some error in node_save and also the patches to the database.inc files got lost... ------------------------------------------------------------------------ February 3, 2005 - 08:07 : robertDouglass Submitting book pages doesn't work on your test site. It puts the entire content of the preview inside the body textarea. I wrote a sentence in the body and the log, and pressing preview put several lines of HTML containing both sentences in the body textarea on the preview page, plus the book page wouldn't submit. -R ------------------------------------------------------------------------ February 3, 2005 - 08:50 : Junyor 0 results here. I started using Drupal with version 4.4, though. ------------------------------------------------------------------------ February 4, 2005 - 00:56 : killes(a)www.drop.org @Junyor: Thanks, that's a good sign. Maybe somebody else has an older db to try. @robertDouglass: The first effect you describe is due to drupaldevs running on PHP 5. I am unsure why the second thing does not work. In node_save() the node object has a nid although there is none in the form. Very strange. I've enabled display of db queries on the testsite. ------------------------------------------------------------------------ February 4, 2005 - 20:17 : dmjossel No results here on the query: select nid,type from node where type like '%/%'; On a database that was put in place prior to Drupal 4 and is now running on 4.5.2. ------------------------------------------------------------------------ February 4, 2005 - 21:44 : killes(a)www.drop.org @dmjossel: thanks. @all. The strange problem I reported was apparently php 5 related. After applying Steven's php 5 patch it went away. One error is remaining: If I create a new forum topic it will be shown as part of the book on preview. Hmm, that was due to a db that got corrupted during testing so that is fixed too. Please poke around at the test site and look if you find more errors. ------------------------------------------------------------------------ February 5, 2005 - 08:16 : Steven By the way, I just remembered that Drupal.org has some blogs lingering on in the database even though blog.module is not enabled. Perhaps this is causing troubles? ------------------------------------------------------------------------ February 5, 2005 - 12:22 : Anonymous I can't see why it would. Drupal.org will need extra updates for images and project nodes because those have their own tables. GK. ------------------------------------------------------------------------ February 6, 2005 - 13:49 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_16.patch (52.49 KB) Updated to apply to cvs again. ------------------------------------------------------------------------ February 22, 2005 - 21:15 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_17.patch (49.64 KB) Updated again. All we need is a patch to upload module and an upgrade path for it. ------------------------------------------------------------------------ March 4, 2005 - 05:22 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_18.patch (52.31 KB) Updated once more. Moved log field from book to node_revisions table as discussed in Antwerp. upload module still missing. We need to decide under which circumstances the log field should be displayed. Should that be added to the workflow? Should it depend on the revisions setting? ------------------------------------------------------------------------ March 5, 2005 - 20:27 : Anonymous Attachment: http://drupal.org/files/issues/revisions_20.patch (75.52 KB) Ok, here it is: Yet another revision of this grrrrreat patch. Changes from previous versions: - supports versioning for uploaded files. A problem is that if you delete a file, it will be gone for all revisions. - the log field is now in the node_revsions table, but each module has to decide whether to show it or not. I've implemented it for the page and the book type odes. Also, the field can be edited when adding non-book nodes to the book. The log is displayed on the revisions page and if a node is moderated. - the revisions are moved to an old_revisions table to a) get the node table smaller and b) still leave the mavailable for contrib modules that want to retreive old version data. The patch has been applied to killes.drupaldevs.org/revision where it can be tested by anybody (especially people who have "site admin" rights on drupal.org) The database is from drupal.org and you should b able to log in with your pass or simply mail yourself a new one. Gerhard ------------------------------------------------------------------------ March 5, 2005 - 20:51 : Anonymous Attachment: http://drupal.org/files/issues/revisions_21.patch (59.42 KB) BTW, I marked this a bug because atm the revisions field can grow quite big. Neil has reported problems from some users who were not able to load some nodes due to to many large revisions. Also, som unrelated stuff crept into the patch. New version attached. ------------------------------------------------------------------------ March 8, 2005 - 06:56 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_22.patch (60.29 KB) Ok, I think I got it. Changes to last version: - uploads are no properly versioned. Missing are still pgsql checks and updates. ------------------------------------------------------------------------ March 10, 2005 - 17:58 : Anonymous Was able to get http://drupal.org/files/issues/revisions_21.patch to work with drupal-cvs.tar.gz (10 March 2005) by: - includes/database.mysql.inc: Commenting out duplicates for functions function db_begin_transaction and function db_commit_transaction - modules: node.module: Removing "'title' => $node->title," from $node_table_values variable declaration and removing "'title' => "'%s'"," from "$node_table_types" variable declaration. Happy to submit a patch if requested. I'll watch this thread. ------------------------------------------------------------------------ March 12, 2005 - 00:59 : killes(a)www.drop.org The duplicate function has been removed in rev 22 of this patch. Why do you think the changes in node_save are needed? Titles are saved in both tables for performance reasons. ------------------------------------------------------------------------ March 13, 2005 - 17:12 : jlerner Hi - I posted comment #62. The changes to node_save appear to be needed because recent patches (both 21 and 22) remove the field 'title' from table 'node'. So without the changes to node_save, node.module is broken and generates errors. Joshua ------------------------------------------------------------------------ March 13, 2005 - 17:29 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_23.patch (61.17 KB) Thanks, Joshua, for catching this. node:title is there to stay. ------------------------------------------------------------------------ April 13, 2005 - 17:29 : moshe weitzman since HEAD is open again, perhaps it is a good time to revisit this patch. once this is committed, lets address - http://drupal.org/node/11071 "node_validate does not respect group editing" ------------------------------------------------------------------------ April 18, 2005 - 16:43 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_24.patch (60.39 KB) Updated. ------------------------------------------------------------------------ April 18, 2005 - 17:16 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_24_0.patch (60.39 KB) Updated. ------------------------------------------------------------------------ April 19, 2005 - 06:19 : Dries I'll commit this patch later this week! If you haven't checked this patch already, I urge you to test/check it out because it will have significant impact on existing code and modules! ------------------------------------------------------------------------ April 19, 2005 - 06:21 : Dries Also, what do people think about the n.title being duplicated? ------------------------------------------------------------------------ April 19, 2005 - 06:26 : chx I won't lose any sleep because of duplicated titles... ------------------------------------------------------------------------ April 19, 2005 - 19:35 : killes(a)www.drop.org Let me explain why I have chosen to duplicate the title (and also the uid): If you look at all the queries in Drupal, you will find that most of them only need the title and th uid of a node. That is, by duplicating it, we save expensive joins on the node_revisions table. Due to this fact, this patch is actually a performance improvement. A note about updating contrib module: Strictly speaking they wouldn't need to be updated. They only need to if their authors decide that their info should be available for revisioning. The upgrade path for forum.module in my update.inc patch (+ the forum patch) should show you what needs to be done. I will write a note for the update page once the patch hits core.

1 0

[drupal-devel] [bug] book admin broken under php5
by killes 24 Apr '05

24 Apr '05

Issue status update for http://drupal.org/node/21195 Project: Drupal Version: 4.6.0 Component: book.module Category: bug reports -Priority: critical +Priority: normal -Assigned to: Anonymous +Assigned to: killes(a)www.drop.org Reported by: kevin lapalme Updated by: killes(a)www.drop.org -Status: active +Status: patch Attachment: http://drupal.org/files/issues/book_29.patch (758 bytes) Here is a proper patch. Should be applied to cvs and 4.6 killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ April 24, 2005 - 03:55 : kevin lapalme [function.array-merge]: Argument #2 is not an array in /home/.../public_html/modules/book.module on line 697. ------------------------------------------------------------------------ April 24, 2005 - 04:09 : kevin lapalme if you change line 697 to: $rows[] .= array_merge($rows, book_admin_view_book($node->nid, $depth + 1)); from: $rows = array_merge($rows, book_admin_view_book($node->nid, $depth + 1)); it fixes it.

1 0