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

Deprecate Validator Class #515

Open
MGatner opened this issue Apr 27, 2022 · 3 comments
Open

Deprecate Validator Class #515

MGatner opened this issue Apr 27, 2022 · 3 comments

Comments

@MGatner
Copy link
Collaborator

MGatner commented Apr 27, 2022

BaseValidator and ValidatorInterface provide almost identical offerings. One of these should be deprecated, depending on whether the intent. Either:

  • keep the base, adding abstract public function check() (less restriction, more changes elsewhere)
  • keep the interface, possibly convert the base to an optional trait

Either solution should account for the following:

  1. The interface still uses CodeIgniter\Entity which is deprecated
  2. The interface method specifies Entity input when most validators require User
  3. PasswordValidator is not a valid interface class because of 2)
@manageruz
Copy link
Collaborator

Maybe keep them both and use an interface to specify the contract and an abstract class as just one implementation thereof?

@MGatner
Copy link
Collaborator Author

MGatner commented Jun 3, 2022

Shield has figured this out, it would probably make sense to imitate the solution there: https://github.com/codeigniter4/shield/tree/develop/src/Authentication/Passwords

@manageruz
Copy link
Collaborator

manageruz commented Jun 23, 2022

So we should modify ValidatorInterface.php :

  1. Replace use CodeIgniter\Entity with Myth\Auth\Entities\User
  2. Replace public function check(string $password, ?Entity $user = null): bool method with public function check(string $password, ?User $user = null): bool
  3. Do all necessary changes to all validators to use User class instead of Entity class.

Is that's all or am i missing something else?

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

2 participants