adventistchurch / alps-wordpress

This Wordpress theme is an implementation of the Adventist Living Pattern System (ALPS) designed to support the work of the Seventh-day Adventist Church.
Other
43 stars 38 forks source link

Fix undefined variable exceptions in header block featured #675

Closed mattleff closed 1 year ago

mattleff commented 1 year ago

In #538 the header block featured was added with two undefined variables, $mediaBlockTitleLink which was assigned the undefined variable $headerTitleLink:

https://github.com/adventistchurch/alps-wordpress/blob/77598f35284385044123a9d46b0dbb65a90fb9e5/resources/views/patterns/01-molecules/blocks/header-block-featured.blade.php#L15

and $mediaBlockKicker which was used in the media block v2 template without being defined in header block featured:

https://github.com/adventistchurch/alps-wordpress/blob/77598f35284385044123a9d46b0dbb65a90fb9e5/resources/views/patterns/01-molecules/blocks/media-block-v2.blade.php#L42-L44

I believe that the getPostData($postId) function in app/template-helpers.php returns the data for the header block featured template without including data for either of these variables:

https://github.com/adventistchurch/alps-wordpress/blob/77598f35284385044123a9d46b0dbb65a90fb9e5/app/template-helpers.php#L122-L130

These undefined variables can cause a fatal exception (depending on PHP version):

Undefined variable $headerTitleLink (View: .../wp-content/themes/alps-wordpress-v3/resources/views/patterns/01-molecules/blocks/header-block-featured.blade.php)

The changes proposed in this PR are only to fix the fatal exceptions without actually addressing the missing variable sources.

YauheniKapliarchuk commented 1 year ago

Hi @mattleff,

I am going to check PR but I want to ask you please do not keep your changes until a new version of the theme is released.

Check status here: https://github.com/adventistchurch/alps-wordpress/pull/668 or on releases in github.

Thanks, Best Regards!

claytonk commented 1 year ago

@YauheniKapliarchuk I believe this issue was resolved by #695

YauheniKapliarchuk commented 1 year ago

@claytonk

Thank you for this, You're right!