Completed
Push — master ( 9f9330...12d38a )
by Marcus
02:02
created

Shoppingcart::validateCheckout()   B

Complexity

Conditions 7
Paths 8

Size

Total Lines 19
Code Lines 12

Duplication

Lines 6
Ratio 31.58 %

Importance

Changes 1
Bugs 0 Features 0
Metric Value
c 1
b 0
f 0
dl 6
loc 19
rs 8.2222
cc 7
eloc 12
nc 8
nop 1
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
        if (HelperConfig::$shop['show_pricesonlytologgedin'] && !CHelper::getUserData()) {
56
            $this->P->oPayload->cl_html = $this->textcats->T('denied_notloggedin');
57
        } else {
58
            $this->P->cb_customcontenttemplate = 'shop/shoppingcart';
59
60
            // Check if there is a message to display above the shoppingcart
61
            $this->P->oPayload->cl_html = $this->getNotification();
62
63
            // Display the shoppingcart
64
            if (isset($_SESSION['cart']) && count($_SESSION['cart']) >= 1) {
65
                $aErr = [];
66
                if (filter_input(INPUT_POST, 'doCheckout') === 'yes') {
67
                    $aErr = $this->validateCheckout($aErr);
68
                    if (count($aErr) === 0) {
69
                        $this->doCheckout();
70
                    }
71
                }
72
73
                $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...
74
75
                $this->P->cb_customdata = $aShoppingcart;
76
            } else {
77
                $this->P->oPayload->cl_html .= $this->textcats->T('shoppingcart_empty');
78
            }
79
        }
80
    }
81
82
    /**
83
     * @param array $aErr
84
     * @return array
85
     */
86
    private function validateCheckout($aErr = [])
87
    {
88
        $aErr = CHelper::validateCustomerForm(HelperConfig::$lang, $aErr, true);
89 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...
90
            $aErr['tos'] = true;
91
        }
92 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...
93
            $aErr['cancellationdisclaimer'] = true;
94
        }
95
        $postpaymentmethod = filter_input(INPUT_POST, 'paymentmethod');
96
        if (
97
            $postpaymentmethod === null
98
            || in_array($postpaymentmethod, HelperConfig::$shop['paymentmethods'], true) === false
99
        ) {
100
            $aErr['paymentmethod'] = true;
101
        }
102
103
        return $aErr;
104
    }
105
106
    /**
107
     * @param $aV
108
     * @return array
109
     */
110
    private function getItemImage($aV)
111
    {
112
        // base64 encode img and prepare for db
113
        // image/png image/jpeg image/gif
114
        // 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...
115
116
        $aImagesToSend = [];
117
        $base64Img = false;
118
        $binImg = false;
119
120
        if (HelperConfig::$shop['email_orderconfirmation_embed_itemimages_method'] === 'glide') {
121
            $sPathToImage = '/'.HelperConfig::$core['directory_images'].'/'.HelperConfig::$shop['directory_images_items'].'/';
122
            $sImageroot = PATH_BASEDIR . HelperConfig::$core['directory_glide_master'];
123
124
            if (
125
                is_file($sImageroot.substr($sPathToImage.$aV['img'], strlen(HelperConfig::$core['directory_images']) + 1))
126
                && $aImgInfo = getimagesize($sImageroot.substr($sPathToImage.$aV['img'], strlen(HelperConfig::$core['directory_images']) + 1))
127
            ) {
128
                $glideserver = \League\Glide\ServerFactory::create([
129
                    'source' => $sImageroot,
130
                    'cache' => PATH_GLIDECACHE,
131
                    'max_image_size' => HelperConfig::$core['glide_max_imagesize'],
132
                ]);
133
                $glideserver->setBaseUrl('/' . HelperConfig::$core['directory_images'] . '/');
134
                $base64Img = $glideserver->getImageAsBase64($sPathToImage.$aV['img'], HelperConfig::$shop['email_orderconfirmation_embed_itemimages_glideparams']);
135
                $TMP = explode(',', $base64Img);
136
                $binImg = base64_decode($TMP[1]);
137
                unset($TMP);
138
            }
139
        } else {
140
            $sPathToImage =
141
                PATH_DOCROOT.HelperConfig::$core['directory_images'].'/'
142
                .HelperConfig::$shop['directory_images_items'].'/'
143
                .HelperConfig::$shop['directory_images_items_email'].'/';
144
            if ($aImgInfo = getimagesize($sPathToImage.$aV['img'])) {
145
                $binImg = file_get_contents($sPathToImage.$aV['img']);
146
                $base64Img = 'data:' . $aImgInfo['mime'] . ';base64,';
147
                $base64Img .= base64_encode($binImg);
148
            }
149
        }
150
        if (HelperConfig::$shop['email_orderconfirmation_embed_itemimages']) {
151
            $aImagesToSend['binimg'] = $binImg;
152
        }
153
        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...
154
            $aImagesToSend['base64img'] = $base64Img;
155
        }
156
        return $aImagesToSend;
157
    }
158
159
    /**
160
     * @return array
161
     */
162
    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...
163
    {
164
        $cartpricesums = $_SESSION['cartpricesums'];
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' => $cartpricesums['sumvoll'],
183
            'o_sumerm' => $cartpricesums['sumerm'],
184
            'o_sumnettoall' => $cartpricesums['sumnettoall'],
185
            'o_taxvoll' => $cartpricesums['taxvoll'],
186
            'o_taxerm' => $cartpricesums['taxerm'],
187
            'o_sumbruttoall' => $cartpricesums['sumbruttoall'],
188
            'o_mindermenge' => isset($cartpricesums['mindergebuehr']) ? $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
        /** @var \PDO $db */
207
        $db = $this->serviceManager->get('db');
208
209
        try {
210
            $db->beginTransaction();
211
212
            $aDataOrder = $this->prepareDataOrder();
213
            $sql = DBTools::buildPSInsertQuery($aDataOrder, 'orders');
214
            $hResult = $db->prepare($sql);
215
            foreach ($aDataOrder as $sKey => $sValue) {
216
                $hResult->bindValue(':' . $sKey, $sValue);
217
            }
218
            $hResult->execute();
219
            $iInsertID = $db->lastInsertId();
220
221
            $aDataOrderItems = [];
222
            $aImagesToSend = [];
223
            foreach ($_SESSION['cart'] as $sK => $aV) {
224
225
                $aImagesToSend[$aV['img']] = $this->getItemImage($aV);
226
227
                $aDataOrderItems[] = [
228
                    'oi_o_id' => $iInsertID,
229
                    'oi_cartkey' => $sK,
230
                    'oi_amount' => $aV['amount'],
231
                    'oi_price_netto_list' => $aV['price']['netto_list'],
232
                    'oi_price_netto_use' => $aV['price']['netto_use'],
233
                    'oi_price_brutto_use' => $aV['price']['brutto_use'],
234
                    'oi_price_netto_sale' => isset($aV['price']['netto_sale']) ? $aV['price']['netto_sale'] : '',
235
                    'oi_price_netto_rebated' => isset($aV['price']['netto_rebated']) ? $aV['price']['netto_rebated'] : '',
236
                    'oi_vat' => HelperConfig::$shop['vat'][$aV['vat']],
237
                    'oi_rg' => $aV['rg'],
238
                    'oi_rg_rebate' => isset(
239
                        HelperConfig::$shop['rebate_groups'][$aV['rg']][trim(CHelper::getUserData('cust_group'))]
240
                    )
241
                        ? HelperConfig::$shop['rebate_groups'][$aV['rg']][trim(CHelper::getUserData('cust_group'))]
242
                        : '',
243
                    'oi_itemname' => $aV['name'],
244
                    'oi_img' => $aImagesToSend[$aV['img']]['base64img'],
245
                ];
246
            }
247
            foreach ($aDataOrderItems as $aV) {
248
                $sql = DBTools::buildPSInsertQuery($aV, 'orders_items');
249
                $hResult = $db->prepare($sql);
250
                foreach ($aV as $sKey => $sValue) {
251
                    $hResult->bindValue(':' . $sKey, $sValue);
252
                }
253
                $hResult->execute();
254
            }
255
            $db->commit();
256
        } catch (\Exception $e) {
257
            // If something raised an exception in our transaction block of statements,
258
            // roll back any work performed in the transaction
259
            print '<p>Unable to complete transaction!</p>';
260
            error_log($e);
261
            $db->rollBack();
262
        }
263
        $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...
264
        $sMailbody_they = $this->buildOrderMailBody(true, $iInsertID);
265
266
        // write to file
267
        $this->writeCheckoutToFile($sMailbody_us);
268
269
        // Send Mails
270
        $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...
271
272
        unset($_SESSION['cart'], $_SESSION['cartpricesums'], $_SESSION['sondercart']);
273
274
        $postpaymentmethod = filter_input(INPUT_POST, 'paymentmethod');
275
        if (
276
            $postpaymentmethod !== null
277
            && isset(HelperConfig::$shop['paypal_interactive'])
278
            && $postpaymentmethod === 'paypal'
279
            && in_array('paypal', HelperConfig::$shop['paymentmethods'], true) !== false
280
            && HelperConfig::$shop['paypal_interactive']
281
        ) {
282
            $redirectto = '/_misc/paypal.html?id=' . $iInsertID;
283
        } elseif (
284
            $postpaymentmethod !== null
285
            && $postpaymentmethod === 'sofortueberweisung'
286
            && in_array('sofortueberweisung', HelperConfig::$shop['paymentmethods'], true) !== false
287
        ) {
288
            $redirectto = '/_misc/sofortueberweisung.html?id=' . $iInsertID;
289
        } else {
290
            $redirectto = '/_misc/checkedout.html?id=' . $iInsertID;
291
        }
292
        \HaaseIT\HCSF\Helper::redirectToPage($redirectto);
293
    }
