Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility aelia currency switcher #117

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

daigo75
Copy link

@daigo75 daigo75 commented Jun 22, 2021

I've been asked by my client to open a new pull request, since the PR sent to the WooCommerce Team was closed, probably by mistake, without a merge.

All Submissions:

  • Does your code follow the Extendables standards? I think so, but can't say, the link to the standards can't be reached.
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changes proposed in this Pull Request:

Integration with the Aelia Currency Switcher, for multi-currency support. We proposed the same integration a few months ago to Sau/Cal, but it hasn't been added to the Amazon Pay plugin, yet.

How to test the changes in this Pull Request:

  1. Install and configure the Aelia Currency Switcher (free copy available on request), enabling multiple currencies.
  2. Go to WooCommerce > Settings > Payments > Amazon Pay.
  3. Verify that the integration is active (see https://prnt.sc/10d2jg0).
  4. Enable Amazon Pay for some currencies in the gateway settings.
  5. Enable Amazon Pay for the same currencies at WooCommerce > Currency Switcher > Payment Gateways (see https://prnt.sc/10d2izo).
  6. Place an order in the enabled currencies, paying with Amazon. and verify that it's processed correctly.
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changelog entry

Added integration with the Aelia Currency Switcher, for multi-currency support.

Diego added 2 commits March 1, 2021 14:28
* Put the Aelia Currency Switcher at the top of the list, to ensure that it's detected even when WCML is installed.
@msaggiorato
Copy link
Member

Hello @daigo75 , can you point us to the PR that was closed by WooCommerce? Thanks

@daigo75
Copy link
Author

daigo75 commented Jun 22, 2021

Sure, it's woocommerce#86. I actually thought I added the link, but I forgot to do that.

Please be aware that I sent another PR to the WooCommerce repository a month ago, and it's still pending (no feedback at all).

@daigo75
Copy link
Author

daigo75 commented Dec 15, 2022

@kalessil @msaggiorato Reminder that this PR is still waiting to be merged. It's ready for that, you just need to run some quick tests with your latest payment plugin. We have quite a few clients who are waiting for this, therefore I'm sending this as a formal solicitation.

@amistry007
Copy link

+100 we need this!

@kate45623
Copy link

We need this feature. Thanks!

@antweb76
Copy link

Please please please do this, it's desperately needed

@caponica
Copy link

Would be great if this could be cleaned up and merged

@RalucaSbN
Copy link

Please release this feature asap as this is needed for multiple teams.
Thanks!

@daigo75
Copy link
Author

daigo75 commented Dec 16, 2022

Would be great if this could be cleaned up and merged

I think that the code could be merged almost as is. It worked fine when I wrote it, almost 18 months ago, now there's probably the need to address the conflict that arose due to the changes made to the files I had to update.

@AppleShane
Copy link

This feature is needed. Hoping you would make it available soon. Thanks!

@daigo75
Copy link
Author

daigo75 commented Dec 16, 2022

In reply to the person from the Amazon Pay Team, who explained their reasons for not proceeding with the integration:

  1. The integration would be difficult to review and test.
    Incorrect. The integration consists of three lines of code, spread across a much larger class which is 90% documentation. I wrote it 18 months ago, without knowing anything about the Amazon Pay plugin, and I find it hard to believe that its own authors would have a hard time managing that code.
  2. he integration could require a lot of maintenance in the future, because the Currency Switcher can break it.
    Nope. The Currency Switcher is one of the few plugins that never introduced breaking changes, in over 10 years. Once the integration has been implemented, it will work indefinitely. If, and only if, significant changes are ever required, I personally inform all the developers who wrote an integration to show them how to adapt their code, before any change becomes public.
  3. The Aelia Currency Switcher is not very popular and it doesn't warrant an integration.
    The Currency Switcher was the first true multi-currency plugin ever released for WooCommerce, and it remains popular, both due to its extensibility and the quality of our support service. For two whole years, between 2013 and 2015, it was also the only one, as other developers believed that multi-currency sites were going to be a fad. I'm the "pioneer" who introduced multi-currency sites, despite all the resistance, and I've been promoting the adoption of a "multi-currency mindset" for a decade. That's why the Aelia Currency Switcher not only keeps going strong, but more developers integrate with it on a regular basis. We have thousands of official installations, not to mention the "GPL Club" ones.

In conclusion, not only an integration would be easy to implement and maintain, it should actually be a priority.

@sheeSbN
Copy link

sheeSbN commented Dec 18, 2022

Please add this feature, it will be a great help. Thanks!

@ChristianAF
Copy link

Hey @amistry007 @kate45623 @antweb76 @sheeSbN @daigo75 @AppleShane @RalucaSbN @caponica,
Please redirect your request here: https://woocommerce.com/feature-requests/pay-with-amazon/. The decision is made by WooCommerce in addition to the Amazon team.
Thanks!

@daigo75
Copy link
Author

daigo75 commented Dec 20, 2022

@ChristianAF unfortunately, I can't use the page you indicated. No matter how I try to log in, I keep receiving an error.

If anyone else is able to file the feature request, I would like to ask them to add the following information to it:

  1. A link to this conversation.
  2. The information that I already wrote the integration myself 18 months ago, and it works. It's just a matter of merging it.
  3. The fact that the integration will be low maintenance and there's basically no risk of it breaking due to changes in the Currency Switcher.

In addition to the above, I'm one of the most responsive developers in the WooCommerce ecosystem, answering questions seven days a week, therefore any developer who has a question just has to reach out to me. 😁

@ChristianAF
Copy link

Hey @amistry007 @kate45623 @antweb76 @sheeSbN @daigo75 @AppleShane @RalucaSbN @caponica, you can add your request here woocommerce#102
Thanks

@amistry007
Copy link

I have raised the feature request on the WooCommerce site. When submitted it said the feature was awaiting approval before it will be available to vote on. WooCommerce team confirmed they work collaboratively with the developer to prioritise the features based on votes.

@daigo75
Copy link
Author

daigo75 commented Jan 2, 2023

Thanks @amistry007. Since the work has already been done, the collaboration should take no time at all. 😄

@amistry007
Copy link

@ChristianAF I have raised the feature request to WooCommerce over a week ago but still it is not showing as approved to be voted on. Can you confirm what the delay on this is?

Following is extract from woocommerce support...

Hi there Ashok, Thanks for contacting WooCommerce.com support. I understand you submitted a feature request for Amazon Pay and this has not been approved or made public to be voted on yet. We do have a number of pending feature requests and each must be approved before being published, and we can't presently provide an ETA on when a request will be approved. If we can help with anything else, please do let us know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants