Skip to content

fix: Change the visibility of validation properties for easier inheritance #868

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

Merged

Conversation

sammyskills
Copy link
Contributor

@sammyskills sammyskills commented Sep 30, 2023

Description
Currently, the Validation class declares the properties $config and $tables as private which makes them impossible to use within child classes. This PR fixes that.

Checklist:

  • Securely signed commits
  • [] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [] Unit testing, with >80% coverage
  • [] User guide updated
  • Conforms to style guide

@datamweb
Copy link
Collaborator

Hey @sammyskills,

I can understand why you want to inherit from Validation?
As you know, you can use customization/validation_rules.
Can you provide an example for better understanding?

@datamweb datamweb added the enhancement New feature or request label Sep 30, 2023
@sammyskills
Copy link
Contributor Author

Hi @datamweb,

Yes, before the refactor of the validation rules from PR #861, one could easily customize the rules. But since the rules have now been placed in a class of its own, devs might want to use(reuse) some of the methods in the validation class, to create custom rules (as another way to create or use rules).

Take for instance, I have an application that uses login via phone number and password. I can create a validation class in my app/Validations directory, named ValidationRules. Within this class, I can create a method: getLoginByPhoneNumberRules(), which also utilizes the getPasswordRules() method from the parent class, like so:

// app/Validation/ValidationRules.php
use CodeIgniter\Shield\Validation\ValidationRules as ShieldValidationRules;

class ValidationRules extends ShieldValidationRules
{
    public function getLoginByPhoneNumberRules(): array
    {
        return [
            'phone' => 'required|numeric',
            'password' => $this->getPasswordRules()
        ]
    }
}

With the above, the getValidationRules() method of my custom LoginController _(which of course extends shield's LoginControler can return my own custom rule: getLoginByPhoneNumberRules().

I hope you get what I mean.

Copy link
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation,
when I reviewed the #861 I was aware that there would be no limit to customization.
Now I wanted to make sure that there was no limit.

Well, I understand with your explanation, you know that you can create and use your own methods and rules in file app/Config/Validation.php. However, this PR helps the integrity of the code to a great extent.
I think this PR is acceptable.

@sammyskills
Copy link
Contributor Author

you know that you can create and use your own methods and rules in file app/Config/Validation.php

Yes, I know, and that is what I've been using for a while now. But the PR #861 provides yet another way to set validation rules, for devs who prefer not to use config files.

@kenjis kenjis merged commit a5483f9 into codeigniter4:develop Oct 1, 2023
@kenjis
Copy link
Member

kenjis commented Oct 1, 2023

I'm fine this change if someone wants to extend the class.

@sammyskills sammyskills deleted the fix-validation-class-visibiliity branch October 1, 2023 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants