spicywebau / craft-fieldlabels

Override Craft CMS field labels and instructions in the field layout designer
MIT License
123 stars 9 forks source link

Craft CMS 3.1.31 Undefined Variable #37

Closed kylecotter closed 5 years ago

kylecotter commented 5 years ago

Description

On updating to Craft CMS 3.1.31 and trying to save a new label we get an Undefined variable: label

This stems from:

// Trigger an `afterSaveLabel` event
        if ($this->hasEventHandlers('afterSaveLabel')) {
            $this->trigger('afterSaveLabel', new FieldLabelsEvent([
                'label'      => $label,
                'isNewLabel' => $isNew,
            ]));
        }

Starting at line 258 in /vendor/spicyweb/craft-fieldlabels/src/Service.php.

Downgrading Craft removes the error.

This is a multisite Craft setup. Not sure that's connected.

Steps to reproduce

  1. Attempt to relabel a field on an entry type
  2. Save

Other information

Thanks!

dreamseeker commented 5 years ago

Hi.

Same error occurred in similar environment(Craft: 3.1.31, Field Label: 1.1.2, and Multi Site).

It seems can fix, like this:

            // Trigger an `afterSaveLabel` event
            if ($this->hasEventHandlers('afterSaveLabel')) {
                $this->trigger('afterSaveLabel', new FieldLabelsEvent([
                    'label'      => $data['name'],  // only this line changed.
                    'isNewLabel' => $isNew,
                ]));
            }

https://github.com/spicywebau/craft-fieldlabels/blob/master/src/Service.php#L261

Thanks!

ttempleton commented 5 years ago

@kylecotter, @dreamseeker

It's correct that this error with the afterSaveLabel event occurs because of $label not being set in the handleChangedLabel() method. I'm honestly not sure what changed in Craft to trigger the bug. Anyway, it should be set to the FieldLabelModel of the saved label, as it is with the beforeSaveLabel event run at line 171 of src/Service.php within saveLabel(). Like this:

// Trigger an `afterSaveLabel` event
if ($this->hasEventHandlers('afterSaveLabel')) {
    $label = new FieldLabelModel([
        'id' => $record->id,
        'fieldId' => $record->fieldId,
        'fieldLayoutId' => $record->fieldLayoutId,
        'name' => $record->name,
        'instructions' => $record->instructions,
        'hideName' => $record->hideName,
        'hideInstructions' => $record->hideInstructions,
        'uid' => $record->uid,
    ]);

    $this->trigger('afterSaveLabel', new FieldLabelsEvent([
        'label'      => $label,
        'isNewLabel' => $isNew,
    ]));
}

I've opened pull request #38 with this solution.