make knowledge base url configurable

[schiessle]

An example how #17959 could be solved.

cc @CaptionF @jancborchardt @cmonteroluque @MTRichards

[jancborchardt]

To be in line with the other variables, shouldn’t it be $themeKnowledgeBaseUrl?

Also, might be simpler to say themeHelpUrl.

[schiessle]

sure... just copy&pasted it from default.php, will update it once Jenkins finished

[karlitschek]

👍

[karlitschek]

@schiesbn cloud we also make it a config.php option? would make stuff easier.
We also need to backport this. (unfortunately)

[schiessle]

@karlitschek We could also implement it as a config.php option, sure. But I think this is the cleaner solution. Even if we want to get this in early, I think we should still try to have sustainable solution. I think it would be quite confusing if some stuff would be set in config.php and other (quite similar) stuff in the theme.

This changes are still small and quite isolated, so I think the risk that the backport breaks something isn't really high.

[karlitschek]

@schiesbn Sure. Both work for me. @MTRichards what do you think?

[MTRichards]

My only concern with making this part of the theme is that it is relatively complicated (setting up a theme) if all you want to do is change the URL for the help menu. I do have this as a feature request from another customer as well, though they do use the theme approach.

What else is configurable as a theme like this?

[ghost]

🚀 Test PASSed.🚀

[schiessle]

@MTRichards as you can see at the following link a lot of URLs to documentation, sync clients, etc. are defined in the theme as well as ownCloud name, slogan, mail footers,... https://github.com/owncloud/core/pull/17961/files#diff-66338aea9c5877c3368c5c7b0ed484ebR36 That's why I think that this would be the logical place for the knowledge url.

The customer doesn't need to create a complete new theme if he want to change the knowledge url. He just need to edit the defaults.php from the theme he already use and set the baseKnowledgeUrl like done here: https://github.com/owncloud/core/blob/make_knowledgebase_configurable/themes/example/defaults.php#L43

[scrutinizer-notifier]

A new inspection was created.

[MTRichards]

Sold! Thanks for explaining Bjoern.
Of course, we need to document it really well. @carlaschroder

[ghost]

🚀 Test PASSed.🚀

[schiessle]

second reviewer needed... @LukasReschke @MorrisJobke @PVince81 maybe? Thanks!

[LukasReschke]

👍

[RobinMcCorkell]

ERM, this may have just broken some apps. Because URLGenerator is instantiated in OC_Defaults, and it requires a Router, app routes are now loaded before the actual app is loaded... This caused an issue in files_external where appinfo/app.php registers some CLASSPATHs (yeah yeah I know, it's legacy code ok?), which are needed in appinfo/routes.php when a route is registered. Since they aren't available at that point, errors start getting thrown.

Note that this only occurred on my WIP refactor of files_external, I'm not sure if there are any apps that will display these issues. But it's still bad that the app routes are getting loaded before the app itself.

Relevant backtrace from when I noticed it (was unit testing at the time):

PHP  10. OC::init() /srv/local/owncloud/core/lib/base.php:1125
PHP  11. OC_Util::checkServer() /srv/local/owncloud/core/lib/base.php:608
PHP  12. OC_Defaults->__construct() /srv/local/owncloud/core/lib/private/util.php:571
PHP  13. OC\URLGenerator->linkToRoute() /srv/local/owncloud/core/lib/private/defaults.php:69
PHP  14. OC\Route\CachingRouter->generate() /srv/local/owncloud/core/lib/private/urlgenerator.php:65
PHP  15. OC\Route\Router->generate() /srv/local/owncloud/core/lib/private/route/cachingrouter.php:53
PHP  16. OC\Route\Router->loadRoutes() /srv/local/owncloud/core/lib/private/route/router.php:305
PHP  17. OC\Route\Router->requireRouteFile() /srv/local/owncloud/core/lib/private/route/router.php:155
PHP  18. include_once() /srv/local/owncloud/core/lib/private/route/router.php:315

[PVince81]

@Xenopathic can you raise a regression ticket so this doesn't get lost ? Thanks.