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

Forms shouldn't require whole Payment object instance #92

Closed
artursmet opened this issue Nov 10, 2015 · 1 comment
Closed

Forms shouldn't require whole Payment object instance #92

artursmet opened this issue Nov 10, 2015 · 1 comment
Labels

Comments

@artursmet
Copy link
Contributor

Now when I want to render payment form (example with Stripe) I have to do something like this:

payment = Payment(variant='stripe', total=1, currency='USD')
payment_form = payment.get_form()

And it will return blank form. Sometimes we don't exactly know total amount, before payment form initialization, for example for single page checkouts.

Also there is a bug I think, when I want to initialize form with data:

payment_form = payment.get_form(request.POST or None)

It performs validation and save at this step ⚠️
https://github.com/mirumee/django-payments/blob/master/payments/stripe/__init__.py#L33

In my opinion get_form() method shouldn't try to validate the form and for sure it shouldn't save the form.

@patrys
Copy link
Contributor

patrys commented Nov 10, 2015

This is not trivial.

Some payment gateways require the form to be cryptographically signed using a hidden input. You need a payment object to do this.

Some payment gateways require that the form's action points to an external server. You can't put these inside a single page checkout.

get_form() is really a view. It either returns a form to display or tells you to redirect the user somewhere. It was not meant to be composed with other such views as there is no way to redirect a single browser to two separate resources. If you really want to call get_form() from a composite view, do not pass in data unless all other forms are valid and you've already saved them (so that get_form() can save whatever it wants and take over).

@patrys patrys closed this as completed Nov 10, 2015
@patrys patrys added the archive label Dec 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants