Last modified: 2010-05-15 15:37:47 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 2014 - Patchlet 1: ENotif code clean-up patch for REL1_5 - revisited
Patchlet 1: ENotif code clean-up patch for REL1_5 - revisited
Status: RESOLVED INVALID
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.5.x
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-need-review
Depends on: 2105
Blocks: 1876 1932 1002 2825
  Show dependency treegraph
 
Reported: 2005-04-29 01:45 UTC by T. Gries
Modified: 2010-05-15 15:37 UTC (History)
0 users

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
ENotif code clean-up; reborn user_newtalk; (33.13 KB, patch)
2005-04-29 01:47 UTC, T. Gries
Details
sql fixlet (216 bytes, text/plain)
2005-04-29 01:48 UTC, T. Gries
Details
maintenance/archives/patch-create-user_newtalk.sql (366 bytes, text/plain)
2005-04-29 02:01 UTC, T. Gries
Details
patchlet 1 bugfix 2014 (no changes; just the diff against todays CVS) (33.13 KB, patch)
2005-04-29 18:55 UTC, T. Gries
Details
CLEAN patch for enotif code clean up and user-page newmessage flag (27.53 KB, patch)
2005-08-13 17:03 UTC, T. Gries
Details
CLEAN patch for enotif code clean-up and user-page newmessage flag plus cosmetics in UserMailer (28.01 KB, patch)
2005-08-13 19:08 UTC, T. Gries
Details
renewed clean patch _without_ introducing "newmessage flag for user_page changes" (21.00 KB, patch)
2005-08-21 23:52 UTC, T. Gries
Details
renewed clean patch _without_ introducing "newmessage flag for user_page changes" (20.09 KB, patch)
2005-08-22 00:04 UTC, T. Gries
Details

Description T. Gries 2005-04-29 01:45:55 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
Comment 1 T. Gries 2005-04-29 01:47:35 UTC
Created attachment 476 [details]
ENotif code clean-up; reborn user_newtalk; 

I forgot to mention: Module UserTalkUpdate.php dropped finally.
Comment 2 T. Gries 2005-04-29 01:48:24 UTC
Created attachment 477 [details]
sql fixlet
Comment 3 T. Gries 2005-04-29 01:51:07 UTC
Brion: is the way how I contributed this okay ? 
Comment 4 T. Gries 2005-04-29 01:59:37 UTC
Comment on attachment 477 [details]
sql fixlet

i submitted the wrong sql fixlet
Comment 5 T. Gries 2005-04-29 02:01:24 UTC
Created attachment 478 [details]
maintenance/archives/patch-create-user_newtalk.sql

The correct re-creator script
Comment 6 T. Gries 2005-04-29 09:09:18 UTC
forgot also to mention this:

* fixes also the problem of not deleting "You have new messages"
Comment 7 T. Gries 2005-04-29 18:55:38 UTC
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 8 T. Gries 2005-04-29 18:56:24 UTC
Comment on attachment 476 [details]
ENotif code clean-up; reborn user_newtalk; 

superseded by same patch against newer CVS
Comment 9 T. Gries 2005-05-02 19:28:24 UTC
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)
Comment 10 T. Gries 2005-05-11 20:24:28 UTC
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
Comment 11 T. Gries 2005-08-13 16:59:46 UTC
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. 
Comment 12 T. Gries 2005-08-13 17:03:14 UTC
Created attachment 784 [details]
CLEAN patch for enotif code clean up and user-page newmessage flag
Comment 13 T. Gries 2005-08-13 19:08:55 UTC
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
Comment 14 Brion Vibber 2005-08-19 10:49:12 UTC
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.
Comment 15 T. Gries 2005-08-19 11:28:28 UTC
(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 ?
Comment 16 T. Gries 2005-08-19 11:32:28 UTC
(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.

Comment 17 T. Gries 2005-08-21 23:52:24 UTC
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.
Comment 18 T. Gries 2005-08-21 23:58:10 UTC
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.  
 
Comment 19 T. Gries 2005-08-22 00:04:13 UTC
Created attachment 823 [details]
renewed clean patch _without_ introducing "newmessage flag for user_page changes"

removed a not-used subroutine in User.php
Comment 20 T. Gries 2005-08-22 08:43:17 UTC
(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. ]
Comment 21 T. Gries 2005-08-22 21:50:18 UTC
(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). 
Comment 22 Brion Vibber 2005-12-07 07:06:40 UTC
Report any actual bugs separately so they can be fixed against current code.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links