search

umbraco / Umbraco.Commerce.PaymentProviders.Worldpay

MIT License
0 stars 1 forks source link

Refactor for future development #8

Open PeterKvayt opened 2 months ago

PeterKvayt commented 2 months ago

Hi @umbracotrd!

As I can see you have merged my previous pr. But I did not confired it yet as you asked.

So here is new changes and you can fix problem with version.

Could you please increase minor version (to 10.1.0.0), so other developers should be aware to review changes before use new version.

Thanks.

umbracotrd commented 2 months ago

Hi @PeterKvayt , as I understand, this PR is just for refactoring code, right? I'm inclined to do this kind of things on v13 and above instead of v10. What do you think about this @mattbrailsford ?

PeterKvayt commented 2 months ago

Hi @umbracotrd !

Yea, this is just refactoring, In some places log level/messages were changed for better log history.

mattbrailsford commented 2 months ago

V13+ is cool with me. V10 is security fixes only now which this doesn’t look to fall into.

VictorTavkin commented 2 weeks ago

Hi everyone!

@mattbrailsford, @umbracotrd,

I would like to request your assistance in merging the proposed changes into v10. I completely understand that the version is currently in the support phase and it should not include any improvements and refactoring. @PeterKvayt and I have invested a significant amount of time in setting up the plugin, investigating/fixing the issues, and refactoring, but got the situation below.

We’ve recently gone live with Umbraco v10 and encountered a few issues related to webhooks and logging, such as: 1) Receiving two callbacks from Worldpay for a single order: one from MC_callbackurl and another from PaymentResponse (requires investigation). 2) An error entry appears in the log files when a user cancels an order (the fix in the current PR). 3) Probably, there will be other issues in the future.

So, taking into account, the information above, now, we are in a situation where we can't further contribute to the plugin in v10, because the current PR is missing in that version. Plus, we can't upgrade the website Umbraco version to v13+ to contribute to the version where the current PR is accepted. So we can't develop either v10 or v13+.

Additionally, given the current situation, two separate fixes are needed:

Thank you for your time and consideration.

umbracotrd commented 2 weeks ago

Hi @VictorTavkin and @PeterKvayt , I appreciate your effort to contribute to this package. It's always nice to have more support from community. Merging this PR for v10 seems fine for me, but it contains refactored code which makes it hard to cherry pick to v12, 13 and 14. Let me bring this up to the team and get back to you.

umbracotrd commented 2 weeks ago

Hi @VictorTavkin and @PeterKvayt , I've pushed version 10.1.0--preview.2+61ae4c6 to Umbraco Nightly Build feed. Could you confirm that it works as you expect?

We'll find a way to merge the changes up to newer versions later.

VictorTavkin commented 2 weeks ago

Hi, @mattbrailsford, @umbracotrd, Thank you for your help. We'll return once we've tested the latest version.

umbracotrd commented 5 days ago

Hi @VictorTavkin, how's it going with the 10.1.0 nightly build? Do you need more time to test it?

VictorTavkin commented 5 days ago

Hi @umbracotrd, We plan to deploy the nightly build to our staging environment this week. then we'll get back to you as soon as we finish testing.