Completed
Pull Request — 2.0 (#516)
by Roman
29:35
created

ShoppingCart   C

Complexity

Total Complexity 57

Size/Duplication

Total Lines 397
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 11

Test Coverage

Coverage 79.64%

Importance

Changes 8
Bugs 3 Features 3
Metric Value
wmc 57
c 8
b 3
f 3
lcom 1
cbo 11
dl 0
loc 397
ccs 133
cts 167
cp 0.7964
rs 6.433

20 Methods

Rating   Name   Duplication   Size   Complexity  
A singleton() 0 4 1
A curr() 0 4 1
B current() 0 18 6
A setCurrent() 0 10 2
A findOrMake() 0 15 4
A setQuantity() 0 19 3
B add() 0 24 4
B remove() 0 29 5
B findOrMakeItem() 0 38 5
B get() 0 27 5
B getCorrectBuyable() 0 16 6
B archiveorderid() 0 17 5
A clear() 0 15 3
A error() 0 6 1
A message() 0 5 1
A getMessage() 0 4 1
A getMessageType() 0 4 1
A clearMessage() 0 4 1
A __clone() 0 4 1
A __wakeup() 0 4 1

How to fix   Complexity   

Complex Class

Complex classes like ShoppingCart often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use ShoppingCart, and based on these observations, apply Extract Interface, too.

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 57 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 101
    public static function singleton()
30
    {
31 101
        return Injector::inst()->get('ShoppingCart');
32
    }
33
34
    /**
35
     * Shortened alias for ShoppingCart::singleton()->current()
36
     *
37
     * @return Order
38
     */
39 18
    public static function curr()
40
    {
41 18
        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 101
    public function current()
50
    {
51
        //find order by id saved to session (allows logging out and retaining cart contents)
52 101
        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 101
        if (!$this->calculateonce && $this->order) {
61 9
            $this->order->calculate();
62 9
            $this->calculateonce = true;
63 9
        }
64
65 101
        return $this->order ? $this->order : false;
66
    }
67
68
    /**
69
     * Set the current cart
70
     *
71
     * @param Order
72
     *
73
     * @return ShoppingCart
74
     */
75 20
    public function setCurrent(Order $cart)
76 20
    {
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 7
        return $this;
84
    }
85
86
    /**
87
     * Helper that only allows orders to be started internally.
88
     *
89
     * @return Order
90
     */
91 20
    protected function findOrMake()
92
    {
93 20
        if ($this->current()) {
94 20
            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 19
    public function add(Buyable $buyable, $quantity = 1, $filter = array())
117
    {
118 19
        $order = $this->findOrMake();
119 19
        $order->extend("beforeAdd", $buyable, $quantity, $filter);
120 19
        if (!$buyable) {
121
122
            return $this->error(_t("ShoppingCart.ProductNotFound", "Product not found."));
123
        }
124 19
        $item = $this->findOrMakeItem($buyable, $filter);
125 19
        if (!$item) {
126
127 1
            return false;
128
        }
129 19
        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...
130 4
            $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...
131 4
        } else {
132 19
            $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...
133
        }
134 19
        $item->write();
135 19
        $order->extend("afterAdd", $item, $buyable, $quantity, $filter);
136 19
        $this->message(_t("ShoppingCart.ItemAdded", "Item has been added successfully."));
137
138 19
        return $item;
139
    }
140
141
    /**
142
     * Remove an item from the cart.
143
     *
144
     * @param     id        or Buyable $buyable
145
     * @param     $item
146
     * @param int $quantity - number of items to remove, or leave null for all items (default)
147
     *
148
     * @return boolean success/failure
149
     */
150 4
    public function remove(Buyable $buyable, $quantity = null, $filter = array())
151
    {
152 4
        $order = $this->current();
153
154 4
        if (!$order) {
155
            return $this->error(_t("ShoppingCart.NoOrder", "No current order."));
156
        }
157
158 4
        $order->extend("beforeRemove", $buyable, $quantity, $filter);
159
160 4
        $item = $this->get($buyable, $filter);
161
162 4
        if (!$item) {
163 1
            return false;
164
        }
165
166
        //if $quantity will become 0, then remove all
167 4
        if (!$quantity || ($item->Quantity - $quantity) <= 0) {
168 4
            $item->delete();
169 4
            $item->destroy();
170 4
        } else {
171 1
            $item->Quantity -= $quantity;
172 1
            $item->write();
173
        }
174 4
        $order->extend("afterRemove", $item, $buyable, $quantity, $filter);
175 4
        $this->message(_t("ShoppingCart.ItemRemoved", "Item has been successfully removed."));
176
177 4
        return true;
178
    }
179
180
    /**
181
     * Sets the quantity of an item in the cart.
182
     * Will automatically add or remove item, if necessary.
183
     *
184
     * @param     id or Buyable $buyable
185
     * @param     $item
186
     * @param int $quantity
187
     *
188
     * @return boolean|OrderItem false or the new/existing item
189
     */
190 3
    public function setQuantity(Buyable $buyable, $quantity = 1, $filter = array())
191
    {
192 3
        if ($quantity <= 0) {
193
            return $this->remove($buyable, $quantity, $filter);
194
        }
195 3
        $order = $this->findOrMake();
196 3
        $item = $this->findOrMakeItem($buyable, $filter);
197 3
        if (!$item) {
198
199
            return false;
200
        }
201 3
        $order->extend("beforeSetQuantity", $buyable, $quantity, $filter);
202 3
        $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...
203 3
        $item->write();
204 3
        $order->extend("afterSetQuantity", $item, $buyable, $quantity, $filter);
205 3
        $this->message(_t("ShoppingCart.QuantitySet", "Quantity has been set."));
206
207 3
        return $item;
208
    }
209
210
    /**
211
     * Finds or makes an order item for a given product + filter.
212
     *
213
     * @param        id or Buyable $buyable
214
     * @param string $filter
215
     *
216
     * @return OrderItem the found or created item
217
     */
218 20
    private function findOrMakeItem(Buyable $buyable, $filter = array())
219
    {
220 20
        $order = $this->findOrMake();
221
222 20
        if (!$buyable || !$order) {
223 3
            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...
224
        }
225
226 20
        $item = $this->get($buyable, $filter);
0 ignored issues
show
Bug introduced by
It seems like $filter defined by parameter $filter on line 218 can also be of type string; however, ShoppingCart::get() does only seem to accept array, maybe add an additional type check?

This check looks at variables that have been passed in as parameters and are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
Bug Compatibility introduced by
The expression $this->get($buyable, $filter); of type the adds the type the to the return on line 254 which is incompatible with the return type documented by ShoppingCart::findOrMakeItem of type OrderItem.
Loading history...
227
228 20
        if (!$item) {
229 20
            $member = Member::currentUser();
230
231 20
            $buyable = $this->getCorrectBuyable($buyable);
232 1
233 1
            if (!$buyable->canPurchase($member)) {
0 ignored issues
show
Bug introduced by
It seems like $member defined by \Member::currentUser() on line 229 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...
234 1
                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...
235 1
                    _t(
236 1
                        'ShoppingCart.CannotPurchase',
237 1
                        'This {Title} cannot be purchased.',
238 1
                        '',
239 1
                        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...
240
                    )
241
                );
242
                //TODO: produce a more specific message
243 20
            }
244 20
245 20
            $item = $buyable->createItem(1, $filter);
0 ignored issues
show
Bug introduced by
It seems like $filter defined by parameter $filter on line 218 can also be of type string; however, Buyable::createItem() does only seem to accept array, maybe add an additional type check?

This check looks at variables that have been passed in as parameters and are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
246
            $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...
247 20
            $item->write();
248
249 20
            $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...
250 20
251
            $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...
252 20
        }
253
254
        return $item;
255
    }
256
257
    /**
258
     * Finds an existing order item.
259
     *
260
     * @param Buyable $buyable
261
     * @param string $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...
262
     *
263 23
     * @return the item requested, or false
264 3
     */
265 23
    public function get(Buyable $buyable, $customfilter = array())
266 23
    {
267 5
        $order = $this->current();
268
        if (!$buyable || !$order) {
269
            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 the.

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...
270 22
        }
271 22
272 22
        $buyable = $this->getCorrectBuyable($buyable);
273 22
274 22
        $filter = array(
275 22
            'OrderID' => $order->ID,
276 22
        );
277 22
        $itemclass = Config::inst()->get(get_class($buyable), 'order_item');
278 22
        $relationship = Config::inst()->get($itemclass, 'buyable_relationship');
279 22
        $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...
280 22
        $required = array('Order', $relationship);
281 22
        if (is_array($itemclass::config()->required_fields)) {
282 22
            $required = array_merge($required, $itemclass::config()->required_fields);
283
        }
284
        $query = new MatchObjectFilter($itemclass, array_merge($customfilter, $filter), $required);
285 12
        $item = $itemclass::get()->where($query->getFilter())->first();
286
        if (!$item) {
287
            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 the.

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...
288
        }
289
290
        return $item;
291
    }
292 1
293
    /**
294
     * Ensure the proper buyable will be returned for a given buyable…
295 1
     * This is being used to ensure a product with variations cannot be added to the cart…
296
     * a Variation has to be added instead!
297
     * @param Buyable $buyable
298
     * @return Buyable
299
     */
300
    public function getCorrectBuyable(Buyable $buyable)
301
    {
302
        if (
303
            $buyable instanceof Product &&
304
            $buyable->hasExtension('ProductVariationsExtension') &&
305
            $buyable->Variations()->count() > 0
306
        ) {
307
            foreach ($buyable->Variations() as $variation) {
308
                if ($variation->canPurchase()) {
309
                    return $variation;
310
                }
311
            }
312
        }
313
314
        return $buyable;
315
    }
316 16
317
    /**
318 11
     * Store old cart id in session order history
319 11
     * @param int|null $requestedOrderId optional parameter that denotes the order that was requested
320 11
     */
321 11
    public function archiveorderid($requestedOrderId = null)
322 5
    {
323
        $sessionId = Session::get(self::config()->cartid_session_name);
324 10
        $order = Order::get()
325 6
            ->filter("Status:not", "Cart")
326 6
            ->byId($sessionId);
327 16
        if ($order && !$order->IsCart()) {
328
            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...
329 10
        }
330
        // in case there was no order requested
331
        // OR there was an order requested AND it's the same one as currently in the session,
332
        // then clear the cart. This check is here to prevent clearing of the cart if the user just
333
        // wants to view an old order (via AccountPage).
334
        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...
335 23
            $this->clear();
336
        }
337 23
    }
338
339 23
    /**
340 17
     * Empty / abandon the entire cart.
341
     *
342
     * @param bool $write whether or not to write the abandoned order
343
     * @return bool - true if successful, false if no cart found
344
     */
345
    public function clear($write = true)
346
    {
347
        Session::clear(self::config()->cartid_session_name);
348 24
        $order = $this->current();
349
        $this->order = null;
350 24
        if (!$order) {
351 24
            return $this->error(_t("ShoppingCart.NoCartFound", "No cart found."));
352 24
        }
353
        if ($write) {
354
            $order->write();
355
        }
356
        $this->message(_t("ShoppingCart.Cleared", "Cart was successfully cleared."));
357
358
        return true;
359
    }
360
361
    /**
362
     * Store a new error.
363
     */
364
    protected function error($message)
365
    {
366
        $this->message($message, "bad");
367
368
        return false;
369
    }
370
371
    /**
372
     * Store a message to be fed back to user.
373
     *
374
     * @param string $message
375
     * @param string $type - good, bad, warning
376
     */
377
    protected function message($message, $type = "good")
378
    {
379
        $this->message = $message;
380
        $this->type = $type;
381
    }
382
383
    public function getMessage()
384
    {
385
        return $this->message;
386
    }
387
388
    public function getMessageType()
389
    {
390
        return $this->type;
391
    }
392
393
    public function clearMessage()
394
    {
395
        $this->message = null;
396
    }
397
398
    //singleton protection
399
    public function __clone()
400
    {
401
        trigger_error('Clone is not allowed.', E_USER_ERROR);
402
    }
403
404
    public function __wakeup()
405
    {
406
        trigger_error('Unserializing is not allowed.', E_USER_ERROR);
407
    }
408
}
409 6
410
/**
411 6
 * Manipulate the cart via urls.
412
 */
