Fix role stacking issue

This commit is contained in:
DariusIII
2025-12-14 17:42:01 +01:00
parent 32431e60a4
commit ef711efd6c
2 changed files with 177 additions and 14 deletions
+31 -14
View File
@@ -588,10 +588,27 @@ class User extends Authenticatable
]);
if ($shouldStack) {
// Determine the effective start date for the stacked role
// If currentExpiryDate is in the future and different from oldExpiryDate, use currentExpiryDate
// This handles the case where a user already has a pending role stacking and the expiry was extended
$stackingStartDate = $currentExpiryDate;
if ($oldExpiryDate && $currentExpiryDate && $currentExpiryDate->isFuture() && $currentExpiryDate->gt($oldExpiryDate)) {
// Use the newer (current) expiry date as the stacking start date
$stackingStartDate = $currentExpiryDate;
Log::info('Using updated expiry date for stacking (expiry was extended)', [
'oldExpiryDate' => $oldExpiryDate->toDateTimeString(),
'currentExpiryDate' => $currentExpiryDate->toDateTimeString(),
'stackingStartDate' => $stackingStartDate->toDateTimeString()
]);
} elseif ($oldExpiryDate && $oldExpiryDate->isFuture()) {
// Use oldExpiryDate if it's still in the future (original behavior for backward compatibility)
$stackingStartDate = $oldExpiryDate;
}
Log::info('Stacking role change', [
'oldExpiryDate' => $oldExpiryDate->toDateTimeString(),
'oldExpiryDate' => $oldExpiryDate?->toDateTimeString(),
'currentExpiryDate' => $currentExpiryDate->toDateTimeString(),
'pendingRoleStartDate' => $oldExpiryDate->toDateTimeString()
'stackingStartDate' => $stackingStartDate->toDateTimeString()
]);
// Calculate a new expiry date for the pending role
@@ -609,12 +626,12 @@ class User extends Authenticatable
$totalDays = $baseDays + $promotionDays;
// New role will start at oldExpiryDate (original expiry date before any admin edits)
// New role will start at stackingStartDate (the most recent future expiry date)
// Then add the total days (base + promotion) to calculate when it will expire
$newExpiryDate = $oldExpiryDate->copy()->addDays($totalDays);
$newExpiryDate = $stackingStartDate->copy()->addDays($totalDays);
Log::info('Calculated new expiry for pending role', [
'pendingRoleStartDate' => $oldExpiryDate->toDateTimeString(),
'stackingStartDate' => $stackingStartDate->toDateTimeString(),
'baseDays' => $baseDays,
'promotionDays' => $promotionDays,
'totalDays' => $totalDays,
@@ -622,28 +639,28 @@ class User extends Authenticatable
]);
// Stack the role change - set it as pending
// Use oldExpiryDate (original expiry) for when the role will start
// Use stackingStartDate (most recent future expiry) for when the role will start
$user->update([
'pending_roles_id' => $roleQuery->id,
'pending_role_start_date' => $oldExpiryDate,
'pending_role_start_date' => $stackingStartDate,
]);
Log::info('Pending role updated', [
'pending_roles_id' => $roleQuery->id,
'pending_role_start_date' => $oldExpiryDate->toDateTimeString(),
'pending_role_start_date_formatted' => $oldExpiryDate->format('Y-m-d H:i:s')
'pending_role_start_date' => $stackingStartDate->toDateTimeString(),
'pending_role_start_date_formatted' => $stackingStartDate->format('Y-m-d H:i:s')
]);
// Record in history as a stacked change
// effective_date should match old_expiry_date (when the stacked role will start)
// effective_date should match stackingStartDate (when the stacked role will start)
try {
$history = UserRoleHistory::recordRoleChange(
userId: $user->id,
oldRoleId: $currentRoleId,
newRoleId: $roleQuery->id,
oldExpiryDate: $oldExpiryDate, // When current role expires
newExpiryDate: $newExpiryDate, // When the NEW role will expire (calculated from oldExpiryDate)
effectiveDate: $oldExpiryDate, // Same as old_expiry_date - when the new role starts
oldExpiryDate: $stackingStartDate, // When current role expires (using the most recent future date)
newExpiryDate: $newExpiryDate, // When the NEW role will expire (calculated from stackingStartDate)
effectiveDate: $stackingStartDate, // Same as old_expiry_date - when the new role starts
isStacked: true,
changeReason: 'stacked_role_change',
changedBy: $changedBy
@@ -654,7 +671,7 @@ class User extends Authenticatable
'effective_date' => $history->effective_date->toDateTimeString(),
'old_expiry_date' => $history->old_expiry_date?->toDateTimeString(),
'new_expiry_date' => $history->new_expiry_date?->toDateTimeString(),
'note' => 'effective_date equals old_expiry_date (when stacked role starts)'
'note' => 'effective_date equals stackingStartDate (when stacked role starts)'
]);
} catch (\Exception $e) {
Log::error('Failed to record role history', [
+146
View File
@@ -400,4 +400,150 @@ final class RoleUpgradeTest extends TestCase
);
Carbon::setTestNow();
}
/**
* Test that when a user has a role stacking and the expiry date is extended,
* subsequent role stackings use the new extended expiry date.
*
* Scenario:
* 1. User has Supporter role expiring on 2025-07-01
* 2. Admin extends the user's expiry to 2025-12-01
* 3. User purchases another Supporter subscription (stacking)
* 4. The stacked role should start from 2025-12-01, NOT 2025-07-01
*/
public function test_role_stacking_uses_updated_expiry_when_extended(): void
{
Carbon::setTestNow(Carbon::parse('2025-01-01 12:00:00'));
// Step 1: User has Supporter role expiring in 6 months (2025-07-01)
$originalExpiryDate = Carbon::parse('2025-07-01 12:00:00');
$this->user->update([
'roles_id' => $this->supporterRole->id,
'rolechangedate' => $originalExpiryDate,
]);
$this->user->syncRoles([$this->supporterRole->name]);
$this->user->refresh();
// Step 2: Admin extends the expiry to 2025-12-01
$extendedExpiryDate = Carbon::parse('2025-12-01 12:00:00');
$this->user->update([
'rolechangedate' => $extendedExpiryDate,
]);
$this->user->refresh();
// Verify the extended expiry is set
$this->assertEquals(
$extendedExpiryDate->toDateString(),
Carbon::parse($this->user->rolechangedate)->toDateString(),
'Expiry date should be extended to 2025-12-01'
);
$addYears = 1;
// Step 3: User purchases another Supporter 1 year subscription (stacking)
// The originalExpiryBeforeEdits simulates what the controller would pass
// (the expiry before the admin extended it)
$result = User::updateUserRole(
uid: $this->user->id,
role: 'Supporter',
applyPromotions: false,
stackRole: true,
changedBy: null,
originalExpiryBeforeEdits: $originalExpiryDate->toDateTimeString(), // Old value
addYears: $addYears
);
$this->assertTrue($result, 'updateUserRole should return true');
$this->user->refresh();
// Step 4: The stacked role should use the EXTENDED expiry date (2025-12-01)
// NOT the original expiry date (2025-07-01)
$this->assertNotNull($this->user->pending_role_start_date, 'Pending role start date should be set');
$pendingStartDate = Carbon::parse($this->user->pending_role_start_date);
// The pending role should start from the extended expiry (2025-12-01),
// not the original expiry (2025-07-01)
$this->assertEquals(
$extendedExpiryDate->toDateString(),
$pendingStartDate->toDateString(),
"BUG: Role stacking should use the extended expiry date (2025-12-01), not the original (2025-07-01). " .
"The pending_role_start_date was {$pendingStartDate->toDateString()}."
);
Carbon::setTestNow();
}
/**
* Test that role stacking correctly handles the case where currentExpiryDate is newer than oldExpiryDate.
* This tests the fix where the stacking start date should be the most recent future date.
*/
public function test_role_stacking_prefers_newer_expiry_date(): void
{
Carbon::setTestNow(Carbon::parse('2025-01-01 12:00:00'));
// Create another paid role for testing
$premiumRole = Role::firstOrCreate(
['name' => 'Premium'],
[
'guard_name' => 'web',
'addyears' => 2,
'apirequests' => 200,
'downloadrequests' => 100,
'defaultinvites' => 10,
'isdefault' => 0,
'donation' => 25,
'canpreview' => 1,
]
);
// User has Supporter role with expiry date that was extended
$currentExpiryDate = Carbon::parse('2026-01-01 12:00:00'); // Extended date
$this->user->update([
'roles_id' => $this->supporterRole->id,
'rolechangedate' => $currentExpiryDate,
]);
$this->user->syncRoles([$this->supporterRole->name]);
$this->user->refresh();
// Simulate that the original expiry was earlier (before extension)
$oldExpiryDate = Carbon::parse('2025-06-01 12:00:00');
$addYears = 2;
// Stack a Premium role upgrade
$result = User::updateUserRole(
uid: $this->user->id,
role: 'Premium',
applyPromotions: false,
stackRole: true,
changedBy: null,
originalExpiryBeforeEdits: $oldExpiryDate->toDateTimeString(),
addYears: $addYears
);
$this->assertTrue($result, 'updateUserRole should return true');
$this->user->refresh();
// Verify the pending role is set
$this->assertEquals($premiumRole->id, $this->user->pending_roles_id, 'Pending role should be Premium');
$this->assertNotNull($this->user->pending_role_start_date, 'Pending role start date should be set');
$pendingStartDate = Carbon::parse($this->user->pending_role_start_date);
// The stacking should use the NEWER (current) expiry date: 2026-01-01
// Not the older expiry date: 2025-06-01
$this->assertEquals(
$currentExpiryDate->toDateString(),
$pendingStartDate->toDateString(),
"Role stacking should use the newer expiry date (2026-01-01) when currentExpiryDate > oldExpiryDate. " .
"Got {$pendingStartDate->toDateString()} instead."
);
Carbon::setTestNow();
}
}