bots: Sync data on bot ownership change. #8606
Conversation
zulipbot
added
size: XL
area: bots
area: settings (user)
bug
and removed
area: bots
area: settings (user)
bug
labels
|
Hello @zulip/ members, this pull request was labeled with the **** labels, so you may want to check it out! |
zerver/lib/actions.py
Outdated
| update_users = update_users - {bot_owner.id, } | ||
|
|
||
| # Delete the bot from previous owner's bot data. | ||
| if not acting_user.is_realm_admin: |
There was a problem hiding this comment.
acting_user may not be the previous owner -- e.g. an admin can re-assign the owner.
I think you want to save previous_owner variable above (before we overwrite it) and use that here.
|
Posted one comment; othewise looks great! Do we already correctly have the frontend remove a user from |
shubham-padia
force-pushed
the
bot_sync3
branch
3 times, most recently
from
a07e5c8
to
3c5700a
Compare
Adds realm_bot delete event. On bot ownership change, add event is sent to the bot_owner(if not admin) and delete event to the previous bot owner(if not admin). For admin, update event is sent.
@timabbott Yes, the I've made the requested changes. Also added a check if the previous owner and new owner are same. |
|
Nice, merged, thanks @shubham-padia! |
Fixes #8577.
The issue mentioned above occurs in case of non-admin users as the bot hasn't been added to their bot_data. Also the bots for which ownership was changed were not being updated in real time for the non-admin users. For admin users, they already have the bot added, so everything works fine there.
Approach taken:
removesets the bot status to inactive if the bot is deleted. A non-admin user's bot_data contains only those bots that the user owns. Using the existingremovewould have just set the bot to inactive, which is not the case and the bot should be removed from bot data.