onaseef / yomobi

YoMobi Mobile Builder Platform
http://www.yomobi.com
2 stars 2 forks source link

Incorrect next payment date calculation for upgrade #406

Closed onaseef closed 12 years ago

onaseef commented 12 years ago

www.deelmob.com aatest29@mailinator.com 123123

In the site manager, select site jrtransport3

Click on Site Grade. NOtice the expiration date is Jul 10, 2013, since it was on an annual subscription.

Select Autopay Montly and make the payment. Notice the next payment date is set to Jul 15, 2012.

Since the custom site doesn't expire until Jul 10, 2013, the next payment shouldn't be until Jul 10, 2013.

For the purposes of custom sites, we should have the following variables in the database:

The logic for subscriptions should be as follows:

  1. All Sites start with the Default Values of: -- Manual-Upgrade-End-Date = 01/01/2012 -- Upgrade-End-Date = NULL -- Next-Payment-Date = NULL -- Subscription-End-Date = NULL -- Subscription-Prepay-Auth-Num = NULL
  2. IF Make-Payment THEN -- Subscription-Cancel -- Do-Subscribe
  3. Subscription-Cancel: -- Upgrade-End-Date = MAX (Manual-Upgrade-End-Date, Upgrade-End-Date) -- Next-Payment-Date = NULL -- Subscription-End-Date = NULL -- If Subscription-Prepay-Auth != NULL Send WePay a Cancel Subscription Request -- Subscription-Prepay-Auth = NULL
  4. Do-Subscribe -- Subscription-Start-Date = MAX (DateTime.NOW, Manual-Upgrade-End-Date, Upgrade-End-Date) -- Send Wepay a Start New Subscription Request with Subscription-Start-Date -- If Wepay Response == FAIL then Subscription-Cancel -- ELSE --- Subscription-Prepay-Auth = WePay Response --- Next-Payment-Date = (Subscription-Start-Date > DateTime.NOW) { Subscription-Start-Date } : { DateTime.NOW + [1 Month || 1 Year] } --- Subscription-End-Date = Subscription-Start-Date + Subscription Length --- Upgrade-End-Date = MAX (Manual-Upgrade-End-Date, Next-Payment-Date, DateTime.NOW)
gilbert commented 12 years ago

I'm still looking into this.

gilbert commented 12 years ago

Just fyi, this logic has already been in place for a while. Right now I'm setting up an automated testing environment for payments.

onaseef commented 12 years ago

Does that mean the problem is more fundamental that a logic error on our side?

I though the fix would be fairly simple for this scenario. What am I missing?

On Jul 17, 2012, at 1:24 PM, Gilbert reply@reply.github.com wrote:

Just fyi, this logic has already been in place for a while. Right now I'm setting up an automated testing environment for payments.


Reply to this email directly or view it on GitHub: https://github.com/onaseef/yomobi/issues/406#issuecomment-7044682

gilbert commented 12 years ago

Basically, there are too many scenarios to keep track of. The only way to confidently cover them all is with automated testing.

For example, fixing one bug might break a previous fix. Without automated testing, we'd have to run into the first situation again manually to discover it. With automated testing, we'd know immediately.

onaseef commented 12 years ago

Is the problem that the feedback from Wepay is asynchronous? Otherwise, the scenarios seem quite simple to me. If I can help with the logic, we can talk about it. I thought the logic I outlined covered all the scenarios. Am I missing some cases?

On Jul 17, 2012, at 2:07 PM, Gilbert wrote:

Basically, there are too many scenarios to keep track of. The only way to confidently cover them all is with automated testing.

For example, fixing one bug might break a previous fix. Without automated testing, we'd have to run into the first situation again manually to discover it. With automated testing, we'd know immediately.


Reply to this email directly or view it on GitHub: https://github.com/onaseef/yomobi/issues/406#issuecomment-7046016

gilbert commented 12 years ago

It's not about the logic, it's about programmatically defining what is correct through tests and ensuring the code passes all of them. The scenarios are simple enough, but many different scenarios combined together start to get messy. The only way to effectively ensure they are all correct after any logic change is through automated testing.

WePay's asynchronous aspect won't be a problem with proper tests; I'll be abstracting it out so I can emulate its behavior.

onaseef commented 12 years ago

Question: How long will the process take?

I know you are going to be busy starting next week and I'd hoped we have it out already, so I'm concerned about extending the process.

For the sake of collaboration, here's how I understand the logic. Is this the logic you have or are you doing something different?

