core icon indicating copy to clipboard operation
core copied to clipboard

[QA] appswitcher does not always save a user based defaultapp

Open jnweiger opened this issue 1 year ago • 11 comments

Seen while testing https://github.com/owncloud/core/pull/39600 with web 5.7.0-rc.12 on core 10.11.0-rc.1

  • occ user:setting admin core defaultapp --value activity
  • log in user admin via web ui. -> he sees the activity app. OK
  • switch to the classic Files view, log out.
  • Check the output of occ user:setting admin core defaultapp - it still reports activity. BAD.
  • login user admin, he still gets the activity app. BAD.
  • switch to New design, log out.
  • Check the output of occ user:setting admin core defaultapp - it still reports activity. BAD.
  • login user admin, he still gets the activity app. BAD.

Expected behaviour:

  • web and files store themeselves as defaultapp in the user settings.
  • Or https://github.com/owncloud/core/issues/40346

Switching the appdefault to files or web via occ command works as expected, but that is not sufficient for the intended user story https://github.com/owncloud/web/issues/5610

jnweiger avatar Sep 08 '22 16:09 jnweiger

It does exactly what I would expect. You set it to activity, it routes there.

Why should this change when logging out?

micbar avatar Sep 08 '22 16:09 micbar

@micbar https://github.com/owncloud/web/pull/6173

jnweiger avatar Sep 08 '22 16:09 jnweiger

So, as far as I know the concept of this feature evolved in two steps:

  1. product decision: we'll allow users to choose their default app (i.e. the app that they see upon login).
  2. product decision: nope, we won't.

Since it seemed particularly important that we let users decide which web ui they want to see by default, https://github.com/owncloud/web/pull/6173 has been developed. That PR makes sure that when you switch back and forth between the classic and the new web ui, oc10 memorizes (via the user setting mentioned above) which web ui was used last and serves that upon next login. See https://github.com/owncloud/web/pull/6173#issuecomment-998589292 for reference.

In the future this could be extended to letting users decide that e.g. the activity app is their default after login. In any case the whole idea is that the user setting survives the user session, i.e. upon relogin the default app chosen by the user gets served.

The fact that you're able to set that default app via CLI is not representative for a typical user in the current state of the feature. :-)

BUT: I found something that I wasn't expecting either. The way the web app works right now is that it sets the core.defaultapp to web when the new design is visited, and removes the core.defaultapp again if you switch back to the classic ui via the classic design item in the application switcher. Apparently this is conflicting with the defaultapp setting in config.php. When you set defaultapp to web in the config.php then the user setting is effectively useless, because if the user switches to the classic design (which removes their user setting) there is no user setting to overrule the defaultapp from config.php -> defaultapp from config.php is served. This feature only works as expected if the defaultapp in config.php is NOT web.

kulmann avatar Sep 08 '22 20:09 kulmann

Thanks for the explanation!

The points, where my expectation was wrong, are

  • when choosing web, it only sets the user's core.defaultapp to web, when it was previously unset.
  • by setting it to activity - I effectively blocked the remembering mechanism.
  • A user's choice (via appswitcher) would always override the system defaultapp set in config.php .

I'd now say, a) the web appswitcher should set the user's defaultapp to files, when the user switches to classic UI. That would fix the third item. b) The other two items need documentation to set the expectation right.

jnweiger avatar Sep 08 '22 21:09 jnweiger

a) the web appswitcher should set the user's defaultapp to files, when the user switches to classic UI. That would fix the third item.

We can implement that easily. However, that means that a potential system wide setting will never apply to users once they have set it on a user basis.

@jnweiger @pmaier1 Should be implement it that way?

JammingBen avatar Sep 09 '22 06:09 JammingBen

~~fyi that'd be a change in core, not in web.~~ sorry, my bad. It's in web. We'll include the change in web v5.7.0 final.

kulmann avatar Sep 09 '22 06:09 kulmann

With today's v5.7.0 build, the appswitcher behaves like this:

  • user admin authorizes web, nothing is written to occ user:setting admin core defaultapp - Okayish
  • when he switches to 'Classic Design', occ user:setting admin core defaultapp reports files - OK
  • when he switches back to 'New Design', nothing changes, it still saysfiles - BAD

  • occ user:setting admin core defaultapp --delete
  • user admin uses the appswitcher in web to switch to activity, nothing is saved. Okayish
  • same when he switches to market, or opens settings. He is basically back in the clasic design, but nothing was saved. Difficult...
  • when he then switches (within the classic design) to files. nothing is saved. BAD
  • when he switches to New Design again, nothing is saved. BAD.

Expected behaviour:

  • When entering Ẁeb, web is saved into defaultapp. (that worked in rc12)
  • When exiting Web through any way, files is saved into defaultapp. (works only when exiting directly through the appswitcher, did not work at all in rc12)

jnweiger avatar Sep 09 '22 15:09 jnweiger

The current implementation is supposed to memorize the user choice of going to the New Design or Classic Design, nothing else. Other nav items are not supposed to be memorized. The fact that e.g. the Activity app is listed in the app switcher of ownCloud Web is just supposed to mitigate that web doesn't have its own implementation of the activity app (and oc10 activity doesn't have an API to expose activity data anyway). If you then switch back to the classic UI is nothing we can influence in ownCloud Web. If that's supposed to memorize the files app as user chosen defaultapp, then that has to happen in core. I doubt that that's necessary, but the decision is up to product management @pmaier1

With today's v5.7.0 build, the appswitcher behaves like this:

  • user admin authorizes web, nothing is written to occ user:setting admin core defaultapp - Okayish
  • when he switches to 'Classic Design', occ user:setting admin core defaultapp reports files - OK
  • when he switches back to 'New Design', nothing changes, it still saysfiles - BAD

Works on my test setup. What I saw in the implementation though is that the url to the classic design (to be configured in config.json) has to end in /apps/files. Please check if you configured it as /apps/files/, i.e. with a trailing slash. If yes then please try without trailing slash. If you can confirm this I can make a patch release that allows /apps/files/ as well. Or we document well that the url has to end without a trailing slash. cc @pmaier1

  • occ user:setting admin core defaultapp --delete
  • user admin uses the appswitcher in web to switch to activity, nothing is saved. Okayish
  • same when he switches to market, or opens settings. He is basically back in the clasic design, but nothing was saved. Difficult...
  • when he then switches (within the classic design) to files. nothing is saved. BAD

As described above, this is not expected behaviour and can anyway not be implemented in web.

  • when he switches to New Design again, nothing is saved. BAD.

Please see above, might be an issue with the configured URL.

  • When exiting Web through any way, files is saved into defaultapp. (works only when exiting directly through the appswitcher, did not work at all in rc12)

That is not expected behaviour. If it is it needs changes in core and we can remove code some from web again. cc @pmaier1

kulmann avatar Sep 12 '22 09:09 kulmann

Expected behavior

  • When a user switches from Classic to Web using the "New Design" nav item, that choice is stored and remembered (= web)
  • When a user switches from Web to Classic using the "Classic Design" nav item, that choice is stored and remembered (= files)

pmaier1 avatar Sep 12 '22 11:09 pmaier1

Ok. I am okay for now to keep the scope small. only remember files and web, and no other apps. I fully understand, that a complete implementation cannot be implemented in web alone.

Let's focus on the problem: "... switches back to 'New Design', nothing changes, it still says files."

My config.json has

"applications" : [
    {
      "title": {
        "en": "Classic Design",
        "de": "Klassisches Design",
        "fr": "Design classique",
        "zh_CN": "文件"
      },
      "icon": "swap-box",
      "url": "https://oc10110rc1-web-app-570-20220909.jw-qa.owncloud.works/index.php/apps/files"
    },
    {
      "icon": "settings-4",
      "menu": "user",
      "target": "_self",
      "title": {
        "de": "Einstellungen",
        "en": "Settings"
      },
      "url": "https://oc10110rc1-web-app-570-20220909.jw-qa.owncloud.works/index.php/settings/personal"
    }
  ]

No trailing slash that I could remove. Sorry. In config.php, I have these web related entries:

  'upgrade.disable-web' => 'false',
  'web.baseUrl' => 'https://oc10110rc1-web-app-570-20220909.jw-qa.owncloud.works/index.php/apps/web',
  'web.rewriteLinks' => true,
  'defaultapp' => 'web',

jnweiger avatar Sep 12 '22 11:09 jnweiger

Having a look into the browser network tab, I see that when switching from classic to new design the request to set the default app gets cancelled. @JammingBen could you look into this? (needs preserve log being checked in the network tab options).

kulmann avatar Sep 12 '22 15:09 kulmann

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Mar 12 '23 01:03 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Sep 09 '23 01:09 github-actions[bot]

This issue has been automatically closed.

github-actions[bot] avatar Sep 19 '23 01:09 github-actions[bot]