Fixed bug where it was not possible to specify the connection to use #66
Conversation
| @@ -34,7 +34,7 @@ public function validateUniqueWith($attribute, $value, $parameters) | |||
| $parameters = array_map('trim', $parameters); | |||
|
|
|||
| // first item equals table name | |||
| $table = array_shift($parameters); | |||
| list($connection, $table) = $this->parseTable(array_shift($parameters)); | |||
There was a problem hiding this comment.
There is no parseTable function defined.
There was a problem hiding this comment.
parseTable() is available from L5.1 and above. I fixed this.
There was a problem hiding this comment.
Hello @felixkiss, are you requesting anything else about this PR ?
There was a problem hiding this comment.
Sorry for the delay. I will try to find time to review this in the near future. You can use your fork in the meantime. Cheers.
There was a problem hiding this comment.
Can you add a test for the new behavior? (Asserting that setConnection is called with the correct parameter)
| @@ -34,7 +34,7 @@ public function validateUniqueWith($attribute, $value, $parameters) | |||
| $parameters = array_map('trim', $parameters); | |||
|
|
|||
| // first item equals table name | |||
| $table = array_shift($parameters); | |||
| list($connection, $table) = $this->parseTable(array_shift($parameters)); | |||
There was a problem hiding this comment.
Can you add a test for the new behavior? (Asserting that setConnection is called with the correct parameter)
| @@ -19,12 +19,15 @@ public function setUp() | |||
| ->andReturn($this->testDefaultErrorMessage); | |||
| $this->translator->shouldReceive('trans') | |||
| ->andReturnUsing(function($arg) { return $arg; }); | |||
| $this->translator->shouldReceive('get') | |||
There was a problem hiding this comment.
We already mock translator->get() in line 17. Why is it necessary again?
|
@LittleBigDev I did a long overdue rewrite of the package, but I will look into rebasing this on master (and adding the new tests we need) in the next few days. |
|
Oh ok then. I had no time for this anyway. Thx a lot. |
All the changes have been borrowed from the following PR made by @LittleBigDev felixkiss#66
|
Is this fixed? |
|
Yes, here : jankivi@a12d4e1 |
|
still relevant for me... |
|
3 months passed, is there anything i could do to see this PR merged? |
See issue #62
I added possibility to specify the connection (same as in core \Illuminate\Validation\Validator::validateUnique() method)