Skip to content

Allow the default photos location to be set server-wide #3796

Description

@bakiburakogun

Describe the feature you'd like

Let an administrator change the default value of photosLocation and photosSourceFolders for the whole server, so a deployment that does not run in English can ship a localized folder name to new accounts.

Why

On a server whose users all work in one non-English language, /Photos is the one folder name they cannot read. Every other folder they see is in their own language: the skeleton folders are localized, the Talk attachment folder can be renamed by an administrator, file names are theirs. /Photos stands out as the only English word in their file list, and it is also the folder the Photos app silently creates for them.

We run a Turkish-language deployment with several thousand accounts. We would like new accounts to get /Fotoğraflar instead.

What is possible today

UserConfigService::getConfigForUser() resolves the default from a class constant:

public const array DEFAULT_CONFIGS = [
	'croppedLayout' => 'false',
	'gridDensity' => 'medium',
	'photosLocation' => '/Photos',
	'photosSourceFolders' => '["/Photos"]',
];

...

$default = self::DEFAULT_CONFIGS[$key];
$value = $this->userConfig->getValueString($userId, Application::APP_ID, $key, $default);

Only the per-user value can be overridden, with occ user:setting <uid> photos photosLocation /Fotoğraflar. That covers existing accounts if you loop over every user, but it does nothing for accounts created afterwards, which keep getting /Photos from the constant. There is no app config anywhere in this path, so occ config:app:set photos photosLocation writes a value that nothing reads.

That leaves patching the app as the only way to change it for new accounts, which we would rather not carry across updates.

Prior art in the same ecosystem

Talk already solves exactly this, in spreed/lib/Config.php:

public function getAttachmentFolder(string $userId): string {
	$defaultAttachmentFolder = $this->config->getAppValue('spreed', 'default_attachment_folder', '/Talk');
	return $this->config->getUserValue($userId, 'spreed', UserPreference::ATTACHMENT_FOLDER, $defaultAttachmentFolder);
}

An app-level default sits behind the per-user value, so occ config:app:set spreed default_attachment_folder --value="/Kırlangıç" changes what new users get without touching anyone who has already chosen a folder.

Suggested implementation

The same shape fits UserConfigService and covers every key in DEFAULT_CONFIGS at once, not just the folder ones:

 	public function __construct(
+		private readonly IAppConfig $appConfig,
 		private readonly IUserConfig $userConfig,
 		private readonly IUserSession $userSession,
 	) {
 	}
 
 	public function getConfigForUser(string $userId, string $key): string {
 		if (!in_array($key, array_keys(self::DEFAULT_CONFIGS))) {
 			throw new Exception('Unknown user config key');
 		}
 
-		$default = self::DEFAULT_CONFIGS[$key];
+		$default = $this->appConfig->getValueString(Application::APP_ID, 'default_' . $key, self::DEFAULT_CONFIGS[$key]);
 		$value = $this->userConfig->getValueString($userId, Application::APP_ID, $key, $default);
 
 		return $value;
 	}

Nothing changes for a server that sets no app config, and a user who has picked their own folder still wins over the administrator's default.

I am happy to open a pull request with this if the approach looks right to you. If you would rather keep the app config narrowed to the two folder keys, or name them differently, say so and I will follow that instead.

A command might be worth more than the config alone

The app config above changes what new accounts get, but it leaves the harder half to the administrator. On an existing server, accounts that never opened the Photos settings have no stored value, so they follow the default — and moving the default out from under them is exactly what an administrator does not want. Pinning the old value for every such account first is possible with occ user:setting in a loop, but on a server with thousands of accounts that is a slow loop and an easy step to get wrong.

An occ command could carry both halves together:

occ photos:default-location /Fotoğraflar --pin-existing

that is, set the app default and write the previous value for accounts that do not have one, so the rename only reaches accounts created afterwards. That is not something occ config:app:set can do, and it is the part an administrator actually has to think about.

We would be glad to see just the app config land, and would use it as it is. The command is a suggestion for what would make the feature complete rather than a condition.

One question

On the pinning above: is that the right way round, or does the app already handle the case somewhere we have not found? We would rather follow an existing pattern than invent one.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions