Users_Register_Controller::RegisterForm() - Code Metrics - Inspection of "PHPBCF" - i-lateral/silverstripe-users - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — 1 ( fdc87b...27ddc3 )

by Morven

created 2018-04-28 15:03 UTC

Users_Register_Controller::RegisterForm() B

↳ Parent: Users_Register_Controller

Complexity

Conditions	4
Paths	4

Size

Total Lines	59
Code Lines	36

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	59
rs	8.9846
c	0
b	0
f	0
cc	4
eloc	36
nc	4
nop	0

How to fix Long Method

<?php

/**
 * Base controller class for users to register. Provides extension hooks to
 * allow third party overwriting of both index and register form actions
 *
 * This controller is also used to allow registered accounts to "verify"
 * their details via email.
 *
 * This is done by adding verified users to the groups stipulated by the
 * $verification_groups config variable
 *
 * @package Users
 * @author  i-lateral <[email protected]>
 */
class Users_Register_Controller extends Controller
namespace YourVendor;

class YourClass { }
{

    /**
     * URL That you can access this from
     *
     * @config
     */
    private static $url_segment = "users/register";


    /**
     * Current actions available to this controller
     *
     * @var array
     */
    private static $allowed_actions = array(

        "index",
        "sendverification",
        "verify",
        "RegisterForm"
    );

    /**
     * Internal function designed to allow us to send a verification
     * email from multiple locations
     *
     * @param $member Member object to send email to
     * 
     * @return boolean
     */
    protected function send_verification_email(Member $member)
    {
        if ($member->exists()) {
            return $member->sendVerificationEmail();            
        }

        return false;
    }

    /**
     * Get the link to this controller
     * 
     * @param string $action The URL endpoint for this controller

     * 
     * @return string
     */
    public function Link($action = null)
    {
        return Controller::join_links(
            $this->config()->url_segment,
            $action
        );
    }

    /**
     * Get an absolute link to this controller
     *
     * @param string $action The URL endpoint for this controller

     *
     * @return string

     */
    public function AbsoluteLink($action = null)
    {
        return Director::absoluteURL($this->Link($action));
    }

    /**
     * Get a relative (to the root url of the site) link to this
     * controller
     *
     * @param string $action The URL endpoint for this controller

     *
     * @return string
     */
    public function RelativeLink($action = null)
    {
        return Controller::join_links(
            $this->Link($action)
        );
    }

    /**
     * If content controller exists, return it's menu function
     *
     * @param int $level Menu level to return.
     *
     * @return ArrayList
     */
    public function getMenu($level = 1)

    {
        if (class_exists(ContentController::class)) {
            $controller = Injector::inst()->get(ContentController::class);
            return $controller->getMenu($level);
        }
    }

    /**
     * Shortcut for getMenu
     * 
     * @param int $level Menu level to return.
     * 
     * @return ArrayList
     */
    public function Menu($level)

    {
        return $this->getMenu();
    }

    /**
     * Default action this controller will deal with
     *
     * @param  SS_HTTPRequest $request Current request
     *
     * @return HTMLText
     */
    public function index(SS_HTTPRequest $request)

    {
        $this->customise(
            array(
            'Title'     => _t('Users.Register', 'Register'),
            'MetaTitle' => _t('Users.Register', 'Register'),
            'Form'      => $this->RegisterForm(),
            )
        );

        $this->extend("updateIndexAction");

        return $this->renderWith(
            array(
            "Users_Register",
            "Users",
            "Page"
            )
        );
    }


    /**
     * Send a verification email to the user provided (if verification
     * emails are enabled and account is not already verified)
     *
     * @param SS_HTTPRequest $request Current request
     * 
     * @return HTMLText
     */
    public function sendverification(SS_HTTPRequest $request)

    {
        // If we don't allow verification emails, return an error
        if (!Users::config()->send_verification_email) {
            return $this->httpError(400);
        }

        $sent = false;

        if (Member::currentUserID()) {
            $member = Member::currentUser();
        } else {
            $member = Member::get()->byID($this->getRequest()->param("ID"));
        }

        if ($member && !$member->isVerified()) {
            $sent = $this->send_verification_email($member);
        }

        $this->customise(
            array(
            "Title" => _t('Users.AccountVerification', 'Account Verification'),
            "MetaTitle" => _t('Users.AccountVerification', 'Account Verification'),
            "Content" => $this->renderWith(
                "UsersSendVerificationContent",
                array("Sent" => $sent)
            ),
            "Sent" => $sent
            )
        );

        $this->extend("updateSendVerificationAction");

        return $this->renderWith(
            array(
            "Users_Register_sendverification",
            "Users",
            "Page"
            )
        );
    }

    /**
     * Verify the provided user (ID) using the verification code (Other
     * ID) provided
     *
     * @param SS_HTTPRequest $request Current Request
     *
     * @return HTMLText
     */
    public function verify(SS_HTTPRequest $request)

    {   
        $member = Member::get()->byID($this->getRequest()->param("ID"));
        $code = $this->getRequest()->param("OtherID");
        $verify = false;

        // Check verification group exists, if not, make it
        // Add a verified users group (only used if we turn on
        // verification)
        $verify_groups = Group::get()
            ->filter("Code", Users::config()->verification_groups);

        $this->extend("onBeforeVerify", $member);

        if (($member && $code) && $code == $member->VerificationCode) {
            foreach ($verify_groups as $group) {
                $group->Members()->add($member);
                $verify = true;
            }
        }

        $this->customise(
            array(
            "Title" => _t('Users.AccountVerification', 'Account Verification'),
            "MetaTitle" => _t('Users.AccountVerification', 'Account Verification'),
            "Content" => $this->renderWith(
                "UsersVerifyContent",
                array("Verify" => $verify)
            ),
            "Verify" => $verify
            )
        );

        $this->extend("onAfterVerify", $member);

        return $this->renderWith(
            array(
            "Users_Register_verify",
            "Users",
            "Page"
            )
        );
    }

    /**
     * Registration form
     *
     * @return Form
     */
    public function RegisterForm()
// 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);
    }
}
    {

        // If back URL set, push to session
        if (isset($_REQUEST['BackURL'])) {
            Session::set('BackURL', $_REQUEST['BackURL']);
        }

        $config = Users::config();

        // Setup form fields
        $fields = FieldList::create(
            TextField::create("FirstName"),
            TextField::create("Surname"),
            EmailField::create("Email"),
            $password_field = ConfirmedPasswordField::create("Password")
        );

        $password_field->minLength = $config->get("password_min_length");
        $password_field->maxLength = $config->get("password_max_length");
        $password_field->requireStrongPassword = $config->get("password_require_strong");

        // Setup form actions
        $actions = new FieldList(
            FormAction::create("doRegister", "Register")
                ->addExtraClass("btn")
                ->addExtraClass("btn-green")
        );

        // Setup required fields
        $required = new RequiredFields(
            array(
            "FirstName",
            "Surname",
            "Email",
            "Password"
            )
        );

        $form = Form::create(
            $this,
            "RegisterForm",
            $fields,
            $actions,
            $required
        )->addExtraClass("forms")
        ->addExtraClass("forms-columnar");

        $this->extend("updateRegisterForm", $form);

        $session_data = Session::get("Form.{$form->FormName()}.data");

        if ($session_data && is_array($session_data)) {
            $form->loadDataFrom($session_data);
            Session::clear("Form.{$form->FormName()}.data");
        }

        return $form;
    }

    /**
     * Register a new member. This action is deigned to be intercepted at 2
     * points:
     *
     *  - Modify the initial member filter (so that you can perfom bespoke
     *    member filtering
     *
     *  - Modify the member user before saving (so we can add extra permissions
     *    etc)
     *
     * @param array $data User submitted data
     * @param Form  $form Registration form
     * 
     * @return SS_HTTPResponse

     */
    public function doRegister($data, $form)
    {
        $filter = array();

        if (isset($data['Email'])) {
            $filter['Email'] = $data['Email'];
        }

        $this->extend("updateMemberFilter", $filter);

        // Check if a user already exists
        if ($member = Member::get()->filter($filter)->first()) {
            if ($member) {
                $form->addErrorMessage(
                    "Blurb",
                    "Sorry, an account already exists with those details.",
                    "bad"
                );

                // Load errors into session and post back
                unset($data["Password"]);
                Session::set("Form.{$form->FormName()}.data", $data);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);

                return $this->redirectBack();
            }
        }

        $member = Member::create();
        $member->Register($data);


        $this->extend("updateNewMember", $member, $data);

        $session_url = Session::get("BackURL");
        $request_url = $this->getRequest()->requestVar("BackURL");

        // If a back URL is used in session.
        if (!empty($session_url)) {
            $redirect_url = $session_url;
        } elseif (!empty($request_url)) {
            $redirect_url = $request_url;
        } else {
            $controller = Injector::inst()->get("Users_Account_Controller");
            $redirect_url = $controller->Link();
        }

        return $this->redirect($redirect_url);
    }
}


1		<?php
2
3		/**
4		* Base controller class for users to register. Provides extension hooks to
5		* allow third party overwriting of both index and register form actions
6		*
7		* This controller is also used to allow registered accounts to "verify"
8		* their details via email.
9		*
10		* This is done by adding verified users to the groups stipulated by the
11		* $verification_groups config variable
12		*
13		* @package Users
14		* @author i-lateral <[email protected]>
15		*/
16		class Users_Register_Controller extends Controller
		0 ignored issues – show Coding Style Compatibility introduced 2018-02-21 15:44 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...
17		{
18
19		/**
20		* URL That you can access this from
21		*
22		* @config
23		*/
24		private static $url_segment = "users/register";
		0 ignored issues – show Unused Code introduced 2018-02-21 15:44 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...
25
26		/**
27		* Current actions available to this controller
28		*
29		* @var array
30		*/
31		private static $allowed_actions = array(
		0 ignored issues – show Comprehensibility introduced 2018-02-21 15:44 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 2018-02-21 15:44 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		"index",
33		"sendverification",
34		"verify",
35		"RegisterForm"
36		);
37
38		/**
39		* Internal function designed to allow us to send a verification
40		* email from multiple locations
41		*
42		* @param $member Member object to send email to
43		*
44		* @return boolean
45		*/
46		protected function send_verification_email(Member $member)
47		{
48		if ($member->exists()) {
49		return $member->sendVerificationEmail();
50		}
51
52		return false;
53		}
54
55		/**
56		* Get the link to this controller
57		*
58		* @param string $action The URL endpoint for this controller
		0 ignored issues – show Documentation introduced 2018-04-28 15:04 UTC by Report Bug Copy Issue Report Should the type for parameter `$action` not be `string\|null`? This check looks for `@param` annotations where the type inferred by our type inference engine differs from the declared type. It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
59		*
60		* @return string
61		*/
62		public function Link($action = null)
63		{
64		return Controller::join_links(
65		$this->config()->url_segment,
66		$action
67		);
68		}
69
70		/**
71		* Get an absolute link to this controller
72		*
73		* @param string $action The URL endpoint for this controller
		0 ignored issues – show Documentation introduced 2018-04-28 15:04 UTC by Report Bug Copy Issue Report Should the type for parameter `$action` not be `string\|null`? This check looks for `@param` annotations where the type inferred by our type inference engine differs from the declared type. It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
74		*
75		* @return string
		0 ignored issues – show Documentation introduced 2018-04-28 15:04 UTC by Report Bug Copy Issue Report Should the return type not be `false\|string`? 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...
76		*/
77		public function AbsoluteLink($action = null)
78		{
79		return Director::absoluteURL($this->Link($action));
80		}
81
82		/**
83		* Get a relative (to the root url of the site) link to this
84		* controller
85		*
86		* @param string $action The URL endpoint for this controller
		0 ignored issues – show Documentation introduced 2018-04-28 15:04 UTC by Report Bug Copy Issue Report Should the type for parameter `$action` not be `string\|null`? This check looks for `@param` annotations where the type inferred by our type inference engine differs from the declared type. It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
87		*
88		* @return string
89		*/
90		public function RelativeLink($action = null)
91		{
92		return Controller::join_links(
93		$this->Link($action)
94		);
95		}
96
97		/**
98		* If content controller exists, return it's menu function
99		*
100		* @param int $level Menu level to return.
101		*
102		* @return ArrayList
103		*/
104	View Code Duplication	public function getMenu($level = 1)
		0 ignored issues – show Duplication introduced 2018-02-22 16:50 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
105		{
106		if (class_exists(ContentController::class)) {
107		$controller = Injector::inst()->get(ContentController::class);
108		return $controller->getMenu($level);
109		}
110		}
111
112		/**
113		* Shortcut for getMenu
114		*
115		* @param int $level Menu level to return.
116		*
117		* @return ArrayList
118		*/
119		public function Menu($level)
		0 ignored issues – show Unused Code introduced 2018-04-28 15:04 UTC by Report Bug Copy Issue Report The parameter `$level` 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...
120		{
121		return $this->getMenu();
122		}
123
124		/**
125		* Default action this controller will deal with
126		*
127		* @param SS_HTTPRequest $request Current request
128		*
129		* @return HTMLText
130		*/
131		public function index(SS_HTTPRequest $request)
		0 ignored issues – show Unused Code introduced 2018-02-21 15:44 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...
132		{
133		$this->customise(
134		array(
135		'Title' => _t('Users.Register', 'Register'),
136		'MetaTitle' => _t('Users.Register', 'Register'),
137		'Form' => $this->RegisterForm(),
138		)
139		);
140
141		$this->extend("updateIndexAction");
142
143		return $this->renderWith(
144		array(
145		"Users_Register",
146		"Users",
147		"Page"
148		)
149		);
150		}
151
152
153		/**
154		* Send a verification email to the user provided (if verification
155		* emails are enabled and account is not already verified)
156		*
157		* @param SS_HTTPRequest $request Current request
158		*
159		* @return HTMLText
160		*/
161		public function sendverification(SS_HTTPRequest $request)
		0 ignored issues – show Unused Code introduced 2018-02-22 09:53 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...
162		{
163		// If we don't allow verification emails, return an error
164		if (!Users::config()->send_verification_email) {
165		return $this->httpError(400);
166		}
167
168		$sent = false;
169
170		if (Member::currentUserID()) {
171		$member = Member::currentUser();
172		} else {
173		$member = Member::get()->byID($this->getRequest()->param("ID"));
174		}
175
176		if ($member && !$member->isVerified()) {
177		$sent = $this->send_verification_email($member);
178		}
179
180		$this->customise(
181		array(
182		"Title" => _t('Users.AccountVerification', 'Account Verification'),
183		"MetaTitle" => _t('Users.AccountVerification', 'Account Verification'),
184		"Content" => $this->renderWith(
185		"UsersSendVerificationContent",
186		array("Sent" => $sent)
187		),
188		"Sent" => $sent
189		)
190		);
191
192		$this->extend("updateSendVerificationAction");
193
194		return $this->renderWith(
195		array(
196		"Users_Register_sendverification",
197		"Users",
198		"Page"
199		)
200		);
201		}
202
203		/**
204		* Verify the provided user (ID) using the verification code (Other
205		* ID) provided
206		*
207		* @param SS_HTTPRequest $request Current Request
208		*
209		* @return HTMLText
210		*/
211		public function verify(SS_HTTPRequest $request)
		0 ignored issues – show Unused Code introduced 2018-02-22 09:53 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...
212		{
213		$member = Member::get()->byID($this->getRequest()->param("ID"));
214		$code = $this->getRequest()->param("OtherID");
215		$verify = false;
216
217		// Check verification group exists, if not, make it
218		// Add a verified users group (only used if we turn on
219		// verification)
220		$verify_groups = Group::get()
221		->filter("Code", Users::config()->verification_groups);
222
223		$this->extend("onBeforeVerify", $member);
224
225		if (($member && $code) && $code == $member->VerificationCode) {
226		foreach ($verify_groups as $group) {
227		$group->Members()->add($member);
228		$verify = true;
229		}
230		}
231
232		$this->customise(
233		array(
234		"Title" => _t('Users.AccountVerification', 'Account Verification'),
235		"MetaTitle" => _t('Users.AccountVerification', 'Account Verification'),
236		"Content" => $this->renderWith(
237		"UsersVerifyContent",
238		array("Verify" => $verify)
239		),
240		"Verify" => $verify
241		)
242		);
243
244		$this->extend("onAfterVerify", $member);
245
246		return $this->renderWith(
247		array(
248		"Users_Register_verify",
249		"Users",
250		"Page"
251		)
252		);
253		}
254
255		/**
256		* Registration form
257		*
258		* @return Form
259		*/
260		public function RegisterForm()
		0 ignored issues – show Coding Style introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report `RegisterForm` uses the super-global variable `$_REQUEST` 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...
261		{
262
263		// If back URL set, push to session
264		if (isset($_REQUEST['BackURL'])) {
265		Session::set('BackURL', $_REQUEST['BackURL']);
266		}
267
268		$config = Users::config();
269
270		// Setup form fields
271		$fields = FieldList::create(
272		TextField::create("FirstName"),
273		TextField::create("Surname"),
274		EmailField::create("Email"),
275		$password_field = ConfirmedPasswordField::create("Password")
276		);
277
278		$password_field->minLength = $config->get("password_min_length");
279		$password_field->maxLength = $config->get("password_max_length");
280		$password_field->requireStrongPassword = $config->get("password_require_strong");
281
282		// Setup form actions
283		$actions = new FieldList(
284		FormAction::create("doRegister", "Register")
285		->addExtraClass("btn")
286		->addExtraClass("btn-green")
287		);
288
289		// Setup required fields
290		$required = new RequiredFields(
291		array(
292		"FirstName",
293		"Surname",
294		"Email",
295		"Password"
296		)
297		);
298
299		$form = Form::create(
300		$this,
301		"RegisterForm",
302		$fields,
303		$actions,
304		$required
305		)->addExtraClass("forms")
306		->addExtraClass("forms-columnar");
307
308		$this->extend("updateRegisterForm", $form);
309
310		$session_data = Session::get("Form.{$form->FormName()}.data");
311
312		if ($session_data && is_array($session_data)) {
313		$form->loadDataFrom($session_data);
314		Session::clear("Form.{$form->FormName()}.data");
315		}
316
317		return $form;
318		}
319
320		/**
321		* Register a new member. This action is deigned to be intercepted at 2
322		* points:
323		*
324		* - Modify the initial member filter (so that you can perfom bespoke
325		* member filtering
326		*
327		* - Modify the member user before saving (so we can add extra permissions
328		* etc)
329		*
330		* @param array $data User submitted data
331		* @param Form $form Registration form
332		*
333		* @return SS_HTTPResponse
		0 ignored issues – show Documentation introduced 2018-04-28 15:04 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...
334		*/
335		public function doRegister($data, $form)
336		{
337		$filter = array();
338
339		if (isset($data['Email'])) {
340		$filter['Email'] = $data['Email'];
341		}
342
343		$this->extend("updateMemberFilter", $filter);
344
345		// Check if a user already exists
346		if ($member = Member::get()->filter($filter)->first()) {
347		if ($member) {
348		$form->addErrorMessage(
349		"Blurb",
350		"Sorry, an account already exists with those details.",
351		"bad"
352		);
353
354		// Load errors into session and post back
355		unset($data["Password"]);
356		Session::set("Form.{$form->FormName()}.data", $data);
		0 ignored issues – show Documentation introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report `$data` is of type `array<string,?,{"Email":"?"}>`, 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...
357
358		return $this->redirectBack();
359		}
360		}
361
362		$member = Member::create();
363		$member->Register($data);
		0 ignored issues – show Bug introduced 2018-02-22 15:30 UTC by Report Bug Copy Issue Report The method `Register()` does not exist on `Member`. Did you maybe mean `registerFailedLogin()`? This check marks calls to methods that do not seem to exist on an object. This is most likely the result of a method being renamed without all references to it being renamed likewise. Loading history...
364
365		$this->extend("updateNewMember", $member, $data);
366
367		$session_url = Session::get("BackURL");
368		$request_url = $this->getRequest()->requestVar("BackURL");
369
370		// If a back URL is used in session.
371		if (!empty($session_url)) {
372		$redirect_url = $session_url;
373		} elseif (!empty($request_url)) {
374		$redirect_url = $request_url;
375		} else {
376		$controller = Injector::inst()->get("Users_Account_Controller");
377		$redirect_url = $controller->Link();
378		}
379
380		return $this->redirect($redirect_url);
381		}
382		}
383

i-lateral / silverstripe-users

Push — 1 ( fdc87b...27ddc3 )

Users_Register_Controller::RegisterForm() B

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Duplication Side-by-Side

Filter issues like