Add default app setting on a user basis

[JammingBen]

Description

This change allows users to to have their own default app defined. The setting is stored in the user preference table and uses its existing API. It will overwrite the system-wide defaultapp setting if set.

Related Issue

• Related https://github.com/owncloud/enterprise/issues/4694

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised: Default app setting on a user basis docs#4508
• Changelog item, see TEMPLATE

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

[jnweiger]

This one is too late for 10.9.0 now. We are in feature freeze. Sorry.
Keep it targeted to master.

[phil-davis]

Tested by:

• have users alice and brian on the system

• enable activity and notes apps

• set 'defaultapp' => 'activity' in config.php (system-wide default app)

• php occ user:setting brian core defaultapp --value=notes

• login as brian

• the Notes app is displayed - good

• login as alice

• the Activity app is displayed - good

• remove defaultapp from config.php

• login as brian

• the Notes app is displayed - good

• login as alice

• the Files app is displayed - good

Works

[jvillafanez]

Taking into account that it won't be in 10.9, I think the users should be able to set the app from the web ui, in the personal settings.
Unless I'm missing something, it seems lacking in terms of UX. I mean, it's a per-user setting but the user can't change it in any way.

[AlexAndBear]

@jvillafanez
No, this is not the plan right now. There will be a setting in ownCloud/web where you can toggle the new design off and on.

[mmattel]

Reading this I think it is not the right approach (while implementation is relatively easy). Having a setting on a per user basis makes management quite complicated. Imho it would be better to do this at a group level.
This makes management from an admin pov much easier. Just imagine to find users who have a particular setting set or move users from one setting to another. Because it is now not in 10.9, it offers a re-thinking about the best approach.

[AlexAndBear]

@mmattel
Please decouple your mind from
Business logic, API and UI.
Don't be to harsh in your judgements.
See: #39600 (comment) and https://github.com/owncloud/enterprise/issues/4694

Anyways this is not a admin setting, it is a user setting, done by the user (UI changes incoming).

It might be docs relevant, but for this setting, we usually don't want anyone to use the occ command.
Therefore an API and UI implementation is planned.

[mmattel]

Sorry if my words sounded harsh, not intended to be so... 😢
I just wanted to highlight, if based on an occ command, a user setting approach will be more challenging for an admin to administrate then a group based approach. If it is planned to have that on a user-UI base, it is a different story.

...for this setting, we usually don't want anyone to use the occ command...

This is a very dangerous path if you create/extend an occ command and then have in mind not using it.

[AlexAndBear]

@mmattel Got you, but this is not a setting the admin want to take care of!

Me as a individual want to decide if I want to use the new design aka owncloud/web or not.
Therefore we use the settings API, like we always did before (business logic).

This is a very dangerous path

See it from the correct perspective, it would be possible for the admin, but why should they do that.

The command was never extended! You can put every parameter you want there!

[phil-davis]

This is a very dangerous path if you create/extend an occ command and then have in mind not using it.

@mmattel the occ user:setting command already exists. That just provides an existing way for an admin to set various user-specific preferences if they want/need to do that at any time.

[JammingBen]

It's pretty much said already, to sum it up:

This PR just lays the groundwork for having this feature. It doesn't include UI (yet) as we currently plan to use this for the web app only. The UI for this will then be in the web app directly.

It's not intended for the admin to set default apps for other users. But because the setting sits in the user preferences, existing APIs such as occ user:setting can be used modify it. The doc issue I created might be misleading though, I'll adjust it 👍

[cdamken]

we need to add the option in the UI, it will be impossible for the admins to set and remove the option for 300K users

btw, which are all other opinions?

occ user:setting --value web admin core defaultapp

only web or remove the setting to go back?

[AlexAndBear]

Please read the description of the related issue!
Again: this is s USER setting!

Trying to explain this over and over again now...

[cdamken]

I read and that's why I asked the second part about the options

The first part means that I can't use this feature so far ;)

[AlexAndBear]

There will be a UI implementation, for the USER to switch between the new and classic design (UI), therefore we need this code.

I read and that's why I asked the second part about the options

It's not planned that you set options on your own.

The first part means that I can't use this feature so far ;)

No, you shouldn't

I removed the command that @JammingBen provided in the description. It seems to be misleading, we store user preferences in the table which can be manipulated via this command, but there are a lot of settings that will be set by the business logic itself and shouldn't by the user/admin.

[mmattel]

@janackermann we have all green - merging? (or are there more changes necessary?)

[phil-davis]

I rebased to get up-to-date CI. "someone" can decide what is to happen with this.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

[phil-davis]

The API for a user to set their email address (for example) is:

curl -X PUT http://aaaa:aaaa@172.17.0.1:8080/ocs/v1.php/cloud/users/aaaa \
    -d key="email" \
    -d value="aaaa@example.com"
<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>100</statuscode>
  <message/>
 </meta>
 <data/>
</ocs>

But if I try:

curl -X PUT http://aaaa:aaaa@172.17.0.1:8080/ocs/v1.php/cloud/users/aaaa \
>     -d key="defaultapp" \
>     -d value="activity"
<?xml version="1.0"?>
<ocs>
 <meta>
  <status>failure</status>
  <statuscode>997</statuscode>
  <message/>
 </meta>
 <data/>
</ocs>

After merging this PR, how can a client app call an API endpoint to set defaultapp ?

Without an API end-point that works, there won't be a way for clients (web, whatever other client) to actual implement this option.

[pmaier1]

merge?

[phil-davis]

Without an API end-point that works, there won't be a way for clients (web, whatever other client) to actual implement this option.

To me, it seems pointless to add this in a release, if there is no way to apply the setting other than with a "back end" occ command. I am hoping that someone can say how to use some API endpoint to set the default app for a user. But I don't know who "someone" is ;)

But it does no harm, so could be merged and then follow-up if an API endpoint needs to be adjusted.

[jvillafanez]

After merging this PR, how can a client app call an API endpoint to set defaultapp ?

Without an API end-point that works, there won't be a way for clients (web, whatever other client) to actual implement this option.

I don't think it's planned for now. The idea was to use it with the new web interface so the admin can change the interface for specific users manually.
Basically, the admin decides which users will have the new web interface by default so they can test it before making it available to all the users.

We should follow up this ticket at some point, but I'm ok with merging this.

[mmattel]

I think this is the key message which is relevant for the new webUI,
therefore this should go into the upcoming release:
Basically, the admin decides which users will have the new web interface by default so they can test it before making it available to all the users.
I will prepare a comment in the webui admin documentation.

[AlexAndBear]

We are mixing up different things here.

The idea was to use it with the new web interface so the admin can change the interface for specific users manually.
Basically, the admin decides which users will have the new web interface by default so they can test it before making it available to all the user

I don't know where this info is gathered, but it is basically wrong.

Please also check:
https://github.com/owncloud/web/pull/6173/files

@mmattel Please don't write up any documentation until we have the user setting somehow (SET BY THE USER, NOT BY THE ADMIN!)

[mmattel]

Post a discussion with @janackermann with good reasons, no doc entry will be made.

[pmaier1]

AFAIR, the idea was to save the state the user had the last time. So each time the user switches from Classic to Web, that would be persisted and vice versa. Basically not an intentional switch but the preference is decided based on the user choice.

Ref. owncloud/web#6173 (comment)