Completed
Branch master (4e8684)
by Michael
05:31 queued 02:57
created

NewbbDigestHandler::buildDigest()   D

Complexity

Conditions 14
Paths 184

Size

Total Lines 79
Code Lines 59

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 14
eloc 59
nc 184
nop 1
dl 0
loc 79
rs 4.7562
c 0
b 0
f 0

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
/**
4
 * NewBB 5.0x,  the forum module for XOOPS project
5
 *
6
 * @copyright      XOOPS Project (http://xoops.org)
7
 * @license        GNU GPL 2 or later (http://www.gnu.org/licenses/gpl-2.0.html)
8
 * @author         Taiwen Jiang (phppp or D.J.) <[email protected]>
9
 * @since          4.00
10
 * @package        module::newbb
11
 */
12
class Digest extends XoopsObject
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
    public $digest_id;
15
    public $digest_time;
16
    public $digest_content;
17
18
    public $items;
19
    public $isHtml    = false;
20
    public $isSummary = true;
21
22
    /**
23
     *
24
     */
25 View Code Duplication
    public function __construct()
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...
26
    {
27
        parent::__construct();
28
        $this->initVar('digest_id', XOBJ_DTYPE_INT);
29
        $this->initVar('digest_time', XOBJ_DTYPE_INT);
30
        $this->initVar('digest_content', XOBJ_DTYPE_TXTAREA);
31
        $this->items = [];
32
    }
33
34
    public function setHtml()
35
    {
36
        $this->isHtml = true;
37
    }
38
39
    public function setSummary()
40
    {
41
        $this->isSummary = true;
42
    }
43
44
    /**
45
     * @param        $title
46
     * @param        $link
47
     * @param        $author
48
     * @param string $summary
49
     */
50
    public function addItem($title, $link, $author, $summary = '')
51
    {
52
        $title  = $this->cleanup($title);
53
        $author = $this->cleanup($author);
54
        if (!empty($summary)) {
55
            $summary = $this->cleanup($summary);
56
        }
57
        $this->items[] = ['title' => $title, 'link' => $link, 'author' => $author, 'summary' => $summary];
58
    }
59
60
    /**
61
     * @param $text
62
     * @return mixed|string
63
     */
64
    public function cleanup($text)
65
    {
66
        global $myts;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

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

1. Pass all data via parameters

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

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

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

    public function myFunction() {
        // Do something
    }
}
Loading history...
67
68
        $clean = stripslashes($text);
69
        $clean =& $myts->displayTarea($clean, 1, 0, 1);
70
        $clean = strip_tags($clean);
71
        $clean = htmlspecialchars($clean, ENT_QUOTES);
72
73
        return $clean;
74
    }
75
76
    /**
77
     * @param  bool $isSummary
78
     * @param  bool $isHtml
79
     * @return bool
80
     */
81
    public function buildContent($isSummary = true, $isHtml = false)
82
    {
83
        $digest_count = count($this->items);
84
        $content      = '';
85
        if ($digest_count > 0) {
86
            $linebreak = $isHtml ? '<br>' : "\n";
87
            for ($i = 0; $i < $digest_count; ++$i) {
88
                if ($isHtml) {
89
                    $content .= ($i + 1) . '. <a href=' . $this->items[$i]['link'] . '>' . $this->items[$i]['title'] . '</a>';
90
                } else {
91
                    $content .= ($i + 1) . '. ' . $this->items[$i]['title'] . $linebreak . $this->items[$i]['link'];
92
                }
93
94
                $content .= $linebreak . $this->items[$i]['author'];
95
                if ($isSummary) {
96
                    $content .= $linebreak . $this->items[$i]['summary'];
97
                }
98
                $content .= $linebreak . $linebreak;
99
            }
100
        }
101
        $this->setVar('digest_content', $content);
102
103
        return true;
104
    }
105
}
106
107
/**
108
 * Class NewbbDigestHandler
109
 */
