diff --git a/app/Models/User.php b/app/Models/User.php index 6e5a1d8b4..9a52c2938 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -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', [ diff --git a/tests/Feature/RoleUpgradeTest.php b/tests/Feature/RoleUpgradeTest.php index c3c5d8ce1..344c82557 100644 --- a/tests/Feature/RoleUpgradeTest.php +++ b/tests/Feature/RoleUpgradeTest.php @@ -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(); + } }