Skip to content

Essential part of https://bugs.oxid-esales.com/view.php?id=7100 as PR#983

Open
SvenBrunk wants to merge 1 commit intoOXID-eSales:b-7.2.xfrom
SvenBrunk:patch-12
Open

Essential part of https://bugs.oxid-esales.com/view.php?id=7100 as PR#983
SvenBrunk wants to merge 1 commit intoOXID-eSales:b-7.2.xfrom
SvenBrunk:patch-12

Conversation

@SvenBrunk
Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Anton Fedurtsya <anton@fedurtsya.com>
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 3, 2024

CLA assistant check
All committers have signed the CLA.

@Sieg Sieg changed the base branch from b-8.0.x to b-7.2.x October 3, 2024 12:07
@Sieg
Copy link
Copy Markdown
Member

Sieg commented Oct 3, 2024

Hey @SvenBrunk, the issue with this approach is that adding this condition would prevent inactive voucher series from being selected at all. That could be fine, but there's already a validation in place elsewhere that throws an error if the voucher is no longer valid.

With this new condition, the voucher wouldn't be selected at all if the series is expired, resulting in a different error – "Invalid coupon" – instead of the current one - "Expired". I can't think of a simple solution that avoids this inconsistency for the current vouchers implementation.

I have added the test for the original (the one we are fixing) case, but it doesn't cover the validation part I have mentioned (as its different subsystem, and that part is still working correctly for the expired coupons if they are given as parameter, but they will not be anymore) and rebased for 7.2.x.

Maybe this is all fine and we should still merge this?

@SvenBrunk
Copy link
Copy Markdown
Contributor Author

@Sieg okay, thanks. Please do not merge this. We need to think about a better solution then

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.

3 participants