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

Forum::getTopics()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 47
Code Lines 29

Duplication

Lines 0
Ratio 0 %
Metric Value
dl 0
loc 47
rs 9.0303
cc 1
eloc 29
nc 1
nop 0
1
<?php
2
3
/**
4
 * Forum represents a collection of forum threads. Each thread is a different topic on
5
 * the site. You can customize permissions on a per forum basis in the CMS.
6
 *
7
 * @todo Implement PermissionProvider for editing, creating forums.
8
 *
9
 * @package forum
10
 */
11
12
class Forum 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...
13
{
14
    private static $allowed_children = 'none';
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...
15
16
    private static $icon = "forum/images/treeicons/user";
0 ignored issues
show
Unused Code introduced by
The property $icon 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...
17
18
    /**
19
     * Enable this to automatically notify moderators when a message is posted
20
     * or edited on his forums.
21
     */
22
    public static $notify_moderators = false;
23
24
    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...
25
        "Abstract" => "Text",
26
        "CanPostType" => "Enum('Inherit, Anyone, LoggedInUsers, OnlyTheseUsers, NoOne', 'Inherit')",
27
        "CanAttachFiles" => "Boolean",
28
    );
29
30
    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...
31
        "Moderator" => "Member",
32
        "Category" => "ForumCategory"
33
    );
34
35
    private static $many_many = array(
0 ignored issues
show
Unused Code introduced by
The property $many_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...
36
        'Moderators' => 'Member',
37
        'PosterGroups' => 'Group'
38
    );
39
40
    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...
41
        "ForumPosters" => "LoggedInUsers"
42
    );
43
44
    /**
45
     * Number of posts to include in the thread view before pagination takes effect.
46
     *
47
     * @var int
48
     */
49
    public static $posts_per_page = 8;
50
51
    /**
52
     * When migrating from older versions of the forum it used post ID as the url token
53
     * as of forum 1.0 we now use ThreadID. If you want to enable 301 redirects from post to thread ID
54
     * set this to true
55
     *
56
     * @var bool
57
     */
58
    public static $redirect_post_urls_to_thread = false;
59
60
    /**
61
     * Check if the user can view the forum.
62
     */
63
    public function canView($member = null)
64
    {
65
        if (!$member) {
66
            $member = Member::currentUser();
67
        }
68
        return (parent::canView($member) || $this->canModerate($member));
69
    }
70
71
    /**
72
     * Check if the user can post to the forum and edit his own posts.
73
     */
74
    public function canPost($member = null)
75
    {
76
        if (!$member) {
77
            $member = Member::currentUser();
78
        }
79
80
        if ($this->CanPostType == "Inherit") {
81
            $holder = $this->getForumHolder();
82
            if ($holder) {
83
                return $holder->canPost($member);
84
            }
85
86
            return false;
87
        }
88
89
        if ($this->CanPostType == "NoOne") {
90
            return false;
91
        }
92
93
        if ($this->CanPostType == "Anyone" || $this->canEdit($member)) {
94
            return true;
95
        }
96
97 View Code Duplication
        if ($member = Member::currentUser()) {
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...
98
            if ($member->IsSuspended()) {
99
                return false;
100
            }
101
            if ($member->IsBanned()) {
102
                return false;
103
            }
104
105
            if ($this->CanPostType == "LoggedInUsers") {
106
                return true;
107
            }
108
109
            if ($groups = $this->PosterGroups()) {
110
                foreach ($groups as $group) {
111
                    if ($member->inGroup($group)) {
112
                        return true;
113
                    }
114
                }
115
            }
116
        }
117
118
        return false;
119
    }
120
121
    /**
122
     * Check if user has access to moderator panel and can delete posts and threads.
123
     */
124
    public function canModerate($member = null)
125
    {
126
        if (!$member) {
127
            $member = Member::currentUser();
128
        }
129
130
        if (!$member) {
131
            return false;
132
        }
133
134
        // Admins
135
        if ($this->canEdit($member)) {
136
            return true;
137
        }
138
139
        // Moderators
140
        if ($member->isModeratingForum($this)) {
141
            return true;
142
        }
143
144
        return false;
145
    }
146
147
    /**
148
     * Can we attach files to topics/posts inside this forum?
149
     *
150
     * @return bool Set to TRUE if the user is allowed to, to FALSE if they're
151
     *              not
152
     */
153
    public function canAttach($member = null)
154
    {
155
        return $this->CanAttachFiles ? true : false;
156
    }
157
158
    public function requireTable()
159
    {
160
        // Migrate permission columns
161
        if (DB::get_schema()->hasTable('Forum')) {
162
            $fields = DB::get_schema()->fieldList('Forum');
163
            if (in_array('ForumPosters', array_keys($fields)) && !in_array('CanPostType', array_keys($fields))) {
164
                DB::get_schema()->renameField('Forum', 'ForumPosters', 'CanPostType');
165
                DB::alteration_message('Migrated forum permissions from "ForumPosters" to "CanPostType"', "created");
166
            }
167
        }
168
169
        parent::requireTable();
170
    }
171
172
    /**
173
     * Add default records to database
174
     *
175
     * This function is called whenever the database is built, after the
176
     * database tables have all been created.
177
     */
178
    public function requireDefaultRecords()
179
    {
180
        parent::requireDefaultRecords();
181
182
        $code = "ACCESS_FORUM";
183
        if (!($forumGroup = Group::get()->filter('Code', 'forum-members')->first())) {
184
            $group = new Group();
185
            $group->Code = 'forum-members';
186
            $group->Title = "Forum Members";
187
            $group->write();
188
189
            Permission::grant($group->ID, $code);
190
            DB::alteration_message(_t('Forum.GROUPCREATED', 'Forum Members group created'), 'created');
191
        } elseif (!Permission::get()->filter(array('GroupID' => $forumGroup->ID, 'Code' => $code))->exists()) {
192
            Permission::grant($forumGroup->ID, $code);
193
        }
194
195
        if (!($category = ForumCategory::get()->first())) {
196
            $category = new ForumCategory();
197
            $category->Title = _t('Forum.DEFAULTCATEGORY', 'General');
0 ignored issues
show
Documentation introduced by
The property Title does not exist on object<ForumCategory>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
198
            $category->write();
199
        }
200
201
        if (!ForumHolder::get()->exists()) {
202
            $forumholder = new ForumHolder();
203
            $forumholder->Title = "Forums";
204
            $forumholder->URLSegment = "forums";
205
            $forumholder->Content = "<p>"._t('Forum.WELCOMEFORUMHOLDER', 'Welcome to SilverStripe Forum Module! This is the default ForumHolder page. You can now add forums.')."</p>";
206
            $forumholder->Status = "Published";
207
            $forumholder->write();
208
            $forumholder->publish("Stage", "Live");
209
            DB::alteration_message(_t('Forum.FORUMHOLDERCREATED', 'ForumHolder page created'), "created");
210
211
            $forum = new Forum();
212
            $forum->Title = _t('Forum.TITLE', 'General Discussion');
213
            $forum->URLSegment = "general-discussion";
214
            $forum->ParentID = $forumholder->ID;
215
            $forum->Content = "<p>"._t('Forum.WELCOMEFORUM', 'Welcome to SilverStripe Forum Module! This is the default Forum page. You can now add topics.')."</p>";
216
            $forum->Status = "Published";
217
            $forum->CategoryID = $category->ID;
218
            $forum->write();
219
            $forum->publish("Stage", "Live");
220
221
            DB::alteration_message(_t('Forum.FORUMCREATED', 'Forum page created'), "created");
222
        }
223
    }
