- few improved alignments
- displays emoji picker button inline on desktop
- keeps composer focused when focusing dropdown button
- align buttons to bottom when increasing height of textarea
- max-height of textarea is now linked to the height of the screen
Co-authored-by: chapoi <charlie@discourse.org>
This commit moves message lookup and querying to the
/chat/api/channel/:id endpoint and adds the ability
to query the tracking state overview for threads as well
as the threads and thread tracking state for any thread
original messages found.
This will allow us to get an initial overview of thread
tracking for a user when they first enter a channel, rather
than pre-emptively loading N threads and tracking state
for those across all channels on the current user serializer,
which would be expensive.
This initial overview will be used in subsequent PRs to
flesh out the thread unread indicators in the UI.
This also moves many chunks of code that were in services
to reusable Query classes, since use of services inside
services is discouraged.
Regressed in eec10efc3d. It means that backend plugin spec failures in CI were not failing the spec suite.
Fixes recent regressions and skips two of them - to be handled next week.
---------
Co-authored-by: Andrei Prigorshnev <a.prigorshnev@gmail.com>
Wrap scroll to bottom inside a next block to ensure the message has correctly been added to the array before actually attempt to scroll.
This commit also removes an unnecessary line which was essentially adding the message two times with no real consequences as we are uniq on ID.
- uses current user as user for fabricators, allows for correct avatar image and presence indicator
- uses a non existing channel ID to avoid setting a draft of an existing channel
- attempts to make color toggle more reliable
- Improves styleguide support
- Adds toggle color scheme to styleguide
- Adds properties mutators to styleguide
- Attempts to quit a session as soon as done with it in system specs, this should at least free resources faster
- Refactors fabricators to simplify them
- Adds more fabricators (uploads for example)
- Starts implementing components pattern in system specs
- Uses Chat::Message creator to create messages in system specs, this should help to have more real specs as the side effects should now happen
Followup from 9953a6edd9,
which broke an issue fixed in e8d6277062.
In the refactor we did not update the place where we resync
channel tracking based on onPresenceChange to use the new
tracking object we added to the serializer, and to directly
update channel.tracking
We had safe-area-inset-bottom still set at a wrong place which was causing issues but was also useful to some cases. This commit removes it and ensures the affected cases are corrected.
This is only the first steps of a redesign
- redesigns the buttons to have a larger hitzone
- generally bigger composer
- clicking near textarea focuses the input
- relies on the fact that safe-area-inset-bottom is set globally and doesn’t need to be set in sub components
This moves chat tracking state calculation for channels
and threads into a central Chat::TrackingStateManager service, that
serves a similar purpose to the TopicTrackingState model
in core.
This service calls down to these query classes:
* ThreadUnreadsQuery
* ChannelUnreadsQuery
To get the unread_count and mention_count for the appropriate
channels and threads.
As well as this, this commit refactors the client-side chat
tracking state.
Now, there is a central ChatTrackingStateManager Ember Service
so all tracking is accessible and can be counted from one place,
which can also initialize tracking from an initial payload.
The actual tracking counts are now maintained in a ChatTrackingState
class that is initialized on the `.tracking` property of both channel and
thread objects.
This removes the attributes on UserChatChannelMembership and decoration
of said membership from ChannelFetcher, preferring instead to have an additional
object for tracking in the JSON.
* DEV: add new thread icon
* FIX: Use new thread icon, fix typo in SVG
UX: move the thread list icon to the right of
the collapse button
---------
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit also removes safe-area-inset-bottom when keyboard is displayed to avoid having a taller than needed space between composer and replying indicator.
Before this commit chat was applying a fixed height on everything under the `/chat` route. It's only really needed on the channel page with the composer at the bottom of the page.
This commits makes the following changes:
- moves height limitation from `#main-outlet-wrapper` to `.chat-channel`
- makes browse channel page and members list page full height and rely on main document scrollbar
- adds height computation for draft header and direct message creator block to ensure the height is correct when creating a draft channel
- makes chat index full height to rely on the browser scrollbar. As a result the <kbd> + </kbd> button used on mobile to create a direct message as been moved out of `<ChannelsList>` into the chat index template
- sidebar height was relying on chat setting a max height, as a result the height computation of sidebar has been changed to work correctly, especially with an opened keyboard on mobile or ipad
Hard to write a test for this behavior, this is a micro optimisation which doesn’t change the behavior but only makes it smoother by happening right before async request.
The failure screenshot shows the message is on screen while the error is:
```
Failure/Error: example.run
expected to find text "My favorite message" in "Community\nEverything\nMy Posts\nMore\nMessages\nInbox\nChannels\nPolitics 1\nPersonal chat\nPolitics 1". (However, it was found 1 time including non-visible text.)
```
I expect the arrow element might e slightly hiding the link, but not 100% sure of this.
When using `navigator.virtualKeyboard.overlaysContent = false` we can rely on using only the resize event. Also attempts to no over trigger `setProperty` when value didn't change.
A follow-up to 54b2a85b. That commit didn't fix the issue because the to_notify hash that we return from the notify_edit method isn't used anywhere apart from tests (that's confusing, we're going to fix that soon).
This issue was for example possibly causing the last visit indicator to be reset by `sent` messages events.
The following was happening:
- a user (bob) had a last message bus ID of 1 on a channel (id:1) subscription
- bob then go to another channel (id:2), unsubscribing from updates of channel (id:1)
- another user (laura) then send messages to channel (id:1)
- bob goes back to channel (id:1)
At this point we we doing in the same sequence:
- loading channel with messages, getting a new last message bus id
- subscribing to updates using the last known message bus id
Most of the times we were lucky enough for this to work (no events while away, or just got the new id in time...) but it was also very likely to do a double fetch of messages as MessageBus would think we were late.
A chat message may be restored later, so we shouldn't be deleting `chat_mentions` records for it.
But we still have to remove notifications (see 082cd139).
This commit fixes the shift+click multi selection in threads. We were not correctly using the manager of the message and would attempt to find messages in the channel instead of the thread.
The `activeThread` was also not correctly set sometimes.
Also adds tests for message selection in threads.
In the past, we create a `chat_mention` records only when we wanted to notify a user about a mention. Since we don't send notifications when a user mentioning himself, we didn't create a `chat_mention` records in those cases.
Now we use `chat_mentions` records in other scenarios too, so when a user is mentioning himself we want to:
1. Create a `chat_mention` record for that mention
2. Do not create a notification for that mention
When the user sends a message in a thread, we want to
create a membership for them in the background (default
to notification level of Watching) so we can track whether
they have read the thread.
Then, for now since we don't have granular message reading/
scrolling in the thread panel, we just update the thread
last_read_message_id for the user to the latest reply in the
thread when they open the thread panel. This at least will
mark the thread as read.
In future PRs we want to show the blue dot indicator in various
places in the UI for unread threads which will also require
some MessageBus functionality.
This takes into account the same issue fixed for channels
in ae3231e140
We were combining both solutions which was apparently causing issues from chrome 113 on Android at least.
The commit will now use `geometrychange` (android) only when available and fallback to `visualViewport` otherwise (iOS).
This feature adds the replying indicator in threads, it uses the same `/chat-reply/CHANNEL_ID` prefix than the channel composer replying indicator as we don't have specific right on threads ATM (if you can access channel, you can access thread). Thread will however use a presence channel name of the following format: `/chat-reply/CHANNEL_ID/thread/THREAD_ID`
This commit also simplifies the computation of `users` to eventually avoid a race-condition leading to a leak of the indicator in another channel/thread.
<!-- 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. -->
When setting DISCOURSE_ZEITWERK_EAGER_LOAD=1 to enable
eager loading the previous solution to adding chat_levels
to the core NotificationLevels would break with a module
loading error (c.f. cc2570fce3)
We don't actually _need_ to extend the core class, we can just
make our own for chat, let's do this instead.
This will enable us to begin work on user tracking
state for a thread so we can show thread-specific
unreads and mentions indicators. In this case are following
the core notification_level paradigm rather than the solution
UserChatChannelMembership went with, and eventually we
will want to refactor the other table to match this as well.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Since we have channel message retention which deletes
messages, we can end up with cases where the thread
is still around but the message is deleted. We will
handle the cascade delete in a different commit --
for now we will ensure the thread list lookup handles
this case and doesn't error.
After this change, in order to join a chat channel, a user needs to be in a group with at least “Reply” permission for the category. If the user only has “See” permission, they are able to preview the channel, but not join it or send messages. The auto-join function also follows this new restriction.
---------
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit adds an initial thread list UI. There are several limitations
with this that will be addressed in future PRs:
* There is no MessageBus reactivity, so e.g. if someone edits the original
message of the thread it will not be reflected in the list. However if
the thread title is updated the original message indicator will be updated.
* There is no unread functionality for threads in the list, if new messages
come into the thread there is no indicator in the UI.
* There is no unread indicator on the actual button to open the thread list.
* No pagination.
In saying that, this is the functionality so far:
* We show a list of the 50 threads that the user has most recently participated
in (i.e. sent a message) for the channel in descending order.
* Each thread we show a rich excerpt, the title, and the user who is the OM creator.
* The title is editable by staff and by the OM creator.
* Thread indicators show a title. We also replace emojis in the titles.
* Thread list works in the drawer/mobile.
The spec now checks we are in the state we expect to be before clicking bottom button. The bottom button could show while it's still loading and on slow systems cause failures.
Checking "uploading" string is tricky because it both takes time before showing, and when it will show it will show for a short period of time. I prefer to reduce the surface tested here while still getting some confidence out of it and making the test more reliable.
- 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.
Previously, the image for the thumbnail was taken from the oembed endpoint.
Since these images are not available for some unlisted videos,
the thumbnails are now sourced from Open Graph instead.
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)`.
This pull request is a full overhaul of the chat-composer and contains various improvements to the thread panel. They have been grouped in the same PR as lots of improvements/fixes to the thread panel needed an improved composer. This is meant as a first step.
### New features included in this PR
- A resizable side panel
- A clear dropzone area for uploads
- A simplified design for image uploads, this is only a first step towards more redesign of this area in the future
### Notable fixes in this PR
- Correct placeholder in thread panel
- Allows to edit the last message of a thread with arrow up
- Correctly focus composer when replying to a message
- The reply indicator is added instantly in the channel when starting a thread
- Prevents a large variety of bug where the composer could bug and prevent sending message or would clear your input while it has content
### Technical notes
To achieve this PR, three important changes have been made:
- `<ChatComposer>` has been fully rewritten and is now a glimmer component
- The chat composer now takes a `ChatMessage` as input which can directly be used in other operations, it simplifies a lot of logic as we are always working a with a `ChatMessage`
- `TextareaInteractor` has been created to wrap the existing `TextareaTextManipulation` mixin, it will make future migrations easier and allow us to have a less polluted `<ChatComposer>`
Note ".chat-live-pane" has been renamed ".chat-channel"
Design for upload dropzone is from @chapoi
- Move the old '`define_include_method`' arg to a `respect_plugin_enabled` kwarg
- Introduce an `include_condition` kwarg which can be passed a lambda with inclusion logic. Lambda will be run via `instance_exec` in the context of the serializer instance
This is backwards compatible - old-style invocations will trigger a deprecation message
- Move the old '`define_include_method`' arg to a `respect_plugin_enabled` kwarg
- Introduce an `include_condition` kwarg which can be passed a lambda with inclusion logic. Lambda will be run via `instance_exec` in the context of the serializer instance
This is backwards compatible - old-style invocations will trigger a deprecation message
Update chat and poll plugins to new pattern
Further followup to 24ec06ff85,
where I prevented other chat scheduled jobs from running if
chat was disabled. We should not be running any plugin scheduled
jobs if that plugin is disabled, it can cause unexpected
behaviour.
This was reverted in 38cebd3ed5.
The issue was that I was using Discourse.redis.delete_prefixed
which does a slow redis KEYS lookup, which is not advised in
production. This commit removes that, and also ensures the periodical
thread count update only happens if threading is enabled.
I changed to use a redis INCR/DECR for reply count
cache. This avoids a round trip to redis to GET the current
count, and also avoids multi-process issues, where
if there's two processes trying to increment at the
same time, they may both receive the same value, add one
to it, then both write the same value back.
Then, it's only n+1 instead of n+2.
This also prevents almost all chat scheduled jobs from
running if chat is disabled, the only one remaining is
the message retention job.
This commit moves the category channel creation out
of the Chat::Api::Channel controller and into a
dedicated CreateCategoryChannel service. A follow up
commit will move the DM channel creation out of
the old DirectMessageChannelCreator service.
Also includes a new on_model_errors helper
for chat service class usage, that collects model
validation errors to present in a nice way.
---------
Co-authored-by: Loïc Guitaut <loic@discourse.org>
This adds these two new test cases:
context "when updating a mentioned user" do
it "updates the mention record" do
# and
context "when there are duplicate mentions" do
it "creates a single mention record per mention" do
Apart from that, this groups mention related tests into a context, renames one test, and moves setup of another test into the test case itself from the before block (to make it more clear, that test is the only one that uses that setup). See the PR's commit history.
Steps to reproduce the bug:
1. Send a chat message
2. Edit the message and add a mention to it
3. The mentioned user won't receive a notification
This PR fixes the problem.
Also:
1. There's no need anymore to have a code for removing notifications in the `notify_edit` method, because a call to `@chat_message.update_mentions` in the first line of the `notify_edit` method does that job:
ff56f403a2/plugins/chat/lib/chat/notifier.rb (L90)
2. There's no need to load mention records from database, it's enough to pluck user ids
Followup to bd5c5c4b5f, a
bug was introduced there for any channel that did not have
threading enabled or sites with the experimental threading
disabled. When the user replied to another chat message,
since this is always a thread in the background, we weren't
sending any MessageBus messages to the main channel, since
the message was a thread reply.
However in the UI these messages still show in the main stream
of the channel if threading is turned off, so the UI was not
reacting to these things happening in the backend. The worst
issue was that new clients would not see new replies sent in
reply to other messages in the channel.
This reverts commit 768851920e.
This was causing issues with the local date popup, cutting off
the top of it, there is no way to overrule an overflow:hidden
on the parent. Not z-index related.
This is to help generate random channels and chat
messages for local dev. This was removed in 12a18d4d55
presumably because it was not worth refactoring at the
time.
I've only added these tasks:
- `rake chat:message:populate\[113,20\]` (channel_id, count)
- Generates the count of messages for a channel ID provided,
otherwise uses a random channel and 200 count.
- `rake chat:category_channel:populate`
- Creates a chat channel for a random category.
- `rake chat🧵populate\[132,5\]` (channel_id, message_count)
- Creates a thread with N messages in the specified channel,
and enables threading in that channel if necessary
This commit fixes an issue where if you pressed a format
shortcut (e.g. bold, italic, code) for the composer and
you had the thread panel open as well, the shortcut would
trigger in both composers, not just the one that was focused.
We currently don't have a nice UI to show unread messages for the thread,
and it will take some time to create one. For now, this commit makes it so
new messages inside a thread do not count towards a chat channel's unread
counts, and new messages sent in a thread do not update a user's `last_read_message_id`
for a channel.
In addition, this PR refactors the `Chat::ChannelFetcher` to use the `Chat::ChannelUnreadsQuery`
query class for consistency, and made said class able to return zeroed-out records
for channels the user is not a member of.
Finally, a small bug is fixed here where if a user's `last_read_message_id` for
a channel was a thread's OM ID, then the thread OM would not show in the
main channel stream for them until another reply to the channel was posted.
We've found these exceptions in logs:
Job exception: undefined method `destroy!' for nil:NilClass
/var/www/discourse/plugins/chat/lib/chat/notifier.rb:102:in `block in notify_edit'
/var/www/discourse/plugins/chat/lib/chat/notifier.rb💯in `each'
/var/www/discourse/plugins/chat/lib/chat/notifier.rb💯in `notify_edit'
/var/www/discourse/plugins/chat/app/jobs/regular/chat/send_message_notifications.rb:18:in `execute'
In the past, we were creating `chat_mention` records only for sending notifications, so every mention record had a related notification. It isn't the case anymore (since fa543cda). This PR fixes the problem by making sure the notification exists before trying to remove it. Also, we shouldn't be deleting a `chat_mention` record itself, only a notification, this PR fixes that too.
It's quite hard to reproduce this bug locally, I wasn't able to do so, the logic in this class is quite complicated, that's why I'm not adding a test. Also, when looking at this I realized that this method isn't in a fully correct state now, I suspect sometimes some notifications may not be delivered after someone edits a chat message and adds new mentions to it. I'm going to refactor and simplify the method in a subsequent PR.
This commit introduces a redis cache over the top of the thread
replies_count DB cache, so that we can quickly and accurately
increment/decrement the reply count for all users and not have
to constantly update the database-level count. This is done so
the UI can have a count that is displayed to the users on each
thread indicator, that appears to live update on each chat
message create/trash/recover inside the thread.
This commit also introduces the `Chat::RestoreMessage` service
and moves the restore endpoint into the `Api::ChannelMessages`
controller as part of incremental migrations to move things out
of ChatController.
Finally, this commit refactors `Chat::Publisher` to be less repetitive
with its `MessageBus` sending code.
Followup to bd5c5c4b5f,
this commit hooks up the bulk delete events for chat
messages inside the thread panel, by fanning out the
deleted message IDs based on whether they belong to
a thread or not.
Also adds a system spec to cover this case, as previously
the bulk delete event would have been broken with an incorrect
`typ` rather than `type` hash key.
This error was only happening on mobile, note we also already have a (mobile) test (plugins/chat/spec/system/transcript_spec.rb:184) for this which was passing as it's only happening at a specific speed. I don't want to complicate the test too much for this case, will reconsider if it regresses again.
Currently, some links aren’t properly built in Discobot when Discourse
is hosted in a subfolder. This is because we’re providing
`Discourse.base_url` to the Rails helpers which contains the base URL
*with* the prefix. But Rails helpers already handle this prefix so the
resulting link gets the prefix twice.
The fix is quite simple: use `Discourse.base_url_no_prefix` instead of
`Discourse.base_url`.
Followup to ba11cf4767,
this commit makes it so that none of the chat message
thread data is serialized if threading_enabled is false
for the channel or if enable_experimental_chat_threaded_discussions
is false, there is no need to serialize the data in this
case.
This regression happened in bd5c5c4b5f and is due to `message_bus_targets = calculate_publish_targets(chat_channel, chat_message)` expecting a `chat_channel` which was only defined after.
Example exception in logs:
```
Job exception: undefined local variable or method `chat_channel' for Chat::Publisher:Module
/var/www/discourse/plugins/chat/app/services/chat/publisher.rb:91:in `publish_processed!'
/var/www/discourse/plugins/chat/app/jobs/regular/chat/process_message.rb:21:in `block in execute'
/var/www/discourse/lib/distributed_mutex.rb:53:in `block in synchronize'
/var/www/discourse/lib/distributed_mutex.rb:49:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:49:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:34:in `synchronize'
/var/www/discourse/plugins/chat/app/jobs/regular/chat/process_message.rb:7:in `execute'
/var/www/discourse/app/jobs/base.rb:249:in `block (2 levels) in perform'
```
This commit also:
- adds a spec to ensure oneboxing is not regressing anymore
- increment the version on message processed to ensure callbacks are correctly ran
Note we should also have more tests in `Chat::Publisher`, this will be done when we move it to a proper service.
<!-- 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. -->
It is yet to investigate the exact reasons leading to this, but probably due to some delete operation or migration, it seems possible to have a message with a thread_id leading to a non existing thread row. This is only a temporary solution to prevent the crash. We should also probably be more defensive here and not include any of this if threading is not enabled.
This codepath was responsible to scroll to the first emoji of a section, however `scrollIntoView` was not super reliable and was also causing the whole page to scroll with drawer. This is also simply not necessary code as native focus behavior will scroll to the element.
This commit introduces a ChatChannelPaneSubscriptionsManager
and a ChatChannelThreadPaneSubscriptionsManager that inherits
from the first service that handle MessageBus subscriptions
for the main channel and the thread panel respectively.
This necessitated a change to Chat::Publisher to be able to
send MessageBus messages to multiple channels based on whether
a message was an OM for a thread, a thread reply, or a regular
channel message.
An initial change to update the thread indicator with new replies
has been done too, but that will be improved in future as we have
more data to update on the indicators.
Still remaining is to fully move over the handleSentMessage
functionality which includes scrolling and new message indicator
things.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
When we "pull hotlinked images" on onebox images, they are added to the uploads table and their dominant color is calculated. This commit adds the data to the HTML so that it can be used by the client in the same way as non-onebox images. It also adds specific handling to the new `discourse-lazy-videos` plugin.
- Back button in drawer will bring you back to channel
- Larger font for thread indicator
- Prevents screen flashing due to clearing messages when they were already loaded
- Fixes a bug where did-update params were inverted causing an error when expanding/collapsing drawer
This feature will allow sites to define which emoji are not allowed. Emoji in this list should be excluded from the set we show in the core emoji picker used in the composer for posts when emoji are enabled. And they should not be allowed to be chosen to be added to messages or as reactions in chat.
This feature prevents denied emoji from appearing in the following scenarios:
- topic title and page title
- private messages (topic title and body)
- inserting emojis into a chat
- reacting to chat messages
- using the emoji picker (composer, user status etc)
- using search within emoji picker
It also takes into account the various ways that emojis can be accessed, such as:
- emoji autocomplete suggestions
- emoji favourites (auto populates when adding to emoji deny list for example)
- emoji inline translations
- emoji skintones (ie. for certain hand gestures)
This commit introduces a new thread indicator for channels with `threading_enabled`
set to true and the `enable_exp` site setting set to true. In addition, in the main channel
stream we now hide all messages that are linked to threads except for the original message,
disabling the concept of an "echo mode" for now, we may revisit this in future. We also
remove the jigsaw puzzle "Open Thread" button for message actions, since the thread
indicator can just be used instead.
This also stops the `Chat::Publisher` from sending any messages related to chat
messages that are linked to a thread, unless that chat message is the OM of the
thread. A subsequent PR will link up all MessageBus events within the thread panel,
and for the message indicators.
Another subsequent PR will add the excerpt of the latest message in each thread,
as well as the avatars of the users messaging in the thread.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
- rounded active style for messages
- better active state on chat message actions rows
- ensures long press on a message is not selecting text
- slightly improved messages actions animation and background fading
- ensures chat emoji picker is not cutoff on right side
- removes old legacy code related to hovered message
Similar to 22a55ef0ce,
this commit adds a replies_count to the Chat::Thread
table, which is updated every 15 minutes via PeriodicalUpdates.
This is done so the new thread indicator for the UI can
show the count without intense serializer queries, but
in future we likely want this to update more frequently.
Followup to c1dc6a2db4,
this commit just missed removing one of the @computed
decorators which was causing multiple active channels
to show in the sidebar. Fix the issue and introduce a
system spec to catch this.
- clicking empty area on the header will toggle collapse/expand it
- applies a background on hover of the channel title
- active state for small buttons
- the back button now has the correct icon color when hovered
- adds missing focus state for heade buttons icons
When hovering the chat message actions we are technically not hovering the message anymore, which was removing the background and is slightly unexpected. This commit ensures we keep this background until closing the message actions.
This PR primarily fixes this case:
- USER A message
- USER B message
- USER B reply to USER A message <-- not showing user info when it should
Moreover, this commit also improves the spec to correctly test more cases.
This commit is a major overhaul of how chat message actions work, to make it so they are reusable between the main chat channel and the chat thread panel, as well as many improvements and fixes for the thread panel.
There are now several new classes and concepts:
* ChatMessageInteractor - This is initialized from the ChatMessage, ChatMessageActionsDesktop, and ChatMessageActionsMobile components. This handles permissions about what actions can be done for each
message based on the context (thread or channel), handles the actions themselves (e.g. copyLink, delete, edit),
and interacts with the pane of the current context to modify the UI
* ChatChannelThreadPane and ChatChannelPane services - This represents the UI context which contains the
messages, and are mostly used for state management for things like message selection.
* ChatChannelThreadComposer and ChatChannelComposer - This handles interaction between the pane, the
message actions, and the composer, dealing with reply and edit message state.
* Scrolling logic for the messages has now been moved to a helper so it can be shared between the main channel pane and the thread pane
* Various improvements with the emoji picker on both mobile and desktop. The DOM node of each component is now located outside of the message which prevents a large range of issues.
The thread panel now also works in the chat drawer, and the thread messages have less
actions than the main panel, since some do not make sense there (e.g. moving messages to
a different channel). The thread panel title, excerpt, and message sender have also been removed
for now to save space.
This gives us a solid base to keep expanding on and fixing up threads. Subsequent PRs will
make the thread MessageBus subscriptions work and disable echo mode
for the initial release of threads.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This commit adds a system to generate CSS variables and classes for categories
and hashtags, which will be used in an effort to remove baked icons for hashtags
and add color to those icons.
This is in two parts. First I added an initializer generate a category color CSS
variable style tag in the head tag that looks like this:
```css
:root {
--category-1-color: #0088CC;
--category-2-color: #808281;
--category-3-color: #E45735;
--category-4-color: #A461EF;
--category-5-color: #ee56c9;
--category-6-color: #da28c2;
--category-7-color: #ab8b0a;
--category-8-color: #45da37;
...
}
```
The number is the category ID. This only generates CSS variables for categories
the user can access based on `site.categories`. If you need the parent color variable
you can just use the `category.parentCategory.id` to get it.
Then, I added an initializer to generate a hashtag CSS style tag using these variables.
Only the category and channel hashtags need this, the category one generates the
background-gradient needed for the swatch, and the channel just generates a color
for the icon. This is done in an extendable way using the new `api.registerHashtagType`
JS plugin API:
```css
hashtag-color--category-1 {
background: linear-gradient(90deg, var(--category-1-color) 50%, var(--category-1-color) 50%);
}
hashtag-color--category-2 {
background: linear-gradient(90deg, var(--category-2-color) 50%, var(--category-2-color) 50%);
}
hashtag-color--category-5 {
background: linear-gradient(90deg, var(--category-5-color) 50%, var(--category-4-color) 50%);
}
...
.hashtag-color--channel-4 {
color: var(--category-12-color);
}
.hashtag-color--channel-92 {
color: var(--category-24-color);
}
```
Note if a category has a parent, its color is used in the gradient correctly. The numbers
here are again IDs (e.g. channel ID, category ID) and the channel’s chatable ID is used
to find the category color variable.
The translation key is built using the name of the reviewable as it was
defined in Ruby. The chat plugin uses the `Chat` namespace and defines
`Chat::ReviewableMessage`. This was then transformed to
`chat::reviewable_message`, but it should be `chat_reviewable_message`
to resemble the other translation keys.
When a chat message is trashed and the message is used
for someone's UserChatChannelMembership#last_read_message_id,
the user would end up with some read state issues until
someone posted a new message in the channel, since we didn't
clear it like we did on bulk message delete.
This commit fixes the issue, and also takes the opportunity
to start a MessagesController in the API namespace, and move
the trash message functionality into the new service format.
Followup to 0924f874bd,
we migrated Chat::Upload records to UploadReference records
there and have not been making new Chat::Upload records
for some time, we can now delete the model and table.
- Raises the scroll distance to 250px instead of 100px to show the arrow down button
- Always have a margin on drawer when showing channel list, removes this margin when the scrollbar is apparent
- Makes all scrollbar used in chat look the same through the chat-scrollbar mixin
- Ensures hover state is not persistent on channel row in mobile
- Makes the channel row full width on mobile
We keep getting this failure on the spec but I
cannot reproduce locally, add this extra log line
to see if it helps:
```
> Chat::Api::ChatablesController#index with chat permissions does not return DM channels for users who are not in the chat allowed group
> Failure/Error: example.run
>
> expected: 200
> got: 500
>
> (compared using ==)
> # ./plugins/chat/spec/requests/chat/api/chatables_controller_spec.rb:158:in `block (4 levels) in <main>'
> # ./spec/rails_helper.rb:358:in `block (2 levels) in <top (required)>'
> # ./vendor/bundle/ruby/3.2.0/gems/webmock-3.18.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
> # ------------------
> # --- Caused by: ---
> #
> # expected: 200
> # got: 500
> #
> # (compared using ==)
> # ./plugins/chat/spec/requests/chat/api/chatables_controller_spec.rb:158:in `block (4 levels) in <main>'
```
This adds specs to the mentioned serializers to catch regressions
with MessageBus last_ids and to ensure the correct ones are being
returned and passed down to the ChannelSerializer.
Followup to d8ad5c3
Joffrey JAFFEUX
Martin Brennan
Jarek Radosz
chapoi
Andrei Prigorshnev
Penar Musaraj
Discourse Translator Bot
Daniel Waterworth
Jan Cernik
Bianca Nenciu
Alan Guo Xiang Tan
David Taylor
Natalie Tay
Meghna
Loïc Guitaut
David Battersby