@diosmosis opened this Pull Request on May 5th 2018 Member

Note: this PR has API changes.

image

This PR makes two changes to the way notifications are handled/displayed:

  • Transient notifications are now removed when the reporting page changes.
  • ViewDataTables have a view property called 'notifications' which can be used to add new notifications when loading a report. This is not in the Config so is not API. (The actual creation of notifications is done in the widget loader directive.)

Individual changes:

  • Remove 'notification_parser.js' and replace w/ notification.service.js angular service.
  • Add 'notifications' property to ViewDataTable's View & handle in _dataTable.twig. The widget loader directive initiates the "parse" of the divs outputted in _dataTable.twig. (Also done by widget loading logic in dashboard code.)
  • In reporting page controller, clear transient notifications after page load.
  • Added getSegmentByDefinition() function to SegmentEditor Model.
  • Add new event Visualization.onLoadingError that is triggered when there an exception is caught when loading report data.
  • Add new event Archive.noArchivedData that is triggered when during an API request no archive data can be found.
  • Use new event to display a notification when a report has no data and is for an unprocessed segment.

Fixes #12256
Fixes #12255

@diosmosis commented on May 6th 2018 Member

Adding to 3.5.0 milestone in case it can go in, if not can remove it.

@diosmosis commented on May 21st 2018 Member

@tsteur would you have time to review this PR?

@diosmosis commented on May 24th 2018 Member

After discussing it appears to risky to introduce this change in 3.5.1, will remove this + the related issues from the current milestone.

CC @tsteur / @mattab

@diosmosis commented on May 28th 2018 Member

@mattab If we only show this notification for dates that are < 2 months old, will it still be useful?

For context: we don't really know if an older date will be archived by the next cron:archive run, so if we say data for every date will become available, it's possible some users will just wait forever & then get angry.

@mattab commented on May 29th 2018 Member

If we only show this notification for dates that are < 2 months old, will it still be useful?
For context: we don't really know if an older date will be archived by the next cron:archive run, so if we say data for every date will become available, it's possible some users will just wait forever & then get angry.

I think we are able to know whether a given date in the past will be archived. we wouldn't want to pick a certain date (eg. 2 months) because that depends on the settings of how far log data was purged. In theory (without reading the whole thread) the historical data will be processed for a segment, as long as we still have the raw data (maybe haveLogsBeenPurged is useful there) @diosmosis

@diosmosis commented on May 29th 2018 Member

@mattab So what about min(2 months, $logDataPurgeTime)? so if the log data purges older than 1 month, we check for 1 month, if it's 3 months, we check for 2 months?

CC @tsteur since he's more aware of the limitations of knowing what core:archive will do than I am and can probably explain the issue better.

@tsteur commented on May 29th 2018 Member

In theory (without reading the whole thread) the historical data will be processed for a segment, as long as we still have the raw data

I'm not quite sure since it might depend on when the archiving ran the last time? See https://github.com/matomo-org/matomo/blob/3.5.1/core/CronArchive.php#L1556-L1577

I reckon it may be best to start a document or comment to write all the things done we know influence when something will be archived and when not. For example the process_new_segments_from setting etc.

@mattab commented on May 31st 2018 Member

I'm not quite sure since it might depend on when the archiving ran the last time? See https://github.com/matomo-org/matomo/blob/3.5.1/core/CronArchive.php#L1556-L1577

The historical segments should always be processed within a day or two... because whenever we next call "last2 month" archiving and then "last2 year" archiving, it will pre-process the last ~ 700 days (last 2 years) / last 24 months / etc.

(assuming we have a compatible process_new_segments_from value eg. beginning_of_time)

@mattab commented on May 31st 2018 Member

So what about min(2 months, $logDataPurgeTime)? so if the log data purges older than 1 month, we check for 1 month, if it's 3 months, we check for 2 months?

@diosmosis don't understand the "2 months", AFAIK we don't want to use a fixed date as most of the time it will even pre-process 1 year, or 2 years, or 3 years of historical segment data.

@diosmosis commented on May 31st 2018 Member

That was @tsteur’s suggestion on slack, I was deferring to his knowledge of the archiving process.

Powered by GitHub Issue Mirror