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

Do not issue update command if nothing has changed in user values #14967

Merged
merged 3 commits into from
Apr 11, 2019

Conversation

MorrisJobke
Copy link
Member

Same as #14566 but rebased on master so that we can debug the failing CI.

@MorrisJobke
Copy link
Member Author

Let me debug this later today

@MorrisJobke
Copy link
Member Author

Partially added via #15052 to properly debug the CI failure in smaller steps.

@MorrisJobke MorrisJobke force-pushed the lib-private-user-trigger-pass-old branch from 146333b to 9a5ca23 Compare April 11, 2019 09:21
@nextcloud nextcloud deleted a comment from faily-bot bot Apr 11, 2019
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 11, 2019
@@ -164,6 +164,7 @@ public function countUsers() {

public function setDisplayName($uid, $displayName) {
$this->displayNames[$uid] = $displayName;
return true;
Copy link
Member Author

Choose a reason for hiding this comment

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

I found the cause of the failure - no idea why it worked before, but it was also failing locally and this fixed it locally for me as well 🙌

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

But of course 😕

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 11, 2019
@@ -138,11 +138,12 @@ public function getDisplayName() {
*/
public function setDisplayName($displayName) {
$displayName = trim($displayName);
if ($this->backend->implementsActions(Backend::SET_DISPLAYNAME) && !empty($displayName)) {
$oldDisplayName = $this->getDisplayName();
if ($this->backend->implementsActions(Backend::SET_DISPLAYNAME) && !empty($displayName) && $displayName !== $oldDisplayName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense. Just a small nitpick: !empty($displayName) && $displayName !== $oldDisplayName is cheaper than implementsActions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Something for later I would say.

@MorrisJobke MorrisJobke merged commit 3308c40 into master Apr 11, 2019
@MorrisJobke MorrisJobke deleted the lib-private-user-trigger-pass-old branch April 11, 2019 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants