Fix for app:enable in case of not valid/not existing enterprise key

[pako81]

Description

This PR fixes the current behaviour of the app:enable occ command in case of not valid/not existing enterprise key.

Related Issue

• Fixes occ app:enable should exit with proper error message if enterprise key is invalid #36351

Motivation and Context

Currently, when running the app:enable occ command for enabling an enterprise app and the enterprise key is not valid/not existing, the command does not return any warning/error message. Still, the enterprise app stays disabled. Only when looking into the owncloud.log file one will see an error message about the invalid license key.

How Has This Been Tested?

Manually, by: - disabling an enterprise app, - setting an invalid enteprise key, - trying to re-enable that enterprise app. Now, the $appId cannot be enabled because of invalid enteprise key message is shown.

Implemented for both cases in case groups are/are not given as input to the command.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised
• Changelog item

[CLAassistant]

All committers have signed the CLA.

[phil-davis]

https://drone.owncloud.com/owncloud/core/21265/3/6

enabling apps
+ echo 'enabling apps'
+ php occ app:enable files_sharing
An unhandled exception has been thrown:
Error: Class 'OCA\Enterprise_Key\EnterpriseKey' not found in /drone/src/core/Command/App/Enable.php:74

It still needs to work for the community edition, when there is no enterprise_key app.

[pako81]

@phil-davis @micbar better now?

[phil-davis]

@micbar is this progressing?
(could have an acceptance test added if it is planned for release...)

[phil-davis]

@pako81 @micbar I rebased and fixed the code-style fail.
Let's see if the "real" CI passes...

code has been adjusted

[codecov]

Codecov Report

Merging #36399 into master will decrease coverage by <.01%.
The diff coverage is 37.5%.

@@             Coverage Diff              @@
##             master   #36399      +/-   ##
============================================
- Coverage     64.68%   64.68%   -0.01%     
- Complexity    19071    19074       +3     
============================================
  Files          1268     1268              
  Lines         74522    74528       +6     
  Branches       1312     1312              
============================================
+ Hits          48207    48209       +2     
- Misses        25930    25934       +4     
  Partials        385      385

Flag	Coverage Δ	Complexity Δ
#javascript	54.05% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	65.86% <37.5%> (-0.01%)	19074 <1> (+3)

Impacted Files	Coverage Δ	Complexity Δ
core/register_command.php	0% <0%> (ø)	0 <0> (ø)	⬇️
core/Command/App/Enable.php	87.87% <42.85%> (-12.13%)	8 <1> (+3)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41b46f7...e0b8d86. Read the comment docs.

[phil-davis]

And adjusted the unit test so it knows about the new param to the constructor.
Maybe this time CI will pass?

[micbar]

And adjusted the unit test so it knows about the new param to the constructor.
Maybe this time CI will pass?

PHpstan fails because the code of Enterprise Key is not available.

[phil-davis]

And adjusted the unit test so it knows about the new param to the constructor.
Maybe this time CI will pass?

PHpstan fails because the code of Enterprise Key is not available.

I "fixed" that by making it ignore that - phpstan CI is passing.

[phil-davis]

I added a changelog. I think this is ready for review.

There are existing cliProvisioning/enableApp.feature tests that cover enabling an ordinary app - so that ensures that this code change did not break anything for ordinary apps.

We are not easily able to get ourselves an enterprise app and the enterprise_key app etc during core CI. So this change will need manual testing during the release QA process.

[phil-davis]

The issue we found now is that if you have the enterprise app enabled, but have an invalid enterprise key, and you are just trying to enable a non-enterprise app, then the app cannot be enabled. e.g. enabling the testing app does not work.

If there is some way to check if the app being enabled is an Enterprise app, then this could be fixed by:

if (\OC_App::isEnterpriseApp($appId) && \OC_App::isEnabled('enterprise_key')) {