Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update AdminScoreController.php #25

Merged
merged 1 commit into from May 1, 2017
Merged

Update AdminScoreController.php #25

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/AppBundle/Controller/AdminScoreController.php
View file
Open in desktop
Expand Up @@ -62,9 +62,9 @@ public function resultAction($project, $username)

$userTable = $lh->getTable("user", $dbName);
$pageTable = $lh->getTable("page", $dbName);
$loggingTable = $lh->getTable("logging", $dbName);
$revisionTable = $lh->getTable("revision", $dbName);
$archiveTable = $lh->getTable("archive", $dbName);
$loggingTable = $lh->getTable("logging_userindex", $dbName);
$revisionTable = $lh->getTable("revision_userindex", $dbName);
$archiveTable = $lh->getTable("archive_userindex", $dbName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just broke backwards compatibility with all versions of MediaWiki. There's a reason we go through the labs helper - specifically to allow for a table mapping.

That should be documented better on my side, which is a separate issue.


// MULTIPLIERS (to review)
$ACCT_AGE_MULT = 1.25; # 0 if = 365 jours
Expand Down Expand Up @@ -105,8 +105,10 @@ public function resultAction($project, $username)
AND log_namespace=0
AND log_deleted=0 and log_user_text=:username
UNION
SELECT 'blocks' as source, COUNT(*) as value FROM $loggingTable
WHERE log_type='block' AND log_action='block' AND log_namespace=2 AND log_deleted=0 AND log_title=:username
SELECT 'blocks' as source, COUNT(*) as value FROM $loggingTable l
INNER JOIN $userTable u ON l.log_user = u.user_id
WHERE l.log_type='block' AND l.log_action='block'
AND l.log_namespace=2 AND l.log_deleted=0 AND u.user_name=:username
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... you attempt to make this more efficient by adding another Join - which is a slow and heavy process. That makes absolutely no sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the "log_user" column in the query makes it better despite adding another join.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'm convinced. Please fix my other comment and we'll give this the thumbs up.

To clarify my other comment - in order to access a table you use the MediaWiki table rather than the labs table. We then map it using app/config/table_map.yml

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 from me. The new query definitely runs significantly faster, at around 21.73 sec run time with one test I did. The same test took over 3 minutes with the old query, I just ended up killing it.

Indeed Matthew is right however that we have a table mapping system in place. All you need to know is to pass the normal table name to $lh->getTable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean. Is there any chance you can make the necessary changes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I'll merge then fix it up.

Thank you for this pull request 😄

UNION
SELECT 'afd' as source, COUNT(*) as value FROM $revisionTable
WHERE rev_page LIKE 'Articles for deletion/%'
Expand Down