I redirected to hardcoded "yesterday" because I don't want to overthink it, but if you think it's a bad idea let me know.
FYI build failed because of random failure: http://builds-artifacts.piwik.org/ui-tests.rang-disabled/11472.7/screenshot-diffs/diffviewer.html
@mnapoli - maybe it could be a config setting defaulting to yesterday? I'm preety much confident that there will be a case at some point of future when somebody will prefer to set different fallback than 'yesterday'
There are config settings: [General]
default_period so let's re-use those
Does it maybe make sense to fix this in the
getDefaultDate method itself? https://github.com/piwik/piwik/blob/2.13.0-b1/plugins/UsersManager/UserPreferences.php#L75-L120
I think the error message / exception is ok if someone manipulates that URL directly etc. Also if we use eg
getDefaultPeriod somewhere else we would otherwise have this problem again. It would maybe also make sure to show correct value in the "User Settings" UI (eg yesterday instead of "last 7" or instead of no preselected value, haven't actually tested it)
Yes thanks it definitely makes sense, I will update the PR for that solution
I have updated the PR with an alternate solution that prevents UserPreferences to return a disallowed period or date. I have also restored the exception.
The refactoring/solution looks good to me (there is one feedback to fix and it can merged then)
Fixed, good to merge