The main thing here is that we check that the
actual data got put into our data structures.
(In general we want to move away from stubbing
data modules; any place where we stub data modules
is a relic of earlier days, where we were just
trying to set the bar for 100% line coverage,
even though some of the original coverage was
quite shallow.)
I also use real stubs instead of noops for
the calls out to UI-oriented modules.
In passing I tweak some comments in the actual
dispatch code.
With update_emojis, it is pretty easy
to set up the tests with reasonable
data without reaching into internal
data structures.
Also, we can begin the process of
sharing the same data with our
dispatch tests (upcoming).
We want to undo overrides in reverse order,
which is important if you override the
same name more than once in the same
function.
Until today the code basically prevented
us from ever using the original implementation
of a name we stubbed, and most of them start
as undefined due to their parent modules
starting with `set_global`.
But I do want this proper, and I introduced
a tiny pitfall today.
There was only one place where we weren't
overriding a function, and the use case there
was fairly unique.
Knowing that we're dealing with only functions
will simplify override and allow us to add
features like detecting spurious stubs.
This forces us to more explicitly document at the
top of the file what dependencies we are stubbing,
plus it's less magical.
Also, we may want to do occasional audits of
set_global to clean up places where we mock
things like stream_data, which are probably just
easier to use the real version of now that we
have cleaner APIs to set up stream data.
The modules most affected by this change are our
dispatch-oriented tests--basically, all the
modules that test handling of Zulip events
plus hotkey.js.
Before we were making it impossible to reuse
the function again (so we were preventing
leaks), but it's fine to just restore the
original function, especially now that some
of our tests have grown bigger.
As of commit 87e72ac8e2 (#15267), we
need to be an owner for some of the tested functionality, not just an
administrator.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Commit 7c0fa3aefc (#15734) added sample
alert words to the test database, so the Casper test can no longer
assume its alert word is the only one.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
$.fn.typeahead, on the other hand, returns the jQuery object back (not
the Typeahead object, which also happens to have a select method), so
this should be converted.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Zulip converts :) to the 1F642 Unicode emoji and promotes the same emoji
in the popular section of the emoji picker.
Previously Zulip has labeled 1F642 as "slight smile". While that name
conforms to the Unicode standard (which describes the code point as
SLIGHTLY SMILING FACE), it didn't match our use case of the emoji.
If a user types :) or selects the first smile in the emoji picker they
probably mean to express a regular "smile" and not a "slight smile",
which raises the question why they are only smiling slightly.
This commit relabels 1F642 as 😄 and our previous 😄 263A as
:smiling_face:. Note that 263A looks different in our three supported
emoji sets, so it is not suited to be our "default smile".
This change does not require a migration since our emoji system stores
both unicode points and names and handles name changes transparently.
Previously, image upload widget delete button CSS class name was
`settings-page-delete-button`.
We can change the CSS class name to `image-delete-button`
so that the name can be more generic.
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>
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>
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>
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>
We hide the spoiler content in browser/desktop notifications.
Note: its not worth adding zjquery tests for this bit of code because
the tests do not operate on the actual data and are likely to get stale
if we change the syntax for spoilers.
This handler adds a neat little effect whereby hovering over the
clickable region to open the navbar triggers the search_icon hover
effect and is a neat little visual cue about what happens onClick.
The previous implementation was slightly messy because it fetched the
color and applied it via ".css(". This commit cleans it up by creating
and using the class "search_icon_hover_highlight" instead. We also
make the selectors more specific, ensuring they target children of
"#tab_bar", this was so because it was reasonable to expect someone to
define eg `search_closed` elsewhere and we wanted to prevent bugs when
that happened.
We were not passing any arguments to needs_subscribe_warning
when testing the function itself.
This commit changes the code to pass the user-id and stream-id
to needs_subscribe_warning. We also remove the stubs for
get_by_user_id and is_user_subscribed and do these tests by
calling the original functions, because passing the arguments
(user-id and stream-id) only makes sense if we use original
functions for them rather than stubs.
Delete stored topic data in `recent_senders` and `recent_topics`
about the message's topics and re-render them. The process is similar
to topic editing. See `recent_senders.process_topic_edit` for
logical details.
We have changed our all instances of list_render to use
simplebar and thus, we will now use simplebar container
to track scroll event for all the lists created by
list_render.
This fixes the bug of new subscribers not rendering on
scrolling at the end of subscriber list in stream settings
and similar bug in some other lists also.
This commit also removes scroll_util.get_list_scrolling_container
function as this is no longer used.
Fixes#15637.
This reverts commit 63643c9d9d.
As the commit mentions, it makes a UI change for legacy search which
has largely been considered a regression. We've been running with
this reverted in zulip.com essentially since it was first merged.
Prior to commit eb4a2b9d4e the center
area of the navbar was based on a structure that appended crumbs or
"tabs" as <li>s, forming a tab_bar and a tab_list.
However, in eb4a2b9d4e we apply a new
style and structure to the navbar which lets go of the convention of
tabs. Hence, we'd like to purge the tab_bar and tab_list labels from
our code base.
It would have been nicer if we could simply purge tab_bar from the
codebase and rename "#tab_list" so that we have an anchor and wrapper
structure in the html, but dropping the float: left on tab_bar causes
some confusing problems such as causing the horizontal border to
disappear and the search_box to shift out of its intended position and
so its simpler to get rid of tab_list from our code base first.
This commit:
- Removes the #tab_list wrapper div from tab_bar.hbs.
- Removes any #tab_list selectors from night_mode.scss so that they
simply target based on "#tab_bar" instead of "#tab_bar #tab_list".
- Removes tab_list selectors from zulip.scss, so that #tab_list
attributes now apply to the #tab_bar, in the process we drop the
duplicated width property and reorder the attributes.
- Replaces all mention of #tab_list with #tab_bar in JS files.
Previously, the navbar sub count would not live update as users
subscribed or unsubscribed, this commit adds the relevant calls in
stream events.
It would have been better to just have a single call within
server_events_dispatch but it seems difficult due to the way of
mark_subscribed and mark_unsubscribed are structured.
stream_events.mark_unsubscribed conditionally calls
subs.update_settings_for_unsubscribed which calls
subs.rerender_subscriptions_settings and as such handles the update
for the subscriptions modal on its own. Hence, we simply rely on the
stream_data.update_calculated_fields to ensure the subscriber counts
are updated and make a call to
tab_bar.maybe_rerender_title_area_for_stream(sub).
stream_events.mark_subscribed is similar.
Previously, we were overriding narrow_state.is_for_stream_id() to make
sure we test the functions we intend to.
In this commit we:
* zrequire('narrow_state')
* set the filter to "stream:frontend" before the test cases which
were overriding is_for_stream_id to return true (and remove the
overrides).
* reset_current_filter() at the end of the above cases (and remove
the lines overriding is_for_stream_id to return false)
This is a prep commit to adding live update for sub_count in the
navbar.
This commit changes stream_data.is_user_subscribed to use stream id
instead of stream name.
We are using stream ids so that we can avoid bugs related to live
update after stream rename.
We have logic in place to update the ui for re-sending messages
on recieving the acknowledgement from the server on that API call.
However, if the acknowledgement is recieved through the get events
request before the `on_success` of `resend_message`, the message
gets re-rendered allowing the failed message actions to be clickable.
Now, we update the ".message_failed" ui for both cases. This helps
in preventing the "Trying to get local_id from row that has reified
message id" exception.
Fixes#15351.
As we add more features where rendered_markdown.update_elements does
something useful, it'll become important to run this code everywhere
we render markdown in the DOM.
One can see in this case that we had actually copied one hunk of
rendered_markdown.update_elements years ago, before we extracted it as
an independent function; we get to delete that copy.
Fixes#15500.
This particular commit has been a long time coming. For reference,
!avatar(email) was an undocumented syntax that simply rendered an
inline 50px avatar for a user in a message, essentially allowing
you to create a user pill like:
`!avatar(alice@example.com) Alice: hey!`
---
Reimplementation
If we decide to reimplement this or a similar feature in the future,
we could use something like `<avatar:userid>` syntax which is more
in line with creating links in markdown. Even then, it would not be
a good idea to add this instead of supporting inline images directly.
Since any usecases of such a syntax are in automation, we do not need
to make it userfriendly and something like the following is a better
implementation that doesn't need a custom syntax:
` Alice: hey!`
---
History
We initially added this syntax back in 2012 and it was 'deprecated'
from the get go. Here's what the original commit had to say about
the new syntax:
> We'll use this internally for the commit bot. We might eventually
> disable it for external users.
We eventually did start using this for our github integrations in 2013
but since then, those integrations have been neglected in favor of
our GitHub webhooks which do not use this syntax.
When we copied `!gravatar` to add the `!avatar` syntax, we also noted
that we want to deprecate the `!gravatar` syntax entirely - in 2013!
Since then, we haven't advertised either of these syntaxes anywhere
in our docs, and the only two places where this syntax remains is
our game bots that could easily do without these, and the git commit
integration that we have deprecated anyway.
We do not have any evidence of someone asking about this syntax on
chat.zulip.org when developing an integration and rightfully so- only
the people who work on Zulip (and specifically, markdown) are likely
to stumble upon it and try it out.
This is also the only peice of code due to which we had to look up
emails -> userid mapping in our backend markdown. By removing this,
we entirely remove the backend markdown's dependency on user emails
to render messages.
---
Relevant commits:
- Oct 2012, Initial commit c31462c278
- Nov 2013, Update commit bot 968c393826
- Nov 2013, Add avatar syntax 761c0a0266
- Sep 2017, Avoid email use c3032a7fe8
- Apr 2019, Remove from webhook 674fcfcce1
To make the typeahead code more readable, we extract this function to
timerender. We also improve the logic to be more readable, and add tests
to confirm its validity.
We have moved our invalid timestamp logic to use timestamp-error class,
however, if there are any valid outputs by the backend markdown that
the frontend considers invalid, we want to debug them. This commit
adds tooling to ensure we log those error messages.
We had been using !time() syntax for timestamps so far. Since its
an unreleased feature, we can make changes without affecting many
people.
Fixes#15442.
There is a bug and race issue that occurs when a message is selected
while we are in the process of reifying a locally echoed message,
raising the "Selected message id not in MessageList" error.
The code flow to get the exception is as follows:
* A user sends a message to the current narrow we are in.
* Before the new message event is received, we sent a message to
the same message list which renders it with a locally echoed id.
* One of the ways of getting the exception is to already have the
locally sent message selected, before receiving an acknowledgment
from the server.
* Thus the Message List Data's `selected_id` now points to the new
message id. The exception is raised on entering the `was_selected`
if block inside `message_list_view` which tries to re-select the
message.
Updating the `_rerender_message` code for this special case won't fix
the entire bug because, as mentioned above there are other ways of
getting the exception:
Ideally, after all our synchronous work (`echo.process_from_server`)
has completed we would expect the re-order and re-render work of the
`change_message_id` would occur first, due to the timer of the
setTimeout being set to 0.
However as evident from the race condition existing, this isn't always
the case. `change_message_id` function is responsible for 3 things:
updation, re-ordering and re-rendering.
The first one which is responsible for updating the message list's
local cache, occurs synchronously while for the latter two, they both
occur asynchronously.
Before the setTimeout which is responsible for the latter two actions,
is encountered the user might select the message by clicking or more
commonly by scrolling, which causes this message selection event to be
ahead of the setTimeout in the callback queue.
During this time frame, our race condition takes place.
And even though the message id is updated it's Message List is not
in the correct sort order, which leads to `closest_id` !== `id` in
`MessageList_select_id` being true and raising the exception.
Now, we only asynchronously call the re_render function, to guarantee
the data is always correct and UI updates should be done at the end.
Extended by tabbott to comment the setTimeout call.
Fixes#15346.
We change validate_stream_message to check the existence of stream from
the stream name in compose box early and we then pass stream_id or the
obtained sub objects accordingly to other validate functions.
Passing stream_id or sub objects to these functions, enables us to use
stream_id instead of stream name in stream_data.get_subscriber_count.
stream_data.get_stream_post_policy is also removed as we only used it in
validate_stream_message_policy, but we do not need it now as we can get
stream_post_policy directly from sub object obtained by early check of
valid stream name.
This commits add data-stream-id attribute to the compose_invite_users
template. This helps in avoiding the error that occured if user
clicked the link after renaming of stream.
As a result of above changes, the checks for empty and invalid stream
name in compose box are done in warn_if_mentioning_unsubscribed_user
function instead of needs_subscribe_warning function.
Steve Howell
Anders Kaseorg
Gittenburg
jagansivam28
Tim Abbott
Rohitt Vashishtha
YashRE42
Sebastian Morr
sahil839
Aman Agrawal
Ryan Rehman