swipestripe / silverstripe-swipestripe

Ecommerce module for SilverStripe
http://swipestripe.com
33 stars 49 forks source link

OrderForm.php - process() misses an edge/rare case #41

Open chrisrae opened 10 years ago

chrisrae commented 10 years ago

Line 273

This will cause an uncaught ValidationException as it does not check the Member table, only the members linked to via the Customer table.

If I create an account (say the primary admin account when I install SS) with an email foo@bar.com and then I try to purchase an item using the foo@bar.com email address on the checkout page without being logged in - the system will try to write that member and blow up as a result. foo@bar.com won't be a customer, but will be a member, which SS will find out about when it tries to write a new member on top of it.

frankmullenger commented 10 years ago

@chrisrae agree that this is an issue, would perhaps be resolved with changes to use member extension instead. Discussion here: https://github.com/frankmullenger/silverstripe-swipestripe/issues/42

Zauberfisch commented 10 years ago

yes, the whole problem is the Customer class, and as far as I can see, we all agree to re factor out the Customer class in the next release. For the time being, I don't really see a fix for this issue. At least not one that we could implement quickly.

I would rather invest the resources we have into a new release, than hunting down bugs that will be obsolete by the next version. My vote goes to close as wont-fix unless someone is willing to do a pull request.

frankmullenger commented 10 years ago

Agree with @Zauberfisch, this should be fixed in the next version with the removal of Customer class so we could close this for now. @chrisrae are you happy with that approach?

chrisrae commented 10 years ago

Totally fine.

Chris Rae - Web Services Manager Darkside Media & Fluid Visual Communications 10 Memorial Street, Queenstown Mob: 021 022 18625 Email: chris@darkside.co.nz Tel: 3 442 6739

On Tue, May 13, 2014 at 8:41 PM, Frank Mullenger notifications@github.comwrote:

Agree with @Zauberfisch https://github.com/Zauberfisch, this should be fixed in the next version with the removal of Customer class so we could close this for now. @chrisrae https://github.com/chrisrae are you happy with that approach?

— Reply to this email directly or view it on GitHubhttps://github.com/frankmullenger/silverstripe-swipestripe/issues/41#issuecomment-42930638 .

MilesSummers commented 8 years ago

Suggest temporary fix change line 276 of OrderForm.php to $existingCustomer = Member::get()->filter('Email', $data['Email']);