What the developer wrote
Authored by Nicolas Dorier
Remove useless code
What changed, and why it matters
This commit is a small cleanup in the BTCPay Server subscriptions plugin. It removes redundant assignments to the PreviousPlan property (which is now set only through the constructor), makes a property read-only, fixes a test to wait for an event, and corrects a namespace reference. There is no indication this fixes a security vulnerability.
No security action required. Treat as routine refactoring/cleanup.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff shows four non-security changes in the Subscriptions plugin: (1) SubscriberModel.ScheduledPlan drops the nullable annotation; (2) a unit test now waits for SubscriptionEvent.PlanStarted before asserting the next phase; (3) PlanStarted.PreviousPlan becomes a read-only getter initialized via constructor; (4) SubscriptionHostedService removes duplicate PreviousPlan = … assignments and uses the constructor parameter instead, plus a minor namespace fix (SubscriberData.PhaseTypes.Normal -> PhaseTypes.Normal). The commit message simply says ‘Remove useless code’.
Changed components
BTCPayServer.Client/Models/Subscriptions/SubscriberModel.csBTCPayServer.Tests/SubscriptionTests.csBTCPayServer/Plugins/Subscriptions/SubscriptionEvent.csBTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.csInspect captured patch +9 / −13
diff --git a/BTCPayServer.Client/Models/Subscriptions/SubscriberModel.cs b/BTCPayServer.Client/Models/Subscriptions/SubscriberModel.cs
index 91528cb..8f530ea 100644
--- a/BTCPayServer.Client/Models/Subscriptions/SubscriberModel.cs
+++ b/BTCPayServer.Client/Models/Subscriptions/SubscriberModel.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
using Newtonsoft.Json;
using Newtonsoft.Json.Converters;
using Newtonsoft.Json.Linq;
@@ -21,7 +21,7 @@ public class SubscriberModel
[JsonConverter(typeof(NBitcoin.JsonConverters.DateTimeToUnixTimeConverter))]
public DateTimeOffset? GracePeriodEnd { get; set; }
- public OfferingPlanModel? ScheduledPlan { get; set; }
+ public OfferingPlanModel ScheduledPlan { get; set; }
[JsonConverter(typeof(NBitcoin.JsonConverters.DateTimeToUnixTimeConverter))]
public DateTimeOffset? ScheduledPlanActivatesAt { get; set; }
diff --git a/BTCPayServer.Tests/SubscriptionTests.cs b/BTCPayServer.Tests/SubscriptionTests.cs
index bca2b2e..6d46f31 100644
--- a/BTCPayServer.Tests/SubscriptionTests.cs
+++ b/BTCPayServer.Tests/SubscriptionTests.cs
@@ -336,8 +336,10 @@ public class SubscriptionTests(ITestOutputHelper testOutputHelper) : UnitTestBas
await portal.AssertScheduledChange("Pro Plan");
- await portal.GoToNextPhase(); // Normal to Grace period
- await portal.GoToNextPhase(); // Grace period to Expired
+ await s.Server.WaitForEvent<SubscriptionEvent.PlanStarted>(async () =>
+ {
+ await portal.GoToNextPhase(); // Normal to Grace period
+ });
await s.Page.ReloadAsync();
await portal.AssertPlan("Pro Plan");
diff --git a/BTCPayServer/Plugins/Subscriptions/SubscriptionEvent.cs b/BTCPayServer/Plugins/Subscriptions/SubscriptionEvent.cs
index ec1ce43..c0717a1 100644
--- a/BTCPayServer/Plugins/Subscriptions/SubscriptionEvent.cs
+++ b/BTCPayServer/Plugins/Subscriptions/SubscriptionEvent.cs
@@ -70,7 +70,7 @@ public class SubscriptionEvent
public class PlanStarted(SubscriberData subscriber, PlanData previous) : SubscriberEvent(subscriber)
{
- public PlanData PreviousPlan { get; set; } = previous;
+ public PlanData PreviousPlan { get; } = previous;
public bool AutoRenew { get; set; }
public override string ToString() => $"Subscriber {Subscriber.ToNiceString()} started plan";
}
diff --git a/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs b/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
index 7c35663..7806ae1 100644
--- a/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
+++ b/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
@@ -259,7 +259,6 @@ public class SubscriptionHostedService(
m.StartNextPlan(now);
subCtx.AddEvent(new SubscriptionEvent.PlanStarted(m, planBefore)
{
- PreviousPlan = planBefore,
AutoRenew = planBefore.Id == m.PlanId
});
}
@@ -272,13 +271,11 @@ public class SubscriptionHostedService(
if (newPhase is PhaseTypes.Expired or PhaseTypes.Grace && m is { NewPlan: not null, NewPlanId: not null } && m.NewPlanId != m.PlanId)
{
- var prevPlanId = m.PlanId;
var prevPlan = m.Plan;
(m.PlanId, m.Plan) = (m.NewPlanId, m.NewPlan);
(m.NewPlanId, m.NewPlan) = (null, null);
subCtx.AddEvent(new SubscriptionEvent.PlanStarted(m, prevPlan)
{
- PreviousPlan = prevPlan,
AutoRenew = false
});
}
@@ -504,10 +501,7 @@ public class SubscriptionHostedService(
}
}
if (checkout.PlanStarted)
- subCtx.AddEvent(new SubscriptionEvent.PlanStarted(sub, prevPlan)
- {
- PreviousPlan = prevPlan
- });
+ subCtx.AddEvent(new SubscriptionEvent.PlanStarted(sub, prevPlan));
}
record MoveTimeRequest(MemberSelector MemberSelector, TimeSpan Period);
@@ -667,7 +661,7 @@ public class SubscriptionHostedService(
if (planChangeRecord.Timing == PlanChangeData.ChangeTiming.AtPeriodEnd)
{
if (portal.Subscriber.PeriodEnd is not null && portal.Subscriber.PeriodEnd > DateTimeOffset.UtcNow
- && portal.Subscriber.Phase == SubscriberData.PhaseTypes.Normal)
+ && portal.Subscriber.Phase == PhaseTypes.Normal)
{
portal.Subscriber.NewPlanId = planId;
portal.Subscriber.NewPlan = plan;
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.