[BUG] After upgrading to v20.16.0 I can't checkout with an existing customer's email

[ioweb-gr]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

I have a customer test@example.com and I'm trying to do a guest checkout using the same email address.

Instead of the order passing through I'm getting an error

This email address is already registered.

Expected Behavior

Checkout proceeds fine

Steps To Reproduce

1. Create a customer and subscribe to the newsletter
2. Try to make a guest order with the same customer email

Environment

- OpenMage: 20.16.0 
- php: 8.2

Anything else?

By a quick check I noticed that this is related to newsletter subscription because that's the only area this message appears but I'm not sure why it tries to subscribe the user even if he hasn't selected the checkbox to subscribe to the newsletter.

[ioweb-gr]

Is this check needed?

app/code/core/Mage/Newsletter/Model/Subscriber.php

elseif ($this->getStatus() == self::STATUS_SUBSCRIBED) {
            Mage::throwException(Mage::helper('newsletter')->__('This email address is already registered.'));
        }

This happens when someone tries to subscribe a user that's already subsribed.

If the status is already subscribed instead of throwing an explicit exception here, shouldn't we just return success?

The user is already subscribed anyways it's not affecting it's status.

Maybe just

return $this->getStatus()
```

[sreichel]

Ping @kiatng ... seems to come from #2214.

Imho ... this check makes sense. Maybe make it optional?

[ioweb-gr]

It would possibly make more sense to throw a different type of exception that we can use try / catch on and handle it then.

e.g. Mage_Newsletter_Already_Subscribed_Exception

Generally there are 30 other types of exceptions that extend the base class. I'm pretty sure if it was a dedicated Exception class it would simplify handling it

Maybe it's just me but I'm not particularly fond of the static \Mage::throwException because it forces one type of exception \Mage_Core_Exception and also in the IDE it doesn't highlight for you that there's an unhandled exception case.

[kiatng]

I tried but I cannot reproduce the error in DDEV following the steps outlined in the description, even in different ways. A typical steps I tried on frontend:

1. Register a customer with email test1.exampl.com, tick Newsletter subscription
2. Logout
3. Select a product and do onepage checkout
4. Select guest checkout
5. Go through onepage and submit order
6. Order is placed, no error

I tried with existing customer, tick subscribe newsletter in backend, repeat from step 2 above. I tried other ways: register customer on onepage checkout.

What steps am I missing?

[edit] Just realized I was testing on 20.15. I did one quick test on 20.16, still cannot replicate.
[edit] I asked AI, it thinks the only way this issue happens is custom code.

[ioweb-gr]

You're correct @kiatng on the standard 6 step checkout that is offered by openmage in default you don't have the option to sign up for the newsletter during checkout. We noticed it because our customer is using a onestepcheckout solution offering newsletter subscription on the checkout page and the error prevented us from checking out.

It's a pretty standard practice here to put newsletter subscription on the checkout page as an option.

However you can do another simpler test that shows a weird state introduced by this change.

Create a user with email test@example.com and sign up for the newsletter there,

Then try to subscribe again with test@example.com on the bottom of the page.

While the customer is successfully subscribed before he is presented with an error (which mentions the user is already subscribed), however. it's still presented as an error not as a successful registration message like before.

I couldn't replicate the inverse with subscribing first and then registering though which is weird as I would expect the same thing to happen there. A guest registered with an email in the newsletter and a new user is trying to register over the same newsletter subscription and should have thrown the exception. Instead it seems to merge the newsletter subscriber in this case.

[sreichel]

Create a user with email test@example.com and sign up for the newsletter there,
Then try to subscribe again with test@example.com on the bottom of the page.

I dont see whats wrong. Both messages seem correct to me.

[ioweb-gr]

Well imho this is a regression.

In 20.16.0, this now breaks guest checkout flows where newsletter subscription is integrated (like in OneStepCheckout or FireCheckout). This was never a problem before, so it’s effectively a behavioral regression.

Repeated subscription request isn’t an actual error, it’s an idempotent operation. The correct approach would be to acknowledge it as successful, because the desired state (subscribed) is already achieved. If we choose to keep the exception as the new behavior, we should also provide a way to catch this exception so that one step checkout solutions can catch it and process the order.

As for the message itself, it's wrong from a UX prespective. Showing in the error color like all other messages could lead the user to think the subscription wasn't actually achieved.