Comments (34)
Hey @dyjo!
Thanks for the good words, it's really motivating! :)
As I stated in the blog post on Sylius.com - the payment gateway handling is our next step.
We're 100% sure we'll use Omnipay for this - so yeah, the integration is a work in progress - feel free to share any ideas about that - I'd be very happy to hear them and discuss.
I'd love to handle several different flows for the payment out of the box, but to keep it flexible at the same time.
Handle standard CC info, Transparent Redirect etc. but it requires some work. :)
We can perhaps rename this issue to Omnipay integration and discuss everything here, what do you think?
Thanks for jumping in!
from sylius.
Narrowing it down to Omnipay Integration helps. My original question was about a checklist of items that would create a working deployment, so I've tried to create one here. I've also tried to keep the descriptions as simple as possible, to prevent incorrect assumptions about the intended architecture. Just tell me if any corrections should be made, but I think maintaining this list will be useful as a roadmap for end-users or new contributors.
- Integrate a
CreditCard
Entity toSyliusCoreBundle
. - Map an Association between
CreditCard
andPayment
. - Add a
CreditCardType
form type. - Add the
CreditCardType
to thePaymentStepType
of theCheckoutProcess
. - Add validation for the
CreditCard
Entity. - Map an Association between
Order
andPayment
. - Integrate submission and capturing of payments to the
PaymentStep
of theCheckoutProcess
. - Persist captured payments to the database.
I'll be posting my suggestions for these items in separate comments, and hope others do as well.
from sylius.
Would be great if you can help Sylius move forward on these tasks!
from sylius.
Why do you want to integrate a CreditCard entity to the CoreBundle? There is already one in PaymentBundle, it is even linked to the User class defined in config.
from sylius.
@winzou Thanks for pointing that out; I've been having trouble understanding how the DI occurs. I see that credit_card_owner
is defined in the config, but I didn't see the credit_card
model defined, and I didn't see a default defined in the DI Configuration.
from sylius.
Everything is in PaymentBundle/Entity, and the doctrine mapping in PaymentBundle/Resources/config/doctrine. There is also the relation between CreditCard and Payment.
You can tick the first 2 items ;)
from sylius.
@winzou I've looked at both of those, but was still confused: how do i access the credit card entity? is it available in the service container as 'sylius.model.credit_card.class'
? Because I couldn't find where the default was set to Sylius\Bundle\PaymentsBundle\Entity\CreditCard
.
from sylius.
Does anyone have already use JMSPayment bundles?
@pjedrzejewski wouldn't be a better thing to rely on JMS' bundles as this is a quite complex feature? It seems that JMS' bundles are on an advanced stage, it would save Sylius a lot of effort. Have you ever thought about that?
from sylius.
@winzou I found Sylius when I was looking for a more full-featured alternative to JMSPayment. The big value added for me is integration with Omnipay, so there's an abstracted method for submitting and capturing payments, and you don't have to define a new provider every time you want to use a new Gateway.
from sylius.
Do you think this is possible to integrate omnipay in a JMsPaymentCore plugin? It's a real question.
from sylius.
@winzou I'm sure it's possible, though I don't know why you would. It seems like the JMSPaymentCore GatewayPlugin duplicates a lot of what Omnipay does. At any rate, it don't know that it really relates to the topic of the issue, which is how does one bridge the gaps that exist in SyliusPayments and SyliusOmnipay
from sylius.
@winzou also a real question: #48 (comment). Figuring that out would be of great help for me, and hopefully flick the switch that helps me start contributing.
from sylius.
Ok, so my proposals for checking off the boxes on the task list. Please keep in mind this is all messy, just ideas not working code. But I want to get feedback before writing a bunch of stuff that doesn't make sense to maintainers.
Integrate a CreditCard
Entity to SyliusCoreBundle
- Make
Sylius\Bundle\PaymentsBundle\Entity\CreditCard
a mapped-superclass, and extend it in theCoreBundle
. This follows the same conventions as all entities defined in/Sylius/app/config/sylius.yml
asmodel
option. NOTE: may be covered per #48 (comment) above. I could not find where the CreditCard class had been passed into the service container.
Add a CreditCardType
form type
- Create
CreditCardType
in SyliusPaymentsBundle withdataClass
ofCreditCard
. - Add validation rules for
CreditCard
Map an Association between Order
and Payment
- Add a bidirectional one-to-one to both entities in the CoreBundle.
- Make it nullable on
Payment
because it will need to be constructed and persisted beforeOrder
is built infinalize
step; and not all payments necessarily come from an order. This could probably be handled better.
Map an Association between CreditCard
and Payment
This one is complicated. Differentiating and defining relationships between Payment, Payment Method, Gateway, Payment Source, and Transaction is my biggest source of confusion right now. The approach outlined below allows for splitting sources of payments, e.g. in a situation where someone pays part of a bill with a credit card and part with a check, or when a bill is split amongst multiple people, like in a restaurant. It also clarifies and facilitates the most important aspect of a commerce codebase: payments!
- Convert
PaymentSourceInterface
toTransactionSourceInterface
. - Add a mapped
$source
property toTransaction
. - Convert
PaymentMethodInterface
toTransactionMethodInterface
and move$method
property from Payment to Transaction; if the transaction is a record of exchanged payment, then it should have knowledge of the method of exchange. - Make
$gateway
nullable onTransactionMethod
(or currentPaymentMethod
). Some transactions may occur outside of a gateway. - So in short, Source (e.g. Credit Card) -> Transaction -> Payment
Add the CreditCardType
to the PaymentStepType
of the CheckoutProcess
- Create
TransactionType
in SyliusPaymentsBundle withdataClass
ofTransaction
. - Use class of the
$source
property of the transaction to determine what form type to add toTransactionType
, e.g. theCreditCardType
type created above.
Integrate submission and capturing of payments to the PaymentStep
of the CheckoutProcess
- Create interface
Sylius\Bundle\OmnipayBundle\Model\CreditCardInterface
with a mapping method to convert any credit card entity to the Omnipay naming conventions. Sylius\Bundle\CoreBundle\Entity\CreditCard ... implements CreditCardInterface
- Add an
obtain()
method toTransactionMethodInterface
which will be called to actually collect the exchange. Also allows for the differing use of logic between on-site and off-site gateways. - In
forwardAction
ofSylius\Bundle\CoreBundle\Checkout\Step\PaymentStep
, after validation but before forward, add logic for confirming a payment has been captured.
For example:
$omnipayCard = $form->get('creditCard')->mapToOmnipay();
$omnipayGateway = $this->get('sylius.omnipay.gateway' . $this->getCurrentCart()->getPaymentMethod()->getGateway());
$transaction = $omnipayGateway->purchase($omnipayCard);
Persist Captured Payments to Database
- Before sending the user to the
finalize
step
Hopefully this is easy enough to follow, and hopefully others can help me understand the code that's already there to make this stuff happen. If any of these sounds reasonable, let me know and I'll get to work on a pull request.
from sylius.
Why do you want CreditCard
entity to be in CoreBundle? It doesn't need any customization (a credit card is a credit card), so the entity can (and should) live in PaymentBundle. Like we do with many other entities. And it isn't available from the service container afaik.
I totally agree with all your following points. There is just one thing I want to raise: the use of credit card. In quite many cases I think end-users will choose to go through the bank interface: it means that the ecommerce website doesn't have to ask for and store the credit card information, avoiding some obligations. For us, it means that the payment process differs from one payment method to another, and we don't show the credit card form everytime.
from sylius.
I totally agree with winzou on the use of credit card. Storing credit card data is very very discouraged. If Sylius wants to store these kind of information, it should follow the PCI-DSS standard. Here is some documentation :
- PCI quick reference guide
- PCI DSS standard
I don't know if these requirements have been taken into consideration at the moment. But if not, we have to understand that :
- this is mandatory
- this can lead to quite a big amount of work :)
from sylius.
The actual use case for CreditCard entity is to store the name, 4 last digits and the token for gateways which support token billing. So the store does not store any sensitive data and customer can enter the info only once, and later just select his credit card from the list. There is no plan to save any cc information on our side, but of course it's up to developer what he'll do!
from sylius.
Re: #48 (comment)
...and other comments about where the credit card entity is made available in the core bundle. I just looked at the driver/doctrine/orm.xml
file in Payments and saw that the default configuration occurs there. This solves a lot of my confusion regarding the handling of sources and credit cards. Accordingly, I've checked off the first item in the task list.
I've left the second item unchecked because I still don't see a concrete mapping between a payment and a credit card (not just association in schema, but forms and embedded forms) in the checkout flow.
@winzou re: #48 (comment).
I agree, if you look at my (admittedly long) set of suggestions, I propose abstracting the payment source so multiple sources can be supported (credit card, off-site transfer, electronic check, heck, even cash). I only mentioned credit card specifically because it is the only source available in Payments at the moment.
But, in the event a credit card is available as a payment source, persistence should definitely be available. Storing a token is very useful for recurring payments, storing the last 4 and a reference to the owner doesn't require PCI compliance, but does allow a user to select the method for a future payment. Further, someone using the bundle independently may want to store more info, and could develop PCI compliance.
from sylius.
Hello! I'd like to join in, plan to use Sylius for some jobs soon. I've been working on a pull request for credit card form type. I've run into a problem with where to get a list of supported brands/types. Different gateways support different brands depending on locale (https://stripe.com/us/help/faq#types-of-payments). So the question is how to get that list. Some ideas
- a config variable set by end user (seems most flexible but most prone to error)
- a
Gateway
method likegetSupportedBrands();
(seems flawed because it's only for credit cards). - a new class, maybe
SourceTypeResolver
that takes a locale, a source type (cc, ACH), and a gateway and spits out a list of supported brands.
In the meantime I'm just using the constants from Omnipay Credit Card class (https://github.com/adrianmacneil/omnipay/blob/master/src/Omnipay/Common/CreditCard.php)
from sylius.
I think this is up to end-users, they can choose to enable or not certain types of credit cards in their country.
from sylius.
Hey guys, thanks for all the input. 👍
Just to clarify confusion about Payment -> CreditCard association - it's there https://github.com/Sylius/SyliusPaymentsBundle/blob/master/Model/Payment.php#L63.
But there are no dedicated get/setters, you can set and get it via get|setSource methods.
Also, credit card is associated with User (which needs to implement CreditCardOwnerInterface
.
This may be confusing as it's hidden behind Doctrine RTEL magic. See this cookbook entry.
This is how all Sylius relations work, so if you want to change one entity class (via configuration) there is no need to remap or redefine all entities. Relation are updated automatically.
This also works for CreditCard.user association, if you configure FOSUserBundle User entity class (or any other user entity you want) it will be associated with CC. I think this is sensible default.
Yes, Order should be associated with Payment. And I agree that Payments do not always have to come from Order, so let's keep the foreign key on Order side.
The plan was to create payment via PaymentFactoryInterface
service, which would take Order & PaymentMethod as argument.
The PaymentSourceInterface
is handly not only for CC, I also used it for recurring payments via PayPal, ELV, BankTransfer. Where each of these had a entity similar to CreditCard. holding some data and token for recurring billing.
@dyjo Thanks for your research, I fully agree that we should be able to set method/source per transaction, not only for payment.
My idea would be to remove Transaction model completely, and use offset payments like Spree does it.
Payment.offsets holds collection of Payment, so each Payment can (but does not need to) have a 1-level-tree structure. This solves the problem of renaming PaymentMethod to TransactionMethod, as each Payment already can hold the method source. All is described here.
@dyjo What do you think about handling this like Spree? We'd get rid of Transaction model, which otherwise would become very similar to Payment itself.
from sylius.
@pjedrzejewski Thanks for clearing all that up! The way Spree handles this looks perfect; certainly clears up the confusion about what is a transaction and what is a payment.
I'm not too familiar with Doctrine Extensions; I have worked with multi-level trees but not 1-level trees. Is there any special configuration required to limit a tree to a single level?
Additionally, curious about integrating two things you mentioned:
- recurring payments
- BankTransfer
Is there any plan to incorporate these into the payments bundle? They would be very helpful for standardizing transactions. I know that Omnipay has "off-site" gateways, so that should make using PayPal easy enough, but I haven't seen anything about Bank Transfer, ACH, etc.
from sylius.
Actually, I think we do not really need to use DoctrineExtensions (we use it for taxonomies) for this. I think that Payment.parent -> Payment association will be enough. With reverse side on Payment.offsets -> many Payments. We can simply perform a check to disallow setting Payment as parent if it is already a "offset" (or child) payment. This way we'll limit it to just one level tree.
Actually I was using Adyen payments system directly (via SyliusPaymentsBundle models), not Omnipay. And I think that Omnipay currently does not support token billing (or maybe it's in dev).
Nevertheless, we can simply contribute to Omnipay with features we need!
Forgive me guys but I need a bit of clarification who is who haha! @dyjo @Eponymi @dylanjohnson? :D
I'm not sure who should I ping. I think the Spree solution is the way to go, I'm currently focused on products refactoring and Order & Cart duplication. I'd be more than happy to help you with a PR for these change though, feel free to post any questions you may have during work on this.
from sylius.
Fair confusion! I'm Carl, I'm the boss :) . I just hired Dylan a couple weeks ago as a contractor, and he's now working with us full-time. So he's been using this account to make comments sometimes, but I've asked him to use his personal account (thought it was @dyjo, don't know why he has @dylanjohnson) for comments/issues now. He does all the code writing through this account. So pinging this account is probably best way to go.
So in short: any comments/issues coming from this account will be me, any code writing is Dylan.
And @dyjo @dylanjohnson pick one personal account name!
from sylius.
Woops. I deleted the other one. But now I'm also Ghost (#48 (comment)). Sorry about that!
from sylius.
from sylius.
@dyjo My plan is to work hard to finish the 2 important things I mentioned (product + cart&order) by the end of this week. Then I can code the Payments stuff with you if you wish, but of course you can start whenever you want and I'll be happy to help/discuss/review a PR! That'd be awesome help. 👍 Thanks for pushing this important feature forward!
from sylius.
@pjedrzejewski I'll have a PR up on PaymentsBundle in the morning tomorrow following the Spree model, and I'll make sure to put in all specs. Hopefully that will be good enough to merge, and if so, I'll get the stuff together for CoreBundle tomorrow afternoon.
from sylius.
@dyjo That's great but don't set so hard deadlines for yourself. :) Take your time to implement it!
from sylius.
Fair enough, thanks for the help!
from sylius.
@pjedrzejewski Any ideas on sources that don't require a form? Off-site sources like PayPal for example. I have a couple ideas but wanted to get your thoughts.
- on radio select a button to appears instead of a form.
- instead of a radio button, if a method is off-site, a button for that method is rendered.
Number one would be quite a bit easier to implement with current code base. Number two seems like a much better UI/UX, but would require a lot more work and would mean that any off-site payment method would have to add a button view, or at the very least, an URL pointing to the gateway.
from sylius.
I think we should do number 1 in a first iteration, and then improve UI over time.
from sylius.
@dyjo @winzou I'd vote for number 1 too, let's do smaller steps, we have a lot to do in terms of UI anyway.
from sylius.
Ok, I've had a lot of trouble figuring out how to implement this abstraction. I'm going to defer to others on this integration and just use some hard-coding in the PaymentStep for the time being. Hopefully someone will be able to put something together for this, I'll be looking for it!
EDIT: If anyone is interested in seeing the route I was going, you can check here: https://github.com/Eponymi/SyliusPaymentsBundle/tree/payment_refactor_branch
from sylius.
Closed in favor of: #112
from sylius.
Related Issues (20)
- Form errors during creation of product remove all product images
- [UI] Text going out of the frame in product description HOT 2
- [Refactor][ApiBundle] Unify section resolver usage HOT 1
- Unable to generate url for route sylius_shop_product_show when product slug contains slash HOT 1
- AverageRatingUpdater is called twice when a review is accepted HOT 1
- The complete checkout validation group is difficult to override
- Cannot complete payment via admin
- Api Cart Context calls wrong endpoint method HOT 2
- Invalid constraint configuration in AddProductReview.xml HOT 6
- Catalog promotion is still applying after end date, price is not restored to original HOT 5
- Would be nice to have sylius/promotion not require doctrine/orm
- Order tax adjustment is treated as shipping tax
- products order in the front-end is not showing in descending order ( the latest to the oldest) of the createdAt date only in production. HOT 1
- As an Admin, I want to resend the confirmation email of an Order that is `fulfilled`
- Incompatibility with SyliusResourceBundle 1.11 HOT 2
- As a Shop API consumer, I want to see the Start and End dates of a Promotion Catalog HOT 2
- As a Shop API consumer, I want to see the current applied catalog promotions of a Product Taxon HOT 1
- Payment request a new way to handle payments
- Stop needing to cache:clear every Api Platform annotation change HOT 1
- The documentation should also have `npm` commands HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sylius.