224
225
    /**
226
     * Check if we can and should show forums in categories
227
     */
228
    public function getShowInCategories()
229
    {
230
        $holder = $this->getForumHolder();
231
        if ($holder) {
232
            return $holder->getShowInCategories();
233
        }
234
    }
235
236
    /**
237
     * Returns a FieldList with which to create the CMS editing form
238
     *
239
     * @return FieldList The fields to be displayed in the CMS.
240
     */
241
    public function getCMSFields()
242
    {
243
        Requirements::javascript("forum/javascript/ForumAccess.js");
244
        Requirements::css("forum/css/Forum_CMS.css");
245
246
        $fields = parent::getCMSFields();
247
248
        $fields->addFieldToTab("Root.Access", new HeaderField(_t('Forum.ACCESSPOST', 'Who can post to the forum?'), 2));
249
        $fields->addFieldToTab("Root.Access", $optionSetField = new OptionsetField("CanPostType", "", array(
250
            "Inherit" => "Inherit",
251
            "Anyone" => _t('Forum.READANYONE', 'Anyone'),
252
            "LoggedInUsers" => _t('Forum.READLOGGEDIN', 'Logged-in users'),
253
            "OnlyTheseUsers" => _t('Forum.READLIST', 'Only these people (choose from list)'),
254
            "NoOne" => _t('Forum.READNOONE', 'Nobody. Make Forum Read Only')
255
        )));
256
257
        $optionSetField->addExtraClass('ForumCanPostTypeSelector');
258
259
        $fields->addFieldsToTab("Root.Access", array(
260
            new TreeMultiselectField("PosterGroups", _t('Forum.GROUPS', "Groups")),
261
            new OptionsetField("CanAttachFiles", _t('Forum.ACCESSATTACH', 'Can users attach files?'), array(
262
                "1" => _t('Forum.YES', 'Yes'),
263
                "0" => _t('Forum.NO', 'No')
264
            ))
265
        ));
266
267
268
        //Dropdown of forum category selection.
269
        $categories = ForumCategory::get()->map();
270
271
        $fields->addFieldsToTab(
272
            "Root.Main",
273
            DropdownField::create('CategoryID', _t('Forum.FORUMCATEGORY', 'Forum Category'), $categories),
274
            'Content'
275
        );
276
277
        //GridField Config - only need to attach or detach Moderators with existing Member accounts.
278
        $moderatorsConfig = GridFieldConfig::create()
279
            ->addComponent(new GridFieldButtonRow('before'))
280
            ->addComponent(new GridFieldAddExistingAutocompleter('buttons-before-right'))
281
            ->addComponent(new GridFieldToolbarHeader())
282
            ->addComponent($sort = new GridFieldSortableHeader())
283
            ->addComponent($columns = new GridFieldDataColumns())
284
            ->addComponent(new GridFieldDeleteAction(true))
285
            ->addComponent(new GridFieldPageCount('toolbar-header-right'))
286
            ->addComponent($pagination = new GridFieldPaginator());
287
288
        // Use GridField for Moderator management
289
        $moderators = GridField::create(
290
            'Moderators',
291
            _t('MODERATORS', 'Moderators for this forum'),
292
            $this->Moderators(),
293
            $moderatorsConfig
294
            );
295
296
        $columns->setDisplayFields(array(
297
            'Nickname' => 'Nickname',
298
            'FirstName' => 'First name',
299
            'Surname' => 'Surname',
300
            'Email'=> 'Email',
301
            'LastVisited.Long' => 'Last Visit'
302
        ));
303
304
        $sort->setThrowExceptionOnBadDataType(false);
305
        $pagination->setThrowExceptionOnBadDataType(false);
306
307
        $fields->addFieldToTab('Root.Moderators', $moderators);
308
309
        return $fields;
310
    }
311
312
    /**
313
     * Create breadcrumbs
314
     *
315
     * @param int $maxDepth Maximal lenght of the breadcrumb navigation
316
     * @param bool $unlinked Set to TRUE if the breadcrumb should consist of
317
     *                       links, otherwise FALSE.
318
     * @param bool $stopAtPageType Currently not used
319
     * @param bool $showHidden Set to TRUE if also hidden pages should be
320
     *                         displayed
321
     * @return string HTML code to display breadcrumbs
322
     */
323
    public function Breadcrumbs($maxDepth = null, $unlinked = false, $stopAtPageType = false, $showHidden = false)
324
    {
325
        $page = $this;
326
        $nonPageParts = array();
327
        $parts = array();
328
329
        $controller = Controller::curr();
330
        $params = $controller->getURLParams();
331
332
        $forumThreadID = $params['ID'];
333
        if (is_numeric($forumThreadID)) {
334
            if ($topic = ForumThread::get()->byID($forumThreadID)) {
335
                $nonPageParts[] = Convert::raw2xml($topic->getTitle());
336
            }
337
        }
338
339
        while ($page && (!$maxDepth || sizeof($parts) < $maxDepth)) {
1 ignored issue
show
Bug Best Practice introduced by
The expression $maxDepth of type integer|null 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...
Coding Style introduced by
As per coding-style, the use of the aliased function sizeof() should be avoided; please use count() instead.
Loading history...
340
            if ($showHidden || $page->ShowInMenus || ($page->ID == $this->ID)) {
341
                if ($page->URLSegment == 'home') {
342
                    $hasHome = true;
0 ignored issues
show
Unused Code introduced by
$hasHome 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...
343
                }
344
345
                if ($nonPageParts) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $nonPageParts 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...
346
                    $parts[] = '<a href="' . $page->Link() . '">' . Convert::raw2xml($page->Title) . '</a>';
347
                } else {
348
                    $parts[] = (($page->ID == $this->ID) || $unlinked)
349
                            ? Convert::raw2xml($page->Title)
350
                            : '<a href="' . $page->Link() . '">' . Convert::raw2xml($page->Title) . '</a>';
351
                }
352
            }
353
354
            $page = $page->Parent;
355
        }
356
357
        return implode(" &raquo; ", array_reverse(array_merge($nonPageParts, $parts)));
358
    }
359
360
    /**
361
     * Helper Method from the template includes. Uses $ForumHolder so in order for it work
362
     * it needs to be included on this page
363
     *
364
     * @return ForumHolder
365
     */
366
    public function getForumHolder()
367
    {
368
        $holder = $this->Parent();
369
        if ($holder->ClassName=='ForumHolder') {
370
            return $holder;
371
        }
372
    }
373
374
    /**
375
     * Get the latest posting of the forum. For performance the forum ID is stored on the
376
     * {@link Post} object as well as the {@link Forum} object
377
     *
378
     * @return Post
379
     */
380
    public function getLatestPost()
