Fixes #7647 Variable overwriting itself #7666
Conversation
mnapoli
added
Bug
|
|
||
| foreach ($allUsersPreferences as $userLogin => $userPreferences) { | ||
|
|
||
| if (!isset($userPreferences[APIUsersManager::PREFERENCE_DEFAULT_REPORT_DATE])) { |
There was a problem hiding this comment.
I think it might be allowed for this to be empty, see UserPreferences::getDefaultPeriod which defaults it to the [General] default_period value. If that config value is set to 'range', then this range archiving will be executed for every user which may not be desired...
CC @mattab What do you think?
There was a problem hiding this comment.
Ah right I think I see what you mean. This api is depressing me, there's 0 encapsulation and we've got to care about the internal details everywhere (got the same kind of problems in another PR)… Thing is changing it is sometimes hard if the method is used/tagged with api as it could break BC. 😞
There was a problem hiding this comment.
If that config value is set to 'range', then this range archiving will be executed for every user which may not be desired...
FYI reading the config setting in global.ini it says:
; Possible values: day, week, month, year.
default_period = day
so range is not supported for this config setting.
There was a problem hiding this comment.
On further examination, my comment isn't accurate, if the preference is empty, it's the actual date to use that's empty, not the period, which means the rest of the code wouldn't work.
Will merge momentarily.
Fixes #7647 In CronArchive:: loadCustomDateRangeToPreProcess, fix variable in for loop overwriting itself, and do not execute archiving of custom date ranges for user if the PREFERENCE_DEFAULT_REPORT_DATE preference is for some reason not set.
Fixes #7647
The original issue also mentioned:
Not able to reproduce but just in case I have added some code to handle that case.