Failed Conditions
Branch master (3ce7e2)
by Nick
14:43
created

User   F

Complexity

Total Complexity 73

Size/Duplication

Total Lines 357
Duplicated Lines 0 %

Test Coverage

Coverage 4.19%

Importance

Changes 0
Metric Value
c 0
b 0
f 0
dl 0
loc 357
rs 2.459
ccs 9
cts 215
cp 0.0419
wmc 73

9 Methods

Rating   Name   Duplication   Size   Complexity  
A getPostCodeChangeURL() 0 8 2
B update() 0 36 5
A constructMPData() 0 18 2
B getRegionalReps() 0 13 5
B getRep() 0 28 6
C getUserDetails() 0 41 8
D getUpdateDetails() 0 60 14
F checkUpdateDetails() 0 120 29
A add() 0 16 2

How to fix   Complexity   

Complex Class

Complex classes like User often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use User, and based on these observations, apply Extract Interface, too.

1
<?php
2
/**
3
 * User Class
4
 *
5
 * @package TheyWorkForYou
6
 */
7
8
namespace MySociety\TheyWorkForYou;
9
10
/**
11
 * User
12
 */
13
14
class User {
15
    public function getUserDetails($user_id = False) {
16
        global $THEUSER;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
17
18
        $user = $THEUSER;
19
        if ($user_id && $user_id != $THEUSER->user_id()) {
20
            $user = new \USER;
21
            $valid = $user->init($user_id);
22
23
            if (!$valid || !$user->confirmed || $user->deleted()) {
24
                return array('error' => 'User does not exist');
25
            }
26
        }
27
28
        $data = array();
29
        $data['firstname'] = $user->firstname();
30
        $data['lastname'] = $user->lastname();
31
        $data['name'] = $user->firstname() . " " . $user->lastname();
32
        $data['url'] = $user->url();
33
        $data['email'] = $user->email();
34
        $data['emailpublic'] = $user->emailpublic() == true ? "Yes" : "No";
35
        $data['optin'] = $user->optin() == true ? "Yes" : "No";
36
        $data['postcode']	= $user->postcode();
37
        $data['website']	= $user->url();
38
        $data['registrationtime']	= $user->registrationtime();
39
        $data['status']= $user->status();
40
        $data["deleted"] = $user->deleted();
41
        $data["confirmed"] = $user->confirmed();
42
        $data["status"] = $user->status();
43
        $data["facebook_id"] = $user->facebook_id();
44
        $data['facebook_user'] = $user->facebook_user();
45
46
        $db = new \ParlDB;
47
        $q = $db->query(
48
            'select count(*) as c from video_timestamps where deleted=0 and user_id = :user_id',
49
            array(
50
                ':user_id' => $user->user_id()
51
            )
52
        );
53
        $data['video'] = $q->field(0, 'c');
54
55
        return $data;
56
    }
57
58
    public function getUpdateDetails($this_page, $user) {
59
        $details = array();
60
61
        if ($user->facebook_user) {
62
            $details = $this->getUserDetails();
63
            $details["password"] = '';
64
            $details["emailpublic"] = false;
65
        } else {
66
            $details["firstname"] = trim(get_http_var("firstname"));
67
            $details["lastname"] = trim(get_http_var("lastname"));
68
69
            $details["password"] = trim(get_http_var("password"));
70
            $details["password2"] = trim(get_http_var("password2"));
71
72
            $details["emailpublic"] = get_http_var("emailpublic") == "true" ? true : false;
73
            $details["email"] = trim(get_http_var("em"));
74
75
            $details["url"] = trim(get_http_var("url"));
76
77
            $details["optin"] = get_http_var("optin") == "true" ? true : false;
78
79
            if (get_http_var("remember") != "") {
80
                $remember = get_http_var("remember");
81
                $details["remember"] = $remember[0] == "true" ? true : false;
82
            }
83
84
            if ($details['url'] != '' && !preg_match('/^http/', $details['url'])) {
85
                $details['url'] = 'http://' . $details['url'];
86
            }
87
88
            # these are used when displaying user details
89
            $details['name'] = $details["firstname"] . " " . $details["lastname"];
90
            $details["website"] = $details["url"];
91
            $details['registrationtime'] = $user->registrationtime();
92
            $details['status'] = $user->status();
93
        }
94
95
        $details['mp_alert'] = get_http_var('mp_alert') == 'true' ? true : false;
96
        $details["postcode"] = trim(get_http_var("postcode"));
97
98
        if ($this_page == "otheruseredit") {
99
            $details["user_id"] = trim(get_http_var("u"));
100
            $details["status"] = trim(get_http_var("status"));
101
102
            if (get_http_var("deleted") != "") {
103
                $deleted = get_http_var("deleted");
104
                $details["deleted"] = $deleted[0] == "true" ? true : false;
105
            } else {
106
                $details['deleted'] = false;
107
            }
108
109
            if (get_http_var("confirmed") != "") {
110
                $confirmed = get_http_var("confirmed");
111
                $details["confirmed"] = $confirmed[0] == "true" ? true : false;
112
            } else {
113
                $details['confirmed'] = false;
114
            }
115
        }
116
117
        return $details;
118
    }
119
120
    public function checkUpdateDetails($details) {
121
        global $THEUSER, $this_page;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
122
123
        // This may be a URL that will send the user back to where they were before they
124
        // wanted to join.
125
        $ret = get_http_var("ret");
0 ignored issues
show
Unused Code introduced by
The assignment to $ret is dead and can be removed.
Loading history...
126
127
        $errors = array();
128
129
        // Check each of the things the user has input.
130
        // If there is a problem with any of them, set an entry in the $errors array.
131
        // This will then be used to (a) indicate there were errors and (b) display
132
        // error messages when we show the form again.
133
134
        // facebook user's can only change their postcode so skip all this
135
        if (!isset($details['facebook_user'])) {
136
            // Check first name.
137
            if ($details["firstname"] == "") {
138
                $errors["firstname"] = "Please enter a first name";
139
            }
140
141
            // They don't need a last name. In case Madonna joins.
142
143
            // Check email address is valid and unique.
144
            if ($this_page == "otheruseredit" || $this_page == 'userjoin' || $this_page == 'useredit') {
145
                if ($details["email"] == "") {
146
                    $errors["email"] = "Please enter an email address";
147
148
                } elseif (!validate_email($details["email"])) {
149
                    // validate_email() is in includes/utilities.php
150
                    $errors["email"] = "Please enter a valid email address";
151
152
                } else {
153
154
                    $USER = new \USER;
155
                    $id_of_user_with_this_addresss = $USER->email_exists($details["email"], true);
156
157
                    if ($this_page == "useredit" &&
158
                        get_http_var("u") == "" &&
159
                        $THEUSER->isloggedin()) {
160
                        // User is updating their own info.
161
                        // Check no one else has this email.
162
163
                        if ($id_of_user_with_this_addresss &&
164
                            $id_of_user_with_this_addresss != $THEUSER->user_id()) {
165
                            $errors["email"] = "Someone else has already joined with this email address";
166
                        }
167
168
                    } else {
169
                        // User is joining. Check no one is already here with this email.
170
                        if ($this_page == "userjoin" && $id_of_user_with_this_addresss) {
171
                            $errors["email"] = "There is already a user with this email address";
172
                        }
173
                    }
174
                }
175
            }
176
177
            // Check passwords.
178
            if ($this_page == "userjoin") {
179
180
                // Only *must* enter a password if they're joining.
181
                if ($details["password"] == "") {
182
                    $errors["password"] = "Please enter a password";
183
184
                } elseif (strlen($details["password"]) < 6) {
185
                    $errors["password"] = "Please enter at least six characters";
186
                }
187
188
                if ($details["password2"] == "") {
189
                    $errors["password2"] = "Please enter a password again";
190
                }
191
192
                if ($details["password"] != "" && $details["password2"] != "" && $details["password"] != $details["password2"]) {
193
                    $errors["password"] = "The passwords did not match. Please try again.";
194
                }
195
196
            } else {
197
198
                // Update details pages.
199
200
                if ($details["password"] != "" && strlen($details["password"]) < 6) {
201
                    $errors["password"] = "Please enter at least six characters";
202
                }
203
204
                if ($details["password"] != $details["password2"]) {
205
                    $errors["password"] = "The passwords did not match. Please try again.";
206
                }
207
            }
208
        }
209
210
        // Check postcode (which is not a compulsory field).
211
        if ($details["postcode"] != "") {
212
            if (!validate_postcode($details["postcode"])) {
213
                $errors["postcode"] = "Sorry, this isn't a valid UK postcode.";
214
            } else {
215
                try {
216
                    $mp = new \MySociety\TheyWorkForYou\Member(array(
0 ignored issues
show
Unused Code introduced by
The assignment to $mp is dead and can be removed.
Loading history...
217
                        'postcode' => $details['postcode'],
218
                        'house' => HOUSE_TYPE_COMMONS,
219
                    ));
220
                } catch (MemberException $e) {
221
                    $errors["postcode"] = "Sorry, we could not find an MP for that postcode.";
222
                }
223
            }
224
        }
225
226
        // No checking of URL.
227
228
229
        if ($this_page == "otheruseredit") {
230
231
            // We're editing another user's info.
232
233
            // Could check status here...?
234
235
236
        }
237
238
        // Send the array of any errors back...
239
        return $errors;
240
    }
241
242
    public function update($details) {
243
        global $THEUSER, $this_page, $PAGE;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
244
245
        $results = array();
246
        // There were no errors when the edit user form was submitted,
247
        // so make the changes in the DB.
248
249
        // Who are we updating? $THEUSER or someone else?
250
        if ($this_page == "otheruseredit") {
251
            $who = 'the user&rsquo;s';
252
            $success = $THEUSER->update_other_user ( $details );
253
        } else {
254
            $who = 'your';
255
            $success = $THEUSER->update_self ( $details );
256
        }
257
258
259
        if ($success) {
260
            // No errors, all updated, show results.
261
262
            if ($this_page == 'otheruseredit') {
263
                $this_page = "userview";
264
            } else {
265
                $this_page = "userviewself";
266
            }
267
268
            if ($details['email'] != $THEUSER->email()) {
269
                $results['email_changed'] = True;
270
            }
271
272
273
        } else {
274
            $results['errors'] = array("db" => "Sorry, we were unable to update $who details. Please <a href=\"mailto:" . str_replace('@', '&#64;', CONTACTEMAIL) . "\">let us know</a> what you were trying to change. Thanks.");
1 ignored issue
show
Bug introduced by
The constant MySociety\TheyWorkForYou\CONTACTEMAIL was not found. Maybe you did not declare it correctly or list all dependencies?
Loading history...
275
        }
276
277
        return $results;
278
    }
279
280
    public function add($details) {
281
        global $THEUSER, $PAGE, $this_page;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
282
283
284
        // If this goes well, the user will have their data
285
        // added to the database and a confirmation email
286
        // will be sent to them.
287
        $success = $THEUSER->add ( $details );
288
289
        $errors = array();
290
291
        if (!$success) {
292
            $errors["db"] = "Sorry, we were unable to create an account for you. Please <a href=\"mailto:". str_replace('@', '&#64;', CONTACTEMAIL) . "\">let us know</a>. Thanks.";
1 ignored issue
show
Bug introduced by
The constant MySociety\TheyWorkForYou\CONTACTEMAIL was not found. Maybe you did not declare it correctly or list all dependencies?
Loading history...
293
        }
294
295
        return $errors;
296
    }
297
298 4
    public function getRep($cons_type, $mp_house) {
299 4
        global $THEUSER;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
300 4
        if ( !$THEUSER->has_postcode() ) {
301 4
            return array();
302
        }
303
304
        // User is logged in and has a postcode, or not logged in with a cookied postcode.
305
306
        // (We don't allow the user to search for a postcode if they
307
        // already have one set in their prefs.)
308
309
        // this is for people who have e.g. an English postcode looking at the
310
        // Scottish homepage
311
        try {
312
            $constituencies = \MySociety\TheyWorkForYou\Utility\Postcode::postcodeToConstituencies($THEUSER->postcode());
313
            if ( isset($constituencies[$cons_type]) ) {
314
                $constituency = $constituencies[$cons_type];
315
                $MEMBER = new Member(array('constituency'=>$constituency, 'house'=> $mp_house));
316
            }
317
        } catch ( MemberException $e ) {
318
            return array();
319
        }
320
321
        if (isset($MEMBER) && $MEMBER->valid) {
322
            return $this->constructMPData($MEMBER, $THEUSER, $mp_house);
323
        }
324
325
        return array();
326
    }
327
328
    private function constructMPData($member, $user, $mp_house) {
329
        $mp_data = array();
330
        $mp_data['name'] = $member->full_name();
331
        $mp_data['party'] = $member->party();
332
        $mp_data['constituency'] = $member->constituency();
333
        $left_house = $member->left_house();
334
        $mp_data['former'] = '';
335
        if ($left_house[$mp_house]['date'] != '9999-12-31') {
336
            $mp_data['former'] = 'former';
337
        }
338
        $mp_data['postcode'] = $user->postcode();
339
        $mp_data['mp_url'] = $member->url();
340
        $mp_data['change_url'] = $this->getPostCodeChangeURL();
341
342
        $image = $member->image();
343
        $mp_data['image'] = $image['url'];
344
345
        return $mp_data;
346
    }
347
348 2
    public function getRegionalReps($cons_type, $mp_house) {
349 2
        global $THEUSER;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
350
351 2
        $mreg = array();
352 2
        if ($THEUSER->isloggedin() && $THEUSER->postcode() != '' || $THEUSER->postcode_is_set()) {
0 ignored issues
show
introduced by
Consider adding parentheses for clarity. Current Interpretation: ($THEUSER->isloggedin() ...USER->postcode_is_set(), Probably Intended Meaning: $THEUSER->isloggedin() &...SER->postcode_is_set())
Loading history...
353
            $reps = \MySociety\TheyWorkForYou\Member::getRegionalList($THEUSER->postcode, $mp_house, $cons_type);
354
            foreach ( $reps as $rep ) {
355
                $member = new \MySociety\TheyWorkForYou\Member(array('person_id' => $rep['person_id']));
356
                $mreg[$rep['person_id']] = $this->constructMPData($member, $THEUSER, $mp_house);
357
            }
358
        }
359
360 2
        return $mreg;
361
    }
362
363
    private function getPostCodeChangeURL() {
364
        global $THEUSER;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
365
        $CHANGEURL = new Url('userchangepc');
366
        if ($THEUSER->isloggedin()) {
367
            $CHANGEURL = new Url('useredit');
368
        }
369
370
        return $CHANGEURL->generate();
371
    }
372
373
374
}
375