Commit Graph

158 Commits

Author SHA1 Message Date
Steve Howell 863660281e code cleanup: Use exports for internal references.
When we call functions inside our own modules that use
the `window.foo = exports` pattern, we have always had
a pretty strong preference to call `exports.internal_function`
instead of `foo.internal_functions`.

The stragglers here weren't violating this convention
for any intentional reason.  Some of the places here
probably were part of code moves where somebody
(probably me) moved functions into the modules to avoid
unnecessary indirection, and I missed a spot where I
could change from `presence` to `exports` (or whatever).

And other places are probably just kinda arbitrary
decisions by the original developer, and we just haven't
bothered to clean it up until now.
2020-06-11 11:05:06 -07:00
Aman Agrawal e6611089fd recent_topics: Update stored message id of locally echoed messages.
This fixes the bug that message was undefined since we used to store
locally echoed message id and were not updating it after new message
id for the same message was received from the server.
2020-06-09 22:08:31 -07:00
Ryan Rehman cfc87e3925 message list: Move the `FetchStatus` object to MessageListData class.
The reason for this change is that, this is where `Filter` and
actual tracking of what messages are contiguous lives. This
will be beneficial when we will to move to a model where we
cache `MessageListData` objects for a large number of views.
2020-06-02 15:45:39 -07:00
Aman Agrawal 7197a7ac68 message_edit: Add support for changing stream of a message.
* This feature is currently only visible to admins.
* Locally echoed messages are also updated.
* Add UI for editing stream if user is admin.
* Show propagate mode selector if either stream or topic changed.
2020-05-11 16:25:47 -07:00
Hashir Sarwar ee0d4541b4 topic_data: Rename `topic_data` module to `stream_topic_history`.
`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
2020-04-16 20:11:04 -07:00
Steve Howell 5d58590f14 local echo: Fix resending of local messages.
This also fixes trying to cancel a resend of a
local message.

The problem were was type confusion between
strings and ints.

The function in `rows.js` may feel like overkill,
but I really want to enforce type safety here,
as we usually treat message ids as floats, but
for the local-echo case we're gonna get
strings.  I put it in `rows` because we mostly
do a good job of encapsulating the "zid" role
in the DOM there.

By going directly to the DOM here, we avoid
parsing a string to a float and then converting
it right back to a string, which always make
me queasy about float rounding, so one less
moving part.
2020-04-09 16:11:57 -07:00
Steve Howell 082a2f9c2e local echo: Avoid waiting_for_ack leak.
Due to type confusion, we were silently failing
to delete local_id values for messages that were
being acked by the server.

This used to work when we kept values in our
old Dict data structure, since client_message.id
and message.local_id are really the same value,
just the former is a float and the latter's a
string, and Dict never cared.

We can avoid all this confusion, though, by just
consistently using `local_id`, which I extract
to a local var.
2020-04-09 16:11:57 -07:00
Steve Howell 9fa4ec56bb local echo: Fix type errors for message_send_error.
The function message_send_error was messing up
on calls to message.get when we were passing in
string versions of `local_id`.  Now we pass in
float ids.

This fixes a traceback where we tried to set
`.failed_request` on to an `undefined` value
that we had instead expected to be a locally
echoed message from our message store.
2020-04-09 16:11:57 -07:00
Steve Howell f4f403decb refactor: Have try_deliver_locally() return a message.
This will allow us access to the float version of the
message's id in an upcoming commit, without us having
to do possibly brittle string-to-float translations.
2020-04-09 16:11:57 -07:00
Steve Howell dccbb25a49 minor: Fix duplicate function names in echo.js. 2020-04-09 16:11:57 -07:00
Anders Kaseorg 6f32ef749f js: Convert $.extend to spread syntax.
This is not always a behavior-preserving translation: $.extend mutates
its first argument.  However, the code does not always appear to have
been written to expect that.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-02-25 14:09:39 -08:00
Tim Abbott af188205cb echo: Add a block comment explaining found_newest=False case.
This is just clarifing some details for postering from the previous
commit.
2020-02-24 11:50:50 -08:00
Steve Howell 6dba3f2420 local echo: Avoid echo until "newest" are found.
We now no longer do local echo if a user has logged in or visited a
narrow so recently that we are still fetching new messages for them in
their current message list.

Since we want any message list we're displaying to show only
contiguous sequences of messages within that view, it's not correct to
append messages that were just sent at the end unless
fetch_status.has_found_newest shows that we are up to date with the
latest messages from the server.

While we have some logic aimed at correcting our-of-order message IDs
in Zulip, even a brief (few seconds) temporary display of that is a
bug that we should avoid.

This means that we should disable local echo when the user's current
narrow is not up to date.  We can be sure that we'll get the message
the user sent from the server either during the catch-up process or
when we receive it back from th server via the events system.

That particular race window can be several seconds in situations where
somebody is in a narrow where their pointer (or equivalent) is far
behind the latest messages.

This commit only fixes the local echo race condition.  There's a
related bug where new messages sent by (potentially other) users
delivered to the client via server_events might race with our fetching
until we get the latest messages in a given narrow, which we'll need
to deal with separately.

See https://github.com/zulip/zulip/issues/8989 for more details.  It's
possible that we'll close the issue after this fix, since any
additional fixes would add a lot of complexity, and I'm not sure how
much of a problem this will really be in practice after this fix.

Note that we don't have great automated testing for
`try_deliver_locally` (or really `echo.js` in general).  For
`try_deliver_locally` the node tests would probably be 8x more complex
than the code itself, since that function is basically "glue" code
touching several external dependencies.  It's also kind of hard to
screw up this code without getting pretty obvious failures early in
the QA process.

Fixes #8989.
2020-02-24 11:45:00 -08:00
shubhamgupta2956 a05f633fc1 util: Replace util.set_message_topic().
Replace `util.set_message_topic(message, topic)` with `message.topic =
topic`.

Fixes #13931
2020-02-21 09:53:45 -05:00
shubhamgupta2956 efda2684ea util: Replace util.get_message_topic().
Replace `util.get_message_topic(message)` with `message.topic`.

Fixes #13931
2020-02-21 09:53:45 -05:00
Steve Howell cb91b7f312 util: Kill off set_topic_links/get_topic_links.
These functions were just shims that were
used in the somewhat painful migration from
subject_* to topic_*.

The commit 4572be8c27
fixed it so that the client never needs to
deal with "subject_links".

So now we just go back to simpler code:

    message.topic_links = links
    links = message.topic_links
2020-02-15 21:15:48 +00:00
Steve Howell 9ab07d1038 util.js: Remove util from window.
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.
2020-02-15 12:20:20 -08:00
Anders Kaseorg 0936381cfd echo: Convert waiting_for_ack from object to Map.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-02-12 10:39:01 -08:00
Anders Kaseorg c42aca3cfb echo: Convert waiting_for_id from object to Map.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-02-12 10:39:01 -08:00
Anders Kaseorg 2ddd3d046a echo: Further reduce local_id string vs. number confusion.
New rule: local_id is always a string, local_id_float is always a
number.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-02-12 10:39:01 -08:00
Anders Kaseorg ac7b09d57e js: Convert _.map(a, …) to a.map(…).
And convert the corresponding function expressions to arrow style
while we’re here.

import * as babelParser from "recast/parsers/babel";
import * as recast from "recast";
import * as tsParser from "recast/parsers/typescript";
import { builders as b, namedTypes as n } from "ast-types";
import K from "ast-types/gen/kinds";
import fs from "fs";
import path from "path";
import process from "process";

const checkExpression = (node: n.Node): node is K.ExpressionKind =>
  n.Expression.check(node);

for (const file of process.argv.slice(2)) {
  console.log("Parsing", file);
  const ast = recast.parse(fs.readFileSync(file, { encoding: "utf8" }), {
    parser: path.extname(file) === ".ts" ? tsParser : babelParser,
  });
  let changed = false;

  recast.visit(ast, {
    visitCallExpression(path) {
      const { callee, arguments: args } = path.node;
      if (
        n.MemberExpression.check(callee) &&
        !callee.computed &&
        n.Identifier.check(callee.object) &&
        callee.object.name === "_" &&
        n.Identifier.check(callee.property) &&
        callee.property.name === "map" &&
        args.length === 2 &&
        checkExpression(args[0]) &&
        checkExpression(args[1])
      ) {
        const [arr, fn] = args;
        path.replace(
          b.callExpression(b.memberExpression(arr, b.identifier("map")), [
            n.FunctionExpression.check(fn) ||
            n.ArrowFunctionExpression.check(fn)
              ? b.arrowFunctionExpression(
                  fn.params,
                  n.BlockStatement.check(fn.body) &&
                    fn.body.body.length === 1 &&
                    n.ReturnStatement.check(fn.body.body[0])
                    ? fn.body.body[0].argument || b.identifier("undefined")
                    : fn.body
                )
              : fn,
          ])
        );
        changed = true;
      }
      this.traverse(path);
    },
  });

  if (changed) {
    console.log("Writing", file);
    fs.writeFileSync(file, recast.print(ast).code, { encoding: "utf8" });
  }
}

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-02-10 14:08:12 -08:00
Anders Kaseorg 02511bff1c js: Automatically convert _.each to for…of.
This commit was automatically generated by the following script,
followed by lint --fix and a few small manual lint-related cleanups.

import * as babelParser from "recast/parsers/babel";
import * as recast from "recast";
import * as tsParser from "recast/parsers/typescript";
import { builders as b, namedTypes as n } from "ast-types";
import { Context } from "ast-types/lib/path-visitor";
import K from "ast-types/gen/kinds";
import { NodePath } from "ast-types/lib/node-path";
import assert from "assert";
import fs from "fs";
import path from "path";
import process from "process";

const checkExpression = (node: n.Node): node is K.ExpressionKind =>
  n.Expression.check(node);
const checkStatement = (node: n.Node): node is K.StatementKind =>
  n.Statement.check(node);

for (const file of process.argv.slice(2)) {
  console.log("Parsing", file);
  const ast = recast.parse(fs.readFileSync(file, { encoding: "utf8" }), {
    parser: path.extname(file) === ".ts" ? tsParser : babelParser,
  });
  let changed = false;
  let inLoop = false;
  let replaceReturn = false;

  const visitLoop = (...args: string[]) =>
    function(this: Context, path: NodePath) {
      for (const arg of args) {
        this.visit(path.get(arg));
      }
      const old = { inLoop };
      inLoop = true;
      this.visit(path.get("body"));
      inLoop = old.inLoop;
      return false;
    };

  recast.visit(ast, {
    visitDoWhileStatement: visitLoop("test"),

    visitExpressionStatement(path) {
      const { expression, comments } = path.node;
      let valueOnly;
      if (
        n.CallExpression.check(expression) &&
        n.MemberExpression.check(expression.callee) &&
        !expression.callee.computed &&
        n.Identifier.check(expression.callee.object) &&
        expression.callee.object.name === "_" &&
        n.Identifier.check(expression.callee.property) &&
        ["each", "forEach"].includes(expression.callee.property.name) &&
        [2, 3].includes(expression.arguments.length) &&
        checkExpression(expression.arguments[0]) &&
        (n.FunctionExpression.check(expression.arguments[1]) ||
          n.ArrowFunctionExpression.check(expression.arguments[1])) &&
        [1, 2].includes(expression.arguments[1].params.length) &&
        n.Identifier.check(expression.arguments[1].params[0]) &&
        ((valueOnly = expression.arguments[1].params[1] === undefined) ||
          n.Identifier.check(expression.arguments[1].params[1])) &&
        (expression.arguments[2] === undefined ||
          n.ThisExpression.check(expression.arguments[2]))
      ) {
        const old = { inLoop, replaceReturn };
        inLoop = false;
        replaceReturn = true;
        this.visit(
          path
            .get("expression")
            .get("arguments")
            .get(1)
            .get("body")
        );
        inLoop = old.inLoop;
        replaceReturn = old.replaceReturn;

        const [right, { body, params }] = expression.arguments;
        const loop = b.forOfStatement(
          b.variableDeclaration("let", [
            b.variableDeclarator(
              valueOnly ? params[0] : b.arrayPattern([params[1], params[0]])
            ),
          ]),
          valueOnly
            ? right
            : b.callExpression(
                b.memberExpression(right, b.identifier("entries")),
                []
              ),
          checkStatement(body) ? body : b.expressionStatement(body)
        );
        loop.comments = comments;
        path.replace(loop);
        changed = true;
      }
      this.traverse(path);
    },

    visitForStatement: visitLoop("init", "test", "update"),

    visitForInStatement: visitLoop("left", "right"),

    visitForOfStatement: visitLoop("left", "right"),

    visitFunction(path) {
      this.visit(path.get("params"));
      const old = { replaceReturn };
      replaceReturn = false;
      this.visit(path.get("body"));
      replaceReturn = old.replaceReturn;
      return false;
    },

    visitReturnStatement(path) {
      if (replaceReturn) {
        assert(!inLoop); // could use labeled continue if this ever fires
        const { argument, comments } = path.node;
        if (argument === null) {
          const s = b.continueStatement();
          s.comments = comments;
          path.replace(s);
        } else {
          const s = b.expressionStatement(argument);
          s.comments = comments;
          path.replace(s, b.continueStatement());
        }
        return false;
      }
      this.traverse(path);
    },

    visitWhileStatement: visitLoop("test"),
  });

  if (changed) {
    console.log("Writing", file);
    fs.writeFileSync(file, recast.print(ast).code, { encoding: "utf8" });
  }
}

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-02-07 14:09:47 -08:00
Tim Abbott 992b15b86c echo: Support senders other than ourself in display_recipient logic.
we don't have a current use case for this feature, but it better
matches what we mean and thus makes the function more readable.
2020-01-31 12:46:19 -08:00
Vishnu KS d7107f47cd echo: Include sender in display_recipient during PM local insert.
Previously the sender was not included in display_recipient when
a private message was locally echoed. This broke the copy conversation
link functionality, if the user try to copy the link immedeatly after
sending the message. This issue is present only during local echo.
This was fixed by including the recipient of the user during
local echo.

Fixes #13547.
2020-01-31 12:45:14 -08:00
Tim Abbott 8ba48d0e4f echo: Add comment explaining zephyr mirroring details. 2020-01-31 12:45:14 -08:00
Tim Abbott 0a6e3b41d3 echo: Simplify return flow in build_display_recipient. 2020-01-31 12:45:14 -08:00
Vishnu Ks dde5d1cd4d echo: Extract emails_to_display_recipient function. 2020-01-31 12:45:14 -08:00
Vishnu KS fbd697ee70 tests: Add test for insert_local_message in echo.js. 2020-01-31 12:45:14 -08:00
Jack Tiggleman 1682d75ea8 message_edit: Add message edit local echo.
Updates the message editing process to do a local 'echo'.

On slow connections, now there is visual confirmation of the edit,
similar to when sending messages.  The contains_backend_only_syntax
logic and check are the same as there.

We showing "(SAVING)" until the edit is completed, and on successful
edit, the word "(EDITED)" appears.  There's likely useful future work
to do on making the animation experience nicer.

Substantially rewritten by tabbott to better handle corner cases and
communicate more clearly about what's happening.

Fixes: #3530.
2019-11-20 17:40:19 -08:00
Tim Abbott 124f5d12a4 message_edit: Adjust API of edit_locally.
This makes it more extensible for future use of locally echoing edits
to fully sent messages.
2019-11-20 17:06:08 -08:00
Anders Kaseorg 28f3dfa284 js: Automatically convert var to let and const in most files.
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>
2019-11-03 12:42:39 -08:00
Anders Kaseorg d17b577d0c js: Purge useless IIFEs.
With webpack, variables declared in each file are already file-local
(Global variables need to be explicitly exported), so these IIFEs are
no longer needed.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
2019-10-25 13:51:21 -07:00
Anders Kaseorg c3a83a82c5 echo: Consistently send local_id as string, convert it back to number.
Fixes: #2734.

`local_id` was being transmitted to the server as a string by the AJAX
transmission path, and as a number by by the WebSocket transmission
path.  Then, one of the two racing success callback paths would use
the original number, while the other would use the type returned by
the server.  Depending on which transmission path was used and which
callback path won the race, `reify_message_id` would sometimes be
passed a string that would fail to compare equal to the numerical
selection id.  If the locally echoed message was selected, this would
cause the selection to disappear.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2019-08-07 17:17:25 -07:00
Rohitt Vashishtha 9d6727d18c echo: Update topic_links when we get messages back from server. (#12832) 2019-07-20 14:38:52 -07:00
YashRE42 dd1a6a97bd group-pms: Update list when new group pm is made.
The function activity.process_loaded_messages(messages) would be called
from message_events.js, this would call people.huddle_string with the
same message object, it was expected that this would return a list of
ids but the message.display_recipient attribute which was being sent
here used a "user_id" field instead of an "id" field.

Fixes: #12503.
2019-07-03 16:39:41 -07:00
Rohitt Vashishtha 4489df7cd6 echo: Add tests for echo.process_from_server. 2019-06-28 06:13:41 +00:00
Rohitt Vashishtha fbd4c389d4 echo: Update is_me_message flag when we get messages back from server.
Fixes #12450.
2019-06-28 06:13:41 +00:00
Tim Abbott 0320355f09 echo: Add a minimum time for sending error spinner.
This at least makes clear that the button did something.
2019-03-19 13:30:46 -07:00
Steve Howell 1ad30c6858 subject -> topic: Sweep "message.subject" in frontend.
These were the last remaining files.  After this, only
util.js has a non-email-related use of "subject".
2019-01-01 20:49:38 -08:00
Steve Howell 7e17b8a392 subject -> topic: Use util.set_message_topic() to set subject. 2019-01-01 20:49:34 -08:00
Tim Abbott d0f71881f4 docs: Add detailed documentation on the process for sending messages.
This has long been something missing from our suite of documentation.
2018-11-29 16:25:35 -08:00
Steve Howell 3a9c81ed8c subject -> topic: Rename add_subject_links. 2018-11-14 23:24:06 -08:00
Armaan Ahluwalia 6d255efe4c app: Prepare JS files for consumption by webpack.
This commit prepares the frontend code to be consumed by webpack.

It is a hack: In theory, modules should be declaring and importing the
modules they depend on and the globals they expose directly.

However, that requires significant per-module work, which we don't
really want to block moving our toolchain to webpack on.

So we expose the modules by setting window.varName = varName; as
needed in the js files.
2018-07-05 10:53:36 +02:00
Joshua Pan a62e10d9c2 slash-commands: Suppress local echo for slash commands. 2018-05-20 14:42:09 -07:00
Steve Howell a84bd6da57 Add client code for handling submessages.
This commit lays the foundation to handle submessages for
plugin widgets.  Right now it just logs events, but subsequent
commits will add widget functionality.
2018-05-16 15:13:33 -07:00
Steve Howell d2ee76896c Extract local_message.js.
We will eventually want these functions to support things
other than local echo, such as ephemeral messages from chat
bots.
2018-05-16 09:30:49 -07:00
Steve Howell 9eb3bdaf6c page load: Make initializations more explicit.
We now initialize most modules in ui_init.js, which
isn't the perfect place to do it, but at least now
we have a mostly consolidated entry point.

All the new foo.initialize() methods introduced in
this module run the same order relative to each
other as before this commit. (I did some console
logging with a hacked version of the program to
get the order right.)  They happen a bit later than
before, though.

A couple modules still have the `$(function() {`
idiom for miscellaneous reasons:

       archive - is a different bundle
       common - used elsewhere
       list_render - non-standard code style
       scroll_bar - no exports
       setup - probably special?
       socket - $(function () is nested!
       transmit - coupled to socket
       translations - i18n is a bigger problem
       ui_init - this bootstraps everything
2018-05-15 15:46:04 -07:00
Tim Abbott 42fe331093 echo: Fix exception when topic-editing locally echoed messages.
Previously, if you topic-edited locally echoed messages, it would
throw this exception:

TypeError: Cannot read property 'indexOf' of undefined
    at Object.h.is_status_message (https://chat.zulip.org/static/min/app.274582375298.js:1742:154)
           = static/js/markdown.js line 125 column 13

    at Object.h.apply_markdown (https://chat.zulip.org/static/min/app.274582375298.js:1741:291)
           = static/js/markdown.js line 93 column 29

    at Object.a.edit_locally (https://chat.zulip.org/static/min/app.274582375298.js:1750:195)
           = static/js/echo.js line 176 column 5

    at Object.d.save (https://chat.zulip.org/static/min/app.274582375298.js:1921:376)
           = static/js/message_edit.js line 132 column 13

The behavior with this patch is still not ideal, in that we don't ever
send the edit to the server (if the issue was lag, not an actual error
on sending), but this at least makes the on-error case correct.
2018-05-03 12:14:43 -07:00
Tim Abbott 8b91ea3a74 Revert "Suppress local echo when we are not caught up."
This reverts commit 80ed8294a1.

We need to do some improvements to FetchStatus for narrows before this
is safe to have running in production.
2018-03-30 11:51:52 -07:00
Steve Howell 80ed8294a1 Suppress local echo when we are not caught up. 2018-03-28 09:12:25 -07:00