Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Administration > Diagnostics > Config file lets Super User view all config values in the UI #9526

Merged
merged 4 commits into from Jan 18, 2016
Merged

Administration > Diagnostics > Config file lets Super User view all config values in the UI #9526

merged 4 commits into from Jan 18, 2016

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jan 14, 2016

  • We list all config values that are defined either in global, common or local ini
  • We list all config values that could be defined to overwrite a plugin system setting
  • We mark all config values that were changed eg in config.ini.php
  • It is possible to view only changed config values
  • We show default values from global.ini.php or from the plugin settings if we know the default value and if the config value is changed
  • We show the description of a config entry if one is available (comment in ini file or plugin setting description / inline help)
  • Available under Diagnostics for super users
  • We always hide database
  • We replace all config keys that contain the term password, salt, apikey, privatekey or secret with **** (I looked through Piwik / Piwik PRO plugins to see which kinda keys could contain too sensitive data)
  • We also replace system setting values that are of type password with ****
  • If development is not enabled we also hide more sections database_tests, debugtests and tests

This is especially useful for #9521 to see which plugin settings can be overwritten and which ones are defined. It also helps when we ask users to check their config etc.

@mattab can you change the ConfigFileIntroduction translation maybe?

@tsteur tsteur added the Needs Review PRs that need a code review label Jan 14, 2016
@tsteur tsteur added this to the 2.16.0 milestone Jan 14, 2016
@tsteur tsteur changed the title Add possibility to view config in the UI View config values in the UI Jan 14, 2016
sgiehl
sgiehl reviewed Jan 17, 2016
View reviewed changes
plugins/Diagnostics/Controller.php
* http://developer.piwik.org/guides/mvc-in-piwik or have a look at the our API references for controller and view:
* http://developer.piwik.org/api-reference/Piwik/Plugin/Controller and
* http://developer.piwik.org/api-reference/Piwik/View
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should remove the auto generated comments here and in the other files

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about that. It might make it easier for when any developers have a look at it and try to understand it or when they want to know more about it. I'm always not sure myself what to do. Sometimes I remove them, sometimes I don't :) Currently I tend to rather have them as it is kinda nice to have such documentation everywhere. Problem is when the links are outdated etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that what the Example plugins are ment for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general yep. It's more like if they want to fix something in that file or try to understand what that file does. Not for developing new plugins. I really don't have a strong opinion on it so happy with leaving them there or removing them. We could merge it now and later check all the files to have it consistent if we decide to remove them.

@mattab any opinion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsteur +1 to remove these comments to keep things simpler (but let's keep them in the example plugin template files used by the generators)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed it.

@sgiehl
Copy link
Member

sgiehl commented Jan 17, 2016

Besides the comments: LGTM

@tsteur
Copy link
Member Author

tsteur commented Jan 17, 2016

Made some changes based on the review

@sgiehl
Copy link
Member

sgiehl commented Jan 17, 2016

changes look fine 👍

tsteur added a commit that referenced this pull request Jan 18, 2016
@tsteur
Merge pull request #9526 from piwik/config_viewer
c74b8fb 
View config values in the UI
@tsteur tsteur merged commit c74b8fb into master Jan 18, 2016
@tsteur tsteur deleted the config_viewer branch January 18, 2016 00:55
@mattab mattab changed the title View config values in the UI Administration > Diagnostics > Config file lets Super User view all config values in the UI Jan 18, 2016
mattab added a commit that referenced this pull request Jan 18, 2016
@mattab
UI tests: Added new config file viewer #9526 + better visitor profile #…
0c1a438 
…9502
@mattab mattab added Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. labels Jan 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Needs Review PRs that need a code review
Projects
None yet
Milestone
2.16.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tsteur @sgiehl @mattab