Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added "skipOnError" ability & minor changes #54

Merged
merged 18 commits into from Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 26 additions & 10 deletions src/Rule.php
Expand Up @@ -15,20 +15,26 @@ abstract class Rule
private ?string $translationDomain = null;
private ?string $translationLocale = null;
private bool $skipOnEmpty = false;
private bool $skipOnError = true;

/**
* Validates the value
*
* @param mixed $value value to be validated
* @param DataSetInterface|null $dataSet optional data set that could be used for contextual validation
* @param bool $previousRulesErrored set to true if rule is part of a group of rules and one of the previous validations failed
* @return Result
*/
final public function validate($value, DataSetInterface $dataSet = null): Result
final public function validate($value, DataSetInterface $dataSet = null, bool $previousRulesErrored = false): Result
{
if ($this->skipOnEmpty && $this->isEmpty($value)) {
return new Result();
}

if ($this->skipOnError && $previousRulesErrored) {
return new Result();
}

return $this->validateValue($value, $dataSet);
}

Expand All @@ -41,22 +47,25 @@ final public function validate($value, DataSetInterface $dataSet = null): Result
*/
abstract protected function validateValue($value, DataSetInterface $dataSet = null): Result;

public function setTranslator(TranslatorInterface $translator): self
public function withTranslator(TranslatorInterface $translator): self
{
$this->translator = $translator;
return $this;
$new = clone $this;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really need to create a new copy? I don't think there is a use-case. If your goal is cascading, you should:

public function translator(TranslatorInterface $translator): self 
{
  $this->translator = $translator;
  return $this;
}

$validator->translator(...)->translatorDomain(...)->translationLocale(...)->...

Copy link
Member

Choose a reason for hiding this comment

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

@kamarton cloning is cheap. I don't think there's anything bad in making object immutable if it's not meant to have state.

$new->translator = $translator;
return $new;
}

public function setTranslationDomain(string $translation): self
public function withTranslationDomain(string $translation): self
{
$this->translationDomain = $translation;
return $this;
$new = clone $this;
$new->translationDomain = $translation;
return $new;
}

public function setTranslationLocale(string $locale): self
public function withTranslationLocale(string $locale): self
{
$this->translationLocale = $locale;
return $this;
$new = clone $this;
$new->translationLocale = $locale;
return $new;
}

public function translateMessage(string $message, array $arguments = []): string
Expand All @@ -73,6 +82,13 @@ public function translateMessage(string $message, array $arguments = []): string
);
}

public function skipOnError(bool $value): self
Copy link
Contributor

Choose a reason for hiding this comment

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

If you have a new instance: withSkipOnError(), but I suggest that you do not have a new instance here:

public function skipOnError(bool $value): self
{
        $this->skipOnError = $value;
        return $this;
}

{
$new = clone $this;
$new->skipOnError = $value;
return $new;
}

/**
* @param bool $value if validation should be skipped if value validated is empty
* @return self
Expand Down
14 changes: 9 additions & 5 deletions src/Rules.php
Expand Up @@ -46,15 +46,15 @@ private function normalizeRule($rule): Rule
}

if ($this->translator !== null) {
$rule->setTranslator($this->translator);
$rule->withTranslator($this->translator);
}

if ($this->translationDomain !== null) {
$rule->setTranslationDomain($this->translationDomain);
$rule->withTranslationDomain($this->translationDomain);
}

if ($this->translationLocale !== null) {
$rule->setTranslationLocale($this->translationLocale);
$rule->withTranslationLocale($this->translationLocale);
}

return $rule;
Expand All @@ -65,12 +65,16 @@ public function add(Rule $rule): void
$this->rules[] = $this->normalizeRule($rule);
}

public function validate($value, DataSetInterface $dataSet = null): Result
public function validate($value, DataSetInterface $dataSet = null, bool $previousRulesErrored = false): Result
{
$compoundResult = new Result();
/**
* @var $rule Rule
*/
foreach ($this->rules as $rule) {
$ruleResult = $rule->validate($value, $dataSet);
$ruleResult = $rule->validate($value, $dataSet, $previousRulesErrored);
if ($ruleResult->isValid() === false) {
$previousRulesErrored = true;
foreach ($ruleResult->getErrors() as $message) {
$compoundResult->addError($message);
}
Expand Down
3 changes: 1 addition & 2 deletions src/Validator.php
Expand Up @@ -44,11 +44,10 @@ public function __construct(
public function validate(DataSetInterface $dataSet): ResultSet
{
$results = new ResultSet();

foreach ($this->attributeRules as $attribute => $rules) {
$results->addResult(
$attribute,
$rules->validate($dataSet->getAttributeValue($attribute))
$rules->validate($dataSet->getAttributeValue($attribute), $dataSet)
Copy link
Member

Choose a reason for hiding this comment

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

This is not related to skip on error but is important bugfix.

);
}
return $results;
Expand Down
16 changes: 16 additions & 0 deletions tests/RulesTest.php
Expand Up @@ -55,4 +55,20 @@ static function ($value): Result {
$this->assertFalse($result->isValid());
$this->assertCount(1, $result->getErrors());
}

public function testSkipOnError()
{
$rules = new Rules(
[
(new Number())->min(10),
(new Number())->min(10)->skipOnError(false),
(new Number())->min(10)
]
);

$result = $rules->validate(1);

$this->assertFalse($result->isValid());
$this->assertCount(2, $result->getErrors());
}
}
2 changes: 1 addition & 1 deletion tests/ValidatorTest.php
Expand Up @@ -72,7 +72,7 @@ static function ($value): Result {

$intResult = $results->getResult('int');
$this->assertFalse($intResult->isValid());
$this->assertCount(2, $intResult->getErrors());
$this->assertCount(1, $intResult->getErrors());
}

public function testAddingRulesOneByOne(): void
Expand Down