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 behavior when editing bot avatar. #24427

Open
sahil839 opened this issue Feb 21, 2023 · 14 comments
Open

Fix behavior when editing bot avatar. #24427

sahil839 opened this issue Feb 21, 2023 · 14 comments

Comments

@sahil839
Copy link
Collaborator

There are two issues in the UI for updating bot avatar.

Current behavior

  • The current avatar is not shown when the bot has a default gravatar set. I can see that it was visible in the GIF shared in first comment of settings: show current avatar in bot editing form #23965, but the behavior might have changed when updating the PR.
    bot-with-gravatar

  • There is no Clear avatar or Clear profile picturebutton in edit form if bot already had a profile picture set. It is only shown once I update the existing avatar and then clicking on it it changes it to the old avatar and does not completely remove it.
    clear-avatar-option

Expected behavior

  • The current profile picture should be always shown in edit bot form whether it is default gravatar or an user uploaded image.
  • We should show Clear profile picture button when avatar is set to something other than the default gravatar. I am not sure what should the Clear profile picture button do when user changes the avatar one time after opening the edit modal. Should it be changed it to the previous avatar or to the default gravatar. @alya thoughts?
@zulipbot
Copy link
Member

Hello @zulip/server-settings members, this issue was labeled with the "area: settings (user)", "area: settings (admin/org)", "area: settings UI" labels, so you may want to check it out!

@sahil839
Copy link
Collaborator Author

@SahilSingh177 FYI.

@arijitghosal03
Copy link

@sahil839 thanks for picking up this issue, I tried the steps myself and faced the same problems as seen in your video reference.I would like to work on this issue and also want to invite you to collaborate on this

@arijitghosal03
Copy link

@zulipbot claim

@zulipbot
Copy link
Member

Hello @arijitghosal03!

Thanks for your interest in Zulip! You have attempted to claim an issue without the label "help wanted". You can only claim and submit pull requests for issues with the help wanted label.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

@sahil839
Copy link
Collaborator Author

@arijitghosal03 To be clear, I have not claimed the issue, I just opened it. I do not intend to work on this as I am occupied with other work.

I have not added "help wanted" label as I first wanted to get the feedback from Alya about the correct expected behavior.

@Ujjawal3
Copy link
Collaborator

@zulipbot claim.

@zulipbot
Copy link
Member

Hello @Ujjawal3!

Thanks for your interest in Zulip! You have attempted to claim an issue without the label "help wanted". You can only claim and submit pull requests for issues with the help wanted label.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

@SahilSingh177
Copy link
Collaborator

I am sorry for the delayed response. I found that this bug was not present at the time when I implemented the changes. It is possible that subsequent modifications may have had an impact on the code, I will investigate further to identify the root cause.

@Ddharmani3
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

zulipbot commented Mar 9, 2023

Hello @Ddharmani3!

Thanks for your interest in Zulip! You have attempted to claim an issue without the label "help wanted". You can only claim and submit pull requests for issues with the help wanted label.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

@SahilSingh177
Copy link
Collaborator

SahilSingh177 commented Mar 12, 2023

@sahil839, I discovered that bot.avatar_url returns null for Gravatar profile pictures, which is the reason why the image is not displayed. It is unclear what is causing this issue since avatar_url should return the Gravatar URL as it does in the Bots section of Personal Settings.

One potential solution is to check if avatar_url is null and if so, retrieve the bot_id and then obtain the avatar_url. This solution appears to resolve the bug.

bot_avatar_bug

bot_avatar_bug2

bot_avatar_bug3

@Ddharmani3
Copy link
Collaborator

Ddharmani3 commented Mar 12, 2023

@sahil839, I discovered that bot.avatar_url returns null for Gravatar profile pictures, which is the reason why the image is not displayed. It is unclear what is causing this issue since avatar_url should return the Gravatar URL as it does in the Bots section of Personal Settings.

@SahilSingh177 @sahil839, I am currently working on this issue and have completed about 70% of the work. Building upon Sahil Singh's observation, I have noticed that bot.avatar_url returns null for Gravatar profile pictures in both personal and organization settings sections, but for some reason, it did not return null in Sahil Singh's personal settings section.

Additionally, I have found that when a new bot is added without any user-uploaded avatar and the edit form is immediately opened, the Gravatar image is displayed and does not return null. However, once the page is reloaded and the edit form is opened again, the image returns null.

One potential solution is to check if avatar_url is null and if so, retrieve the bot_id and then obtain the avatar_url. This solution appears to resolve the bug.

In my solution, I used bot_data.get(user_id) instead of people.get_by_user_id(user_id) in the show_edit_bot_info_modal function in settings_bots.js, which does not return null for avatar_url. This solved the problem, but I believe that the issue of avatar_url being null for the user returned by people.get_by_user_id(user_id) should also be addressed to prevent any mishaps in the future.

I tried to locate the part of the code that is causing avatar_url to be null when the page is reloaded, but I haven't been able to find it yet. @sahil839, could you please guide me on what part of the code could be causing this issue? Also, if you have any suggestions on how I can improve my current approach, please let me know.

@sahil839
Copy link
Collaborator Author

