silvershop / silvershop-discounts

Adds Discount and Coupon support for SilverShop
Other
9 stars 38 forks source link

This module relies on class `ShippingFrameworkModifier` existing #34

Closed jameshoward closed 8 years ago

jameshoward commented 9 years ago

If I install the discount module on a base shop install I get the following notice:

Modifier class "ShippingFrameworkModifier" does not exist.

I've tracked this down to Shop\Discount\Calculator line 92:

if($shipping = $this->order->getModifier("ShippingFrameworkModifier")){

In Order::getModifier there's a ClassInfo::exists check that's throwing the user_error because that class doesn't exist.

Either there's a dependency on silverstripe-shop-shipping or a better way of determining if the order has that modifier without needing the class to exist.

markguinn commented 9 years ago

Good catch, @jameshoward. I've run into that before as well. Just created pull request #35. Assuming it passes CI I'll merge it today. @jedateach I don't think we want to require the shipping framework do we?

jameshoward commented 9 years ago

Two potential solutions I can think of:

  1. Change getModifier so it returns NULL rather than throwing an error. Without knowing where else this method is called from, I can't tell if this actually breaks more than it solves.
  2. Implement a hasModifier method. This could do a couple of lazy checks first:
// Lazy checks for class existing and order having modifiers.
if (!ClassInfo::exists($className) || ($this->order->Modifiers()->count() == 0)) {
    return false;
}

// Check whether the order has the modifier...
jameshoward commented 9 years ago

Or your 3rd option in #35 that looks much less risky! :)

markguinn commented 9 years ago

@jedateach it's worth thinking about #1 above as a fix in the shop module itself.

markguinn commented 9 years ago

35 is merged

wilr commented 8 years ago

This has been resolved.

isaac-at-room9 commented 8 years ago

How long until this is released? We're using this module and it has been over a year since anything was released. Ran into this issue today with the latest release, had to fix the package at a commit (not preferable)

wilr commented 8 years ago

@isaac-at-room9 Tagged a 1.2.0 release.

isaac-at-room9 commented 8 years ago

@wilr oh, i must've missed that one when I first installed the package... I can't see it on https://packagist.org/packages/silvershop/discounts