-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[PM 29610]Update Account Storage Endpoint #6750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
ca40b3f
e0a1e26
a3e6679
56bf37d
d58c0da
505428e
c81faf1
cf6d7f7
77420e7
d7d080d
13117e1
3e2c981
89560de
bcb625c
ceee636
243a6c6
836fff6
6a5ccb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| using System.ComponentModel.DataAnnotations; | ||
|
|
||
| namespace Bit.Api.Billing.Models.Requests.Storage; | ||
|
|
||
| /// <summary> | ||
| /// Request model for updating storage allocation on a user's premium subscription. | ||
| /// Allows for both increasing and decreasing storage in an idempotent manner. | ||
| /// </summary> | ||
| public class StorageUpdateRequest : IValidatableObject | ||
| { | ||
| /// <summary> | ||
| /// The desired total storage in GB (including base storage). | ||
| /// Must be between the base storage amount and the maximum allowed (100 GB). | ||
| /// </summary> | ||
| [Required] | ||
| [Range(1, 100)] | ||
| public short StorageGb { get; set; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the user selects the total amount of storage they want in the web app. Rather, they submit how much additional storage they want to purchase. Please adjust this accordingly. |
||
|
|
||
| public IEnumerable<ValidationResult> Validate(ValidationContext validationContext) | ||
| { | ||
| if (StorageGb <= 0) | ||
| { | ||
| yield return new ValidationResult( | ||
| "Storage must be greater than 0 GB.", | ||
| new[] { nameof(StorageGb) }); | ||
| } | ||
|
|
||
| if (StorageGb > 100) | ||
| { | ||
| yield return new ValidationResult( | ||
| "Maximum storage is 100 GB.", | ||
| new[] { nameof(StorageGb) }); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| using Bit.Core.Billing.Commands; | ||
| using Bit.Core.Billing.Pricing; | ||
| using Bit.Core.Billing.Services; | ||
| using Bit.Core.Entities; | ||
| using Bit.Core.Services; | ||
| using Bit.Core.Utilities; | ||
| using Microsoft.Extensions.Logging; | ||
|
|
||
| namespace Bit.Core.Billing.Storage.Commands; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should live in the Premium folder and probably be named |
||
|
|
||
| /// <summary> | ||
| /// Updates the storage allocation for a premium user's subscription. | ||
| /// Handles both increases and decreases in storage in an idempotent manner. | ||
| /// </summary> | ||
| public interface IUpdateStorageCommand | ||
| { | ||
| /// <summary> | ||
| /// Updates the user's storage to the specified amount. | ||
| /// </summary> | ||
| /// <param name="user">The premium user whose storage should be updated.</param> | ||
| /// <param name="storageGb">The desired total storage amount in GB (must be between base storage and 100 GB).</param> | ||
| /// <returns>A billing command result with payment intent client secret if payment is required.</returns> | ||
| Task<BillingCommandResult<string?>> Run(User user, short storageGb); | ||
| } | ||
|
|
||
| public class UpdateStorageCommand( | ||
| IStripePaymentService paymentService, | ||
| IUserService userService, | ||
| IPricingClient pricingClient, | ||
| ILogger<UpdateStorageCommand> logger) | ||
| : BaseBillingCommand<UpdateStorageCommand>(logger), IUpdateStorageCommand | ||
| { | ||
| public Task<BillingCommandResult<string?>> Run(User user, short storageGb) => HandleAsync<string?>(async () => | ||
| { | ||
| if (user == null) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. User should never be null if it's not marked as nullable. |
||
| { | ||
| return new BadRequest("User not found."); | ||
| } | ||
|
|
||
| if (!user.Premium) | ||
| { | ||
| return new BadRequest("User does not have a premium subscription."); | ||
| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(user.GatewayCustomerId)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the GatewayCustomerId and GatewaySubscriptionId checks are redundant with checking if the user has premium. The user has to have a customer and subscription to have premium. |
||
| { | ||
| return new BadRequest("No payment method found."); | ||
| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(user.GatewaySubscriptionId)) | ||
| { | ||
| return new BadRequest("No subscription found."); | ||
| } | ||
|
|
||
| var premiumPlan = await pricingClient.GetAvailablePremiumPlan(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're going to be supporting users across multiple plans - you'll have to fetch all plans and find the one the user is on. |
||
| var baseStorageGb = (short)premiumPlan.Storage.Provided; | ||
|
|
||
| if (storageGb < baseStorageGb) | ||
| { | ||
| return new BadRequest($"Storage cannot be less than the base amount of {baseStorageGb} GB."); | ||
| } | ||
|
|
||
| if (storageGb > 100) | ||
| { | ||
| return new BadRequest("Maximum storage is 100 GB."); | ||
| } | ||
|
|
||
| // Check if the requested storage would fit the user's current usage | ||
| if (!user.MaxStorageGb.HasValue) | ||
| { | ||
| return new BadRequest("No access to storage."); | ||
| } | ||
|
|
||
| // Idempotency check: if user already has the requested storage, return success | ||
| if (user.MaxStorageGb == storageGb) | ||
| { | ||
| return (string?)null; // No payment intent needed for no-op | ||
| } | ||
|
|
||
| var remainingStorage = user.StorageBytesRemaining(storageGb); | ||
| if (remainingStorage < 0) | ||
| { | ||
| return new BadRequest( | ||
| $"You are currently using {CoreHelpers.ReadableBytesSize(user.Storage.GetValueOrDefault(0))} of storage. " + | ||
| "Delete some stored data first."); | ||
| } | ||
|
|
||
| // Calculate the additional storage beyond base | ||
| var additionalStorage = storageGb - baseStorageGb; | ||
|
|
||
| // Call the payment service to adjust the subscription | ||
| var paymentIntentClientSecret = await paymentService.AdjustStorageAsync( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is our chance to get away from the bloated and complicated All that would be required is that you update the premium subscription with correct storage amount and invoice a proration for it since storage is always billed annually. |
||
| user, | ||
| additionalStorage, | ||
| premiumPlan.Storage.StripePriceId); | ||
|
|
||
| // Update the user's max storage | ||
| user.MaxStorageGb = storageGb; | ||
| await userService.SaveUserAsync(user); | ||
|
|
||
| return paymentIntentClientSecret; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not used and not needed. |
||
| }); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This work needs to go behind a feature flag. Since it is part of the subscription page redesign, I suggest creating this flag which I'll have created for
clients: pm-29594-update-individual-subscription-pageWe can't remove this endpoint until that feature flag is removed or the old version of the subscription page would fail on adding storage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue still applies with applicable comments regarding FF on the command.