I've yet to find out why.
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.
live627
-
-
- Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
- Posts: 1,670
Arantor
-
-
- As powerful as possible, as complex as necessary.
- Posts: 14,278
My bet would be the scheduled task didn't use to fire because of not having the right dependencies (and thus becoming disabled because the function wasn't loaded and therefore not callable)... check that your scheduled_tasks table has the right content as per install.sql and check it's enabled.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial
live627
-
-
- Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
- Posts: 1,670
I actually meant here. I should be more clear. :P
@Nao> Please ensure the task has the right parameters in the DB. Also, the task itself must return true in order to not get disabled. :^^;:
Code: [Select]
@Dragooon> Why is this template so complicated? Ideally, the variables should be put into $context in the source file. I think templates/views should never do any logic work, only presentation.
@Nao> Please ensure the task has the right parameters in the DB. Also, the task itself must return true in order to not get disabled. :^^;:
Index: Sources/Notifications.php
===================================================================
--- Sources/Notifications.php (revision 2166)
+++ Sources/Notifications.php (working copy)
@@ -670,7 +765,7 @@
continue;
// Assemble the notifications into one huge text.
- $body = template_notification_email($m['notifications']);
+ $body = template_notification_email($m['notifications'], $m['id']);
$subject = sprintf($txt['notification_email_periodical_subject'], $m['name'], $m['unread']);
sendmail($m['email'], $subject, $body, null, null, true);
Index: Themes/default/Notifications.template.php
===================================================================
--- Themes/default/Notifications.template.php (revision 2166)
+++ Themes/default/Notifications.template.php (working copy)
@@ -64,46 +64,14 @@
}
-function template_notification_email($notifications)
+function template_notification_email($notifications, $member_id)
{
global $txt;
@@ -111,7 +79,7 @@
foreach ($notifications as $notifier => $notifs)
{
- list ($title) = weNotif::getNotifiers($notifier)->getProfile();
+ list ($title) = weNotif::getNotifiers($notifier)->getProfile($member_id);
$str .= '
<h3>' . $title . '</h3>
@Dragooon> Why is this template so complicated? Ideally, the variables should be put into $context in the source file. I think templates/views should never do any logic work, only presentation.
Dragooon
- I can code! Really!
-
-
- polygon.com has to be one of the best sites I've seen recently.
- Posts: 1,841
@live627: It isn't doing any logical work, I wouldn't count getting parameters from an object as logical work.
Okay, on a fresh Wedge installation periodical seems to be working. So it might be a problem just here.
 | The way it's meant to be |
Arantor
-
-
- As powerful as possible, as complex as necessary.
- Posts: 14,278
That points to the scheduled task then, of which I know I changed the semantics recently for calling (changes in DB table)
This is still an issue. @Nao please confirm what is in the scheduled tasks table matches what a new installation would generate.
Nao
- Dadman with a boy
-
-
- Posts: 16,082
Missed this topic... (As I did many others, obviously!)
Yes, the database structure had yet to be updated; including the size for 'task'.
If it still doesn't work, then... It's something else, I guess.
Yes, the database structure had yet to be updated; including the size for 'task'.
If it still doesn't work, then... It's something else, I guess.
Arantor
-
-
- As powerful as possible, as complex as necessary.
- Posts: 14,278
It wasn't just structural but also content in that change. But if it's updated now to match, we'll see if it starts working.
live627
-
-
- Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
- Posts: 1,670
Fatal error: Call to undefined method Notification::getEmail() in I:\www\wedge\trunk\Themes\default\Notifications.template.php on line 119
That's when trying to run the schd task.
That's when trying to run the schd task.
Nao
- Dadman with a boy
-
-
- Posts: 16,082
I had a quick look, and to be honest with you, I'm not exactly sure where the thing goes wrong... getEmail is a Notifier method, not a Notification one, but then as I said before, Notifiers in general are somehow quite unclear to me, and I tend to easily forget the difference, so... Maybe Dragooon, or someone else, can fix this for me..? :^^;:
live627
-
-
- Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
- Posts: 1,670
Is that someone else me? :P
I'm also finding it hard to understand. I've been doing my head in trying to make it work. The emails send, but the data isn't quite right. :^^;:
I'm also finding it hard to understand. I've been doing my head in trying to make it work. The emails send, but the data isn't quite right. :^^;:
Nao
- Dadman with a boy
-
-
- Posts: 16,082
Hmm, not particularly you, but, just in case you had a ready-made solution, I'd have gladly taken it... ;)
live627
-
-
- Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
- Posts: 1,670
Ok, so I've been playing around some more. The biggest issue is that the email body is duplicated since it is HTML. II have yet to understand why.
The above error is because it is calling on the Notification object and wants the getEmail() method from a Notifier.
Code: [Select]
Let's save some memory! $valid_notifiers is only checked for existence, so why not use a bool instead of an object?
Code: [Select]
Objects shouldn't be this hard to understand. I think it's the duplication that's the problem. Perhaps Notification can go, and its leftovers merged with weNotif. Also, shouldn't Notifier be abstract?
The above error is because it is calling on the Notification object and wants the getEmail() method from a Notifier.
Index: I:/repos/wedge/trunk/Themes/default/Notifications.template.php
===================================================================
--- I:/repos/wedge/trunk/Themes/default/Notifications.template.php (revision 2194)
+++ I:/repos/wedge/trunk/Themes/default/Notifications.template.php (working copy)
@@ -116,9 +116,10 @@
$str .= "\n" . $title . "\n" . str_repeat('=', strlen($title)) . "\n";
foreach ($notifs as $n)
- $str .= $n->getEmail();
-
- $str .= "\n\n";
+ {
+ list (, $n) = $n->getNotifier()->getEmail($n, array());
+ $str .= $n . "\n\n";
+ }
}
return $str;
Let's save some memory! $valid_notifiers is only checked for existence, so why not use a bool instead of an object?
Index: I:/repos/wedge/trunk/Sources/Notifications.php
===================================================================
--- I:/repos/wedge/trunk/Sources/Notifications.php (revision 2194)
+++ I:/repos/wedge/trunk/Sources/Notifications.php (working copy)
@@ -609,7 +608,7 @@
{
$status = isset($data['email_notifiers'][$notifier->getName()]) ? $data['email_notifiers'][$notifier->getName()] : 0;
if ($status < 2 && (empty($data['disabled_notifiers']) || !in_array($notifier, $data['disabled_notifiers'])))
- $valid_notifiers[$notifier->getName()] = $notifier;
+ $valid_notifiers[$notifier->getName()] = true;
}
if (empty($valid_notifiers))Objects shouldn't be this hard to understand. I think it's the duplication that's the problem. Perhaps Notification can go, and its leftovers merged with weNotif. Also, shouldn't Notifier be abstract?
Arantor
-
-
- As powerful as possible, as complex as necessary.
- Posts: 14,278
Last point first, Nao didn't want the Notifier to be completely abstract so that anything common to all notifiers could be extended easily. Quite what would be common to all notifiers, I don't know, I can't think of anything but that's the theory behind it.
The notification system is hard. When I wrote the likes notification I started from one of the others that was done and just hacked it about until it worked. I actually don't understand all the guts of it, I don't think I ever have, actually...
The notification system is hard. When I wrote the likes notification I started from one of the others that was done and just hacked it about until it worked. I actually don't understand all the guts of it, I don't think I ever have, actually...