-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
[stable19] Fix account data visibility after disabling public addressbook upload #25974
Merged
rullzer
merged 11 commits into
stable19
from
backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload
Apr 29, 2021
Merged
[stable19] Fix account data visibility after disabling public addressbook upload #25974
rullzer
merged 11 commits into
stable19
from
backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload
Apr 29, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danxuliu
force-pushed
the
backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload
branch
from
March 6, 2021 08:22
c45dd4c
to
bfbffed
Compare
Merged
danxuliu
force-pushed
the
backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload
branch
from
April 14, 2021 16:06
bfbffed
to
5691573
Compare
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Right now it makes no difference, but this should make future tests clearer, specially in case of failure. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
"AccountManager::updateUser()" wipes previous user data with whichever user data is given (except for some adjustments, like resetting the verified status when needed). As the controller overrode the properties those properties would lose some of their attributes even if they are not affected by the changes made by the controller. Now the controller only modifies the attributes set ("value" and "scope") to prevent that. Note that with this change the controller no longer removes the "verified" status, but this is not a problem because, as mentioned, "AccountManager::updateUser()" resets them when needed (for example, when the value of the website property changes). This change is a previous step to fix overwritting properties with null values, and it will prevent the controller from making unexpected changes if more attributes are added in the future. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The controller can receive an optional subset of the properties of the user settings; values not given are set to "null" by default. However, those null values overwrote the previously existing values, so in practice any value not given was deleted from the user settings. Now only non null values overwrite the previous values. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
"sendingAToWithRequesttoken" needs to be used to test some non OCS endpoints which require the request token to be sent in the request. Now it is possible to specify the body (or, rather, additional contents beside the cookies and the request token) for those requests, as it will be needed for example to update the user profile. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When upload to the lookup server is disabled the scope menu was hidden in the personal information settings. However, even if the lookup server upload is disabled the personal information is still accesible from the local server as well as trusted servers. Users should be able to still set if their information is available to other users or if it is private, so now the scope menu is always show (although the "Public" option is hidden if the lookup server upload is disabled). If the user set the information as public before the upload to the lookup server was disabled the menu will also show the "Public" option as active, although disabled. Setting the visibility to any of the other options will hide the "Public" option from the menu (until the lookup server upload is enabled again). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Due to a bug (fixed some commits ago) in the UsersController of the settings app the scope of the properties can be null (for example, if lookup server upload was disabled and the user then changed the display name in the profile information). In that case now the scope menu icon shows an error to inform the user. The scope value will not change when other properties are modified until the user chooses an explicit value from the menu. Note that until a scope is explicitly set the property will behave as if it is private. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
force-pushed
the
backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload
branch
from
April 23, 2021 15:20
5691573
to
184a7ca
Compare
Unit test failures are unrelated. |
MorrisJobke
approved these changes
Apr 23, 2021
Merged
rullzer
approved these changes
Apr 29, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐘
rullzer
deleted the
backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload
branch
April 29, 2021 18:50
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #25417
This was backported from the original version of #25417; #25417 was rebased and adjusted as it conflicted with changes introduced in #26243 and #26548, but those pull requests were not backported to
stable19
.Kept as draft until #25417 is merged (just in case something else that needs to be adjusted instable19
is found too).