Users_EditAccountForm::__construct() - Code Metrics - i-lateral/silverstripe-users - Measure and Improve Code Quality continuously with Scrutinizer

Users_EditAccountForm::__construct() B
last analyzed 2020-01-20 14:33 UTC

↳ Parent: Users_EditAccountForm

Complexity

Conditions	2
Paths	2

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	60
rs	8.8727
c	0
b	0
f	0
cc	2
nc	2
nop	2

How to fix Long Method

<?php

/**
 * Default form for editing Member details
 *
 * @package Users
 * @author  i-lateral <[email protected]>
 */
class Users_EditAccountForm extends Form
{

    /**
     * These fields will be ignored by the `Users_EditAccountForm`
     * when generating fields
     * 
     * @var array
     */
    private static $ignore_member_fields = array(
        "LastVisited",
        "FailedLoginCount",
        "DateFormat",
        "TimeFormat",
        "VerificationCode",
        "Password",
        "HasConfiguredDashboard",
        "URLSegment",
        "BlogProfileSummary",
        "BlogProfileImage"
    );

    /**
     * Setup this form
     * 
     * @param Controller $controller Current Controller
     * @param string     $name       Name of this form
     * 
     * @return void

     */
    public function __construct($controller, $name = "Users_EditAccountForm")
    {
        $member = Member::singleton();
        $hidden_fields = array_merge(
            $member->config()->hidden_fields,
            static::config()->ignore_member_fields
        );

        $fields = $member->getFrontEndFields();

        // Remove all "hidden fields"
        foreach ($hidden_fields as $field_name) {
            $fields->removeByName($field_name);
        }

        // Add the current member ID
        $fields->add(HiddenField::create("ID"));

        // Switch locale field
        $fields->replaceField(
            'Locale',
            DropdownField::create(
                "Locale",
                $member->fieldLabel("Locale"),
                i18n::get_existing_translations()
            )
        );

        $this->extend("updateFormFields", $fields);

        $cancel_url = Controller::join_links($controller->Link());

        $actions = new FieldList(
            LiteralField::create(
                "cancelLink",
                '<a class="btn btn-red" href="'.$cancel_url.'">'. _t("Users.CANCEL", "Cancel") .'</a>'
            ),
            FormAction::create("doUpdate", _t("CMSMain.SAVE", "Save"))
                ->addExtraClass("btn")
                ->addExtraClass("btn-green")
        );

        $this->extend("updateFormActions", $actions);

        $required = new RequiredFields(
            $member->config()->required_fields
        );

        $this->extend("updateRequiredFields", $required);

        parent::__construct(
            $controller,
            $name,
            $fields,
            $actions,
            $required
        );
        
        $this->extend("updateForm", $this);
    }

    /**
     * Register a new member
     *
     * @param array $data User submitted data
     * 
     * @return SS_HTTPResponse
     */
    public function doUpdate($data)
    {
        $filter = array();
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
        $member = Member::get()->byID($data["ID"]);

        $this->extend("onBeforeUpdate", $data);

        // Check that a member isn't trying to mess up another users profile
        if (Member::currentUserID() && $member->canEdit(Member::currentUser())) {

            try {
                // Save member
                $this->saveInto($member);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
                $member->write();
                
                $this->sessionMessage(
                    _t("Users.DETAILSUPDATED", "Account details updated"),
                    "success"
                );
            } catch (Exception $e) {
                $this->sessionMessage(
                    $e->getMessage(),
                    "warning"
                );
            }
        } else {
            $this->sessionMessage(
                _t("Users.CANNOTEDIT", "You cannot edit this account"),
                "warning"
            );
        }

        $this->extend("onAfterUpdate", $data);

        return $this->controller->redirectBack();
    }
}


1			<?php
2
3			/**
4			* Default form for editing Member details
5			*
6			* @package Users
7			* @author i-lateral <[email protected]>
8			*/
9			class Users_EditAccountForm extends Form
10			{
11
12			/**
13			* These fields will be ignored by the `Users_EditAccountForm`
14			* when generating fields
15			*
16			* @var array
17			*/
18			private static $ignore_member_fields = array(
19			"LastVisited",
20			"FailedLoginCount",
21			"DateFormat",
22			"TimeFormat",
23			"VerificationCode",
24			"Password",
25			"HasConfiguredDashboard",
26			"URLSegment",
27			"BlogProfileSummary",
28			"BlogProfileImage"
29			);
30
31			/**
32			* Setup this form
33			*
34			* @param Controller $controller Current Controller
35			* @param string $name Name of this form
36			*
37			* @return void
			0 ignored issues – show Comprehensibility Best Practice introduced 2018-04-28 15:04 UTC by Report Bug Copy Issue Report Adding a `@return` annotation to constructors is generally not recommended as a constructor does not have a meaningful return value. Adding a `@return` annotation to a constructor is not recommended, since a constructor does not have a meaningful return value. Please refer to the PHP core documentation on constructors. Loading history...
38			*/
39			public function __construct($controller, $name = "Users_EditAccountForm")
40			{
41			$member = Member::singleton();
42			$hidden_fields = array_merge(
43			$member->config()->hidden_fields,
44			static::config()->ignore_member_fields
45			);
46
47			$fields = $member->getFrontEndFields();
48
49			// Remove all "hidden fields"
50			foreach ($hidden_fields as $field_name) {
51			$fields->removeByName($field_name);
52			}
53
54			// Add the current member ID
55			$fields->add(HiddenField::create("ID"));
56
57			// Switch locale field
58			$fields->replaceField(
59			'Locale',
60			DropdownField::create(
61			"Locale",
62			$member->fieldLabel("Locale"),
63			i18n::get_existing_translations()
64			)
65			);
66
67			$this->extend("updateFormFields", $fields);
68
69			$cancel_url = Controller::join_links($controller->Link());
70
71			$actions = new FieldList(
72			LiteralField::create(
73			"cancelLink",
74			'<a class="btn btn-red" href="'.$cancel_url.'">'. _t("Users.CANCEL", "Cancel") .'</a>'
75			),
76			FormAction::create("doUpdate", _t("CMSMain.SAVE", "Save"))
77			->addExtraClass("btn")
78			->addExtraClass("btn-green")
79			);
80
81			$this->extend("updateFormActions", $actions);
82
83			$required = new RequiredFields(
84			$member->config()->required_fields
85			);
86
87			$this->extend("updateRequiredFields", $required);
88
89			parent::__construct(
90			$controller,
91			$name,
92			$fields,
93			$actions,
94			$required
95			);
96
97			$this->extend("updateForm", $this);
98			}
99
100			/**
101			* Register a new member
102			*
103			* @param array $data User submitted data
104			*
105			* @return SS_HTTPResponse
106			*/
107			public function doUpdate($data)
108			{
109			$filter = array();
			0 ignored issues – show Unused Code introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report `$filter` 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...
110			$member = Member::get()->byID($data["ID"]);
111
112			$this->extend("onBeforeUpdate", $data);
113
114			// Check that a member isn't trying to mess up another users profile
115			if (Member::currentUserID() && $member->canEdit(Member::currentUser())) {
			0 ignored issues – show Bug introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report It seems like `\Member::currentUser()` targeting `Member::currentUser()` can also be of type `object<DataObject>`; however, `DataObject::canEdit()` does only seem to accept `object<Member>\|null`, maybe add an additional type check? This check looks at variables that are passed out again to other methods. If the outgoing method call has stricter type requirements than the method itself, an issue is raised. An additional type check may prevent trouble. Loading history...
116			try {
117			// Save member
118			$this->saveInto($member);
			0 ignored issues – show Bug introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report It seems like `$member` defined by `\Member::get()->byID($data['ID'])` on line `110` can be `null`; however, `Form::saveInto()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
119			$member->write();
120
121			$this->sessionMessage(
122			_t("Users.DETAILSUPDATED", "Account details updated"),
123			"success"
124			);
125			} catch (Exception $e) {
126			$this->sessionMessage(
127			$e->getMessage(),
128			"warning"
129			);
130			}
131			} else {
132			$this->sessionMessage(
133			_t("Users.CANNOTEDIT", "You cannot edit this account"),
134			"warning"
135			);
136			}
137
138			$this->extend("onAfterUpdate", $data);
139
140			return $this->controller->redirectBack();
141			}
142			}
143

i-lateral / silverstripe-users

Users_EditAccountForm::__construct() B last analyzed 2020-01-20 14:33 UTC

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Duplication Side-by-Side

Filter issues like

Users_EditAccountForm::__construct() B
last analyzed 2020-01-20 14:33 UTC