MessageController   F
last analyzed

Complexity

Total Complexity 58

Size/Duplication

Total Lines 436
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 31

Test Coverage

Coverage 47.37%

Importance

Changes 0
Metric Value
wmc 58
lcom 1
cbo 31
dl 0
loc 436
ccs 99
cts 209
cp 0.4737
rs 1.3043
c 0
b 0
f 0

17 Methods

Rating   Name   Duplication   Size   Complexity  
A setup() 0 4 1
C composeAction() 0 45 7
A listAction() 0 4 1
A showRenameForm() 0 19 2
A showMessageForm() 0 20 4
A assertCanParticipate() 0 7 2
A assertCanEdit() 0 6 2
A prepareTwig() 0 20 1
C showAction() 0 63 9
A searchAction() 0 16 3
B showInviteForm() 0 29 5
A sendMessage() 0 21 2
A leaveAction() 0 18 3
A teamLeaveAction() 0 20 3
B teamRefreshAction() 0 27 4
B kickAction() 0 30 5
A getErrorMessage() 0 14 4

How to fix   Complexity   

Complex Class

Complex classes like MessageController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

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

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

1
<?php
2
3
use BZIon\Event\ConversationAbandonEvent;
4
use BZIon\Event\ConversationJoinEvent;
5
use BZIon\Event\ConversationKickEvent;
6
use BZIon\Event\ConversationRenameEvent;
7
use BZIon\Event\Events;
8
use BZIon\Event\NewMessageEvent;
9
use BZIon\Form\Creator\ConversationFormCreator;
10
use BZIon\Form\Creator\ConversationInviteFormCreator;
11
use BZIon\Form\Creator\ConversationRenameFormCreator;
12
use BZIon\Form\Creator\MessageFormCreator;
13
use BZIon\Form\Creator\MessageSearchFormCreator;
14
use BZIon\Search\MessageSearch;
15
use Symfony\Component\Form\Form;
16
use Symfony\Component\HttpFoundation\JsonResponse;
17
use Symfony\Component\HttpFoundation\RedirectResponse;
18
use Symfony\Component\HttpFoundation\Request;
19
20
class MessageController extends JSONController
21
{
22 1
    public function setup()
23
    {
24 1
        $this->requireLogin();
25 1
    }
26
27 1
    protected function prepareTwig()
28
    {
29 1
        $currentPage = $this->getCurrentPage();
30
31 1
        $query = $this->getQueryBuilder('Conversation')
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class QueryBuilder as the method forPlayer() does only exist in the following sub-classes of QueryBuilder: ConversationQueryBuilder, MessageQueryBuilder. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
32 1
            ->forPlayer($this->getMe())
33 1
            ->sortBy('last_activity')->reverse()
34 1
            ->limit(5)->fromPage($currentPage);
35
36 1
        $creator = new MessageSearchFormCreator();
37 1
        $searchForm = $creator->create();
38
39 1
        $twig = $this->container->get('twig');
40 1
        $twig->addGlobal("conversations", $query->getModels());
41 1
        $twig->addGlobal("currentPage", $currentPage);
42 1
        $twig->addGlobal("totalPages", $query->countPages());
43 1
        $twig->addGlobal("searchForm", $searchForm->createView());
44
45 1
        return $twig;
46
    }
47
48
    public function listAction()
49
    {
50
        return array();
51
    }
52
53 1
    public function composeAction(Player $me, Request $request)
54
    {
55 1
        if (!$me->hasPermission(Permission::SEND_PRIVATE_MSG)) {
56 1
            throw new ForbiddenException("You are not allowed to send messages");
57
        }
58
59 1
        $creator = new ConversationFormCreator($me);
60 1
        $form = $creator->create()->handleRequest($request);
61
62 1
        if ($form->isSubmitted()) {
63 1
            if ($form->isValid()) {
64 1
                $subject = $form->get('Subject')->getData();
65 1
                $content = $form->get('Message')->getData();
66 1
                $recipients = $form->get('Recipients')->getData();
67
68 1
                $conversation_to = Conversation::createConversation($subject, $me->getId(), $recipients);
69 1
                $message = $conversation_to->sendMessage($me, $content);
70
71 1
                $event = new NewMessageEvent($message, true);
72 1
                $this->dispatch(Events::MESSAGE_NEW, $event);
73
74 1
                if ($this->isJson()) {
75
                    return new JsonResponse(array(
76
                        'success' => true,
77
                        'message' => 'Your message was sent successfully',
78
                        'id'      => $conversation_to->getId()
79
                    ));
80
                } else {
81 1
                    return new RedirectResponse($conversation_to->getUrl());
82
                }
83 1
            } elseif ($this->isJson()) {
84 1
                throw new BadRequestException($this->getErrorMessage($form));
85
            }
86
        } else {
87
            // Load the list of recipients from the URL
88 1
            if ($request->query->has('recipients')) {
89
                $form->get('Recipients')->setData($this->decompose(
90
                    $request->query->get('recipients'),
91
                    array('Player', 'Team')
92
                ));
93
            }
94
        }
95
96 1
        return array("form" => $form->createView());
97
    }
98
99 1
    public function showAction(Conversation $conversation, Player $me, Request $request)
100
    {
101 1
        $this->assertCanParticipate($me, $conversation);
102 1
        $conversation->markReadBy($me->getId());
103
104 1
        $form = $this->showMessageForm($conversation, $me);
105 1
        $inviteForm = $this->showInviteForm($conversation, $me);
106 1
        $renameForm = $this->showRenameForm($conversation, $me);
107
108 1
        $messages = $this->getQueryBuilder('AbstractMessage')
109 1
                  ->where('conversation')->is($conversation)
110 1
                  ->sortBy('time')->reverse()
111 1
                  ->limit(10)->fromPage($this->getCurrentPage())
112 1
                  ->startAt($request->query->get('end'))
113 1
                  ->endAt($request->query->get('start'))
114 1
                  ->getModels();
115
116
        // Hide the details (author, timestamp) of the first message if they're
117
        // already shown in the previous message (useful for AJAX calls)
118 1
        if ($request->query->getBoolean('hideFirstDetails')) {
119
            $previousMessage = Message::get($request->query->get('end'));
120
        } else {
121 1
            $previousMessage = null;
122
        }
123
124
        $params = array(
125 1
            "form"            => $form->createView(),
126 1
            "inviteForm"      => $inviteForm->createView(),
0 ignored issues
show
Bug introduced by
The method createView does only exist in Symfony\Component\Form\FormInterface, but not in MessageController.

It seems like the method you are trying to call exists only in some of the possible types.

Let’s take a look at an example:

class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

Available Fixes

  1. Add an additional type-check:

    /**
     * @param A|B $x
     */
    function someFunction($x)
    {
        $x->foo();
    
        if ($x instanceof B) {
            $x->bar();
        }
    }
    
  2. Only allow a single type to be passed if the variable comes from a parameter:

    function someFunction(B $x) { /** ... */ }
    
Loading history...
127 1
            "renameForm"      => $renameForm->createView(),
128 1
            "conversation"    => $conversation,
129 1
            "messages"        => $messages,
130 1
            "previousMessage" => $previousMessage
131
        );
132
133 1
        if ($request->query->has('nolayout')) {
134
            if ($request->query->getBoolean('reviewLastDetails')) {
135
                // An AJAX call has asked us to check if details (author,
136
                // timestamp) will need to be shown for the next message
137
138
                $nextMessage = Message::get($request->query->get('start'));
139
                $lastMessage = reset($messages);
140
141
                if ($lastMessage !== false
142
                    && $lastMessage->isMessage()
143
                    && $nextMessage->isMessage()
144
                    && $lastMessage->getTimestamp()->isSameDay($nextMessage->getTimestamp())
145
                    && $lastMessage->getAuthor()->isSameAs($nextMessage->getAuthor())
146
                ) {
147
                    $hideLastDetails = true;
148
                } else {
149
                    $hideLastDetails = false;
150
                }
151
152
                // Add $hideLastDetails to the list of JSON parameters
153
                $this->attributes->set('hideLastDetails', $hideLastDetails);
154
            }
155
156
            // Don't show the layout so that ajax can just load the messages
157
            return $this->render('Message/messages.html.twig', $params);
158
        } else {
159 1
            return $params;
160
        }
161
    }
162
163
    public function leaveAction(Player $me, Conversation $conversation)
164
    {
165
        if (!$conversation->isMember($me, $distinct = true)) {
166
            throw new ForbiddenException("You are not a member of this discussion.");
167
        } elseif ($conversation->getCreator()->isSameAs($me)) {
168
            throw new ForbiddenException("You can't abandon the conversation you started!");
169
        }
170
171
        return $this->showConfirmationForm(function () use ($conversation, $me) {
172
            $conversation->removeMember($me);
173
174
            $event = new ConversationAbandonEvent($conversation, $me);
175
            Service::getDispatcher()->dispatch(Events::CONVERSATION_ABANDON, $event);
176
177
            return new RedirectResponse(Service::getGenerator()->generate('message_list'));
178
        },  "Are you sure you want to abandon this conversation?",
179
            "You will no longer receive messages from this conversation", "Leave");
180
    }
181
182
    public function teamRefreshAction(Player $me, Conversation $conversation)
183
    {
184
        $team = $me->getTeam();
185
        if (!$team->isValid()) {
186
            throw new ForbiddenException("You are not a member of a team.");
187
        }
188
189
        $missing = $conversation->getMissingTeamMembers($team);
190
        if (empty($missing)) {
191
            throw new ForbiddenException("All members of the specified team are already part of that conversation.");
192
        }
193
194
        $names = array();
195
        foreach ($missing as $player) {
196
            $names[] = $player->getEscapedUsername();
0 ignored issues
show
Bug introduced by
The method getEscapedUsername() does not exist on Model. Did you maybe mean escape()?

This check marks calls to methods that do not seem to exist on an object.

This is most likely the result of a method being renamed without all references to it being renamed likewise.

Loading history...
197
        }
198
        $names = implode(', ', $names);
199
200
        return $this->showConfirmationForm(function () use ($conversation, $missing, $team) {
201
            $conversation->addMember($team); // does all the hard work
202
203
            $event = new ConversationJoinEvent($conversation, $missing);
204
            Service::getDispatcher()->dispatch(Events::CONVERSATION_JOIN, $event);
205
206
            return new RedirectResponse($conversation->getURL());
207
        }, "Are you sure you want to invite $names from {$team->getEscapedName()} to that conversation?");
208
    }
209
210
    public function teamLeaveAction(Player $me, Conversation $conversation)
211
    {
212
        $team = $me->getTeam();
213
214
        if (!$me->canEdit($team)) {
215
            throw new ForbiddenException("You are not allowed to remove your team from this conversation.");
216
        } elseif (!$conversation->isMember($team)) {
217
            throw new ForbiddenException("That team is not participating in this conversation.");
218
        }
219
220
        return $this->showConfirmationForm(function () use ($conversation, $team) {
221
            $conversation->removeMember($team);
222
223
            $event = new ConversationAbandonEvent($conversation, $team);
224
            Service::getDispatcher()->dispatch(Events::CONVERSATION_ABANDON, $event);
225
226
            return new RedirectResponse($conversation->getURL());
227
        },  "Are you sure you want to remove {$team->getEscapedName()} from this conversation?",
228
            "Your team is no longer participating in that conversation.", "Remove team");
229
    }
230
231
    public function kickAction(Conversation $conversation, Player $me, $type, $member)
232
    {
233
        $this->assertCanEdit($me, $conversation, "You are not allowed to kick a member off that discussion!");
234
235
        if (strtolower($type) === 'player') {
236
            $member = Player::fetchFromSlug($member);
237
        } elseif (strtolower($type) === 'team') {
238
            $member = Team::fetchFromSlug($member);
239
        } else {
240
            throw new BadRequestException("Unrecognized member type.");
241
        }
242
243
        if ($conversation->isCreator($member->getId())) {
244
            throw new ForbiddenException("You can't leave your own conversation.");
245
        }
246
247
        if (!$conversation->isMember($member)) {
0 ignored issues
show
Documentation introduced by
$member is of type object<AliasModel>, but the function expects a object<Player>|object<Team>.

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...
248
            throw new ForbiddenException("The specified player is not a member of this conversation.");
249
        }
250
251
        return $this->showConfirmationForm(function () use ($conversation, $member, $me) {
252
            $conversation->removeMember($member);
253
254
            $event = new ConversationKickEvent($conversation, $member, $me);
255
            Service::getDispatcher()->dispatch(Events::CONVERSATION_KICK, $event);
256
257
            return new RedirectResponse($conversation->getUrl());
258
        },  "Are you sure you want to kick {$member->getEscapedName()} from the discussion?",
259
            "{$member->getName()} has been kicked from the conversation", "Kick");
260
    }
261
262 1
    public function searchAction(Player $me, Request $request)
263
    {
264 1
        $query = $request->query->get('q');
265
266 1
        if (strlen($query) < 3 && !$this->isDebug()) {
267
            // TODO: Find a better error message
268
            throw new BadRequestException('The search term you have provided is too short');
269
        }
270
271 1
        $search  = new MessageSearch($this->getQueryBuilder(), $me);
0 ignored issues
show
Compatibility introduced by
$this->getQueryBuilder() of type object<QueryBuilder> is not a sub-type of object<MessageQueryBuilder>. It seems like you assume a child class of the class QueryBuilder 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...
272 1
        $results = $search->search($query);
273
274
        return array(
275 1
            'messages' => $results
276
        );
277
    }
278
279
    /**
280
     * @param Conversation  $conversation
281
     * @param Player $me
282
     *
283
     * @return $this|Form|\Symfony\Component\Form\FormInterface
284
     */
285 1
    private function showInviteForm($conversation, $me)
286
    {
287 1
        $creator = new ConversationInviteFormCreator($conversation);
288 1
        $form = $creator->create()->handleRequest($this->getRequest());
289
290 1
        if ($form->isValid()) {
291
            $this->assertCanEdit($me, $conversation);
292
            $invitees = array();
293
294
            foreach ($form->get('players')->getData() as $player) {
295
                if (!$conversation->isMember($player, $distinct = true)) {
296
                    $conversation->addMember($player);
297
                    $invitees[] = $player;
298
                }
299
            }
300
301
            if (!empty($invitees)) {
302
                $event = new ConversationJoinEvent($conversation, $invitees);
303
                Service::getDispatcher()->dispatch(Events::CONVERSATION_JOIN, $event);
304
            }
305
306
            $this->getFlashBag()->add('success', "The conversation has been updated");
307
308
            // Reset the form fields
309
            return $creator->create();
310
        }
311
312 1
        return $form;
313
    }
314
315
    /**
316
     * @param Conversation  $conversation
317
     * @param Player $me
318
     */
319 1
    private function showRenameForm($conversation, $me)
320
    {
321 1
        $creator = new ConversationRenameFormCreator($conversation);
322 1
        $form = $creator->create()->handleRequest($this->getRequest());
323
324 1
        if ($form->isValid()) {
325
            $this->assertCanEdit($me, $conversation);
326
327
            $newName = $form->get('subject')->getData();
328
329
            $event = new ConversationRenameEvent($conversation, $conversation->getSubject(), $newName, $me);
330
            $conversation->setSubject($newName);
331
            Service::getDispatcher()->dispatch(Events::CONVERSATION_RENAME, $event);
332
333
            $this->getFlashBag()->add('success', "The conversation has been updated");
334
        }
335
336 1
        return $form;
337
    }
338
339
    /**
340
     * @param Conversation  $conversation
341
     * @param Player $me
342
     */
343 1
    private function showMessageForm($conversation, $me)
344
    {
345
        // Create the form to send a message to the conversation
346 1
        $creator = new MessageFormCreator($conversation);
347 1
        $form = $creator->create();
348
349
        // Keep a cloned version so we can come back to it later, if we need
350
        // to reset the fields of the form
351 1
        $cloned = clone $form;
352 1
        $form->handleRequest($this->getRequest());
353
354 1
        if ($form->isValid()) {
355
            // The player wants to send a message
356 1
            $this->sendMessage($me, $conversation, $form, $cloned);
357 1
        } elseif ($form->isSubmitted() && $this->isJson()) {
358
            throw new BadRequestException($this->getErrorMessage($form));
359
        }
360
361 1
        return $form;
362
    }
363
364
    /**
365
     * Make sure that a player can participate in a conversation
366
     *
367
     * Throws an exception if a player is not an admin or a member of that conversation
368
     * @todo Permission for spying on other people's conversations?
369
     * @param  Player        $player  The player to test
370
     * @param  Conversation         $conversation   The message conversation
371
     * @param  string        $message The error message to show
372
     * @throws HTTPException
373
     * @return void
374
     */
375 1
    private function assertCanParticipate(Player $player, Conversation $conversation,
376
        $message = "You are not allowed to participate in that discussion"
377
    ) {
378 1
        if (!$conversation->isMember($player)) {
379
            throw new ForbiddenException($message);
380
        }
381 1
    }
382
383
    /**
384
     * Sends a message to a conversation
385
     *
386
     * @param  Player        $from   The sender
387
     * @param  Conversation         $to     The conversation that will receive the message
388
     * @param  Form          $form   The message's form
389
     * @param  Form          $form   The form before it handled the request
390
     * @param  Form          $cloned
391
     * @throws HTTPException Thrown if the user doesn't have the
392
     *                              SEND_PRIVATE_MSG permission
393
     * @return void
394
     */
395 1
    private function sendMessage(Player $from, Conversation $to, &$form, $cloned)
396
    {
397 1
        if (!$from->hasPermission(Permission::SEND_PRIVATE_MSG)) {
398 1
            throw new ForbiddenException("You are not allowed to send messages");
399
        }
400
401 1
        $message = $form->get('message')->getData();
402 1
        $message = $to->sendMessage($from, $message);
403
404 1
        $this->getFlashBag()->add('success', "Your message was sent successfully");
405
406
        // Let javascript know the message's ID
407 1
        $this->attributes->set('id', $message->getId());
408
409
        // Reset the form
410 1
        $form = $cloned;
411
412
        // Notify everyone that we sent a new message
413 1
        $event = new NewMessageEvent($message, false);
414 1
        $this->dispatch(Events::MESSAGE_NEW, $event);
415 1
    }
416
417
    /**
418
     * Get an error message from a form
419
     *
420
     * @param Form $form The form to check
421
     *
422
     * @return string Will return "Unknown error" if an error is not found
423
     */
424
    private function getErrorMessage(Form $form)
425
    {
426
        foreach ($form->all() as $child) {
427
            foreach ($child->getErrors() as $error) {
428
                return $error->getMessage();
429
            }
430
        }
431
432
        foreach ($form->getErrors() as $error) {
433
            return $error->getMessage();
434
        }
435
436
        return "Unknown Error";
437
    }
438
439
    /**
440
     * Make sure that a player can edit a conversation
441
     *
442
     * Throws an exception if a player is not an admin or the leader of a team
443
     * @param  Player        $player  The player to test
444
     * @param  Conversation         $conversation   The team
445
     * @param  string        $message The error message to show
446
     * @throws HTTPException
447
     * @return void
448
     */
449
    private function assertCanEdit(Player $player, Conversation $conversation, $message = "You are not allowed to edit the discussion")
450
    {
451
        if ($conversation->getCreator()->getId() != $player->getId()) {
452
            throw new ForbiddenException($message);
453
        }
454
    }
455
}
456