Hi. Found a couple of bugs (patch attached).

1. In accordance to Drupal API, hook_nodeapi() should return an array of keys => values for $node object. In your case you simply set $node->simplenews, which won't work. Fixed.
2. In function template_preprocess_simplenews_newsletter_footer(), $variables['context']['node'] is not set and this was leading to cron failing to run (in my case emails were being sending via cron run). Instead, $variables['context'] should be used (dropping ['node'] completely).

3. In accordance with Drupal coding standards, we use
if() {
}
else {
}
model instead of if ... endif;

These changes were proven to work on my client's site (upgraded from Drupal 5 recently).

CommentFileSizeAuthor
simplenews-footer.patch698 bytesivrh
simplenews.module.patch1.17 KBivrh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Simon Georges’s picture

Status: Needs review » Needs work

I agree with the hook_nodeapi('load') part, but not the preprocess_newsletter_footer part (neither when using cron, nor when not using it). Could you have another module eventually messing things up ?

ivrh’s picture

I don't think I have module affecting simplenews.
So you are saying that


function template_preprocess_simplenews_newsletter_footer(&$variables) {
  $variables['format'] = $variables['context']['node']->simplenews['s_format'];
  var_dump($variables['context']['node']->simplenews['s_format']); exit;

will display valid format for you? This was NULL for me, while the code below returned valid format string for me:


function template_preprocess_simplenews_newsletter_footer(&$variables) {
  $variables['format'] = $variables['context']['node']->simplenews['s_format'];
  var_dump($variables['context']->simplenews['s_format']); exit;

This could be the case, though that somehow my installation works differently (at least this was failing in Pressflow 6.20) with custom built theme, modified from Garland.

Simon Georges’s picture

If you have a custom theme, it may affects the variables. Could you try with Garland theme ?
What you display in the comment just above is exactly what is happening for me : $variables['context']['node']->simplenews['s_format'] is returning the correct value.

Do you have a standard Simplenews installation, or did you apply some patches ?