413
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...
414 2
{
415
    private static $url_segment         = "shoppingcart";
416 2
417
    private static $direct_to_cart_page = false;
418
419 2
    protected      $cart;
420
421 2
    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...
422
        '$Action/$Buyable/$ID' => 'handleAction',
423
    );
424 3
425
    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...
426 3
        'add',
427
        'additem',
428
        'remove',
429
        'removeitem',
430
        'removeall',
431
        'removeallitem',
432 6
        'setquantity',
433
        'setquantityitem',
434 6
        'clear',
435
        'debug',
436
    );
437 6
438 1
    public static function add_item_link(Buyable $buyable, $parameters = array())
439 1
    {
440 6
        return self::build_url("add", $buyable, $parameters);
441 6
    }
442 6
443 6
    public static function remove_item_link(Buyable $buyable, $parameters = array())
444 6
    {
445
        return self::build_url("remove", $buyable, $parameters);
446
    }
447
448
    public static function remove_all_item_link(Buyable $buyable, $parameters = array())
449
    {
450
        return self::build_url("removeall", $buyable, $parameters);
451
    }
452
453
    public static function set_quantity_item_link(Buyable $buyable, $parameters = array())
454
    {
455 6
        return self::build_url("setquantity", $buyable, $parameters);
456
    }
