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

Wrong default parameter with new installation #92

Closed
Donach opened this issue Feb 8, 2024 · 15 comments
Closed

Wrong default parameter with new installation #92

Donach opened this issue Feb 8, 2024 · 15 comments
Assignees
Labels
bug Something isn't working

Comments

@Donach
Copy link

Donach commented Feb 8, 2024

Describe the bug
When I installed the BTCpay module, out of the box, I was having issue, where the code apparently tried to submit array into bool argument when creating the Configuration object.

To Reproduce
Steps to reproduce the behavior:

  1. Have a new PrestaShop 8.1.3
  2. Add BTCPay module
  3. Click configure
  4. See Error

Expected behavior
Configuration tab shows normally.

Screenshots
image

Your BTCPay Environment (please complete the following information):

  • BTCPay Server Version 1.12.4 (irrelevant for this issue)
  • Deployment Method: Presta - Docker, BTCPay - Start9
  • Browser Firefox

Logs (if applicable)
See image

Setup Parameters
Default

Additional context
It can be fixed by replacing a line in constants:
public const CONFIGURATION_SHARE_METADATA = 'BTCPAY_SHARE_METADATA';
with:
public const CONFIGURATION_SHARE_METADATA = false;

@BitcoinMitchell
Copy link
Collaborator

Thank you for the issue report! Will look into this tonight as I'm currently unable to test anything.

@BitcoinMitchell BitcoinMitchell self-assigned this Feb 8, 2024
@BitcoinMitchell BitcoinMitchell added the bug Something isn't working label Feb 8, 2024
@Donach
Copy link
Author

Donach commented Feb 8, 2024

On testing some more, Even though I'm able to "see" settings, I Cannot save, new error pops up:
image

@BitcoinMitchell
Copy link
Collaborator

BitcoinMitchell commented Feb 8, 2024

On testing some more, Even though I'm able to "see" settings, I Cannot save, new error pops up:

Can I assume that you used valid data for the form? If so, not great.

@Donach
Copy link
Author

Donach commented Feb 8, 2024

On testing some more, Even though I'm able to "see" settings, I Cannot save, new error pops up:

Can I assume that you used valid data for the form? If so, not great.

Yes, the problem is with my change, it botches later on on here:
image

So I need to figure out how to fix the default values - seems like it uses some wrong reference to an array instead of the value inside the array - since I'm not PHP coder, I cannot really see the issue myself...

@BitcoinMitchell
Copy link
Collaborator

No worries, I'll do a deep dive and resolve it tonight. I'm already insanely happy with the thorough issue report you've given. Thanks again for that.

@Donach
Copy link
Author

Donach commented Feb 8, 2024

