Page MenuHomePhabricator

Figure out what to do with opinions_created
Open, Needs TriagePublic

Description

So the user_stats table has a field named stats_opinion_created, which is meant to hold info about how many opinions (=blog posts; "opinion" being an ArmchairGM-ism) the user has created. Too bad that it doesn't work, yet some code uses it (namely BlogPageHooks::getArticles()), which results in profile pages showing stuff like "0 of 2" (when $wgUserProfileDisplay['articles'] = true;) next to the "Blogs" header.
It goes without saying that "0 of 2" makes no logical sense, although when you look at the code it's painfully obvious why: the first count is from the user_stats.stats_opinion_created field and the second one is the result of a direct SQL query to get the blog posts created by the user.

The sucky piece of code is called BlogPageHooks::updateCreatedOpinionsCount(). For whatever reason(s) it's hooked to both PageContentSave and PageContentSaveComplete hooks. I think it only needs to be hooked into the latter, because we want to update social statistics once the edit has definitely gone through, yes?
But more than that, there's the crappy code which pulls (or tries to, anyway) the user name from the MediaWiki:Blog-by-user-category i18n msg. Like the associated code comment says:

			// @todo CHECKME/FIXME: This probably no longer works as intended
			// due to the recent (as of 20 September 2014) i18n message change

If core had something like Title::isInCategory() we could probably just do something like this:

$userBlogCat = wfMessage( 'blog-by-user-category', $user->getName() )->inContentLanguage()->text();
if ( $ctg->isInCategory( $userBlogCat ) ) {
	// do our stuff here
}

But alas, core has no such method so we need to roll out our own logic here. One possible solution (with some tweaks, obviously) would be this snippet used by the Games skin:

	/**
	 * Checks if the current page is in the given category.
	 *
	 * @param string $category Category name
	 * @return bool True if the page is in the category, otherwise false
	 */
	function inCategory( $category ) {
		$categoryText = array();

		// This originally used getCategoryLinks() which returned HTML instead
		// of pure category names...how silly.
		foreach ( $this->skin->getOutput()->getCategories() as $ctg ) {
			$categoryText[] = strip_tags( $ctg );
		}

		if ( is_array( $categoryText ) && in_array( $category, $categoryText ) ) {
			return true;
		} else {
			return false;
		}
	}

But I have no idea about how well (or not-so-well) that'd perform, so we oughta look into that as well.

Related to BlogPage i18n suckage: T87213: Investigate and rework BlogPage byline code

Event Timeline

ashley created this task.Dec 26 2016, 5:37 PM
Restricted Application added a project: Social-Tools. · View Herald TranscriptDec 26 2016, 5:37 PM
SamanthaNguyen moved this task from Backlog to Feedback on the SocialProfile board.Jan 1 2017, 3:27 AM
SamanthaNguyen moved this task from Backlog to Feedback on the BlogPage board.Jan 7 2017, 10:20 PM

The todo CHECKME/FIXME is right. The message contains a $1 parameter that isn't substituted, which means strpos will not find anything. gj. The message should substitute the parameter and it should match (using a simple equality comparision). There's another flaw, however: It updates the stats for the user performing the edit. However, what if the blog is updated by a different user which adds its own category?

None of the hooks PageContentSave nor PageContentSaveComplete will work, because the categorylinks table is updated by a background job. This means when the page is created, the categorylinks is not populated for that page ID. It should work if you edit the blog page afterwards, though. The fix for this would be to use a different hook that runs when the refreshlinks job does all its duties.

In theory, a more elegant solution could be to use the CategoryAfterPageAdded and CategoryAfterPageRemoved hooks, to increment and decrement the stat instead of doing a full calculation. However, in case those hooks fail to run (I'm not sure what would happen when running a refreshLinks.php or similar) it can cause counts to go wrong, so a full calculation would probably be OK here.

Another very simple solution could be to use the category counts from the category table directly, and avoid using the stats altogether. It could be inaccurate if there are other pages in that category not in the blog namespace, but I doubt people would add other pages to those categories. This looks like the cheaper solution and with low chance of errors.