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

Make it easier to add custom validation #196

Merged
merged 5 commits into from Jan 21, 2022
Merged

Conversation

nstCactus
Copy link
Contributor

Description

As suggested in the PHPdoc of the Yii Model::defineRules() method:

Models should override this method instead of [[rules()]] so [[EVENT_DEFINE_RULES]] handlers can modify the class-defined rules.

As suggested in the PHPdoc of the Yii `Model::defineRules()` method:

> Models should override this method instead of [[rules()]] so [[EVENT_DEFINE_RULES]] handlers can modify the class-defined rules.
@ademers
Copy link

ademers commented Nov 2, 2020

I want to make the fromName and subject fields required, so in a custom module, I added:

Event::on(
    Submission::class,
    Submission::EVENT_DEFINE_RULES,
    function(DefineRulesEvent $event) {
        $event->rules[] = [['fromName', 'subject'], 'required'];
    }
);

However, it only works when I change public function rules() to public function defineRules(): array in Submission.php.

Discord: https://discordapp.com/channels/456442477667418113/456442884258922529/772923383573905470

@MoritzLost
Copy link

Any status update on this? Changing the method from rules to defineRules shouldn't change the behaviour and make it much easier to add custom validation.

@ademers
Copy link

ademers commented Jan 20, 2022

Would really like to see this implemented!

@AugustMiller
Copy link

AugustMiller commented Jan 20, 2022

Discovered with @bencroker that this might be a breaking change, because Model::defineRules() is only available in Craft ^3.4.0…

Edit: Maybe not? Seems like we could quietly support 3.4+, as long as the method signature is compatible. The real fix might be more like adding parent::rules() to the overridden method?

@nstCactus
Copy link
Contributor Author

@AugustMiller Wouldn't it be possible to bump the required Craft CMS version to 3.4 in composer.json for the next version?
That way, the plugin would still be compatible with old Craft CMS installs in its current version but users of Craft CMS > 3.4 would get new versions.

@brandonkelly brandonkelly merged commit 7361fe2 into craftcms:v2 Jan 21, 2022
@brandonkelly
Copy link
Member

Thanks @nstCactus! Just merged and released as 2.3.0.

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.

None yet

5 participants