Skip to content

Commit

Permalink
formatting
Browse files Browse the repository at this point in the history
  • Loading branch information
@taylorotwell
taylorotwell committed Mar 6, 2018
1 parent 5fb5258 commit 3657d66
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 33 deletions.
20 changes: 5 additions & 15 deletions src/Illuminate/Validation/Validator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,11 @@ public function validate()
throw new ValidationException($this);
}

return $this->getDataForRules();
$data = collect($this->getData());

return $data->only(collect($this->getRules())->keys()->map(function ($rule) {
return Str::contains($rule, '.') ? explode('.', $rule)[0] : $rule;
}))->unique()->toArray();
}

/**
Expand Down Expand Up @@ -727,20 +731,6 @@ public function getData()
return $this->data;
}

/**
* Get the data under validation only for the loaded rules.
*
* @return array
*/
public function getDataForRules()
{
$ruleKeys = collect($this->getRules())->keys()->map(function ($rule) {
return explode('.', $rule, 2)[0];
})->unique()->toArray();

return collect($this->getData())->only($ruleKeys)->toArray();
}

/**
* Set the data under validation.
*
Expand Down
22 changes: 4 additions & 18 deletions tests/Validation/ValidationValidatorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3874,28 +3874,14 @@ public function message()
$this->assertTrue($rule->called);
}

public function testGetDataForRules()
public function testValidateReturnsValidatedData()
{
$post = ['first'=>'john', 'last'=>'doe', 'type' => 'admin'];
$post = ['first' => 'john', 'last' => 'doe', 'type' => 'admin'];

$v = new Validator($this->getIlluminateArrayTranslator(), $post, ['first' => 'required']);
$data = $v->getDataForRules();
$data = $v->validate();

$this->assertSame($data, ['first'=>'john']);

$v->sometimes('last', 'required', function () {
return true;
});
$data = $v->getDataForRules();

$this->assertSame($data, ['first'=>'john', 'last'=>'doe']);

$v->sometimes('type', 'required', function () {
return false;
});
$data = $v->getDataForRules();

$this->assertSame($data, ['first'=>'john', 'last'=>'doe']);
$this->assertEquals(['first' => 'john'], $data);
}

protected function getTranslator()
Expand Down

4 comments on commit 3657d66

@adam1010
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@taylorotwell Your refactor created a regression. You didn't get the parenthesis right when you tried to one-line that return statement.

You need to call unique() on the keys, not on the data. The correct code is below. Can we get a new release out for or do I have to hotfix it on my production server?

return $data->only(collect($this->getRules())->keys()->map(function ($rule) {
     return Str::contains($rule, '.') ? explode('.', $rule)[0] : $rule
})->unique())->toArray();

@brunogaspar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adam1010 If it's not yet fixed send a pull request with the fix, if one was not created already.

Also, if it's a regression as you say, it's probably better to include a unit test to ensure it doesn't happen again in the future.

@adam1010
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's a big regression, I had 3 integration tests fail which is how I caught it.

@adam1010
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a pull request that includes a unit test that highlights the problem:

#23432

Please sign in to comment.