Skip to content

Commit d506119

Browse files
dati18outdooracorn
andauthored
Add Carbon 3 boundary coverage for timestamp parsing and integer-cast diff behavior (#1132)
- `MWTimestampHelperTest.php`: Removed `if` block that checked for type of `$carbon` since `getCarbonFromMWTimestamp()` returns `CarbonImmutable`, `TypeError` will be thrown if not. - `SendEmptyWikiNotificationsJobTest.php`: Tightened notification-threshold coverage so a wiki that is almost old enough (for example: 29.5 days old) does not trigger an empty-wiki notification early. - `PlatformStatsSummaryJobTest.php`: Added a test to ensure a second-precision `lastEdit` timestamp exactly at the inactivity cutoff is still counted as active. - `ConversionMetricTest.php`: Added a test to verify fractional day diffs are truncated to the integer values expected by the conversion metrics API. Bug: T426592 --------- Co-authored-by: Ollie <43674967+outdooracorn@users.noreply.github.com>
1 parent 14866b1 commit d506119

7 files changed

Lines changed: 110 additions & 5 deletions

app/Helper/MWTimestampHelper.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,12 @@
99
namespace App\Helper;
1010

1111
use Carbon\CarbonImmutable;
12-
use Carbon\Exceptions\InvalidFormatException;
1312

1413
class MWTimestampHelper {
1514
private const MWTimestampFormat = 'YmdHis';
1615

1716
public static function getCarbonFromMWTimestamp(string $MWTimestamp): CarbonImmutable {
1817
$carbon = CarbonImmutable::createFromFormat(self::MWTimestampFormat, $MWTimestamp);
19-
if ($carbon === null) {
20-
throw new InvalidFormatException('Unable to create Carbon object');
21-
}
2218

2319
return $carbon;
2420
}

app/Http/Controllers/ConversionMetricController.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,16 @@ public function index(Request $request) {
3434
$daysSinceLastEdit = null;
3535

3636
if ($wikiLastEditedTime !== null) {
37+
// cast to int to retain Carbon 2 behaviour of using whole days
3738
$daysSinceLastEdit = (int) $wikiLastEditedTime->diffInDays($current_date, false);
3839
}
3940

4041
if ($daysSinceLastEdit !== null && $daysSinceLastEdit >= 90) {
42+
// cast to int to retain Carbon 2 behaviour of using whole days
4143
$time_before_wiki_abandoned_days = (int) $wiki->created_at->diffInDays($wikiLastEditedTime, false);
4244
}
4345
if ($wikiFirstEditedTime !== null) {
46+
// cast to int to retain Carbon 2 behaviour of using whole days
4447
$time_to_engage_days = (int) $wiki->created_at->diffInDays($wikiFirstEditedTime, false);
4548
}
4649
$wiki_number_of_editors = $wiki->wikiSiteStats()->first()['activeusers'] ?? null;

app/Jobs/PlatformStatsSummaryJob.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@ public function prepareStats(array $allStats, $wikis): array {
132132
// is it edited in the last 90 days?
133133
if (!is_null($stats['lastEdit'])) {
134134
$lastTimestamp = MWTimestampHelper::getCarbonFromMWTimestamp(intval($stats['lastEdit']));
135+
// cast to int retain Carbon 2 behaviour of using whole days
136+
// pass `$absolute = true` argument to retain Carbon 2 behaviour of returning an absolute diff
135137
$diff = (int) $lastTimestamp->diffInSeconds($currentTime, true);
136138

137139
if ($diff <= $this->inactiveThreshold) {

app/Jobs/SendEmptyWikiNotificationsJob.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public function checkIfWikiIsOldAndEmpty(Wiki $wiki) {
3434
$emptyDaysThreshold = config('wbstack.wiki_empty_notification_threshold');
3535
$createdAt = $wiki->created_at;
3636
$now = CarbonImmutable::now();
37+
// cast to int to retain Carbon 2 behaviour of using whole days
3738
$emptyWikiDays = (int) $createdAt->diffInDays($now, true);
3839

3940
$firstEdited = $wiki->wikiLifecycleEvents->first_edited;

tests/Jobs/PlatformStatsSummaryJobTest.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,4 +274,55 @@ public function testCreationStats() {
274274
);
275275

276276
}
277+
278+
public function testPrepareStatsTreatsSecondPrecisionTimestampAtThresholdAsActive() {
279+
$currentTime = CarbonImmutable::now();
280+
281+
$wiki = Wiki::factory()->create(['deleted_at' => null, 'domain' => 'thresholdtest.com']);
282+
WikiDb::create([
283+
'name' => 'mwdb_threshold_' . $wiki->id,
284+
'user' => 'user',
285+
'password' => 'password',
286+
'version' => 'version',
287+
'prefix' => 'prefix',
288+
'wiki_id' => $wiki->id,
289+
]);
290+
291+
Http::fake([
292+
$this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=122&apcontinue=&aplimit=max&format=json' => Http::response([
293+
'query' => ['allpages' => []],
294+
], 200),
295+
$this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=120&apcontinue=&aplimit=max&format=json' => Http::response([
296+
'query' => ['allpages' => []],
297+
], 200),
298+
]);
299+
300+
$job = new PlatformStatsSummaryJob;
301+
302+
// This is a hack to override the `private` `PlatformStatsSummaryJob::mwHostResolver` property.
303+
// See https://www.php.net/manual/en/closure.call.php for more details on how this works.
304+
// TODO: figure out how to stub the `DatabaseManager` correctly and/or refactor the Job so that
305+
// we can more easily inject dependencies in the tests.
306+
(function ($resolver): void {
307+
$this->mwHostResolver = $resolver;
308+
})->call($job, $this->mockMwHostResolver);
309+
310+
$groups = $job->prepareStats([
311+
[
312+
'wiki' => 'thresholdtest.com',
313+
'edits' => 1,
314+
'pages' => 1,
315+
'users' => 1,
316+
'active_users' => 1,
317+
'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(
318+
$currentTime->subSeconds(config('wbstack.platform_summary_inactive_threshold'))
319+
),
320+
'first100UsingOauth' => '0',
321+
'platform_summary_version' => 'v1',
322+
],
323+
], [$wiki]);
324+
325+
$this->assertSame(1, $groups['edited_last_90_days']);
326+
$this->assertSame(0, $groups['not_edited_last_90_days']);
327+
}
277328
}

