pretashop after paymment fix #19
Conversation
|
@RooseveltTech sorry for the late reply here. I'll review this today and share feedback asap |
|
Thank you |
|
@RooseveltTech just tested out an installation and checkout on Prestashop v8.2.1 and everything worked as expected 🎉 I'll go ahead and merge this now. I think we'll also need to update some of the documentation for the plugin, and indicate that it works with version 8.2 as well |
|
Thanks so much for your contribution @RooseveltTech, and sorry again for the delay in getting around to reviewing it |
|
Thank you so much @tolu-paystack |
|
@RooseveltTech I spoke too soon 😅. Just noticed a couple of things, I'll leave comments for you now |
| $this->name = 'paystack'; | ||
| $this->tab = 'payments_gateways'; | ||
| $this->version = '1.0.3'; | ||
| $this->version = '1.0.2'; |
There was a problem hiding this comment.
@RooseveltTech why the rollback of the version number here?
There was a problem hiding this comment.
You have two versions on the 1.7 1. on GitHub
2. on paystack website which is a zip file
I used the one on the paystack website to create the solution
There was a problem hiding this comment.
Can you share a link to the paystack website one? Do you mean the link on the help article?
| $this->version = '1.0.2'; | ||
| $this->ps_versions_compliancy = array('min' => '1.7', 'max' => _PS_VERSION_); | ||
| $this->author = 'Paystack'; | ||
| $this->author = 'Douglas Kendyson'; |
There was a problem hiding this comment.
@RooseveltTech did you mean to change this as well?
There was a problem hiding this comment.
If you check the zip file that exist on paystack website you can find it there
Although at some point I almost changed it to my name but I left it like that
There was a problem hiding this comment.
I think this is the official name that shows for the plugin, which is why we have it as Paystack (so that anyone installing it knows it's the officially supported one). Perhaps there's a way to add contributors as well, so that we can include your name (I'll check)
| $currency_order = new Currency($cart->id_currency); | ||
| if ($currency_order->iso_code == 'NGN' || $currency_order->iso_code == 'GHS' || $currency_order->iso_code == 'ZAR' || $currency_order->iso_code == 'USD' || $currency_order->iso_code == 'XOF' || $currency_order->iso_code == 'KES' || $currency_order->iso_code == 'EGP') { | ||
| return true; | ||
| if ($currency_order->iso_code == 'NGN') { |
There was a problem hiding this comment.
@RooseveltTech it looks like you've removed a bunch of the other currencies the plugin supports here
There was a problem hiding this comment.
Yeah. You can leave out EGP though
There was a problem hiding this comment.
@RooseveltTech I see you also hardcoded NGN as the currency and removed the custom metadata being passed when initializing transactions. Can we add that back please? We use it for tracking transactions via the plugin used on our end.
| $this->author = 'Douglas Kendyson'; | ||
| $this->controllers = array('payment', 'validation'); | ||
| $this->is_eu_compatible = 0; | ||
| $this->module_key = '7bd648045911885fe8a9a3c6f550d76e'; |
There was a problem hiding this comment.
@RooseveltTech I noticed you removed this as well. Was that intentional?
There was a problem hiding this comment.
alright
let me fix it and test
I would be back in a bit
When I was working on it
- I used the github file which we are reviewing on, it didn't work
- I check on the official paystack page for pretashop doc and i found this article https://support.paystack.com/en/articles/2126594 i'ts a zip file
- I tested with the zip file it still didn't work
- I checked if the zip file and github file were the same, they were not the same
- I made changes on the zip file, but I had to push the changes to the github repo
that's why its seem difference. I also don't know why the files are different
There was a problem hiding this comment.
Gotcha. Seems like some of the issues stem from the version of the code on our master branch here not matching what's on the help desk article. I think we should just have that point to this repo instead.
But please go ahead and fix and test your changes against what's in master here. Sorry for the confusion @RooseveltTech
tolu-paystack
left a comment
tolu-paystack
left a comment
There was a problem hiding this comment.
Hey @RooseveltTech I've left some comments in a few different places. Please review
2 participants
Closes #7
Closes #16
This PR helps to fix issues on