381
    {
382
        return Post::get()->filter('ForumID', $this->ID)->sort('"Post"."ID" DESC')->first();
383
    }
384
385
    /**
386
     * Get the number of total topics (threads) in this Forum
387
     *
388
     * @return int Returns the number of topics (threads)
389
     */
390
    public function getNumTopics()
391
    {
392
        return (int)DB::prepared_query(
393
            'SELECT COUNT("ID") FROM "ForumThread" WHERE "ForumID" = ?',
394
            array($this->ID)
395
        )->value();
396
    }
397
398
    /**
399
     * Get the number of total posts
400
     *
401
     * @return int
402
     */
403
    public function getNumPosts()
404
    {
405
        return (int)DB::prepared_query(
406
            'SELECT COUNT("ID") FROM "Post" WHERE "ForumID" = ?',
407
            array($this->ID)
408
        )->value();
409
    }
410
411
    /**
412
     * Get the number of distinct Authors
413
     *
414
     * @return int
415
     */
416
    public function getNumAuthors()
417
    {
418
        return (int)DB::prepared_query(
419
            'SELECT COUNT(DISTINCT "AuthorID") FROM "Post" WHERE "ForumID" = ?',
420
            array($this->ID)
421
        )->value();
422
    }
423
424
    /**
425
     * Returns the Topics (the first Post of each Thread) for this Forum
426
     * @return DataList
427
     */
428
    public function getTopics()
429
    {
430
        // Get a list of Posts
431
        $posts = Post::get();
432
433
        // Get the underlying query and change it to return the ThreadID and Max(Created) and Max(ID) for each thread
434
        // of those posts
435
        $postQuery = $posts->dataQuery()->query();
436
437
        $postQuery
438
            ->setSelect(array())
439
            ->selectField('MAX("Post"."Created")', 'PostCreatedMax')
440
            ->selectField('MAX("Post"."ID")', 'PostIDMax')
441
            ->selectField('"ThreadID"')
442
            ->setGroupBy('"ThreadID"')
443
            ->addWhere(array('"ForumID" = ?' => $this->ID))
444
            ->setDistinct(false);
445
        $postSQL = $postQuery->sql($postParameters);
0 ignored issues
show
Bug introduced by
The variable $postParameters does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
446
447
        // Get a list of forum threads inside this forum that aren't sticky
448
        $threads = ForumThread::get()->filter(array(
449
            'ForumID' => $this->ID,
450
            'IsGlobalSticky' => 0,
451
            'IsSticky' => 0
452
        ));
453
454
        // Get the underlying query and change it to inner join on the posts list to just show threads that
455
        // have approved (and maybe awaiting) posts, and sort the threads by the most recent post
456
        $threadQuery = $threads->dataQuery()->query();
457
        $threadQuery
458
            ->addSelect(array('"PostMax"."PostCreatedMax", "PostMax"."PostIDMax"'))
459
            ->addInnerJoin(
460
                '('.$postSQL.')',
461
                '"PostMax"."ThreadID" = "ForumThread"."ID"',
462
                'PostMax',
463
                20,
464
                $postParameters
465
            )
466
            ->addOrderBy(array('"PostMax"."PostCreatedMax" DESC', '"PostMax"."PostIDMax" DESC'))
467
            ->setDistinct(false);
468
469
        // Alter the forum threads list to use the new query
470
        $threads = $threads->setDataQuery(new Forum_DataQuery('ForumThread', $threadQuery));
471
472
        // And return the results
473
        return new PaginatedList($threads, Controller::curr()->getRequest());
474
    }
475
476
477
478
    /*
479
     * Returns the Sticky Threads
480
     * @param boolean $include_global Include Global Sticky Threads in the results (default: true)
481
     * @return DataList
482
     */
483
     public function getStickyTopics($include_global = true)
484
     {
485
         // Get Threads that are sticky & in this forum
486
        $where = '("ForumThread"."ForumID" = '.$this->ID.' AND "ForumThread"."IsSticky" = 1)';
487
        // Get Threads that are globally sticky
488
        if ($include_global) {
489
            $where .= ' OR ("ForumThread"."IsGlobalSticky" = 1)';
490
        }
491
492
        // Get the underlying query
493
        $query = ForumThread::get()->where($where)->dataQuery()->query();
494
495
        // Sort by the latest Post in each thread's Created date
496
        $query
497
            ->addSelect('"PostMax"."PostMax"')
498
            // TODO: Confirm this works in non-MySQL DBs
499
            ->addFrom(sprintf(
500
                'LEFT JOIN (SELECT MAX("Created") AS "PostMax", "ThreadID" FROM "Post" WHERE "ForumID" = \'%s\' GROUP BY "ThreadID") AS "PostMax" ON ("PostMax"."ThreadID" = "ForumThread"."ID")',
501
                $this->ID
502
            ))
503
            ->addOrderBy('"PostMax"."PostMax" DESC')
504
            ->setDistinct(false);
505
506
        // Build result as ArrayList
507
        $res = new ArrayList();
508
         $rows = $query->execute();
509
         if ($rows) {
510
             foreach ($rows as $row) {
511
                 $res->push(new ForumThread($row));
512
             }
513
         }
514
515
         return $res;
516
     }
517
}
518
519
/**
520
 * The forum controller class
521
 *
522
 * @package forum
523
 */
524
class Forum_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...
525
{
526
    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...
527
        'AdminFormFeatures',
528
        'deleteattachment',
529
        'deletepost',
530
        'doAdminFormFeatures',
531
        'doPostMessageForm',
532
        'editpost',
533
        'markasspam',
534
        'PostMessageForm',
535
        'reply',
536
        'show',
537
        'starttopic',
538
        'subscribe',
539
        'unsubscribe',
540
        'rss',
541
        'ban',
542
        'ghost'
543
    );
544
545
546
    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...
547
    {
548
        parent::init();
549
        if ($this->redirectedTo()) {
550
            return;
551
        }
552
553
        Requirements::javascript(THIRDPARTY_DIR . "/jquery/jquery.js");
554
        Requirements::javascript("forum/javascript/Forum.js");
555
        Requirements::javascript("forum/javascript/jquery.MultiFile.js");
556
557
        Requirements::themedCSS('Forum', 'forum', 'all');
558
559
        RSSFeed::linkToFeed($this->Parent()->Link("rss/forum/$this->ID"), sprintf(_t('Forum.RSSFORUM', "Posts to the '%s' forum"), $this->Title));
560
        RSSFeed::linkToFeed($this->Parent()->Link("rss"), _t('Forum.RSSFORUMS', 'Posts to all forums'));
561
562
        if (!$this->canView()) {
563
            $messageSet = array(
564
                'default' => _t('Forum.LOGINDEFAULT', 'Enter your email address and password to view this forum.'),
565
                'alreadyLoggedIn' => _t('Forum.LOGINALREADY', 'I&rsquo;m sorry, but you can&rsquo;t access this forum until you&rsquo;ve logged in. If you want to log in as someone else, do so below'),
566
                'logInAgain' => _t('Forum.LOGINAGAIN', 'You have been logged out of the forums. If you would like to log in again, enter a username and password below.')
567
            );
568
569
            Security::permissionFailure($this, $messageSet);
570
            return;
571
        }
572
573
        // Log this visit to the ForumMember if they exist
574
        $member = Member::currentUser();
575
        if ($member && Config::inst()->get('ForumHolder', 'currently_online_enabled')) {
576
            $member->LastViewed = date("Y-m-d H:i:s");
577
            $member->write();
578
        }
579
580
        // Set the back url
581 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...
582
            Session::set('BackURL', $_SERVER['REQUEST_URI']);
583
        } else {
584
            Session::set('BackURL', $this->Link());
585
        }
586
    }
587
588
    /**
589
     * A convenience function which provides nice URLs for an rss feed on this forum.
590
     */
591
    public function rss()
592
    {
593
        $this->redirect($this->Parent()->Link("rss/forum/$this->ID"), 301);
594
    }
595
596
    /**
597
     * Is OpenID support available?
598
     *
599
     * This method checks if the {@link OpenIDAuthenticator} is available and
600
     * registered.
601
     *
602
     * @return bool Returns TRUE if OpenID is available, FALSE otherwise.
603
     */
604
    public function OpenIDAvailable()
605
    {
606
        return $this->Parent()->OpenIDAvailable();
607
    }
608
609
    /**
610
     * Subscribe a user to a thread given by an ID.
611
     *
612
     * Designed to be called via AJAX so return true / false
613
     *
614
     * @return bool
615
     */
616
    public function subscribe(SS_HTTPRequest $request)
617
    {
618
        // Check CSRF
619
        if (!SecurityToken::inst()->checkRequest($request)) {
620
            return $this->httpError(400);
621
        }
622
623
        if (Member::currentUser() && !ForumThread_Subscription::already_subscribed($this->urlParams['ID'])) {
624
            $obj = new ForumThread_Subscription();
625
            $obj->ThreadID = (int) $this->urlParams['ID'];
0 ignored issues
show
Documentation introduced by
The property ThreadID does not exist on object<ForumThread_Subscription>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
626
            $obj->MemberID = Member::currentUserID();
0 ignored issues
show
Documentation introduced by
The property MemberID does not exist on object<ForumThread_Subscription>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
627
            $obj->LastSent = date("Y-m-d H:i:s");
0 ignored issues
show
Documentation introduced by
The property LastSent does not exist on object<ForumThread_Subscription>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
628
            $obj->write();
629
630
            die('1');
0 ignored issues
show
Coding Style Compatibility introduced by
The method subscribe() 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...
631
        }
632
633
        return false;
634
    }
635
636
    /**
637
     * Unsubscribe a user from a thread by an ID
638
     *
639
     * Designed to be called via AJAX so return true / false
640
     *
641
     * @return bool
642
     */
643
    public function unsubscribe(SS_HTTPRequest $request)
644
    {
645
        $member = Member::currentUser();
646
647
        if (!$member) {
648
            Security::permissionFailure($this, _t('LOGINTOUNSUBSCRIBE', 'To unsubscribe from that thread, please log in first.'));
649
        }
650
651
        if (ForumThread_Subscription::already_subscribed($this->urlParams['ID'], $member->ID)) {
652
            DB::prepared_query("
653
				DELETE FROM \"ForumThread_Subscription\"
654
				WHERE \"ThreadID\" = ?
655
				AND \"MemberID\" = ?",
656
                array($this->urlParams['ID'], $member->ID)
657
            );
658
659
            die('1');
0 ignored issues
show
Coding Style Compatibility introduced by
The method unsubscribe() 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...
660
        }
661
662
        return false;
663
    }
664
665
    /**
666
     * Mark a post as spam. Deletes any posts or threads created by that user
667
     * and removes their user account from the site
668
     *
669
     * Must be logged in and have the correct permissions to do marking
670
     */
671
    public function markasspam(SS_HTTPRequest $request)
672
    {
673
        $currentUser = Member::currentUser();
674
        if (!isset($this->urlParams['ID'])) {
675
            return $this->httpError(400);
676
        }
677
        if (!$this->canModerate()) {
678
            return $this->httpError(403);
679
        }
680
681
        // Check CSRF token
682
        if (!SecurityToken::inst()->checkRequest($request)) {
683
            return $this->httpError(400);
684
        }
685
686
        $post = Post::get()->byID($this->urlParams['ID']);
687
        if ($post) {
688
            // post was the start of a thread, Delete the whole thing
689
            if ($post->isFirstPost()) {
690
                $post->Thread()->delete();
691
            }
692
693
            // Delete the current post
694
            $post->delete();
695
            $post->extend('onAfterMarkAsSpam');
696
697
            // Log deletion event
698
            SS_Log::log(sprintf(
699
                'Marked post #%d as spam, by moderator %s (#%d)',
700
                $post->ID,
701
                $currentUser->Email,
702
                $currentUser->ID
703
            ), SS_Log::NOTICE);
704
705
            // Suspend the member (rather than deleting him),
706
            // which gives him or a moderator the chance to revoke a decision.
707
            if ($author = $post->Author()) {
708
                $author->SuspendedUntil = date('Y-m-d', strtotime('+99 years', SS_Datetime::now()->Format('U')));
709
                $author->write();
710
            }
711
712
            SS_Log::log(sprintf(
713
                'Suspended member %s (#%d) for spam activity, by moderator %s (#%d)',
714
                $author->Email,
715
                $author->ID,
716
                $currentUser->Email,
717
                $currentUser->ID
718
            ), SS_Log::NOTICE);
719
        }
720
721
        return (Director::is_ajax()) ? true : $this->redirect($this->Link());
722
    }
723
724
725 View Code Duplication
    public function ban(SS_HTTPRequest $r)
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
726
    {
727
        if (!$r->param('ID')) {
728
            return $this->httpError(404);
729
        }
730
        if (!$this->canModerate()) {
731
            return $this->httpError(403);
732
        }
733
734
        $member = Member::get()->byID($r->param('ID'));
735
        if (!$member || !$member->exists()) {
736
            return $this->httpError(404);
737
        }
738
739
        $member->ForumStatus = 'Banned';
740
        $member->write();
741
742
        // Log event
743
        $currentUser = Member::currentUser();
744
        SS_Log::log(sprintf(
745
            'Banned member %s (#%d), by moderator %s (#%d)',
746
            $member->Email,
747
            $member->ID,
748
            $currentUser->Email,
749
            $currentUser->ID
750
        ), SS_Log::NOTICE);
751
752
        return ($r->isAjax()) ? true : $this->redirectBack();
753
    }
754
755 View Code Duplication
    public function ghost(SS_HTTPRequest $r)
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
756
    {
757
        if (!$r->param('ID')) {
758
            return $this->httpError(400);
759
        }
760
        if (!$this->canModerate()) {
761
            return $this->httpError(403);
762
        }
763
764
        $member = Member::get()->byID($r->param('ID'));
765
        if (!$member || !$member->exists()) {
766
            return $this->httpError(404);
767
        }
768
769
        $member->ForumStatus = 'Ghost';
770
        $member->write();
771
772
        // Log event
773
        $currentUser = Member::currentUser();
774
        SS_Log::log(sprintf(
775
            'Ghosted member %s (#%d), by moderator %s (#%d)',
776
            $member->Email,
777
            $member->ID,
778
            $currentUser->Email,
779
            $currentUser->ID
780
        ), SS_Log::NOTICE);
781
782
        return ($r->isAjax()) ? true : $this->redirectBack();
783
    }
