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

Use cached getDisplayName method #33605

Merged
merged 2 commits into from
Nov 3, 2022
Merged

Conversation

miaulalala
Copy link
Contributor

instead of the current slower $usermanager->get($user)->getDisplayName()

Fixes #33598

apps/dav/lib/CalDAV/Activity/Provider/Base.php Outdated Show resolved Hide resolved
apps/dav/lib/CalDAV/CalDavBackend.php Outdated Show resolved Hide resolved
apps/dav/lib/CalDAV/Schedule/IMipPlugin.php Outdated Show resolved Hide resolved
apps/dav/lib/CardDAV/CardDavBackend.php Outdated Show resolved Hide resolved
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

With @nickvergessen's suggestions

apps/dav/lib/CalDAV/Activity/Provider/Base.php Outdated Show resolved Hide resolved
@tcitworld
Copy link
Member

Needs a rebase after #33615

@miaulalala
Copy link
Contributor Author

Needs a rebase after #33615

will be done when squashing the fixups

apps/dav/lib/CalDAV/Schedule/IMipPlugin.php Outdated Show resolved Hide resolved
@miaulalala
Copy link
Contributor Author

The $senderName variable doesn't need a check on null as it's checked again later and it will use the $sender if the $senderName is null.

@tcitworld
Copy link
Member

The $senderName variable doesn't need a check on null as it's checked again later and it will use the $sender if the $senderName is null.

But $sender is the email, not the uid. Also for the definition of $fromName it's not handled properly.

@miaulalala
Copy link
Contributor Author

The $senderName variable doesn't need a check on null as it's checked again later and it will use the $sender if the $senderName is null.

But $sender is the email, not the uid. Also for the definition of $fromName it's not handled properly.

The fallback for not having a display name is the email, yes. Which is fine in this case, that is a good way to handle not having a Display Name for the sender when it comes to emails.

Added a null check on the $senderName for the $fromName, thanks for pointing that out.

Signed-off-by: Anna Larch <anna@nextcloud.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good

@miaulalala miaulalala merged commit d648518 into master Nov 3, 2022
@miaulalala miaulalala deleted the perf/user-getdisplayname-cache branch November 3, 2022 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CalDAV: use getDisplayName cache
4 participants