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

allowEmpty can be populated with the list of what we consider 'empty'. The validation only will stop if the empty validation returns true. #12519

Merged
merged 1 commit into from
Jan 10, 2017

Conversation

gguridi
Copy link
Contributor

@gguridi gguridi commented Dec 30, 2016

Hello!

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Small description of change:

Added the possibility to specify in allowEmpty option what we consider 'empty'. Fixed the behaviour so only in case it returns true we move to the next validation. Added recursivity in the preChecking method so we don't duplicate code. Updated changelog.

  • allowEmtpy = true (this means that everything that evaluates to empty is ok)
  • allowEmpty = ['', null] (this means that empty only means empty string and null, false and 0 won't evaluate as empty).

I've added a pull request with this change. The benefits are:

  • Backwards compatible.
  • Flexible, we can specify 'unknown', 'empty' or -1 as empty values in case we are using a shitty api.

An improvement also added in the pull request is that only if the allowEmpty validation is true the validation will stop there, in case of false it will continue with the next validators.

Thanks


This change is Reviewable

…r 'empty'. Fixed the behaviour so only in case it returns true we move to the next validation. Added recursivity in the preChecking method so we don't duplicate code. Updated changelog.
@sergeyklay sergeyklay added this to the 3.1.x milestone Jan 3, 2017
@sergeyklay sergeyklay merged commit 27eb58f into phalcon:3.1.x Jan 10, 2017
@sergeyklay
Copy link
Contributor

Thanks

@sergeyklay sergeyklay modified the milestones: 3.2.x, 3.2.0 Apr 8, 2017
@someson
Copy link

someson commented Oct 22, 2018

what if

    // ...
    allowEmpty => ['', null, new RawValue('NULL')],

?

new RawValue('NULL') === new RawValue('NULL') → false

I have to prefill the entity already on form-level (save/update), not model-level.
Extended form handler:

    private function _nullableValue(Model $entry, string $fieldName, $value)
    {
        $metaData = $entry->getModelsMetaData();
        $attributes = $metaData->getNotNullAttributes($entry);
        return \in_array($fieldName, $attributes, true) ? $value : new RawValue('NULL');
    }

Model:

    public function validation(): bool
    {
        // ...
        $validation->add('vat', new Numericality(['allowEmpty' => [new RawValue('NULL')]]));

@sergeyklay
Copy link
Contributor

@someson could you please create a PR with test which fails?

@someson
Copy link

someson commented Oct 22, 2018

@sergeyklay, yeah, I'll try :)

someson added a commit to someson/cphalcon that referenced this pull request Oct 22, 2018
tests updated for phalcon#12519
see phalcon#12519#issuecomment-431848960
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants