Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#1836 closed Bug (fixed)

Weekly PDF reports not sent on mondays, Monthly reports not sent the 1st of month

Reported by: JulienM Owned by: JulienM
Priority: major Milestone: Piwik 1.1
Component: Core Keywords:
Cc: Sensitive: no

Description

As reported on the forum, the pdf plugin needs to be updated to force weekly reports to be sent on mondays.

In plugins/PDFReports/PDFReports.

TODO: Use setDay(1) on Piwik_ScheduledTime_Weekly.

Should we set a specific hour aswell ?

Change History (18)

comment:1 Changed 3 years ago by matt (mattab)

Actually it can get tricky, as the PDF reports should really only be set once all "days" have been finished, ie. in all timezones that the websites are registered.

The ideal solution would be to send the PDF reports for each website, as soon as the website day is finished... ie. send 3 reports at midnight UTC for the websites set to UTC, no reports sent until 10AM, and then sent reports to websites set to UTC-10 at 10am... but not necessary to be that precise, the previous solution would be easier!

So I think the simpler fix would be to setHour( LATEST HOUR where a day finishes for ANY website registered), so all PDF reports are sent after we are confident that all days are finished...

Does it make sense?

comment:2 Changed 3 years ago by matt (mattab)

Note: Monthly reports should be similarly sent the first day of the month (as soon as all months are finished, in the websites timezones)

comment:3 Changed 3 years ago by matt (mattab)

  • Milestone set to 1.1 - Piwik 1.1

comment:4 Changed 3 years ago by matt (mattab)

  • Summary changed from weekly PDF reports not sent on mondays to Weekly PDF reports not sent on mondays, Monthly reports not sent the 1st of month

comment:5 Changed 3 years ago by JulienM (JulienMoumne)

Correct me if I'm wrong.

So that all websites will have reached their local 00h00 when the report is sent, setHour should be set at :

(00h00 + [Server time zone number] - [Minimum time zone number of all sites]), Server local time

comment:6 Changed 3 years ago by JulienM (JulienMoumne)

With this equation, in the following case, the report would be (correctly) sent the last day of the month instead of the first :

  • Piwik Server is UTC +1
  • Only 1 site is set-up and uses UTC +15

The report could correctly be sent the last day of the month at 10AM from the Piwik Server because the site using UTC +15 will have already finished its day.

Should we implement it like that or simply send the report at 00h00 Piwik server time ?

The algorithm would then become :

timeShift = [Server time zone number] - [Minimum time zone number of all sites]
if (timeShift < 0) {
    setHour(0)
} else {
    setHour(timeShift)
}

comment:7 Changed 3 years ago by matt (mattab)

The server timezone is always set to UTC (piwik forces UTC in index.php)

Interesting file to look at Date/timezone manipulation helpers is tests/core/Date.test.php

A slightly similar function to look at also is setMinMaxDateAcrossWebsites in Multisites/controller

Finally, to get all websites and then the timezone, you can use the SitesManager_API.getSitesWithAtLeastViewAccess function (as the script will run as super user).

It is not the best performance wise (each call to runTasks() will result in selecting all websites and comparing all timezones), but it will work fine. To make it fast for a Piwik server with thousands of websites, we could for example record, on each website update, the earliest and latest timezone available in the _option table (but this will be a feature request when we hit problems, not yet the case :)

comment:8 Changed 3 years ago by JulienM (JulienMoumne)

I was indeed wondering if Piwik forces UTC 0, thanks for clarifying.

This issue, still applies though.

comment:9 Changed 3 years ago by JulienM (JulienMoumne)

Should CodeAdminHome scheduled task be updated as well to be triggered the first day of the month ?

comment:10 Changed 3 years ago by matt (mattab)

Yes maybe this task could be ran 1st day of month as well.
Maybe the default could be that all monthly tasks run the 1st day of the month, after the 1st initial run?

comment:11 Changed 3 years ago by JulienM (JulienMoumne)

Interesting idea. In that case, for API consistency, we need to set defaults for Daily and Weekly as well.

comment:12 Changed 3 years ago by matt (mattab)

Defaults should be:

  • Daily = midnight by default (server time, always UTC)
  • Weekly = monday by default
  • Monthly = 1st day of month

comment:13 Changed 3 years ago by JulienM (JulienMoumne)

(In [3387]) refs #1836 comment:12 - set defaults to setHour & setDay, updating unit tests

comment:14 Changed 3 years ago by JulienM (JulienMoumne)

Here is a working proposal to set the correct hour to the scheduled tasks.
I wanted to submit here first as I'm not entirely sure if it is the most elegant way this can be done.

If you have 3 sites with UTC+1, UTC-11, UTC-8.5, it will

->setHour(11);

If you have 3 sites with UTC+1, UTC+10, UTC+15, it will

->setHour(0);

If you have 1 site with UTC-8.5, it will

->setHour(9);
	function getScheduledTasks ( $notification )
	{
		// Computes which hour should the tasks run according to their timezones
		$maxOffset = 0;
		$mySites = Piwik_SitesManager_API::getInstance()->getSitesWithAtLeastViewAccess();
		foreach($mySites as &$site)
		{
			$baseDate = Piwik_Date::factory("1971-01-01");
			$offsetDate = Piwik_Date::factory("1971-01-01",  $site['timezone']);

			// Earlier means a negative timezone
			if ( $offsetDate->isEarlier($baseDate) )
			{
				// Gets the timezone offset
				$hourOffset = (24 - date ('H', $offsetDate->getTimestamp()));

				if ( $hourOffset > $maxOffset )
				{
					$maxOffset = $hourOffset;
				}
			}
		}

		$tasks = &$notification->getNotificationObject();

		$dailySchedule = new Piwik_ScheduledTime_Daily();
		$dailySchedule->setHour($maxOffset);
		$tasks[] = new Piwik_ScheduledTask ( $this, 'dailySchedule', $dailySchedule );

		$weeklySchedule = new Piwik_ScheduledTime_Weekly();
		$weeklySchedule->setHour($maxOffset);
		$tasks[] = new Piwik_ScheduledTask ( $this, 'weeklySchedule', $weeklySchedule );

		$monthlySchedule = new Piwik_ScheduledTime_Monthly();
		$monthlySchedule->setHour($maxOffset);
		$tasks[] = new Piwik_ScheduledTask ( $this, 'monthlySchedule', $monthlySchedule );
	}

comment:15 Changed 3 years ago by matt (mattab)

what is the status of the ticket? can you please post a patch, or commit if you're confident

comment:16 Changed 3 years ago by JulienM (JulienMoumne)

  • Resolution set to fixed
  • Status changed from new to closed

(In [3524]) fixed #1836 - sets hour on scheduled tasks to wait for end of period for all configured websites timezone, should be refactored in utility class

comment:17 Changed 3 years ago by matt (mattab)

  • Priority changed from normal to major

comment:18 Changed 3 years ago by JulienM (JulienMoumne)

As forecasted in comment:7 and experienced in 1981, loading the list of all websites does not scale.

Note: See TracTickets for help on using tickets.