CountryPrices_ChangeCountryController::set_new_country() - Code Metrics - Inspection of "Merge branch 'master' of https://github.com/sunnys..." - sunnysideup/silverstripe-ecommerce_countries - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( 2b3013...123071 )

unknown

created 2018-05-11 04:48 UTC

set_new_country() B

↳ Parent: CountryPrices_ChangeCountryController

Complexity

Conditions	5
Paths	3

Size

Total Lines	16
Code Lines	10

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	16
rs	8.8571
c	0
b	0
f	0
cc	5
eloc	10
nc	3
nop	2

<?php

/**
 *
 * Common use:
 * ```php
 *    CountryPrices_ChangeCountryController::changeto('XX');
 *    CountryPrices_ChangeCountryController::new_country_link('XX');
 * ```
 *
 */

class CountryPrices_ChangeCountryController extends ContentController
namespace YourVendor;

class YourClass { }
{


    /**
     * make sure to match route...
     * @var string
     */
    private static $url_segment = 'shoppingcart-countries';


    /**
     * needs to be saved like this:
     * ZA => 'myshop.co.za'
     *
     * @var array
     */
    private static $off_site_url_redirects = array();


    private static $allowed_actions = array(

        "changeto" => true,
        "confirmredirection" => true
    );

    /**
     * only call this function if it is a NEW country we are dealing with!
     *
     * without force it is a halfway house.
     *
     * @param string  $newCountryCode
     * @param bool    $force
     *
     */
    public static function set_new_country($newCountryCode, $force = true)
    {
        $newCountryCode = strtoupper($newCountryCode);
        $o = Shoppingcart::current_order();
        if($o && $o->exists()) {
            $orderCountry = $o->getCountry();
            if ($orderCountry === $newCountryCode || CountryPrice_EcommerceCountry::countries_belong_to_same_group($orderCountry, $newCountryCode)) {
                //nothing to see here, please move on!
                return;
            }
            ShoppingCart::singleton()->clear();
        }

        Session::set('MyCloudFlareCountry', $newCountryCode);
        CountryPrice_OrderDOD::localise_order($newCountryCode, $force, $runAgain = true);
    }

    /**
     * link to change to a new country.
     *
     * @param string $newCountryCode
     *
     */
    public static function new_country_link($newCountryCode)

    {
        $newCountryCode = strtoupper($newCountryCode);
        $redirectsArray = Config::inst()->get('CountryPrices_ChangeCountryController', 'off_site_url_redirects');
        if (isset($redirectsArray[$newCountryCode])) {
            return $redirectsArray[$newCountryCode];
        }

        return Injector::inst()->get('CountryPrices_ChangeCountryController')->Link('changeto/'.strtolower($newCountryCode).'/');
    }

    /**
     *
     * @param  SS_HTTPRequest $request
     *
     * @return SS_HTTPResponse

     */
    public function changeto($request)
// 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);
    }
}
    {
        //check for offsite redirects???

        $newCountryCode = Convert::raw2sql(strtoupper($request->param('ID')));
        self::set_new_country($newCountryCode);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

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

        //redirect now
        if (isset($_GET['force']) && $_GET['force']) {
            return $this->redirect($this->Link($newCountryCode) . '?force-back-home');
        }
        if (isset($_GET['force-back-home']) && $_GET['force-back-home']) {
            return $this->redirect(Director::baseURL('/'));

        }

        $newLink = $this->findNewURL($newCountryCode);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
        if ($newLink) {
            return $this->redirect($newLink);
        }

        return [];
    }

    public function Link($action = null)
    {
        $link = Controller::join_links(
            Config::inst()->get('CountryPrices_ChangeCountryController', 'url_segment'),
            $action
        );

        return $link . '/';
    }

    /**
     * Remove a query string parameter from an URL.
     *
     * @param string $newCountryCode
     *
     * @return string
     */
    public function findNewURL($newCountryCode)
    {

        //COPIED FROM DIRECTOR::redirectBack()
        // Don't cache the redirect back ever
        HTTP::set_cache_age(0);

        $url = null;

        // In edge-cases, this will be called outside of a handleRequest() context; in that case,
        // redirect to the homepage - don't break into the global state at this stage because we'll
        // be calling from a test context or something else where the global state is inappropraite
        if ($this->getRequest()) {
            if ($this->getRequest()->requestVar('BackURL')) {
                $url = $this->getRequest()->requestVar('BackURL');
            } elseif ($this->getRequest()->isAjax() && $this->getRequest()->getHeader('X-Backurl')) {
                $url = $this->getRequest()->getHeader('X-Backurl');
            } elseif ($this->getRequest()->getHeader('Referer')) {
                $url = $this->getRequest()->getHeader('Referer');
            }
        }

        if (!$url) {
            $url = Director::baseURL();
        }

        // absolute redirection URLs not located on this site may cause phishing
        if (Director::is_site_url($url)) {
            $oldURL = Director::absoluteURL($url, true);
            $parsedUrl = parse_url($oldURL);
            $query = array();

            if (isset($parsedUrl['query'])) {
                $varname = $this->Config()->get('locale_get_parameter');
                parse_str($parsedUrl['query'], $query);
                if (isset($query[$varname])) {
                    if ($query[$varname] !== $newCountryCode) {
                        unset($query[$varname]);
                    }
                }
            }

            $path = isset($parsedUrl['path']) ? $parsedUrl['path'] : '';
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

            $query = empty($query) ? '' :  '?'. http_build_query($query);
            $newURL =
                $parsedUrl['scheme'] .
                '://' .
                Controller::join_links(
                    $parsedUrl['host'],
                    $parsedUrl['path']
                ).
                $query;
            $newURLwithNewCountryCode = CountryPrice_Translation::get_country_url_provider()

                ->replaceCountryCodeInUrl(
                    $newCountryCode,
                    $newURL
                );
            if ($newURLwithNewCountryCode) {
                return $newURLwithNewCountryCode;
            } else {
                return $newURL;
            }
        }
        return '/';
    }

    public function confirmredirection($request)

    {
    }
}


1			<?php
2
3			/**
4			*
5			* Common use:
6			* ```php
7			* CountryPrices_ChangeCountryController::changeto('XX');
8			* CountryPrices_ChangeCountryController::new_country_link('XX');
9			* ```
10			*
11			*/
12
13			class CountryPrices_ChangeCountryController extends ContentController
			0 ignored issues – show Coding Style Compatibility introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions. You can fix this by adding a namespace to your class: namespace YourVendor; class YourClass { } When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries. Loading history...
14			{
15
16
17			/**
18			* make sure to match route...
19			* @var string
20			*/
21			private static $url_segment = 'shoppingcart-countries';
			0 ignored issues – show Unused Code introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report The property `$url_segment` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
22
23			/**
24			* needs to be saved like this:
25			* ZA => 'myshop.co.za'
26			*
27			* @var array
28			*/
29			private static $off_site_url_redirects = array();
			0 ignored issues – show Unused Code introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report The property `$off_site_url_redirects` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
30
31			private static $allowed_actions = array(
			0 ignored issues – show Comprehensibility introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report Consider using a different property name as you override a private property of the parent class. Loading history... Unused Code introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report The property `$allowed_actions` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
32			"changeto" => true,
33			"confirmredirection" => true
34			);
35
36			/**
37			* only call this function if it is a NEW country we are dealing with!
38			*
39			* without force it is a halfway house.
40			*
41			* @param string $newCountryCode
42			* @param bool $force
43			*
44			*/
45			public static function set_new_country($newCountryCode, $force = true)
46			{
47			$newCountryCode = strtoupper($newCountryCode);
48			$o = Shoppingcart::current_order();
49			if($o && $o->exists()) {
50			$orderCountry = $o->getCountry();
51			if ($orderCountry === $newCountryCode \|\| CountryPrice_EcommerceCountry::countries_belong_to_same_group($orderCountry, $newCountryCode)) {
52			//nothing to see here, please move on!
53			return;
54			}
55			ShoppingCart::singleton()->clear();
56			}
57
58			Session::set('MyCloudFlareCountry', $newCountryCode);
59			CountryPrice_OrderDOD::localise_order($newCountryCode, $force, $runAgain = true);
60			}
61
62			/**
63			* link to change to a new country.
64			*
65			* @param string $newCountryCode
66			*
67			*/
68			public static function new_country_link($newCountryCode)
			0 ignored issues – show Documentation introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report The return type could not be reliably inferred; please add a `@return` annotation. Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a `@return` annotation as described here. Loading history...
69			{
70			$newCountryCode = strtoupper($newCountryCode);
71			$redirectsArray = Config::inst()->get('CountryPrices_ChangeCountryController', 'off_site_url_redirects');
72			if (isset($redirectsArray[$newCountryCode])) {
73			return $redirectsArray[$newCountryCode];
74			}
75
76			return Injector::inst()->get('CountryPrices_ChangeCountryController')->Link('changeto/'.strtolower($newCountryCode).'/');
77			}
78
79			/**
80			*
81			* @param SS_HTTPRequest $request
82			*
83			* @return SS_HTTPResponse
			0 ignored issues – show Documentation introduced 2018-01-16 22:48 UTC by Report Bug Copy Issue Report Should the return type not be `SS_HTTPResponse\|null`? This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. Loading history...
84			*/
85			public function changeto($request)
			0 ignored issues – show Coding Style introduced 2018-01-16 22:48 UTC by Report Bug Copy Issue Report `changeto` uses the super-global variable `$_GET` 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...
86			{
87			//check for offsite redirects???
			0 ignored issues – show Unused Code Comprehensibility introduced 2018-01-16 22:48 UTC by Report Bug Copy Issue Report `40%` 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...
88			$newCountryCode = Convert::raw2sql(strtoupper($request->param('ID')));
89			self::set_new_country($newCountryCode);
			0 ignored issues – show Bug introduced 2018-05-11 04:51 UTC by Report Bug Copy Issue Report It seems like `$newCountryCode` defined by `\Convert::raw2sql(strtou...$request->param('ID')))` on line `88` can also be of type `array`; however, `CountryPrices_ChangeCoun...ller::set_new_country()` 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...
90
91			//redirect now
92			if (isset($_GET['force']) && $_GET['force']) {
93			return $this->redirect($this->Link($newCountryCode) . '?force-back-home');
94			}
95			if (isset($_GET['force-back-home']) && $_GET['force-back-home']) {
96			return $this->redirect(Director::baseURL('/'));
			0 ignored issues – show Unused Code introduced 2018-02-14 08:36 UTC by Report Bug Copy Issue Report The call to `Director::baseURL()` has too many arguments starting with `'/'`. This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue. If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. In this case you can add the `@ignore` PhpDoc annotation to the duplicate definition and it will be ignored. Loading history...
97			}
98
99			$newLink = $this->findNewURL($newCountryCode);
			0 ignored issues – show Bug introduced 2018-05-11 04:51 UTC by Report Bug Copy Issue Report It seems like `$newCountryCode` defined by `\Convert::raw2sql(strtou...$request->param('ID')))` on line `88` can also be of type `array`; however, `CountryPrices_ChangeCoun...ontroller::findNewURL()` 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...
100			if ($newLink) {
101			return $this->redirect($newLink);
102			}
103
104			return [];
105			}
106
107			public function Link($action = null)
108			{
109			$link = Controller::join_links(
110			Config::inst()->get('CountryPrices_ChangeCountryController', 'url_segment'),
111			$action
112			);
113
114			return $link . '/';
115			}
116
117			/**
118			* Remove a query string parameter from an URL.
119			*
120			* @param string $newCountryCode
121			*
122			* @return string
123			*/
124			public function findNewURL($newCountryCode)
125			{
126
127			//COPIED FROM DIRECTOR::redirectBack()
128			// Don't cache the redirect back ever
129			HTTP::set_cache_age(0);
130
131			$url = null;
132
133			// In edge-cases, this will be called outside of a handleRequest() context; in that case,
134			// redirect to the homepage - don't break into the global state at this stage because we'll
135			// be calling from a test context or something else where the global state is inappropraite
136			if ($this->getRequest()) {
137			if ($this->getRequest()->requestVar('BackURL')) {
138			$url = $this->getRequest()->requestVar('BackURL');
139			} elseif ($this->getRequest()->isAjax() && $this->getRequest()->getHeader('X-Backurl')) {
140			$url = $this->getRequest()->getHeader('X-Backurl');
141			} elseif ($this->getRequest()->getHeader('Referer')) {
142			$url = $this->getRequest()->getHeader('Referer');
143			}
144			}
145
146			if (!$url) {
147			$url = Director::baseURL();
148			}
149
150			// absolute redirection URLs not located on this site may cause phishing
151			if (Director::is_site_url($url)) {
152			$oldURL = Director::absoluteURL($url, true);
153			$parsedUrl = parse_url($oldURL);
154			$query = array();
155
156			if (isset($parsedUrl['query'])) {
157			$varname = $this->Config()->get('locale_get_parameter');
158			parse_str($parsedUrl['query'], $query);
159			if (isset($query[$varname])) {
160			if ($query[$varname] !== $newCountryCode) {
161			unset($query[$varname]);
162			}
163			}
164			}
165
166			$path = isset($parsedUrl['path']) ? $parsedUrl['path'] : '';
			0 ignored issues – show Unused Code introduced 2017-03-30 04:17 UTC by Report Bug Copy Issue Report `$path` is not used, you could remove the assignment. This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently. $myVar = 'Value'; $higher = false; if (rand(1, 6) > 3) { $higher = true; } else { $higher = false; } Both the `$myVar` assignment in line 1 and the `$higher` assignment in line 2 are dead. The first because `$myVar` is never used and the second because `$higher` is always overwritten for every possible time line. Loading history...
167
168			$query = empty($query) ? '' : '?'. http_build_query($query);
169			$newURL =
170			$parsedUrl['scheme'] .
171			'://' .
172			Controller::join_links(
173			$parsedUrl['host'],
174			$parsedUrl['path']
175			).
176			$query;
177			$newURLwithNewCountryCode = CountryPrice_Translation::get_country_url_provider()
			0 ignored issues – show Bug introduced 2018-02-14 08:36 UTC by Report Bug Copy Issue Report The method `replaceCountryCodeInUrl` cannot be called on `\CountryPrice_Translatio..._country_url_provider()` (of type `string`). Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects. Loading history...
178			->replaceCountryCodeInUrl(
179			$newCountryCode,
180			$newURL
181			);
182			if ($newURLwithNewCountryCode) {
183			return $newURLwithNewCountryCode;
184			} else {
185			return $newURL;
186			}
187			}
188			return '/';
189			}
190
191			public function confirmredirection($request)
			0 ignored issues – show Unused Code introduced 2017-10-18 01:15 UTC by Report Bug Copy Issue Report The parameter `$request` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history...
192			{
193			}
194			}
195

sunnysideup / silverstripe-ecommerce_countries

Push — master ( 2b3013...123071 )

set_new_country() B

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like