sahil839 commented Mar 14, 2023

Thanks for the digging around the code. So, the issue is that bot_data module uses the data recieved in realm_bots field recieved in page_params object from the server, while the people module uses the data sent in realm_users field. The code for this is in zerver/lib/events.py.

The realm_bots data consists of avatar url for all cases, while the realm_users data consists of avatat_url only if client cannot compute the gravatar url themselves. I think the short-term solution is to just use helper functions in people.js which already check whether avatar_url is null or not and then return the url accordingly.

And in long-term we probably would want to fix the realm_bots data to do not contain avatar_url field when client can compute it.

Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Mar 14, 2023
Fixed gravatar not visible in edit bot form by making bot_avatar_url
equal to bot.avatar_url || people.medium_avatar_url_for_person(bot)
which solves the case when avatar_url is returned null.

Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and changed
value of a temporary variable bot_details.avatar_source to "G".

The temporary avatar_source is passed to backend only when
dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Mar 15, 2023
Fixed gravatar not visible in edit bot form by making bot_avatar_url
equal to bot.avatar_url || people.medium_avatar_url_for_person(bot)
which solves the case when avatar_url is returned null.

Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and changed
value of a temporary variable bot_details.avatar_source to "G".

The temporary avatar_source is passed to backend only when
dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source i realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Mar 15, 2023
Fixed gravatar not visible in edit bot form by making bot_avatar_url
equal to bot.avatar_url || people.medium_avatar_url_for_person(bot)
which solves the case when avatar_url is returned null.

Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and changed
value of a temporary variable bot_details.avatar_source to "G".

The temporary avatar_source is passed to backend only when
dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source i realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Mar 23, 2023
Fixed gravatar not visible in edit bot form by making bot_avatar_url
equal to bot.avatar_url || people.medium_avatar_url_for_person(bot)
which solves the case when avatar_url is returned null.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Mar 23, 2023
Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar only if its not gravtar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and changed
value of a temporary variable bot_details.avatar_source(passed to
build_bot_edit_widget function from settings_bots.js) to "G".

The temporary avatar_source is passed to backend only when
dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source i realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Mar 23, 2023
Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar only if its not a gravtar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and changed
value of a temporary variable bot_details.avatar_source(passed to
build_bot_edit_widget function from settings_bots.js) to "G".

The temporary avatar_source is passed to backend only when
dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source i realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Mar 26, 2023
Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar only if its not a gravtar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and changed
value of a `#current_bot_avatar_source` which is an input of
type hidden to "G".

Thevalue of a `#current_bot_avatar_source` is passed to backend
only when dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source i realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Mar 26, 2023
Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar only if its not a gravtar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and changed
value of a `#current_bot_avatar_source` which is an input of
type hidden to "G".

Thevalue of a `#current_bot_avatar_source` is passed to backend
only when dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source i realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Apr 7, 2023
Fixed gravatar not visible in edit bot form by making bot_avatar_url
equal to bot.avatar_url || people.medium_avatar_url_for_person(bot)
which solves the case when avatar_url is returned null.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Apr 7, 2023
Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar only if its not a gravtar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and change
value of a `#current_bot_avatar_source` which is an input of
type hidden to "G".

The value of a `#current_bot_avatar_source` is passed to backend
only when dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source in realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Apr 7, 2023
Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar only if its not a gravtar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and change
value of a `#current_bot_avatar_source` which is an input of
type hidden to "G".

The value of a `#current_bot_avatar_source` is passed to backend
only when dialog_submit_button in edit bot form is clicked.

Updated format_user_row function in lib/users.py to also return
avatar_source field in realm_user. Updated zulip.yaml and 4 other
tests to incorporate the new field avatar_source in realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Apr 23, 2023
Fixed gravatar not visible in edit bot form by making bot_avatar_url
equal to bot.avatar_url || people.medium_avatar_url_for_person(bot)
which solves the case when avatar_url is returned null.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Apr 23, 2023
This is a prep commit to include avatar_source field in realm_user
to enable delete bot avatar in bot edit modal.

Updated format_user_row function in lib/users.py to also
return avatar_source field in realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Apr 23, 2023
Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar only if its not a gravtar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and change
value of a `#current_bot_avatar_source` which is an input of
type hidden to "G".

The value of a `#current_bot_avatar_source` is passed to backend
only when dialog_submit_button in edit bot form is clicked.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Apr 23, 2023
This is a prep commit to include avatar_source field in realm_user
to enable delete bot avatar in bot edit modal.

Updated format_user_row function in lib/users.py to also
return avatar_source field in realm_user.

Fixes zulip#24427
Ddharmani3 added a commit to Ddharmani3/zulip that referenced this issue Apr 23, 2023
Added new feature for completely removing user uploaded avatar
by adding a delete button which shows on hovering on current
avatar only if its not a gravtar.

Added a onclick event listener for the delete button in avatar.js
which change the current avatar image to bot gravatar and change
value of a `#current_bot_avatar_source` which is an input of
type hidden to "G".

The value of a `#current_bot_avatar_source` is passed to backend
only when dialog_submit_button in edit bot form is clicked.

Fixes zulip#24427
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants