We had a bug in this code recently, sometimes users saw weird notifications
like:
User mentioned all_mentioned_user_ids in the help chat channel
We fixed that bug in b85d057.
This refactoring is a follow-up to that fix. As that bug showed, it’s quite easy
to introduce a key that may end up being sent to the `NotifyMentioned` job,
which can lead to such weird notifications. This refactoring makes sure that
the `to_notify` hash contains only IDs of users that should be notified about
mentions.
This PR adds a new parameter to fetch chat messages: `target_date`.
It can be used to fetch messages by a specific date string. Note that it does not need to be the `created_at` date of an existing message, it can be any date. Similar to `target_message_id`, it retrieves an array of past and future messages following the query limits.
This commit adds an aria-label attribute to cooked hashtags using
the post/chat message decorateCooked functionality. I have just used
the inner content of the hashtag (the tag/category/channel name) for
the label -- we can reexamine at some point if we want something
different like "Link to dev category" or something, but from what I
can tell things like Twitter don't even have aria-labels for hashtags
so the text would be read out directly.
This commit also refactors any ruby specs checking the HTML of hashtags
to use rspec-html-matchers which is far clearer than having to maintain
the HTML structure in a HEREDOC for comparison, and gives better spec
failures.
c.f. https://meta.discourse.org/t/hashtags-are-getting-a-makeover/248866/23?u=martin
https://meta.discourse.org/t/markdown-preview-and-result-differ/263878
The result of this markdown had different results in the composer preview and the post. This is solved by updating Loofah to the latest version and using html5 fragments like our user had reported. While the change was only needed in cooked_post_processor.rb for this fix, other areas also had to be updated due to various side effects.
- Moves `<ChatMessageInfo />` to `<Chat::Message::Info />`
- Moves `<ChatMessageAvatar />` to `<Chat::Message::Avatar />`
- Moves `<ChatMessageLeftGutter />` to `<Chat::Message::LeftGutter />`, adds tests
- Creates `<Chat::Message::Error />`
- Creates `<Chat::Message::MentionWarning />`, adds tests and a styleguide
- Creates a model for ChatMessageMentionWarning, adds fabricator for it
- Keeps the enter/leave viewport logic inside the `<ChatMessage />` component instead of bubbling it to the channel and thread components
- Adds a scale animation when clicking a reaction
- Creates `chat/later-fn` modifier which accepts a function and a delay. It allows to call a function Xms after a component has been inserted, it's useful for animations.
- Moves css code out of chat-message into relevant files
- Deletes unused code
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
Previously this was defining methods like `UserOption#never?`, `UserOption#all_new?`, `UserOption#dm_and_mentions?`. Now they will be prefixed like `UserOption#chat_header_indicator_never?`
The layout was broken for messages replying to another message in non threaded channels.
This commit also refactors the chat-message-test to use fabricators.
Unfortunately, Discourse's `UserOption` model is not currently autoloaded. That means that modifying it via a `reloadable_patch` will try to apply the changes repeatedly. Normally this doesn't matter since the changes are idempotent. However, introducing ActiveRecord enums is not idempotent - they raise an error if the same enum already exists on the model. This commit adds a check to avoid hitting this 'duplicate definition' error.
Reproduced
```
rails runner 'Rails.application.reloader.reload!'
```
- FIX: improves reactions and thread indicator touch event on mobile
These "buttons" are located inside a scroll list which makes them very specific. The general idea is to ensure these events are passive and are not bubbling to the parent.
- DEV: moves state on top level message node
- FIX: ensures popover arrow has the correct border
- FIX: makes a message expanded by default
- FIX applies the same ios scroll fix on thread and channel
- UI: better active/hover state for thread indicator
- UI: attempts to follow more closely our BEM naming scheme
- FIX: reduces bottom padding on message with thread indicator and user info hidden
- UI: add padding for first message in thread
- FIX: prevents actions backdrop to open thread
- UI: makes thread indicator resizable
This method is a huge footgun in production, since it calls
the Redis KEYS command. From the Redis documentation at
https://redis.io/commands/keys/:
> Warning: consider KEYS as a command that should only be used in
production environments with extreme care. It may ruin performance when
it is executed against large databases. This command is intended for
debugging and special operations, such as changing your keyspace layout.
Don't use KEYS in your regular application code.
Since we were only using `delete_prefixed` in specs (now that we
removed the usage in production in 24ec06ff85)
we can remove this and instead rely on `use_redis_snapshotting` on the
particular tests that need this kind of clearing functionality.
This commit adds a tracking dropdown to each individual thread, similar to topics,
that allows the user to change the notification level for a thread manually. Previously
the user had to reply to a thread to track it and see unread indicators.
Since the user can now manually track threads, the thread index has also been changed
to only show threads that the user is a member of, rather than threads that they had sent
messages in.
Unread indicators also respect the notification level -- Normal level thread tracking
will not show unread indicators in the UI when new messages are sent in the thread.
Why is this change required?
When a site is newly setup and a user has just been created, the
categories and tags sections are hidden from the user. This happens
because the admin has not configured the `default_navigation_menu_categories` or
`default_navigation_menu_tags` site settings. When the categories and tags
sections are hidden from the user, the sidebar looks extremely bare and
does not create a good experience.
What is being change?
In this commit, we're changing the logic such that the site's top
categories and tags are displayed if the user does not have any
categories/tags configured in each respective section. The only
regression introduced in this change is that the categories and tags
section can no longer be hidden as a result. However, we have plans to
address this in the future by allowing sidebar sections to be configured
to be hidden by each individual user.
The events leading to this mistake are unclear but we decided few months ago to make direct messages NOT flaggable and even wrote a spec for this, when we actually support flagging of direct messages.
This commit ensures it will show for direct messages channels and inverses the existing spec.
https://meta.discourse.org/t/updating-our-initializer-naming-patterns/241919
For historical reasons, Discourse has different initializers conventions than standard Ember:
```
| Ember | Discourse | |
| initializers | pre-initializers | runs once per app load |
| instance-initializers | (api-)initializers | runs once per app boot |
```
In addition, the arguments to the initialize function is different – Ember initializers get either the `Application` or `ApplicationInstance` as the only argument, but the "Discourse style" gets an extra container argument preceding that.
This is confusing, but it also causes problems with Ember addons, which expects the standard naming and argument conventions:
1. Typically, V1 addons will define their (app, instance) initializers in the `addon/(instance-)initializers/*`, which appears as `ember-some-addon-package-name/(instance-)initializers/*` in the require registry.
2. Just having those modules defined isn't supposed to do anything, so typically they also re-export them in `app/(instance-)initializers/*`, which gets merged into `discourse/(instance-)initializers/*` in the require registry.
3. The `ember-cli-load-initializers` package supplies a function called `loadInitializers`, which typically gets called in `app.js` to load the initializers according to the conventions above. Since we don't follow the same conventions, we can't use this function and instead have custom code in `app.js`, loosely based on official version but attempts to account for the different conventions.
The custom code that loads initializers is written with Discourse core and plug-ins/themes in mind, but does not take into account the fact that addons can also bring initializers, which causes the following problems:
* It does not check for the `discourse/` module prefix, so initializers in the `addon/` folders (point 1 above) get picked up as well. This means the initializer code is probably registered twice (once from the `addon/` folder, once from the `app/` re-export). This either causes a dev mode assertion (if they have the same name) or causes the code to run twice (if they have different names somehow).
* In modern Ember blueprints, it is customary to omit the `"name"` of the initializer since `ember-cli-load-initializers` can infer it from the module name. Our custom code does not do this and causes a dev mode assertion instead.
* It runs what then addon intends to be application initializers as instance initializers due to the naming difference. There is at least one known case of this where the `ember-export-application-global` application initialize is currently incorrectly registered as an instance initializer. (It happens to not use the `/addon` folder convention and explicitly names the initializer, so it does not trigger the previous error scenarios.)
* It runs the initializers with the wrong arguments. If all the addon initializer does is lookup stuff from the container, it happens to work, otherwise... ???
* It does not check for the `/instance-initializers/` module path so any instance initializers introduced by addons are silently ignored.
These issues were discovered when trying to install an addon that brings an application initializer in #22023.
To resolve these issues, this commit:
* Migrates Discourse core to use the standard Ember conventions – both in the naming and the arguments of the initialize function
* Updates the custom code for loading initializers:
* For Discourse core, it essentially does the same thing as `ember-cli-load-initializers`
* For plugins and themes, it preserves the existing Discourse conventions and semantics (to be revisited at a later time)
This ensures that going forward, Ember addons will function correctly.
This commit fixes the selection of message in threads and also applies various refactorings
- improves specs and especially page objects/components
- makes the channel/thread panes responsible of the state
- adds an animationend modifier
- continues to follow the logic of "state" should be displayed as data attributes on component by having a new `data-selected` attribute on chat messages
When merging users, polls may error out if the source and target users have both voted on the same poll before. 😢
There is no constraint on the `poll_votes` table either to support this. Ideally a composite primary key can be used `(poll_id, user_id)`, but alas there is no support yet, which is probably why it wasn't created in the first place.
This fix ensures that merging is successful by only keeping the target poll votes if duplicates exist.
This fix also runs a migration on older poll votes where failed merges would have caused a single user to have voted twice on a single poll. e.g. this weird edge case
This commit adds the initial part of thread indicator improvements:
* Show the reply count, last reply date and excerpt,
and the participants of the thread's avatars and
count of additional participants
* Add a participants component for the thread that
can be reused for the list
* Add a query class to get the thread participants
* Live update the thread indicator more consistently
with the last reply and participant details
image image
In subsequent PRs we will cache the participants since
they do not change often, and improve the thread list
further with participants.
This commit also adds a showPresence boolean (default
true) to ChatUserAvatar, since we don't want to show the
online indicator for thread participants.
---------
Co-authored-by: chapoi <charlie@discourse.org>
Since we created user_chat_thread_memberships in
cc2570f we haven't
yet backfilled it for users who previously sent a message in
in threads -- this migration creates the UserChatThreadMemberships
needed for those threads, making sure the last read message id
is accurate for those participants.
* FEATURE: Content custom summarization strategies.
This PR establishes a pattern for plugins to register alternative ways of summarizing content by extending a class that defines an interface.
Core controls which strategy we'll use and who has access to it through the `summarization_strategy` and `custom_summarization_allowed_groups`. It also defines the UI for summarizing topics.
Other plugins can access this summarization mechanism and implement their features, removing cross-plugin customizations, as it currently happens between chat and the discourse-ai plugin.
* Group membership validation and rate limiting
* Work with objects instead of classes
* Port summarization feature from discourse-ai to chat
* Rename available summaries to 'Top Replies' and 'Summary'
In early 2015, the poll plugin was writing its data to custom fields on
the post containing the poll. It was later changed to have dedicated SQL
tables and the polls were migrated but we forgot to clean the existing
data.
* move the chat unread indicator to top to match the profile avatar indicator
* add white border to profile avatar indicator (badge notification) to match chat indicator and userstatus styling
* change `.urgent` to BEM
* congregate all styling into mixin
* update chat index to use mixin
* update thread indicator to use mixin
* update header indicator to use mixin
---------
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Co-authored-by: Martin Brennan <martin@discourse.org>
This is actually making things more sluggish than necessary. If any perf issue happen out of this they should be handled in the consequences of the resizing, not the resizing itself.
Currently navigating a long topic and then opening chat would cause the view to be scrolled to the bottom. Using `scrollTop` here ensures we correctly scroll to top.
This had been incorrectly moved into `deactivate` during another change.
* FIX: increases resize observer throttle delay
25ms is not necessary and was sometimes causing jankyness.
* FIX: removes ios momentum fix delay
Instead of a 50ms, simply use next+schedule("afterRender") to attempt to have the shortest delay possible.
* FIX: backdrop event propagation
Prevents backdrop touch to propagate to underlying channel/thread.
* UX: adds is-active class to container of active message
This change allows to keep the background on the active message while the actions menu is displayed.
* FIX: prevents skip-link to be selected on press
* UX: allows to close actions menu instantly
The backdrop should always receive events, we don't need to wait for the menu to be fully displayed.
* UI: adds spacing between last message and composer
* UI: makes backdrop less dark
* FIX: makes events passive on long-press modifier
We have been struggling a lot on this lately as it's almost impossible to write a decent test for this.
The important things which need to happen:
- fetch the unread/mention state and last message bus channel ids of each chat channels
- stop all subscriptions
- restart global chat subscriptions
- update channels with new state and ensure the message bus ids are updated
- restart subscriptions of each chat channel
As a followup we need to start implementing a standard way to query for a resource state. Something similar to: `/channels/tracking` and `/channels/:id/tracking`
Each of these endpoints would return a state similar to:
```json
{
tracking: { ... },
message_bus_ids: { ... }
}
Removing a reaction could start a long press at the same time and put the screen in a stuck state.
This commit ensures we give an opportunity to the reaction to capture the event first and not propagate further.
These spec are flaky only in CI, not locally and not in GitHub actions.
The previous attempt was in 44eabde, but actually the failure happens
a bit earlier. This is another attempt to fix these specs. Quite a lot of
async logic is happening in emulateAutocomplete(), a call to settled()
in the end should help make it more reliable.
This commit attempts to have a bullet proof solution to the following case:
- long press on message (finger is still pressed)
- menu appears
- a button is now at finger location
- user releases finger
- a click is triggered on the button
Classic event canceling solution won't work here for performance reasons as we need the event to be passive in a scroll list.
In some cases, plugins may want to hide some of these actions
at all times, overriding the rules for canX with hiding these
buttons. To achieve this, a plugin can call the API
`removeChatComposerSecondaryButtons` and pass the list of button
IDs that should be removed as argument, like the example below:
```
withPluginApi("1.2.0", (api) => {
api.removeChatComposerSecondaryActions("copyLink", "select");
});
```
---------
Co-authored-by: Martin Brennan <martin@discourse.org>
These specs were skipped in d6d5eae1. They sometimes failed, and only on CI,
not in GitHub Actions.
I wasn't able to reproduce failures locally, but I expect clicking the send button
in chat composer should be more reliable than emulating pressing <kbd>Enter</kbd>.
When editing a message, we call `message.cook()` in the beginning of
`#sendEditMessage` methods, but when sending a new message,
the call to `message.cook()` is hidden in the `stageMessage` method.
We can just call `message.cook()` before sending the message, no matter
whether this is a new message or an edited message.
No test as this is very much a hack while lightbox is being revamped. We currently have no good/easy way AFAIK to stop event propagation on escape in magnificpopup.
Andrei Prigorshnev
Discourse Translator Bot
Jan Cernik
Martin Brennan
Natalie Tay
Sam
Joffrey JAFFEUX
David Taylor
Sérgio Saquetim
Alan Guo Xiang Tan
Godfrey Chan
Kris
Roman Rizzi
Loïc Guitaut
chapoi
Bianca Nenciu