tests/Jobs/SendEmptyWikiNotificationsJobTest.php

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function testEmptyWikiNotificationsSendNotification() {
3838
Notification::fake();
3939
$user = User::factory()->create(['verified' => true]);
4040
$wiki = Wiki::factory()->create(['created_at' => $thresholdDaysAgo]);
41-
$manager = WikiManager::factory()->create(['wiki_id' => $wiki->id, 'user_id' => $user->id]);
41+
WikiManager::factory()->create(['wiki_id' => $wiki->id, 'user_id' => $user->id]);
4242
$wiki->wikiLifecycleEvents()->updateOrCreate(['first_edited' => null]);
4343

4444
$job = new SendEmptyWikiNotificationsJob;
@@ -50,6 +50,25 @@ public function testEmptyWikiNotificationsSendNotification() {
5050
);
5151
}
5252

53+
// empty wikis, that are almost old enough (29 days and 23 hrs)
54+
public function testEmptyWikiNotificationsNotificationNotSent() {
55+
$thresholdDaysAgo = Carbon::now()
56+
->subDays((config('wbstack.wiki_empty_notification_threshold') - 1))
57+
->subHours(23)
58+
->toDateTimeString();
59+
60+
Notification::fake();
61+
$user = User::factory()->create(['verified' => true]);
62+
$wiki = Wiki::factory()->create(['created_at' => $thresholdDaysAgo]);
63+
WikiManager::factory()->create(['wiki_id' => $wiki->id, 'user_id' => $user->id]);
64+
$wiki->wikiLifecycleEvents()->updateOrCreate(['first_edited' => null]);
65+
66+
$job = new SendEmptyWikiNotificationsJob;
67+
$job->handle();
68+
69+
Notification::assertNothingSent();
70+
}
71+
5372
// fresh wiki that does not have lifecycle event records yet
5473
public function testEmptyWikiNotificationsFreshWiki() {
5574
$now = Carbon::now()->toDateTimeString();

tests/Routes/Wiki/ConversionMetricTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,39 @@ public function testDownloadJson() {
134134
);
135135
}
136136

137+
public function testDownloadJsonTruncatesFractionalDayDiffs() {
138+
$currentDate = CarbonImmutable::now();
139+
$createdAt = $currentDate->subDays(200)->subHours(12); // 200.5 days ago
140+
$firstEditedAt = $createdAt->addDays(1)->addHours(12); // 1.5 days after
141+
$lastEditedAt = $currentDate->subDays(100); // 100 days ago
142+
143+
$wiki = Wiki::factory()->create([
144+
'domain' => 'fractional.days.cloud',
145+
'sitename' => 'Fractional Days Site',
146+
]);
147+
WikiSiteStats::factory()->create([
148+
'wiki_id' => $wiki->id,
149+
'pages' => 77,
150+
'activeusers' => 2,
151+
]);
152+
$wiki->created_at = $createdAt;
153+
$wiki->wikiLifecycleEvents()->updateOrCreate([
154+
'first_edited' => $firstEditedAt,
155+
'last_edited' => $lastEditedAt,
156+
]);
157+
$wiki->save();
158+
159+
$response = $this->getJson($this->route);
160+
161+
$response->assertStatus(200);
162+
$response->assertJsonFragment([
163+
'domain' => 'fractional.days.cloud',
164+
'time_to_engage_days' => 1,
165+
'time_before_wiki_abandoned_days' => 100,
166+
'number_of_active_editors' => 2,
167+
]);
168+
}
169+
137170
public function testFunctionalWithMissingLifecycleEventsandStats() {
138171
$wiki = Wiki::factory()->create([
139172
'domain' => 'very.new.wikibase.cloud', 'sitename' => 'bsite',

0 commit comments

Comments
 (0)