Added check for coupon expiration date

[fballiano]

Mage_SalesRule_Model_Validator::_canProcessRule() checks the validity of a coupon code that is in the process of being added to the cart, but it doesn't check for the single coupon expiration date. This PR adds this check.

Manual testing scenarios (*)

1. in the backend go to "promotions -> Shopping Cart Price Rules"
2. create a new rule with "specific coupon" (but also autogenerated coupons have the same issue) and type a coupon code
3. be sure that the rule also has some "action", ex: 10% discount
4. save the rule and check that it's working on the frontend

Now, what we're about to check isn't really supported by the core but I think it's still worth fixing.

5. open your database console and check for the salesrule_coupon table, you'll find a record with the coupon code you just created above
6. modify the expiration_date column for that specific coupon and set it in the past
7. go to the frontend, apply the coupon and notice that it gets applied anyway

It is true that the core wouldn't manage the expiration_date column but, since it exists, I know extensions (actually I was developing one today when I found this bug) that set the expiration_data for a single coupon (let's say you want a coupon to be usable only for 7 days after its generation), and it doesn't make sense to me that the core doesn't check for the expiration_date of a coupon.

After applying this PR you'll notice that the expiration_date gets correctly checked, if present.

Related PRs

This PR was already approved in #3029 but I had to recreate it due to rebasing problems.

[luigifab]

This create a huge side effect! If you have multiple coupon code with same code in multiple rules, if one of the coupon is expired, this prevent another rule to work (here 8829).

select * from salesrule_coupon where code = "JULIA30";
+-----------+----------+---------+-------------+--------------------+------------+---------------------+
| coupon_id |  rule_id | code    | usage_limit | usage_per_customer | times_used | expiration_date     |
+-----------+----------+---------+-------------+--------------------+------------+---------------------+
|   1180785 | off 5409 | JULIA30 |        NULL |               NULL |         84 | 2018-11-30 00:00:00 |
|   1203366 | off 5828 | JULIA30 |        NULL |               NULL |          0 | NULL                |
|   1357897 | off 7914 | JULIA30 |        NULL |               NULL |          0 | NULL                |
|   1358070 | off 7915 | JULIA30 |        NULL |               NULL |          0 | NULL                |
|   1358243 | off 7916 | JULIA30 |        NULL |               NULL |          0 | NULL                |
|   1519071 |  on 8829 | JULIA30 |        NULL |               NULL |          0 | NULL                |
+-----------+----------+---------+-------------+--------------------+------------+---------------------+

[colinmollenhour]

Yikes!

What should the correct behavior be? Check all coupons or just the newest one? For example here is checking the newest one:

                $couponCollection = Mage::getResourceModel('salesrule/coupon_collection');
                $couponCollection->addFieldToFilter('code', $couponCode);
                $couponCollection->setOrder('rule_id', 'desc');
                $couponCollection->setPageSize(1);
                if ($couponCollection->count()) {
                    $coupon = $couponCollection->getFirstItem(); /* @var $coupon Mage_SalesRule_Model_Coupon */

[fballiano]

@luigifab checkin the code, the same thing would happen if usage limit is surpassed:

                    // check entire usage limit
                    if ($coupon->getUsageLimit() && $coupon->getTimesUsed() >= $coupon->getUsageLimit()) {
                        $rule->setIsValidForAddress($address, false);
                        return false;
                    }
                    // check coupon expiration
                    if ($coupon->hasExpirationDate() && ($coupon->getExpirationDate() < Mage::getModel('core/date')->date())) {
                        $rule->setIsValidForAddress($address, false);
                        return false;
                    }
                    // check per customer usage limit
                    $customerId = $address->getQuote()->getCustomerId();
                    if ($customerId && $coupon->getUsagePerCustomer()) {
                        $couponUsage = new Varien_Object();
                        Mage::getResourceModel('salesrule/coupon_usage')->loadByCustomerCoupon(
                            $couponUsage,
                            $customerId,
                            $coupon->getId()
                        );
                        if ($couponUsage->getCouponId() &&
                            $couponUsage->getTimesUsed() >= $coupon->getUsagePerCustomer()
                        ) {
                            $rule->setIsValidForAddress($address, false);
                            return false;
                        }
                    }

so I don't know...

[luigifab]

I'm not sure, I simply reverted this PR, and now my coupon code is working.
Perhaps the truth is out there.

Here (l. 177) we are loading the coupon by code, perhaps we can load it by code and rule id?

$coupon = Mage::getModel('salesrule/coupon');
$coupon->load($couponCode, 'code');

[colinmollenhour]

Here (l. 177) we are loading the coupon by code, perhaps we can load it by code and rule id?

Ahh yes, by rule id sounds correct to me..

                $couponCollection = Mage::getResourceModel('salesrule/coupon_collection');
                $couponCollection->addFieldToFilter('code', $couponCode);
                $couponCollection->addFieldToFilter('rule_id', $rule->getId());
                $couponCollection->setOrder('coupon_id', 'desc');
                $couponCollection->setPageSize(1);
                if ($couponCollection->count()) {
                    $coupon = $couponCollection->getFirstItem(); /* @var $coupon Mage_SalesRule_Model_Coupon */

However, I never use this feature so am no expert on it. Are there other circumstances where a rule can have multiple coupons with the same code making it necessary to filter out used up ones or loop through all of them? Just trying to throw out some suggestions, please open a PR if you see fit.

[kyrena]

A new PR for this new problem is required?

[colinmollenhour]

@kyrena I think yes

[fballiano]

@luigifab @kyrena how can you create multiple rules with the same coupon code (since the records you copied do not seem to be autogenerated coupons)?

because it is forbidden:

[luigifab]

Oh yes, true, we are using a module to allow multi coupon code.