coarchy / stripe

Stripe Component for Moqui based on Coarchy code written by Michael Jones
Other
0 stars 0 forks source link

Having successUrl/cancelUrl required, but also with default-value, doesn't make sense #4

Open eigood opened 2 months ago

eigood commented 2 months ago

https://github.com/coarchy/stripe/blob/3ea626d9b6b4eebbe47a9a0316d92fdba56d2034/service/stripe/StripeServices.xml#L27

Either remove the default-value from these 2 parameters, or allow them to be blank, so the default can be used.

acetousk commented 2 months ago

What's the use case for this? A cancelUrl and successUrl are required for the application to work properly. You can and should set your own value, but a good default is the current Moqui url.

eigood commented 2 months ago

I get that stripe needs those values to not be blank; but having both required=true and a default-value, means the default-value is never parsed or used. Is that there for some sort of documentation?

You say "can and should", but not "must", and then go on to say a good default is. By that logic, the values aren't required.

It's just a code review.