Skip to content

Conversation

@aktky
Copy link
Contributor

@aktky aktky commented May 11, 2025

No description provided.

Copilot AI review requested due to automatic review settings May 11, 2025 10:32

This comment was marked as outdated.

@aktky aktky requested review from Copilot and removed request for Copilot August 14, 2025 06:47
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements subscription and billing functionality for a self-hosted portal application. The changes add support for trial subscriptions, paid plans, Stripe integration, and user account deletion scheduling.

Key changes:

  • Adds subscription management with trial periods, paid plans, and status tracking
  • Integrates Stripe for payment processing and billing portal access
  • Implements user account scheduled deletion for expired subscriptions
  • Creates separate license validation server endpoint

Reviewed Changes

Copilot reviewed 57 out of 59 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
nginx.dev.conf Updates backend port from 8080 to 8081
migrations/000001_initial_schema.up.sql Adds plan, subscription tables and user scheduled deletion field
internal/server/subscription.go New subscription management endpoints
internal/server/stripe.go New Stripe integration for payments and webhooks
internal/server/server.go Splits handlers into onprem portal and license endpoints
internal/postgres/subscription.go New subscription database operations
frontend/src/routes/billing/index.tsx New billing/pricing page component
cmd/license/main.go New separate license validation server
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)

internal/server/stripe.go:116

  • Missing validation to ensure sub.SeatCount is positive before passing to Stripe. Zero or negative seat counts could cause issues with Stripe subscription items.
	sigHeader := r.Header.Get("Stripe-Signature")

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

LineItems: []*stripe.CheckoutSessionLineItemParams{
{
Price: stripe.String(plan.StripePriceID),
Quantity: stripe.Int64(max(sub.SeatCount, 1)),
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The max function is used but not imported. This will cause a compilation error in Go versions prior to 1.21. Consider using an explicit comparison or import the cmp package.

Suggested change
Quantity: stripe.Int64(max(sub.SeatCount, 1)),
Quantity: stripe.Int64(maxInt64(sub.SeatCount, 1)),

Copilot uses AI. Check for mistakes.
if err := json.Unmarshal(event.Data.Raw, &subObj); err != nil {
break
}
ctx := r.Context()
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing null check for subObj.Items.Data[0].Price. If the Price field is nil, this will cause a panic when accessing the ID field.

Suggested change
ctx := r.Context()
ctx := r.Context()
if subObj.Items == nil || subObj.Items.Data == nil || len(subObj.Items.Data) == 0 || subObj.Items.Data[0].Price == nil {
break
}

Copilot uses AI. Check for mistakes.
if err := json.Unmarshal(event.Data.Raw, &subObj); err != nil {
break
}
if subObj.Items == nil || subObj.Items.Data == nil || len(subObj.Items.Data) == 0 {
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This validation pattern is repeated in multiple places. Consider extracting it into a helper function like validateSubscriptionItems(subObj *stripe.Subscription) bool to reduce code duplication.

Suggested change
if subObj.Items == nil || subObj.Items.Data == nil || len(subObj.Items.Data) == 0 {
if !validateSubscriptionItems(&subObj) {

Copilot uses AI. Check for mistakes.

INSERT INTO "plan" ("id", "name", "price", "stripe_price_id") VALUES
(gen_random_uuid(), 'Team', 10, 'price_1RNXs507QE3crwMYYSFnC4WZ'),
(gen_random_uuid(), 'Business', 24, 'price_1RNXz907QE3crwMY2ZlTpV5R');
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coded Stripe price IDs in database migrations can cause issues if these IDs don't exist in the target Stripe environment. Consider making these configurable or documenting the requirement to update these values.

Suggested change
(gen_random_uuid(), 'Business', 24, 'price_1RNXz907QE3crwMY2ZlTpV5R');
-- IMPORTANT: Update the 'stripe_price_id' values below after creating corresponding prices in your Stripe account.
-- Use the actual Stripe price IDs for your environment. Placeholder values are used here to ensure migration compatibility.
INSERT INTO "plan" ("id", "name", "price", "stripe_price_id") VALUES
(gen_random_uuid(), 'Team', 10, 'REPLACE_ME_STRIPE_PRICE_ID_TEAM'),
(gen_random_uuid(), 'Business', 24, 'REPLACE_ME_STRIPE_PRICE_ID_BUSINESS');

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants