Today, on "2 hours of my life I'll never get back"... :P
The defaults for simplefeed_save_engine and simplefeed_parse_engine are both 0 instead of something sensible, like, say... "simplefeed_item"
When module_invoke attemps to call 0_save_engine it fails, since there is no function called 0_save_engine, however there are no errors to inidicate this. Worse, the last checked time updates so those feeds that ran during the cron job that fails will never get parsed again unless you go into the database manually to reset their checked time.
You can argue that people should be visiting the settings page before using the module, but the module should either warn people about that on every single page and not work at all, or (preferably) do *something* if people forget to do that step.
Here's a patch.
| Comment | File | Size | Author |
|---|---|---|---|
| sensible-defaults.patch | 2.58 KB | webchick |
Comments
Comment #1
csevb10 commentedTechnically, this should never be a problem unless you had simplefeed installed before we updated the code with the save_engine, parse_engine additions. In order to set up a feed, you'll have to select the saving and parsing engines (or if you don't select them, they'll just default to an available parser). I don't know that defaulting to simplefeed_item is any worse than defaulting to 0, so I don't have any problem with setting that as a default, but it shouldn't ever be an issue for new installs of simplefeed.
Comment #2
webchickReally? I don't see how it would not be an issue if you installed the module but didn't ever hit the settings page (which you might not, since in most modules, settings pages are for optional things). But if that use case has been tested already and it's found to not be the case, feel free to won't fix.
Comment #3
csevb10 commentedWhen you create the feeds (on the node page for feeds), it has options for that particular feed to be parsed/saved by any particular supporting module (at the moment, just simplefeed item), but you have no choice but to select an applicable parsing module/saving module at that point. Therefore, even though the admin default is 0, you'll be forced to save the feed with something selected....meaning the situation only affects people that created feeds before the parsing/saving engine stuff was added. Does that make sense? This patch should clear up any legacy issues that anyone who had simplefeed installed before the parse & save engine elements were added.
Comment #4
webchickOh, ew. I didn't realize that was exposed in the UI to end users. :( ok, fair enough.
Comment #5
m3avrck commentedOnly to those that have "admin feeds" though, not every user :-)
Case in point: feed 1 is parsed by engine A and feed 2 by engine B -- tons of use cases :-)