After disabling this piece of code (line 54-56) (I assumed that it should not matter, if it's bool value only, and I have it fixed to "false"), I get another issue at checkout, just as I'm about to enter into "Shipping" (so even before going to choose the payment method).
The error is long AF:
error.log

EDIT: That's probably as far as I can get, hope it was/is helpful. Let me know if you'd have some hotfix I can test - my eshop is not online yet, so I'm not in a hurry, but I really would love to start it with BTCPay-enabled sometime this quarter :-)

@BitcoinMitchell
Copy link
Collaborator

BitcoinMitchell commented Feb 8, 2024

I've tried to recreate the issue, but I have not yet been able to. I've tested the flow using the latest docker-compose in this MR. This way I don't need to sign up/pay for cloud hosting.

Steps taken

  • Used the docker-compose to spin up BTCPay Server + PrestaShop.
  • Created a brand new empty store in PrestaShop and BTCPay Server.
  • Logged in and enabled the debug mode.
  • Added the module to the PrestaShop and clicked configure, no errors.
  • Setup the connection to BTCPay Server, no problems.
  • Tried to do an order, all went as expected.

Questions

  • Is there anything else you can give me, info wise, like PHP version of your host?
  • You are using the latest PrestaShop + module version?
  • Does using the reset option on the module resolve your issues?
    • It resets the modules configuration, so there should be a sane default again.

I'll keep digging as well. I can add some "hacks" (like casting the value to a bool, no matter what), but I rather find the root cause.

Background information

The flow when installing and going to configure is that we store a sane default during installation (in this case, false).

The form then calls the FormDataProvider, specifically getData() which calls Configuration::create($this->configuration) where $this->configuration is a reference to the Prestashop config storage stuff.

Create takes the information needed and tries to construct the Configuration-model. As you can see, it does everything well, except the $shareMetadata.

That said, the fact it gives an '' for the API key is a bit weird, since it's default value is null not an empty string.

It's all very weird and confusing. It clearly doesn't work for you, but I've tried three times and I cannot reproduce it.

Side-note: CONFIGURATION_SHARE_METADATA is a simple constant that contains the name of the configuration value, which is then used to get the saved data from Prestashop. Changing that to false will indeed break things.

@Donach
Copy link
Author

Donach commented Feb 9, 2024

Okay, could be my installatino is somehow botched.
These are the versions:
image

Presta: 8.1.3
BTC Module: 6.0.2

Trying to do "reset" shows error message - I'll try to reinstall my presta, and import the configs and products and try again, will report back then...

@BitcoinMitchell
Copy link
Collaborator

If you will be re-using the same database, you might have to remove configuration there as well (or make sure to remove the module before you re-install everything). Otherwise the broken configuration value will still be there.

As a quick fix you can also try the ZIP in this MR, which casts the value we get from the configuration for shareMetadata to always be a bool.


Thank you for providing the versions. I'll try again and test it against your exact versions (module and presta do look up-to-date).

@Donach
Copy link
Author

Donach commented Feb 9, 2024

If you will be re-using the same database, you might have to remove configuration there as well (or make sure to remove the module before you re-install everything). Otherwise the broken configuration value will still be there.

As a quick fix you can also try the ZIP in this MR, which casts the value we get from the configuration for shareMetadata to always be a bool.

Thank you for providing the versions. I'll try again and test it against your exact versions (module and presta do look up-to-date).

Quickly tested on fresh install, without any other config, new DB all clean - and I'm still having issues.
The configure now works to get opened, but when I wanna save configuration, I get some syntax error:
image

Once I got error that server is not responding, but other times it's just this...

EDIT: Lol, so after just trying to save the same config like 10 times, it eventually got saved :-D... Might be my BTCpay bisheving?
Anyway, will try to run an order and will update this comment.

EDIT2: After the same issue of the big JSON error, I simply refreshed the screen and went through with order.
However, the BTCpay option is missing from the payment options:
image

Although I'm quite sure it's well set and enabled:
image

(Sorry for Czech, I can reupload with EN translation later or tomorrow)

@BitcoinMitchell
Copy link
Collaborator

BitcoinMitchell commented Feb 9, 2024

Is there anything in your PrestaShop/BTCPayServer logs that might explain what went wrong? For PrestaShop logs you can go to: Advanced Settings -> Logs.


EDIT: Lol, so after just trying to save the same config like 10 times, it eventually got saved :-D... Might be my BTCpay bisheving?

Maybe. I just can't think of what could go wrong like that. Having said that, this would be a separate issue from this issue, which is nice for me (but still sucks for you).

@Donach
Copy link
Author

Donach commented Feb 9, 2024

Is there anything in your PrestaShop/BTCPayServer logs that might explain what went wrong? For PrestaShop logs you can go to: Advanced Settings -> Logs.

EDIT: Lol, so after just trying to save the same config like 10 times, it eventually got saved :-D... Might be my BTCpay bisheving?

Maybe. I just can't think of what could go wrong like that. Having said that, this would be a separate issue from this issue, which is nice for me (but still sucks for you).

I get plenty of seemingly same error:
image

After closer look, it might also simply be the case, that BTCPay is not really fond of the way how I set it up with reverse proxy over SSH tunnel to my Start9...
I'll try different setup over the weekend, and let you know if I still have same issue - but it could simply be connected to the connectivity issue problem...

@BitcoinMitchell
Copy link
Collaborator

I get plenty of seemingly same error:

image

Reading the error, that does look like nothing has been received by the plugin. the first argument in json_decode is an empty string, which suggests to me that no data has been received.


However, the BTCpay option is missing from the payment options:

That would be because the plugin could not reach your BTCPay Server. The option is only available if it can reach the server and your node is fully synced (to prevent payment issues).

@Donach
Copy link
Author

Donach commented Feb 12, 2024

Hey, I've tested your latest MR 6.0.3, and it seems to work now.
Funily though, when clicking the "Configure" button, I still get some weird error.
But when going through the left menu bar, and clicking directly on BTCPay under "Payments" category, it worked just fine...
So with that, I guess this issue can be closed - but it would be nice if it was possible to add some notification saying that if BTCpay communication fails, you will not see it under payments for example, I was not aware of that...
Thanks a lot for prompt replies and taking look into it! @BitcoinMitchell

@BitcoinMitchell
Copy link
Collaborator

BitcoinMitchell commented Feb 12, 2024

be nice if it was possible to add some notification saying that if BTCpay communication fails, you will not see it under payments for example, I was not aware of that...

Will do! I am working on improving the configuration screen as well speak. I'll also update the README.md and/or the documentation on the BTCPay Server website.

Thank you for your report and feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants