Completed
Push — master ( b765dd...e454df )
by Daniel
13:34 queued 10:58
created

ForumHolder::CurrentlyOnline()   B

Complexity

Conditions 4
Paths 5

Size

Total Lines 22
Code Lines 13

Duplication

Lines 0
Ratio 0 %
Metric Value
dl 0
loc 22
rs 8.9197
cc 4
eloc 13
nc 5
nop 0
1
<?php
2
3
/**
4
 * ForumHolder represents the top forum overview page. Its children
5
 * should be Forums. On this page you can also edit your global settings
6
 * for the entire forum.
7
 *
8
 * @package forum
9
 */
10
11
class ForumHolder extends Page
0 ignored issues
show
Coding Style Compatibility introduced by
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...
12
{
13
    private static $avatars_folder = 'forum/avatars/';
0 ignored issues
show
Unused Code introduced by
The property $avatars_folder 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...
14
15
    private static $attachments_folder = 'forum/attachments/';
0 ignored issues
show
Unused Code introduced by
The property $attachments_folder 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...
16
17
    private static $db = array(
0 ignored issues
show
Unused Code introduced by
The property $db 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...
18
        "HolderSubtitle" => "Varchar(200)",
19
        "ProfileSubtitle" => "Varchar(200)",
20
        "ForumSubtitle" => "Varchar(200)",
21
        "HolderAbstract" => "HTMLText",
22
        "ProfileAbstract" => "HTMLText",
23
        "ForumAbstract" => "HTMLText",
24
        "ProfileModify" => "HTMLText",
25
        "ProfileAdd" => "HTMLText",
26
        "DisplaySignatures" => "Boolean",
27
        "ShowInCategories" => "Boolean",
28
        "AllowGravatars" => "Boolean",
29
        "GravatarType" => "Varchar(10)",
30
        "ForbiddenWords" => "Text",
31
        "CanPostType" => "Enum('Anyone, LoggedInUsers, OnlyTheseUsers, NoOne', 'LoggedInUsers')",
32
    );
33
34
    private static $has_one = array();
0 ignored issues
show
Unused Code introduced by
The property $has_one 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...
35
36
    private static $has_many = array(
0 ignored issues
show
Unused Code introduced by
The property $has_many 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...
37
        "Categories" => "ForumCategory"
38
    );
39
40
    private static $allowed_children = array('Forum');
0 ignored issues
show
Unused Code introduced by
The property $allowed_children 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...
41
42
    private static $defaults = array(
0 ignored issues
show
Unused Code introduced by
The property $defaults 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...
43
        "HolderSubtitle" => "Welcome to our forum!",
44
        "ProfileSubtitle" => "Edit Your Profile",
45
        "ForumSubtitle" => "Start a new topic",
46
        "HolderAbstract" => "<p>If this is your first visit, you will need to <a class=\"broken\" title=\"Click here to register\" href=\"ForumMemberProfile/register\">register</a> before you can post. However, you can browse all messages below.</p>",
47
        "ProfileAbstract" => "<p>Please fill out the fields below. You can choose whether some are publically visible by using the checkbox for each one.</p>",
48
        "ForumAbstract" => "<p>From here you can start a new topic.</p>",
49
        "ProfileModify" => "<p>Thanks, your member profile has been modified.</p>",
50
        "ProfileAdd" => "<p>Thanks, you are now signed up to the forum.</p>",
51
    );
52
53
    /**
54
     * If the user has spam protection enabled and setup then we can provide spam
55
     * prevention for the forum. This stores whether we actually want the registration
56
     * form to have such protection
57
     *
58
     * @var bool
59
     */
60
    public static $use_spamprotection_on_register = true;
61
62
    /**
63
     * If the user has spam protection enabled and setup then we can provide spam
64
     * prevention for the forum. This stores whether we actually want the posting
65
     * form (adding, replying) to have such protection
66
     *
67
     * @var bool
68
     */
69
    public static $use_spamprotection_on_posts = false;
70
71
    /**
72
     * Add a hidden field to the form which should remain empty
73
     * If its filled out, we can assume that a spam bot is auto-filling fields.
74
     *
75
     * @var bool
76
     */
77
    public static $use_honeypot_on_register = false;
78
79
    /**
80
     * @var bool If TRUE, each logged in Member who visits a Forum will write the LastViewed field
81
     * which is for the "Currently online" functionality.
82
     */
83
    private static $currently_online_enabled = true;
0 ignored issues
show
Unused Code introduced by
The property $currently_online_enabled 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...
84
85
    public function getCMSFields()
86
    {
87
        $fields = parent::getCMSFields();
88
        $fields->addFieldsToTab("Root.Messages", array(
89
            TextField::create("HolderSubtitle", "Forum Holder Subtitle"),
90
            HTMLEditorField::create("HolderAbstract", "Forum Holder Abstract"),
91
            TextField::create("ProfileSubtitle", "Member Profile Subtitle"),
92
            HTMLEditorField::create("ProfileAbstract", "Member Profile Abstract"),
93
            TextField::create("ForumSubtitle", "Create topic Subtitle"),
94
            HTMLEditorField::create("ForumAbstract", "Create topic Abstract"),
95
            HTMLEditorField::create("ProfileModify", "Create message after modifing forum member"),
96
            HTMLEditorField::create("ProfileAdd", "Create message after adding forum member")
97
        ));
98
        $fields->addFieldsToTab("Root.Settings", array(
99
            CheckboxField::create("DisplaySignatures", "Display Member Signatures?"),
100
            CheckboxField::create("ShowInCategories", "Show Forums In Categories?"),
101
            CheckboxField::create("AllowGravatars", "Allow <a href='http://www.gravatar.com/' target='_blank'>Gravatars</a>?"),
102
            DropdownField::create("GravatarType", "Gravatar Type", array(
103
                "standard" => _t('Forum.STANDARD', 'Standard'),
104
                "identicon" => _t('Forum.IDENTICON', 'Identicon'),
105
                "wavatar" => _t('Forum.WAVATAR', 'Wavatar'),
106
                "monsterid" => _t('Forum.MONSTERID', 'Monsterid'),
107
                "retro" => _t('Forum.RETRO', 'Retro'),
108
                "mm" => _t('Forum.MM', 'Mystery Man'),
109
            ))->setEmptyString('Use Forum Default')
110
        ));
111
112
        // add a grid field to the category tab with all the categories
113
        $categoryConfig = GridFieldConfig::create()
114
            ->addComponents(
115
                new GridFieldSortableHeader(),
116
                new GridFieldButtonRow(),
117
                new GridFieldDataColumns(),
118
                new GridFieldEditButton(),
119
                new GridFieldViewButton(),
120
                new GridFieldDeleteAction(),
121
                new GridFieldAddNewButton('buttons-before-left'),
122
                new GridFieldPaginator(),
123
                new GridFieldDetailForm()
124
            );
125
126
        $categories = GridField::create(
127
            'Category',
128
            _t('Forum.FORUMCATEGORY', 'Forum Category'),
129
            $this->Categories(),
130
            $categoryConfig
131
        );
132
133
        $fields->addFieldsToTab("Root.Categories", $categories);
134
135
136
        $fields->addFieldsToTab("Root.LanguageFilter", array(
137
            TextField::create("ForbiddenWords", "Forbidden words (comma separated)"),
138
            LiteralField::create("FWLabel", "These words will be replaced by an asterisk")
139
        ));
140
141
        $fields->addFieldToTab("Root.Access", HeaderField::create(_t('Forum.ACCESSPOST', 'Who can post to the forum?'), 2));
142
        $fields->addFieldToTab("Root.Access", OptionsetField::create("CanPostType", "", array(
143
            "Anyone" => _t('Forum.READANYONE', 'Anyone'),
144
            "LoggedInUsers" => _t('Forum.READLOGGEDIN', 'Logged-in users'),
145
            "NoOne" => _t('Forum.READNOONE', 'Nobody. Make Forum Read Only')
146
        )));
147
148
        return $fields;
149
    }
150
151
    public function canPost($member = null)
152
    {
153
        if (!$member) {
154
            $member = Member::currentUser();
155
        }
156
157
        if ($this->CanPostType == "NoOne") {
158
            return false;
159
        }
160
161
        if ($this->CanPostType == "Anyone" || $this->canEdit($member)) {
162
            return true;
163
        }
164
165 View Code Duplication
        if ($member) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across 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...
166
            if ($member->IsSuspended()) {
167
                return false;
168
            }
169
            if ($member->IsBanned()) {
170
                return false;
171
            }
172
            if ($this->CanPostType == "LoggedInUsers") {
173
                return true;
174
            }
175
176
            if ($groups = $this->PosterGroups()) {
177
                foreach ($groups as $group) {
178
                    if ($member->inGroup($group)) {
179
                        return true;
180
                    }
181
                }
182
            }
183
        }
184
185
        return false;
186
    }
187
188
    /**
189
     * Ensure that any categories that exist with no forum holder are updated to be owned by the first forum holder
190
     * if there is one. This is required now that multiple forum holds are allowed, and categories belong to holders.
191
     *
192
     * @see sapphire/core/model/DataObject#requireDefaultRecords()
193
     */
194
    public function requireDefaultRecords()
195
    {
196
        parent::requireDefaultRecords();
197
198
        $forumCategories = ForumCategory::get()->filter('ForumHolderID', 0);
199
        if (!$forumCategories->exists()) {
200
            return;
201
        }
202
203
        $forumHolder = ForumHolder::get()->first();
204
        if (!$forumHolder) {
205
            return;
206
        }
207
208
        foreach ($forumCategories as $forumCategory) {
209
            $forumCategory->ForumHolderID = $forumHolder->ID;
210
            $forumCategory->write();
211
        }
212
    }
213
214
    /**
215
     * If we're on the search action, we need to at least show
216
     * a breadcrumb to get back to the ForumHolder page.
217
     * @return string
218
     */
219
    public function Breadcrumbs($maxDepth = 20, $unlinked = false, $stopAtPageType = false, $showHidden = false)
220
    {
221
        if (isset($this->urlParams['Action'])) {
222
            switch ($this->urlParams['Action']) {
223
                case 'search':
224
                    return '<a href="' . $this->Link() . '">' . $this->Title . '</a> &raquo; ' . _t('SEARCHBREADCRUMB', 'Search');
225
                case 'memberlist':
226
                    return '<a href="' . $this->Link() . '">' . $this->Title . '</a> &raquo; ' . _t('MEMBERLIST', 'Member List');
227
                case 'popularthreads':
228
                    return '<a href="' . $this->Link() . '">' . $this->Title . '</a> &raquo; ' . _t('MOSTPOPULARTHREADS', 'Most popular threads');
229
            }
230
        }
231
    }
232
233
234
    /**
235
     * Get the number of total posts
236
     *
237
     * @return int Returns the number of posts
238
     */
239
    public function getNumPosts()
240
    {
241
        return Post::get()
242
            ->innerJoin(ForumHolder::baseForumTable(), "\"Post\".\"ForumID\" = \"ForumPage\".\"ID\"", "ForumPage")
243
            ->filter(array(
244
                "ForumPage.ParentID" => $this->ID
245
            ))
246
            ->count();
247
    }
248
249
250
    /**
251
     * Get the number of total topics (threads)
252
     *
253
     * @return int Returns the number of topics (threads)
254
     */
255
    public function getNumTopics()
256
    {
257
        return ForumThread::get()
258
            ->innerJoin(ForumHolder::baseForumTable(), "\"ForumThread\".\"ForumID\" = \"ForumPage\".\"ID\"", "ForumPage")
259
            ->filter(array(
260
                "ForumPage.ParentID" => $this->ID
261
            ))
262
            ->count();
263
    }
264
265
266
    /**
267
     * Get the number of distinct authors
268
     *
269
     * @return int Returns the number of distinct authors
270
     */
271
    public function getNumAuthors()
272
    {
273
        $baseTable = ForumHolder::baseForumTable();
274
        return (int)DB::prepared_query("
275
			SELECT COUNT(DISTINCT \"Post\".\"AuthorID\")
276
			FROM \"Post\"
277
			JOIN \"{$baseTable}\" AS \"ForumPage\" ON \"Post\".\"ForumID\" = \"ForumPage\".\"ID\"
278
			AND \"ForumPage\".\"ParentID\" = ?",
279
            array($this->ID)
280
        )->value();
281
    }
282
283
    /**
284
     * Is the "Currently Online" functionality enabled?
285
     * @return bool
286
     */
287
    public function CurrentlyOnlineEnabled()
288
    {
289
        return $this->config()->currently_online_enabled;
290
    }
291
292
    /**
293
     * Get a list of currently online users (last 15 minutes)
294
     * that belong to the "forum-members" code {@link Group}.
295
     *
296
     * @return DataList of {@link Member} objects
297
     */
298
    public function CurrentlyOnline()
299
    {
300
        if (!$this->CurrentlyOnlineEnabled()) {
301
            return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type documented by ForumHolder::CurrentlyOnline of type DataList.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
302
        }
303
304
        $groupIDs = array();
305
306
        if ($forumGroup = Group::get()->filter('Code', 'forum-members')->first()) {
307
            $groupIDs[] = $forumGroup->ID;
308
        }
309
310
        if ($adminGroup = Group::get()->filter('Code', array('administrators', 'Administrators'))->first()) {
311
            $groupIDs[] = $adminGroup->ID;
312
        }
313
314
        return Member::get()
315
            ->leftJoin('Group_Members', '"Member"."ID" = "Group_Members"."MemberID"')
316
            ->filter('GroupID', $groupIDs)
317
            ->where('"Member"."LastViewed" > ' . DB::getConn()->datetimeIntervalClause('NOW', '-15 MINUTE'))
0 ignored issues
show
Deprecated Code introduced by
The method DB::getConn() has been deprecated with message: since version 4.0 Use DB::get_conn instead

This method has been deprecated. The supplier of the class has supplied an explanatory message.

The explanatory message should give you some clue as to whether and when the method will be removed from the class and what other method or class to use instead.

Loading history...
318
            ->sort('"Member"."FirstName", "Member"."Surname"');
319
    }
320
321
    /**
322
     * @deprecated 0.5
323
     */
324
    public function LatestMember($limit = 1)
325
    {
326
        user_error('Please use LatestMembers($limit) instead of LatestMember', E_USER_NOTICE);
327
328
        return $this->LatestMembers($limit);
329
    }
330
331
    /**
332
     * Get the latest members from the forum group.
333
     *
334
     * @param int $limit Number of members to return
335
     * @return ArrayList
336
     */
337
    public function getLatestMembers($limit = null)
338
    {
339
        if (!is_null($limit)) {
1 ignored issue
show
Coding Style introduced by
As per coding-style, please use === null instead of is_null.
Loading history...
340
            Deprecation::notice('1.0', '$limit parameter is deprecated, please chain the limit clause');
341
        }
342
        $groupID = DB::query('SELECT "ID" FROM "Group" WHERE "Code" = \'forum-members\'')->value();
343
344
        // if we're just looking for a single MemberID, do a quicker query on the join table.
345
        if ($limit == 1) {
346
            $latestMemberId = DB::prepared_query(
347
                'SELECT MAX("MemberID")
348
				FROM "Group_Members"
349
				WHERE "Group_Members"."GroupID" = ?',
350
                array($groupID)
351
            )->value();
352
353
            $latestMembers = Member::get()->byId($latestMemberId);
354
        } else {
355
            $latestMembers = Member::get()
356
                ->leftJoin('Group_Members', '"Member"."ID" = "Group_Members"."MemberID"')
357
                ->filter('GroupID', $groupID)
358
                ->sort('"Member"."ID" DESC');
359
            if ($limit) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $limit of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. 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...
360
                $latestMembers = $latestMembers->limit($limit);
361
            }
362
        }
363
364
        return $latestMembers;
365
    }
366
367
    /**
368
     * Get a list of Forum Categories
369
     * @return DataList
370
     */
371
    public function getShowInCategories()
372
    {
373
        $forumCategories = ForumCategory::get()->filter('ForumHolderID', $this->ID);
374
        $showInCategories = $this->getField('ShowInCategories');
375
        return $forumCategories->exists() && $showInCategories;
376
    }
377
378
    /**
379
     * Get the forums. Actually its a bit more complex than that
380
     * we need to group by the Forum Categories.
381
     *
382
     * @return ArrayList
383
     */
384
    public function Forums()
0 ignored issues
show
Coding Style introduced by
Forums 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...
385
    {
386
        $categoryText = isset($_REQUEST['Category']) ? Convert::raw2xml($_REQUEST['Category']) : null;
387
        $holder = $this;
388
389
        if ($this->getShowInCategories()) {
390
            return ForumCategory::get()
391
                ->filter('ForumHolderID', $this->ID)
392
                ->filterByCallback(function ($category) use ($categoryText, $holder) {
393
                    // Don't include if we've specified a Category, and it doesn't match this one
394
                    if ($categoryText !== null && $category->Title != $categoryText) {
395
                        return false;
396
                    }
397
398
                    // Get a list of forums that live under this holder & category
399
                    $category->CategoryForums = Forum::get()
400
                        ->filter(array(
401
                            'CategoryID' => $category->ID,
402
                            'ParentID' => $holder->ID,
403
                            'ShowInMenus' => 1
404
                        ))
405
                        ->filterByCallback(function ($forum) {
406
                            return $forum->canView();
407
                        });
408
409
                    return $category->CategoryForums->exists();
410
                });
411
        } else {
412
            return Forum::get()
413
                ->filter(array(
414
                    'ParentID' => $this->ID,
415
                    'ShowInMenus' => 1
416
                ))
417
                ->filterByCallback(function ($forum) {
418
                    return $forum->canView();
419
                });
420
        }
421
    }
422
423
    /**
424
     * A function that returns the correct base table to use for custom forum queries. It uses the getVar stage to determine
425
     * what stage we are looking at, and determines whether to use SiteTree or SiteTree_Live (the general case). If the stage is
426
     * not specified, live is assumed (general case). It is a static function so it can be used for both ForumHolder and Forum.
427
     *
428
     * @return String
429
     */
430
    public static function baseForumTable()
431
    {
432
        $stage = (Controller::curr()->getRequest()) ? Controller::curr()->getRequest()->getVar('stage') : false;
433
        if (!$stage) {
434
            $stage = Versioned::get_live_stage();
435
        }
436
437
        if (
438
            (class_exists('SapphireTest', false) && SapphireTest::is_running_test())
439
            || $stage == "Stage"
440
        ) {
441
            return "SiteTree";
442
        } else {
443
            return "SiteTree_Live";
444
        }
445
    }
446
447
448
    /**
449
     * Is OpenID support available?
450
     *
451
     * This method checks if the {@link OpenIDAuthenticator} is available and
452
     * registered.
453
     *
454
     * @return bool Returns TRUE if OpenID is available, FALSE otherwise.
455
     */
456
    public function OpenIDAvailable()
457
    {
458
        if (class_exists('Authenticator') == false) {
0 ignored issues
show
Coding Style Best Practice introduced by
It seems like you are loosely comparing two booleans. Considering using the strict comparison === instead.

When comparing two booleans, it is generally considered safer to use the strict comparison operator.

Loading history...
459
            return false;
460
        }
461
462
        return Authenticator::is_registered("OpenIDAuthenticator");
463
    }
464
465
466
    /**
467
     * Get the latest posts
468
     *
469
     * @param int $limit Number of posts to return
470
     * @param int $forumID - Forum ID to limit it to
471
     * @param int $threadID - Thread ID to limit it to
472
     * @param int $lastVisit Optional: Unix timestamp of the last visit (GMT)
473
     * @param int $lastPostID Optional: ID of the last read post
474
     */
475
    public function getRecentPosts($limit = 50, $forumID = null, $threadID = null, $lastVisit = null, $lastPostID = null)
476
    {
477
        $filter = array();
478
479
        if ($lastVisit) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $lastVisit of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. 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...
480
            $lastVisit = @date('Y-m-d H:i:s', $lastVisit);
481
        }
482
483
        $lastPostID = (int) $lastPostID;
484
485
        // last post viewed
486
        if ($lastPostID > 0) {
487
            $filter[] = "\"Post\".\"ID\" > '". Convert::raw2sql($lastPostID) ."'";
488
        }
489
490
        // last time visited
491
        if ($lastVisit) {
492
            $filter[] = "\"Post\".\"Created\" > '". Convert::raw2sql($lastVisit) ."'";
493
        }
494
495
        // limit to a forum
496
        if ($forumID) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $forumID of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. 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...
497
            $filter[] = "\"Post\".\"ForumID\" = '". Convert::raw2sql($forumID) ."'";
498
        }
499
500
        // limit to a thread
501
        if ($threadID) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $threadID of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. 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...
502
            $filter[] = "\"Post\".\"ThreadID\" = '". Convert::raw2sql($threadID) ."'";
503
        }
504
505
        // limit to just this forum install
506
        $filter[] = "\"ForumPage\".\"ParentID\"='{$this->ID}'";
507
508
        $posts = Post::get()
509
            ->leftJoin('ForumThread', '"Post"."ThreadID" = "ForumThread"."ID"')
510
            ->leftJoin(ForumHolder::baseForumTable(), '"ForumPage"."ID" = "Post"."ForumID"', 'ForumPage')
511
            ->limit($limit)
512
            ->sort('"Post"."ID"', 'DESC')
513
            ->where($filter);
514
515
        $recentPosts = new ArrayList();
516
        foreach ($posts as $post) {
517
            $recentPosts->push($post);
518
        }
519
        if ($recentPosts->count() > 0) {
520
            return $recentPosts;
521
        }
522
        return null;
523
    }
524
525
526
    /**
527
     * Are new posts available?
528
     *
529
     * @param int $id
530
     * @param array $data Optional: If an array is passed, the timestamp of
531
     *                    the last created post and it's ID will be stored in
532
     *                    it (keys: 'last_id', 'last_created')
533
     * @param int $lastVisit Unix timestamp of the last visit (GMT)
534
     * @param int $lastPostID ID of the last read post
535
     * @param int $thread ID of the relevant topic (set to NULL for all
0 ignored issues
show
Documentation introduced by
There is no parameter named $thread. Did you maybe mean $threadID?

This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit.

Consider the following example. The parameter $ireland is not defined by the method finale(...).

/**
 * @param array $germany
 * @param array $ireland
 */
function finale($germany, $island) {
    return "2:1";
}

The most likely cause is that the parameter was changed, but the annotation was not.

Loading history...
536
     *                     topics)
537
     * @return bool Returns TRUE if there are new posts available, otherwise
538
     *              FALSE.
539
     */
540
    public static function new_posts_available($id, &$data = array(), $lastVisit = null, $lastPostID = null, $forumID = null, $threadID = null)
541
    {
542
        // last post viewed
543
        $query = new SQLSelect(
544
            array(
545
                'LastID' => 'MAX("Post"."ID")',
546
                'LastCreated' => 'MAX("Post"."Created")'
547
            ),
548
            '"Post"',
549
            array('"ForumPage"."ParentID" = ?' => $id)
550
        );
551
        $query->addInnerJoin(ForumHolder::baseForumTable(), '"Post"."ForumID" = "ForumPage"."ID"', 'ForumPage');
552
553
        // Filter by parameters specified
554
        if ($lastPostID) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $lastPostID of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. 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...
555
            $query->addWhere(array('"Post"."ID" > ?' => $lastPostID));
556
        }
557
        if ($lastVisit) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $lastVisit of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. 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...
558
            $query->addWhere(array('"Post"."Created" > ?' => $lastVisit));
559
        }
560
        if ($forumID) {
561
            $query->addWhere(array('"Post"."ForumID" = ?' => $forumID));
562
        }
563
        if ($threadID) {
564
            $query->addWhere(array('"Post"."ThreadID" = ?' => $threadID));
565
        }
566
567
        // Run
568
        $version = $query->execute()->first();
569
        if (!$version) {
570
            return false;
571
        }
572
573
        if ($data) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $data of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using ! empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
574
            $data['last_id'] = (int)$version['LastID'];
575
            $data['last_created'] = strtotime($version['LastCreated']);
576
        }
577
578
        $lastVisit = (int) $lastVisit;
579
580
        if ($lastVisit <= 0) {
581
            $lastVisit = false;
582
        }
583
584
        $lastPostID = (int)$lastPostID;
585
        if ($lastPostID <= 0) {
586
            $lastPostID = false;
587
        }
588
589
        if (!$lastVisit && !$lastPostID) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $lastVisit of type false|integer is loosely compared to false; this is ambiguous if the integer can be zero. 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...
Bug Best Practice introduced by
The expression $lastPostID of type false|integer is loosely compared to false; this is ambiguous if the integer can be zero. 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...
590
            return true;
591
        }
592
        if ($lastVisit && (strtotime($version['LastCreated']) > $lastVisit)) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $lastVisit of type false|integer is loosely compared to true; this is ambiguous if the integer can be zero. 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...
593
            return true;
594
        }
595
596
        if ($lastPostID && ((int)$version['LastID'] > $lastPostID)) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $lastPostID of type false|integer is loosely compared to true; this is ambiguous if the integer can be zero. 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...
597
            return true;
598
        }
599
600
        return false;
601
    }
602
603
    /**
604
     * Helper Method from the template includes. Uses $ForumHolder so in order for it work
605
     * it needs to be included on this page
606
     *
607
     * @return ForumHolder
608
     */
609
    public function getForumHolder()
610
    {
611
        return $this;
612
    }
613
}
614
615
616
class ForumHolder_Controller extends Page_Controller
0 ignored issues
show
Coding Style Compatibility introduced by
PSR1 recommends that each class should be in its own file to aid autoloaders.

Having each class in a dedicated file usually plays nice with PSR autoloaders and is therefore a well established practice. If you use other autoloaders, you might not want to follow this rule.

Loading history...
Coding Style Compatibility introduced by
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...
617
{
618
    private static $allowed_actions = array(
0 ignored issues
show
Unused Code introduced by
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...
619
        'popularthreads',
620
        'login',
621
        'logout',
622
        'search',
623
        'rss',
624
    );
625
626
    public function init()
0 ignored issues
show
Coding Style introduced by
init uses the super-global variable $_SERVER 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...
627
    {
628
        parent::init();
629
630
        Requirements::javascript(THIRDPARTY_DIR . "/jquery/jquery.js");
631
        Requirements::javascript("forum/javascript/jquery.MultiFile.js");
632
        Requirements::javascript("forum/javascript/forum.js");
633
634
        Requirements::themedCSS('Forum', 'forum', 'all');
635
636
        RSSFeed::linkToFeed($this->Link("rss"), _t('ForumHolder.POSTSTOALLFORUMS', "Posts to all forums"));
637
638
        // Set the back url
639 View Code Duplication
        if (isset($_SERVER['REQUEST_URI'])) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across 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...
640
            Session::set('BackURL', $_SERVER['REQUEST_URI']);
641
        } else {
642
            Session::set('BackURL', $this->Link());
643
        }
644
    }
645
646
    /**
647
     * Generate a complete list of all the members data. Return a
648
     * set of all these members sorted by a GET variable
649
     *
650
     * @todo Sort via AJAX
651
     * @return DataObjectSet A DataObjectSet of all the members which are signed up
652
     */
653
    public function memberlist()
0 ignored issues
show
Coding Style introduced by
memberlist uses the super-global variable $_GET 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...
654
    {
655
        return $this->httpError(404);
656
657
        $forumGroupID = (int) DataObject::get_one('Group', "\"Code\" = 'forum-members'")->ID;
0 ignored issues
show
Unused Code introduced by
$forumGroupID = (int) \D...'forum-members\'')->ID; does not seem to be reachable.

This check looks for unreachable code. It uses sophisticated control flow analysis techniques to find statements which will never be executed.

Unreachable code is most often the result of return, die or exit statements that have been added for debug purposes.

function fx() {
    try {
        doSomething();
        return true;
    }
    catch (\Exception $e) {
        return false;
    }

    return false;
}

In the above example, the last return false will never be executed, because a return statement has already been met in every possible execution path.

Loading history...
658
659
        // If sort has been defined then save it as in the session
660
        $order = (isset($_GET['order'])) ? $_GET['order']: "";
661
662
        if (!isset($_GET['start']) || !is_numeric($_GET['start']) || (int) $_GET['start'] < 1) {
663
            $_GET['start'] = 0;
664
        }
665
666
        $SQL_start = (int) $_GET['start'];
667
668
        switch ($order) {
669
            case "joined":
670
//				$members = DataObject::get("Member", "\"GroupID\" = '$forumGroupID'", "\"Member\".\"Created\" ASC", "LEFT JOIN \"Group_Members\" ON \"Member\".\"ID\" = \"Group_Members\".\"MemberID\"", "{$SQL_start},100");
0 ignored issues
show
Unused Code Comprehensibility introduced by
56% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
671
                $members = Member::get()
672
                        ->filter('Member.GroupID', $forumGroupID)
673
                        ->leftJoin('Group_Members', '"Member"."ID" = "Group_Members"."MemberID"')
674
                        ->sort('"Member"."Created" ASC')
675
                        ->limit($SQL_start . ',100');
676
            break;
1 ignored issue
show
Coding Style introduced by
Terminating statement must be indented to the same level as the CASE body
Loading history...
677 View Code Duplication
            case "name":
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across 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...
678
//				$members = DataObject::get("Member", "\"GroupID\" = '$forumGroupID'", "\"Member\".\"Nickname\" ASC", "LEFT JOIN \"Group_Members\" ON \"Member\".\"ID\" = \"Group_Members\".\"MemberID\"", "{$SQL_start},100");
0 ignored issues
show
Unused Code Comprehensibility introduced by
56% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
679
                $members = Member::get()
680
                        ->filter('Member.GroupID', $forumGroupID)
681
                        ->leftJoin('Group_Members', '"Member"."ID" = "Group_Members"."MemberID"')
682
                        ->sort('"Member"."Nickname" ASC')
683
                        ->limit($SQL_start . ',100');
684
            break;
1 ignored issue
show
Coding Style introduced by
Terminating statement must be indented to the same level as the CASE body
Loading history...
685 View Code Duplication
            case "country":
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across 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...
686
//				$members = DataObject::get("Member", "\"GroupID\" = '$forumGroupID' AND \"Member\".\"CountryPublic\" = TRUE", "\"Member\".\"Country\" ASC", "LEFT JOIN \"Group_Members\" ON \"Member\".\"ID\" = \"Group_Members\".\"MemberID\"", "{$SQL_start},100");
0 ignored issues
show
Unused Code Comprehensibility introduced by
55% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
687
                $members = Member::get()
688
                        ->filter(array('Member.GroupID' => $forumGroupID, 'Member.CountryPublic' => true))
689
                        ->leftJoin('Group_Members', '"Member"."ID" = "Group_Members"."MemberID"')
690
                        ->sort('"Member"."Nickname" ASC')
691
                        ->limit($SQL_start . ',100');
692
            break;
1 ignored issue
show
Coding Style introduced by
Terminating statement must be indented to the same level as the CASE body
Loading history...
693
            case "posts":
694
                $query = singleton('Member')->extendedSQL('', "\"NumPosts\" DESC", "{$SQL_start},100");
695
                $query->select[] = "(SELECT COUNT(*) FROM \"Post\" WHERE \"Post\".\"AuthorID\" = \"Member\".\"ID\") AS \"NumPosts\"";
696
                $records = $query->execute();
697
                $members = singleton('Member')->buildDataObjectSet($records, 'DataObjectSet', $query, 'Member');
698
                $members->parseQueryLimit($query);
699
            break;
1 ignored issue
show
Coding Style introduced by
Terminating statement must be indented to the same level as the CASE body
Loading history...
700
            default:
701
                //$members = DataObject::get("Member", "\"GroupID\" = '$forumGroupID'", "\"Member\".\"Created\" DESC", "LEFT JOIN \"Group_Members\" ON \"Member\".\"ID\" = \"Group_Members\".\"MemberID\"", "{$SQL_start},100");
0 ignored issues
show
Unused Code Comprehensibility introduced by
57% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
702
                $members = Member::get()
703
                        ->filter('Member.GroupID', $forumGroupID)
704
                        ->leftJoin('Group_Members', '"Member"."ID" = "Group_Members"."MemberID"')
705
                        ->sort('"Member"."Created" DESC')
706
                        ->limit($SQL_start . ',100');
707
            break;
1 ignored issue
show
Coding Style introduced by
Terminating statement must be indented to the same level as the CASE body
Loading history...
708
        }
