fix app_type check #29110
fix app_type check #29110
Conversation
|
@phisch why would it be necessary to get app types of apps that have never been enabled before ? From what I understand only enabled apps / theme apps are of interest here ? |
|
@PVince81 there is the possibility that a theme has never been enabled, for example if you install a fresh ownCloud with lets say the enterprise theme, the example theme and a custom one. Usually this would not be a problem, since there is no component in the code that needs a list of all apps, enabled or disabled, filtered by the type. The TemplateEditor does need such a list though. It needs to grab all apps that are of the type Theme. Currently the ThemeManager provides a function to return all Themes, and that fails with never enabled themes. (just doesn't return those themes) |
|
Why does the template editor need to list theme apps that were never enabled ? I don't think the template editor should let you edit templates from disabled apps anyway |
|
@ownclouders rebase |
…types are available, update app types when setting them
|
Hey! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost! |
|
Automated rebase was successful! |
ownclouders
force-pushed
the
app_types_never_enabled_apps
branch
from
30620b5
to
4ea736b
Compare
Codecov Report
@@ Coverage Diff @@
## master #29110 +/- ##
============================================
+ Coverage 62.15% 62.16% +<.01%
- Complexity 17516 17517 +1
============================================
Files 1045 1045
Lines 57740 57744 +4
============================================
+ Hits 35890 35895 +5
+ Misses 21850 21849 -1
Continue to review full report at Codecov.
|
|
@PVince81 we agreed that only enabled app-theme should be editable with Templateeditor. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Currently we have no way to get all apps of a certain type that includes apps that have never been enabled before.
When getting a list of all themes, the AppManager does only look into the appconfig table to get a list of apps. This means apps that have never been enabled will not get returned by the AppManager. That's why i am using the legacy
OC_App::getAllApps()which iterates over the fs.When checking if an app has a specific app_type, we only ever look into the appconfig table as well. That is why I included a check to also look into the info.xml whenever there is nothing in the appconfig table and also add the found types to the static list of app types, so they will be available through
self::getAppTypes($app).To be clear, this is a hack. It will add unreasonable fs reads, but it is necessary for the Templateeditor to work properly.
@DeepDiver1975 @PVince81 we should talk about a small refactoring of the app structure or how to extend the AppManager so even never enabled apps are properly available in the code.
Related Issue
owncloud/templateeditor#52
How Has This Been Tested?
manual only
Types of changes
Checklist: