Skip to content

Keep group restrictions when reenabling apps after an update #29020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
nickvergessen merged 1 commit into from
Oct 13, 2021
Merged

Keep group restrictions when reenabling apps after an update #29020

nickvergessen merged 1 commit into from
Oct 13, 2021

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Oct 1, 2021 •
edited by szaimen
Loading

Fix #4538

Signed-off-by: Joas Schilling [email protected]

@nickvergessen nickvergessen added this to the Nextcloud 23 milestone Oct 1, 2021
@nickvergessen nickvergessen requested review from a team, ArtificialOwl, CarlSchwan, ChristophWurst, kesselb and skjnldsv and removed request for a team October 1, 2021 14:42
@szaimen
Copy link
Contributor

szaimen commented Oct 1, 2021

Great to see this getting fixed :)
Will this be backported?

@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 1, 2021

Great to see this getting fixed :)

I wasn't aware it was actually broken. I think this is quite sever 😬

@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 1, 2021

/backport to stable22

@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 1, 2021

/backport to stable21

@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 1, 2021

/backport to stable20

@szaimen
Copy link
Contributor

szaimen commented Oct 1, 2021 •
edited
Loading

I wasn't aware it was actually broken. I think this is quite severe 😬

jup. there was an issue open since a while about this #4538 ;-)

@szaimen szaimen mentioned this pull request Oct 1, 2021
Closed
@szaimen

This comment has been minimized.

Sign in to view
@szaimen szaimen mentioned this pull request Oct 3, 2021
Closed
@nickvergessen nickvergessen requested a review from szaimen October 7, 2021 07:20
@nickvergessen nickvergessen force-pushed the bugfix/noid/keep-group-restrictions-when-reenabling-apps branch from 97fcc7b to 9c017c9 Compare October 8, 2021 10:09
@blizzz
Copy link
Member

blizzz commented Oct 11, 2021 •
edited
Loading

why is the method removed from the interface?

@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 11, 2021

why is the method removed from the interface?

So, the thing is we change the return of the method. It only returned the appids so far and now returns a map of appid => old state
The main reason why I removed it however is that the function only works within the update process. It feels weird to have a updater specific thing exposed as public API.

@enoch85 enoch85 mentioned this pull request Oct 11, 2021
Closed
@blizzz
Copy link
Member

blizzz commented Oct 11, 2021

Psalm is impious

@nickvergessen nickvergessen force-pushed the bugfix/noid/keep-group-restrictions-when-reenabling-apps branch from 9c017c9 to 24ce6da Compare October 12, 2021 11:56
@nickvergessen nickvergessen force-pushed the bugfix/noid/keep-group-restrictions-when-reenabling-apps branch from 24ce6da to 0b74753 Compare October 13, 2021 07:35
@nickvergessen nickvergessen merged commit c1e7f6e into master Oct 13, 2021
@nickvergessen nickvergessen deleted the bugfix/noid/keep-group-restrictions-when-reenabling-apps branch October 13, 2021 08:37
This was referenced Oct 13, 2021
Merged
Merged
Merged
@juliusknorr
Copy link
Member

juliusknorr commented Oct 15, 2021

FYI this seems to have caused a regression with the app store on upgrade: #29257

@szaimen szaimen mentioned this pull request Nov 19, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blizzz blizzz blizzz approved these changes

@kesselb kesselb kesselb approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@szaimen szaimen Awaiting requested review from szaimen

Assignees

No one assigned

Labels

3. to review Waiting for reviews bug feature: install and update

Projects

None yet

Milestone

Nextcloud 23

Development

Successfully merging this pull request may close these issues.

Group restrictions for an app are lost when disabling it

6 participants

@nickvergessen @szaimen @blizzz @juliusknorr @kesselb