Page MenuHomePhabricator

Calls to LogPage::addEntry should pass a user
Closed, ResolvedPublic

Description

Currently accepts a user and defaults to $wgUser
Callers should provide this user

While all uses should be replaced, for repos not deployed by WMF / not hosted on gerrit / that cannot be updated by WMF developers this isn't a priority, and not prevent deprecation and removal

Event Timeline

Restricted Application added a project: User-DannyS712. · View Herald TranscriptJan 25 2020, 1:19 AM

Change 567174 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/CentralAuth@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567174

DannyS712 updated the task description. (Show Details)Jan 25 2020, 1:58 AM

Change 567177 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/SocialProfile@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567177

Change 567178 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/GlobalBlocking@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567178

Change 567179 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/ApprovedRevs@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567179

Change 567180 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/AutoProxyBlock@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567180

Change 567181 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/BlockAndNuke@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567181

Change 567182 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/Cargo@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567182

Change 567184 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/CloseWikis@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567184

Change 567186 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/Farmer@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567186

Change 567187 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/Interwiki@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567187

Change 567188 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/Poll@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567188

Change 567188 abandoned by DannyS712:
Pass a user to LogPage::addEntry

Reason:
Woops, already provided

https://gerrit.wikimedia.org/r/567188

Restricted Application added a project: Social-Tools. · View Herald TranscriptJan 25 2020, 2:36 AM
DannyS712 updated the task description. (Show Details)Jan 25 2020, 8:50 AM

Change 567200 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/RevisionCommentSupplement@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567200

Change 567201 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/SpecialNamespaces@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567201

Change 567202 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/Sudo@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567202

DannyS712 updated the task description. (Show Details)Jan 25 2020, 10:23 AM

Change 567202 merged by jenkins-bot:
[mediawiki/extensions/Sudo@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567202

Change 567187 merged by jenkins-bot:
[mediawiki/extensions/Interwiki@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567187

Change 567201 merged by jenkins-bot:
[mediawiki/extensions/SpecialNamespaces@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567201

DannyS712 updated the task description. (Show Details)Jan 25 2020, 6:12 PM

Change 567177 merged by Jack Phoenix:
[mediawiki/extensions/SocialProfile@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567177

Change 567179 merged by jenkins-bot:
[mediawiki/extensions/ApprovedRevs@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567179

DannyS712 updated the task description. (Show Details)Jan 27 2020, 5:15 AM
DannyS712 updated the task description. (Show Details)Jan 28 2020, 4:59 PM

I just want noted that LogPage::addEntry is an old way to provide log entries on Special:Log.
The new way is direct use of LogFormatter class.
In my opinion there must be no complex actions to avoid global state in that class.
It would be more future proofed (but sometimes more complex) to migrate the extension to the new logging
But when the caller already have a context, than it is okay to avoid the fallback to $wgUser. That are simpel patch sets.

https://www.mediawiki.org/wiki/Manual:Logging_to_Special:Log

Noted. Perhaps it should officially be deprecated? Its currently not even soft deprecated.

ashley added a subscriber: ashley.Jan 29 2020, 9:18 AM

Noted. Perhaps it should officially be deprecated? Its currently not even soft deprecated.

That would certainly make sense. I do want to note that for things like SocialProfile, the reason why it's still using the old class instead of the newer class is that I'm not sure how to properly migrate old log entries over. Migrating the code to use the newer classes would also help improve the i18n as well as overall aesthetic appearance of the relevant log entries (see, for example, the user profile edit log on Brickipedia to see how the entries currently display).

Noted. Perhaps it should officially be deprecated? Its currently not even soft deprecated.

That would certainly make sense. I do want to note that for things like SocialProfile, the reason why it's still using the old class instead of the newer class is that I'm not sure how to properly migrate old log entries over. Migrating the code to use the newer classes would also help improve the i18n as well as overall aesthetic appearance of the relevant log entries (see, for example, the user profile edit log on Brickipedia to see how the entries currently display).

There is no need to migrate old log entries. The old log entries are detected as legacy and displayed the old way.
New log entries are stored (in the same table) after the new code is there and used. But the new logformatter must be always backward compatibility when adding new params to the log type or removing parameters. It must show the older log entries (but not the legacy one) like it was on insert. Look at the block formatter or protect formatter how complicated that could be. There are also tests for both classes to test against older parameter combination.

Change 567186 merged by jenkins-bot:
[mediawiki/extensions/Farmer@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567186

Change 567184 merged by jenkins-bot:
[mediawiki/extensions/CloseWikis@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567184

Change 567180 merged by jenkins-bot:
[mediawiki/extensions/AutoProxyBlock@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567180

Change 567178 merged by jenkins-bot:
[mediawiki/extensions/GlobalBlocking@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567178

DannyS712 updated the task description. (Show Details)Jan 31 2020, 5:19 PM

Change 567181 merged by jenkins-bot:
[mediawiki/extensions/BlockAndNuke@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567181

DannyS712 updated the task description. (Show Details)Feb 4 2020, 9:14 PM

Change 570145 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/LiquidThreads@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/570145

DannyS712 updated the task description. (Show Details)Feb 4 2020, 9:48 PM

Change 570149 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/ProtectSite@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/570149

Change 570149 merged by jenkins-bot:
[mediawiki/extensions/ProtectSite@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/570149

DannyS712 updated the task description. (Show Details)Feb 5 2020, 6:08 AM
DannyS712 updated the task description. (Show Details)

Change 567182 merged by jenkins-bot:
[mediawiki/extensions/Cargo@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567182

DannyS712 updated the task description. (Show Details)Feb 5 2020, 4:11 PM

Change 567200 merged by jenkins-bot:
[mediawiki/extensions/RevisionCommentSupplement@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567200

Change 570145 merged by jenkins-bot:
[mediawiki/extensions/LiquidThreads@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/570145

DannyS712 updated the task description. (Show Details)Feb 14 2020, 3:29 AM

Change 567174 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Pass a user to LogPage::addEntry

https://gerrit.wikimedia.org/r/567174

Change 563759 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Deprecate falling back to $wgUser in some functions

https://gerrit.wikimedia.org/r/563759

DannyS712 updated the task description. (Show Details)Feb 19 2020, 5:25 AM

Change 563759 merged by jenkins-bot:
[mediawiki/core@master] Deprecate falling back to $wgUser in some functions

https://gerrit.wikimedia.org/r/563759

DannyS712 closed this task as Resolved.Feb 20 2020, 2:04 AM
DannyS712 removed a project: Patch-For-Review.

Not passing a user is deprecated