[Core] [WebBundle] Implemented Guest checkout #1816

dbalabka · 2014-08-21T17:42:10Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	no
Fixed tickets	#90
License	MIT
Doc PR	(Sylius/Sylius-Docs#199)

Submit changes to the documentation
Maybe add more tests
Fix tests
Make possibility to disable Guest checkout

I have created a possibility for Guest customers to checkout only using email. Order entity now always has information about customer's email(even if customer was registered). I have removed duplicated registration logic from Security step. Registration form still exists on Security step page as is, but registration request will be addressed to main registration controller. After successful registration customer will be redirected back to checkout.

arnolanglade · 2014-08-21T17:51:59Z

src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/GuestType.php

+    public function buildForm(FormBuilderInterface $builder, array $options)
+    {
+        $builder
+            ->add('email', 'email'/*, array('label' => 'sylius.form.checkout.')*/)


You should remove this comment...

stloyd · 2014-08-21T20:46:04Z

Instead of again adding something to order, what about creating something like AnonymousOrder? It could work like Single Table Inheritance...

Additionally that feature should be optional, and probably not enabled by default ;)

stloyd · 2014-08-21T20:47:03Z

src/Sylius/Bundle/CoreBundle/Resources/config/services.xml

@@ -191,6 +192,10 @@
            <argument>%sylius.model.cart.class%</argument>
            <tag name="form.type" alias="sylius_checkout_payment" />
        </service>
+        <service id="sylius.checkout_form.guest" class="%sylius.checkout_form.guest.class%">


Please use same naming as in other forms: sylius.form.order.guest_checkout.

So other forms for checkout steps also has incorrect ID's names like:

sylius.checkout_form.payment

sylius.checkout_form.shipment

sylius.checkout_form.shipping

am i right?

Those are one of the oldest ones, I have in mind creating PR that fix this, but IMO new one should be named more correctly (note that suggested by my is just proposal).

stloyd · 2014-08-21T21:54:45Z

src/Sylius/Bundle/WebBundle/Resources/config/services.xml

@@ -20,6 +20,7 @@

        <parameter key="sylius.form.frontend.profile.type.class">Sylius\Bundle\WebBundle\Form\ProfileFormType</parameter>
        <parameter key="sylius.form.frontend.registration.type.class">Sylius\Bundle\CoreBundle\Form\Type\RegistrationFormType</parameter>
+        <parameter key="sylius.listener.registration.class">Sylius\Bundle\WebBundle\EventListener\RegistrationListener</parameter>


Seems like you removed that file from commit.

Accidentally during rebasing. Now should be good.

dbalabka · 2014-08-21T23:16:42Z

@stloyd the reason why I was added email field into Order was that both types of customers(registered and guest) should have correct history of orders(in this particular case we keep information which email address was used to send Invoice). Keep this information in order entity we cover both cases at once. Even if Guest checkout will be disabled we should keep email history for registered customers.
For big stores with huge amount of order history it is a good to denormalize tables to avoid additional JOINs. From my point of view Single Inheritance as approach is good for customisation to end-point developer that wants to extend Core module and add some new fields w/o changes in it.

According to previous discussion in #90 may be it will be better idea to treat it like proposed @Richtermeister :

Guest checkout would just be a matter of dis-associating customers from users, which is probably a good idea.

pjedrzejewski · 2014-08-26T06:53:54Z

src/Sylius/Bundle/CoreBundle/Checkout/Step/SecurityStep.php

-
+        if ($guestForm->handleRequest($request)->isValid()) {
+            $this->getManager()->persist($order);
+            $this->getManager()->flush();


Blank line before return.

pjedrzejewski · 2014-08-26T06:58:43Z

Nice work @torinaki, I think it is the simplest (and best for now) approach for this. I also like the registration code duplication removal. 👍

One thing to consider, when I merge this, we need to be always aware that Order does not always contain a reference to user. This is important for promotions, shipping and pretty much everything else.

It seems quite ready to me (at least for initial implemenation), so I'd love to gather some feedback. @kayue @stloyd @umpirsky @Arn0d @Sylius/core-team

I know that @umpirsky and @kayue implemented something like this, right? For HB?

kayue · 2014-08-26T07:24:12Z

src/Sylius/Bundle/WebBundle/Resources/views/Backend/Order/macros.html.twig

+                {% if order.user %}
+                    <p>{{ order.user.fullName }} <br><a href="{{ path('sylius_backend_user_show', {'id': order.user.id}) }}">{{ order.user.email }}</a></p>
+                {% else %}
+                    <p>{{ order.email }}</p>


How about this?

<a href="mailto:{{order.email}}">{{ order.billingAddress.firstName }} {{ order.billingAddress.lastName }}</a>

I fix it a little bit different. I did not added mailto links, because I want make different look between user that have profile and link to it and user that do not have profile.
Please take a look I guess you will like it.

kayue · 2014-08-26T07:35:05Z

@pjedrzejewski @torinaki I like this too.

I think we can use the name from the billing address. This can avoid some if order.user check too.

In our project we create user on the fly to avoid changing the core. This PR is better 👍

robholmes · 2014-08-27T09:28:55Z

@kayue Its quite possible that the billing address is not your customer. It could be a wife using the husband's card, or an employee using the company card, etc.

For these scenarios the billing address cannot reliably give you the customer details. Even the shipping address would be unreliable, as it could be a gift being sent to a relative, etc.

kayue · 2014-08-27T09:43:15Z

@robholmes I tend to disagree with you. If customer info can be different from billing address, then we should ask for customer info in addressing page. (ie: a separated Customer model)
With your current approach, guest customer has no way to provide his full name :p

And in fact I think having a separated Customer model could be a good idea, because all order details (name, address) should be immutable. Relying on user name maybe not so good, user can change his name after order is placed, this could be misleading.

kayue · 2014-08-28T04:20:55Z

@robholmes I am working on a payment gateway and it has the following XML model, I think we can use it as reference.

I like how they have a Contact model, and has phone number etc. I need it in my project too. (I store phone number to Address model at the moment, I don't think it is the best way to do it.)

<Checkout>
  <Card>
    <BrandId>master</BrandId>
    <BrandName>MasterCard</BrandName>
    <AccountNumber>5204740009900071</AccountNumber>
    <BillingAddress>
      <City>St Louis</City>
      <Country>US</Country>
      <CountrySubdivision>US-MO</CountrySubdivision>
      <Line1>123 Main Street</Line1>
      <PostalCode>63123</PostalCode>
    </BillingAddress>
    <CardHolderName>JOE Test</CardHolderName>
    <ExpiryMonth>12</ExpiryMonth>
    <ExpiryYear>2015</ExpiryYear>
  </Card>
  <TransactionId>5007680</TransactionId>
  <Contact>
    <FirstName>JOE</FirstName>
    <LastName>Test</LastName>
    <Country>US</Country>
    <EmailAddress>joe.test@email.com</EmailAddress>
    <PhoneNumber>1-9876543210</PhoneNumber>
  </Contact>
  <ShippingAddress>
    <City>hk</City>
    <Country>HK</Country>
    <CountrySubdivision>123</CountrySubdivision>
    <Line1>testing</Line1>
    <Line2>testing</Line2>
    <RecipientName>JOE Test</RecipientName>
    <RecipientPhoneNumber>1-9876543210</RecipientPhoneNumber>
  </ShippingAddress>
  <PayPassWalletIndicator>101</PayPassWalletIndicator>
</Checkout>

kayue · 2014-08-28T04:30:50Z

In fact the current Sylius checkout flow is not perfect:

1. Billing -> 2. Shipping -> 3. Payment

If customer choose to pay with PayPal, he shouldn't have to provide billing address to Sylius, because billing address will be returned from PayPal anyway. The current checkout flow is duplicated.

So it is better to use Contact to replace Billing Address at the first step. It also solve @robholmes 's wife scenario.

1. Contact -> 2. Shipping -> 3. Payment (and billing address form if needed)

Fixed fatal on checkout. Improved guest customer info in admin.

…-guest-checkout

arnolanglade · 2014-08-28T17:52:12Z

src/Sylius/Bundle/CoreBundle/Checkout/Step/SecurityStep.php

    }

    /**
     * Render step.
     *
     * @param ProcessContextInterface $context
-     * @param FormInterface           $registrationForm
+     * @param FormInterface $guestForm


$guestForm and $registrationForm should be aligned

Done. I have adjusted my IDE settings, hope it will be last comments about code style :)

…-guest-checkout

dbalabka · 2014-08-28T20:42:46Z

Do I need to wait green build on travis? I guess there is a problem with Behat execution time.

…uest-checkout

stloyd · 2014-09-11T07:08:02Z

src/Sylius/Bundle/CoreBundle/Resources/config/doctrine/model/Order.orm.xml

@@ -50,6 +50,7 @@
        <field name="currency" length="3" />
        <field type="string" name="paymentState" column="payment_state" />
        <field type="string" name="shippingState" column="shipping_state" />
+        <field type="string" name="email" column="email" nullable="true"/>


IMO this is acceptable to land in OrderBundle instead of CoreBundle, along with code changes.

I think CoreBundle is a better place. In OrderBundle we don't even have address there.

https://github.com/Sylius/Sylius/blob/9264b61c40b4578f3b160dcefe1408e2a0181a36/src/Sylius/Component/Order/Model/OrderInterface.php

I would agree that e-mail field could go to the Order component & bundle, but only this field.

Hmmm... in fact, we could even consider this field on address instead of order...

No, I think it should stay on the order, that's what Spree does and it feels most natural. Simplifies searching by e-mail etc.

Fixed and added spec tests

stloyd · 2014-09-11T07:42:04Z

src/Sylius/Bundle/CoreBundle/Mailer/AbstractMailer.php

@@ -46,6 +46,7 @@ public function __construct(TwigMailerInterface $mailer, array $parameters)
     */
    protected function sendEmail(array $context, $recipient)
    {
+        echo $this->parameters['template'];


This is for sure something that can't be in accepted.

:D Sorry about that. Fixed

stloyd · 2014-09-12T09:18:10Z

@torinaki Please merge dbalabka#2 as it finishes this PR so it can be merged into Sylius! =)

Fix code after feedback, add ability to disable guest orders

stloyd · 2014-09-17T08:29:03Z

@pjedrzejewski Can you have a look on this? IMO it's good for merge!

[Core] [WebBundle] Implemented Guest checkout

pjedrzejewski · 2014-09-17T13:06:10Z

Thank you @torinaki for this super-cool feature and Joseph + Arnaud for the review! :)

…checkout [Core] [WebBundle] Implemented Guest checkout

arnolanglade reviewed Aug 21, 2014
View reviewed changes

stloyd reviewed Aug 21, 2014
View reviewed changes

dbalabka force-pushed the GH90-Implementing-guest-checkout branch from ffee690 to 924bf07 Compare August 21, 2014 21:44

stloyd reviewed Aug 21, 2014
View reviewed changes

[Core] [WebBundle] Implemented Guest checkout

2534b7b

dbalabka force-pushed the GH90-Implementing-guest-checkout branch from 924bf07 to 2534b7b Compare August 21, 2014 22:32

pjedrzejewski reviewed Aug 26, 2014
View reviewed changes

kayue reviewed Aug 26, 2014
View reviewed changes

dbalabka added 2 commits August 28, 2014 17:29

Fixed code style.

a5f07a5

Fixed fatal on checkout. Improved guest customer info in admin.

Merge remote-tracking branch 'upstream/master' into GH90-Implementing…

d82c245

…-guest-checkout

arnolanglade reviewed Aug 28, 2014
View reviewed changes

arnolanglade added the Enhancement label Aug 28, 2014

dbalabka added 3 commits August 28, 2014 21:01

Merge remote-tracking branch 'upstream/master' into GH90-Implementing…

f1de437

…-guest-checkout

Cover order placing with behat tests.

d8e812c

Code style fixes

55d8aee

kayue mentioned this pull request Sep 2, 2014

[Address] Add phone number field #1857

Merged

1 task

dbalabka mentioned this pull request Sep 5, 2014

Update Order documentation according to Guest Checkout Sylius/Sylius-Docs#199

Merged

Merge remote-tracking branch 'origin/master' into GH90-Implementing-g…

ae470b3

…uest-checkout

stloyd reviewed Sep 11, 2014
View reviewed changes

Fixed order confirmation email.

ae175b9

Fixed and added spec tests

stloyd reviewed Sep 11, 2014
View reviewed changes

Remove debug code

1ae1afd

stloyd and others added 4 commits September 15, 2014 13:26

Fix code after feedback, add ability to disable guest orders

6f2f678

Added test case that we should stay on checkout page during registration

cbe5078

Merge pull request #2 from stloyd/feature/guest-checkout

a57f54c

Fix code after feedback, add ability to disable guest orders

Fix test

24fc314

pjedrzejewski pushed a commit that referenced this pull request Sep 17, 2014

Merge pull request #1816 from torinaki/GH90-Implementing-guest-checkout

256f1bb

[Core] [WebBundle] Implemented Guest checkout

pjedrzejewski merged commit 256f1bb into Sylius:master Sep 17, 2014

arnolanglade mentioned this pull request Sep 17, 2014

[Feature]: Buy without registration. #90

Closed

This was referenced Oct 14, 2014

Guest checkout is missing email validation #2016

Merged

Order user can be null #2017

Merged

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019

Merge pull request Sylius#1816 from torinaki/GH90-Implementing-guest-…

1b3affc

…checkout [Core] [WebBundle] Implemented Guest checkout

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019

Merge pull request Sylius#1816 from torinaki/GH90-Implementing-guest-…

2467b1b

…checkout [Core] [WebBundle] Implemented Guest checkout

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019

Merge pull request Sylius#1816 from torinaki/GH90-Implementing-guest-…

998359e

…checkout [Core] [WebBundle] Implemented Guest checkout

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] [WebBundle] Implemented Guest checkout #1816

[Core] [WebBundle] Implemented Guest checkout #1816

dbalabka commented Aug 21, 2014

arnolanglade Aug 21, 2014

dbalabka Aug 21, 2014

stloyd commented Aug 21, 2014

stloyd Aug 21, 2014

dbalabka Aug 21, 2014

stloyd Aug 21, 2014

dbalabka Aug 21, 2014

stloyd Aug 21, 2014

dbalabka Aug 21, 2014

dbalabka commented Aug 21, 2014

pjedrzejewski Aug 26, 2014

dbalabka Aug 28, 2014

pjedrzejewski commented Aug 26, 2014

kayue Aug 26, 2014

dbalabka Aug 28, 2014

kayue commented Aug 26, 2014

robholmes commented Aug 27, 2014

kayue commented Aug 27, 2014

kayue commented Aug 28, 2014

kayue commented Aug 28, 2014

arnolanglade Aug 28, 2014

dbalabka Aug 28, 2014

dbalabka commented Aug 28, 2014

stloyd Sep 11, 2014

kayue Sep 11, 2014

pjedrzejewski Sep 11, 2014

stloyd Sep 11, 2014

pjedrzejewski Sep 11, 2014

stloyd Sep 11, 2014

dbalabka Sep 11, 2014

stloyd commented Sep 12, 2014

stloyd commented Sep 17, 2014

pjedrzejewski commented Sep 17, 2014

[Core] [WebBundle] Implemented Guest checkout #1816

[Core] [WebBundle] Implemented Guest checkout #1816

Conversation

dbalabka commented Aug 21, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stloyd commented Aug 21, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbalabka commented Aug 21, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjedrzejewski commented Aug 26, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kayue commented Aug 26, 2014

robholmes commented Aug 27, 2014

kayue commented Aug 27, 2014

kayue commented Aug 28, 2014

kayue commented Aug 28, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbalabka commented Aug 28, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stloyd commented Sep 12, 2014

stloyd commented Sep 17, 2014

pjedrzejewski commented Sep 17, 2014