Completed
Push — master ( b17e40...2d3b60 )
by Marcus
04:02
created

Shoppingcart   F

Complexity

Total Complexity 82

Size/Duplication

Total Lines 435
Duplicated Lines 1.38 %

Coupling/Cohesion

Components 1
Dependencies 13

Importance

Changes 6
Bugs 0 Features 1
Metric Value
wmc 82
c 6
b 0
f 1
lcom 1
cbo 13
dl 6
loc 435
rs 3.4814

10 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 5 1
C preparePage() 6 56 16
C getItemImage() 0 48 7
C prepareDataOrder() 0 40 7
F doCheckout() 0 102 21
B sendCheckoutMails() 0 31 3
A writeCheckoutToFile() 0 7 1
A getPostValue() 0 5 2
F buildOrderMailBody() 0 52 12
C getNotification() 0 37 12

How to fix   Duplicated Code    Complexity   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

Complex Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

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
    HCSF - A multilingual CMS and Shopsystem
5
    Copyright (C) 2014  Marcus Haase - [email protected]
6
7
    This program is free software: you can redistribute it and/or modify
8
    it under the terms of the GNU General Public License as published by
9
    the Free Software Foundation, either version 3 of the License, or
10
    (at your option) any later version.
11
12
    This program is distributed in the hope that it will be useful,
13
    but WITHOUT ANY WARRANTY; without even the implied warranty of
14
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
15
    GNU General Public License for more details.
16
17
    You should have received a copy of the GNU General Public License
18
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
19
 */
20
21
namespace HaaseIT\HCSF\Controller\Shop;
22
23
use HaaseIT\HCSF\HelperConfig;
24
use HaaseIT\Toolbox\Tools;
25
use HaaseIT\HCSF\Helper;
26
use HaaseIT\HCSF\Customer\Helper as CHelper;
27
use HaaseIT\HCSF\Shop\Helper as SHelper;
28
use HaaseIT\Toolbox\DBTools;
29
30
class Shoppingcart extends Base
31
{
32
    /**
33
     * @var \HaaseIT\Toolbox\Textcat
34
     */
35
    private $textcats;
36
37
    /**
38
     * Shoppingcart constructor.
39
     * @param \Zend\ServiceManager\ServiceManager $serviceManager
40
     */
41
    public function __construct(\Zend\ServiceManager\ServiceManager $serviceManager)
42
    {
43
        parent::__construct($serviceManager);
44
        $this->textcats = $this->serviceManager->get('textcats');
45
    }
46
47
    /**
48
     *
49
     */
50
    public function preparePage()
0 ignored issues
show
Coding Style introduced by
preparePage uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
51
    {
52
        $this->P = new \HaaseIT\HCSF\CorePage($this->serviceManager);
53
        $this->P->cb_pagetype = 'contentnosubnav';
54
55
56
        if (HelperConfig::$shop['show_pricesonlytologgedin'] && !CHelper::getUserData()) {
57
            $this->P->oPayload->cl_html = $this->textcats->T('denied_notloggedin');
58
        } else {
59
            $this->P->cb_customcontenttemplate = 'shop/shoppingcart';
60
61
            // ----------------------------------------------------------------------------
62
            // Check if there is a message to display above the shoppingcart
63
            // ----------------------------------------------------------------------------
64
            $this->P->oPayload->cl_html = $this->getNotification();
65
66
            // ----------------------------------------------------------------------------
67
            // Display the shoppingcart
68
            // ----------------------------------------------------------------------------
69
            $aErr = [];
70
            if (isset($_SESSION['cart']) && count($_SESSION['cart']) >= 1) {
71
                if (filter_input(INPUT_POST, 'doCheckout') === 'yes') {
72
                    $aErr = CHelper::validateCustomerForm(HelperConfig::$lang, $aErr, true);
73 View Code Duplication
                    if (!CHelper::getUserData() && filter_input(INPUT_POST, 'tos') !== 'y') {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

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

Loading history...
74
                        $aErr['tos'] = true;
75
                    }
76 View Code Duplication
                    if (!CHelper::getUserData() && filter_input(INPUT_POST, 'cancellationdisclaimer') !== 'y') {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

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

Loading history...
77
                        $aErr['cancellationdisclaimer'] = true;
78
                    }
79
                    $postpaymentmethod = filter_input(INPUT_POST, 'paymentmethod');
80
                    if (
81
                        $postpaymentmethod === null
82
                        || in_array($postpaymentmethod, HelperConfig::$shop['paymentmethods'], true) === false
83
                    ) {
84
                        $aErr['paymentmethod'] = true;
85
                    }
86
                }
87
                $aShoppingcart = SHelper::buildShoppingCartTable($_SESSION['cart'], false, '', $aErr);
0 ignored issues
show
Documentation introduced by
$aErr is of type array, 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...
88
            }
89
90
            // ----------------------------------------------------------------------------
91
            // Checkout
92
            // ----------------------------------------------------------------------------
93
            if (!isset($aShoppingcart)) {
94
                $this->P->oPayload->cl_html .= $this->textcats->T('shoppingcart_empty');
95
            } else {
96
                if (filter_input(INPUT_POST, 'doCheckout') === 'yes' && count($aErr) === 0) {
97
                    $this->doCheckout();
98
                }
99
            }
100
101
            if (isset($aShoppingcart)) {
102
                $this->P->cb_customdata = $aShoppingcart;
103
            }
104
        }
105
    }
106
107
    /**
108
     * @param $aV
109
     * @return array
110
     */
111
    private function getItemImage($aV)
112
    {
113
        // base64 encode img and prepare for db
114
        // image/png image/jpeg image/gif
115
        // data:{mimetype};base64,XXXX
1 ignored issue
show
Unused Code Comprehensibility introduced by
45% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
116
117
        $aImagesToSend = [];
118
        $base64Img = false;
119
        $binImg = false;
120
121
        if (HelperConfig::$shop['email_orderconfirmation_embed_itemimages_method'] === 'glide') {
122
            $sPathToImage = '/'.HelperConfig::$core['directory_images'].'/'.HelperConfig::$shop['directory_images_items'].'/';
123
            $sImageroot = PATH_BASEDIR . HelperConfig::$core['directory_glide_master'];
124
125
            if (
126
                is_file($sImageroot.substr($sPathToImage.$aV['img'], strlen(HelperConfig::$core['directory_images']) + 1))
127
                && $aImgInfo = getimagesize($sImageroot.substr($sPathToImage.$aV['img'], strlen(HelperConfig::$core['directory_images']) + 1))
128
            ) {
129
                $glideserver = \League\Glide\ServerFactory::create([
130
                    'source' => $sImageroot,
131
                    'cache' => PATH_GLIDECACHE,
132
                    'max_image_size' => HelperConfig::$core['glide_max_imagesize'],
133
                ]);
134
                $glideserver->setBaseUrl('/' . HelperConfig::$core['directory_images'] . '/');
135
                $base64Img = $glideserver->getImageAsBase64($sPathToImage.$aV['img'], HelperConfig::$shop['email_orderconfirmation_embed_itemimages_glideparams']);
136
                $TMP = explode(',', $base64Img);
137
                $binImg = base64_decode($TMP[1]);
138
                unset($TMP);
139
            }
140
        } else {
141
            $sPathToImage =
142
                PATH_DOCROOT.HelperConfig::$core['directory_images'].'/'
143
                .HelperConfig::$shop['directory_images_items'].'/'
144
                .HelperConfig::$shop['directory_images_items_email'].'/';
145
            if ($aImgInfo = getimagesize($sPathToImage.$aV['img'])) {
146
                $binImg = file_get_contents($sPathToImage.$aV['img']);
147
                $base64Img = 'data:' . $aImgInfo['mime'] . ';base64,';
148
                $base64Img .= base64_encode($binImg);
149
            }
150
        }
151
        if (HelperConfig::$shop['email_orderconfirmation_embed_itemimages']) {
152
            $aImagesToSend['binimg'] = $binImg;
153
        }
154
        if ($base64Img) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $base64Img of type string|false is loosely compared to true; this is ambiguous if the string can be empty. You might want to explicitly use !== false instead.

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

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
155
            $aImagesToSend['base64img'] = $base64Img;
156
        }
157
        return $aImagesToSend;
158
    }
159
160
    /**
161
     * @return array
162
     */
163
    private function prepareDataOrder()
0 ignored issues
show
Coding Style introduced by
prepareDataOrder uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
prepareDataOrder uses the super-global variable $_POST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
164
    {
165
        return [
166
            'o_custno' => filter_var(trim(Tools::getFormfield('custno')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
167
            'o_email' => filter_var(trim(Tools::getFormfield('email')), FILTER_SANITIZE_EMAIL),
168
            'o_corpname' => filter_var(trim(Tools::getFormfield('corpname')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
169
            'o_name' => filter_var(trim(Tools::getFormfield('name')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
170
            'o_street' => filter_var(trim(Tools::getFormfield('street')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
171
            'o_zip' => filter_var(trim(Tools::getFormfield('zip')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
172
            'o_town' => filter_var(trim(Tools::getFormfield('town')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
173
            'o_phone' => filter_var(trim(Tools::getFormfield('phone')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
174
            'o_cellphone' => filter_var(trim(Tools::getFormfield('cellphone')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
175
            'o_fax' => filter_var(trim(Tools::getFormfield('fax')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
176
            'o_country' => filter_var(trim(Tools::getFormfield('country')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
177
            'o_group' => trim(CHelper::getUserData('cust_group')),
178
            'o_remarks' => filter_var(trim(Tools::getFormfield('remarks')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
179
            'o_tos' => (filter_input(INPUT_POST, 'tos') === 'y' || CHelper::getUserData()) ? 'y' : 'n',
180
            'o_cancellationdisclaimer' => (filter_input(INPUT_POST, 'cancellationdisclaimer') === 'y' || CHelper::getUserData()) ? 'y' : 'n',
181
            'o_paymentmethod' => filter_var(trim(Tools::getFormfield('paymentmethod')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
182
            'o_sumvoll' => $_SESSION['cartpricesums']['sumvoll'],
183
            'o_sumerm' => $_SESSION['cartpricesums']['sumerm'],
184
            'o_sumnettoall' => $_SESSION['cartpricesums']['sumnettoall'],
185
            'o_taxvoll' => $_SESSION['cartpricesums']['taxvoll'],
186
            'o_taxerm' => $_SESSION['cartpricesums']['taxerm'],
187
            'o_sumbruttoall' => $_SESSION['cartpricesums']['sumbruttoall'],
188
            'o_mindermenge' => isset($_SESSION['cartpricesums']['mindergebuehr']) ? $_SESSION['cartpricesums']['mindergebuehr'] : '',
189
            'o_shippingcost' => SHelper::getShippingcost(),
190
            'o_orderdate' => date('Y-m-d'),
191
            'o_ordertimestamp' => time(),
192
            'o_authed' => CHelper::getUserData() ? 'y' : 'n',
193
            'o_sessiondata' => serialize($_SESSION),
194
            'o_postdata' => serialize($_POST),
195
            'o_remote_address' => filter_input(INPUT_SERVER, 'REMOTE_ADDR', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
196
            'o_ordercompleted' => 'n',
197
            'o_paymentcompleted' => 'n',
198
            'o_srv_hostname' => filter_input(INPUT_SERVER, 'SERVER_NAME', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW),
199
            'o_vatfull' => HelperConfig::$shop['vat']['full'],
200
            'o_vatreduced' => HelperConfig::$shop['vat']['reduced'],
201
        ];
202
    }
203
204
    private function doCheckout()
0 ignored issues
show
Coding Style introduced by
doCheckout uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
205
    {
206
        if (empty($_SESSION['cart'])) {
207
            return false;
208
        }
209
210
        /** @var \PDO $db */
211
        $db = $this->serviceManager->get('db');
212
213
        try {
214
            $db->beginTransaction();
215
216
            $aDataOrder = $this->prepareDataOrder();
217
            $sql = DBTools::buildPSInsertQuery($aDataOrder, 'orders');
218
            $hResult = $db->prepare($sql);
219
            foreach ($aDataOrder as $sKey => $sValue) {
220
                $hResult->bindValue(':' . $sKey, $sValue);
221
            }
222
            $hResult->execute();
223
            $iInsertID = $db->lastInsertId();
224
225
            $aDataOrderItems = [];
226
            $aImagesToSend = [];
227
            foreach ($_SESSION['cart'] as $sK => $aV) {
228
229
                $aImagesToSend[$aV['img']] = $this->getItemImage($aV);
230
231
                $aDataOrderItems[] = [
232
                    'oi_o_id' => $iInsertID,
233
                    'oi_cartkey' => $sK,
234
                    'oi_amount' => $aV['amount'],
235
                    'oi_price_netto_list' => $aV['price']['netto_list'],
236
                    'oi_price_netto_use' => $aV['price']['netto_use'],
237
                    'oi_price_brutto_use' => $aV['price']['brutto_use'],
238
                    'oi_price_netto_sale' => isset($aV['price']['netto_sale']) ? $aV['price']['netto_sale'] : '',
239
                    'oi_price_netto_rebated' => isset($aV['price']['netto_rebated']) ? $aV['price']['netto_rebated'] : '',
240
                    'oi_vat' => HelperConfig::$shop['vat'][$aV['vat']],
241
                    'oi_rg' => $aV['rg'],
242
                    'oi_rg_rebate' => isset(
243
                        HelperConfig::$shop['rebate_groups'][$aV['rg']][trim(CHelper::getUserData('cust_group'))]
244
                    )
245
                        ? HelperConfig::$shop['rebate_groups'][$aV['rg']][trim(CHelper::getUserData('cust_group'))]
246
                        : '',
247
                    'oi_itemname' => $aV['name'],
248
                    'oi_img' => $aImagesToSend[$aV['img']]['base64img'],
249
                ];
250
            }
251
            foreach ($aDataOrderItems as $aV) {
252
                $sql = DBTools::buildPSInsertQuery($aV, 'orders_items');
253
                $hResult = $db->prepare($sql);
254
                foreach ($aV as $sKey => $sValue) {
255
                    $hResult->bindValue(':' . $sKey, $sValue);
256
                }
257
                $hResult->execute();
258
            }
259
            $db->commit();
260
        } catch (\Exception $e) {
261
            // If something raised an exception in our transaction block of statements,
262
            // roll back any work performed in the transaction
263
            print '<p>Unable to complete transaction!</p>';
264
            error_log($e);
265
            $db->rollBack();
266
        }
267
        $sMailbody_us = $this->buildOrderMailBody(false, $iInsertID);
0 ignored issues
show
Bug introduced by
The variable $iInsertID does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
268
        $sMailbody_they = $this->buildOrderMailBody(true, $iInsertID);
269
270
        // write to file
271
        $this->writeCheckoutToFile($sMailbody_us);
272
273
        // Send Mails
274
        $this->sendCheckoutMails($iInsertID, $sMailbody_us, $sMailbody_they, $aImagesToSend);
0 ignored issues
show
Bug introduced by
The variable $aImagesToSend does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
275
276
        if (isset($_SESSION['cart'])) {
277
            unset($_SESSION['cart']);
278
        }
279
        if (isset($_SESSION['cartpricesums'])) {
280
            unset($_SESSION['cartpricesums']);
281
        }
282
        if (isset($_SESSION['sondercart'])) {
283
            unset($_SESSION['sondercart']);
284
        }
285
286
        $postpaymentmethod = filter_input(INPUT_POST, 'paymentmethod');
287
        if (
288
            $postpaymentmethod !== null
289
            && isset(HelperConfig::$shop['paypal_interactive'])
290
            && $postpaymentmethod === 'paypal'
291
            && in_array('paypal', HelperConfig::$shop['paymentmethods'], true) !== false
292
            && HelperConfig::$shop['paypal_interactive']
293
        ) {
294
            $redirectto = '/_misc/paypal.html?id=' . $iInsertID;
295
        } elseif (
296
            $postpaymentmethod !== null
297
            && $postpaymentmethod === 'sofortueberweisung'
298
            && in_array('sofortueberweisung', HelperConfig::$shop['paymentmethods'], true) !== false
299
        ) {
300
            $redirectto = '/_misc/sofortueberweisung.html?id=' . $iInsertID;
301
        } else {
302
            $redirectto = '/_misc/checkedout.html?id=' . $iInsertID;
303
        }
304
        \HaaseIT\HCSF\Helper::redirectToPage($redirectto);
305
    }
306
307
    /**
308
     * @param $iInsertID
309
     * @param $sMailbody_us
310
     * @param $sMailbody_they
311
     * @param $aImagesToSend
312
     */
313
    private function sendCheckoutMails($iInsertID, $sMailbody_us, $sMailbody_they, $aImagesToSend)
314
    {
315
        if (
316
            isset(HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_' .HelperConfig::$lang])
317
            && file_exists(
318
                PATH_DOCROOT.HelperConfig::$core['directory_emailattachments']
319
                .'/'.HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_'
320
                .HelperConfig::$lang]
321
            )
322
        ) {
323
            $aFilesToSend[] =
0 ignored issues
show
Coding Style Comprehensibility introduced by
$aFilesToSend was never initialized. Although not strictly required by PHP, it is generally a good practice to add $aFilesToSend = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
324
                PATH_DOCROOT.HelperConfig::$core['directory_emailattachments'].'/'
325
                .HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_' .HelperConfig::$lang];
326
        } else {
327
            $aFilesToSend = [];
328
        }
329
330
        Helper::mailWrapper(
331
            filter_input(INPUT_POST, 'email', FILTER_SANITIZE_EMAIL),
332
            $this->textcats->T('shoppingcart_mail_subject') . ' ' . $iInsertID,
333
            $sMailbody_they,
334
            $aImagesToSend,
335
            $aFilesToSend
336
        );
337
        Helper::mailWrapper(
338
            HelperConfig::$core['email_sender'],
339
            'Bestellung im Webshop Nr: ' . $iInsertID,
340
            $sMailbody_us,
341
            $aImagesToSend
342
        );
343
    }
344
345
    /**
346
     * @param $sMailbody_us
347
     */
348
    private function writeCheckoutToFile($sMailbody_us)
349
    {
350
        $fp = fopen(PATH_LOGS . 'shoplog_' . date('Y-m-d') . '.html', 'a');
351
        // Write $somecontent to our opened file.
352
        fwrite($fp, $sMailbody_us . "\n\n-------------------------------------------------------------------------\n\n");
353
        fclose($fp);
354
    }
355
356
    /**
357
     * @param $field
358
     * @return string
359
     */
360
    private function getPostValue($field)
361
    {
362
        $postvalue = filter_input(INPUT_POST, $field);
363
        return (!empty($postvalue) ? $postvalue : '');
364
    }
365
366
    /**
367
     * @param bool $bCust
368
     * @param int $iId
369
     * @return mixed
370
     */
371
    private function buildOrderMailBody($bCust = true, $iId = 0)
0 ignored issues
show
Coding Style introduced by
buildOrderMailBody uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
buildOrderMailBody uses the super-global variable $_POST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
372
    {
373
        $aSHC = SHelper::buildShoppingCartTable($_SESSION['cart'], true);
374
375
        $postcustno = trim(filter_input(INPUT_POST, 'custno', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW));
376
        $postcountry = trim(filter_input(INPUT_POST, 'country', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW));
377
        $postpaymentmethod = filter_input(INPUT_POST, 'paymentmethod', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW);
378
        $serverservername = filter_input(INPUT_SERVER, 'SERVER_NAME', FILTER_SANITIZE_URL);
379
        $aData = [
380
            'customerversion' => $bCust,
381
            //'shc_css' => file_get_contents(PATH_DOCROOT.'screen-shc.css'),
0 ignored issues
show
Unused Code Comprehensibility introduced by
55% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
382
            'datetime' => date('d.m.Y - H:i'),
383
            'custno' => $postcustno !== null && strlen($postcustno) >= HelperConfig::$customer['minimum_length_custno'] ? $postcustno : '',
384
            'corpname' => $this->getPostValue('corpname'),
385
            'name' => $this->getPostValue('name'),
386
            'street' => $this->getPostValue('street'),
387
            'zip' => $this->getPostValue('zip'),
388
            'town' => $this->getPostValue('town'),
389
            'phone' => $this->getPostValue('phone'),
390
            'cellphone' => $this->getPostValue('cellphone'),
391
            'fax' => $this->getPostValue('fax'),
392
            'email' => $this->getPostValue('email'),
393
            'country' => !empty($postcountry) ?
394
            (
395
                isset(
396
                    HelperConfig::$countries['countries_' .HelperConfig::$lang][$postcountry]
397
                )
398
                    ? HelperConfig::$countries['countries_' .HelperConfig::$lang][$postcountry]
399
                    : $postcountry)
400
            : '',
401
            'remarks' => $this->getPostValue('remarks'),
402
            'tos' => $this->getPostValue('tos'),
403
            'cancellationdisclaimer' => $this->getPostValue('cancellationdisclaimer'),
404
            'paymentmethod' => $this->getPostValue('paymentmethod'),
405
            'shippingcost' => !isset($_SESSION['shippingcost']) || $_SESSION['shippingcost'] == 0 ? false : $_SESSION['shippingcost'],
406
            'paypallink' => $postpaymentmethod === 'paypal' ? $serverservername.'/_misc/paypal.html?id='.$iId : '',
407
            'sofortueberweisunglink' => $postpaymentmethod === 'sofortueberweisung' ?  $serverservername.'/_misc/sofortueberweisung.html?id='.$iId : '',
408
            'SESSION' => !$bCust ? Tools::debug($_SESSION, '$_SESSION', true, true) : '',
409
            'POST' => !$bCust ? Tools::debug($_POST, '$_POST', true, true) : '',
410
            'orderid' => $iId,
411
        ];
412
413
        $aM['customdata'] = $aSHC;
0 ignored issues
show
Coding Style Comprehensibility introduced by
$aM was never initialized. Although not strictly required by PHP, it is generally a good practice to add $aM = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
414
        $aM['currency'] = HelperConfig::$shop['waehrungssymbol'];
415
        if (isset(HelperConfig::$shop['custom_order_fields'])) {
416
            $aM['custom_order_fields'] = HelperConfig::$shop['custom_order_fields'];
417
        }
418
        $aM['customdata']['mail'] = $aData;
419
420
        return $this->serviceManager->get('twig')->render('shop/mail-order-html.twig', $aM);
421
422
    }
423
424
    /**
425
     * @return string
426
     */
427
    private function getNotification()
428
    {
429
        $return = '';
430
        $getmsg = filter_input(INPUT_GET, 'msg');
431
        $getcartkey = filter_input(INPUT_GET, 'cartkey', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW);
432
        $getamount = filter_input(INPUT_GET, 'cartkey', FILTER_SANITIZE_NUMBER_INT);
433
        if (!empty($getmsg)) {
434
            if (
435
                ($getmsg === 'updated' && !empty($getcartkey) && !empty($getamount))
436
                || ($getmsg === 'removed' && !empty($getcartkey))
437
            ) {
438
                $return .= $this->textcats->T('shoppingcart_msg_' . $getmsg . '_1') . ' ';
439
                if (isset(HelperConfig::$shop['custom_order_fields']) && mb_strpos($getcartkey, '|') !== false) {
440
                    $mCartkeys = explode('|', $getcartkey);
441
                    foreach ($mCartkeys as $sKey => $sValue) {
442
                        if ($sKey == 0) {
443
                            $return .= $sValue . ', ';
444
                        } else {
445
                            $TMP = explode(':', $sValue);
446
                            $return .= $this->textcats->T('shoppingcart_item_' . $TMP[0]) . ' ' . $TMP[1] . ', ';
447
                            unset($TMP);
448
                        }
449
                    }
450
                    $return = Tools::cutStringend($return, 2);
451
                } else {
452
                    $return .= $getcartkey;
453
                }
454
                $return.= ' ' . $this->textcats->T('shoppingcart_msg_'.$getmsg.'_2');
455
                if ($getmsg === 'updated') {
456
                    $return .= ' '.$getamount;
457
                }
458
                $return .= '<br><br>';
459
            }
460
        }
461
462
        return $return;
463
    }
464
}
465