Completed
Pull Request — master (#557)
by Roman
41:00 queued 10:48
created

ShoppingCart::remove()   C

Complexity

Conditions 7
Paths 7

Size

Total Lines 42
Code Lines 23

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 19
CRAP Score 7.0422

Importance

Changes 0
Metric Value
dl 0
loc 42
ccs 19
cts 21
cp 0.9048
rs 6.7272
c 0
b 0
f 0
cc 7
eloc 23
nc 7
nop 3
crap 7.0422
1
<?php
2
3
/**
4
 * Encapsulated manipulation of the current order using a singleton pattern.
5
 *
6
 * Ensures that an order is only started (persisted to DB) when necessary,
7
 * and all future changes are on the same order, until the order has is placed.
8
 * The requirement for starting an order is to adding an item to the cart.
9
 *
10
 * @package shop
11
 */
12
class ShoppingCart extends Object
0 ignored issues
show
Complexity introduced by
This class has a complexity of 63 which exceeds the configured maximum of 50.

The class complexity is the sum of the complexity of all methods. A very high value is usually an indication that your class does not follow the single reponsibility principle and does more than one job.

Some resources for further reading:

You can also find more detailed suggestions for refactoring in the “Code” section of your repository.

Loading history...
13
{
14
    private static $cartid_session_name = 'shoppingcartid';
15
16
    private $order;
17
18
    private $calculateonce = false;
19
20
    private $message;
21
22
    private $type;
23
24
    /**
25
     * Access for only allowing access to one (singleton) ShoppingCart.
26
     *
27
     * @return ShoppingCart
28
     */
29 107
    public static function singleton()
30
    {
31 107
        return Injector::inst()->get('ShoppingCart');
32
    }
33
34
    /**
35
     * Shortened alias for ShoppingCart::singleton()->current()
36
     *
37
     * @return Order
38
     */
39 16
    public static function curr()
40
    {
41 16
        return self::singleton()->current();
42
    }
43
44
    /**
45
     * Get the current order, or return null if it doesn't exist.
46
     *
47
     * @return Order
48
     */
49 107
    public function current()
50
    {
51
        //find order by id saved to session (allows logging out and retaining cart contents)
52 107
        if (!$this->order && $sessionid = Session::get(self::config()->cartid_session_name)) {
53
            $this->order = Order::get()->filter(
54
                array(
55
                    "Status" => "Cart",
56
                    "ID" => $sessionid,
57
                )
58
            )->first();
59
        }
60 107
        if (!$this->calculateonce && $this->order) {
61 9
            $this->order->calculate();
62 9
            $this->calculateonce = true;
63 9
        }
64
65 107
        return $this->order ? $this->order : false;
66
    }
67
68
    /**
69
     * Set the current cart
70
     *
71
     * @param Order
72
     *
73
     * @return ShoppingCart
74
     */
75 28
    public function setCurrent(Order $cart)
76
    {
77 7
        if (!$cart->IsCart()) {
78
            trigger_error("Passed Order object is not cart status", E_ERROR);
79
        }
80 7
        $this->order = $cart;
81 7
        Session::set(self::config()->cartid_session_name, $cart->ID);
82
83 28
        return $this;
84
    }
85
86
    /**
87
     * Helper that only allows orders to be started internally.
88
     *
89
     * @return Order
90
     */
91 21
    protected function findOrMake()
92
    {
93 21
        if ($this->current()) {
94 21
            return $this->current();
95
        }
96 14
        $this->order = Order::create();
97 14
        if (Member::config()->login_joins_cart && Member::currentUserID()) {
98 5
            $this->order->MemberID = Member::currentUserID();
99 5
        }
100 14
        $this->order->write();
101 14
        $this->order->extend('onStartOrder');
102 14
        Session::set(self::config()->cartid_session_name, $this->order->ID);
103
104 14
        return $this->order;
105
    }
106
107
    /**
108
     * Adds an item to the cart
109
     *
110
     * @param Buyable $buyable
111
     * @param number $quantity
112
     * @param unknown $filter
113
     *
114
     * @return boolean|OrderItem false or the new/existing item
115
     */
116 20
    public function add(Buyable $buyable, $quantity = 1, $filter = array())
117
    {
118 20
        $order = $this->findOrMake();
119 20
120 20
        // If an extension throws an exception, error out
121
        try {
122
            $order->extend("beforeAdd", $buyable, $quantity, $filter);
123
        } catch (Exception $exception){
124 20
            return $this->error($exception->getMessage());
125 20
        }
126
127 1
        if (!$buyable) {
128
            return $this->error(_t("ShoppingCart.ProductNotFound", "Product not found."));
129 20
        }
130 4
131 4
        $item = $this->findOrMakeItem($buyable, $quantity, $filter);
132 20
        if (!$item) {
133
            return false;
134 20
        }
135 20
        if (!$item->_brandnew) {
0 ignored issues
show
Documentation introduced by
The property _brandnew does not exist on object<OrderItem>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
136 20
            $item->Quantity += $quantity;
0 ignored issues
show
Documentation introduced by
The property Quantity does not exist on object<OrderItem>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
137
        } else {
138 20
            $item->Quantity = $quantity;
0 ignored issues
show
Documentation introduced by
The property Quantity does not exist on object<OrderItem>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
139
        }
140
141
        // If an extension throws an exception, error out
142
        try {
143
            $order->extend("afterAdd", $item, $buyable, $quantity, $filter);
144
        } catch (Exception $exception){
145
            return $this->error($exception->getMessage());
146
        }
147
148
        $item->write();
149
        $this->message(_t("ShoppingCart.ItemAdded", "Item has been added successfully."));
150 4
151
        return $item;
152 4
    }
153
154 4
    /**
155
     * Remove an item from the cart.
156
     *
157
     * @param     id        or Buyable $buyable
158 4
     * @param     $item
159
     * @param int $quantity - number of items to remove, or leave null for all items (default)
160 4
     *
161
     * @return boolean success/failure
162 4
     */
163 1
    public function remove(Buyable $buyable, $quantity = null, $filter = array())
164
    {
165
        $order = $this->current();
166
167 4
        if (!$order) {
168 4
            return $this->error(_t("ShoppingCart.NoOrder", "No current order."));
169 4
        }
170 4
171 1
        // If an extension throws an exception, error out
172 1
        try {
173
            $order->extend("beforeRemove", $buyable, $quantity, $filter);
174 4
        } catch (Exception $exception){
175 4
            return $this->error($exception->getMessage());
176
        }
177 4
178
        $item = $this->get($buyable, $filter);
179
180
        if (!$item) {
181
            return false;
182
        }
183
184
        //if $quantity will become 0, then remove all
185
        if (!$quantity || ($item->Quantity - $quantity) <= 0) {
0 ignored issues
show
Documentation introduced by
The property Quantity does not exist on object<OrderItem>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
186
            $item->delete();
187
            $item->destroy();
188
        } else {
189
            $item->Quantity -= $quantity;
0 ignored issues
show
Documentation introduced by
The property Quantity does not exist on object<OrderItem>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
190 3
            $item->write();
191
        }
192 3
193
        // If an extension throws an exception, error out
194
        // TODO: There should be a rollback
195 3
        try {
196 3
            $order->extend("afterRemove", $item, $buyable, $quantity, $filter);
197 3
        } catch (Exception $exception){
198
            return $this->error($exception->getMessage());
199
        }
200
201 3
        $this->message(_t("ShoppingCart.ItemRemoved", "Item has been successfully removed."));
202 3
203 3
        return true;
204 3
    }
205 3
206
    /**
207 3
     * Sets the quantity of an item in the cart.
208
     * Will automatically add or remove item, if necessary.
209
     *
210
     * @param     id or Buyable $buyable
211
     * @param     $item
212
     * @param int $quantity
213
     *
214
     * @return boolean|OrderItem false or the new/existing item
215
     */
216
    public function setQuantity(Buyable $buyable, $quantity = 1, $filter = array())
217
    {
218
        if ($quantity <= 0) {
219 21
            return $this->remove($buyable, $quantity, $filter);
220
        }
221 21
        $order = $this->findOrMake();
222
        $item = $this->findOrMakeItem($buyable, $quantity, $filter);
223 21
        if (!$item) {
224
225
            return false;
226
        }
227 21
228
        // If an extension throws an exception, error out
229 21
        try {
230 21
            $order->extend("beforeSetQuantity", $buyable, $quantity, $filter);
231
        } catch (Exception $exception){
232 21
            return $this->error($exception->getMessage());
233
        }
234 21
235 1
        $item->Quantity = $quantity;
0 ignored issues
show
Documentation introduced by
The property Quantity does not exist on object<OrderItem>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
236 1
237 1
        // If an extension throws an exception, error out
238 1
        try {
239 1
            $order->extend("afterSetQuantity", $item, $buyable, $quantity, $filter);
240 1
        } catch (Exception $exception){
241 1
            return $this->error($exception->getMessage());
242 1
        }
243
244
        $item->write();
245
        $this->message(_t("ShoppingCart.QuantitySet", "Quantity has been set."));
246 21
247 21
        return $item;
248 21
    }
249
250 21
    /**
251
     * Finds or makes an order item for a given product + filter.
252 21
     *
253 21
     * @param Buyable $buyable the buyable
254
     * @param int $quantity quantity to add
255 21
     * @param array $filter
256
     *
257
     * @return OrderItem the found or created item
258
     */
259
    private function findOrMakeItem(Buyable $buyable, $quantity = 1, $filter = array())
260
    {
261
        $order = $this->findOrMake();
262
263
        if (!$buyable || !$order) {
264
            return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type documented by ShoppingCart::findOrMakeItem of type OrderItem.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
265
        }
266 26
267 4
        $item = $this->get($buyable, $filter);
268 26
269 26
        if (!$item) {
270 4
            $member = Member::currentUser();
271
272
            $buyable = $this->getCorrectBuyable($buyable);
273 24
274
            if (!$buyable->canPurchase($member, $quantity)) {
0 ignored issues
show
Bug introduced by
It seems like $member defined by \Member::currentUser() on line 270 can also be of type object<DataObject>; however, Buyable::canPurchase() does only seem to accept object<Member>|null, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
275
                return $this->error(
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $this->error(_t('...18n_singular_name()))); (boolean) is incompatible with the return type documented by ShoppingCart::findOrMakeItem of type OrderItem.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
276 24
                    _t(
277 24
                        'ShoppingCart.CannotPurchase',
278 24
                        'This {Title} cannot be purchased.',
279 24
                        '',
280 24
                        array('Title' => $buyable->i18n_singular_name())
0 ignored issues
show
Documentation introduced by
array('Title' => $buyable->i18n_singular_name()) is of type array<string,?,{"Title":"?"}>, but the function expects a string.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
281 24
                    )
282 24
                );
283 24
                //TODO: produce a more specific message
284 24
            }
285 24
286 24
            $item = $buyable->createItem($quantity, $filter);
287 24
            $item->OrderID = $order->ID;
0 ignored issues
show
Documentation introduced by
The property OrderID does not exist on object<OrderItem>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
288 24
            $item->write();
289
290
            $order->Items()->add($item);
0 ignored issues
show
Documentation Bug introduced by
The method Items does not exist on object<Order>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
291 12
292
            $item->_brandnew = true; // flag as being new
0 ignored issues
show
Documentation introduced by
The property _brandnew does not exist on object<OrderItem>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
293
        }
294
295
        return $item;
296
    }
297
298
    /**
299
     * Finds an existing order item.
300
     *
301 24
     * @param Buyable $buyable
302
     * @param array $filter
0 ignored issues
show
Documentation introduced by
There is no parameter named $filter. Did you maybe mean $customfilter?

This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit.

Consider the following example. The parameter $ireland is not defined by the method finale(...).

/**
 * @param array $germany
 * @param array $ireland
 */
function finale($germany, $island) {
    return "2:1";
}

The most likely cause is that the parameter was changed, but the annotation was not.

Loading history...
303
     *
304 24
     * @return OrderItem the item requested, or false
305 24
     */
306 21
    public function get(Buyable $buyable, $customfilter = array())
307 24
    {
308 1
        $order = $this->current();
309 1
        if (!$buyable || !$order) {
310 1
            return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type documented by ShoppingCart::get of type OrderItem.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
311
        }
312 1
313
        $buyable = $this->getCorrectBuyable($buyable);
314
315 23
        $filter = array(
316
            'OrderID' => $order->ID,
317
        );
318
        $itemclass = Config::inst()->get(get_class($buyable), 'order_item');
319
        $relationship = Config::inst()->get($itemclass, 'buyable_relationship');
320
        $filter[$relationship . "ID"] = $buyable->ID;
0 ignored issues
show
Bug introduced by
Accessing ID on the interface Buyable suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
321
        $required = array('Order', $relationship);
322 17
        if (is_array($itemclass::config()->required_fields)) {
323
            $required = array_merge($required, $itemclass::config()->required_fields);
324
        }
325
        $query = new MatchObjectFilter($itemclass, array_merge($customfilter, $filter), $required);
326
        $item = $itemclass::get()->where($query->getFilter())->first();
327
        if (!$item) {
328
            return $this->error(_t("ShoppingCart.ItemNotFound", "Item not found."));
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $this->error(_t('...', 'Item not found.')); (boolean) is incompatible with the return type documented by ShoppingCart::get of type OrderItem.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
329
        }
330
331
        return $item;
332
    }
333
334
    /**
335
     * Ensure the proper buyable will be returned for a given buyable…
336
     * This is being used to ensure a product with variations cannot be added to the cart…
337
     * a Variation has to be added instead!
338 17
     * @param Buyable $buyable
339
     * @return Buyable
340
     */
341
    public function getCorrectBuyable(Buyable $buyable)
342
    {
343
        if (
344
            $buyable instanceof Product &&
345
            $buyable->hasExtension('ProductVariationsExtension') &&
346 11
            $buyable->Variations()->count() > 0
347
        ) {
348 11
            foreach ($buyable->Variations() as $variation) {
349 11
                if ($variation->canPurchase()) {
350 11
                    return $variation;
351 11
                }
352 5
            }
353
        }
354 10
355 6
        return $buyable;
356 6
    }
357 10
358
    /**
359 10
     * Store old cart id in session order history
360
     * @param int|null $requestedOrderId optional parameter that denotes the order that was requested
361
     */
362
    public function archiveorderid($requestedOrderId = null)
363
    {
364
        $sessionId = Session::get(self::config()->cartid_session_name);
365 25
        $order = Order::get()
366
            ->filter("Status:not", "Cart")
367 25
            ->byId($sessionId);
368
        if ($order && !$order->IsCart()) {
369 25
            OrderManipulation::add_session_order($order);
0 ignored issues
show
Compatibility introduced by
$order of type object<DataObject> is not a sub-type of object<Order>. It seems like you assume a child class of the class DataObject to be always present.

This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass.

Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type.

Loading history...
370
        }
371
        // in case there was no order requested
372
        // OR there was an order requested AND it's the same one as currently in the session,
373
        // then clear the cart. This check is here to prevent clearing of the cart if the user just
374
        // wants to view an old order (via AccountPage).
375
        if (!$requestedOrderId || ($sessionId == $requestedOrderId)) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $requestedOrderId of type integer|null is loosely compared to false; this is ambiguous if the integer can be zero. You might want to explicitly use === null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For integer values, zero is a special case, in particular the following results might be unexpected:

0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
Loading history...
376
            $this->clear();
377
        }
378 26
    }
379
380 26
    /**
381 26
     * Empty / abandon the entire cart.
382 26
     *
383
     * @param bool $write whether or not to write the abandoned order
384
     * @return bool - true if successful, false if no cart found
385
     */
386
    public function clear($write = true)
387
    {
388
        Session::clear(self::config()->cartid_session_name);
389
        $order = $this->current();
390
        $this->order = null;
391
        if (!$order) {
392
            return $this->error(_t("ShoppingCart.NoCartFound", "No cart found."));
393
        }
394
        if ($write) {
395
            $order->write();
396
        }
397
        $this->message(_t("ShoppingCart.Cleared", "Cart was successfully cleared."));
398
399
        return true;
400
    }
401
402
    /**
403
     * Store a new error.
404
     */
405
    protected function error($message)
406
    {
407
        $this->message($message, "bad");
408
409
        return false;
410
    }
411
412
    /**
413
     * Store a message to be fed back to user.
414
     *
415
     * @param string $message
416
     * @param string $type - good, bad, warning
417
     */
418
    protected function message($message, $type = "good")
419
    {
420
        $this->message = $message;
421
        $this->type = $type;
422
    }
423
424
    public function getMessage()
425
    {
426
        return $this->message;
427
    }
428
429
    public function getMessageType()
430
    {
431
        return $this->type;
432
    }
433
434
    public function clearMessage()
435
    {
436
        $this->message = null;
437
    }
438
439 6
    //singleton protection
440
    public function __clone()
441 6
    {
442
        trigger_error('Clone is not allowed.', E_USER_ERROR);
443
    }
444 2
445
    public function __wakeup()
446 2
    {
447
        trigger_error('Unserializing is not allowed.', E_USER_ERROR);
448
    }
449 2
}
450
451 2
/**
452
 * Manipulate the cart via urls.
453
 */
454 3
class ShoppingCart_Controller extends Controller
0 ignored issues
show
Complexity introduced by
This class has a complexity of 52 which exceeds the configured maximum of 50.

The class complexity is the sum of the complexity of all methods. A very high value is usually an indication that your class does not follow the single reponsibility principle and does more than one job.

Some resources for further reading:

You can also find more detailed suggestions for refactoring in the “Code” section of your repository.

Loading history...
455
{
456 3
    private static $url_segment         = "shoppingcart";
457
458
    private static $direct_to_cart_page = false;
459
460
    protected      $cart;
461
462 6
    private static $url_handlers        = array(
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
463
        '$Action/$Buyable/$ID' => 'handleAction',
464 6
    );
465
466
    private static $allowed_actions     = array(
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
467 6
        'add',
468 1
        'additem',
469 1
        'remove',
470 6
        'removeitem',
471 6
        'removeall',
472 6
        'removeallitem',
473 6
        'setquantity',
474 6
        'setquantityitem',
475
        'clear',
476
        'debug',
477
    );
478
479
    public static function add_item_link(Buyable $buyable, $parameters = array())
480
    {
481
        return self::build_url("add", $buyable, $parameters);
482
    }
483
484
    public static function remove_item_link(Buyable $buyable, $parameters = array())
485 6
    {
486
        return self::build_url("remove", $buyable, $parameters);
487 6
    }
488 3
489 3
    public static function remove_all_item_link(Buyable $buyable, $parameters = array())
490
    {
491 6
        return self::build_url("removeall", $buyable, $parameters);
492
    }
493
494
    public static function set_quantity_item_link(Buyable $buyable, $parameters = array())
495
    {
496
        return self::build_url("setquantity", $buyable, $parameters);
497
    }
498
499
    /**
500
     * Helper for creating a url
501 4
     */
502
    protected static function build_url($action, $buyable, $params = array())
503 4
    {
504
        if (!$action || !$buyable) {
505
            return false;
506 4
        }
507
        if (SecurityToken::is_enabled() && !self::config()->disable_security_token) {
508
            $params[SecurityToken::inst()->getName()] = SecurityToken::inst()->getValue();
509
        }
510 4
        return self::config()->url_segment . '/' .
511 4
        $action . '/' .
512
        $buyable->class . "/" .
513
        $buyable->ID .
514
        self::params_to_get_string($params);
515 4
    }
516
517 4
    /**
518 4
     * Creates the appropriate string parameters for links from array
519 4
     *
520
     * Produces string such as: MyParam%3D11%26OtherParam%3D1
521
     *     ...which decodes to: MyParam=11&OtherParam=1
522
     *
523
     * you will need to decode the url with javascript before using it.
524 4
     */
525
    protected static function params_to_get_string($array)
526 4
    {
527
        if ($array & count($array > 0)) {
528 4
            array_walk($array, create_function('&$v,$k', '$v = $k."=".$v ;'));
529 4
            return "?" . implode("&", $array);
530 1
        }
531 4
        return "";
532 1
    }
533 1
534 1
    /**
535 1
     * This is used here and in VariationForm and AddProductForm
536
     *
537 4
     * @param bool|string $status
538 4
     *
539
     * @return bool
540
     */
541
    public static function direct($status = true)
542 4
    {
543 4
        if (Director::is_ajax()) {
544 4
            return $status;
545 4
        }
546 4
        if (self::config()->direct_to_cart_page && $cartlink = CartPage::find_link()) {
547
            Controller::curr()->redirect($cartlink);
548
            return;
549
        } else {
550
            Controller::curr()->redirectBack();
551 4
            return;
552
        }
553 4
    }
554 4
555 4
    public function init()
556 4
    {
557
        parent::init();
558 1
        $this->cart = ShoppingCart::singleton();
559
    }
560
561 4
    /**
562
     * @return Product|ProductVariation|Buyable
563
     */
564
    protected function buyableFromRequest()
0 ignored issues
show
Complexity introduced by
This operation has 480 execution paths which exceeds the configured maximum of 200.

A high number of execution paths generally suggests many nested conditional statements and make the code less readible. This can usually be fixed by splitting the method into several smaller methods.

You can also find more information in the “Code” section of your repository.

Loading history...
565
    {
566
        $request = $this->getRequest();
567
        if (
568
            SecurityToken::is_enabled() &&
569
            !self::config()->disable_security_token &&
570
            !SecurityToken::inst()->checkRequest($request)
571 4
        ) {
572
            return $this->httpError(
573 4
                400,
574 4
                _t("ShoppingCart.InvalidSecurityToken", "Invalid security token, possible CSRF attack.")
575 4
            );
576 4
        }
577 4
        $id = (int)$request->param('ID');
578 4
        if (empty($id)) {
579 4
            //TODO: store error message
580
            return null;
581 4
        }
582 4
        $buyableclass = "Product";
583 4
        if ($class = $request->param('Buyable')) {
584
            $buyableclass = Convert::raw2sql($class);
585
        }
586
        if (!ClassInfo::exists($buyableclass)) {
0 ignored issues
show
Bug introduced by
It seems like $buyableclass defined by \Convert::raw2sql($class) on line 584 can also be of type array; however, ClassInfo::exists() does only seem to accept string, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
587
            //TODO: store error message
588
            return null;
589
        }
590
        //ensure only live products are returned, if they are versioned
591
        $buyable = Object::has_extension($buyableclass, 'Versioned')
0 ignored issues
show
Bug introduced by
It seems like $buyableclass defined by \Convert::raw2sql($class) on line 584 can also be of type array; however, Object::has_extension() does only seem to accept string, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
592
            ?
593 1
            Versioned::get_by_stage($buyableclass, 'Live')->byID($id)
0 ignored issues
show
Bug introduced by
It seems like $buyableclass defined by \Convert::raw2sql($class) on line 584 can also be of type array; however, Versioned::get_by_stage() does only seem to accept string, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
594
            :
595 1
            DataObject::get($buyableclass)->byID($id);
0 ignored issues
show
Bug introduced by
It seems like $buyableclass defined by \Convert::raw2sql($class) on line 584 can also be of type array; however, DataObject::get() does only seem to accept string|null, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
596 1
        if (!$buyable || !($buyable instanceof Buyable)) {
597 1
            //TODO: store error message
598
            return null;
599 1
        }
600 1
601 1
        return $this->cart->getCorrectBuyable($buyable);
602
    }
603
604
    /**
605
     * Action: add item to cart
606
     *
607
     * @param SS_HTTPRequest $request
608
     *
609
     * @return SS_HTTPResponse
610
     */
611 1
    public function add($request)
612
    {
613 1
        if ($product = $this->buyableFromRequest()) {
614 1
            $quantity = (int)$request->getVar('quantity');
615 1
            if (!$quantity) {
616
                $quantity = 1;
617 1
            }
618 1
            $this->cart->add($product, $quantity, $request->getVars());
619 1
        }
620
621
        $this->updateLocale($request);
622
        $this->extend('updateAddResponse', $request, $response, $product, $quantity);
623
        return $response ? $response : self::direct();
624
    }
625
626
    /**
627
     * Action: remove a certain number of items from the cart
628
     *
629 2
     * @param SS_HTTPRequest $request
630
     *
631 2
     * @return SS_HTTPResponse
632 2
     */
633 2
    public function remove($request)
634 2
    {
635 2
        if ($product = $this->buyableFromRequest()) {
636
            $this->cart->remove($product, $quantity = 1, $request->getVars());
637 2
        }
638 2
639 2
        $this->updateLocale($request);
640
        $this->extend('updateRemoveResponse', $request, $response, $product, $quantity);
641
        return $response ? $response : self::direct();
642
    }
643
644
    /**
645
     * Action: remove all of an item from the cart
646
     *
647
     * @param SS_HTTPRequest $request
648
     *
649
     * @return SS_HTTPResponse
650
     */
651
    public function removeall($request)
652
    {
653
        if ($product = $this->buyableFromRequest()) {
654
            $this->cart->remove($product, null, $request->getVars());
655
        }
656
657
        $this->updateLocale($request);
658
        $this->extend('updateRemoveAllResponse', $request, $response, $product);
659
        return $response ? $response : self::direct();
660
    }
661
662
    /**
663
     * Action: update the quantity of an item in the cart
664
     *
665
     * @param $request
666
     *
667
     * @return AjaxHTTPResponse|bool
668
     */
669
    public function setquantity($request)
670
    {
671
        $product = $this->buyableFromRequest();
672
        $quantity = (int)$request->getVar('quantity');
673
        if ($product) {
674
            $this->cart->setQuantity($product, $quantity, $request->getVars());
675
        }
676
677
        $this->updateLocale($request);
678
        $this->extend('updateSetQuantityResponse', $request, $response, $product, $quantity);
679
        return $response ? $response : self::direct();
680
    }
681
682
    /**
683
     * Action: clear the cart
684
     *
685
     * @param $request
686
     *
687
     * @return AjaxHTTPResponse|bool
688
     */
689 4
    public function clear($request)
690
    {
691 4
        $this->updateLocale($request);
692 4
        $this->cart->clear();
693
        $this->extend('updateClearResponse', $request, $response);
694
        return $response ? $response : self::direct();
695 4
    }
696
697 2
    /**
698
     * Handle index requests
699
     */
700
    public function index()
701
    {
702
        if ($cart = $this->Cart()) {
0 ignored issues
show
Documentation Bug introduced by
The method Cart does not exist on object<ShoppingCart_Controller>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
703
            $this->redirect($cart->CartLink);
704
            return;
705
        } elseif ($response = ErrorPage::response_for(404)) {
706
            return $response;
707
        }
708
        return $this->httpError(404, _t("ShoppingCart.NoCartInitialised", "no cart initialised"));
709
    }
710
711
    /**
712
     * Displays order info and cart contents.
713
     */
714
    public function debug()
715
    {
716
        if (Director::isDev() || Permission::check("ADMIN")) {
717
            //TODO: allow specifying a particular id to debug
718
            Requirements::css(SHOP_DIR . "/css/cartdebug.css");
719
            $order = ShoppingCart::curr();
720
            $content = ($order)
721
                ?
722
                Debug::text($order)
0 ignored issues
show
Documentation introduced by
$order is of type object<Order>, but the function expects a object<unknown_type>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
723
                :
724
                "Cart has not been created yet. Add a product.";
725
            return array('Content' => $content);
0 ignored issues
show
Bug Best Practice introduced by
The return type of return array('Content' => $content); (array<string,unknown|string>) is incompatible with the return type of the parent method ViewableData::Debug of type ViewableData_Debugger.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
726
        }
727
    }
728
729
    protected function updateLocale($request)
730
    {
731
        $order = $this->cart->current();
732
        if ($request && $request->isAjax() && $order) {
733
            ShopTools::install_locale($order->Locale);
734
        }
735
    }
736
}
737