coupon discount is also applied to shipping

[guyvanbael]

When applying a discount... discount is applied to shipping cost also.
Don't think that's correct. Shipping should be added AFTER the discount.

[jordanmkoncz]

+1 for this, I agree that the discount should be applied to the total cost of the items in the cart and the shipping should be a flat (non-discounted) fee added to the discounted total of the items in the cart.

[jordanmkoncz]

For anyone else who needs to make this change, I was able to get it working by changing the Amount() function in class Coupon to the following:

public function Amount($order) {

    // TODO: Multi currency

    $shopConfig = ShopConfig::current_shop_config();

    $amount = new Price();
    $amount->setCurrency($shopConfig->BaseCurrency);
    $amount->setSymbol($shopConfig->BaseCurrencySymbol);

    $total = $order->SubTotal()->getAmount();
    $mods = $order->TotalModifications();

    //<MODIFY_START>
    //- Calculate total value of items in order and apply discount to that, instead of to the total including modifications (such as shipping)

    $order_modifications = $order->Modifications();
    $order_modifications_total = 0;

    foreach($order_modifications as $order_modification) {
        if ($order_modification->ClassName != 'CouponModification') {
            $order_modifications_total += $order_modification->Amount()->getAmount();
        }
    }

    $order_items_total = $total - $order_modifications_total;

    $amount->setAmount(- ($order_items_total * ($this->Discount / 100)));

    //<MODIFY_END>

    return $amount;
}

I think ideally, you would allow the shop admin to pick whether they want to apply coupons to the total value of the items or to the total value of the order (including modifications). This could either be done as a shop-wide setting or on an individual coupon basis (or both).

[frankmullenger]

I think you could achieve this by using the SortOrder and SubTotalModifier properties to rearrange where the modifier is applied in the Order, but it would require creating a new class or exposing the settings to the admin user in the UI which could get very confusing.