Skip to content
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

enforce forceAll #577

Merged
merged 2 commits into from May 3, 2021
Merged

enforce forceAll #577

merged 2 commits into from May 3, 2021

Conversation

ArtificialOwl
Copy link
Member

Should fix #576

@ArtificialOwl
enforce forcaAll
91b28a9 
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@ArtificialOwl ArtificialOwl mentioned this pull request Apr 29, 2021
Closed
@ArtificialOwl
Copy link
Member Author

/backport to stable21

@ArtificialOwl
Copy link
Member Author

/backport to stable20

@ArtificialOwl
Copy link
Member Author

/backport to stable19

@luckyhandler
Copy link

I only checked the code and didn't test it.

@solracsf
Copy link
Member

solracsf commented Apr 30, 2021 •
edited

Not working here. Only reverting #566 fixes those issues, with the downside of #566 (comment)

@Art4
Copy link

Art4 commented Apr 30, 2021 •
edited

Please excuse me if I'm wrong but this PR doesn't do anything. (I neglect a possible speed advantage)
It just moves the $forceAll === true to the first position in the if-condition.

@juliushaertl
Pass forceAll also when fetching the members list
45b87a7 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl
Copy link
Member

@daita I pushed another commit in order to also pass the forceAll parameter to the method that fetches the members list, which fixes the issue on my test setup.

Please excuse me if I'm wrong but this PR doesn't do anything. (I neglect a possible speed advantage)
It just moves the $forceAll === true to the first position in the if-condition.

It is not only a speed advantage but due to the OR operation if the first one matches the others will never get evaluated, so the $circle->getHigherViewer()->isLevel() which attempted to call on null then would never be executed if forceAll is used. Unfortunately it was only the first half of the fix 😉

Anyways testing of the two patches together is highly appreciated.

This was referenced May 3, 2021
Merged
Closed
@Art4
Copy link

Art4 commented May 3, 2021

@juliushaertl Thank you for the explanation.

@ArtificialOwl ArtificialOwl merged commit c57b953 into master May 3, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/576/enforce-forceAll branch May 3, 2021 12:24
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request May 3, 2021
Merged
@armouredking
Copy link

armouredking commented May 3, 2021 •
edited

Modifications do not fix for me, see #576 (I can't reopen bug).

@ArtificialOwl ArtificialOwl mentioned this pull request May 4, 2021
Merged
@ArtificialOwl
Copy link
Member Author

/backport to stable20

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request May 4, 2021
Merged
@ArtificialOwl
Copy link
Member Author

/backport to stable19

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request May 4, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luckyhandler luckyhandler luckyhandler approved these changes

Assignees
No one assigned
Labels
backport-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Circles 0.21.1 with NC 21.0.1 and Calendar 2.2.0 Results in Calendar App Broken
6 participants
@ArtificialOwl @luckyhandler @solracsf @Art4 @juliushaertl @armouredking