victorjonsson / Arlima

Article List Manager - Wordpress plugin suitable for online newspapers that's in need of a fully customizable front page
28 stars 16 forks source link

Improved validation in arlima_get_list() #81

Closed aaslun closed 9 years ago

aaslun commented 9 years ago

Sorry for this PR, but $current_arlima_list['list']->getVersion() sometimes causes fatal errors for us. This PR should not impact any other implementation as it is just an additional is_object() check in the if-statement.

Reason: It seems that in some of our page rendering contexts, the static $current_arlima_list has a list and a status that is null. I'm guessing this has to do with how our theme implementation and Arlima plays together since this is not a known issue you are experiencing? I'm not yet sure why or where this method call is even done at these times, but it seems to be connected with some filter or action inside or near the_content since the page rendering breaks right after the_title on any WP page. Anyway, I tried to trace down the method call to avoid this situation in the first place but it seems that I need to do some more debugging and testing.

victorjonsson commented 9 years ago

The code in that function will change a lot when issue #65 is finished.

Was the version null? Wasn't it so that the $current_arlima_list['list'] was false? The logic managing scheduled versions should not be there at all. The function, as the docs says, will return the list connected to currently visited page. The function will return false (or an array with 'list' property set to false) in case you're not visiting a page, or if the page isn't connected to a list. The list will however never contain a scheduled version.

When #65 is finished I will write up som documentation in the wiki about this.

aaslun commented 9 years ago

Ah, right. It was false, not null. Thanks for the merge. I'll try to see if we in our theme files can avoid introducing any contexts where we're calling this function when it's not applicable.

victorjonsson commented 9 years ago

Your code must always handle that the function might return false.

aaslun commented 9 years ago

Yes, I agree, but in this case it was not the function return value that was the issue. The issue was the fact that this function was called when not supposed to (apparently in some filter, needs more research) and that the static $current_arlima_list that was instantiated by the function itself on line 180 in arlima.php contained the keys ['list'] and ['post'] that was false.

Here's how the static $current_arlima_list looked when I dumped it on line 184 in arlima.php:

[12-Jan-2015 11:12:02 UTC] Array
(
    [list] =>
    [post] =>
)

[12-Jan-2015 11:12:02 UTC] PHP Fatal error:  Call to a member function getVersion() on a non-object in /var/www/stage/public/wp-content/plugins/arlima/arlima.php on line 185

But as you said, I guess we don't need to linger on this since the code will probably differ a lot from it's current implementation after #65.

victorjonsson commented 9 years ago

yes... Lets wait and see how the changes in #65 plays out for you