Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.4k stars 1.98k forks source link

Billing: Homepage Banner Shows for Sites that Autorenew #52180

Closed ccwalburn closed 3 years ago

ccwalburn commented 3 years ago

Steps to reproduce the behavior

This message appears on the dashboard even for users who have autorenew on:

screenshot Image URL: https://d.pr/i/5cxFk9

What I expected to happen

I wouldn't have expected to see a message telling the user that they need to renew, but instead maybe a message telling them that it was about to renew.

What actually happened

There is a message telling the user to renew.

3926308-zen

@shilling

DavidRothstein commented 3 years ago

I think the issue is that it's a monthly plan. When they added this message in https://github.com/Automattic/wp-calypso/pull/47375 they seem to have intentionally made it appear within 30 days (which probably makes sense for annual plans that already would have tried auto-renewing by then, and therefore would have already had a failed auto-renewal) but that doesn't make sense for monthly plans.

I'm adding this to the Shilling backlog for now, but we should probably pass this on to whatever team works on it.

michaeldcain commented 3 years ago

@kwight: is this something that y'all have the bandwidth to address?

kwight commented 3 years ago

@michaeldcain It should be straightforward to add a ! plan_will_renew() type check, but I'm having a hard time finding where I can get that info, given the plan or subscription – any hints?

shilling commented 3 years ago

Why am I tagged on this? I have no idea what you guys are talking about.

On Thu, Apr 22, 2021 at 1:04 PM Kirk Wight @.***> wrote:

@michaeldcain https://github.com/michaeldcain It should be straightforward to add a ! plan_will_renew() type check, but I'm having a hard time finding where I can get that info, given the plan or subscription – any hints?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Automattic/wp-calypso/issues/52180#issuecomment-825069556, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFDOONYSP6FKFEOJ6S5ANTTKBQKPANCNFSM43LFXKRA .

-- Thanks, Nadine

“Art enables us to find ourselves and lose ourselves at the same time.” -- Thomas Merton View my artwork: https://charcoalatte.com

kwight commented 3 years ago

Sorry @shilling , you happen to have the username of an internal team of ours – apologies 🙃

shilling commented 3 years ago

No worries!

On Thu, Apr 22, 2021 at 3:22 PM Kirk Wight @.***> wrote:

Sorry @shilling https://github.com/shilling , you happen to have the username of an internal team of ours – apologies 🙃

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Automattic/wp-calypso/issues/52180#issuecomment-825159391, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFDOOLQ3R2DF2URCPXXQ7DTKCARTANCNFSM43LFXKRA .

-- Thanks, Nadine

“Art enables us to find ourselves and lose ourselves at the same time.” -- Thomas Merton View my artwork: https://charcoalatte.com

michaeldcain commented 3 years ago

@kwight as @DavidRothstein mentions, the 30-day check isn't necessarily the issue (it's actually probably the right move, given that they either don't have auto-renew on, or something has failed with their renewal). I think just hiding this message for monthly plans (or moving the time check to < 7 days before expiration for monthly plans). There are probably some utilities to check for monthly plans in @sirbrillig's new calypso-products package.

obenland commented 3 years ago

I'm adding this to the Shilling backlog for now, but we should probably pass this on to whatever team works on it.

@simison This sounds like something that fits the My Home pod agenda?

simison commented 3 years ago

I think this would be good for My home pod, yeah. 👍

That said, I'd really welcome it if the team that normally would own this (Martech, Shilling?) tackles it this week because I bet you'd have way better context to make the right choices quicker. I.e. don't just punt because you can punt, but if you do, no worries. You folks likely anyway need to think about the right solution that the pod team just implements then. Does that make sense? 😅 cc @autumnfjeld

I remember talking about this "expires soon" problem with @paulbonahora and @michaeldcain and that it got fixed, but this is a different problem, then?

Slack convo: p1614084130027200-slack-mp-wpcom-pricing-and-packaging Fix was in D58277-code

michaeldcain commented 3 years ago

As @simison pointed out in the comment above, we (I) already fixed this banner for monthly plans. I missed it before, but in the initial screenshot, the user is 5 days from expiration, which means that either the renewal has failed or auto-renew is on but won't succeed (i.e. a purchase made with credits).

I just double-checked with a monthly plan.

> 7 days until expiration:

Screen Shot 2021-04-27 at 11 28 12 AM

< 7 days until expiration (renewal failed or not auto-renewing):

Screen Shot 2021-04-27 at 11 31 00 AM

I'm going to close this issue and mark it as invalid. Sorry for all the back and forth.

DavidRothstein commented 3 years ago

We discussed it more and need to reopen this; renewal attempts for monthly subscriptions start on the day of expiration, so (at least for a subscription with auto-renew on) it's still a bug to show this starting 7 days before expiration (there haven't been renewal attempts yet).

Probably the "pre-expiration" version of the banner (shown in the above screenshots) should never display for monthly subscriptions at all, or at least never display if auto-renew is on. But the "post-expiration" version still can.

michaeldcain commented 3 years ago

Patch in D60688-code

michaeldcain commented 3 years ago

@ccwalburn this should be fixed now that the patch above has been deployed. Let me know if you see any more issues.