Skip to content

feat(updater): hide overwrites from disabled apps list on upgrade - #29988

Merged
skjnldsv merged 1 commit into
masterfrom
enh/hide-overwrites-from-disabled-apps-list
Feb 29, 2024
Merged

feat(updater): hide overwrites from disabled apps list on upgrade#29988
skjnldsv merged 1 commit into
masterfrom
enh/hide-overwrites-from-disabled-apps-list

Conversation

@MichaIng

@MichaIngMichaIng commented Nov 30, 2021

Copy link
Copy Markdown
Member

If an incompatible app is enabled manually, it is added to the app_install_overwrite array in config.php. Nextcloud upgrades won't disable any app in this array, but they were still shown on the upgrade page as being disabled.

This commit assures that only apps are shown as "These incompatible apps will be disabled:" which are really disabled, i.e. which are not in the app_install_overwrite array.

@MichaIngMichaIng added enhancement 3. to review Waiting for reviews php Pull requests that update Php code labels Nov 30, 2021
@MichaIngMichaIng added this to the Nextcloud 24 milestone Nov 30, 2021
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from 6867b9e to 9a86907CompareDecember 20, 2021 02:41
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from 9a86907 to 9dc53c3CompareJanuary 3, 2022 23:38
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from 9dc53c3 to 4d909a3CompareJanuary 20, 2022 15:32
@szaimen

Copy link
Copy Markdown
Contributor

Hiding app overwrite on minor updates is fine, imo.
For major updates, app overwrites should be deleted, though, ihmo.

@MichaIng

MichaIng commented Jan 27, 2022

Copy link
Copy Markdown
MemberAuthor

For major updates, app overwrites should be deleted, though, ihmo.

This actually makes sense. Hmm, this would need to be done as an additional migration step, I guess? The array would then not be updated yet when this code runs, so we'd need to additionally check for a major upgrade and then change this code like:

if ($isMajorUpgrade || !in_array($appInfo['name'], $incompatibleOverwrites)) {

with $isMajorUpgrade being a boolean depending on whether $installedVersion and $currentVersion differ in major version number or not. I can do that once the overwrite deletion step is added.

@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from 4d909a3 to 4744ce8CompareFebruary 4, 2022 14:36
@skjnldsvskjnldsv mentioned this pull request Mar 24, 2022
@blizzzblizzz mentioned this pull request Mar 31, 2022
@blizzzblizzz mentioned this pull request Apr 7, 2022
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from 4744ce8 to a5e8fb3CompareApril 12, 2022 00:05
@skjnldsv

Copy link
Copy Markdown
Member

Hiding app overwrite on minor updates is fine, imo. For major updates, app overwrites should be deleted, though, ihmo.

This is very important, can you raise that today at the server call @szaimen ? :)

@szaimen

Copy link
Copy Markdown
Contributor

This is very important, can you raise that today at the server call @szaimen ? :)

I can try :)

@blizzzblizzz mentioned this pull request Apr 13, 2022
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from a5e8fb3 to d9c5524CompareApril 18, 2022 21:01
@blizzzblizzz modified the milestones: Nextcloud 24, Nextcloud 25Apr 21, 2022
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from d9c5524 to cf76077CompareJune 1, 2022 20:30
This was referenced Aug 12, 2022
This was referenced Aug 24, 2022
This was referenced Sep 6, 2022
@skjnldsvskjnldsv mentioned this pull request Sep 15, 2022
});
$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use ($output) {
$output->writeln('<comment>Disabled incompatible app: ' . $app . '</comment>');
$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use ($output, &$incompatibleOverwrites) {

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\Hooks\EmitterTrait::listen has been marked as deprecated
});
$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use ($output) {
$output->writeln('<comment>Disabled incompatible app: ' . $app . '</comment>');
$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use ($output, &$incompatibleOverwrites) {

Check notice

Code scanning / Psalm

MissingClosureParamType

Parameter $app has no provided type
Comment threadcore/ajax/update.php
});
$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use (&$incompatibleApps) {
$incompatibleApps[] = $app;
$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use (&$incompatibleApps, &$incompatibleOverwrites) {

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\Hooks\EmitterTrait::listen has been marked as deprecated
Comment threadcore/ajax/update.php
});
$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use (&$incompatibleApps) {
$incompatibleApps[] = $app;
$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use (&$incompatibleApps, &$incompatibleOverwrites) {

Check notice

Code scanning / Psalm

MissingClosureParamType

Parameter $app has no provided type
@skjnldsvskjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzzblizzz modified the milestones: Nextcloud 28, Nextcloud 29Nov 23, 2023
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from 4ecc956 to d98e29aCompareJanuary 26, 2024 15:45
@skjnldsv
skjnldsvforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from d98e29a to 2d5a6cfCompareFebruary 27, 2024 16:34
@skjnldsvskjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 27, 2024
@MichaIngMichaIng changed the title Hide overwrites from disabled apps list on upgradeenh(updater): hide overwrites from disabled apps list on upgradeFeb 27, 2024
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from 2d5a6cf to ab6c32eCompareFebruary 27, 2024 17:47
@MichaIng

Copy link
Copy Markdown
MemberAuthor

Any idea what "Conventional Commits" does not like about the commits text? I added type and scope, so that should be fine: https://www.conventionalcommits.org/en/v1.0.0/

Probably something for the dev documentation, and probably the action can be configured to show the actual tests/rules which have been failed, to give any hint.

@skjnldsv

Copy link
Copy Markdown
Member

Any idea what "Conventional Commits" does not like about the commits text? I added type and scope, so that should be fine: https://www.conventionalcommits.org/en/v1.0.0/

Enh is not valid. Feat is :)

@MichaIng

MichaIng commented Feb 27, 2024

Copy link
Copy Markdown
MemberAuthor

Ah, it says "types other than fix: and feat: are allowed", so I thought any type is valid. But it seams that it is one of this array by default: https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional#type-enum

This PR is not really a (new) feature, I'd say, but there is no better matching type indeed 🙂.
EDIT: Jep, that was it, thanks!

@MichaIngMichaIng changed the title enh(updater): hide overwrites from disabled apps list on upgradefeat(updater): hide overwrites from disabled apps list on upgradeFeb 27, 2024
If an incompatible app is enabled manually, it is added to the "app_install_overwrite" array in config.php. Nextcloud upgrades won't disable any app in this array, but they were still shown on the upgrade page and logs as being disabled.
This commit assures that only apps which are really disabled, i.e. which are not in the "app_install_overwrite" array, are shown and logged as disabled during upgrades.
Signed-off-by: MichaIng <micha@dietpi.com>
@MichaIng
MichaIngforce-pushed the enh/hide-overwrites-from-disabled-apps-list branch from ab6c32e to 7b137ddCompareFebruary 27, 2024 19:20
@skjnldsv
skjnldsv disabled auto-merge February 29, 2024 10:47
@skjnldsv
skjnldsv merged commit 8df55ef into masterFeb 29, 2024
@skjnldsv
skjnldsv deleted the enh/hide-overwrites-from-disabled-apps-list branch February 29, 2024 10:47
@blizzzblizzz mentioned this pull request Mar 5, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishenhancementfeature: apps managementfeature: install and updatephpPull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@MichaIng@szaimen@skjnldsv@icewind1991@nickvergessen@github-advanced-security@joshtrichards@blizzz