- do not render sidepanel resizer as it's not usable on mobile
- removes 1px to the bottom spacing, this spacing won't be necessary once we implement chat-replying-indicator for thread
- correctly subscribe/unsubscribe channel
- instantly changes users list
- adds a test for testing channel change
- rewrites tests to be less verbose
- ensures users is always an array
When making the list of users to notify we set `all_mentioned_user_ids` key on the `to_notify` Hash.
This hash will be passed around until the actual moment where we send the notifications:
```ruby
identifier_text =
case identifier_type
when :here_mentions
"@here"
when :global_mentions
"@all"
when :direct_mentions
""
else
"@#{identifier_type}"
end
```
As not found `all_mentioned_user_ids` would end up being sent as `@all_mentioned_user_ids` which is obviously incorrect.
This commit is a direct fix to the issue and will remove the key as soon as we have used it sooner up in the chain.
This bug was reproducible when doing this sequence of events:
- create a message with a direct mention: `@bob hi`
- edit this message into a global mention `@all hi`
Every replies creates a thread, even when threading is disabled. This is how we ensure we can go back and forth. However, a message bus event should only be published when threading is enabled, otherwise frontend will attempt to display a thread which is not possible when disabled.
This fixes a silent background 404 when doing a reply in a direct message channel or a non threading enabled category channel.
This commit also adds a component test for it and fixes a bug in `chat-channel-archive-status` `#getTopicURL` property which was incorrectly called as a function.
What is the problem?
Previously, this was the query used to move change messages into another
channel.
```
INSERT INTO chat_messages(
chat_channel_id, user_id, last_editor_id, message, cooked, cooked_version, created_at, updated_at
)
SELECT :destination_channel_id,
user_id,
last_editor_id,
message,
cooked,
cooked_version,
CLOCK_TIMESTAMP(),
CLOCK_TIMESTAMP()
FROM chat_messages
WHERE id IN (:message_ids)
RETURNING id
```
The problem is that this incorrectly assumes that the insertion will be based on the order of `message_ids`. However, that
is not the case as PostgreSQL provides no such guarantee. Instead we need to explicitly order the messages to ensure
the right order of insertion.
This problem was discovered by a flaky test which exposed the non-guarantee order of insertion.
- `ChatChannel`
- `UserChatChannelMembership`
Also creates a new `chat-direct-message` model used as the object for the`chatable` property of the `ChatChannel` when the `ChatChannel` is a direct message channel. When the chatable is a category a real `Category` object will now be returned.
Archive state of a `ChatChannel` is now hold in a `ChatChannelArchive` object.
After a long time with no activity or hidden browser (2.5 minutes), the app will re-sync the chat user-tracking-state to ensure unreads are synced.
We might also need to couple this later with more recovering logic.
When we were deleting messages in chat, we would find all of
the UserChatChannelMembership records that had a matching
last_read_message_id and set that column to NULL.
This became an issue when multiple users had that deleted message
set to their last_read_message_id. When we called ChannelUnreadsQuery
to get the unread count for each of the user's channels, we were
COALESCing the last_read_message_id and returning 0 if it was NULL,
which meant that the unread count for the channel would be the total
count of the messages not sent by the user in that channel.
This was particularly noticeable for DM channels since we show
the count with the indicator in the header. This issue would disappear
as soon as the user opened the problem channel, because we would then
set the last_read_message_id to an actual ID.
To circumvent this, instead of NULLifying the last_read_message_id in
most cases, it makes more sense to just set it to the most recent
non-deleted chat message ID for the channel. The only time it will
be set to NULL now is when there are no more other messages in the
channel.
We need to create and update `chat_mentions` records for messages earlier. They should be created or updated before we call `Chat::Publisher.publish_new!` `Chat::Publisher.publish_edit!` to send the message to message bus subscribers).
This logic is covered with tests in `message_creator_spec.rb`, `message_updater_spec.rb`, `notifier_spec.rb` and `notify_mentioned_spec.rb`.
See the commits history for steps of refactoring.
Since our recent change of inverting thread scrolling direction it feels more responsive to scroll down in thread panel as soon as message is staged and not after it's actually persisted.
When hovering a thread indicator in a channel we will now append two `<link rel="preload" ...>` to the `<head>` of the document. Clicking on it should be significantly faster.
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit implements all the necessary logic to create thread seamlessly. For this it relies on the same logic used for messages and generates a `staged-id`(using the format: `staged-thread-CHANNEL_ID-MESSAGE_ID` which is used to re-conciliate state client sides once the thread has been persisted on the backend.
Part of this change the client side is now always using real thread and channel objects instead of sometimes relying on a flat `threadId` or `channelId`.
This PR also brings three UX changes:
- thread starts from top
- number of buttons on message actions is dependent of the width of the enclosing container
- <kbd>shift + ArrowUp</kbd> will reply to the last message
See e323628d8a for more details.
This commit speeds up the tests by roughly 10 seconds locally where the
default wait time is 2 seconds. On CI, this speeds up the tests by 20
seconds where the default wait time is 4 seconds.
* FIX: Link to thread for mentions inside thread
When mentioning a user in a thread, when we send the
notification and display it in the UI we want the URL
of the notification to point to the thread URL to open
the panel, rather than the main channel which is confusing.
For now, we don't have a way to highlight the linked-to message
in the thread, we can revisit this later.
* FIX: Mark mention notifications read when thread opens
Since we have no scrolling/message visibility/thread membership
for now, when a user opens the thread panel we just want to mark
all mention notifications relating to messages in the thread
for the user as read.
In the past this was happening on scroll so we needed to be very conservative here. Also, if we wait too much theres a visible element flashing so this PR attempts to compute right away, and a second time 100ms later in case the first one happened too early.
It seems more reliable to revert state at the end of the it block. In another PR I noticed that the network state was leaking in other tests when I was reverting in the after block.
Also trashes a suspicious spec.
This will avoid the messages actions floating around while scrolling. Note it's not testing the thread counterpart yet as I have a plan in mind to tests channels and threads in a clean way in the near future.
Before this fix if the underlying model of a reviewable was changed, the filter wouldn't work anymore as it was expecting a 1:1 relation between filter type and model name.
This commit also relies on the `Reviewable.types` array to check against valid types instead of a regex not checking much.
Finally this commit adds a spec to ensure chat reviewables are listable from the review index page.
Prior to this fix uploads event could end up in the wrong textarea. This will most importantly allow pasting an image in the thread composer.
Also fixes a minor padding issue on thread when uploads are associated to it.
`.chat-channel` had `300px` min width, when `.chat-drawer` was `250px`, resulting in overflowing channel when in drawer. This commits ensures the limits are always set at `250px`.
- using BEM notation
- making animation linear instead of default ease
- small tweaks to composer state (disabled/send-disabled/send-enabled)
- fixing bug with disabled composer on mobile
- It seems that `window_opened_by/within_window` it not reliable in our current setup/test
- System specs should avoid at all cost to rely on backend state, any change should be visible one way or another on the front to be properly tested
1. `this.chat.activeChannel = null` was being done in twice
2. using `willTransition()` and checking transition.to.name prefix for route cleanup rather than using `deactivate()` was unnecessarily verbose and could be premature (if something aborted the transition you'd end up in a broken state)
3. `activeChannel` on Chat service can be null, check for that before accessing
It's very hard to repro but under specific circumstances I suspect it was possible for this sequence to happen:
- set message TEXT
- cooking starts
- set message COOKED through another mean (like a message bus)
- the cooking started sooner finished and erases the cooked set at the step before causing the message to have the incorrect cooked
After removing `TextareaTextManipulation` from `ChatComposer` and using `TextareaInteractor` as a proxy, one function has been forgotten: `paste(event)` which is not available in glimmer components anymore, and even less avaiable now that the mixin is not tied to a component anymore but a real DOM node. As a solution we now add a manual paste event listener which will call `paste(event)`.
