I lift this function out of message_store to
break some dependencies, and it's also more
consistent with the rest of the codebase:
alert_words.process_message
pm_conversations.process_message
recent_topics.process_messages
recent_senders.process_message_for_senders
We can do further cleanup to make these names
consistent (and possibly have them all work in
bulk), but that's out of the scope of the current PR.
We move the message_store.add_message_metadata function
(and all its dependencies) into a new module called
message_helper and rename the function to process_new_message.
(It does a bit more than adding message metadata, such
as updating our message store.)
We also have a "protected" interface now in message_store,
so that message_helper can access the message store:
update_message_cache
get_cached_message
Because update_message_cache is identical to
the former create_mock_message, we just renamed it
in the tests.
Most callers should use these functions:
message_helper.process_new_message (setting)
message_store.get (getting)
It's slightly annoying that the setter interface
is in a different module than the getter interface,
but that's how you break a bunch of dependencies.
We also extract the tiny message_user_ids class:
user_ids()
add_user_ids()
All the code moves here are pretty trivial, and
the code that was moved maintains 100% line
coverage.
The module name `message_helper` is not ideal, but it's a single
function and it'll save time to just do the topology change now and
leave thinking through the right name to later.
This function should be named similarly to the well-named
end_inline_topic_edit; previously the main hint that this was the
inline code path was just that the recipient_row was the parameter.
Before this we did not have remove event in server_events_dispatch.js
for the user group delete event even though server had. This was
leading to blueslip errors. Extracted the logic which was used in
success() of channel.del for user_groups into the remove case in
server_events_dispatch. Also removed the redundant reload call as
we already do that in server events.
This is a mostly verbatim extraction.
I re-phrased one line of code to work around a lint
false alarm. (Look for `preamble` in the diff.)
There are about 8 lines missing coverage here, so
the new module might be a good candidate to get
100% line coverage on.
Before this change, you would need to remove 74
edges from our dependency graph to make it
acyclic. Now it's 72.
This change does not impact the overall complexity
of our dependency graph (at least in terms of the
number of edges that we would need to remove to get
a tree), but it does clarify the picture a bit.
Rename few functions referencing the "CHOICE" field to
instead use the new "SELECT" name. This is done so that they
can be reused in "SELECT_MULTIPLE" field.
When a user entered an invalid character (whitespace or characters not
present in a name), the cleaned-up array, and hence the query,
would be empty which resulted in an error.
Fixes#17542
We were loading filters from localstorage after rendering the
filters block which caused the incorrect icons being displayed
for filters.
Since usually recent_topics renders a couple times when it
is loaded directly and the filters were being loaded as part of
show_selected_filters after we rendered recent_topics filters,
it meant the correct filters were being displayed in the
second render.
But, when user loads any other narrow directly when Zulip is loaded,
and opens recent_topics, recent_topics is only rendered once,
hence the bug gets noticed.
Fixes#17496
Extend our markdown system to support mentioning of users
by id also. Following these changes, it would be possible
to mention users with @**|user_id** and silently mention
using @_**|user_id**.
Main intention for extending the mention syntax is to make
it convenient for bots to mention a users using their ids. It
is to be noted that previous syntax are also supported.
Documentation tweaked by tabbott for better readability.
The changes were tested manually in development server, and also
by adding some new backend and frontend tests.
Fixes: #17487.
We add support to shorten links and test their shortening in
well-organized, clean manner that makes it trivial to extend the
GitHub approach for GitLab and perhaps other services.
We only shorten basic types of GitHub links (issue, PR, commit) that
fit a set of simple common patterns; the default behaviour of Autolink
is kept for everything else.
Logic added in frontend and backend Markdown Processor is identical.
This makes easy to extend the logic for other services like GitLab.
Fixes#11895.
The only reason to use typeof foo === "undefined" is when foo is a
global identifier that might not have been declared at all, so it
might raise a ReferenceError if evaluated. For a variable declared
with const or let or import, a function argument, or a complex
expression, simply foo === undefined is equivalent.
Some of these conditions have become impossible and can be removed
entirely, and some can be replaced more idiomatically with default
parameters (note that JavaScript does not share the Python misfeature
of evaluating the default parameter at function declaration time).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
These were introduced in ff9a929d7a
with no explanation of why they were necessary.
Generally you only render a few things, and it's
important that they're up to date.
We weren't doing a good job of invalidating the cache.
Eliminating the cache will fix bugs (like presence circles
being out of date) and break some dependencies.
I removed some very fragile test code that was relying
on invalid values taken out of the cache. (We now have
less line coverage, but if we want to test our rendering,
there are much cleaner ways to do it.)
As part of testing this, I renamed Hamlet to "aaron", so
that there are two aarons, and then I logged on as Iago
to see the "secondary" code in action that shows their
emails to distinguish them.
This add the schema checker, openapi schema, and also a test for
realm/deactivated event.
With several block comments by tabbott explaining the logic behind our
behavior here.
Part of #17568.
This helper was added in eac6463031 and
used by the "message.handlebars" file. This is no current call for
this helper in the codebase, hence it is removed to improve coverage.
This commit also marks template.js to have 100% test coverage.
This adds support for unstarring all (starred)
messages from a particular topic, from the topic
popover.
The earlier implementation of this in #16898
was reverted in bc55aa6a01 (#17429)
because it had two problems-
1. The crash reported in bc55aa6a01
was due to message_store returning undefined. This happens
when the message itself hasn't been fetched from the server
yet, but we know that the message is starred from the ids
in `page_params` in `starred_messages.js`.
This commit handles this case explicitly.
Note that, we simply ignore those messages which
we haven't fetched, and because of this, it may
happen that we don't unstar some messages from that
topic. The correct implementation for this would
be to ask the backend for starred IDs in a topic.
2. The earlier implementation actually unstarred **all**
messages. This was because it grabbed the topic and stream_id
from the topic popover `data` attributes, after the topic
popover had been closed. This passed `undefined`, which
the function then interpreted as an action to unstar all
messages.
With this commit, we use the confirm_dialog widget,
which eliminates the need to store this data in the DOM.
* Currently, the confirm_dialog is used only in
the settings pane, which already has the `new-style`
class in the main `settings_overlay.hbs` file. So,
the confirm modal is rendered correctly there. But to
make it available for use outside of the settings pane,
we add the `new-style` class to the confirm container
itself, without which, the buttons look ugly.
* The other change here is the click handler for
removing the modal element. Previously, when the
modal was closed (with any of the "yes"/"no"/"cross"
buttons), there was a small time interval of around
a second during which the modal had disappeared,
but the background content was still in the faded-out
state. This change fixes this glitch. This glitch was
probably not noticable earlier, because the settings
pane itself causes the background to be slightly faded
out.
This code was added in 2d414fa897, after
the `window.exports` variable was removed from
`stream_popovers.js`, while converting it to an ES6 module
in c71af35461. This resulted
in opening the starred messages or all messages
popovers throw `Error: exports in undefined.`.
Follow up to #14768. This feature was already non-functional due to
.alert-display { display: none; }, and if we want to reimplement it,
we should do it using a modern library.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Remove the unused notifications-area wrapper. Remove the feature
detection code as all browsers recognize the <audio> element. Create
the <audio> statically with the page template. Use multiple <source>s
to let the browser detect the appropriate format instead of trying to
do its job for it. Remove the absurd loop="yes" attribute, which had
fortunately been specified on the wrong element.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This is mostly a refactoring to break the unnecessary
dependency of bot_data on settings_bots.
This is a bit more than a refactoring, as I remove all
the debounced calls to render bots during the
initialization of bot_data. (The debouncing probably
meant we only rendered once, but it was still needless
work.)
We don't need to explicitly render bots during
bot_data.initialize(), which you can verify by loading
"#settings/your-bots" as the home page. It was just an
artifact of how add() was implemented.
Note that for the **admin** screen, we did not and
still do not do live updates for add/remove; we only do
it for updates. Fixing that is out of the scope of this
change. The code that was moved here affects
**personal** bot settings.
Note that the debounce code is quite fragile. See my
code comment that explains it. I don't have time to go
down the rabbit hole of a deep fix here. The puppeteer
tests would fail without the debounce, even though I
was able to eliminate the debounce in an earlier
version of this fix and see good results during manual
testing. (My testing may have just been on the "lucky"
side of the race.) I created #17743 to address this
problem.
The keyboard-shortcuts icon currently has a fix position
causing design related bugs such as overlapping with userlist
in the sidebar.
The fix wraps the invite-more-users link and keyboard icon inside
a div with display property as flex instead of just using the anchor
tags inside the side-bar items.
This mainly extracts a new module called
browser_history. It has much fewer dependencies
than hashchange.js, so any modules that just
need the smaller API from browser_history now
have fewer transitive dependencies.
Here are some details:
* Move is_overlay_hash to hash_util.
* Rename hashchange.update_browser_history to
brower_history.update
* Move go_to_location verbatim.
* Remove unused argument for exit_overlay.
* Introduce helper functions:
* old_hash()
* set_hash_before_overlay()
* save_old_hash()
We now have 100% line coverage on the extracted
code.
I moved four functions, verbatim, to a new module.
They were in message_util before, which led to
filter.js having several accidental indirect
dependencies.
I considered just putting these four functions in
filter.js, but I think it's a nice abstraction boundary
that filter.js delegates actual message parsing, and
the original author apparently had a similar thought
process.
I also wanted to make it so that a casual reader of
filter.js doesn't think we are manipulating DOM. It's
true that we still indirectly require jquery here, but
it's only for parsing, and it seems plausible we would
eventually use a more low-level parser.
I can see us maybe using these functions in something
like MessageListData in the future, so speculatively
splitting them out might future-proof us from some
cyclical dependencies.
I also think it's plausible that we will just modify
our two markdown processors to attach that kind of
metadata to the messages.
Last but not least, I think there might be opportunity
here to simplify the filter tests and remove some of
the zjquery hacks. We would instead just mock the
message_has_* helpers for the filter tests, and then
do more detailed direct testing on the functions
themselves.
The only caller for this function was settings_config,
so we put it there.
For the stream_edit test we no longer mock the function.
(The reason we mocked the function was more about avoiding
the heavy settings_notifications import than the function
itself.) This gives some incidental coverage, but then I
also add some more real coverage on it.
We extract compose_fade_users and compose_fade_helper.
This is a pretty verbatim extraction of code, apart from adding a few
exports and changing the callers.
This change makes the buddy_data module no longer sit "above" these
files in the dependency graph (at least not via compose_fade):
* jquery
* lodash (not a big deal)
* compose_state
* floating_recipient_bar
* message_viewport
* rows
The new moules have dependencies that buddy_data already
had directly for other reasons:
* people
* util
And then buddy_data still depends on stream_data indirectly through
the compose-fade logic for stream_data. Even without compose-fade, it
would depend indirectly on stream_data via hash_util.
Note that we could have lifted the calls to compose_fade out of
buddy_data to move some dependencies around, but it's useful to have
buddy_data fully encapsulate what goes into the buddy list without
spreading responsibilities to things like activity.js and
buddy_list.js. We can now unit-test the logic at the level of
buddy_data, which is a lot easier than trying to do it via modules
that delegate drawing or do drawing (such as activity.js and
buddy_list.js).
Note that we still don't have 100% line coverage on the
compose_fade.js module, but all the code that we extracted now is
covered, mostly via buddy_data tests.
This commit adds vertical-align: middle to .message_failed in zulip.css
which was necessary as the alignment of .message_failed wasn't matching
with rest of the message controls like .edit_content. This makes the
look of the message controls better that they don't look shifted.
Follow up #17666
Previously, it was tedious to create actual message
objects in message_store for use in node tests.
This was mainly because, `add_message_metadata`
in message_store has many dependencies and
validation checks. Since it was difficult to create
actual message objects, many tests just mocked
the `message_store.get()` method to return the desired
message.
This commit adds a new helper method (`create_mock_message`)
to message_store, for use in node tests. This just stores
the object passed to it in the `stores_messages` map,
without any validation. We do not add any
default fields to the message object before saving
it from this helper, because doing so would decrease
the utility of this helper, and, if a test
depends on some field having a particular value,
then it would be better to just pass the field: value
pair from the test itself, for readability, rather
than relying on the helper to add the field for us.
This helper allows us to write deeper tests.
This commit also replaces some instances of mocking
`message_store.get()` to use this new helper method.
Previously, if a user had zero total starred messages,
we would still show the "Unstar all messages" in the
left sidebar on opening the starred messages popover.
This commit adds a check to show button only if the
user had non-zero starred messages. This is done
because-
1. The button, when shown when the user has zero
starred messages, is redundant and may be confusing.
2. Clicking on the button when having zero starred
messages sends a zero-length array to the backend,
resulting in HTTP 400 error.
Computed indexing into an object, especially with a user-provided key,
can be dangerous in JavaScript because of nonsense features like
obj["__proto__"]. In this case there’s no vulnerability because the
possible keys are strictly limited by the regex, but it’s always
better practice to use a Map for computed indexing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit removes the unless msg/locally_echoed condition for the
edit content div, which has the consequence of making the "view
message source" widget always available for locally echoed
messages. This ensures that the message source can be seen if a very
long message has been drafted and it fails due to a server-side error
(See #17425 for the original report).
Fixes#17650.
This commit takes the blocks of code from "build_message_groups" that are the
same as "_rerender_message", and move those into a function called
"set_calculated_message_container_variables". This helps to avoid bugs in
future as in #17663. Like timestr was being updated in one of them, but needed
in both. So, it takes care that message variables are correctly set.
Part of #17663
This commit updates the _rerender_message to update the message_time
string with the current timestamp on the message rerender.
When we locally echo a message, we store a local timestamp that will
generally not be used as it is replaced by the server time in
echo.process_from_server when we confirm receipt of the message.
echo.process_from_server correctly updates the .timestamp field on
the message and triggers a rerender but that rerender reuses
the message_container object without recomputing the
message_container.timestr due to which wrong older timestr was shown
on the message box.
This commit fix this by calling set_timestr in the rerender code path,
alongside calls to update similar data structures like
this._maybe_format_me_message.
Fixes#17655
In responsive narrow windows where the left sidebar is an overlay, clicking the \vdots menus for
'All messages' and 'Starred messages' would result in the navigation closing and the menu appearing
somewhere weird.
We fix this the same way that we address this issue with the similar stream/topic menus, by calling
the function to show this sidebar after closing all popovers.
Fixes: #17537.
The custom-profile-fields-form element custom_user_field contains
the textarea for Biography that expands. The textarea treats the
user-avatar-section as an disturbing obstacle when expanded beyond
the certain width. To fix this, the custom-profile-fields-form
is placed out from the account-settings-form.
Fixes#17617.
Fixes#17466
This commit will change encoding logic. Initial logic
was not encoding parenthesis, and this creates conflicts
with the markdown link format. To resolve this while encoding,
we're now replacing parenthesis with ".28" and ".29."
There is no need to change decoding logic because before
decoding any URL, we first convert all the “.” to “%.”
optimization: No need to replace parenthesis in popovers.js.
The scroll position of recent topics table is according to the
element which is in focus.
While this behaviour is correct, when
user clicks on an element in recent topics after scrolling to
a different position, the scroll position is lost as the focus
was not being set on the element. This commit ensures that
we set focus on the element when user clicks on it. Thus, the
scroll position being lost is naturally fixed.
Fixes#17587
This commit removes the option to add more streams out of scrollbar
as it is not visible on mobile devices or organizations with large number of
streams until scrolled down.
Steve Howell
Tim Abbott
Anders Kaseorg
tushar912
Aman Agrawal
Sumanth V Rao
PIG208
Gaurav Pandey
Vishnu KS
Abhijeet Prasad Bodas
Ganesh Pawar
Nishant Mittal
m-e-l-u-h-a-n
akshatdalton
YashRE42
smit_patel
shanukun
Signior-X
Megamind
100RABHpy
yasiruRathnayaka97
Nikhil Maske
pilgrim2308