784
785
    /**
786
     * Get posts to display. This method assumes an URL parameter "ID" which contains the thread ID.
787
     * @param string sortDirection The sort order direction, either ASC for ascending (default) or DESC for descending
788
     * @return DataObjectSet Posts
789
     */
790
    public function Posts($sortDirection = "ASC")
0 ignored issues
show
Coding Style introduced by
Posts 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...
791
    {
792
        $numPerPage = Forum::$posts_per_page;
793
794
        $posts = Post::get()
795
            ->filter('ThreadID', $this->urlParams['ID'])
796
            ->sort('Created', $sortDirection);
797
798
        if (isset($_GET['showPost']) && !isset($_GET['start'])) {
799
            $postIDList = clone $posts;
800
            $postIDList = $postIDList->select('ID')->toArray();
801
802
            if ($postIDList->exists()) {
803
                $foundPos = array_search($_GET['showPost'], $postIDList);
804
                $_GET['start'] = floor($foundPos / $numPerPage) * $numPerPage;
805
            }
806
        }
807
808
        if (!isset($_GET['start'])) {
809
            $_GET['start'] = 0;
810
        }
811
812
        $paginated = new PaginatedList($posts, $_GET);
813
        $paginated->setPageLength(Forum::$posts_per_page);
814
        return $paginated;
815
    }
816
817
    /**
818
     * Get the usable BB codes
819
     *
820
     * @return DataObjectSet Returns the usable BB codes
821
     * @see BBCodeParser::usable_tags()
822
     */
823
    public function BBTags()
824
    {
825
        return BBCodeParser::usable_tags();
826
    }
827
828
    /**
829
     * Section for dealing with reply / edit / create threads form
830
     *
831
     * @return Form Returns the post message form
832
     */
833
    public function PostMessageForm($addMode = false, $post = false)
834
    {
835
        $thread = false;
836
837
        if ($post) {
838
            $thread = $post->Thread();
0 ignored issues
show
Bug introduced by
The method Thread cannot be called on $post (of type boolean).

Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects.

Loading history...
839
        } elseif (isset($this->urlParams['ID']) && is_numeric($this->urlParams['ID'])) {
840
            $thread = DataObject::get_by_id('ForumThread', $this->urlParams['ID']);
841
        }
842
843
        // Check permissions
844
        $messageSet = array(
845
            'default' => _t('Forum.LOGINTOPOST', 'You\'ll need to login before you can post to that forum. Please do so below.'),
846
            'alreadyLoggedIn' => _t('Forum.LOGINTOPOSTLOGGEDIN',
847
                 'I\'m sorry, but you can\'t post to this forum until you\'ve logged in.'
848
                .'If you want to log in as someone else, do so below. If you\'re logged in and you still can\'t post, you don\'t have the correct permissions to post.'),
849
            'logInAgain' => _t('Forum.LOGINTOPOSTAGAIN', 'You have been logged out of the forums.  If you would like to log in again to post, enter a username and password below.'),
850
        );
851
852
        // Creating new thread
853
        if ($addMode && !$this->canPost()) {
854
            Security::permissionFailure($this, $messageSet);
855
            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 Forum_Controller::PostMessageForm of type Form.

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...
856
        }
857
858
        // Replying to existing thread
859 View Code Duplication
        if (!$addMode && !$post && $thread && !$thread->canPost()) {
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...
860
            Security::permissionFailure($this, $messageSet);
861
            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 Forum_Controller::PostMessageForm of type Form.

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...
862
        }
863
864
        // Editing existing post
865
        if (!$addMode && $post && !$post->canEdit()) {
0 ignored issues
show
Bug introduced by
The method canEdit cannot be called on $post (of type boolean).

Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects.

Loading history...
866
            Security::permissionFailure($this, $messageSet);
867
            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 Forum_Controller::PostMessageForm of type Form.

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...
868
        }
869
870
        $forumBBCodeHint = $this->renderWith('Forum_BBCodeHint');
871
872
        $fields = new FieldList(
873
            ($post && $post->isFirstPost() || !$thread) ? new TextField("Title", _t('Forum.FORUMTHREADTITLE', 'Title')) : new ReadonlyField('Title',  _t('Forum.FORUMTHREADTITLE', ''), 'Re:'. $thread->Title),
0 ignored issues
show
Bug introduced by
The method isFirstPost cannot be called on $post (of type boolean).

Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects.

Loading history...
874
            new TextareaField("Content", _t('Forum.FORUMREPLYCONTENT', 'Content')),
875
            new LiteralField(
876
                "BBCodeHelper",
877
                "<div class=\"BBCodeHint\">[ <a href=\"#BBTagsHolder\" id=\"BBCodeHint\">" .
878
                _t('Forum.BBCODEHINT', 'View Formatting Help') .
879
                "</a> ]</div>" .
880
                $forumBBCodeHint
881
            ),
882
            new CheckboxField("TopicSubscription",
883
                _t('Forum.SUBSCRIBETOPIC', 'Subscribe to this topic (Receive email notifications when a new reply is added)'),
884
                ($thread) ? $thread->getHasSubscribed() : false)
885
        );
886
887
        if ($thread) {
888
            $fields->push(new HiddenField('ThreadID', 'ThreadID', $thread->ID));
889
        }
890
        if ($post) {
891
            $fields->push(new HiddenField('ID', 'ID', $post->ID));
892
        }
893
894
        // Check if we can attach files to this forum's posts
895
        if ($this->canAttach()) {
896
            $fields->push(FileField::create("Attachment", _t('Forum.ATTACH', 'Attach file')));
897
        }
898
899
        // If this is an existing post check for current attachments and generate
900
        // a list of the uploaded attachments
901
        if ($post && $attachmentList = $post->Attachments()) {
0 ignored issues
show
Bug introduced by
The method Attachments cannot be called on $post (of type boolean).

Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects.

Loading history...
902
            if ($attachmentList->exists()) {
903
                $attachments = "<div id=\"CurrentAttachments\"><h4>". _t('Forum.CURRENTATTACHMENTS', 'Current Attachments') ."</h4><ul>";
904
                $link = $this->Link();
905
                // An instance of the security token
906
                $token = SecurityToken::inst();
907
908
                foreach ($attachmentList as $attachment) {
909
                    // Generate a link properly, since it requires a security token
910
                    $attachmentLink = Controller::join_links($link, 'deleteattachment', $attachment->ID);
911
                    $attachmentLink = $token->addToUrl($attachmentLink);
912
913
                    $attachments .= "<li class='attachment-$attachment->ID'>$attachment->Name [<a href='{$attachmentLink}' rel='$attachment->ID' class='deleteAttachment'>"
914
                            . _t('Forum.REMOVE', 'remove') . "</a>]</li>";
915
                }
916
                $attachments .= "</ul></div>";
917
918
                $fields->push(new LiteralField('CurrentAttachments', $attachments));
919
            }
920
        }
921
922
        $actions = new FieldList(
923
            new FormAction("doPostMessageForm", _t('Forum.REPLYFORMPOST', 'Post'))
924
        );
925
926
        $required = $addMode === true ? new RequiredFields("Title", "Content") : new RequiredFields("Content");
927
928
        $form = new Form($this, 'PostMessageForm', $fields, $actions, $required);
929
930
        $this->extend('updatePostMessageForm', $form, $post);
931
932
        if ($post) {
933
            $form->loadDataFrom($post);
0 ignored issues
show
Documentation introduced by
$post is of type boolean, but the function expects a array|object<DataObject>.

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...
934
        }
935
936
        return $form;
937
    }
938
939
    /**
940
     * Wrapper for older templates. Previously the new, reply and edit forms were 3 separate
941
     * forms, they have now been refactored into 1 form. But in order to not break existing
942
     * themes too much just include this.
943
     *
944
     * @deprecated 0.5
945
     * @return Form
946
     */
947
    public function ReplyForm()
948
    {
949
        user_error('Please Use $PostMessageForm in your template rather that $ReplyForm', E_USER_WARNING);
950
951
        return $this->PostMessageForm();
952
    }
953
954
    /**
955
     * Post a message to the forum. This method is called whenever you want to make a
956
     * new post or edit an existing post on the forum
957
     *
958
     * @param Array - Data
959
     * @param Form - Submitted Form
960
     */
961
    public function doPostMessageForm($data, $form)
962
    {
963
        $member = Member::currentUser();
964
        $content = (isset($data['Content'])) ? $this->filterLanguage($data["Content"]) : "";
965
        $title = (isset($data['Title'])) ? $this->filterLanguage($data["Title"]) : false;
966
967
        // If a thread id is passed append the post to the thread. Otherwise create
968
        // a new thread
969
        $thread = false;
970
        if (isset($data['ThreadID'])) {
971
            $thread = DataObject::get_by_id('ForumThread', $data['ThreadID']);
972
        }
973
974
        // If this is a simple edit the post then handle it here. Look up the correct post,
975
        // make sure we have edit rights to it then update the post
976
        $post = false;
977
        if (isset($data['ID'])) {
978
            $post = DataObject::get_by_id('Post', $data['ID']);
979
980
            if ($post && $post->isFirstPost()) {
981
                if ($title) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $title of type string|false is loosely compared to true; this is ambiguous if the string can be empty. You might want to explicitly use !== false instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
982
                    $thread->Title = $title;
983
                }
984
            }
985
        }
986
987
988
        // Check permissions
989
        $messageSet = array(
990
            'default' => _t('Forum.LOGINTOPOST', 'You\'ll need to login before you can post to that forum. Please do so below.'),
991
            'alreadyLoggedIn' => _t('Forum.NOPOSTPERMISSION', 'I\'m sorry, but you do not have permission post to this forum.'),
992
            'logInAgain' => _t('Forum.LOGINTOPOSTAGAIN', 'You have been logged out of the forums.  If you would like to log in again to post, enter a username and password below.'),
993
        );
994
995
        // Creating new thread
996
        if (!$thread && !$this->canPost()) {
997
            Security::permissionFailure($this, $messageSet);
998
            return false;
999
        }
1000
1001
        // Replying to existing thread
1002 View Code Duplication
        if ($thread && !$post && !$thread->canPost()) {
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...
1003
            Security::permissionFailure($this, $messageSet);
1004
            return false;
1005
        }
1006
1007
        // Editing existing post
1008
        if ($thread && $post && !$post->canEdit()) {
1009
            Security::permissionFailure($this, $messageSet);
1010
            return false;
1011
        }
1012
1013
        if (!$thread) {
1014
            $thread = new ForumThread();
1015
            $thread->ForumID = $this->ID;
0 ignored issues
show
Documentation introduced by
The property ForumID does not exist on object<ForumThread>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
1016
            if ($title) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $title of type string|false is loosely compared to true; this is ambiguous if the string can be empty. You might want to explicitly use !== false instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
1017
                $thread->Title = $title;
0 ignored issues
show
Documentation introduced by
The property Title does not exist on object<ForumThread>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
1018
            }
1019
            $starting_thread = true;
1020
        }
1021
1022
        // Upload and Save all files attached to the field
1023
        // Attachment will always be blank, If they had an image it will be at least in Attachment-0
1024
        //$attachments = new DataObjectSet();
0 ignored issues
show
Unused Code Comprehensibility introduced by
50% 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...
1025
        $attachments = new ArrayList();
1026
1027
        if (!empty($data['Attachment-0']) && !empty($data['Attachment-0']['tmp_name'])) {
1028
            $id = 0;
1029
            //
1030
            // @todo this only supports ajax uploads. Needs to change the key (to simply Attachment).
1031
            //
1032
            while (isset($data['Attachment-' . $id])) {
1033
                $image = $data['Attachment-' . $id];
1034
1035
                if ($image && !empty($image['tmp_name'])) {
1036
                    $file = Post_Attachment::create();
1037
                    $file->OwnerID = Member::currentUserID();
1038
                    $folder = Config::inst()->get('ForumHolder', 'attachments_folder');
1039
1040
                    try {
1041
                        $upload = Upload::create()->loadIntoFile($image, $file, $folder);
0 ignored issues
show
Unused Code introduced by
$upload 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...
1042
                        $file->write();
1043
                        $attachments->push($file);
1044
                    } catch (ValidationException $e) {
1045
                        $message = _t('Forum.UPLOADVALIDATIONFAIL', 'Unallowed file uploaded. Please only upload files of the following: ');
1046
                        $message .= implode(', ', Config::inst()->get('File', 'allowed_extensions'));
1047
                        $form->addErrorMessage('Attachment', $message, 'bad');
1048
1049
                        Session::set("FormInfo.Form_PostMessageForm.data", $data);
1050
1051
                        return $this->redirectBack();
1052
                    }
1053
                }
1054
1055
                $id++;
1056
            }
1057
        }
1058
1059
        // from now on the user has the correct permissions. save the current thread settings
1060
        $thread->write();
1061
1062
        if (!$post || !$post->canEdit()) {
1063
            $post = new Post();
1064
            $post->AuthorID = ($member) ? $member->ID : 0;
0 ignored issues
show
Documentation introduced by
The property AuthorID does not exist on object<Post>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
1065
            $post->ThreadID = $thread->ID;
0 ignored issues
show
Documentation introduced by
The property ThreadID does not exist on object<Post>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
1066
        }
1067
1068
        $post->ForumID = $thread->ForumID;
1069
        $post->Content = $content;
1070
        $post->write();
1071
1072
1073
        if ($attachments) {
1074
            foreach ($attachments as $attachment) {
1075
                $attachment->PostID = $post->ID;
1076
                $attachment->write();
1077
            }
1078
        }
1079
1080
        // Add a topic subscription entry if required
1081
        $isSubscribed = ForumThread_Subscription::already_subscribed($thread->ID);
1082
        if (isset($data['TopicSubscription'])) {
1083
            if (!$isSubscribed) {
1084
                // Create a new topic subscription for this member
1085
                $obj = new ForumThread_Subscription();
1086
                $obj->ThreadID = $thread->ID;
0 ignored issues
show
Documentation introduced by
The property ThreadID does not exist on object<ForumThread_Subscription>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
1087
                $obj->MemberID = Member::currentUserID();
0 ignored issues
show
Documentation introduced by
The property MemberID does not exist on object<ForumThread_Subscription>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
1088
                $obj->write();
1089
            }
1090
        } elseif ($isSubscribed) {
1091
            // See if the member wanted to remove themselves
1092
            DB::prepared_query(
1093
                'DELETE FROM "ForumThread_Subscription" WHERE "ThreadID" = ? AND "MemberID" = ?',
1094
                array($post->ThreadID, $member->ID)
1095
            );
1096
        }
1097
1098
        // Send any notifications that need to be sent
1099
        ForumThread_Subscription::notify($post);
0 ignored issues
show
Compatibility introduced by
$post of type object<DataObject> is not a sub-type of object<Post>. It seems like you assume a child class of the class DataObject to be always present.

This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass.

Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type.

Loading history...
1100
1101
        // Send any notifications to moderators of the forum
1102
        if (Forum::$notify_moderators) {
1103
            if (isset($starting_thread) && $starting_thread) {
1104
                $this->notifyModerators($post, $thread, true);
1105
            } else {
1106
                $this->notifyModerators($post, $thread);
1107
            }
1108
        }
1109
1110
        return $this->redirect($post->Link());
1111
    }
1112
1113
    /**
1114
     * Send email to moderators notifying them the thread has been created or post added/edited.
1115
     */
1116
    public function notifyModerators($post, $thread, $starting_thread = false)
1117
    {
1118
        $moderators = $this->Moderators();
1119
        if ($moderators && $moderators->exists()) {
1120
            foreach ($moderators as $moderator) {
1121
                if ($moderator->Email) {
1122
                    $adminEmail = Config::inst()->get('Email', 'admin_email');
1123
1124
                    $email = new Email();
1125
                    $email->setFrom($adminEmail);
1126
                    $email->setTo($moderator->Email);
1127
                    if ($starting_thread) {
1128
                        $email->setSubject('New thread "' . $thread->Title . '" in forum ['. $this->Title.']');
1129
                    } else {
1130
                        $email->setSubject('New post "' . $post->Title. '" in forum ['.$this->Title.']');
1131
                    }
1132
                    $email->setTemplate('ForumMember_NotifyModerator');
1133
                    $email->populateTemplate(new ArrayData(array(
1134
                        'NewThread' => $starting_thread,
1135
                        'Moderator' => $moderator,
1136
                        'Author' => $post->Author(),
1137
                        'Forum' => $this,
1138
                        'Post' => $post
1139
                    )));
1140
1141
                    $email->send();
1142
                }
1143
            }
1144
        }
1145
    }
1146
1147
    /**
1148
     * Return the Forbidden Words in this Forum
1149
     *
1150
     * @return Text
1151
     */
1152
    public function getForbiddenWords()
1153
    {
1154
        return $this->Parent()->ForbiddenWords;
1155
    }
1156
1157
    /**
1158
    * This function filters $content by forbidden words, entered in forum holder.
1159
    *
1160
    * @param String $content (it can be Post Content or Post Title)
1161
    * @return String $content (filtered string)
1162
    */
1163
    public function filterLanguage($content)
1164
    {
1165
        $words = $this->getForbiddenWords();
1166
        if ($words != "") {
1167
            $words = explode(",", $words);
1168
            foreach ($words as $word) {
1169
                $content = str_ireplace(trim($word), "*", $content);
1170
            }
1171
        }
1172
1173
        return $content;
1174
    }
1175
1176
    /**
1177
     * Get the link for the reply action
1178
     *
1179
     * @return string URL for the reply action
1180
     */
1181
    public function ReplyLink()
1182
    {
1183
        return $this->Link() . 'reply/' . $this->urlParams['ID'];
1184
    }
1185
1186
    /**
1187
     * Show will get the selected thread to the user. Also increments the forums view count.
1188
     *
1189
     * If the thread does not exist it will pass the user to the 404 error page
1190
     *
1191
     * @return array|SS_HTTPResponse_Exception
1192
     */
1193
    public function show()
1194
    {
1195
        $title = Convert::raw2xml($this->Title);
1196
1197
        if ($thread = $this->getForumThread()) {
1198
1199
            //If there is not first post either the thread has been removed or thread if a banned spammer.
1200
            if (!$thread->getFirstPost()) {
1201
                // don't hide the post for logged in admins or moderators
1202
                $member = Member::currentUser();
1203
                if (!$this->canModerate($member)) {
1204
                    return $this->httpError(404);
1205
                }
1206
            }
1207
1208
            $thread->incNumViews();
1209
1210
            $posts = sprintf(_t('Forum.POSTTOTOPIC', "Posts to the %s topic"), $thread->Title);
0 ignored issues
show
Documentation introduced by
The property Title does not exist on object<ForumThread>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
1211
1212
            RSSFeed::linkToFeed($this->Link("rss") . '/thread/' . (int) $this->urlParams['ID'], $posts);
1213
1214
            $title = Convert::raw2xml($thread->Title) . ' &raquo; ' . $title;
0 ignored issues
show
Documentation introduced by
The property Title does not exist on object<ForumThread>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
1215
            $field = DBField::create_field('HTMLText', $title);
1216
1217
            return array(
1218
                'Thread' => $thread,
1219
                'Title' => $field
1220
            );
1221
        } else {
1222
            // if redirecting post ids to thread id is enabled then we need
1223
            // to check to see if this matches a post and if it does redirect
1224
            if (Forum::$redirect_post_urls_to_thread && isset($this->urlParams['ID']) && is_numeric($this->urlParams['ID'])) {
1225
                if ($post = Post::get()->byID($this->urlParams['ID'])) {
1226
                    return $this->redirect($post->Link(), 301);
1227
                }
1228
            }
1229
        }
1230
1231
        return $this->httpError(404);
1232
    }
1233
1234
    /**
1235
     * Start topic action
1236
     *
1237
     * @return array Returns an array to render the start topic page
1238
     */
1239
    public function starttopic()
1240
    {
1241
        $topic = array(
1242
            'Subtitle' => DBField::create_field('HTMLText', _t('Forum.NEWTOPIC', 'Start a new topic')),
1243
            'Abstract' => DBField::create_field('HTMLText', DataObject::get_one("ForumHolder")->ForumAbstract)
1244
        );
1245
        return $topic;
1246
    }
1247
1248
    /**
1249
     * Get the forum title
1250
     *
1251
     * @return string Returns the forum title
1252
     */
1253
    public function getHolderSubtitle()
1254
    {
1255
        return $this->dbObject('Title');
1256
    }
1257
1258
    /**
1259
     * Get the currently viewed forum. Ensure that the user can access it
1260
     *
1261
     * @return ForumThread
1262
     */
1263
    public function getForumThread()
