Talk can not be set as default app on Nextcloud 28 using the app order

[susnux]

With Nextcloud 28 you can set a custom app order on a per-user basis, this also works with Talk.
But using that feature it is currently not possible to set Talk as the default app if sorted first, because Talk uses a closure to register its navigation entry instead of an appinfo entry.

To fix this talk needs to also provide the app key in the navigation entry ("app" => "spreed"):

spreed/lib/AppInfo/Application.php

Line 358 in 0cc7077

'id' => self::APP_ID,

[nickvergessen]

What's the difference between app and id?
Couldn't the code that fails fall back to id when app is missing?

[nickvergessen]

Also just for the record, Talk can be hidden for some users:
https://github.com/nextcloud/spreed/blob/a48b84fb1088702af623fb5ac23cad277783e0a2/lib/AppInfo/Application.php#L363C1-L363C1
Does the code that manages the default app fall back to another app then?

[susnux]

Does the code that manages the default app fall back to another app then?

Yes the app needs to be enabled for that user and in the closure case it must resolve.

[susnux]

What's the difference between app and id?
Couldn't the code that fails fall back to id when app is missing?

The ID can be arbitrary set to user defined values even for info.xml entries, or so it makes not much sense. For appinfo info.xml loaded navigation entries the app is guaranteed to be set correctly. So for consistent behavior navigation entry closures should also set the app key.

[nickvergessen]

So for consistent behavior navigation entry closures should also set the app key.

Well it's an API break/missing feature which could be solved in 99% of the cases with a fallback :) That's why I'm asking.

Yes the app needs to be enabled for that user and in the closure case it must resolve.

Well the closure does resolve, but it should not be loaded because the type is hidden.

---

Just tested and it causes a redirect loop. Will send a PR to server to fix this

[susnux]

I was pretty sure we check for type === 'link'

[susnux]

Well it's an API break/missing feature which could be solved in 99% of the cases with a fallback :) That's why I'm asking.

Well we could also check if ID is a valid app and use that as a fallback, seems a bit hacky but might work 99%.

[nickvergessen]

Just tested and it causes a redirect loop. Will send a PR to server to fix this

Okay seems to be impossible with the current code, because the getDefaultAppForUser is not going with the app-navigation manager, so the check if Talk can be used is never executed.

[nickvergessen]

I was pretty sure we check for type === 'link'

Well not in here:
https://github.com/nextcloud/server/blob/aa5f037af71c915424c6dcfd5ad2dc82797dc0d6/lib/private/App/AppManager.php#L834-L849
And that is what is used in the redirect respoonse