Apparently, local rendering of previews had broken sometime in the
last few months in a refactoring that resulted in us passing a string,
rather than an object, into markdown.js.
This commit extract send_messages.js to clean up code related
to the following things:
* sending data to /json/report_send_time
* restarting the event loop if events don't arrive on time
The code related to /json/report changes the following ways:
* We track the state almost completely in the new
send_messages.js module, with other modules just
making one-line calls.
* We no longer send "displayed" times to the servers, since
we were kind of lying about them anyway.
* We now explicitly track the state of each single sent
message in its own object.
* We now look up data related to the messages by local_id,
instead of message_id. The problem with message_id was
that is was mutable. Now we use local_id, and we extend
the local_id concept to messages that don't get rendered
client side. We no longer need to react to the
'message_id_changed' event to change our hash key.
* The code used to live in many places:
* various big chunks were scattered among compose.js,
and those were all moved or reduced to one-line
calls into the new module
* echo.js continues to make basically one-line calls,
but it no longer calls compose.report_as_received(),
nor does it set the "start" time.
* message_util.js used to report received events, but
only when they finally got drawn in the home view;
this code is gone now
The code related to restarting the event loop if events don't arrive
changes as follows:
* The timer now gets set up from within
send_messages.message_state.report_server_ack,
where we can easily inspect the current state of the
possibly-still-in-flight message.
* The code to confirm that an event was received happens now
in server_events.js, rather than later, so that we don't
falsely blame the event loop for a downstream bug. (Plus
it's easier to just do it one place.)
This change removes a fair amount of code from our node tests. Some
of the removal is good stuff related to us completing killing off
unnecessary code. Other removals are more expediency-driven, and
we should make another sweep at ramping up our coverage on compose.js,
with possibly a little more mocking of the new `send_messages` code
layer, since it's now abstracted better.
There is also some minor cleanup to echo.resend_message() in this
commit.
See #5968 for a detailed breakdown of the changes.
This commit specifically addresses the issue when in preview mode,
while "enter sends" is enabled. Previously the messages were just
sent, now they must pass validation.
Fixes#5574.
This function no longer sets properties to false, so the supported
way of doing this is to instead use prop(foo, false). Some tests
had to be fixed to accommodate this.
I pushed a bunch of commits that attempted to introduce
the concept of `client_message_id` into our server, as
part of cleaning up our codepaths related to messages you
sent (both for the locally echoed case and for the host
case).
When we deployed this, we had some strange failures involving
double-echoed messages and issues advancing the pointer that appeared
related to #5779. We didn't get to the bottom of exactly why the PR
caused havoc, but I decided there was a cleaner approach, anyway.
This change has us tracking messages as soon as we start
sending the message to the server. The next step is to
reconfigure the timeouts a bit to deal with the server not
responding.
We now use a client-side message id to track the state of our
sent messages. This sets up future commits to start tracking
state earlier in the message's life cycle.
It also avoids ugly reify logic where we capture an event to
update our data structure to key on the server's message id
instead of the local id. That eliminates the node test as well.
Another node test gets deleted here, just because it's not
worth the trouble with upcoming refactorings.
This mostly sets the stage for a subsequent commit to start
using client_message_id as the key into sent_messages.
It has the nice side effect of making it more explicit that
certain things should always happen when transmit_message()
succeeds.
This commit does regress our node test coverage a bit.
This commit starts to decouple client_message_id from local_id.
We don't really take advantage of the decoupling in this
commit--in fact, it's a bit of a pain at first. But this should
be a fully working checkpoint commit.
This is mostly straightforward moving of code out of compose.js.
The code that was moved currently supports sending time
reports for sent messages, but we intend to grow out the new
module to track more state about sent messages.
The following function names in this commit are new, but their
code was basically pulled over verbatim:
process_success (was process_send_time)
set_timer_for_restarting_event_loop
clear
initialize
All the code in the new module is covered by previous tests that
had been written for compose.js. This commit only modifies
a few things to keep those tests.
The new module has 100% node coverage, so we updated `enforce_fully_covered`.
We are deprecating local_id/local_message_id on the Python server.
Instead of the server knowing about the client's implementation of
local id, with the message id = 9999.01 scheme, we just send the
server an opaque id to send back to us.
This commit changes the name from local_id -> client_message_id,
but it doesn't change the actual values passed yet.
The goal for client_key in future commits will be to:
* Have it for all messages, not just locally rendered messages
* Not have it overlap with server-side message ids.
The history behind local_id having numbers like 9999.01 is that
they are actually interim message ids and the numerical value is
used for rendering the message list when we do client-side rendering.
The piece of code is dead since there can be no instance where
email === ''. This is ensured by util.extract_pm_recipients
by filtering for empty strings in the pm_recipients list.
This piece was dead because exports.send_times_data[message_id]
cannot be undefined since the only place this function is called
from is exports.report_as_received() and that function has a call
to mark_end_to_end_receive_time() before a call is made to the
function in question for dead code. The function call to
mark_end_to_end_receive_time results in
exports.send_times_data[message_id] = {} if this was not defined
already. So there can be no instance where we end up the code
being removed.
There is no reason to render the template for compose mention
warnings if the user is already in the widget.
This commit also restructures the unit test significantly to more
carefully exercise each case, particularly in regard to when
templates get rendered.
In this commit we remove a small piece of dead code from
check_stream_for_send() function and also rename it to
check_unsubscribed_stream_for_send() which makes more sense.
The file input used for attaching files and images was not being reset
after each use. This resulted in irregular behaviour (sometimes failure)
in attaching the same file consecutively.
This fixes the bug in the reset method.
Fixes#5074.
This commit removes all references to feature_flags.local_echo.
It's been a core feature for about four years, so I think we
can safely say the experiment was successful.:)
This doesn't quite complete the goals of #4650, which has a plan for
how to remove this entirely, but it causes this problematic code to
now be contained to a very rare case.
Refactored significantly by tabbott just due to rebase age.
Fixes#3629.
The function check_stream_existence() temporarliy got moved
to stream_create.js, and our call from compose.js was still trying
to find it in subs.js. Now we move the function to compose.js,
since we no longer use it stream_create.js.
This function is pretty dubious, and we may want to only check
for duplicate stream names locally.
Reusing code from the main compose_message component so that resizing now
behaves correctly. This means that when the user tries to resize vertically,
the autoresize code is disabled, and the textbox reverts to manual resizing.
Fixes#4573
This code makes the right pane work in "Manage Streams" when
you are editing a stream subscription. It handles basic
functionality (submitting forms, etc.), live updates, and
showing the pane as needed.
Most of the code here was simply moved from subs.js, but some
functions were pulled out of larger functions:
live update:
add_me_to_member_list
update_stream_name
update_stream_description
collapse/show:
collapse
show_sub
We also now export subs.show_subs_pane.
We eventually want stream_edit not to call into subs.js, and
this should be fairly easy--we just need to move some shared
methods to a new module.
We now only call compose_state.composing() in a boolean context,
where we simply care whether or not the compose box is open. The
function now also returns true/false.
Callers who need to know the actual message type (e.g. "stream" or
"private") now call compose_state.get_message_type().
This is mostly just moving methods out of compose.js.
The variable `is_composing_message`, which isn't a boolean, has
been renamed to `message_type`, and there are new functions
set_message_type() and get_message_type() that wrap it.
This commit removes some shims related to the global variable
`compose_state`; now, `compose_state` is a typical global
variable with a 1:1 relationship with the module by the same
name.
The new module has 100% line coverage, most of it coming
via the tests on compose_actions.js. (The methods here are
super simple, so it's a good thing that the tests are somewhat
integrated with a higher layer.)
Some Handlebars strings contained whitespaces characters at their ends.
With this, such characters are removed, as well as multiple spaces
(like the ones produced by code indentation).
This also includes a couple of fixes that removes spaces that were
intentionally placed before/after the string to translate.
This moves respond_to_mention() and reply_with_mention() to
compose_actions.js. These methods are basically thin layers
on top of compose_actions.start().
This module extracts these two functions that get called by
several other modules:
start()
cancel()
It is a little bit arbitrary which functions got pulled over
with them, but it's generally functions that would have only
been called via start/cancel.
There are two goals for splitting out this code. The first
goal is simply to make `compose.js` have fewer responsibilities.
The second goal is to help break up circular dependencies.
The extraction of this module does more to clarify
dependencies than actually break them. The methods start()
and cancel() had actually been shimmed in an earlier commit,
and now they no longer have a shim.
Besides start/cancel, most of the functions here are only
exported to facilitate test stubbing. An exception is
decorate_stream_bar(), which is currently called from
ui_init.js. We probably should move the "blur" handler out
of there, but cleaning up ui_init.js is a project for another
day.
It may seem slightly odd that this commit doesn't pull over
finish() into this module, but finish() would bring in the
whole send-message codepath. You can think of it like this:
* compose_actions basically just populates the compose box
* compose.finish() makes the compose box do its real job,
which is to send a message
The extraction here is straightforward, but where we put the
caller is a slightly subtle change. Instead of continuing to
invoke this code at the end of show_box(), we instead call it
at the beginning of complete_starting_tasks(). This change is
valid, because show_box() and complete_starting_tasks() are only
ever called from compose.start().
Previously drafts called compose.snapshot_message which would then
get the message object from compose.create_message_object. This method often
checked for the validity of stream/user recipients which would often cause tracebacks.
The new method in drafts.snapshot message just gets the data from the fields and
stores them in the draft model without any additional checking.
This allows for users to resize the message compose box without it
collapsing back down to jQuery autosize’s preferred height.
When you hide the compose box and then re-show it, it keeps the
previous height but reactivates the jQuery module.
Fixes: #2236.
This fixes the mobile web experience for Chrome on iOS.
Apparently, Chrome-on-iOS silently has a `viewport` module that
overrides and user-defined module by that name, causing all of our
code that accesses the viewport module to not work on that platform.
We fix this by renaming it.
* Created a drafts modal to display/restore/delete drafts
* Created a Draft model to support storing draft data in localstorage
* Removed existing restore-draft functionality
* Added casper and node tests for drafts functionality
Fixes#1717.
The current logic that we have is as follows:
* If a message is locally echoed, the draft is stored via the locally
rendered message, and that system takes care of it. So no need to
store it here.
* If the message isn't locally echoed, we don't close the compose box
until, so the content is safe here as well. It'll be saved as a draft
if the compose box is later closed due to a failure sending.
The new behavior is:
(1) If enter-sends is enabled, just send the messsage.
(2) If enter-sends is not enabled, return focus to the compose area.
Based on great work by khantaalaman in #3673.
Fixes#3489.
We already do detection of the client on the backend based on
User-Agent and the fact that it's a JSON view, which is pretty safe.
This fixes an issue where the server was not treating the Electron app
as its own client.
This significantly simplify the logic for our logging process, making
it the case that websockets message sending requests always are logged
as having the exact same client as a normal AJAX request from that
server.
When we get notified of an email change and the compose box is
open for PMs, we should update the email in the compose box.
This helper will be useful when we start handling such events.
The issue is that we were trying to validate the mentions before
checking that the recipient stream was valid, leading to problems
checking the membership of the stream.
Fixes#3040.
Tim Abbott
Steve Howell
hollywoodno
Cory Lynch
Aditya Bansal
Harshit Bansal
Tejas Kasetty
Mahim Goyal
fionabunny
Rafid Aslam
Benjamin Gilbert
Yago González
Sampriti Panda
Brock Whittaker
Raghav Jajodia
Philip Skomorokhov
Elliott Jin
kpdp
khantaalaman
Eitan Adler