-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
[stable30] fix(Apps): fix install command check on existing apps #55065
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.
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
[stable30] fix(Apps): fix install command check on existing apps #55065
Conversation
|
otoh not sure whether this approach might cause other side effects, as UPDATE: |
- AppManager::isInstalled() is misleading, as it checks only whether it is enabled. But an app might not be present in some edge cases. - AppManager::getAppPath() does however only check whether an app dir is present, independent of the enabled-state. Signed-off-by: Arthur Schiwon <[email protected]>
8e06240 to
ac6653e
Compare
- includes nextcloud/server#55065 Signed-off-by: Arthur Schiwon <[email protected]>
- includes nextcloud/server#55065 Signed-off-by: Arthur Schiwon <[email protected]>
come-nc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not a good change.
To install an application manually, you put it in the apps folder and then run app:install, no? With this change it would not be possible anymore?
No, you run |
Summary
Currently, enabled apps are returned, whether they are installed or not. This is a behavioral breaking change compared to 29.
Based this now on 30 for nextcloud/univention-app#208, and I am not sure this is what we really want in server (cf. my comment at https://github.com/nextcloud/server/pull/49648/files#r2341962849).
If so, I'd do a new PR based on master, which has diverges further.
Checklist