1264
    {
1265
        if (isset($this->urlParams['ID'])) {
1266
            $SQL_id = Convert::raw2sql($this->urlParams['ID']);
1267
1268
            if (is_numeric($SQL_id)) {
1269
                if ($thread = DataObject::get_by_id('ForumThread', $SQL_id)) {
1270
                    if (!$thread->canView()) {
1271
                        Security::permissionFailure($this);
1272
1273
                        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 Forum_Controller::getForumThread of type ForumThread.

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...
1274
                    }
1275
1276
                    return $thread;
1277
                }
1278
            }
1279
        }
1280
1281
        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 Forum_Controller::getForumThread of type ForumThread.

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...
1282
    }
1283
1284
    /**
1285
     * Delete an Attachment
1286
     * Called from the EditPost method. Its Done via Ajax
1287
     *
1288
     * @return boolean
1289
     */
1290
    public function deleteattachment(SS_HTTPRequest $request)
1291
    {
1292
        // Check CSRF token
1293
        if (!SecurityToken::inst()->checkRequest($request)) {
1294
            return $this->httpError(400);
1295
        }
1296
1297
        // check we were passed an id and member is logged in
1298
        if (!isset($this->urlParams['ID'])) {
1299
            return false;
1300
        }
1301
1302
        $file = DataObject::get_by_id("Post_Attachment", (int) $this->urlParams['ID']);
1303
1304
        if ($file && $file->canDelete()) {
1305
            $file->delete();
1306
1307
            return (!Director::is_ajax()) ? $this->redirectBack() : true;
1308
        }
1309
1310
        return false;
1311
    }
1312
1313
    /**
1314
     * Edit post action
1315
     *
1316
     * @return array Returns an array to render the edit post page
1317
     */
1318
    public function editpost()
1319
    {
1320
        return array(
1321
            'Subtitle' => _t('Forum.EDITPOST', 'Edit post')
1322
        );
1323
    }
1324
1325
    /**
1326
     * Get the post edit form if the user has the necessary permissions
1327
     *
1328
     * @return Form
1329
     */
1330
    public function EditForm()
1331
    {
1332
        $id = (isset($this->urlParams['ID'])) ? $this->urlParams['ID'] : null;
1333
        $post = DataObject::get_by_id('Post', $id);
1334
1335
        return $this->PostMessageForm(false, $post);
1336
    }
1337
1338
1339
    /**
1340
     * Delete a post via the url.
1341
     *
1342
     * @return bool
1343
     */
1344
    public function deletepost(SS_HTTPRequest $request)
1345
    {
1346
        // Check CSRF token
1347
        if (!SecurityToken::inst()->checkRequest($request)) {
1348
            return $this->httpError(400);
1349
        }
1350
1351
        if (isset($this->urlParams['ID'])) {
1352
            if ($post = DataObject::get_by_id('Post', (int) $this->urlParams['ID'])) {
1353
                if ($post->canDelete()) {
1354
                    // delete the whole thread if this is the first one. The delete action
1355
                    // on thread takes care of the posts.
1356
                    if ($post->isFirstPost()) {
1357
                        $thread = DataObject::get_by_id("ForumThread", $post->ThreadID);
1358
                        $thread->delete();
1359
                    } else {
1360
                        // delete the post
1361
                        $post->delete();
1362
                    }
1363
                }
1364
            }
1365
        }
1366
1367
        return (Director::is_ajax()) ? true : $this->redirect($this->Link());
1368
    }
1369
1370
    /**
1371
     * Returns the Forum Message from Session. This
1372
     * is used for things like Moving thread messages
1373
     * @return String
1374
     */
1375
    public function ForumAdminMsg()
1376
    {
1377
        $message = Session::get('ForumAdminMsg');
1378
        Session::clear('ForumAdminMsg');
1379
1380
        return $message;
1381
    }
1382
1383
1384
    /**
1385
     * Forum Admin Features form.
1386
     * Handles the dropdown to select the new forum category and the checkbox for stickyness
1387
     *
1388
     * @return Form
1389
     */
1390
    public function AdminFormFeatures()
1391
    {
1392
        if (!$this->canModerate()) {
1393
            return;
1394
        }
1395
1396
        $id = (isset($this->urlParams['ID'])) ? $this->urlParams['ID'] : false;
1397
1398
        $fields = new FieldList(
1399
            new CheckboxField('IsSticky', _t('Forum.ISSTICKYTHREAD', 'Is this a Sticky Thread?')),
1400
            new CheckboxField('IsGlobalSticky', _t('Forum.ISGLOBALSTICKY', 'Is this a Global Sticky (shown on all forums)')),
1401
            new CheckboxField('IsReadOnly', _t('Forum.ISREADONLYTHREAD', 'Is this a Read only Thread?')),
1402
            new HiddenField("ID", "Thread")
1403
        );
1404
1405
        if (($forums = Forum::get()) && $forums->exists()) {
1406
            $fields->push(new DropdownField("ForumID", _t('Forum.CHANGETHREADFORUM', "Change Thread Forum"), $forums->map('ID', 'Title', 'Select New Category:')), '', null, 'Select New Location:');
0 ignored issues
show
Unused Code introduced by
The call to FieldList::push() has too many arguments starting with null.

This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.

In this case you can add the @ignore PhpDoc annotation to the duplicate definition and it will be ignored.

Loading history...
1407
        }
1408
1409
        $actions = new FieldList(
1410
            new FormAction('doAdminFormFeatures', _t('Forum.SAVE', 'Save'))
1411
        );
1412
1413
        $form = new Form($this, 'AdminFormFeatures', $fields, $actions);
1414
1415
        // need this id wrapper since the form method is called on save as
1416
        // well and needs to return a valid form object
1417
        if ($id) {
1418
            $thread = ForumThread::get()->byID($id);
1419
            $form->loadDataFrom($thread);
0 ignored issues
show
Bug introduced by
It seems like $thread defined by \ForumThread::get()->byID($id) on line 1418 can be null; however, Form::loadDataFrom() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

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

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

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
1420
        }
1421
1422
        return $form;
1423
    }
1424
1425
    /**
1426
     * Process's the moving of a given topic. Has to check for admin privledges,
1427
     * passed an old topic id (post id in URL) and a new topic id
1428
     */
1429
    public function doAdminFormFeatures($data, $form)
1430
    {
1431
        if (isset($data['ID'])) {
1432
            $thread = ForumThread::get()->byID($data['ID']);
1433
1434
            if ($thread) {
1435
                if (!$thread->canModerate()) {
1436
                    return Security::permissionFailure($this);
1437
                }
1438
1439
                $form->saveInto($thread);
1440
                $thread->write();
1441
            }
1442
        }
1443
1444
        return $this->redirect($this->Link());
1445
    }
1446
}
1447
1448
/**
1449
 * This is a DataQuery that allows us to replace the underlying query. Hopefully this will
1450
 * be a native ability in 3.1, but for now we need to.
1451
 * TODO: Remove once API in core
1452
 */
1453
class Forum_DataQuery extends DataQuery
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...
1454
{
1455
    public function __construct($dataClass, $query)
1456
    {
1457
        parent::__construct($dataClass);
1458
        $this->query = $query;
1459
    }
1460
}
1461