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.

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

Move Request::cleanExpiredUnconfirmedRequests to maintenance script #452

Merged
merged 3 commits into from Jun 29, 2017
Merged

Move Request::cleanExpiredUnconfirmedRequests to maintenance script #452

merged 3 commits into from Jun 29, 2017

Conversation

DatGuy1
Copy link
Contributor

@DatGuy1 DatGuy1 commented Jun 28, 2017

Copy link
Member

@FastLizard4 FastLizard4 left a comment

Choose a reason for hiding this comment

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

Code does not verify

@@ -20,6 +20,10 @@
":mail" => $cDataClearEmail
));

if ($enableEmailConfirm == 1) {
Copy link
Member

Choose a reason for hiding this comment

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

This code will cause a syntax error because you didn't add $enableEmailConfirm to the global directive on line 15. Please remember to test your code to avoid issues like this.

Copy link
Member

Choose a reason for hiding this comment

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

Correction, not a syntax error, but an undefined variable error.

@codecov-io
Copy link

Codecov Report

Merging #452 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #452   +/-   ##
=======================================
  Coverage   20.54%   20.54%           
=======================================
  Files           9        9           
  Lines         584      584           
=======================================
  Hits          120      120           
  Misses        464      464

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef7e460...0141993. Read the comment docs.

Copy link
Member

@stwalkerster stwalkerster left a comment

Choose a reason for hiding this comment

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

lgtm, will test properly when I have a more stable internet connection to test with.

Copy link
Member

@FastLizard4 FastLizard4 left a comment

Choose a reason for hiding this comment

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

Code looks good, though it needs testing before approval

@FastLizard4 FastLizard4 dismissed their stale review June 28, 2017 23:03

Concerns addressed

Copy link
Member

@stwalkerster stwalkerster left a comment

Choose a reason for hiding this comment

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

MariaDB [(none)]> use waca_production;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
MariaDB [waca_production]> select count(*) from request where ip <> '127.0.0.1' and emailconfirm <> 'Confirmed';
+----------+
| count(*) |
+----------+
|      210 |
+----------+
1 row in set (0.01 sec)

MariaDB [waca_production]> select count(*) from request where ip <> '127.0.0.1' and status = 'Closed';
+----------+
| count(*) |
+----------+
|      590 |
+----------+
1 row in set (0.00 sec)

MariaDB [waca_production]> select 'running ClearOldData.php script' from dual;
+---------------------------------+
| running ClearOldData.php script |
+---------------------------------+
| running ClearOldData.php script |
+---------------------------------+
1 row in set (0.00 sec)

MariaDB [waca_production]> select count(*) from request where ip <> '127.0.0.1' and emailconfirm <> 'Confirmed';
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (0.00 sec)

MariaDB [waca_production]> select count(*) from request where ip <> '127.0.0.1' and status = 'Closed';
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (0.00 sec)

MariaDB [waca_production]> \t
C:\apps\php\5.6.24-nts-VC11-x86\php.exe C:\Users\stwalkerster\Projects\wikipedia-account-creation-tool-2\ClearOldData.php
Deletion complete.
Process finished with exit code 0

Tests fine on my old sanitised production clone, where everything is over the dataclear interval.

@stwalkerster stwalkerster merged commit ad5541f into enwikipedia-acc:master Jun 29, 2017
@DatGuy1 DatGuy1 deleted the maintenancemove branch February 12, 2022 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants