FormUserUpdate::save() - Code Metrics - Inspection of "Fix newcomments locale show" - phpffcms/ffcms - Measure and Improve Code Quality continuously with Scrutinizer

Test Failed

Push — master ( e3c39f...fe570d )

by Mihail

created 2019-01-11 11:36 UTC

FormUserUpdate::save() B

↳ Parent: FormUserUpdate

Complexity

Conditions	10
Paths	14

Size

Total Lines	30
Code Lines	19

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	10
eloc	19
nc	14
nop	0
dl	0
loc	30
rs	7.6666
c	0
b	0
f	0

How to fix Complexity

<?php

namespace Apps\Model\Admin\User;

use Apps\ActiveRecord\Profile;
use Apps\ActiveRecord\Role;
use Apps\ActiveRecord\User;
use Ffcms\Core\Arch\Model;
use Ffcms\Core\Helper\Crypt;
use Ffcms\Core\Helper\Type\Any;
use Ffcms\Core\Helper\Type\Str;
use Ffcms\Core\Interfaces\iUser;

/**
 * Class FormUserUpdate. Update user data business logic model
 * @package Apps\Model\Admin\User
 */
class FormUserUpdate extends Model
{
    public $email;
    public $login;
    public $password;
    public $newpassword;
    public $role_id;
    public $approved = false;

    public $approve_token;

    /** @var iUser */
    public $_user;

    /**
     * FormUserUpdate constructor. Pass user object inside the model
     * @param iUser $user
     */
    public function __construct(iUser $user)
    {
        $this->_user = $user;
        parent::__construct(true);
    }

    /**
     * Load user data on before method
     */
    public function before()
    {
        foreach ($this->getAllProperties() as $property => $old_data) {
class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
if ($a->getObject()) {
            if ($this->_user->{$property}) {
                $this->{$property} = $this->_user->{$property};
            }
        }

        if (!$this->approve_token) {
            $this->approved = true;
        }
    }

    /**
     * Form labels to display
     * @return array
     */
    public function labels(): array
    {
        return [
            'email' => __('Email'),
            'login' => __('Login'),
            'newpassword' => __('New password'),
            'role_id' => __('Role'),
            'approved' => __('Approved')
        ];
    }

    /**
     * Validation rules for input data
     * @return array
     */
    public function rules(): array
    {
        return [
            [['email', 'login', 'role_id', 'approved'], 'required'],
            ['newpassword', 'used'],
            ['email', 'email'],
            ['login', 'length_min', 3],
            ['email', 'Apps\Model\Admin\User\FormUserUpdate::isUniqueEmail', $this->_user->getParam('id')],
            ['login', 'Apps\Model\Admin\User\FormUserUpdate::isUniqueLogin', $this->_user->getParam('id')]
        ];
    }

    /**
     * Get all roles as id=>name array
     * @return array|null
     */
    public function getRoleList()
    {
        return Role::getIdNameAll();
class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
if ($a->getObject()) {
    }

    /**
     * Update user information in database based on current obj attributes passed from input data
     */
    public function save()
    {
        foreach ($this->getAllProperties() as $property => $value) {
class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
if ($a->getObject()) {
            if ($property === 'password' || $property === 'newpassword') {
                // update password only if new is set and length >= 3
                if ($this->newpassword && Str::length($this->newpassword) >= 3) {
                    $this->_user->password = Crypt::passwordHash($this->newpassword);
                }
            } elseif ($property === 'approved') {
                if ($this->approved) {
                    $this->_user->approve_token = null;
                } else {
                    $this->_user->approve_token = $this->approve_token ?? Crypt::randomString(mt_rand(32, 128));
                }
            } elseif ($property === 'approve_token') {
                continue;
            } else {
                $this->_user->{$property} = $value;
            }
        }

        // get user id before save to determine "add" action
        $id = $this->_user->id;
        // safe user row
        $this->_user->save();
        // if new user - add profile link
        if ($id < 1) {
            $profile = new Profile();
            $profile->user_id = $this->_user->id;
            $profile->save();
        }
    }

    /**
     * Check if new email is always exist
     * @param string $email
     * @param int|null $userId
     * @return bool
     */
    public static function isUniqueEmail($email, $userId = null)
    {
        $find = User::where('email', '=', $email);

        if ($userId && Any::isInt($userId)) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
            $find->where('id', '!=', $userId);
        }

        return $find->count() === 0;
    }

    /**
     * Check if new login is always exist
     * @param string $login
     * @param int|null $userId
     * @return bool
     */
    public static function isUniqueLogin($login, $userId = null)
    {
        $find = User::where('login', '=', $login);

        if ($userId && Any::isInt($userId)) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
            $find->where('id', '!=', $userId);
        }

        return $find->count() === 0;
    }
}


1			<?php
2
3			namespace Apps\Model\Admin\User;
4
5			use Apps\ActiveRecord\Profile;
6			use Apps\ActiveRecord\Role;
7			use Apps\ActiveRecord\User;
8			use Ffcms\Core\Arch\Model;
9			use Ffcms\Core\Helper\Crypt;
10			use Ffcms\Core\Helper\Type\Any;
11			use Ffcms\Core\Helper\Type\Str;
12			use Ffcms\Core\Interfaces\iUser;
13
14			/**
15			* Class FormUserUpdate. Update user data business logic model
16			* @package Apps\Model\Admin\User
17			*/
18			class FormUserUpdate extends Model
19			{
20			public $email;
21			public $login;
22			public $password;
23			public $newpassword;
24			public $role_id;
25			public $approved = false;
26
27			public $approve_token;
28
29			/** @var iUser */
30			public $_user;
31
32			/**
33			* FormUserUpdate constructor. Pass user object inside the model
34			* @param iUser $user
35			*/
36			public function __construct(iUser $user)
37			{
38			$this->_user = $user;
39			parent::__construct(true);
40			}
41
42			/**
43			* Load user data on before method
44			*/
45			public function before()
46			{
47			foreach ($this->getAllProperties() as $property => $old_data) {
			0 ignored issues – show Bug introduced 2018-11-23 12:45 UTC by Report Bug Copy Issue Report The expression `$this->getAllProperties()` of type `null` is not traversable. Loading history... Bug introduced 2018-11-23 12:45 UTC by Report Bug Copy Issue Report Are you sure the usage of `$this->getAllProperties()` targeting `Ffcms\Core\Arch\Model::getAllProperties()` seems to always return null. This check looks for function or method calls that always return null and whose return value is used. class A { function getObject() { return null; } } $a = new A(); if ($a->getObject()) { The method `getObject()` can return nothing but null, so it makes no sense to use the return value. The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes. Loading history...
48			if ($this->_user->{$property}) {
49			$this->{$property} = $this->_user->{$property};
50			}
51			}
52
53			if (!$this->approve_token) {
54			$this->approved = true;
55			}
56			}
57
58			/**
59			* Form labels to display
60			* @return array
61			*/
62			public function labels(): array
63			{
64			return [
65			'email' => __('Email'),
66			'login' => __('Login'),
67			'newpassword' => __('New password'),
68			'role_id' => __('Role'),
69			'approved' => __('Approved')
70			];
71			}
72
73			/**
74			* Validation rules for input data
75			* @return array
76			*/
77			public function rules(): array
78			{
79			return [
80			[['email', 'login', 'role_id', 'approved'], 'required'],
81			['newpassword', 'used'],
82			['email', 'email'],
83			['login', 'length_min', 3],
84			['email', 'Apps\Model\Admin\User\FormUserUpdate::isUniqueEmail', $this->_user->getParam('id')],
85			['login', 'Apps\Model\Admin\User\FormUserUpdate::isUniqueLogin', $this->_user->getParam('id')]
86			];
87			}
88
89			/**
90			* Get all roles as id=>name array
91			* @return array\|null
92			*/
93			public function getRoleList()
94			{
95			return Role::getIdNameAll();
			0 ignored issues – show Bug introduced 2018-03-25 17:11 UTC by Report Bug Copy Issue Report Are you sure the usage of `Apps\ActiveRecord\Role::getIdNameAll()` targeting `Apps\ActiveRecord\Role::getIdNameAll()` seems to always return null. This check looks for function or method calls that always return null and whose return value is used. class A { function getObject() { return null; } } $a = new A(); if ($a->getObject()) { The method `getObject()` can return nothing but null, so it makes no sense to use the return value. The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes. Loading history...
96			}
97
98			/**
99			* Update user information in database based on current obj attributes passed from input data
100			*/
101			public function save()
102			{
103			foreach ($this->getAllProperties() as $property => $value) {
			0 ignored issues – show Bug introduced 2018-11-23 12:45 UTC by Report Bug Copy Issue Report Are you sure the usage of `$this->getAllProperties()` targeting `Ffcms\Core\Arch\Model::getAllProperties()` seems to always return null. This check looks for function or method calls that always return null and whose return value is used. class A { function getObject() { return null; } } $a = new A(); if ($a->getObject()) { The method `getObject()` can return nothing but null, so it makes no sense to use the return value. The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes. Loading history... Bug introduced 2018-11-23 12:45 UTC by Report Bug Copy Issue Report The expression `$this->getAllProperties()` of type `null` is not traversable. Loading history...
104			if ($property === 'password' \|\| $property === 'newpassword') {
105			// update password only if new is set and length >= 3
106			if ($this->newpassword && Str::length($this->newpassword) >= 3) {
107			$this->_user->password = Crypt::passwordHash($this->newpassword);
108			}
109			} elseif ($property === 'approved') {
110			if ($this->approved) {
111			$this->_user->approve_token = null;
112			} else {
113			$this->_user->approve_token = $this->approve_token ?? Crypt::randomString(mt_rand(32, 128));
114			}
115			} elseif ($property === 'approve_token') {
116			continue;
117			} else {
118			$this->_user->{$property} = $value;
119			}
120			}
121
122			// get user id before save to determine "add" action
123			$id = $this->_user->id;
124			// safe user row
125			$this->_user->save();
126			// if new user - add profile link
127			if ($id < 1) {
128			$profile = new Profile();
129			$profile->user_id = $this->_user->id;
130			$profile->save();
131			}
132			}
133
134			/**
135			* Check if new email is always exist
136			* @param string $email
137			* @param int\|null $userId
138			* @return bool
139			*/
140			public static function isUniqueEmail($email, $userId = null)
141			{
142			$find = User::where('email', '=', $email);
143
144			if ($userId && Any::isInt($userId)) {
			0 ignored issues – show Bug Best Practice introduced 2018-11-23 12:45 UTC by Report Bug Copy Issue Report The expression `$userId` of type `integer\|null` is loosely compared to `true`; this is ambiguous if the integer can be 0. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
145			$find->where('id', '!=', $userId);
146			}
147
148			return $find->count() === 0;
149			}
150
151			/**
152			* Check if new login is always exist
153			* @param string $login
154			* @param int\|null $userId
155			* @return bool
156			*/
157			public static function isUniqueLogin($login, $userId = null)
158			{
159			$find = User::where('login', '=', $login);
160
161			if ($userId && Any::isInt($userId)) {
			0 ignored issues – show Bug Best Practice introduced 2018-11-23 12:45 UTC by Report Bug Copy Issue Report The expression `$userId` of type `integer\|null` is loosely compared to `true`; this is ambiguous if the integer can be 0. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
162			$find->where('id', '!=', $userId);
163			}
164
165			return $find->count() === 0;
166			}
167			}
168

phpffcms / ffcms

Push — master ( e3c39f...fe570d )

FormUserUpdate::save() B

Complexity

Size

Duplication

Importance

How to fix Complexity

Long Method

Duplication Side-by-Side

Filter issues like