110
class NewbbDigestHandler extends XoopsObjectHandler
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...
111
{
112
    public $last_digest;
113
114
    /**
115
     * @param  bool $isNew
116
     * @return XoopsObject Digest
117
     */
118
    public function create($isNew = true)
119
    {
120
        $digest = new Digest();
121
        if ($isNew) {
122
            $digest->setNew();
123
        }
124
125
        return $digest;
126
    }
127
128
    /**
129
     * @param  int $id
130
     * @return Digest|null
131
     */
132
    public function get($id)
133
    {
134
        $digest = null;
135
        $id     = (int)$id;
136
        if (!$id) {
137
            return $digest;
138
        }
139
        $sql = 'SELECT * FROM ' . $this->db->prefix('newbb_digest') . ' WHERE digest_id=' . $id;
140 View Code Duplication
        if ($array = $this->db->fetchArray($this->db->query($sql))) {
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...
141
            //if ($var) {
0 ignored issues
show
Unused Code Comprehensibility introduced by
72% 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...
142
            //    return $array[$var];
0 ignored issues
show
Unused Code Comprehensibility introduced by
75% 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...
143
            //}
144
            $digest = $this->create(false);
145
            $digest->assignVars($array);
146
        }
147
148
        return $digest;
149
    }
150
151
    /**
152
     * @param  bool $isForced
153
     * @return int
154
     */
155
    public function process($isForced = false)
156
    {
157
        $this->getLastDigest();
158
        if (!$isForced) {
159
            $status = $this->checkStatus();
160
            if ($status < 1) {
161
                return 1;
162
            }
163
        }
164
        $digest = $this->create();
165
        $status = $this->buildDigest($digest);
166
        if (!$status) {
167
            return 2;
168
        }
169
        $status = $this->insert($digest);
170
        if (!$status) {
171
            return 3;
172
        }
173
        $status = $this->notify($digest);
174
        if (!$status) {
175
            return 4;
176
        }
177
178
        return 0;
179
    }
180
181
    /**
182
     * @param  XoopsObject $digest
183
     * @return bool
184
     */
185
    public function notify(XoopsObject $digest)
186
    {
187
        //$content                = $digest->getVar('digest_content');
0 ignored issues
show
Unused Code Comprehensibility introduced by
64% 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...
188
        /** @var \XoopsNotificationHandler $notificationHandler */
189
        $notificationHandler    = xoops_getHandler('notification');
190
        $tags['DIGEST_ID']      = $digest->getVar('digest_id');
0 ignored issues
show
Coding Style Comprehensibility introduced by
$tags was never initialized. Although not strictly required by PHP, it is generally a good practice to add $tags = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
191
        $tags['DIGEST_CONTENT'] = $digest->getVar('digest_content', 'E');
192
        $notificationHandler->triggerEvent('global', 0, 'digest', $tags);
193
194
        return true;
195
    }
196
197
    /**
198
     * @param        $start
199
     * @param  int   $perpage
200
     * @return array
201
     */
202
    public function getAllDigests($start = 0, $perpage = 5)
203
    {
204
        //        if (empty($start)) {
0 ignored issues
show
Unused Code Comprehensibility introduced by
73% 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...
205
        //            $start = 0;
0 ignored issues
show
Unused Code Comprehensibility introduced by
43% 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...
206
        //        }
207
208
        $sql    = 'SELECT * FROM ' . $this->db->prefix('newbb_digest') . ' ORDER BY digest_id DESC';
209
        $result = $this->db->query($sql, $perpage, $start);
210
        $ret    = [];
211
        //        $reportHandler = xoops_getModuleHandler('report', 'newbb');
0 ignored issues
show
Unused Code Comprehensibility introduced by
54% 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...
212
        while ($myrow = $this->db->fetchArray($result)) {
213
            $ret[] = $myrow; // return as array
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...
214
        }
215
216
        return $ret;
217
    }
218
219
    /**
220
     * @return int
221
     */
222
    public function getDigestCount()
223
    {
224
        $sql    = 'SELECT COUNT(*) AS count FROM ' . $this->db->prefix('newbb_digest');
225
        $result = $this->db->query($sql);
226
        if (!$result) {
227
            return 0;
228
        } else {
229
            $array = $this->db->fetchArray($result);
230
231
            return $array['count'];
232
        }
233
    }
234
235
    public function getLastDigest()
236
    {
237
        $sql    = 'SELECT MAX(digest_time) AS last_digest FROM ' . $this->db->prefix('newbb_digest');
238
        $result = $this->db->query($sql);
239
        if (!$result) {
240
            $this->last_digest = 0;
241
            // echo "<br>no data:".$query;
0 ignored issues
show
Unused Code Comprehensibility introduced by
58% 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...
242
        } else {
243
            $array             = $this->db->fetchArray($result);
244
            $this->last_digest = isset($array['last_digest']) ? $array['last_digest'] : 0;
245
        }
246
    }
247
248
    /**
249
     * @return int
0 ignored issues
show
Documentation introduced by
Should the return type not be integer|double?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
250
     */
251
    public function checkStatus()
0 ignored issues
show
Coding Style introduced by
checkStatus uses the super-global variable $GLOBALS 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...
252
    {
253
        if (!isset($this->last_digest)) {
254
            $this->getLastDigest();
255
        }
256
        $deadline  = ($GLOBALS['xoopsModuleConfig']['email_digest'] == 1) ? 60 * 60 * 24 : 60 * 60 * 24 * 7;
257
        $time_diff = time() - $this->last_digest;
258
259
        return $time_diff - $deadline;
260
    }
261
262
    /**
263
     * @param  XoopsObject $digest
264
     * @return bool
265
     */
266
    public function insert(XoopsObject $digest)
267
    {
268
        $content = $digest->getVar('digest_content', 'E');
269
270
        $id  = $this->db->genId($digest->table . '_digest_id_seq');
271
        $sql = 'INSERT INTO ' . $digest->table . ' (digest_id, digest_time, digest_content)    VALUES (' . $id . ', ' . time() . ', ' . $this->db->quoteString($content) . ' )';
272
273
        if (!$this->db->queryF($sql)) {
274
            //echo "<br>digest insert error::" . $sql;
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...
275
            return false;
276
        }
277
        if (empty($id)) {
278
            $id = $this->db->getInsertId();
279
        }
280
        $digest->setVar('digest_id', $id);
281
282
        return true;
283
    }
284
285
    /**
286
     * @param  XoopsObject $digest
287
     * @return bool
288
     */
289
    public function delete(XoopsObject $digest)
290
    {
291
        $digest_id = $digest;
292
        if (is_object($digest)) {
293
            $digest_id = $digest->getVar('digest_id');
294
        }
295
        if (!isset($this->last_digest)) {
296
            $this->getLastDigest();
297
        }
298
        if ($this->last_digest == $digest_id) {
299
            return false;
300
        } // It is not allowed to delete the last digest
301
        $sql = 'DELETE FROM ' . $this->db->prefix('newbb_digest') . ' WHERE digest_id=' . $digest_id;
302
        if (!$result = $this->db->queryF($sql)) {
303
            return false;
304
        }
305
306
        return true;
307
    }
308
309
    /**
310
     * @param  XoopsObject|\Digest $digest
311
     * @return bool
312
     */
313
    public function buildDigest(XoopsObject $digest)
0 ignored issues
show
Coding Style introduced by
buildDigest uses the super-global variable $GLOBALS 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...
314
    {
315
        global $xoopsModule;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

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

1. Pass all data via parameters

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

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

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

    public function myFunction() {
        // Do something
    }
}
Loading history...
316
317
        if (!defined('SUMMARY_LENGTH')) {
318
            define('SUMMARY_LENGTH', 100);
319
        }
320
321
        /** @var \NewbbForumHandler $forumHandler */
322
        $forumHandler         = xoops_getModuleHandler('forum', 'newbb');
323
        $thisUser             = $GLOBALS['xoopsUser'];
324
        $GLOBALS['xoopsUser'] = null; // To get posts accessible by anonymous
325
        $GLOBALS['xoopsUser'] = $thisUser;
326
327
        $accessForums    = $forumHandler->getIdsByPermission(); // get all accessible forums
328
        $forumCriteria   = ' AND t.forum_id IN (' . implode(',', $accessForums) . ')';
329
        $approveCriteria = ' AND t.approved = 1 AND p.approved = 1';
330
        $time_criteria   = ' AND t.digest_time > ' . $this->last_digest;
331
332
        $karma_criteria = $GLOBALS['xoopsModuleConfig']['enable_karma'] ? ' AND p.post_karma=0' : '';
333
        $reply_criteria = $GLOBALS['xoopsModuleConfig']['allow_require_reply'] ? ' AND p.require_reply=0' : '';
334
335
        $query = 'SELECT t.topic_id, t.forum_id, t.topic_title, t.topic_time, t.digest_time, p.uid, p.poster_name, pt.post_text FROM '
336
                 . $this->db->prefix('newbb_topics')
337
                 . ' t, '
338
                 . $this->db->prefix('newbb_posts_text')
339
                 . ' pt, '
340
                 . $this->db->prefix('newbb_posts')
341
                 . ' p WHERE t.topic_digest = 1 AND p.topic_id=t.topic_id AND p.pid=0 '
342
                 . $forumCriteria
343
                 . $approveCriteria
344
                 . $time_criteria
345
                 . $karma_criteria
346
                 . $reply_criteria
347
                 . ' AND pt.post_id=p.post_id ORDER BY t.digest_time DESC';
348
        if (!$result = $this->db->query($query)) {
349
            //echo "<br>No result:<br>$query";
350
            return false;
351
        }
352
        $rows  = [];
353
        $users = [];
354
        while ($row = $this->db->fetchArray($result)) {
355
            $users[$row['uid']] = 1;
356
            $rows[]             = $row;
357
        }
358
        if (count($rows) < 1) {
359
            return false;
360
        }
361
        $uids = array_keys($users);
362
        if (count($uids) > 0) {
363
            /** @var \XoopsMemberHandler $memberHandler */
364
            $memberHandler = xoops_getHandler('member');
365
            $user_criteria = new Criteria('uid', '(' . implode(',', $uids) . ')', 'IN');
366
            $users         = $memberHandler->getUsers($user_criteria, true);
367
        } else {
368
            $users = [];
369
        }
370
371
        foreach ($rows as $topic) {
372
            if ($topic['uid'] > 0) {
373
                if (isset($users[$topic['uid']]) && is_object($users[$topic['uid']])
374
                    && $users[$topic['uid']]->isActive()) {
375
                    $topic['uname'] = $users[$topic['uid']]->getVar('uname');
376
                } else {
377
                    $topic['uname'] = $GLOBALS['xoopsConfig']['anonymous'];
378
                }
379
            } else {
380
                $topic['uname'] = $topic['poster_name'] ?: $GLOBALS['xoopsConfig']['anonymous'];
381
            }
382
            $summary = xoops_substr(newbb_html2text($topic['post_text']), 0, SUMMARY_LENGTH);
383
            $author  = $topic['uname'] . ' (' . formatTimestamp($topic['topic_time']) . ')';
384
            $link    = XOOPS_URL . '/modules/' . $xoopsModule->dirname() . '/viewtopic.php?topic_id=' . $topic['topic_id'] . '&amp;forum=' . $topic['forum_id'];
385
            $title   = $topic['topic_title'];
386
            $digest->addItem($title, $link, $author, $summary);
387
        }
388
        $digest->buildContent();
389
390
        return true;
391
    }
392
}
393