The Site states are: 1: A site is standard 2: A site is custom with no subscription 3: A site is custom with a subscription

The variables we need are: premiumEndDate: NULL or DateTime subscription-id: NULL or Wepay-ID next-payment-date: NULL or DateTime subscription-end-date: NULL or DateTime

The actions are: Make Payment Cancel Subscription

Steps for Each Action:

def User-Make-Payment() { cancel-subscription(); var sub-start-date = premiumEndDate || DateTime.NOW; sub-end-date = sub-start-date + 5.years. var payment-term = MONTH || YEAR; next-payment-date = (sub-start-date > DateTime.Now) ? sub-start-date : sub-start-date + payment-term; subscription-id = Wepay.start(sub-start-date, payment-term) }

def User-Cancel-Subscription() { if (subscription-id == NULL) return;

WePay.cancel(subscription-id);
subscription-id = NULL;
next-payment-date = NULL;
subscription-end-date = NULL;

}

WepayListener-Payment-Made { premium-end-date = DateTime.NOW + payment-term; next-payment-date = premium-end-date; }

WepayListenter-Payment-Failed { Cancel-Subscription(); premium-end-date = premium-end-date - subscription-term;

}

Wepay-Listener-SubCancelled { subscription-id = NULL; next-payment-date = NULL; subscription-end-date = NULL;
}

On Jul 17, 2012, at 2:32 PM, Gilbert wrote:

It's not about the logic, it's about programmatically defining what is correct through tests and ensuring the code passes all of them. The scenarios are simple enough, but many different scenarios combined together start to get messy. The only way to effectively ensure they are all correct after any logic change is through automated testing.

WePay's asynchronous aspect won't be a problem with proper tests; I'll be abstracting it out so I can emulate its behavior.


Reply to this email directly or view it on GitHub: https://github.com/onaseef/yomobi/issues/406#issuecomment-7046706

gilbert commented 12 years ago

Believe me, automated testing is going to save a lot more time than it will take to set up.

For example, let's take that list of logic you just wrote. How do we determine its correctness? A list of input/output scenarios is a good way of doing this. One example:

If a monthly subscription is created, then cancelled, on a company with previously zero payments:

Similar scenarios can be created for companies with existing subscriptions, with previously failed payments, yearly subscriptions, etc. Surely we don't want to manually enter cc data into WePay for every scenario after every code change. With automated testing, all these scenarios can be testing with a single command.

onaseef commented 12 years ago

Automated testing is great. I just also want to make sure that the logic is correct. I guess my point is, please double check the logic to make sure it looks good, in addition to the automated testing.

My primary concern is that we aren't able to roll out before you get busy next week. These updates are vital so it's very important that we release them before the end of the month at the latest.

On Jul 17, 2012, at 3:15 PM, Gilbert wrote:

Believe me, automated testing is going to save a lot more time than it will take to set up.

For example, let's take that list of logic you just wrote. How do we determine its correctness? A list of input/output scenarios is a good way of doing this. One example:

If a monthly subscription is created, then cancelled, on a company with previously zero payments:
  • The company's expire date should equal to today plus one month
  • Re-subscribing should correctly change the expire date
  • Re-subscribing should correctly set the next charge date
  • etc.

Similar scenarios can be created for companies with existing subscriptions, with previously failed payments, yearly subscriptions, etc. Surely we don't want to manually enter cc data into WePay for every scenario after every code change. With automated testing, all these scenarios can be testing with a single command.


Reply to this email directly or view it on GitHub: https://github.com/onaseef/yomobi/issues/406#issuecomment-7047998

gilbert commented 12 years ago

Status update: As I'm writing tests, I'm discovering more bugs in the date logic. I'm pretty sure they are what caused this issue in the first place.

onaseef commented 12 years ago

Thank you for the update. When you say bugs in the date logic, do you mean bugs in the way we are calculating the dates?

gilbert commented 12 years ago

Yes, expire date calculation.

gilbert commented 12 years ago

Fixed. I feel a lot better about payments now that these tests are in place.

onaseef commented 12 years ago

Gilbert,

If you're available, we can push the changes to production and I can start doing some testing.

There are 4 sites that I need to manually set the upgrade expiration date on. What are the steps to do that once we push the changes? I presume I go into the private user database and fill in the expiration date there. Is that correct?

Also, are you confident that existing user information will be safe and uncorrupted in the event we have to roll back?

Also, a reminder to implement the batch task that checks the site states