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

Fix null displayname crash as described in #21885 #23065

Merged
merged 2 commits into from
May 25, 2021

Conversation

TomG736
Copy link
Contributor

@TomG736 TomG736 commented Sep 27, 2020

As discussed in #21885, displayname is sometimes Null when updating from older versions. This pull requests adds the code suggested in that issue by @dh-connect with modifications suggested by @kbzowski

@TomG736 TomG736 force-pushed the FixEmptyDisplayName branch from 5860824 to 4c3be97 Compare September 27, 2020 12:28
@skjnldsv
Copy link
Member

@TomG736 what is the status? :)

@Alekzanther
Copy link

@rullzer This issue has been plaguing me (and many others) for a long time... It breaks large sections of Nextcloud. What's happening here? Isn't @TomG736:s fix good enough for now? Or are we missing something?

@jospoortvliet do you know?

@PVince81
Copy link
Member

@TomG736 can you rebase ?

If the fix as-is has already proven to work for many people in #21885 maybe we can take it as is ?

@TomG736 TomG736 force-pushed the FixEmptyDisplayName branch from 4c3be97 to 23dbbca Compare March 17, 2021 23:05
@TomG736
Copy link
Contributor Author

TomG736 commented Mar 17, 2021

Rebased back off current master

@PVince81
Copy link
Member

Oracle fails:

1) Test\User\DatabaseTest::testSearch
Failed asserting that actual size 3 matches expected size 2.

/home/runner/work/server/server/tests/lib/User/Backend.php:114
/home/runner/work/server/server/tests/lib/User/DatabaseTest.php:125

Looks slightly related, but not sure

@PVince81
Copy link
Member

looking closer, if the search finds 3 users containing "bar" instead of 2, maybe there's some random cleanup issue in the tests, likely unrelated. I'll restart the tests just in case...

@Alekzanther
Copy link

looking closer, if the search finds 3 users containing "bar" instead of 2, maybe there's some random cleanup issue in the tests, likely unrelated. I'll restart the tests just in case...

Did it work? There seems to be some "requested changes" (that don't really need to be addressed) blocking merge?

@J0WI J0WI requested a review from rullzer April 28, 2021 12:44
@J0WI J0WI added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 28, 2021
@J0WI J0WI added this to the Nextcloud 22 milestone Apr 28, 2021
@MorrisJobke MorrisJobke mentioned this pull request May 20, 2021
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

@MorrisJobke MorrisJobke merged commit 3d1c786 into nextcloud:master May 25, 2021
@welcome
Copy link

welcome bot commented May 25, 2021

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@TomG736 TomG736 deleted the FixEmptyDisplayName branch May 25, 2021 21:09
@MorrisJobke MorrisJobke mentioned this pull request May 26, 2021
98 tasks
@szaimen
Copy link
Contributor

szaimen commented Jun 23, 2021

backport for stable21?

@SyZn
Copy link

SyZn commented Sep 2, 2021

backport for stable21?

Hi,
A backport would be greatly appreciated. I run into this issue on every update since NC 19 and had to fix it manually each time.

@blizzz
Copy link
Member

blizzz commented Sep 10, 2021

/backport to stable22

@blizzz
Copy link
Member

blizzz commented Sep 10, 2021

/backport to stable21

@blizzz
Copy link
Member

blizzz commented Sep 10, 2021

/backport to stable20

@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

@szaimen
Copy link
Contributor

szaimen commented Sep 10, 2021

/backport to stable22

@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

@szaimen
Copy link
Contributor

szaimen commented Sep 10, 2021

backprt to stable22 fails because it is already included in stable22

@blizzz
Copy link
Member

blizzz commented Sep 10, 2021

backprt to stable22 fails because it is already included in stable22

makes sense :)

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.

10 participants