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.

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

[stable21] Fix account data visibility after disabling public addressbook upload #26724

Merged
merged 19 commits into from
Jul 27, 2021

Conversation

backportbot-nextcloud[bot]
Copy link

@backportbot-nextcloud backportbot-nextcloud bot commented Apr 23, 2021

⚠️ This backport had conflicts and is incomplete ⚠️

backport of #25417

Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Conflicts are most likely caused by #26650 not being merged yet, so let's wait until it is merged to fix them.

@rullzer rullzer mentioned this pull request Apr 29, 2021
@blizzz blizzz mentioned this pull request Jun 23, 2021
9 tasks
@blizzz blizzz mentioned this pull request Jul 1, 2021
10 tasks
nickvergessen and others added 6 commits July 1, 2021 09:43
Signed-off-by: Joas Schilling <[email protected]>
Signed-off-by: npmbuildbot-nextcloud[bot] <npmbuildbot-nextcloud[bot]@users.noreply.github.com>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
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 <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
"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 <[email protected]>
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 <[email protected]>
"parsePhoneNumber()" expects a string, so a TypeError would be thrown if
the phone number value is null.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Depending on some settings (for example, if lookup server upload is
disabled) some items can be hidden in the scope menu. However, if the
user selected an scope in the past once the settings were changed the
scope was no longer visible in the menu. Now the active scope will be
always visible in the menu, although if it is an excluded scope it will
be disabled. Selecting any other scope will then hide the excluded and
no longer active one.

When upload to the lookup server is disabled the scope menu was hidden
for display name and email in the personal information settings; now the
menu will be always shown to enable the above described behaviour.

Note that the menu will be shown even if there is a single available
scope so the user can read its description.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Although not merged yet, #26650 seems to be ready now, so I have rebased on top of #26650 and fixed the conflicts. It will need a final rebase on stable21 once #26650 is merged, but then there should not be any conflict.

Tested and works 👍

@MorrisJobke MorrisJobke removed their request for review July 4, 2021 11:34
@skjnldsv skjnldsv mentioned this pull request Jul 26, 2021
14 tasks
@skjnldsv skjnldsv merged commit 0aed3ec into stable21 Jul 27, 2021
@skjnldsv skjnldsv deleted the backport/25417/stable21 branch July 27, 2021 12:05
@skjnldsv skjnldsv mentioned this pull request Aug 3, 2021
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants