Skip to content

Fix tracking of auto disabled apps in Updater - #16507

Merged
rullzer merged 2 commits into
masterfrom
bugfix/15416/update_autodisabled_apps
Aug 15, 2019
Merged

Fix tracking of auto disabled apps in Updater#16507
rullzer merged 2 commits into
masterfrom
bugfix/15416/update_autodisabled_apps

Conversation

@georgehrke

@georgehrkegeorgehrke commented Jul 22, 2019

Copy link
Copy Markdown
Member

fixes#15416

@georgehrkegeorgehrke added the 3. to review Waiting for reviews label Jul 22, 2019
@georgehrkegeorgehrke mentioned this pull request Jul 22, 2019
Closed
@georgehrke

georgehrke commented Jul 22, 2019

Copy link
Copy Markdown
MemberAuthor

@nickvergessen I'm not too deep into \OC_App, so any feedback is welcome.

@georgehrke

georgehrke commented Jul 22, 2019

Copy link
Copy Markdown
MemberAuthor

Would like to have some review / comment whether this is the fix we are looking for.
Afterwards we should add some tests to make sure it doesn't break again.

nickvergessen
nickvergessen reviewed Jul 23, 2019
View reviewed changes
Comment threadlib/private/Updater.php Outdated
throw new \UnexpectedValueException('The files of the app "' . $app . '" were not correctly replaced before running the update');
}
\OC::$server->getAppManager()->disableApp($app);
$this->autoDisabledApps[] = $app;

@nickvergessennickvergessenJul 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe instead we move the logic to the appmanager method with a bool flag.
So when someone adds another spot that disables an app, they are reminded about the option.

@nickvergessennickvergessen self-assigned this Jul 23, 2019
@nickvergessen

nickvergessen commented Jul 23, 2019

Copy link
Copy Markdown
Member

Added a commit, I think that's the cleaner way to do it.

georgehrke
georgehrke commented Jul 23, 2019
View reviewed changes
Comment threadlib/private/Updater.php Outdated
@kyrofa

kyrofa commented Jul 24, 2019

Copy link
Copy Markdown
Member

Thanks for fixing this, @georgehrke. I assume backporting to v16 is also part of the plan, here, but wanted to mention it anyway.

@nickvergessen

nickvergessen commented Jul 25, 2019

Copy link
Copy Markdown
Member

Failing tests are

PHP Fatal error: Uncaught PDOException: SQLSTATE[HY000]: General error: 2006 MySQL server has gone away in /drone/src/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:41

So time to rebase?

@nickvergessen

nickvergessen commented Jul 25, 2019

Copy link
Copy Markdown
Member

/backport to stable16

@nickvergessen

nickvergessen commented Jul 25, 2019

Copy link
Copy Markdown
Member

/backport to stable15

MorrisJobke
MorrisJobke approved these changes Jul 29, 2019
View reviewed changes

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code makes sense and all usages of disableApp that are affected are covered 👍

@MorrisJobke
MorrisJobkeforce-pushed the bugfix/15416/update_autodisabled_apps branch from c9d713d to c3752efCompareJuly 29, 2019 12:50
@MorrisJobke

MorrisJobke commented Jul 29, 2019

Copy link
Copy Markdown
Member

So time to rebase?

Rebased and auto squashed.

@MorrisJobkeMorrisJobke added this to the Nextcloud 17 milestone Jul 29, 2019
@kyrofa

kyrofa commented Aug 5, 2019

Copy link
Copy Markdown
Member

Still have a few test failures, here.

@the-sanethe-sane mentioned this pull request Aug 6, 2019
Merged
@MorrisJobkeMorrisJobke mentioned this pull request Aug 8, 2019
Merged
28 tasks
georgehrkeand others added 2 commits August 15, 2019 11:12
@georgehrke@rullzer
Fix tracking of auto disabled apps in Updater
f6c3424
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@nickvergessen@rullzer
Make the auto-disabled list more broad
810ee7d
Signed-off-by: Joas Schilling <coding@schilljs.com>
@rullzer
rullzerforce-pushed the bugfix/15416/update_autodisabled_apps branch from c3752ef to 810ee7dCompareAugust 15, 2019 09:12
@kyrofa

kyrofa commented Aug 15, 2019•
edited
Loading

Copy link
Copy Markdown
Member

Hey folks, what's the hold up here? We were really hoping this would be in 16.0.4.

@rullzer
rullzer merged commit 9c2d701 into masterAug 15, 2019
@rullzer
rullzer deleted the bugfix/15416/update_autodisabled_apps branch August 15, 2019 18:09
@backportbot-nextcloud

backportbot-nextcloudBot commented Aug 15, 2019

Copy link
Copy Markdown

The backport to stable16 failed. Please do this backport manually.

@backportbot-nextcloud

backportbot-nextcloudBot commented Aug 15, 2019

Copy link
Copy Markdown

The backport to stable15 failed. Please do this backport manually.

@kyrofakyrofa mentioned this pull request Aug 15, 2019
Merged
@kyrofa

kyrofa commented Aug 15, 2019

Copy link
Copy Markdown
Member

The backport to stable16 failed. Please do this backport manually.

Backport to stable16 is #16753.

rullzer pushed a commit that referenced this pull request Sep 4, 2019
@georgehrke@kyrofa
Fix tracking of auto disabled apps in Updater
639ff3d
This is a backport of #16507 (fixing #15416) for stable16.
Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
@kyrofakyrofa mentioned this pull request Oct 1, 2019
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessennickvergessennickvergessen left review comments
@rullzerrullzerrullzer approved these changes
@MorrisJobkeMorrisJobkeMorrisJobke approved these changes
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

@nickvergessennickvergessen

Labels

3. to reviewWaiting for reviews

Projects

None yet

Milestone

Nextcloud 17

Development

Successfully merging this pull request may close these issues.

Full automatic app update option in setting Gui

5 participants

@georgehrke@nickvergessen@kyrofa@MorrisJobke@rullzer