Skip to content

[User] Added proper listener to CustomerGuestType#3372

Closed
Zales0123 wants to merge 5 commits intoSylius:masterfrom
Zales0123:customer-guest-type-listener-improvements
Closed

[User] Added proper listener to CustomerGuestType#3372
Zales0123 wants to merge 5 commits intoSylius:masterfrom
Zales0123:customer-guest-type-listener-improvements

Conversation

@Zales0123
Copy link
Copy Markdown
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
Doc PR

While developing SyliusReviewBundle, I realized current CustomerGuestType is not working properly, while embedding in another form. I implemented new listener, that I hope would work better with such form.
Now there are 3 possibilities while using sylius_customer_guest:

  • automatically set logged in user as form data
  • if there is registered customer in database, validation in RegisteredUserValidator is triggered
  • if passed email is not used by any customer, new customer is created and set as form data

@Zales0123 Zales0123 force-pushed the customer-guest-type-listener-improvements branch 2 times, most recently from 37ebb5e to 40f66f8 Compare September 30, 2015 07:06
@Zales0123 Zales0123 mentioned this pull request Sep 30, 2015
7 tasks
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use interface

@Zales0123 Zales0123 force-pushed the customer-guest-type-listener-improvements branch 2 times, most recently from cecb4b1 to cc9f5ca Compare October 6, 2015 08:15
@pjedrzejewski pjedrzejewski added this to the v0.16.0 milestone Oct 7, 2015
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say we need one more scenario, placing order as guest for already existing customer or is it covered somewhere else?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, fixed 👍

@pjedrzejewski pjedrzejewski removed this from the v0.16.0 milestone Dec 14, 2015
@pjedrzejewski
Copy link
Copy Markdown
Contributor

Ping @michalmarcinkowski @Zales0123.

@Zales0123 Zales0123 force-pushed the customer-guest-type-listener-improvements branch from 9729ff4 to 078666c Compare December 14, 2015 15:33
@Zales0123
Copy link
Copy Markdown
Contributor Author

@pjedrzejewski Rebased and ready for merge (at least I hope so ;))

@pjedrzejewski
Copy link
Copy Markdown
Contributor

@michalmarcinkowski I'd like to hear your thoughts as it is your design but I'd love to avoid making this form type stateful. Perhaps we could pass the current customer as option?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing typehint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants