matomo-org / matomo

Empowering People Ethically with the leading open source alternative to Google Analytics that gives you full control over your data. Matomo lets you easily collect data from websites & apps and visualise this data and extract insights. Privacy is built-in. Liberating Web Analytics. Star us on Github? +1. And we love Pull Requests!
https://matomo.org/
GNU General Public License v3.0
19.87k stars 2.65k forks source link

System check reports incorrect results when enable_general_settings_admin = 0 #19099

Open DBX12 opened 2 years ago

DBX12 commented 2 years ago

Expected Behavior

System Check reports correct results no matter if enable_general_settings_admin is set to 1 or 0

Current Behavior

System Check reports Cron as not set up and suggests to disable Browser Archiving

#### Set up Cron (faster report-loading):
 ⚠ Warning: For optimal performance and a speedy Matomo, it is highly recommended to set up a crontab to automatically archive your reports, and to disable browser triggering in the Matomo settings. Learn more.

#### Browser and Auto-archiving enabled:
 ⚠ Warning: It looks like both browser and auto archiving are enabled. Auto archiving last started 40 min 26s ago. If auto archiving is enabled, you should disable browser archiving in "General Settings".

Steps to Reproduce (for Bugs)

  1. Configure auto archiving and disable Browser archiving in the UI
  2. Run system check and see that no issues regarding these two options are reported
  3. Set [General] enable_general_settings_admin=0 in config/config.ini.php
  4. Run system check again and see the two entries described in Current Behavior

Your Environment

bx80 commented 2 years ago

Hi @DBX12,

Setting [General] enable_general_settings_admin=0 in config/config.ini.php will completely prevent use of the browser archiving setting that was configured in the UI.

By default browser archiving is enabled, so to disable browser archiving when also using enable_general_settings_admin=0 you should also set enable_browser_archiving_triggering = 0 in the [General] section.

DBX12 commented 2 years ago

Hi bx80, I understand that the setting enable_general_settings_admin=0 will disable all settings in regard to browser archiving in the web UI. But if I have a cronjob set up, why would the system check report it as missing when I set enable_general_settings_admin=0? It seems like it is linked to enable_browser_archiving_triggering. Basically "if enable_browser_archiving_triggering == 0 assume there is a cronjob set up and don't mention it on the system check" and "if `enable_browser_archiving_triggering == 1 AND enable_general_settings_admin == 0 then complain about missing cronjob".

It seems a bit misleading to me, I could set enable_browser_archiving_triggering = 0 and not configure a cronjob and the system check would say all good, right?

bx80 commented 2 years ago

Hi @DBX12,

But if I have a cronjob set up, why would the system check report it as missing when I set enable_general_settings_admin=0?

I agree, this is misleading.

It's happening because enable_general_settings_admin=0 will completely disable the browser archiving option you set in the UI, it doesn't just disable the UI but also no longer checks the (separate) UI setting at all, so enable_browser_archiving_triggering reverts back to it's default config file value of 1.

The diagnostic check is fairly simplistic and now sees that you have both a cronjob setup and browser archiving enabled, this results in both the "setup cron" and "Browser and Auto-archiving enabled" warning being shown. The "Set up cronjob" warning shouldn't be really shown in this case as the wording implies that a cronjob isn't setup. The "Browser and Auto-archiving enabled" warning is also misleading as it suggested changing the disabled UI setting.

I could set enable_browser_archiving_triggering = 0 and not configure a cronjob and the system check would say all good, right?

From what I can see, there would be no explicit warnings shown, though the 'Set up cronjob' check would show that no cronjob was configured.

I'll tag this as an Usability improvement.

Suggested changes: