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

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

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

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

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

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

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.

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

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

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

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

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

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

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

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

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

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

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

DannyS712 removed a project: Patch-For-Review.

Not passing a user is deprecated