-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(settings): migrate AppAPI ExApps management to settings #48665
base: master
Are you sure you want to change the base?
Conversation
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.
Checked, the functionality works.
Note for future reviewers: a backport of this pull request will be included in version 30.0.2, the merge is planned to be right after the release of 30.0.1, so that everyone has enough time to test.
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.
Nothing very critical but a lot of stuff summing up. See comments.
$defaultDaemonConfig = null; | ||
|
||
if ($appApiEnabled) { | ||
$exAppFetcher = Server::get(\OCA\AppAPI\Fetcher\ExAppFetcher::class); |
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.
This probably should be part of server somewhen^^
@provokateurin do you think we can have stubs here or will have to fill up the baseline?
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.
AppAPI is shipped since 30.0.1
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.
How can this happen? I was pretty sure the license is a required field in appinfo.xml thus the appstore should deny uploading such an app? |
It's a Test Deploy app, or any other |
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.
Thank you! Some more comments, then it should be fine.
Signed-off-by: Andrey Borysenko <[email protected]> # Conflicts: # apps/settings/lib/Controller/AppSettingsController.php
Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
…s bundled Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: Andrey Borysenko <[email protected]>
55f212e
to
1ef44de
Compare
Summary
This PR migrates the UI for ExApp management to default Apps management page (settings app).
On backend side in settings app there is only addition for initial state for ExApps UI.
All ExApp related logic is in separate store that uses backend API part of AppAPI.
Also small fix for markdown render.
TODO
Checklist