Skip to content

Compute Gravatar URLs where needed (handle client_gravatar true) #255

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

Open
gnprice opened this issue Aug 3, 2023 · 2 comments
Open

Compute Gravatar URLs where needed (handle client_gravatar true) #255

gnprice opened this issue Aug 3, 2023 · 2 comments
Labels
a-api Implementing specific parts of the Zulip server API a-model Implementing our data model (PerAccountStore, etc.) help wanted performance Smooth and responsive UI; fixing jank, stutters, and lag

Comments

@gnprice
Copy link
Member

gnprice commented Aug 3, 2023

We should properly handle this optimization:
https://zulip.com/api/register-queue#parameter-client_gravatar

At the moment we don't pass that client_gravatar parameter, so we get the default behavior of true, which signs us up for the server to pass null as a user's avatar_url to express that it's a Gravatar URL we can compute from the email address. But we haven't actually written down the bit of code to compute that Gravatar URL, so we end up with a missing avatar.

I think we'll shortly flip that for the moment to pass client_gravatar as false, so that we get correct behavior. But we should follow up by actually implementing the client side of that — it's not hard — and then re-enabling the optimization.

For code demonstrating the logic, see zulip-mobile's GravatarURL in src/utils/avatar.js.

Another piece we'll need as part of implementing this issue is to deserialize the User.avatarUrl field in a way that distinguishes whether the server set the avatar_url field to null or left it out entirely. The former indicates a Gravatar, but the latter comes from user_avatar_url_field_optional (#254) and carries no information about what the avatar is.

That will likely involve a class AvatarUrl, with subclasses for a provided URL string or indicating that the avatar is a computable Gravatar or that it was omitted. This resembles the AvatarURL class hierarchy in zulip-mobile — but one thing I would like to do differently here is to avoid putting redundant information on these avatar-URL objects, in order to avoid unnecessary allocation. (After all, the list of users is a fairly hot path both for startup CPU time and for memory consumption, when in a large realm.) It's fine for the AvatarUrl value to only be possible to fully interpret by using other data from the User.

@gnprice gnprice added a-api Implementing specific parts of the Zulip server API a-model Implementing our data model (PerAccountStore, etc.) labels Aug 3, 2023
@gnprice gnprice added this to the Launch milestone Sep 14, 2023
@gnprice
Copy link
Member Author

gnprice commented Sep 23, 2023

I think we'll shortly flip that for the moment to pass client_gravatar as false, so that we get correct behavior.

(This was done in caa9603 in #249. Implementing the optimization, which is what the present issue is about, is still open.)

@gnprice gnprice added the performance Smooth and responsive UI; fixing jank, stutters, and lag label Nov 18, 2023
@gnprice
Copy link
Member Author

gnprice commented Jul 2, 2024

Another piece we'll need as part of implementing this issue is to deserialize the User.avatarUrl field in a way that distinguishes whether the server set the avatar_url field to null or left it out entirely.

This can be expressed using the JsonNullable class introduced in #F784. For discussion, see chat thread (from 2023-04).

@gnprice gnprice modified the milestones: Launch, Post-launch Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-api Implementing specific parts of the Zulip server API a-model Implementing our data model (PerAccountStore, etc.) help wanted performance Smooth and responsive UI; fixing jank, stutters, and lag
Projects
Status: No status
Development

No branches or pull requests

1 participant