Identified bug in renewal
By Weeblr on Wednesday, 06 October 2021
Posted in General Issues
Replies 16
Likes 0
Views 1K
Votes 0
Hi

A customer at our site is faced with a fatal error in Payplans each time they try to renew their subscription:



Call to undefined method stdClass::refresh()


I did the debugging again to find out the cause of the problem, and it's a bug in Payplans, in include/renewal/renewal.php

I was about to report all the details here but then I looked at my past tickets and I realized I HAD ALREADY DEBUGGED AND REPORTED THIS ISSUE in this ticket, a year ago.

I am really p...ed that you had a paying customer provide a full report on a serious bug and you did not fix it. That's really not up to Stackideas reputation.

Granted, the bug is slightly different now because you changed some other parts of the code, but I think you simply re-introduced it. Or it was just not tested again.

So the bug now is as follow:

- the description of the problem is the same as in this older ticket: last invoice for that customer has been refunded
- in administrator/components/com_payplans/includes/renewal/renewal.php at line 71, you call



 $order->createInvoice()


- in administrator/components/com_payplans/includes/order/order.php at line 394 you have:



$invoices = $this->getInvoices(array(PP_INVOICE_PAID, PP_INVOICE_REFUNDED));


At line 413 you do:



$invoice = $this->createChildInvoice($invoiceCount);


In createChildInvoices() you do:



$masterInvoice = $this->getInvoice($invoiceCount);


In getInvoice() you do:



$invoices = $this->getInvoices(PP_INVOICE_PAID);


and there is the problem, because in getInvoice(), you only include the paid invoices and not the refunded ones, so the invoice counter is incorrect and this causes a fatal error later on.

You do not need any access to our system, just re-create the problem your self by creating 3 subscriptions for a user, with the last one being refunded. Here is the description from my report a year ago:

- User purchased a sub in November 2018
- They renew in November 2019 (Paypal) but there's a problem somewhere and they pay twice
- We refund one of the invoice. User now has one Active sub and 3 invoices: 2018, 2019 paid and 2019 refunded
- Now they want to renew in 2020: go to dashboard, hit the Renew button and they get a Fatal error:



IMPORTANT: we are waiting for a fix. This customer cannot renew their subscription. The same sort of of issue happened last year. I cannot change the code or change data in the database because I don't know of the side effects. Can I just delete in the database the last refunded invoice? is there any side effect now or in the future (with the value of the counter for instance).

I am waiting for an urgent fix to this, considering I already reported this bug in details last year and no action was taken.


Yannick Gaultier
Hi,

Yes, we've checked on our side also, and the bug described by Yannick is confirmed.
Please advise.

Thanks.
Tristan
·
Wednesday, 06 October 2021 16:04
·
0 Likes
·
0 Votes
·
0 Comments
·
Hey Weeblr,

Sorry for the inconvenience caused to you.

Sometimes without admin details, we don't know the exact configuration at your site so might be that we will not able to replicable the same issue. This is why the site admin details we asked because there are so many cases that will create conflict, we can only investigate the issue on the site directly unless we can replicate the issue internally.

Finally, we are able to replicate the issue. I've already logged this issue into the issue tracker and we will include this fix in our coming release.

For now to fix this issue, find attached file and replace it with the below-mentioned file path.
root\administrator\components\com_payplans\includes\order\order.php

Can you give it a try and see if the issue still persists?
Manisha Ranawat
·
Wednesday, 06 October 2021 19:21
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi Manisha,

There never was any need for admin details. I explained that in my first message a year ago, providing every detail required to reproduce: you just had to create 2 subscriptions, one active and one refunded. And the problem appeared, all the times.

The change in that file does solve the specific problem.

However, I can see that Order::getInvoice() is called from many locations, I trust that you have double-checked and triple-checked the side effects or removing PP_INVOICE_PAID from the call to getInvoices() because it will changes the number and type of invoices returned.

For instance, onPayplansInvoiceAfterSave() (in /includes/event/events) is using this and now it will also get REFUNDED invoices instead of just paid ones.

Same in getMainInvoice() in /includes/invoice/invoice.php. Now this function can receive a refunded invoice instead of a paid one, that looks like a cause of problems.

Even createChildInvoice() in /includes/order/order.php will now possibly try to create a child invoice from a refunded invoice instead of a paid one. Does not sound good to me.

Please make sure you have all these examples controlled and checked before making any change to Payplans.

Best regards
Weeblr
·
Wednesday, 06 October 2021 19:40
·
0 Likes
·
0 Votes
·
0 Comments
·
Hey,

The change in that file does solve the specific problem.
Thank you for updating us on this and sounds good that it solves the issue.

However, I can see that Order::getInvoice() is called from many locations, I trust that you have double-checked and triple-checked the side effects or removing PP_INVOICE_PAID from the call to getInvoices() because it will changes the number and type of invoices returned.

For instance, onPayplansInvoiceAfterSave() (in /includes/event/events) is using this and now it will also get REFUNDED invoices instead of just paid ones.

Same in getMainInvoice() in /includes/invoice/invoice.php. Now this function can receive a refunded invoice instead of a paid one, that looks like a cause of problems.

Even createChildInvoice() in /includes/order/order.php will now possibly try to create a child invoice from a refunded invoice instead of a paid one. Does not sound good to me.

Please make sure you have all these examples controlled and checked before making any change to Payplans.
The fix will not affect the above-mentioned things.
This issue ticket is in the testing phase and we will check each and every scenario before releasing the fix to make sure that's it's not affecting other things.

Thank you for understanding
Manisha Ranawat
·
Wednesday, 06 October 2021 19:46
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi

The fix will not affect the above-mentioned things.
I am confused by your response here. The fix your propose will directly affect those instances because the caller will now received both PAID and REFUNDED invoices.

Previously, only PAID invoices would have been returned. This sounds like some hard to find bugs in the making.

Best regards
Weeblr
·
Wednesday, 06 October 2021 20:34
·
0 Likes
·
0 Votes
·
0 Comments
·
Hey Weeblr,

First of all, I would like to share with you that the PayPlans subscription will go to an "Inactive" state means no action will be taken till the subscription become active again once the invoice is refunded.

Base on what we test so far, the changes we add will not cause any side effects yet. In this function code, we only look for the invoice counter and the user will not be able to renew if the subscription state is "inactive" so it will not cause any issue.

However, I can see that Order::getInvoice() is called from many locations, I trust that you have double-checked and triple-checked the side effects or removing PP_INVOICE_PAID from the call to getInvoices() because it will changes the number and type of invoices returned.

If the subscription state is "inactive" means the system will not create any child invoice unless you activate the subscription again, so the system only applies the invoice counter and it will create any impact.

For instance, onPayplansInvoiceAfterSave() (in /includes/event/events) is using this and now it will also get REFUNDED invoices instead of just paid ones.

This event will get triggered whenever any invoice saves in the system and normally we use this event we check for invoice status also on which we need to work. It's not related to getting an invoice, it will call this when any invoice is created/updated.

Same in getMainInvoice() in /includes/invoice/invoice.php. Now this function can receive a refunded invoice instead of a paid one, that looks like a cause of problems.

I've cross-checked internally, so far not encounter any issue.

Even createChildInvoice() in /includes/order/order.php will now possibly try to create a child invoice from a refunded invoice instead of a paid one. Does not sound good to me.

As I explained above, if subscription status is "inactive" then no action will be taken, it will create an invoice only when the subscription is active/expired in renewal case. Also, we find invoices for getting the plan parameters saved on invoice params to create further invoices. So no issue with this.

By the way, we will test every different scenario first internally and make sure that it will not impact any process in the system before we release this in the next release version.

Thank you for understanding!
Manisha Ranawat
·
Thursday, 07 October 2021 14:27
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi

As I explained above, if subscription status is "inactive" then no action will be taken, it will create an invoice only when the subscription is active/expired in renewal case

Well, good example, because in this case (createChildInvoice()), you do not check for the status. So a child invoice will be created for the last invoice (ie with highest counter value) and there is no check whether that invoice is PP_INVOICE_PAID or PP_INVOICE_REFUNDED.
Before the change, the createChildInvoice() was sure to receive only a PP_INVOICE_PAID invoice because the check was made in getInvoice():



	public function getInvoice($counter = null)

	{

		$invoices = $this->getInvoices(PP_INVOICE_PAID);

                ...

but now this is removed and createChildInvoice does not check any more the invoice status.

This all needs a more thorough check I think.

Best regards
Weeblr
·
Thursday, 07 October 2021 18:10
·
0 Likes
·
0 Votes
·
0 Comments
·
Hey Weeblr,

Thank you for sharing your thoughts on this in a detailed explanation.
Yes, we will go through all the checks to make sure things are working fine.

Thank you for understanding!
Manisha Ranawat
·
Thursday, 07 October 2021 19:32
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi

yes, I hope you do, this is a fairly important part of the software and a tricky one, I'm sure.

I look forward to a prompt resolution of this issue.

Best regards
Weeblr
·
Thursday, 07 October 2021 23:33
·
0 Likes
·
0 Votes
·
0 Comments
·
Sure, will do.
Arlex Wong
·
Friday, 08 October 2021 11:47
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi

I see that Payplans 4.2.10 is out now but I don't see any trace of this bug fix in the changelog?

What's happening? did you fix the problem? can I have this customer renew now?
Weeblr
·
Tuesday, 26 October 2021 15:31
·
0 Likes
·
0 Votes
·
0 Comments
·
Yes, we already included the same fix in the current latest version.

We already tested this internally with a lot of different scenarios whether will cause any other issue or not, we don't found any issue so far after applying that fix.

You can give it a try update to the latest version of Payplans and see if you still experience this issue.
Arlex Wong
·
Tuesday, 26 October 2021 19:35
·
0 Likes
·
0 Votes
·
0 Comments
·
OK, thanks, I will do that.
Weeblr
·
Tuesday, 26 October 2021 19:56
·
0 Likes
·
0 Votes
·
0 Comments
·
You're most welcome.
Keep us updated if you will have any issues.

Thank you for understanding!
Manisha Ranawat
·
Tuesday, 26 October 2021 23:08
·
0 Likes
·
0 Votes
·
0 Comments
·
Ok, I had a client take up a monthly subscription on October 8th 2021
I was expecting the monthly subscription to be invoiced again on 8th if I am correct?
Also, would this update ensure that a client subscribed on 8th October is invoiced correctly for following month, seen as though the update for payplans came out afterthat date with a fix?
Thank you
Lee Currie
·
Tuesday, 09 November 2021 03:41
·
0 Likes
·
0 Votes
·
0 Comments
·
Hey Lee,

I was expecting the monthly subscription to be invoiced again on the 8th if I am correct?

Yes, If your subscription plan is monthly then the user will be charged every month when payment is expected.

Also, would this update ensure that a client subscribed on 8th October is invoiced correctly for following month, seen as though the update for payplans came out after that date with a fix?
Yes, Recurring subscriptions will work as they do, there is no issue.

Update us if you will have any issues.
Manisha Ranawat
·
Tuesday, 09 November 2021 11:23
·
0 Likes
·
0 Votes
·
0 Comments
·
View Full Post