Last modified: 2010-05-15 15:37:47 UTC
Patchlet number 1 (for issues in collective bugzilla 454). * moved the notification calls to module RecentChange * fully reestablished memcache-efficient user_newtalk table,but only for the newmessage marker. * it works now that way: when someone writes on another user's X talk page, Article.php creates an watchlist entry for this user_talk:X (and the user-page) on-the-fly now (calls addwatch for it), whereas the immediately following editupdate call triggers sending the ENotif via the hook in RecentChange. This is safe and clean now. * needed to move two RecentChange calls in Article just behind the editupdates calls to make that working; I did not notice any problem with that move * TimestampOrNull slightly modified, but still database independent * updaters.inc, tables.sql and maintenance/patch-create-user_newtalk.sql supplied with the patch * WatchedItem:duplicatEntries now copies both pages (subject/talk) when a page is moved
Created attachment 476 [details] ENotif code clean-up; reborn user_newtalk; I forgot to mention: Module UserTalkUpdate.php dropped finally.
Created attachment 477 [details] sql fixlet
Brion: is the way how I contributed this okay ?
Comment on attachment 477 [details] sql fixlet i submitted the wrong sql fixlet
Created attachment 478 [details] maintenance/archives/patch-create-user_newtalk.sql The correct re-creator script
forgot also to mention this: * fixes also the problem of not deleting "You have new messages"
Created attachment 479 [details] patchlet 1 bugfix 2014 (no changes; just the diff against todays CVS) CVS REL1_5 as of 29.04.2005 18:50 UTC incl. this patch: full directory /phase3 incl. CVS tag files can be downloaded from http://www.wikinaut.de/mw/cvs15+bugfix2014.tgz
Comment on attachment 476 [details] ENotif code clean-up; reborn user_newtalk; superseded by same patch against newer CVS
This is a kind reminder to bring this into CVS HEAD, please. It patches several bugs in current ENotif (e.g. non-deleting of "You have new messages"). and allows further improvments in smaller steps. On request, I can submit a fresh "diff" file against CVS. If needed, the full directory /phase3 incl. CVS tag files can be downloaded from http://www.wikinaut.de/mw/cvs15+bugfix2014.tgz next patchlet (enotif for new pages is waiting)
bug: ENotifs were sent also to unconfirmed addresses :-(( bugfix: add two parenthesis in UserMailer.php :-)) Excerpt: - if ( ( $enotifwatchlistpage && $watchingUser->getOption('enotifwatchlistpages') ) || - ( $enotifusertalkpage && $watchingUser->getOption('enotifusertalkpages') ) + if ( ( ( $enotifwatchlistpage && $watchingUser->getOption('enotifwatchlistpages') ) || + ( $enotifusertalkpage && $watchingUser->getOption('enotifusertalkpages') ) ) Whole patch: not available here. Consult download section http://meta.wikipedia.org/wiki/Enotif
Tim, as promised during Wikimania and Brion (and Hashar, Avar, Duesentrieb), here comes my _clean_ patch against CVS REL1_5 of today which also fixes 1876 (new message also for _User_ page changes) and 2825 (new message not shown for anons). Let me explain, what you can expect and how it works. 1) UserTalkUpdate.php --> drop this file finally (it is _not_ needed any longer) 2) updaters.inc --> drop the code of copy_newtalk_to_watchlist (it is _not_ needed any longer) 3) The whole business for newmessage flag handling for _user_ and user_talk page is still done by the _one_(!) entry in newtalk and memchache for efficiency reasons. Only when the owner of the user page visits his page, the watchlist table entry is asked to distinguish the three possible cases (user, user_talk or both pages changed). This reuslts in three different texts in the marker. 4) When a user or user_talk page is edited by someone else, then editupdate() in Article.php calls addWatch() without further user action and adds automatically that page to the watchlist of the user-page owner of course. This is needed to store information, which one of the both pages has been changed. It is also used for sending out enotifs, but the former aspect is the most essential one here. 5) the two calls to enotif modules are moved to RecentChange.php, where they logically belong to. 6) ++ no other things in that patch ++ I would like to ask you to have look to this clean patch now and ask to commit as soon as you can to REL1_5 and to CVS HEAD.
Created attachment 784 [details] CLEAN patch for enotif code clean up and user-page newmessage flag
Created attachment 785 [details] CLEAN patch for enotif code clean-up and user-page newmessage flag plus cosmetics in UserMailer CLEAN patch for enotif code clean-up and user-page newmessage flag plus cosmetics in UserMailer
This doesn't seem to be a cleanup patch, but rather introduces some kind of new feature where the function of the new messages notification is radically different. Closing as WONTFIX for now as this doesn't seem to do as advertised; please reopen if you have a cleanup patch that doesn't introduce large, controversial feature changes. For future reference: try not to make variable and function names quite so long. When they get to extremes like $wgShowNewtalkForUserOrUserTalkPage and $wgUser->checkNotificationPendingForArticleOrTalk() they harm readability.
(In reply to comment #14) > This .. introduces some kind of new > feature where the function of the new messages notification is radically different. It still is linked to http://bugzilla.wikipedia.org/show_bug.cgi?id=1876 which it solves. My decision to do so was the following, please allow me this defensive comment: The http://bugzilla.wikipedia.org/show_bug.cgi?id=1876 issue was discussed during your absence in the irc last week and very positively welcomed in the irc (i.a. Avar, Hashar, Duesentrieb) and there wasn't any objection, that I decided not to remove this sub-issue and to publish patchlet 1 - revisited with that feature built-in. The reason is also, that I use *the same* memcache bit as re-introduced by Tim Starling, which is efficient. I agree on better having split-off that issue, if there wasn't the wholeheartedly welcome to my "you have new message on your user page" question in the irc. Brion: Would you patch, if I removed the 1876 issue from 2014 patch ?
(In reply to comment #14) > For future reference: try not to make variable and function names quite so long. > When they get to extremes like $wgShowNewtalkForUserOrUserTalkPage and > $wgUser->checkNotificationPendingForArticleOrTalk() they harm readability. I used these ultralong names with the intention to help you and the rest of the developers who usually substitutes them by names in conformity with the general MediaWiki naming schemes. I will duly consider to follow your advise, if I publish the one or other patch.
Created attachment 822 [details] renewed clean patch _without_ introducing "newmessage flag for user_page changes" I removed the non-enotif related extension (newmessage marker also shown for changes on user pages) as requested; there is still one long variable name in it. Brion, pls. rename at your discretion.
Here comes my (now) clean patch against CVS REL1_5 of today Let me explain, what you can expect and how it works. 1) UserTalkUpdate.php --> drop this file finally. This file is not needed any longer. 2) updaters.inc --> drop the code of copy_newtalk_to_watchlist. This is not needed any longer. 3) The whole business for newmessage flag handling is done by the one entry in newtalk and memchache for efficiency reasons. 4) When a user_talk page is edited by someone else, then editupdate() in Article.php calls addWatch() without further user action and adds automatically that page to the watchlist of the user-page owner of course. 5) the two calls to enotif modules are moved to RecentChange.php, where they logically belong to.
Created attachment 823 [details] renewed clean patch _without_ introducing "newmessage flag for user_page changes" removed a not-used subroutine in User.php
(In reply to comment #19) > Created an attachment (id=823) [edit]renewed clean patch _without_ introducing "newmessage flag for user_pagechanges"removed a not-used subroutine in User.php Addendum Brion, the instruction $other->addWatch( $this->mTitle ); in about line 2123 in Article.php is _not_ needed, I forgot to remove that line. [ There is no need to add user_talk page X automatically to the watchlist of the user X; this was only necessary within my withdrawn extension. I'll upload a corrected patch without that line in a couple of hours. ]
(In reply to comment #20 ** correction ** $other->addWatch( $this->mTitle ); in about line 2123 in Article.php _is_ needed because it actually adds the changed user_talk page X to the watchlist of user X so that an e-notif can be sent on that event. The code as published in the patch is correct and reestablishes "send me an e-mail if someone changes my talk page" (if the user has checked that box in Special:Preferences i.e. opted-in).
Report any actual bugs separately so they can be fixed against current code.
