Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(settings): migrate AppAPI ExApps management to settings #48665

Merged
merged 19 commits into from
Oct 29, 2024
Merged

feat(settings): migrate AppAPI ExApps management to settings #48665

merged 19 commits into from
Oct 29, 2024

Conversation

andrey18106
Copy link
Contributor

@andrey18106 andrey18106 commented Oct 11, 2024 •
edited
Loading

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.

Screen Shot 2024-10-14 at 17 01 48

TODO

Checklist

@andrey18106 andrey18106 mentioned this pull request Oct 11, 2024
Merged
2 tasks
bigcat88
bigcat88 approved these changes Oct 14, 2024
View reviewed changes
Copy link
Member

@bigcat88 bigcat88 left a comment •
edited
Loading

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.

susnux
susnux requested changes Oct 14, 2024
View reviewed changes
Copy link
Contributor

@susnux susnux left a 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.

apps/settings/lib/Controller/AppSettingsController.php Outdated Show resolved Hide resolved
apps/settings/lib/Controller/AppSettingsController.php Outdated Show resolved Hide resolved
apps/settings/src/components/AppList/AppItem.vue Outdated Show resolved Hide resolved
apps/settings/src/mixins/AppManagement.js Outdated Show resolved Hide resolved
apps/settings/src/mixins/AppManagement.js Outdated Show resolved Hide resolved
apps/settings/src/mixins/AppManagement.js Outdated Show resolved Hide resolved
apps/settings/src/mixins/AppManagement.js Outdated Show resolved Hide resolved
apps/settings/src/mixins/AppManagement.js Outdated Show resolved Hide resolved
apps/settings/src/store/app_api_apps.js Outdated Show resolved Hide resolved
apps/settings/src/store/app_api_apps.js Outdated Show resolved Hide resolved
@susnux susnux added this to the Nextcloud 31 milestone Oct 14, 2024
julien-nc
julien-nc reviewed Oct 14, 2024
View reviewed changes
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how likely it is to happen in a production environment but in my dev setup, some apps don't have a licence so when clicking on their list item, the sidebar does not open.
image

@susnux
Copy link
Contributor

susnux commented Oct 15, 2024

I'm not sure how likely it is to happen in a production environment but in my dev setup, some apps don't have a licence so when clicking on their list item, the sidebar does not open.

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?

@andrey18106
Copy link
Contributor Author

I'm not sure how likely it is to happen in a production environment but in my dev setup, some apps don't have a licence so when clicking on their list item, the sidebar does not open.

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 manual-install type ExApp, it has no appstore information, only short info from DB.

susnux
susnux reviewed Oct 23, 2024
View reviewed changes
Copy link
Contributor

@susnux susnux left a 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.

apps/settings/src/app-types.ts Outdated Show resolved Hide resolved
apps/settings/src/app-types.ts Outdated Show resolved Hide resolved
apps/settings/src/components/AppList/AppDaemonBadge.vue Outdated Show resolved Hide resolved
apps/settings/src/components/AppList/AppItem.vue Outdated Show resolved Hide resolved
apps/settings/src/components/AppList/AppItem.vue Outdated Show resolved Hide resolved
apps/settings/src/components/AppList/AppItem.vue Outdated Show resolved Hide resolved
apps/settings/src/components/AppList/AppItem.vue Outdated Show resolved Hide resolved
apps/settings/src/components/AppStoreSidebar/AppDetailsTab.vue Outdated Show resolved Hide resolved
apps/settings/src/components/AppStoreSidebar/AppDetailsTab.vue Outdated Show resolved Hide resolved
apps/settings/src/views/AppStoreSidebar.vue Outdated Show resolved Hide resolved
@andrey18106 andrey18106 force-pushed the feat/settings/app_api_apps_management branch from 55f212e to 1ef44de Compare October 23, 2024 20:49
susnux
susnux approved these changes Oct 24, 2024
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not test with external app but code looks sane, as we need to backport (there will be a refactor on master soon, so lets get this in first).

@andrey18106 andrey18106 force-pushed the feat/settings/app_api_apps_management branch from 56e3f45 to b0e3af3 Compare October 29, 2024 12:06
@andrey18106
Copy link
Contributor Author

/compile

@andrey18106 andrey18106 force-pushed the feat/settings/app_api_apps_management branch from a8cd19e to fe354d3 Compare October 29, 2024 13:23
@andrey18106
Copy link
Contributor Author

/compile

@andrey18106
feat(app_api): add initial state data for AppAPI UI part
95b0630 
Signed-off-by: Andrey Borysenko <[email protected]>

# Conflicts:
#	apps/settings/lib/Controller/AppSettingsController.php

# Conflicts:
#	apps/settings/lib/Controller/AppSettingsController.php
@andrey18106
feat(settings): migrate AppAPI ExApps management (1)
7e31088 
Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
WIP: add Daemon badge in app sidebar (2)
a5291c6 
Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
WIP: use global mutation, since app_api_apps is namespaced
4f109be 
Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
WIP: add missing state checks
70a8d26 
Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
WIP: minor fixes
2f25546 
Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
fix(ci): resolve eslint errors
46e643b 
Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
fix(ci): suppress UndefinedClass for AppAPI related classes since it'…
3a840c1 
…s bundled

Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
WIP: address review comments, small fixes
e4d5866 
Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
WIP: migrate to Pinia, minor fixes
c8b35fa 
Signed-off-by: Andrey Borysenko <[email protected]>
@andrey18106
WIP: remove app_api_apps vuex store
6e75b41 
Signed-off-by: Andrey Borysenko <[email protected]>
andrey18106 and others added 7 commits October 29, 2024 20:54
@andrey18106 andrey18106 force-pushed the feat/settings/app_api_apps_management branch from 99bf58e to be23e7d Compare October 29, 2024 18:54
@andrey18106
Copy link
Contributor Author

/compile

@andrey18106
Copy link
Contributor Author

/backport to stable30

@andrey18106 andrey18106 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 29, 2024
@nextcloud-command
chore(assets): Recompile assets
c2f1c3d 
Signed-off-by: nextcloud-command <[email protected]>
@andrey18106 andrey18106 merged commit d266779 into master Oct 29, 2024
177 checks passed
@andrey18106 andrey18106 deleted the feat/settings/app_api_apps_management branch October 29, 2024 19:24
@backportbot backportbot bot mentioned this pull request Oct 29, 2024
Merged
2 tasks
andrey18106 added a commit to nextcloud/app_api that referenced this pull request Oct 29, 2024
@andrey18106 @nextcloud-command
chore: sync changes for Apps management migration to settings (#417)
b8200b7 
This PR synchronizes backend logic for ExAppFetcher and required changes
for Apps management.

![Screen Shot 2024-10-14 at 17 01
48](https://github.com/user-attachments/assets/60667041-92b1-4d9e-a78a-9476a1271d1f)

## TODO

- [ ] Merge with the server PR for settings app changes:
nextcloud/server#48665
- [x] Remove old UI parts that are not needed anymore

---------

Signed-off-by: Andrey Borysenko <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
Co-authored-by: nextcloud-command <[email protected]>
@skjnldsv skjnldsv mentioned this pull request Nov 19, 2024
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@provokateurin provokateurin provokateurin left review comments

@julien-nc julien-nc julien-nc left review comments

@bigcat88 bigcat88 bigcat88 approved these changes

@susnux susnux susnux approved these changes

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: apps management
Projects
None yet
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.
6 participants
@andrey18106 @susnux @julien-nc @bigcat88 @provokateurin @nextcloud-command