457 6
458 3
    /**
459 3
     * Helper for creating a url
460
     */
461 6
    protected static function build_url($action, $buyable, $params = array())
462
    {
463
        if (!$action || !$buyable) {
464
            return false;
465
        }
466
        if (SecurityToken::is_enabled() && !self::config()->disable_security_token) {
467
            $params[SecurityToken::inst()->getName()] = SecurityToken::inst()->getValue();
468
        }
469
        return self::config()->url_segment . '/' .
470
        $action . '/' .
471 4
        $buyable->class . "/" .
472
        $buyable->ID .
473 4
        self::params_to_get_string($params);
474
    }
475
476 4
    /**
477
     * Creates the appropriate string parameters for links from array
478
     *
479
     * Produces string such as: MyParam%3D11%26OtherParam%3D1
480 4
     *     ...which decodes to: MyParam=11&OtherParam=1
481 4
     *
482
     * you will need to decode the url with javascript before using it.
483
     */
484
    protected static function params_to_get_string($array)
485 4
    {
486
        if ($array & count($array > 0)) {
487 4
            array_walk($array, create_function('&$v,$k', '$v = $k."=".$v ;'));
488 4
            return "?" . implode("&", $array);
489 4
        }
490
        return "";
491
    }
492
493
    /**
494 4
     * This is used here and in VariationForm and AddProductForm
495
     *
496 4
     * @param bool|string $status
497
     *
498 4
     * @return bool
499 4
     */
500 1
    public static function direct($status = true)
501 4
    {
502 1
        if (Director::is_ajax()) {
503 1
            return $status;
504 1
        }
505 1
        if (self::config()->direct_to_cart_page && $cartlink = CartPage::find_link()) {
506
            Controller::curr()->redirect($cartlink);
507 4
            return;
508 4
        } else {
509
            Controller::curr()->redirectBack();
510
            return;
511
        }
512 4
    }
513 4
514 4
    public function init()
515 4
    {
516 4
        parent::init();
517
        $this->cart = ShoppingCart::singleton();
518
    }
519
520
    /**
521 4
     * @return Product|ProductVariation|Buyable
522
     */
523 4
    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...
524 4
    {
525 4
        $request = $this->getRequest();
526 4
        if (
527
            SecurityToken::is_enabled() &&
528 1
            !self::config()->disable_security_token &&
529
            !SecurityToken::inst()->checkRequest($request)
530 4
        ) {
531
            return $this->httpError(
532
                400,
533
                _t("ShoppingCart.InvalidSecurityToken", "Invalid security token, possible CSRF attack.")
534
            );
535
        }
536
        $id = (int)$request->param('ID');
537
        if (empty($id)) {
538
            //TODO: store error message
539
            return null;
540 4
        }
541
        $buyableclass = "Product";
542 4
        if ($class = $request->param('Buyable')) {
543 4
            $buyableclass = Convert::raw2sql($class);
544 4
        }
545 4
        if (!ClassInfo::exists($buyableclass)) {
0 ignored issues
show
Bug introduced by
It seems like $buyableclass defined by \Convert::raw2sql($class) on line 543 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...
546 4
            //TODO: store error message
547 4
            return null;
548 4
        }
549
        //ensure only live products are returned, if they are versioned
550 4
        $buyable = Object::has_extension($buyableclass, 'Versioned')
0 ignored issues
show
Bug introduced by
It seems like $buyableclass defined by \Convert::raw2sql($class) on line 543 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...
551 4
            ?
552 4
            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 543 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...
553
            :
554
            DataObject::get($buyableclass)->byID($id);
0 ignored issues
show
Bug introduced by
It seems like $buyableclass defined by \Convert::raw2sql($class) on line 543 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...
555
        if (!$buyable || !($buyable instanceof Buyable)) {
556
            //TODO: store error message
557
            return null;
558
        }
559
560
        return $this->cart->getCorrectBuyable($buyable);
561
    }
