I am increasing the threshold even more for when
codecov complains. Since the tool reports so many
false negatives, we should probably change our philosophy
on code coverage so that this tool only reports
catastrophic commits (like .e.g. a commit that prevents
running a whole test suite) and let our more specific
tools continue to catch things like regressions of files
from 100% coverage.
This uses the correct regex for strikethrough. Also, added
a test to make sure that strikethrough works when it contains
link with whitespace.
Fixes#7596.
We join extra text message for notification and xhr response text by ':'.
Before joining, check if xhr response text already include ':', to
prevent notification with more than one ':'.
A `None` value is not properly handled in this function, which
indicates some lack of testing or a recent regression we don't
understand. We were getting lots of tracebacks from this line
of code on our test server:
mentioned = 'mentioned' in flags and 'read' not in flags
This change has a few cleanups:
* We early-return on last_msg_container === undefined
to make the function flatter.
* We avoid comparing two boolean values for equality,
which can be a landmine if one of the values is
`undefined`, which is falsy but not actually `false`.
* We extract some local variables for readability.
* We make the conditions for subscribe/unsubscribe
more explicit.
This fixes a bug where this used to happen:
* Alice has not read a message
* Bob edits the message
* Alice immediately reads the message
* Bob's edit arrives to Alice and sets her
message status back to unread
Essentially, the root cause of the bug is that we update
message.unread for edits, possibly from stale data, even
though Alice has more current info about reading the message.
This is the final fix to that scenario. There were some
aggravating factors that widened the race window which were
fixed in earlier commits.
Fixes#6248
We were queuing up individual messages to be flagged as
read on the server before this change, and we used debounce
to avoid sending individual POSTs, but this created delays
that were ripe for race conditions.
Now we batch them in the caller and use throttle instead.
This now prevents us from slamming the server with lots of
individual requests, without as many opportunities for races.
(Note that we still have some possibility of race conditions,
but they should be rare now, and other commits will address
some of the other contributors to read/unread glitches.)
Once we convert message.flags to more specific boolean attributes
like message.mentioned and message.alerted, we should get rid of
the `flags` attribute, as it will only confuse debugging.
We no longer set message.flags in the local echo path.
In the markdown parsing step, we just set message.mentioned
directly.
And then we change `insert_new_messages` to no longer
convert flags to booleans, and move that code to only
happen for incoming server message events.
We want to call `set_message_booleans` as soon as we
get data from the server, to avoid confusion about whether
`flags` is the authoritative field.
This commit has callers to `add_message_metadata` call
`set_message_booleans`.
This also sets us up to **not** call `set_message_booleans`
in the local echo codepath, where we can just have the
markdown processor set booleans natively.
In all cases the value of `flags` we were passing in was
actually `message.flags` (although it was slightly obscured in
one place), so now we just pass in `message`.
(We also move a tiny bit of defensive code to set `flags`
into `set_message_booleans`.)
In the JS code, we now use `message.unread` universally as
the indicator of whether a message is unread, rather than
the `message.flags` array that gets passed down to us
from the server.
In particular, we use the unread flag for filtering when
you search.
A lot of this commit is just removing logic to add/remove
"read" from `message.flags` and updating tests.
We also explicitly set `message.unread` to `false` inside of
`unread.mark_as_read()` and no longer have `unread.set_flag()`.
(Some of the callers to `unread.set_flag` were also calling
`unread.mark_as_read`, which was updating the `message`
object, so now we just have `unread.mark_as_read` update
the `message` object. And then unread_ops.mark_all_as_read()
was already calling unread.declare_bankruptcy().)
This adds two similar functions to simplify
our batch processing of unread messages.
unread.get_unread_messages
unread.get_unread_message_ids
They are used to simplify two functions that loop
over messages. Before this change, the functions
would short circuit the loop to ignore messages
that were already read; now they just use the
helpers before the loop.
We now make direct calls to `send_flags_update` for
seting collapsed/uncollapsed, rather than going through
the `batched_updaters` mechanism.
Also the methods in `message_flags` to set
collapsed/uncollapsed now take a single message.