Completed
Push — feature/player-list ( 62d21d...ee00f1 )
by Vladimir
06:57
created

PlayerController::handleSorting()   B

Complexity

Conditions 5
Paths 3

Size

Total Lines 20
Code Lines 11

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 0
CRAP Score 30

Importance

Changes 0
Metric Value
dl 0
loc 20
ccs 0
cts 13
cp 0
rs 8.8571
c 0
b 0
f 0
cc 5
eloc 11
nc 3
nop 3
crap 30
1
<?php
2
3
use BZIon\Form\Creator\PlayerAdminNotesFormCreator as FormCreator;
4
use Symfony\Component\Form\Form;
5
use Symfony\Component\HttpFoundation\Request;
6
7
class PlayerController extends JSONController
8
{
9
    private $creator;
10
11
    public function showAction(Player $player, Player $me, Request $request)
12
    {
13
        if ($me->hasPermission(Permission::VIEW_VISITOR_LOG)) {
14
            $this->creator = new FormCreator($player);
15
            $form = $this->creator->create()->handleRequest($request);
16
17
            if ($form->isValid()) {
18
                $form = $this->handleAdminNotesForm($form, $player, $me);
0 ignored issues
show
Compatibility introduced by
$form of type object<Symfony\Component\Form\FormInterface> is not a sub-type of object<Symfony\Component\Form\Form>. It seems like you assume a concrete implementation of the interface Symfony\Component\Form\FormInterface 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...
19
            }
20
21
            $formView = $form->createView();
22
        } else {
23
            // Don't spend time rendering the form unless we need it
24
            $formView = null;
25
        }
26
27
        return array(
28
            "player"         => $player,
29
            "adminNotesForm" => $formView,
30
        );
31
    }
32
33
    public function editAction(Player $player, Player $me)
34
    {
35
        if (!$me->canEdit($player)) {
36
            throw new ForbiddenException("You are not allowed to edit other players");
37
        }
38
39
        $params = array(
40
            'me'   => $player,
41
            'self' => false,
42
        );
43
44
        return $this->forward('edit', $params, 'Profile');
45
    }
46
47
    public function listAction(Request $request, Player $me, Team $team = null)
48
    {
49
        $query = Player::getQueryBuilder();
50
51
        // Load all countries into the cache so they are ready for later
52
        Country::getQueryBuilder()->addToCache();
53
54
        if ($team) {
55
            $query->where('team')->is($team);
56
        } else {
57
            // Add all teams to the cache
58
            $this->getQueryBuilder('Team')
59
                ->where('members')->greaterThan(0)
60
                ->addToCache();
61
        }
62
63
        if ($request->query->has('exceptMe')) {
64
            $query->except($me);
65
        }
66
67
        $players = $query
68
            ->withMatchActivity()
69
            ->getModels($fast = true)
70
        ;
71
72
        $groupBy = $request->query->get('groupBy');
73
        $sortBy = $request->query->get('sortBy');
74
        $sortOrder = $request->query->get('sortOrder');
75
76
        if ($groupBy) {
77
            $grouped = [];
78
79
            /** @var Player $player */
80
            foreach ($players as $player) {
81
                $key = '';
82
83
                if ($groupBy == 'country') {
84
                    $key = $player->getCountry()->getName();
85
                } elseif ($groupBy == 'team') {
86
                    $key = $player->getTeam()->getEscapedName();
87
88
                    if ($key == '<em>None</em>') {
89
                        $key = ' ';
90
                    }
91
                } elseif ($groupBy == 'activity') {
92
                    $key = ($player->getMatchActivity() > 0.0) ? 'Active' : 'Inactive';
93
                }
94
95
                $grouped[$key][] = $player;
96
            }
97
98
            ksort($grouped);
99
            $players = $grouped;
100
        }
101
102
        if ($sortBy || $sortOrder) {
103
            $sortBy = $sortBy ? $sortBy : 'callsign';
104
            $sortOrder = $sortOrder ? $sortOrder : 'ASC';
105
106
            if ($groupBy === null) {
107
                $this->handleSorting($players, $sortBy, $sortOrder);
108
            } else {
109
                foreach ($players as &$playerList) {
110
                    $this->handleSorting($playerList, $sortBy, $sortOrder);
111
                }
112
            }
113
        }
114
115
        return array(
116
            'grouped' => ($groupBy !== null),
117
            'players' => $players,
118
        );
119
    }
120
121
    /**
122
     * Handle the admin notes form
123
     * @param  Form   $form   The form
124
     * @param  Player $player The player in question
125
     * @param  Player $me     The currently logged in player
126
     * @return Form   The updated form
127
     */
128
    private function handleAdminNotesForm($form, $player, $me)
129
    {
130
        $notes = $form->get('notes')->getData();
131
        if ($form->get('save_and_sign')->isClicked()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Form\FormInterface as the method isClicked() does only exist in the following implementations of said interface: Symfony\Component\Form\SubmitButton.

Let’s take a look at an example:

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

class MyUser implements 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 implementation 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 interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
132
            $notes .= ' — ' . $me->getUsername() . ' on ' . TimeDate::now()->toRFC2822String();
133
        }
134
135
        $player->setAdminNotes($notes);
136
        $this->getFlashBag()->add('success', "The admin notes for {$player->getUsername()} have been updated");
137
138
        // Reset the form so that the user sees the updated admin notes
139
        return $this->creator->create();
140
    }
141
142
    private function handleSorting(&$playerList, $sortBy, $sortOrder)
143
    {
144
        if ($sortBy == 'callsign') {
145
            usort($playerList, function($a, $b) use ($sortOrder) {
146
                if ($sortOrder == 'DESC') {
147
                    return strcasecmp($b->getUsername(), $a->getUsername());
148
                }
149
150
                return strcasecmp($a->getUsername(), $b->getUsername());
151
            });
152
        } elseif ($sortBy == 'activity') {
153
            usort($playerList, function($a, $b) use ($sortOrder) {
154
                if ($sortOrder == 'DESC') {
155
                    return ($b->getMatchActivity() > $a->getMatchActivity());
156
                }
157
158
                return ($a->getMatchActivity() > $b->getMatchActivity());
159
            });
160
        }
161
    }
162
}
163