562 1
563
    /**
564 1
     * Action: add item to cart
565 1
     *
566 1
     * @param SS_HTTPRequest $request
567
     *
568 1
     * @return SS_HTTPResponse
569 1
     */
570 1
    public function add($request)
571
    {
572
        if ($product = $this->buyableFromRequest()) {
573
            $quantity = (int)$request->getVar('quantity');
574
            if (!$quantity) {
575
                $quantity = 1;
576
            }
577
            $this->cart->add($product, $quantity, $request->getVars());
578
        }
579
580 1
        $this->updateLocale($request);
581
        $this->extend('updateAddResponse', $request, $response, $product, $quantity);
582 1
        return $response ? $response : self::direct();
583 1
    }
584 1
585
    /**
586 1
     * Action: remove a certain number of items from the cart
587 1
     *
588 1
     * @param SS_HTTPRequest $request
589
     *
590
     * @return SS_HTTPResponse
591
     */
592
    public function remove($request)
593
    {
594
        if ($product = $this->buyableFromRequest()) {
595
            $this->cart->remove($product, $quantity = 1, $request->getVars());
596
        }
597
598 9
        $this->updateLocale($request);
599
        $this->extend('updateRemoveResponse', $request, $response, $product, $quantity);
600 2
        return $response ? $response : self::direct();
601 2
    }
602 2
603 2
    /**
604 2
     * Action: remove all of an item from the cart
605
     *
606 2
     * @param SS_HTTPRequest $request
607 2
     *
608 9
     * @return SS_HTTPResponse
609
     */
610
    public function removeall($request)
611
    {
612
        if ($product = $this->buyableFromRequest()) {
613
            $this->cart->remove($product, null, $request->getVars());
614
        }
615
616
        $this->updateLocale($request);
617
        $this->extend('updateRemoveAllResponse', $request, $response, $product);
618
        return $response ? $response : self::direct();
619
    }
620
621
    /**
622
     * Action: update the quantity of an item in the cart
623
     *
624
     * @param $request
625
     *
626
     * @return AjaxHTTPResponse|bool
627
     */
628
    public function setquantity($request)
629
    {
630
        $product = $this->buyableFromRequest();
631
        $quantity = (int)$request->getVar('quantity');
632
        if ($product) {
633
            $this->cart->setQuantity($product, $quantity, $request->getVars());
634
        }
635
636
        $this->updateLocale($request);
637
        $this->extend('updateSetQuantityResponse', $request, $response, $product, $quantity);
638
        return $response ? $response : self::direct();
639
    }
640
641
    /**
642
     * Action: clear the cart
643
     *
644
     * @param $request
645
     *
646
     * @return AjaxHTTPResponse|bool
647
     */
648
    public function clear($request)
649
    {
650
        $this->updateLocale($request);
651
        $this->cart->clear();
652
        $this->extend('updateClearResponse', $request, $response);
653
        return $response ? $response : self::direct();
654
    }
655
656
    /**
657
     * Handle index requests
658 4
     */
659
    public function index()
660 4
    {
661 4
        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...
662
            $this->redirect($cart->CartLink);
663
            return;
664 4
        } elseif ($response = ErrorPage::response_for(404)) {
665
            return $response;
666 2
        }
667
        return $this->httpError(404, _t("ShoppingCart.NoCartInitialised", "no cart initialised"));
668
    }
669
670
    /**
671
     * Displays order info and cart contents.
672
     */
673
    public function debug()
674
    {
675
        if (Director::isDev() || Permission::check("ADMIN")) {
676
            //TODO: allow specifying a particular id to debug
677
            Requirements::css(SHOP_DIR . "/css/cartdebug.css");
678
            $order = ShoppingCart::curr();
679
            $content = ($order)
680
                ?
681
                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...
682
                :
683
                "Cart has not been created yet. Add a product.";
684
            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...
685
        }
686
    }
687
688
    protected function updateLocale($request)
689
    {
690
        $order = $this->cart->current();
691
        if ($request && $request->isAjax() && $order) {
692
            ShopTools::install_locale($order->Locale);
693
        }
694
    }
695
}
696