1

I'm guessing I'm doing something bad with my code and it's bringing down the server. Here is the full code:

Event::on(Element::class, Element::EVENT_BEFORE_SAVE, function (ModelEvent $event) {
    if ($event->sender instanceof craft\elements\Entry) {
        $entry = $event->sender;
        if ($entry->typeId == '19') {
            $entry->expiryDate = $entry->eventDate->modify('+1 day');
            Craft::$app->elements->saveElement($entry);
        }
    }
});

This fails with the 502 error. But if I just change the 4th line to if ($entry->typeId == '19' && !$entry->expiryDate) { it works ok.

Can anyone help me learn why that would be? I specifically want to update the dates if eventDate has changed, so I figured I'd just reapply it everytime. Maybe that's too intensive?

1 Answer 1

3

Looks like you're causing an endless loop there. You're calling Elements::saveElement from within the beforeSave event. This will cause another save event, which will trigger your hook again, which will cause another save event, and so on. This will cause the PHP process to fail, so you get a 502 from the server.

This fails with the 502 error. But if I just change the 4th line to if ($entry->typeId == '19' && !$entry->expiryDate) { it works ok.

With this adjustment, you've prevented the recursive loop, because you're only calling Elements::saveElement once. The second iteration will not cause an additional save, because at that point, the entry has an expiryDate.

It's still inefficient because this causes the element to be saved twice instead of once.

Can anyone help me learn why that would be? I specifically want to update the dates if eventDate has changed, so I figured I'd just reapply it everytime. Maybe that's too intensive?

You can figure out what native attributes and custom fields have changed using Element::getModifiedAttributes and Element::getModifiedFields, but it's much easier and less error-prone to just set the expiry date every time.

The key is to leave out the call to Craft::$app->elements->saveElement($entry);. You're already in the EVENT_BEFORE_SAVE hook, so the element will be saved after your hook has run. So any changes you make to the element will persist automatically.

2
  • 1
    Oh wow, I never even considered it was saving twice...but that makes total sense. Thanks for the detailed explanation!
    – supazu
    Oct 14, 2022 at 13:30
  • 1
    This bit is most relevant - that's really the issue. "The key is to leave out the call to Craft::$app->elements->saveElement($entry);. You're already in the EVENT_BEFORE_SAVE hook, so the element will be saved after your hook has run. So any changes you make to the element will persist automatically." I missed it first reading, good work @MoritzLost.
    – Wallace
    Oct 27, 2022 at 16:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.