709
710
        return array(
0 ignored issues
show
Bug Best Practice introduced by
The return type of return array('Subtitle' ... 'Forum member List')); (array) is incompatible with the return type documented by ForumHolder_Controller::memberlist of type DataObjectSet.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
711
            'Subtitle' => _t('ForumHolder.MEMBERLIST', 'Forum member List'),
712
            'Abstract' => $this->MemberListAbstract,
713
            'Members' => $members,
714
            'Title' => _t('ForumHolder.MEMBERLIST', 'Forum member List')
715
        );
716
    }
717
718
    /**
719
     * Show the 20 most popular threads across all {@link Forum} children.
720
     *
721
     * Two configuration options are available:
722
     * 1. "posts" - most popular threads by posts
723
     * 2. "views" - most popular threads by views
724
     *
725
     * e.g. mysite.com/forums/popularthreads?by=posts
726
     *
727
     * @return array
728
     */
729
    public function popularthreads()
0 ignored issues
show
Coding Style introduced by
popularthreads uses the super-global variable $_GET 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...
730
    {
731
        $start = isset($_GET['start']) ? (int) $_GET['start'] : 0;
732
        $limit = 20;
733
        $method = isset($_GET['by']) ? $_GET['by'] : null;
734
        if (!$method) {
735
            $method = 'posts';
736
        }
737
738
        if ($method == 'posts') {
739
            $threadsQuery = singleton('ForumThread')->buildSQL(
740
                "\"SiteTree\".\"ParentID\" = '" . $this->ID ."'",
741
                "\"PostCount\" DESC",
742
                "$start,$limit",
743
                "LEFT JOIN \"Post\" ON \"Post\".\"ThreadID\" = \"ForumThread\".\"ID\" LEFT JOIN \"SiteTree\" ON \"SiteTree\".\"ID\" = \"ForumThread\".\"ForumID\""
744
            );
745
            $threadsQuery->select[] = "COUNT(\"Post\".\"ID\") AS 'PostCount'";
746
            $threadsQuery->groupby[] = "\"ForumThread\".\"ID\"";
747
            $threads = singleton('ForumThread')->buildDataObjectSet($threadsQuery->execute());
748
            if ($threads) {
749
                $threads->setPageLimits($start, $limit, $threadsQuery->unlimitedRowCount());
750
            }
751
        } elseif ($method == 'views') {
752
            $threads = DataObject::get('ForumThread', '', "\"NumViews\" DESC", '', "$start,$limit");
753
        }
754
755
        return array(
756
            'Title' => _t('ForumHolder.POPULARTHREADS', 'Most popular forum threads'),
757
            'Subtitle' => _t('ForumHolder.POPULARTHREADS', 'Most popular forum threads'),
758
            'Method' => $method,
759
            'Threads' => $threads
0 ignored issues
show
Bug introduced by
The variable $threads does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
760
        );
761
    }
762
763
    /**
764
     * The login action
765
     *
766
     * It simple sets the return URL and forwards to the standard login form.
767
     */
768
    public function login()
769
    {
770
        Session::set('Security.Message.message', _t('Forum.CREDENTIALS'));
771
        Session::set('Security.Message.type', 'status');
772
        Session::set("BackURL", $this->Link());
773
774
        $this->redirect('Security/login');
775
    }
776
777
778
    public function logout()
779
    {
780
        if ($member = Member::currentUser()) {
781
            $member->logOut();
782
        }
783
784
        $this->redirect($this->Link());
785
    }
786
787
    /**
788
     * The search action
789
     *
790
     * @return array Returns an array to render the search results.
791
     */
792
    public function search()
0 ignored issues
show
Coding Style introduced by
search 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...
793
    {
794
        $keywords    = (isset($_REQUEST['Search'])) ? Convert::raw2xml($_REQUEST['Search']) : null;
795
        $order        = (isset($_REQUEST['order'])) ? Convert::raw2xml($_REQUEST['order']) : null;
796
        $start        = (isset($_REQUEST['start'])) ? (int) $_REQUEST['start'] : 0;
797
798
        $abstract = ($keywords) ? "<p>" . sprintf(_t('ForumHolder.SEARCHEDFOR', "You searched for '%s'."), $keywords) . "</p>": null;
799
800
        // get the results of the query from the current search engine
801
        $search = ForumSearch::get_search_engine();
802
803
        if ($search) {
804
            $engine = new $search();
805
806
            $results = $engine->getResults($this->ID, $keywords, $order, $start);
807
        } else {
808
            $results = false;
809
        }
810
811
        //Paginate the results
812
        $results = PaginatedList::create(
813
            $results,
814
            $this->request->getVars()
815
        );
816
817
818
        // if the user has requested this search as an RSS feed then output the contents as xml
819
        // rather than passing it to the template
820
        if (isset($_REQUEST['rss'])) {
821
            $rss = new RSSFeed($results, $this->Link(), _t('ForumHolder.SEARCHRESULTS', 'Search results'), "", "Title", "RSSContent", "RSSAuthor");
822
823
            return $rss->outputToBrowser();
824
        }
825
826
        // attach a link to a RSS feed version of the search results
827
        $rssLink = $this->Link() ."search/?Search=".urlencode($keywords). "&amp;order=".urlencode($order)."&amp;rss";
828
        RSSFeed::linkToFeed($rssLink, _t('ForumHolder.SEARCHRESULTS', 'Search results'));
829
830
        return array(
831
            "Subtitle"        => DBField::create_field('Text', _t('ForumHolder.SEARCHRESULTS', 'Search results')),
832
            "Abstract"        => DBField::create_field('HTMLText', $abstract),
833
            "Query"            => DBField::create_field('Text', $_REQUEST['Search']),
834
            "Order"            => DBField::create_field('Text', ($order) ? $order : "relevance"),
835
            "RSSLink"        => DBField::create_field('HTMLText', $rssLink),
836
            "SearchResults"    => $results
837
        );
838
    }
839
840
    /**
841
     * Get the RSS feed
842
     *
843
     * This method will output the RSS feed with the last 50 posts to the
844
     * browser.
845
     */
846
    public function rss()
0 ignored issues
show
Coding Style introduced by
rss uses the super-global variable $_SERVER 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...
847
    {
848
        HTTP::set_cache_age(3600); // cache for one hour
849
850
        $threadID = null;
851
        $forumID = null;
852
853
        // optionally allow filtering of the forum posts by the url in the format
854
        // rss/thread/$ID or rss/forum/$ID
855
        if (isset($this->urlParams['ID']) && ($action = $this->urlParams['ID'])) {
856
            if (isset($this->urlParams['OtherID']) && ($id = $this->urlParams['OtherID'])) {
857
                switch ($action) {
858
                    case 'forum':
859
                        $forumID = (int) $id;
860
                        break;
861
                    case 'thread':
862
                        $threadID = (int) $id;
863
                }
864
            } else {
865
                // fallback is that it is the ID of a forum like it was in
866
                // previous versions
867
                $forumID = (int) $action;
868
            }
869
        }
870
871
        $data = array('last_created' => null, 'last_id' => null);
872
873
        if (!isset($_SERVER['HTTP_IF_MODIFIED_SINCE']) && !isset($_SERVER['HTTP_IF_NONE_MATCH'])) {
874
            // just to get the version data..
875
            $available = ForumHolder::new_posts_available($this->ID, $data, null, null, $forumID, $threadID);
0 ignored issues
show
Unused Code introduced by
$available 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...
876
877
            // No information provided by the client, just return the last posts
878
            $rss = new RSSFeed(
879
                $this->getRecentPosts(50, $forumID, $threadID),
880
                $this->Link() . 'rss',
881
                sprintf(_t('Forum.RSSFORUMPOSTSTO'), $this->Title),
882
                "",
883
                "Title",
884
                "RSSContent",
885
                "RSSAuthor",
886
                $data['last_created'],
887
                $data['last_id']
888
            );
889
            return $rss->outputToBrowser();
890
        } else {
891
892
            // Return only new posts, check the request headers!
893
            $since = null;
894
            $etag = null;
895
896
            if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) {
897
                // Split the If-Modified-Since (Netscape < v6 gets this wrong)
898
                $since = explode(';', $_SERVER['HTTP_IF_MODIFIED_SINCE']);
899
                // Turn the client request If-Modified-Since into a timestamp
900
                $since = @strtotime($since[0]);
901
                if (!$since) {
902
                    $since = null;
903
                }
904
            }
905
906
            if (isset($_SERVER['HTTP_IF_NONE_MATCH']) && is_numeric($_SERVER['HTTP_IF_NONE_MATCH'])) {
907
                $etag = (int)$_SERVER['HTTP_IF_NONE_MATCH'];
908
            }
909
            if ($available = ForumHolder::new_posts_available($this->ID, $data, $since, $etag, $forumID, $threadID)) {
0 ignored issues
show
Unused Code introduced by
$available 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...
910
                HTTP::register_modification_timestamp($data['last_created']);
911
                $rss = new RSSFeed(
912
                    $this->getRecentPosts(50, $forumID, $threadID, $etag),
913
                    $this->Link() . 'rss',
914
                    sprintf(_t('Forum.RSSFORUMPOSTSTO'), $this->Title),
915
                    "",
916
                    "Title",
917
                    "RSSContent",
918
                    "RSSAuthor",
919
                    $data['last_created'],
920
                    $data['last_id']
921
                );
922
                return $rss->outputToBrowser();
923
            } else {
924
                if ($data['last_created']) {
925
                    HTTP::register_modification_timestamp($data['last_created']);
926
                }
927
928
                if ($data['last_id']) {
929
                    HTTP::register_etag($data['last_id']);
930
                }
931
932
                // There are no new posts, just output an "304 Not Modified" message
933
                HTTP::add_cache_headers();
934
                header('HTTP/1.1 304 Not Modified');
935
            }
936
        }
937
        exit;
0 ignored issues
show
Coding Style Compatibility introduced by
The method rss() contains an exit expression.

An exit expression should only be used in rare cases. For example, if you write a short command line script.

In most cases however, using an exit expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage.

Loading history...
938
    }
939
940
    /**
941
     * Return the GlobalAnnouncements from the individual forums
942
     *
943
     * @return DataObjectSet
944
     */
945
    public function GlobalAnnouncements()
946
    {
947
        //dump(ForumHolder::baseForumTable());
0 ignored issues
show
Unused Code Comprehensibility introduced by
67% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
948
949
        // Get all the forums with global sticky threads
950
        return ForumThread::get()
951
            ->filter('IsGlobalSticky', 1)
952
            ->innerJoin(ForumHolder::baseForumTable(), '"ForumThread"."ForumID"="ForumPage"."ID"', "ForumPage")
953
            ->where('"ForumPage"."ParentID" = '.$this->ID)
954
            ->filterByCallback(function ($thread) {
955
                if ($thread->canView()) {
956
                    $post = Post::get()->filter('ThreadID', $thread->ID)->sort('Post.Created DESC');
957
                    $thread->Post = $post;
958
                    return true;
959
                }
960
            });
961
    }
962
}
963