users_list: Fix updating of users list on reactivate/deactivate. #14288
Conversation
sahil839
force-pushed
the
deactivated_users
branch
2 times, most recently
from
02eff01 to
5294bca
Compare
|
@sahil839 Can you give an update on the status of this PR? I know this issue sort of sent us down the rabbit hole of making sure that we weren't showing the wrong "active" date for users after we reactivated them, but is that actually a blocker for this commit? |
|
@showell The only problem is that this will show |
static/js/settings_users.js
Outdated
| @@ -392,7 +388,6 @@ exports.on_load_success = function (realm_people_data) { | |||
| const url = '/json/users/' + encodeURIComponent(user_id) + "/reactivate"; | |||
| const data = {}; | |||
| const status = get_status_field(); | |||
|
|
|||
There was a problem hiding this comment.
Can you undo the blank line removals here? I think those lines were there for readability, and in any case it's distracting from the rest of the changes in this commit.
static/js/settings_users.js
Outdated
|
|
||
| populate_users(realm_people_data); | ||
|
|
||
| exports.register_click_handlers = function () { |
There was a problem hiding this comment.
Can you clean up this commit series to have steps like extracting register_click_handlers as a new function as independent commits? It'd be more readable that way.
sahil839
force-pushed
the
deactivated_users
branch
2 times, most recently
from
de3ba6f to
a133cf2
Compare
sahil839
force-pushed
the
deactivated_users
branch
3 times, most recently
from
58d69e1 to
9025fd1
Compare
|
@timabbott I have made the changes. |
|
I'm working on a deeper fix here. I'll probably close this PR once I get to a pretty solid point on my branch. Thanks for working on this @sahil839 -- reading your code here has been helpful for me to understand where we can clean this up a bit. |
|
Heads up @sahil839, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
|
I haven't quite fixed this yet, but you can see that my recent commits are leading up to a fix here: |
|
In particular the below commit is sort of a preview of how I will split up handling of Users and Deactivated Users, which should help fix this issue: There's a little bit of extra work to do, though. |
Previously, when a user was deactivated, the user was added in the deactivated users list only after opening the settings menu again or reloading, but now the user is added to the deactivated users list if we open the deactivated users section. Note that sticky behavior is stil present which means that the deactivated user will still be present in the active users list until we go away from that section. And the behavior is same for reactivating a user from deactivated users list.
sahil839
force-pushed
the
deactivated_users
branch
from
9025fd1 to
73f3e27
Compare
|
Hello @sahil839, it seems like you have referenced #14165 in your pull request description, but you have not referenced them in your commit message description(s). When you reference an issue in a commit message, it automatically closes the corresponding issue when the commit is merged. Please run An example of a correctly-formatted commit: Thank you for your contributions to Zulip! |
|
Heads up @sahil839, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
timabbott
force-pushed
the
main
branch
2 times, most recently
from
4ec3636 to
88b200c
Compare
User is added to deactivated users list when user is deactivated from
active users list and it remains in the active users for re-toggle
(in case of mistake) and same happens for reactivating users from
deactivated users list.
Whenever user enters any of the list from somehwhere else, list is
up-to-date.
Fixes #14165
Currently, the last active on reactivation gives Unknown as some change is needed in presence code and issue is opened currently #14279.
I am working on that issue and will soon make a PR.
Testing Plan: This is manually tested.
GIFs or Screenshots:
