spicywebau / craft-neo

A Matrix-like field type for Craft CMS that uses existing fields
Other
402 stars 63 forks source link

Stops clearing errors already applied to neo blocks #882

Closed JasonStainton closed 4 months ago

JasonStainton commented 5 months ago

When custom errors have been applied to neo blocks via events (for example Entry::EVENT_DEFINE_RULES), this call to validate blocks removes them. This change retains existing errors while allowing Neo to include its own.

Unsure if this is the correct way to go about this, I couldn't understand why none of my errors was displaying on the block, but model errors were fine. I traced it back to this and found that calling block validate was stripping them out.

ttempleton commented 5 months ago

Thanks for opening this PR @JasonStainton.

Is there any chance you'd be able to share your entry EVENT_DEFINE_RULES listening code that's applying Neo block errors? Depending on how/what you're applying, it might be better to listen for EVENT_DEFINE_RULES events triggered for the Neo blocks themselves and adding rules there instead.

JasonStainton commented 5 months ago

Hi @ttempleton - before I go into what I was doing, I want to say I have since changed my approach and used the neo block events and this is working for me. I've included some code below that gives the gist of what I was doing and although I now don't need what I suggested, would it still be a relevant change? There are probably some knock-on effects I don't know about and you can promptly shut me down 😅

        Event::on(
            Entry::class,
            Entry::EVENT_DEFINE_RULES,
            function (DefineRulesEvent $event) {
                if ($event->sender->type->handle === 'ENTRY_TYPE_HANDLE') {
                    $event->rules[] = [['NEO_FIELD_HANDLE'], ExampleValidator::class];
                }
            }
        );
class ExampleValidator extends \yii\validators\Validator
{
    //TODO:JS - Keeping this here for future research.
    /**
     * @inheritdoc
     */
    public function validateAttribute($model, $attribute): void
    {
        if (ElementHelper::isDraftOrRevision($model)) {
            return;
        }

        if (!in_array($model->getScenario(), [Element::SCENARIO_ESSENTIALS, Element::SCENARIO_LIVE])) {
            return;
        }

        if (!in_array($model->otherField, ['values', 'to', 'check'])) {
            return;
        }

        $attributeQuery = $model->$attribute;

        $blocks = $attributeQuery->getCachedResult() ?? $attributeQuery->limit(null)->anyStatus()->all();

        foreach ($blocks as $block) {
            if ($block->type->handle === 'block_handle') {
                if ($model->anthotherField == false) {
                    $block->addError('BLOCK_FIELD_ERROR', 'Block level field error. This gets wiped.');
                    $block->addError('__NEO_BLOCK_ERROR__', 'Block level error. This gets wiped.');
                }
            }
        }

        $attributeQuery->setCachedResult($blocks);

        $model->addError($attribute, 'Model level error. This does not get wiped.');
    }
}
ttempleton commented 4 months ago

I don't think this would have negative effects, and I think it's an interesting way to allow custom Neo block validation based on other fields' values. I'll merge this into the 5.x branch. Thanks again!