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 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.
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.
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.
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.
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.
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.
A few years ago I introduced the anti-pattern
of automatically calling success functions within
channel.get stubs. It's better to just capture
the success function and call it explicitly.
Also, we now have override(), so it's easy to
inline these types of things in a safe way.
Steve Howell
YashRE42
shanukun
Anders Kaseorg
Abhijeet Prasad Bodas
100RABHpy