294
295
    /**
296
     * @param int $iInsertID
297
     * @param string $sMailbody_us
298
     * @param string $sMailbody_they
299
     * @param array $aImagesToSend
300
     */
301
    private function sendCheckoutMails($iInsertID, $sMailbody_us, $sMailbody_they, $aImagesToSend)
302
    {
303
        if (
304
            isset(HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_' .HelperConfig::$lang])
305
            && file_exists(
306
                PATH_DOCROOT.HelperConfig::$core['directory_emailattachments']
307
                .'/'.HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_'
308
                .HelperConfig::$lang]
309
            )
310
        ) {
311
            $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...
312
                PATH_DOCROOT.HelperConfig::$core['directory_emailattachments'].'/'
313
                .HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_' .HelperConfig::$lang];
314
        } else {
315
            $aFilesToSend = [];
316
        }
317
318
        Helper::mailWrapper(
319
            filter_input(INPUT_POST, 'email', FILTER_SANITIZE_EMAIL),
320
            $this->textcats->T('shoppingcart_mail_subject') . ' ' . $iInsertID,
321
            $sMailbody_they,
322
            $aImagesToSend,
323
            $aFilesToSend
324
        );
325
        Helper::mailWrapper(
326
            HelperConfig::$core['email_sender'],
327
            'Bestellung im Webshop Nr: ' . $iInsertID,
328
            $sMailbody_us,
329
            $aImagesToSend
330
        );
331
    }
332
333
    /**
334
     * @param string $sMailbody_us
335
     */
336
    private function writeCheckoutToFile($sMailbody_us)
337
    {
338
        $fp = fopen(PATH_LOGS . 'shoplog_' . date('Y-m-d') . '.html', 'a');
339
        // Write $somecontent to our opened file.
340
        fwrite($fp, $sMailbody_us . "\n\n-------------------------------------------------------------------------\n\n");
341
        fclose($fp);
342
    }
343
344
    /**
345
     * @param string $field
346
     * @return string
347
     */
348
    private function getPostValue($field)
349
    {
350
        $postvalue = filter_input(INPUT_POST, $field);
351
        return (!empty($postvalue) ? $postvalue : '');
352
    }
353
354
    /**
355
     * @param bool $bCust
356
     * @param int $iId
357
     * @return mixed
358
     */
359
    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...
360
    {
361
        $aSHC = SHelper::buildShoppingCartTable($_SESSION['cart'], true);
362
363
        $postcustno = trim(filter_input(INPUT_POST, 'custno', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW));
364
        $postcountry = trim(filter_input(INPUT_POST, 'country', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW));
365
        $postpaymentmethod = filter_input(INPUT_POST, 'paymentmethod', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW);
366
        $serverservername = filter_input(INPUT_SERVER, 'SERVER_NAME', FILTER_SANITIZE_URL);
367
        $aData = [
368
            'customerversion' => $bCust,
369
            //'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...
370
            'datetime' => date('d.m.Y - H:i'),
371
            'custno' => $postcustno !== null && strlen($postcustno) >= HelperConfig::$customer['minimum_length_custno'] ? $postcustno : '',
372
            'corpname' => $this->getPostValue('corpname'),
373
            'name' => $this->getPostValue('name'),
374
            'street' => $this->getPostValue('street'),
375
            'zip' => $this->getPostValue('zip'),
376
            'town' => $this->getPostValue('town'),
377
            'phone' => $this->getPostValue('phone'),
378
            'cellphone' => $this->getPostValue('cellphone'),
379
            'fax' => $this->getPostValue('fax'),
380
            'email' => $this->getPostValue('email'),
381
            'country' => !empty($postcountry) ?
382
            (
383
                isset(
384
                    HelperConfig::$countries['countries_' .HelperConfig::$lang][$postcountry]
385
                )
386
                    ? HelperConfig::$countries['countries_' .HelperConfig::$lang][$postcountry]
387
                    : $postcountry)
388
            : '',
389
            'remarks' => $this->getPostValue('remarks'),
390
            'tos' => $this->getPostValue('tos'),
391
            'cancellationdisclaimer' => $this->getPostValue('cancellationdisclaimer'),
392
            'paymentmethod' => $this->getPostValue('paymentmethod'),
393
            'shippingcost' => empty($_SESSION['shippingcost']) ? false : $_SESSION['shippingcost'],
394
            'paypallink' => $postpaymentmethod === 'paypal' ? $serverservername.'/_misc/paypal.html?id='.$iId : '',
395
            'sofortueberweisunglink' => $postpaymentmethod === 'sofortueberweisung' ?  $serverservername.'/_misc/sofortueberweisung.html?id='.$iId : '',
396
            'SESSION' => !$bCust ? Tools::debug($_SESSION, '$_SESSION', true, true) : '',
397
            'POST' => !$bCust ? Tools::debug($_POST, '$_POST', true, true) : '',
398
            'orderid' => $iId,
399
        ];
400
401
        $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...
402
        $aM['currency'] = HelperConfig::$shop['waehrungssymbol'];
403
        if (isset(HelperConfig::$shop['custom_order_fields'])) {
404
            $aM['custom_order_fields'] = HelperConfig::$shop['custom_order_fields'];
405
        }
406
        $aM['customdata']['mail'] = $aData;
407
408
        return $this->serviceManager->get('twig')->render('shop/mail-order-html.twig', $aM);
409
410
    }
411
412
    /**
413
     * @return string
414
     */
415
    private function getNotification()
416
    {
417
        $return = '';
418
        $getmsg = filter_input(INPUT_GET, 'msg');
419
        $getcartkey = filter_input(INPUT_GET, 'cartkey', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW);
420
        $getamount = filter_input(INPUT_GET, 'cartkey', FILTER_SANITIZE_NUMBER_INT);
421
        if (!empty($getmsg)) {
422
            if (
423
                ($getmsg === 'updated' && !empty($getcartkey) && !empty($getamount))
424
                || ($getmsg === 'removed' && !empty($getcartkey))
425
            ) {
426
                $return .= $this->textcats->T('shoppingcart_msg_' . $getmsg . '_1') . ' ';
427
                if (isset(HelperConfig::$shop['custom_order_fields']) && mb_strpos($getcartkey, '|') !== false) {
428
                    $mCartkeys = explode('|', $getcartkey);
429
                    foreach ($mCartkeys as $sKey => $sValue) {
430
                        if ($sKey == 0) {
431
                            $return .= $sValue . ', ';
432
                        } else {
433
                            $TMP = explode(':', $sValue);
434
                            $return .= $this->textcats->T('shoppingcart_item_' . $TMP[0]) . ' ' . $TMP[1] . ', ';
435
                            unset($TMP);
436
                        }
437
                    }
438
                    $return = Tools::cutStringend($return, 2);
439
                } else {
440
                    $return .= $getcartkey;
441
                }
442
                $return.= ' ' . $this->textcats->T('shoppingcart_msg_'.$getmsg.'_2');
443
                if ($getmsg === 'updated') {
444
                    $return .= ' '.$getamount;
445
                }
446
                $return .= '<br><br>';
447
            }
448
        }
449
450
        return $return;
451
    }
452
}
453