Run repair steps on uninstall in all cases

[LEDfan]

Fixes #5832

Maybe it's a good idea to mark \OC_app::disable as deprecated?

[mention-bot]

@LEDfan, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bartv2, @icewind1991 and @blizzz to be potential reviewers.

[LEDfan]

BTW it looks like the \OC_app::enable method actually also installs and downloads the app, maybe it's a good idea to move this code a downloadAndInstallApp method inside the appmanager and then call this method inside the enable method command? (

server/core/Command/App/Enable.php

Line 78 in 8d83e23

$appClass = new \OC_App();

)

And another interesting thing is that the installApp method of OC_App is not used according to PHPstorm and grep.

[codecov]

Codecov Report

Merging #5833 into master will increase coverage by 0.07%.
The diff coverage is 50%.

@@             Coverage Diff              @@
##             master    #5833      +/-   ##
============================================
+ Coverage     51.81%   51.88%   +0.07%     
+ Complexity    25354    25352       -2     
============================================
  Files          1606     1606              
  Lines         95077    95070       -7     
  Branches       1378     1378              
============================================
+ Hits          49260    49325      +65     
+ Misses        45817    45745      -72

Impacted Files	Coverage Δ	Complexity Δ
lib/base.php	2.15% <0%> (ø)	168 <0> (ø)	⬇️
lib/private/Updater.php	5.11% <0%> (ø)	117 <0> (ø)	⬇️
lib/private/legacy/app.php	58.87% <0%> (+0.98%)	195 <0> (-3)	⬇️
settings/ajax/disableapp.php	0% <0%> (ø)	0 <0> (ø)	⬇️
lib/private/App/AppManager.php	96.2% <100%> (+0.09%)	60 <0> (+1)	⬆️
apps/files_trashbin/lib/Trashbin.php	72.46% <0%> (-0.25%)	136% <0%> (ø)
lib/private/Files/ObjectStore/SwiftFactory.php	52.87% <0%> (+52.87%)	35% <0%> (ø)	⬇️
lib/private/Files/ObjectStore/Swift.php	75% <0%> (+75%)	8% <0%> (ø)	⬇️

[juliusknorr]

Maybe it's a good idea to mark \OC_app::disable as deprecated?

Yes, let's get rid of that. Can you add that and check for other usages in the server code?

BTW it looks like the \OC_app::enable method actually also installs and downloads the app, maybe it's a good idea to move this code a downloadAndInstallApp method inside the appmanager and then call this method inside the enable method command?

Maybe that is more something for the Installer class. We should keep the install code separated from the app management.

And another interesting thing is that the installApp method of OC_App is not used according to PHPstorm and grep.

Good catch, although it has already been removed by a68895e 😉

[LEDfan]

Yes, let's get rid of that. Can you add that and check for other usages in the server code?

Done, I removed the method because it's a private API.

I also removed the $enableAppsCache because it wasn't used anymore.

[MorrisJobke]

I also removed the $enableAppsCache because it wasn't used anymore.

👍

Yes, let's get rid of that. Can you add that and check for other usages in the server code?

Looks good and I also didn't found any other apps using it.

[MorrisJobke]

Let me rebase and review this one here.

[MorrisJobke]

Tested and works 👍

[MorrisJobke]

@nickvergessen @rullzer Please review as this is some crucial cleanup.

[MorrisJobke]

I'm also fine with removing this, because I couldn't find any reference to this in any app or in the server itself. Also there is a new proper event dispatched just two lines below.

[nickvergessen]

Don't see any issues and +1 for removing the trap (calling appmanager directly skips uninstall repair steps)