[Cookbook][Security] Update Voters: add class properties #3211
Conversation
| private $request; | ||
| private $blacklistedIp; | ||
|
|
||
| public function __construct(Request $request, array $blacklistedIp = array()) |
There was a problem hiding this comment.
i wonder if we should start using the request_stack?
There was a problem hiding this comment.
we can do that, but not for 2.2.
|
This can cause "not in scope"-errors. I haven't test this, let's wait what @weaverryan think about this. However, as of Symfony 2.3, we should use the request_stack here, as suggested by @cordoval |
|
Good point, i can update this PR to only add the class properties since they were missing. I was a bit over-enthusiastic with the rest of the changes ;). Thanks for the feedback. |
|
yeah, I think that's the best. And if you are enthousiast, you can create a PR for 2.3 using request_stack ;-) |
|
Otherwise, you should change the service definition as well. :) |
|
FYI, someone just submitted a change for 2.3 (I hope he'll correct his PR): #3216 |
|
Updated PR, only adds class properties now. |
| @@ -67,6 +67,9 @@ and compare the IP address against a set of blacklisted IP addresses: | |||
|
|
|||
| class ClientIpVoter implements VoterInterface | |||
| { | |||
| private $container; | |||
There was a problem hiding this comment.
add a blank line between the properties
There was a problem hiding this comment.
Thought it was part of PSR-2. But it turns out that this seems to be only my personal taste.
There was a problem hiding this comment.
personal taste is personal 👶
|
Fixed the issues reported by @xabbuh, thanks |
[Cookbook][Security] Update Voters: add class properties
|
Hi guys! Nice work on this - I completely agree that we should use the container here and then handle it better in Symfony 2.4 with the request stack (#3220). Thanks! |
Added class properties.