For some widgets we now avoid duplicate redraw
events from this old pattern:
widget = list_render.create(..., {
}).init();
widget.sort(...);
The above code was wasteful and possibly
flicker-y due to the fact that `init` and
`sort` both render.
Now we do this:
widget = list_render.create(..., {
init_sort: [...],
});
For other widgets we just clean up the need
to call `init()` right after `create()`.
We also allow widgets to pass in `sort_fields`
during initialization (since you may want to
have `init_sort` use a custom sort before the
first render.)
Giving these functions a name and moving them to
the top-level scope has a couple tactical advantages:
- names show in tracebacks
- code is less indented
- setup code is less cluttered
- will be easier to add unit tests
- will make some upcoming diffs nicer
These are technically more `compare_foo` than `sort_foo`,
but we already had a naming convention that was sort of
in place.
Clicking on the 'Owner' value for a row in the list of bots does
nothing, and causes a blueslip error.
This is because the map object in which we store the users have
integer keys, while we pass the owner id as string.
This is fixed by parsing the owner id to integer before passing it
on.
Fixes#14107.
This extracts a new module with three
functions, which we will test with 100%
line coverage:
- show_email
- email_for_user_settings
- get_time_preferences
The first two break several dependencies
in the codebase on `settings_org.js`. The
`get_time_preferences` breaks an annoying
dependency on `page_params` within people.
The module is pretty cohesive, in terms that
all three functions are just light wrappers
around `page_params` and/or `settings_config`.
Now all the modules that want to call show_email()
only have to require `settings_data`, instead of
having a dependency on the much heavier
`settings_org.js` module.
I also make some of the unit tests here be more
full-stack, where instead of stubbing show_email,
I basically just toggle `page_params.is_admin`.
We mostly needed this for Casper tests, and that
usage was eliminated in the prior commit.
There was also some strange defensive code from
ecc42bc9f8 that
is really ancient and which I am eliminating:
const email = row.attr("data-email");
if ($("#deactivation_user_modal .email").html() !== email) {
blueslip.error("User deactivation canceled due to non-matching fields.");
ui_report.message(i18n.t("Deactivation encountered an error. Please reload and try again."),
$("#home-error"), 'alert-error');
}
If the code was there to protect against live
updates for email changes, then we no longer
have to worry about that, since we use user_ids
now as keys.
Or it might have to do with some ancient bug
where you could pop open two modals at once
or something. You can actually change users while
the modal is open (which is kinda strange, but ok),
and it works fine.
When testing this, I ran into the glitch that we
don't open redraw the Deactivated Users panel after
going into the User panel and deactivating a user.
This change sets us up to optimize how we
filter users in the admin user settings.
See #13554 for more context on the user
facing issues.
This fix is basically three related things:
- Add filterer options to list_render.
- Add helper method to people.js.
- Use filterer in settings_users.js.
The filter "callback" was only a "callback" in the
most general sense of the word.
It's just a filter predicate that returns a bool.
This is to prepare for another filtering option,
where the caller can filter the whole list
themselves. I haven't figured out what I will name
the new option yet, but I know I want to make the
two options have specific names.
This fixes the error where we pass `user_id` of 'string' type as the
argument instead of 'integer' to `exports.get_person_from_user_id` which
further passes `user_id` to InDict.has() function which accepts integer
argument only.
We want to be able to unit test this value,
since it's conditional on several factors:
- am I an admin?
- can non-admins view emails?
- do we have delivery_email for the user?
I'm mocking show_email in the tests, since the
show_email code is in `settings_org` and
kind of hard to unit test. It's not impossible,
but it's too much for this commit. (Either
we need to extract it out to a nice file or
deal with mocking jQuery. That module is
mostly data-oriented, so it would be nice
to have something like `settings_config` that
is actually pure data.)
This was duplicate code. I'm moving it to people
for pragmatic reasons--it's hard to unit test stuff
in settings_users.js due to all the jQuery.
It's also nice to have all people-related search
code in one place, just for auditing purposes.
It appears c28c3015 caused a regression where we
set `email` to undefined if a user does not have
`delivery_email` set, and this causes filtering
of users to fail for admins doing user settings.
This fixes only one of the issues reported in
issue #13554.
There's probably no easy fix to scrolling taking
long, but I think fixing search will mostly
address that complaint.
The Rust folks seem to agree with me that the
search results are too noisy. If I search for
"s" I get:
* names like Steve (good)
* names like Jesse (noisy)
* anybody with s in their email (super noisy)
Here is the relevant code:
return (
item.full_name.toLowerCase().indexOf(value) >= 0 ||
email.toLowerCase().indexOf(value) >= 0
);
This fixes two regressions in 1946692f9a.
The first bug was actually introduced much earlier, namely that we
were not sending a `bot_owner_id` field at all for bot users without
an owner. The correct behavior would have been send `None` for the
owner field.
The second bug was simply that we needed to update the webapp to look
for the `bot_owner_id` field, rather than an old email-address format
`bot_owner` field.
Thanks to Vinit Singh for reporting this bug.
This commit was originally automatically generated using `tools/lint
--only=eslint --fix`. It was then modified by tabbott to contain only
changes to a set of files that are unlikely to result in significant
merge conflicts with any open pull request, excluding about 20 files.
His plan is to merge the remaining changes with more precise care,
potentially involving merging parts of conflicting pull requests
before running the `eslint --fix` operation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
With webpack, variables declared in each file are already file-local
(Global variables need to be explicitly exported), so these IIFEs are
no longer needed.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
It seems `presence.presence_info[item.user_id]` works fine for the current
user as well and there is no need to hardcode extra condition for the
current user.
In the emails-hidden case, for non-admins, we should remove the email
field from "Users" list in the organization settings page.
Tweaked by tabbott to correctly handle the bots and deactivated users pages.
Mostly rewritten by Tim Abbott to ensure it correctly implements the
desired security model.
Administrators should have access to users' real email address so that
they can contact users out-of-band.
With perfectScrollbar, we needed to call a function from JavaScript to
enable a scrollbar on a new element, but simplebar has a much simpler
default API one can do by using data-simplebar attributes in the HTML.
So we can delete all the scrollbar creation/deletion code.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This also remove:
- meta.current_bot_element: As usage of meta has been wrongly exploited, we
should refrain us from using meta this way i.e. to share variable between
function using the global variable, as they reduce code readability.
- update_view_on_deactivate_reactivate_failure: Again to deduplicate the the
code we're compromising with readability which isn't worth it here, also
we need to this because we have removed above meta key.
We should pass row as an argument to update_view_on_deactivate because we
update deactivate view of a row when the user get activated/deactivated by
the event system.
This also removes a redundant data variable.
Steve Howell
Stefan Weil
Tim Abbott
Anders Kaseorg
Pragati Agrawal
Gaurav Thapar
Thomas Ip
Yashashvi Dave
Joshua Pan
Hemanth V. Alluri
Shubham Dhama