Users_Register_Controller::doRegister() - Code Metrics - Inspection of "Move registration and verification email tasks to..." - i-lateral/silverstripe-users - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — 1 ( 9b7a9d...a879de )

by Morven

created 2018-02-22 15:23 UTC

Users_Register_Controller::doRegister() B

↳ Parent: Users_Register_Controller

Complexity

Conditions	6
Paths	14

Size

Total Lines	47
Code Lines	27

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	47
rs	8.5125
c	0
b	0
f	0
cc	6
eloc	27
nc	14
nop	2

<?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
 *
 */
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

     * @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

     * @return false|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

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

    /**
     * Default action this controller will deal with
     *
     * @param SS_HTTPRequest $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
     * @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
     * @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']);
        }

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

        // 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
     */
    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			*/
14			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...
15			{
16
17			/**
18			* URL That you can access this from
19			*
20			* @config
21			*/
22			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...
23
24			/**
25			* Current actions available to this controller
26			*
27			* @var array
28			*/
29			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...
30			"index",
31			"sendverification",
32			"verify",
33			"RegisterForm"
34			);
35
36			/**
37			* Internal function designed to allow us to send a verification
38			* email from multiple locations
39			*
40			* @param $member Member object to send email to
41			* @return boolean
42			*/
43			protected function send_verification_email(Member $member)
44			{
45			if ($member->exists()) {
46			return $member->sendVerificationEmail();
47			}
48
49			return false;
50			}
51
52			/**
53			* Get the link to this controller
54			*
55			* @param string $action
			0 ignored issues – show Documentation introduced 2018-02-21 15:59 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...
56			* @return string
57			*/
58			public function Link($action = null)
59			{
60			return Controller::join_links(
61			$this->config()->url_segment,
62			$action
63			);
64			}
65
66			/**
67			* Get an absolute link to this controller
68			*
69			* @param string $action
			0 ignored issues – show Documentation introduced 2018-02-21 15:59 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...
70			* @return false\|string
71			*/
72			public function AbsoluteLink($action = null)
73			{
74			return Director::absoluteURL($this->Link($action));
75			}
76
77			/**
78			* Get a relative (to the root url of the site) link to this
79			* controller
80			*
81			* @param string $action
			0 ignored issues – show Documentation introduced 2018-02-21 15:59 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...
82			* @return string
83			*/
84			public function RelativeLink($action = null)
85			{
86			return Controller::join_links(
87			$this->Link($action)
88			);
89			}
90
91			/**
92			* Default action this controller will deal with
93			*
94			* @param SS_HTTPRequest $request
95			* @return HTMLText
96			*/
97			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...
98			{
99			$this->customise(array(
100			'Title' => _t('Users.Register', 'Register'),
101			'MetaTitle' => _t('Users.Register', 'Register'),
102			'Form' => $this->RegisterForm(),
103			));
104
105			$this->extend("updateIndexAction");
106
107			return $this->renderWith(array(
108			"Users_Register",
109			"Users",
110			"Page"
111			));
112			}
113
114
115			/**
116			* Send a verification email to the user provided (if verification
117			* emails are enabled and account is not already verified)
118			*
119			* @param SS_HTTPRequest $request
120			* @return HTMLText
121			*/
122			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...
123			{
124			// If we don't allow verification emails, return an error
125			if (!Users::config()->send_verification_email) {
126			return $this->httpError(400);
127			}
128
129			$sent = false;
130
131			if (Member::currentUserID()) {
132			$member = Member::currentUser();
133			} else {
134			$member = Member::get()->byID($this->getRequest()->param("ID"));
135			}
136
137			if ($member && !$member->isVerified()) {
138			$sent = $this->send_verification_email($member);
139			}
140
141			$this->customise(array(
142			"Title" => _t('Users.AccountVerification','Account Verification'),
143			"MetaTitle" => _t('Users.AccountVerification','Account Verification'),
144			"Content" => $this->renderWith(
145			"UsersSendVerificationContent",
146			array("Sent" => $sent)
147			),
148			"Sent" => $sent
149			));
150
151			$this->extend("updateSendVerificationAction");
152
153			return $this->renderWith(array(
154			"Users_Register_sendverification",
155			"Users",
156			"Page"
157			));
158			}
159
160
161			/**
162			* Verify the provided user (ID) using the verification code (Other
163			* ID) provided
164			*
165			* @param SS_HTTPRequest $request
166			* @return HTMLText
167			*/
168			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...
169			{
170			$member = Member::get()->byID($this->getRequest()->param("ID"));
171			$code = $this->getRequest()->param("OtherID");
172			$verify = false;
173
174			// Check verification group exists, if not, make it
175			// Add a verified users group (only used if we turn on
176			// verification)
177			$verify_groups = Group::get()
178			->filter("Code", Users::config()->verification_groups);
179
180			$this->extend("onBeforeVerify", $member);
181
182			if (($member && $code) && $code == $member->VerificationCode) {
183			foreach ($verify_groups as $group) {
184			$group->Members()->add($member);
185			$verify = true;
186			}
187			}
188
189			$this->customise(array(
190			"Title" => _t('Users.AccountVerification','Account Verification'),
191			"MetaTitle" => _t('Users.AccountVerification','Account Verification'),
192			"Content" => $this->renderWith(
193			"UsersVerifyContent",
194			array("Verify" => $verify)
195			),
196			"Verify" => $verify
197			));
198
199			$this->extend("onAfterVerify", $member);
200
201			return $this->renderWith(array(
202			"Users_Register_verify",
203			"Users",
204			"Page"
205			));
206			}
207
208			/**
209			* Registration form
210			*
211			* @return Form
212			*/
213			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...
214			{
215
216			// If back URL set, push to session
217			if (isset($_REQUEST['BackURL'])) {
218			Session::set('BackURL', $_REQUEST['BackURL']);
219			}
220
221			// Setup form fields
222			$fields = FieldList::create(
223			TextField::create("FirstName"),
224			TextField::create("Surname"),
225			EmailField::create("Email"),
226			ConfirmedPasswordField::create("Password")
227			);
228
229			// Setup form actions
230			$actions = new FieldList(
231			FormAction::create("doRegister", "Register")
232			->addExtraClass("btn")
233			->addExtraClass("btn-green")
234			);
235
236			// Setup required fields
237			$required = new RequiredFields(array(
238			"FirstName",
239			"Surname",
240			"Email",
241			"Password"
242			));
243
244			$form = Form::create($this, "RegisterForm", $fields, $actions, $required)
245			->addExtraClass("forms")
246			->addExtraClass("forms-columnar");
247
248			$this->extend("updateRegisterForm", $form);
249
250			$session_data = Session::get("Form.{$form->FormName()}.data");
251
252			if ($session_data && is_array($session_data)) {
253			$form->loadDataFrom($session_data);
254			Session::clear("Form.{$form->FormName()}.data");
255			}
256
257			return $form;
258			}
259
260			/**
261			* Register a new member. This action is deigned to be intercepted at 2
262			* points:
263			*
264			* - Modify the initial member filter (so that you can perfom bespoke
265			* member filtering
266			*
267			* - Modify the member user before saving (so we can add extra permissions
268			* etc)
269			*
270			* @param array $data User submitted data
271			* @param Form $form Registration form
272			*/
273			public function doRegister($data, $form)
274			{
275			$filter = array();
276
277			if (isset($data['Email'])) {
278			$filter['Email'] = $data['Email'];
279			}
280
281			$this->extend("updateMemberFilter", $filter);
282
283			// Check if a user already exists
284			if ($member = Member::get()->filter($filter)->first()) {
285			if ($member) {
286			$form->addErrorMessage(
287			"Blurb",
288			"Sorry, an account already exists with those details.",
289			"bad"
290			);
291
292			// Load errors into session and post back
293			unset($data["Password"]);
294			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...
295
296			return $this->redirectBack();
297			}
298			}
299
300			$member = Member::create();
301			$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...
302
303			$this->extend("updateNewMember", $member, $data);
304
305			$session_url = Session::get("BackURL");
306			$request_url = $this->getRequest()->requestVar("BackURL");
307
308			// If a back URL is used in session.
309			if (!empty($session_url)) {
310			$redirect_url = $session_url;
311			} elseif (!empty($request_url)) {
312			$redirect_url = $request_url;
313			} else {
314			$controller = Injector::inst()->get("Users_Account_Controller");
315			$redirect_url = $controller->Link();
316			}
317
318			return $this->redirect($redirect_url);
319			}
320			}
321

i-lateral / silverstripe-users

Push — 1 ( 9b7a9d...a879de )

Users_Register_Controller::doRegister() B

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like