Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This merges the `exports.get_search_result_legacy` and
`exports.get_search_result` function.
The key differences between the two code paths are as follows:
* We only want to generate suggestions for the queries which
the user is typing or can edit.
For the legacy version, suggestions are displayed for the
entire search string in the searchbox. (`all_operators`)
For the pills enabled version, suggestions are displayed
only for the input which hasn't been converted to pills.
(`query_operators`)
`all_operators` = `base_query_operators` + " " + `query_operators`.
trim is added at the end just to handle the legacy case
where we pass the `base_query` as ''.
* It is not possible to detect whether the user wants to
continue typing in the legacy version. However if the
the searchbox is still focused even after pill creation
we can assume the user still wants to continue typing.
To handle this we push an empty term as the `last` operator.
This is possible since the previous queries have been
completely entered as evident from it's generated pill.
* When using the legacy version, `search_operators` are
the same as `all_operators`, as mentioned in point 1.
In the pills enabled version we perform most of the
computations from the `query_operators`, but we do
require all `all_operators`, only for filtering the last
query's suggestion.
* And there is just one block unique to the legacy search
system. More details are mentioned in the comments of that
block.
We also refactor both the search suggestions node tests,
mainly to make them similar and easier to detect differences
when we switch over to the new version.
This makes it so that search_suggestion.js
does not depend on activity.js.
That dependency hasn't really been "elegant"
for quite some time, but it will become particularly
unnecessary when we go to remove the "Group PMs"
section from the right sidebar.
This commit introduces a temporary wart
where we have these two functions with the
same name in a sort of unnecessarily
complicated code stack:
activity.process_loaded_messages
huddle_data.process_loaded_messages
But we will eliminate the former function
very soon, and our message-related codepaths
will just call the `huddle_data` version
directly.
TESTING NOTES:
Now that `huddle_data` is a tiny leaf
module, it's super easy to just use the
real implementation of what was formerly
called `activity.get_huddles()` (and is
now in `huddle_data`).
When I first wrote this commit, introducing
the real implementation of `get_huddles` exposed
some bugs that I fixed in the immediately
prior commits to this.
When the tests were originally written,
I believe `activity.js` had some annoying
`jQuery` dependencies that made it hard
to unit test against. We've slimmed it over
time to be mostly just a "controller" module.
But even in its current state it would have
been a bit of a bloated dependency.
The other friction for using the actual
version of `get_huddles` was setting up
the message data, but that's pretty minor.
If you have a group PM where some users have
three-digit user_ids and some with four-digit
user_ids (or similar), a huddle could effectively
be ignored when determining the order of
search search suggestions.
Basically, we need a way to canonically sort
user_ids in "huddle" strings, and it's somewhat
arbitrary whether you sort lexically or sort
numerically, but you do need to be consistent
about it.
And JS is not exactly helpful here:
> [99, 101].sort()
[ 101, 99 ]
This is a pretty obscure bug with pretty low
user-facing consequences, and it was never
reported to us as far as I know, but the fix
here is pretty straightforward.
We have had similar bugs of slightly more consequence
in the past. The reason this bug has shown
up multiple times in our codebase is that every
component that deals with huddles has slightly
different forces that determine how it wants
to serialize the huddle. It's just one of those
annoying things. Plus, bugs with group PMs
do tend to escape detection, since most people
spend most of their time either on streams
or in 1:1 PMs.
This creates a little bit of noise in some
tests where we don't care about users, but
it's worth avoiding confusion about which
users exist at which time. Also the noisy
aspects here may actually catch regressions.
Finally, if the noise gets annoying, we can
do things like rename "Ted" not to collide
with the "Test" stream.
Using "bob" as the current user was a bad
choice, as our convention is to use "me" or
"myself" or "alice" for the current user.
It also particularly complicated the tests
around Group PMs.
Now we have both "bob" and "myself", which
makes the intentions of the tests a little
more clear.
`stream_topic_history` is a more appropriate name as this
module will contain information about last message of a
stream in upcoming commits. Function and variable names
are changed accordingly like:
* topic_history() -> per_stream_history()
* get_recent_names() -> get_recent_topic_names()
* name -> topic_name
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`.
Explicitly stubbing i18n in 48 different files
is mostly busy work at this point, and it doesn't
provide much signal, since often it's invoked
only to satisfy transitive dependencies.
We now treat util like a leaf module and
use "require" to import it everywhere it's used.
An earlier version of this commit moved
util into our "shared" library, but we
decided to wait on that. Once we're ready
to do that, we should only need to do a
simple search/replace on various
require/zrequire statements plus a small
tweak to one of the custom linter checks.
It turns out we don't really need util.js
for our most immediate code-sharing goal,
which is to reuse our markdown code on
mobile. There's a little bit of cleanup
still remaining to break the dependency,
but it's minor.
The util module still calls the global
blueslip module in one place, but that
code is about to be removed in the next
few commits.
I am pretty confident that once we start
sharing things like the typeahead code
more aggressively, we'll start having
dependencies on util. The module is barely
more than 300 lines long, so we'll probably
just move the whole thing into shared
rather than break it apart. Also, we
can continue to nibble away at the
cruftier parts of the module.
This commit includes a new `stream_post_policy` setting,
by replacing the `is_announcement_only` field from the Stream model,
which is done by mirroring the structure of the existing
`create_stream_policy`.
It includes the necessary schema and database migrations to migrate
the is_announcement_only boolean field to stream_post_policy,
a smallPositiveInteger field similar to many other settings.
This change is done to allow organization administrators to restrict
new members from creating and posting to a stream. However, this does
not affect admins who are new members.
With many tweaks by tabbott to documentation under /help, etc.
Fixes#13616.
We only ever show 3 or 4 people in search suggestions
(possibly w/a couple variations, like pm-with/sender/etc.),
so we can try to search a smaller subset of people
before going through the entire realm.
We use message_store.user_ids() for this, since you
typically want to search messages for people that
have sent messages recently, and we already sort
based on PM conversations.
Once we have max_items results, stop trying
to get more items.
This should really help large realms when
you do a search on streams that turns up
more than N streams (where N is about 12).
We won't even bother to find people.
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>
Add ability to search entire message history of all public streams at
once. It includes all subscibed, non subscribed public streams messages
and even historical public stream messages sent before user had joined
an organization or stream.
Fixes#8859.
Fix the .get_suggestions and .get_suggestions_legacy
to correctly handle search terms in group PM and treat
it as search term by not concatenating it at end of pm-with
email list operand.
This large function will need to be modified significantly as part of
the pills effort, and copying it lets us preserve behavior in
production until we're ready to cut things over.
This forks off search_legacy.js and search_suggestion_legacy.js so
that we can continue running automated tests against the legacy search
code while we develop the input pills feature.
Anders Kaseorg
Ryan Rehman
clarammdantas
Steve Howell
Hashir Sarwar
Steve Howell
Anders Kaseorg
Mohit Gupta
Pragati Agrawal
Tim Abbott