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

Add user-configurable defaultapp_path (fixes broken external defaultapp) #28962

Closed
wants to merge 1 commit into from
Closed

Add user-configurable defaultapp_path (fixes broken external defaultapp) #28962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config/config.sample.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,14 @@
*/
'defaultapp' => 'files',

/**
* Set the default app path to open on login. For example if the default app is
* "external", set this to /1/ to default to the first External Site. You can
* use a comma-separated list of paths to match up with the apps in defaultapp.
* e.g. if defaultapp='external,files' defaultapp_path='/1/,/?dir=accounting'
*/
'defaultapp_path' => '',
Copy link
Contributor

Choose a reason for hiding this comment

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

since this is a PHP file, you could even use a PHP array directly like it's done for other settings

Copy link
Author

Choose a reason for hiding this comment

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

Cool, I wasn't sure if these options could be set by environment variable or other config file so I assumed they had to be strings, but an array is definitely better.


/**
* ``true`` enables the Help menu item in the user menu (top right of the
* ownCloud Web interface). ``false`` removes the Help item.
Expand Down
26 changes: 20 additions & 6 deletions lib/private/legacy/util.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* @author Marvin Thomas Rabe <[email protected]>
* @author Michael Gapczynski <[email protected]>
* @author Morris Jobke <[email protected]>
* @author Nick Sweeting <[email protected]>
* @author Philipp Schaffrath <[email protected]>
* @author Robin Appelman <[email protected]>
* @author Robin McCorkell <[email protected]>
Expand Down Expand Up @@ -1089,20 +1090,33 @@ public static function getDefaultPageUrl() {
$location = $urlGenerator->getAbsoluteURL($defaultPage);
} else {
$appId = 'files';
$defaultApps = explode(',', \OCP\Config::getSystemValue('defaultapp', 'files'));
$appPath = '/';
$defaultApps = explode(',', \OCP\Config::getSystemValue('defaultapp', $appId));
$defaultAppPaths = explode(',', \OCP\Config::getSystemValue('defaultapp_path', $appPath));

// find the first app that is enabled for the current user
foreach ($defaultApps as $defaultApp) {
foreach ($defaultApps as $idx=>$defaultApp) {
$defaultApp = OC_App::cleanAppId(strip_tags($defaultApp));
// if app has a corresponding path, use it, otherwise default to /
if (isset($defaultAppPaths[$idx]) && $defaultAppPaths[$idx] !== '') {
$defaultAppPath = strip_tags($defaultAppPaths[$idx]);
} else {
$defaultAppPath = '/';
}
if (static::getAppManager()->isEnabledForUser($defaultApp)) {
$appId = $defaultApp;
$appPath = $defaultAppPath;
break;
}
}

if(getenv('front_controller_active') === 'true') {
$location = $urlGenerator->getAbsoluteURL('/apps/' . $appId . '/');
if ($appId === 'external' && $appPath === '/') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Core should not know about any external apps, please remove.

I suggest that you instead improve the external sites app to have a special route that accepts indices so that when it's opened in the browser, it will redirect to the external site page.

// correct external/ to external/1/ if custom path wasn't specified
$appPath = '/1/';
}
if (getenv('front_controller_active') === 'true') {
$location = $urlGenerator->getAbsoluteURL('/apps/' . $appId . $appPath);
} else {
$location = $urlGenerator->getAbsoluteURL('/index.php/apps/' . $appId . '/');
$location = $urlGenerator->getAbsoluteURL('/index.php/apps/' . $appId . $appPath);
}
}
}
Expand Down
43 changes: 38 additions & 5 deletions tests/lib/UtilTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ public function dataProviderForTestIsSharingDisabledForUser() {
*
* @dataProvider defaultAppsProvider
*/
function testDefaultApps($defaultAppConfig, $expectedPath, $enabledApps) {
function testDefaultApps($defaultAppConfig, $defaultAppPathConfig, $expectedPath, $enabledApps) {
$oldDefaultApps = \OCP\Config::getSystemValue('defaultapp', '');
// CLI is doing messy stuff with the webroot, so need to work it around
$oldWebRoot = \OC::$WEBROOT;
Expand All @@ -316,6 +316,7 @@ function testDefaultApps($defaultAppConfig, $expectedPath, $enabledApps) {
// need to set a user id to make sure enabled apps are read from cache
\OC_User::setUserId($this->getUniqueID());
\OCP\Config::setSystemValue('defaultapp', $defaultAppConfig);
\OCP\Config::setSystemValue('defaultapp_path', $defaultAppPathConfig);
$this->assertEquals('http://localhost/' . $expectedPath, Dummy_OC_Util::getDefaultPageUrl());

// restore old state
Expand All @@ -326,30 +327,62 @@ function testDefaultApps($defaultAppConfig, $expectedPath, $enabledApps) {

function defaultAppsProvider() {
return [
// none specified, default to files
// neither app nor path specified, default to /files/
[
'',
'',
'index.php/apps/files/',
['files'],
],
// unexisting or inaccessible app specified, default to files
// non-existant or inaccessible app specified, default to /files/
[
'unexist',
'',
'index.php/apps/files/',
['files'],
],
// non-standard app
// non-standard app with no path specified
[
'calendar',
'',
'index.php/apps/calendar/',
['files', 'calendar'],
],
// non-standard app with fallback
// non-standard app with fallback and no paths specified
[
'contacts,calendar',
'',
'index.php/apps/calendar/',
['files', 'calendar'],
],
// files app with manual app path specified
[
'files',
'/?dir=/testfolder',
'index.php/apps/files/?dir=/testfolder',
['files'],
],
// external app with manual app path specified
[
'external',
'/3/',
'index.php/apps/external/3/',
['external'],
],
// external app with path / corrected to /1/ automatically
[
'external',
'/',
'index.php/apps/external/1/',
['external'],
],
// non-standard app with fallback and custom paths specified
[
'contacts,calendar',
'/?view=1,/?view=2',
'index.php/apps/calendar/?view=2',
['files', 'calendar'],
],
];
}

Expand Down