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

Custom Validation Rules not processed if no value in field #11151

Open
jonathan-bird opened this issue Nov 21, 2024 · 14 comments
Open

Custom Validation Rules not processed if no value in field #11151

jonathan-bird opened this issue Nov 21, 2024 · 14 comments

Comments

@jonathan-bird
Copy link

jonathan-bird commented Nov 21, 2024

Bug description

If you create a new custom validation rule and set it in a blueprint. Eg.

validate:
  - 'new App\Rules\TemplateDefaultOrNotSet()'

Then it won't run the validate() method unless there's a value set in the field.

It should be run regardless of whether there is a value.

How to reproduce

Create a new custom rule:

<?php

namespace App\Rules;

use Closure;
use Illuminate\Contracts\Validation\DataAwareRule;
use Illuminate\Contracts\Validation\ValidationRule;

class TemplateDefaultOrNotSet implements ValidationRule, DataAwareRule
{
    public function validate(string $attribute, mixed $value, Closure $fail): void
    {
        $fail('It will not hit this.');
     }
}

Then go to blueprint yaml file and set this (or just set new App\Rules\TemplateDefaultOrNotSet() in the Blueprint view):

validate:
  - 'new App\Rules\TemplateDefaultOrNotSet()'

And then once saved, create a new page and don't set any value in that field, and you will notice that it won't hit that validate() method (although it will hit a __construct() if set in the custom rule)

Logs

No response

Environment

Environment
Application Name: xxxxx
Laravel Version: 11.33.2
PHP Version: 8.3.13
Composer Version: 2.7.7
Environment: local
Debug Mode: ENABLED
URL: xxxx-statamic.test
Maintenance Mode: OFF
Timezone: Australia/Brisbane
Locale: en

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: NOT CACHED

Drivers
Broadcasting: log
Cache: redis
Database: mysql
Logs: stack / daily
Mail: smtp
Queue: sync
Session: redis

Livewire
Livewire: v3.5.12

Statamic
Addons: 7
Sites: 1
Stache Watcher: Enabled (auto)
Static Caching: Disabled
Version: 5.38.1 PRO

Statamic Addons
aerni/advanced-seo: 2.9.3
aerni/livewire-forms: 9.4.1
digital-bird/statamic-toc: 1.0.2
jonassiewertsen/statamic-livewire: 3.8.1
mitydigital/sitemapamic: 3.2.0
mitydigital/statamic-two-factor: 2.3.0

Installation

Fresh statamic/statamic site via CLI

Additional details

No response

@jasonvarga
Copy link
Member

jasonvarga commented Nov 21, 2024

It should be run regardless of whether there is a value.

I don't think that's accurate. The field is only validated if it's included, or if you mark it as required.

Validator::make([
  // foo missing
], [
  'foo' => 'min:5',
])->validate();  // passes
Validator::make([
  'foo' => 'h',
], [
  'foo' => 'min:5',
])->validate();  // fails: the foo field must be at least 5 characters
Validator::make([
  'foo' => 'hello',
], [
  'foo' => 'min:5',
])->validate();  // passes
Validator::make([
  // foo missing
], [
  'foo' => 'required|min:5',
])->validate();  // fails: the foo field is required

@jonathan-bird
Copy link
Author

@jasonvarga this is kind of the point though, based on that logic you can't write your own required-type validation because the class can't even run a validate method. Why can't it pass through the field as null like what Laravel would do in a form?

Or do I have to set "always save" for that logic to apply?

Because of the fact the data isn't always saved, I'm stuck in a circumstance where it's hard to validate logic with the template field too where it's empty by default so it makes it difficult to validate if not set or someone has set it to default template.

Or I get the field to show sometimes and set "sometimes" validation and it's still not required despite the label saying it will be required when it shows.

Interested to hear your thoughts on the Statamic way, this is just how I approach things developing in Laravel.

@jasonvarga
Copy link
Member

If the field is hidden, it won't be submitted, and won't be validated.

That's consistent to traditional forms with Laravel. If a form is missing an input, it doesn't get submitted, and wouldn't be validated.

What visibility condition are you applying to it?

In order to force the field to be submitted/validated:

  • Add always_save: true to it
  • Or make the visibility based on a Revealer field. We detect those and will still submit the hidden values.

@jonathan-bird
Copy link
Author

@jasonvarga just to clarify, in my original post the field was set, it was just an empty value (was visible).

I'll take a look at your suggestions too

@jasonvarga
Copy link
Member

I'll eat my words now. You're absolutely right.

We add nullable to non-required fields behind the scenes.

f0ff892

It's been like that for a while though. We'll have to think about how to resolve this in a non-breaking way.

@jasonvarga jasonvarga reopened this Nov 21, 2024
@jonathan-bird
Copy link
Author

Haha all good! If I can help at all please let me know.

I thought I wasn't going crazy cause the screenshots from Marty a while back show it as working - https://www.martyfriedel.com/blog/how-to-use-custom-laravel-validationrule-rules-in-statamic

@jasonvarga
Copy link
Member

Those class based rules definitely work, but as you've pointed out, they need to have a value.

@jasonvarga
Copy link
Member

Actually thinking about it a little more, I think it's probably working fine.

Can you elaborate on what you actually wanted to do in your custom validation rule?

You've named it TemplateDefaultOrNotSet which sounds like maybe you're planning to allow the value to be null anyway, in which case it shouldn't matter if the rule doesn't run.

@robdekort
Copy link
Contributor

robdekort commented Nov 22, 2024

I think @freshface is running into a similar issue with frontend forms (precognition). Since you can't use required|sometimes currently due to #9172, he made a custom validation rule called RequiredIfArrayContains. This validation rule should make a field required if an array (coming from a checkbox field) contains a certain value. However this validation logic doesn't seem to trigger if the field doesn't have a value. And that's exactly when you'd want it to trigger. It only triggers on blur when there's already data in the field.

@freshface
Copy link
Contributor

You need to add public $implicit = true;

<?php

namespace App\Rules;

use Closure;
use Illuminate\Contracts\Validation\ValidationRule;
use Illuminate\Contracts\Validation\DataAwareRule ;

class RequiredIfArrayContains implements DataAwareRule, ValidationRule
{

    protected $data = [];
    public $implicit = true;


    public function __construct(
        private $key = null,
        private $containsValue = null,
    ) {
        //

    }

    /**
     * Run the validation rule.
     *
     * @param  \Closure(string): \Illuminate\Translation\PotentiallyTranslatedString  $fail
     */
    public function validate(string $attribute, mixed $value, Closure $fail): void
    {
        ray($this->key);
       if(isset($this->data[$this->key]) && in_array($this->containsValue, $this->data[$this->key])) {
         if(empty($value)) {
            $fail('The :attribute field is required.')->translate();
         }
        }
    }


    public function setData(array $data): static
    {
        $this->data = $data;

        return $this;
    }
}

@jasonvarga
Copy link
Member

Oh, there you go. Nice.

@robdekort
Copy link
Contributor

Neat!

@freshface
Copy link
Contributor

It works, but would be nice to make the 'sometimes' option work again.
Due to this limitation its not possible anymore for clients to build a form with "show if when condition x is met" anymore.

@jasonvarga
Copy link
Member

Maybe it's because it's Friday afternoon but I'm really having trouble understanding your issue.

If your field has a value, the rule runs. Cool.

If your field doesn't have a value, your rule won't run. But if you're allowing nulls, why does your validation rule even need to run at all?

If you want to make it required, add required.

If you want it to be required only when its conditionally shown based on another field, you can add sometimes|required.

What am I missing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants