vercel / nextjs-subscription-payments

Clone, deploy, and fully customize a SaaS subscription application with Next.js.
https://subscription-payments.vercel.app/
MIT License
6.24k stars 1.27k forks source link

[bug] Subscribe a second time with a different plan causes "You are not currently subscribed to any plan." #366

Open XiChenn opened 1 month ago

XiChenn commented 1 month ago

To reproduce:

k-thornton commented 3 weeks ago

I also hit this early on in this template. It's due to the maybeSingle() in the query that gets the active subscriptions

https://github.com/vercel/nextjs-subscription-payments/blob/25a19504c85a2353f23e907521d0cdb1979c66d6/utils/supabase/queries.ts#L11-L16

0 subscriptions is ok, 1 subscription is ok, but >1 subscriptions throws an error and returns null. It's not terribly difficult to handle this by customizing the template, but I agree this was the biggest earliest roadblock I hit when using it too.

Veeeetzzzz commented 2 weeks ago

+1 to having the same issue - it's what was said above but also, by default when a subscription ends it gets removed from your Stripe dashboard, but the subscriptions table doesn't delete the subscription record. It just changes the status to cancelled

image

I thought of two possible solutions and you can use either or both depending on your preference.

  const { data: subscription, error } = await supabase
    .from('subscriptions')
    .select('*, prices(*, products(*))')
    .eq('user_id', user.id)  // Add this line to filter by user_id
    .in('status', ['trialing', 'active'])
    .maybeSingle();

  if (error) {
    console.error('Error fetching subscription:', error);
    throw error;
  }

  console.log('Fetched subscription:', subscription);
  return subscription;
});
  1. In Supabase set up a query to see all your cancelled subscriptions (i.e everyone who shouldn't have access)

SELECT
  *
FROM
  subscriptions
WHERE
  status NOT IN ('trialing', 'active');

Replace with the below to get rid of any cancelled subscriptions. Run manually or set up a job using pg_cron

DELETE FROM subscriptions
WHERE
  status NOT IN ('trialing', 'active');
gbopola commented 1 day ago

I think it's best to just create some logic that will cancel any current subscription and replace with the new one. That way you will always have just one subscription and this error won't persist.

k-thornton commented 23 hours ago
  • Modify utils/supabase/queries.ts with a new query that filters by user ID & then use it to determine if there's a valid sub.
  const { data: subscription, error } = await supabase
    .from('subscriptions')
    .select('*, prices(*, products(*))')
    .eq('user_id', user.id)  // Add this line to filter by user_id
    .in('status', ['trialing', 'active'])
    .maybeSingle();

  if (error) {
    console.error('Error fetching subscription:', error);
    throw error;
  }

  console.log('Fetched subscription:', subscription);
  return subscription;
});

This change actually shouldn't have any effect, because row-level security is set up on this table to only allow people to see their own entries, and no others. This means the .eq('user_id', user.id) is implicit and always happens.

Similarly, the record changing to "canceled" status will also remove it from the query (since it's filtered on ['trialing','active'], which means deletion should be unnecessary. The .maybeSingle() only cares about the outcome of the query, so as long as a given user doesn't have more than 1 subscription in 'trialing' or 'active' state, then it won't error out.

k-thornton commented 23 hours ago

I'd say the closest thing to a bug here is that there's no guard at all around allowing the user to trigger a Stripe checkout when they already have an active subscription.

Because the template breaks in a couple of places when this state occurs, some guard code should be put here to catch that state and return a getErrorRedirect instead https://github.com/vercel/nextjs-subscription-payments/blob/1623fb9d1820637ba55cd0af1012bb85237118b8/components/ui/Pricing/Pricing.tsx#L52

k-thornton commented 23 hours ago

posted a PR for how I'd fix this

gbopola commented 11 hours ago
  • Modify utils/supabase/queries.ts with a new query that filters by user ID & then use it to determine if there's a valid sub.
  const { data: subscription, error } = await supabase
    .from('subscriptions')
    .select('*, prices(*, products(*))')
    .eq('user_id', user.id)  // Add this line to filter by user_id
    .in('status', ['trialing', 'active'])
    .maybeSingle();

  if (error) {
    console.error('Error fetching subscription:', error);
    throw error;
  }

  console.log('Fetched subscription:', subscription);
  return subscription;
});

This change actually shouldn't have any effect, because row-level security is set up on this table to only allow people to see their own entries, and no others. This means the .eq('user_id', user.id) is implicit and always happens.

Similarly, the record changing to "canceled" status will also remove it from the query (since it's filtered on ['trialing','active'], which means deletion should be unnecessary. The .maybeSingle() only cares about the outcome of the query, so as long as a given user doesn't have more than 1 subscription in 'trialing' or 'active' state, then it won't error out.

Oh I see what you mean. Makes more sense.