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

[1.x] Migrate validation rule classes from Rule to Validation rule #84

Closed
wants to merge 43 commits into from

Conversation

kiankamgar
Copy link
Contributor

I just refactored rule classes, If you want I can refactor tests as well

@milwad-dev milwad-dev marked this pull request as draft March 1, 2024 15:51
@kiankamgar kiankamgar marked this pull request as ready for review April 17, 2024 08:52
@kiankamgar
Copy link
Contributor Author

Hi again, do you have any plans to merge these changes in the future?

@kiankamgar kiankamgar marked this pull request as draft April 17, 2024 08:55
@milwad-dev
Copy link
Owner

The Laravel-Validate support Laravel 9 and I don't think we can merge this!

@kiankamgar
Copy link
Contributor Author

kiankamgar commented Apr 17, 2024

I can make the code support laravel 8 and 9 as well, if you want. We can decide to initialize rule class based on the laravel version

@kiankamgar
Copy link
Contributor Author

kiankamgar commented Apr 17, 2024

The old rule class is deprecated and soon or later it will be removed

@milwad-dev
Copy link
Owner

Can you show how to do it?

@kiankamgar
Copy link
Contributor Author

We can change every rule class to a facade, for example consider ValidPhoneNumber:

<?php

namespace Milwad\LaravelValidate\Rules;

use Illuminate\Support\Facades\Facade;

class ValidPhoneNumber extends Facade
{
    protected static function getFacadeAccessor()
    {
        if (app()->version() < 10) {

            return new \Milwad\LaravelValidate\Rules\Legacy\ValidPhoneNumber();
        }

        return new \Milwad\LaravelValidate\Rules\Modern\ValidPhoneNumber();
    }
}

Now we have Modern and Legacy forders in our Rule folder. When a user requests a validation, we check laravel version and return related class based on it:
\Milwad\LaravelValidate\Rules\Legacy\ValidPhoneNumber

<?php

namespace Milwad\LaravelValidate\Rules\Legacy;

use Illuminate\Contracts\Validation\Rule;
use Milwad\LaravelValidate\Utils\CountryPhoneCallback;

class ValidPhoneNumber implements Rule
{
    public function __construct(protected ?string $code = null)
    {
    }

    /**
     * Check phone number is valid.
     *
     * @param  string  $attribute
     * @param  mixed  $value
     * @return bool
     */
    public function passes($attribute, $value)
    {
        if (is_string($this->code)) {
            $passes = (new CountryPhoneCallback($value, $this->code))->callPhoneValidator();

            return collect($passes)->some(fn ($passe) => $passe);
        }

        return preg_match('/^[\+]?[(]?[0-9]{3}[)]?[-\s\.]?[0-9]{3}[-\s\.]?[0-9]{4,6}$/', $value);
    }

    /**
     * Get the validation error message.
     *
     * @return string
     */
    public function message()
    {
        return __('validate.phone-number');
    }
}

\Milwad\LaravelValidate\Rules\Modern\ValidPhoneNumber:

<?php

namespace Milwad\LaravelValidate\Rules\Modern;

use Closure;
use Illuminate\Contracts\Validation\ValidationRule;
use Milwad\LaravelValidate\Utils\CountryPhoneCallback;
use function collect;

class ValidPhoneNumber implements ValidationRule
{
    public function __construct(
        protected ?string $code = null
    ){}

    /**
     * Check phone number is valid.
     *
     * @param string $attribute
     * @param mixed $value
     * @param Closure $fail
     * @return void
     */
    public function validate(string $attribute, mixed $value, Closure $fail): void
    {
        if (! $this->isPhoneNumberValid($value)) {

            $fail('validate.phone-number')->translate();
        }
    }

    /**
     * Check if phone number is valid
     *
     * @param mixed $value
     * @return bool
     */
    private function isPhoneNumberValid(mixed $value): bool
    {
        if (is_string($this->code)) {
            $passes = (new CountryPhoneCallback($value, $this->code))->callPhoneValidator();

            return collect($passes)->some(fn ($passe) => $passe);
        }

        return preg_match('/^[\+]?[(]?[0-9]{3}[)]?[-\s\.]?[0-9]{3}[-\s\.]?[0-9]{4,6}$/', $value);
    }
}

Even we can extract our boolean logic from these two classes and bring it to another class that is responsible for the result and in each class we call the Logic class and return its value

@kiankamgar
Copy link
Contributor Author

kiankamgar commented Apr 17, 2024

Tell me what do you think about this approach. If you want I can do it and you can see the result and decide

@milwad-dev
Copy link
Owner

I don't like this way, if Laravel doesn't support these rules in the future, we think about it and you can create a new PR again or create an issue to talk about this.

@milwad-dev milwad-dev closed this Apr 19, 2024
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.

2 participants