Skip to content
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions extension.json
Original file line number Diff line number Diff line change
Expand Up @@ -382,10 +382,18 @@
"CreateWikiStateDays": {
"description": "Array. Integer values in days when a wiki is deemed inactive, closed, removed (hidden) and deleted. Number of days passed since last change - not from initial inactivity.",
"value": {
"inactive": 45,
"closed": 15,
"removed": 120,
"deleted": 7
"default": {
"inactive": 45,
"closed": 15,
"removed": 120,
"deleted": 7
},
"no-edits": {
"inactive": 15,
"closed": 30,
"removed": 0,
"deleted": 0
}
}
},
"CreateWikiSubdomain": {
Expand Down
2 changes: 1 addition & 1 deletion includes/Services/WikiManagerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ public function delete( bool $force ): ?string {
$unixNow = (int)wfTimestamp( TS_UNIX, $this->dbw->timestamp() );

// Return error if the wiki is not deleted, force is not used, or the deletion grace period has not passed.
$deletionGracePeriod = (int)$this->options->get( ConfigNames::StateDays )['deleted'] * 86400;
$deletionGracePeriod = (int)$this->options->get( ConfigNames::StateDays )["no-edits"]['deleted'] * 86400;
$deletedWiki = (bool)$row->wiki_deleted && (bool)$row->wiki_deleted_timestamp;

if (
Expand Down
5 changes: 5 additions & 0 deletions maintenance/CheckLastWikiActivity.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ public function execute(): void {
}

public function getTimestamp(): int {
$defaultActor = $this->getServiceContainer()->getUserFactory()
Copy link
Member

Choose a reason for hiding this comment

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

Probably should have a test case in CheckLastWikiActivityTest to make sure properly excludes MediaWiki default edits.

->newFromName( 'MediaWiki default' )
->getActorId();
Comment on lines +24 to +26
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

❓ Verification inconclusive

Improvement to exclude MediaWiki default actor from activity checks

This change adds logic to filter out log entries from the MediaWiki default actor, ensuring that only meaningful user activities are counted when determining wiki activity.

A test case in CheckLastWikiActivityTest would be valuable to verify that this properly excludes MediaWiki default edits, as mentioned in a previous review.


🏁 Script executed:

#!/bin/bash
# Check if any test exists for this functionality
rg -t php "MediaWiki default" tests/phpunit

Length of output: 45


Action Required: Add a test to verify MediaWiki default actor exclusion

The logic in maintenance/CheckLastWikiActivity.php (lines 22–24) correctly retrieves the actor ID for "MediaWiki default" to later exclude its log entries from wiki activity checks. However, our search in the test suite didn’t find any test case covering this functionality. To fully validate that log entries from the MediaWiki default actor are excluded, please add a test case (preferably in CheckLastWikiActivityTest.php) that explicitly checks for this behavior.


$dbr = $this->getDB( DB_REPLICA );

// Get the latest revision timestamp
Expand All @@ -37,6 +41,7 @@ public function getTimestamp(): int {
->where( [
$dbr->expr( 'log_type', '!=', 'renameuser' ),
$dbr->expr( 'log_type', '!=', 'newusers' ),
$dbr->expr( 'log_actor', '!=', $defaultActor ),
] )
->orderBy( 'log_timestamp', SelectQueryBuilder::SORT_DESC )
->limit( 1 )
Expand Down
30 changes: 23 additions & 7 deletions maintenance/ManageInactiveWikis.php
Copy link
Member

Choose a reason for hiding this comment

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

This also needs tests in ManageInactiveWikisTest but I can probably put some work into doing that at some point.

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Miraheze\CreateWiki\Maintenance;

use MediaWiki\Maintenance\Maintenance;
use MediaWiki\Config\ConfigException;
use Miraheze\CreateWiki\ConfigNames;
use Miraheze\CreateWiki\Services\RemoteWikiFactory;

Expand Down Expand Up @@ -32,6 +33,8 @@ public function execute(): void {
$this->fatalError( 'Enable $wgCreateWikiEnableManageInactiveWikis to run this script.' );
}

$inactiveDays = (int)$this->getConfig()->get( ConfigNames::StateDays )['no-edits']['inactive'];
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we hardcode no-edits here? I'm uncertain about that.

Copy link
Author

Choose a reason for hiding this comment

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

Performance wise, the check this is used in is to tell if it's worth doing more processing. Using the function to get the proper check would mean that code(querying the database) runs for every wiki even if they're active, while if we leave it here, checklastActivity() would run on every wiki that hasn't been edited for at least 7 days under default config. So I guess it depends on which performance hit we think will hurt less? It would probably be activity->getTimestamp() I feel.. in which case we should change it.


$databaseUtils = $this->getServiceContainer()->get( 'CreateWikiDatabaseUtils' );
$remoteWikiFactory = $this->getServiceContainer()->get( 'RemoteWikiFactory' );

Expand All @@ -49,8 +52,6 @@ public function execute(): void {

foreach ( $wikis as $wiki ) {
$remoteWiki = $remoteWikiFactory->newInstance( $wiki );
$inactiveDays = (int)$this->getConfig()->get( ConfigNames::StateDays )['inactive'];

$remoteWiki->disableResetDatabaseLists();

// Check if the wiki is inactive based on creation date
Expand All @@ -68,18 +69,33 @@ private function checkLastActivity(
string $dbname,
RemoteWikiFactory $remoteWiki
): bool {
$inactiveDays = (int)$this->getConfig()->get( ConfigNames::StateDays )['inactive'];
$closeDays = (int)$this->getConfig()->get( ConfigNames::StateDays )['closed'];
$removeDays = (int)$this->getConfig()->get( ConfigNames::StateDays )['removed'];
$canWrite = $this->hasOption( 'write' );

/** @var CheckLastWikiActivity $activity */
$activity = $this->createChild( CheckLastWikiActivity::class );
'@phan-var CheckLastWikiActivity $activity';

$activity->setDB( $this->getDB( DB_PRIMARY, [], $dbname ) );
$lastActivityTimestamp = $activity->getTimestamp();

$stateConfig = $this->getConfig()->get( ConfigNames::StateDays );

if ( !isset( $stateConfig['default'] ) ) {
throw new ConfigException(
'Default state config not found. Please check your configuration.'
);
}
if ( !isset( $stateConfig['no-edits'] ) ) {
$stateConfig['no-edits'] = $stateConfig['default'];
$this->output(
'No edits state config not found, using default instead.'
);
}
$track = $lastActivityTimestamp !== 0 ? 'default' : 'no-edits';

$inactiveDays = (int)$stateConfig[$track]['inactive'];
$closeDays = (int)$stateConfig[$track]['closed'];
$removeDays = (int)$stateConfig[$track]['removed'];
$canWrite = $this->hasOption( 'write' );

// If the wiki is still active, mark it as active
if ( $lastActivityTimestamp > date( 'YmdHis', strtotime( "-{$inactiveDays} days" ) ) ) {
if ( $canWrite && $remoteWiki->isInactive() ) {
Expand Down