This commit creates a shared implementation of the dates computation and moves all the logic (new messages since last visit and dates separator into one single component <ChatMessageSeparator />).
The frontend tests have been removed and only a single system spec has been added for threads as everything is sharing the same implementation and the existing channel specs should catch any regression.
Allows users to create DMs by selecting groups as a target. It also allows adding user groups to an existing chat
- When creating the channel, it expands the user group and adds all its members with chat enabled to the channel.
- After creation, there's no difference between adding a group or adding its members individually.
- Users can add multiple groups and users simultaneously.
- There are UI validations; the member count preview updates according to the member count of added groups, and it does not allow users to add more members than SiteSetting.chat_max_direct_message_users."
At the moment, when someone is mentioning a group, or using here or
all mention, we create a chat_mention record per user. What we want
instead is to have special kinds of mentions, so we can create only one
chat_mention record in such cases. This PR implements that.
Note, that such mentions will still have N related notifications, one
notification per a user. We don't expect we'll have performance
problems on the notifications side, but if at some point we do, we
should be able to solve them on the side of notifications
(notifications are handled in jobs, also some little delays with
the notifications are acceptable, so we can make sure notifications
are properly queued, and that processing of every notification is
fast enough to make delays small enough).
The preparation work for this PR was done in fbd24fa, where we make
it possible for one mention to have several related notifications.
A pretty tricky part of this PR is schema and data migration, I've explained
related details inline on the migration files.
This update adds three tabs to the bottom of the chat overlay to make it easier for users to navigate chat on mobile.
As a result of this change:
- Direct Messages are now shown separately from public channels on mobile
- My Threads has now moved from the channel list to it's own tab on mobile
- My Threads can still be accessed on desktop via the sidebar and within the drawer channel list
- Chat back button has been updated to navigate to the correct tab (for both channels and threads)
Some special cases:
- If DMs are not used then the tab is not rendered
- If the user has no threads then the tab is not rendered
- If both the tabs for DMs and Threads aren't available then the whole footer will not be rendered
- Chat footer is only shown on the listing pages (DMs, Channels, My Threads)
---------
Co-authored-by: chapoi <101828855+chapoi@users.noreply.github.com>
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This regressed in 2791e75072. That commit
fixed subfolder URLs in general, but the `full_url` was adding the
subfolder prefix a second time, thus breaking this URL in emails.
Prior to this fix the number of users rendered by mentioned_users could equal the number of members in a channel which would be slow but could in more extreme case crash the page and/or server.
When validating with a dynamic set of values, especially one that might change during runtime, we should use a lambda or a proc to ensure that the validation uses the most up-to-date set of values. This is particularly important when using config.eager_load = true, which can cause some elements to be loaded only once at startup, thus not reflecting changes made at runtime.
This was the root cause of the issues here, as we were adding more ReviewableScore types after initial load through: `register_reviewable_type Chat::ReviewableMessage`
This PR is a reworked version of https://github.com/discourse/discourse/pull/24670.
In chat, we need the ability to have several notifications per `chat_mention`.
Currently, we have one_to_one relationship between `chat_mentions` and `notifications`:
d7a09fb08d/plugins/chat/app/models/chat/mention.rb (L9)
We want to have one_to_many relationship. This PR implements that by introducing
a join table between `chat_mentions` and `notifications`.
The main motivation for this is that we want to solve some performance problems
with mentions that we're having now. Let's say a user sends a message with @ all
in a channel with 50 members, we do two things in this case at the moment:
- create 50 chat_mentions
- create 50 notifications
We don't want to change how notifications work in core, but we want to be more
efficient in chat, and create only 1 `chat_mention` which would link to 50 notifications.
Also note, that on the side of notifications, having a lot of notifications is not so
big problem, because notifications processing can be queued.
Apart from improving performance, this change will make the code design better.
Note that I've marked the old `chat_mention.notification_id` column as ignored, but
I'm not deleting it in this PR. We'll delete it later in https://github.com/discourse/discourse/pull/24800.
This commit adds a new "My threads" link in sidebar and drawer. This link will open the "/chat/threads" page which contains all threads where the current user is a member. It's ordered by activity (unread and then last message created).
Moreover, the threads list of a channel page is now showing every threads of a channel, and not just the ones where you are a member.
I took the wrong approach here, need to rethink.
* Revert "FIX: Use Guardian.basic_user instead of new (anon) (#24705)"
This reverts commit 9057272ee2.
* Revert "DEV: Remove unnecessary method_missing from GuardianUser (#24735)"
This reverts commit a5d4bf6dd2.
* Revert "DEV: Improve Guardian devex (#24706)"
This reverts commit 77b6a038ba.
* Revert "FIX: Introduce Guardian::BasicUser for oneboxing checks (#24681)"
This reverts commit de983796e1.
c.f. de983796e1
There will soon be additional login_required checks
for Guardian, and the intent of many checks by automated
systems is better fulfilled by using BasicUser, which
simulates a logged in TL0 forum user, rather than an
anon user.
In some cases the use of anon still makes sense (e.g.
anonymous_cache), and in that case the more explicit
`Guardian.anon_user` is used
The previous query would look at the existing messages, count them, and update the associated thread.
But, if for some reason messages were **ALL** deleted without updating the `replies_count`, then the query wouldn't find any message, and wouldn't update any thread's `replies_count`.
In other kind of channels we will only unfollow but for group channels we don't want people to keep appearing in members list.
This commit also creates appropriate services:
- `Chat::LeaveChannel`
- `Chat::UnfollowChannel`
And dedicated endpoint for unfollow: `DELETE /chat/api/channels/:id/memberships/me/follows`
This bug was very reproducible when your last read was a message you didn't read and an admin would delete it. When coming back to the channel you would get a not found, in this case we will now reset last read and present you the last message of the channel.
We could be more fancy and try to detect the next readable message but that would be more code and complexity for such a rare case.
Chat will now check for the state of `SiteSetting.private_email` when sending the summary, when enabled, the mail will not display user information, channel information other than the ID and no message information, only the count of messages.
Mentions and other post processing (like images) are still done asynchronously in the background. This should ensure reloading a channel while the message has not been processed yet doesn’t renders a blank message.
As a followup, we could probably simplify the staged message logic, given we have the new cooked on send.
This commit implements drafts for threads by adding a new `thread_id` column to `chat_drafts` table. This column is used to create draft keys on the frontend which are a compound key of the channel and the thread. If the draft is only for the channel, the key will be `c-${channelId}`, if for a thread: `c-${channelId}:t-${threadId}`.
This commit also moves the draft holder from the service to the channel or thread model. The current draft can now always be accessed by doing: `channel.draft` or `thread.draft`.
Other notable changes of this commit:
- moves ChatChannel to gjs
- moves ChatThread to gjs
Group channels will allow users to create channels with a name and invite people. It's possible to add people even after creation of the channel. Removing users is not yet possible but will be added in the near future.
Technically a group channel is `direct_message_channel` with a group attribute set to true on its direct message (chatable). This model might evolve in the future but offers much flexibility for now without having to rely on a complex migration.
The commit essentially consists of:
- a migration to set existing direct message channels with more than 2 users to a group
- a new message creator which allows to search, add members, and create groups
- a new `AddUsersToChannel` service
- a modified `SearchChatable` service
This commit starts from a simple observation: cooking messages on the hot path can be slow. Especially with a lot of mentions.
To move cooking from the hot path, this commit has made the following changes:
- updating cooked, inserting mentions and notifying user of new mentions has been moved inside the `process_message` job. It happens right after the `Chat::MessageProcessor` run, which is where the cooking happens.
- the similar existing code in `rebake!` has also been moved to rely on the `process_message`job only
- refactored `create_mentions` and `update_mentions` into one single `upsert_mentions` which can be called invariably
- allows services to decide if their job is ran inline or later. It avoids to need to know you have to use `Jobs.run_immediately!` in this case, in tests it will be inline per default
- made various frontend changes to make the chat-channel component lifecycle clearer. we had to handle `did-update @channel` which was super awkward and creating bugs with listeners which the changes of the PR made clear in failing specs
- adds a new `-processed` (and `-not-processed`) class on the chat message, this is made to have a good lifecyle hook in system specs
We were incorrectly generating URLs with message id even when it was not provided, resulting in a route ending with "undefined", which was causing an error.
This commit also uses this opportunity to:
- move `invite_users` into a proper controller inside the API namespace
- refactors the code into a service: `Chat::InviteUsersToChannel`
This change allows users to edit their chat messages based on the criteria added to Site Settings.
If the grace period conditions are met then there will be no (edited) text applied to the message.
The following site settings are added to chat:
chat editing grace period (seconds since message created)
chat editing grace period max diff for low trust levels (number of characters changed)
chat editing grace period max diff for high trust levels (number of characters changed)
Users can decide to hide their profile and presence. It seems more correct to also not return the status in this case.
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
At the moment writing a mention similar to `@bob...hi` would have resulted in chat trying to find a user named `bob...hi` which would fail.
This was due to the `replacements` rule not being present in the rules used to cook chat messages.
We are still missing few default rules like: normalize, smartquotes, text_join, ... which don't seem to be necessary but could be added if we found a reason for. More info at: e476f78bc3/lib/parser_core.js
Workaround for an issue we are experiencing on thread index frontend where thread loads participants correctly (up to 10), then refreshes the threads and then limits to 3 participants.
There is an issue with storing threads for the main channel view and the thread list in the same store so handling the max participants on the frontend is a workaround until channel.threadsManager is updated.
I've adjusted the tests to handle the additional data being returned from ThreadParticipantQuery.
UX changes to thread item:
- drop "last reply" timestamp copy
- drop last reply excerpt
- show up 9+OP members
Co-authored-by: David Battersby <info@davidbattersby.com>
This would cause an error when deleting the original message of a thread, due to the non existing `last_message`. This fix is implemented using the null pattern.
Note this commit is also using this opportunity to unify naming of null objects, `Chat::DeletedUser` becomes `Chat::NullUser`, it feels better to have a name describing what is the object, instead of a name describing why this object has to be used, which can change depending on cases.
While very fast and powerful staged threads forces a lot of gymnastic and edge cases. This patch adds a new service `Chat::CreateThread` and uses it to create a thread unconditionally when a user replies to a message in a threading enabled channel. If the user actually doesn’t send a message we will have a thread with no messages which has no important impact and could even be periodically cleaned if necessary.
Note that this commit also moves message actions to .gjs as it was the original goal of this PR to correctly check for staged thread to show the menu or not.
Introduce max length on text columns for description and slug fields within chat.
At a later date we will probably want to convert these text columns to string/varchar through a migration, but for now this change introduces a limit within the active record model.
Currently, the logic for creating a new chat message is scattered
between a controller and an “old” service.
This patch address this issue by creating a new service (using the “new”
sevice object system) encapsulating all the necessary logic.
(authorization, publishing events, etc.)
Chat review queue flags were missing the context message above the actions.
This is probably because the (reasonably complex) logic was somewhat hard-coded to posts. After some investigation I concluded we can reuse this logic with some small amendments.
This PR swaps out the custom pathway to publishing and rendering mention warnings after a message is sent.
ChatPublisher#publish_notice is used, and expanded. Now, instead of only accepting text_content as an argument, component and component_args are accepted and there is a renderer for these components.
Translations moved to server, as notices expect text to be passed in unless a component is rendered
The warnings are rendered at the top now, outside of the scope of the single message that sent it.
I entirely removed the jit_messages_spec b/c it's duplicate testing of other parts of the app. IMO we don't need a backend test for a feature, a component test for the feature AND a system test (that is slow and potentially even flakey due to timing issues with wait) to test the same thing. So jit_messages_spec is gone.
This moves the "delete message" action (if it is available) of a flagged chat message under the "ignore" menu. This puts it on par with the menu for flagged posts.
- drop @
- prevents +X (participants) to show on next line
- few spacing/fonts adjustments
Note that this commit is also stripping links from chat excerpts.
It will now replies count and participants list. Also the title will be OM excerpt or user defined title, no more default "Thread" title. Lastly, the author of the last reply is also shown as prefix of it.
This is also fixes the issue of chat composer warnings persisting across channels. Currently if you try to mention more groups than is allowed for example, a mention warning pops up. When you change channels the mention warning will not disappear even if there is no text in the composer.
This adds a reset function to the chat-composer-warnings-tracker.js, which is called when the channel is changed and the message is empty. In the event that the message is not empty we call captureMentions to check the loaded drafts' mentions.
This PR would be nicer if the post-send notice used the new chat notices API to publish the mention warnings but we would have to change the existing ones and I thought that would be too much change for this PR. It'd be a good followup though.
Each time a message is created through a webhook, we create we webhook_event associated to this webhook.
When destroying a webhook, we were not destroying the webhook_events which was causing orphans records and more importantly errors in the app expecting to find and associated webhook.
I suspect it was moreover possibly related to a flaky spec:
```
1) Chat::AutoJoinChannelBatch.call when arguments are valid when channel is found when more than one membership is created publishes an event
Failure/Error: subject(:result) { described_class.call(params) }
Mocha::ExpectationError:
unexpected invocation: Chat::Publisher.publish_new_channel(#<Chat::CategoryChannel:0x401f28>, #<User::ActiveRecord_Relation:0x401f50>)
unsatisfied expectations:
- expected exactly once, invoked never: Chat::Publisher.publish_new_channel(#<Chat::CategoryChannel:0x401f78>, [#<User:0x401fa0>, #<User:0x401fc8>])
satisfied expectations:
- allowed any number of times, invoked once: Chat::Action::CreateMembershipsForAutoJoin.call(has_entries({:channel => #<Chat::CategoryChannel:0x401f78>, :contract => instance_of(Chat::AutoJoinChannelBatch::Contract)}))
- allowed any number of times, invoked never: Chat::ChannelMembershipManager.new(#<Chat::CategoryChannel:0x401f78>)
- allowed any number of times, invoked never: #<Mock:0x402018>.recalculate_user_count(any_parameters)
# ./plugins/chat/app/services/chat/auto_join_channel_batch.rb:65:in `publish_new_channel'
# ./plugins/chat/app/services/service/base.rb:118:in `instance_exec'
# ./plugins/chat/app/services/service/base.rb:118:in `call'
# ./plugins/chat/app/services/service/base.rb:368:in `block in run!'
# ./plugins/chat/app/services/service/base.rb:368:in `each'
# ./plugins/chat/app/services/service/base.rb:368:in `run!'
# ./plugins/chat/app/services/service/base.rb:361:in `run'
# ./plugins/chat/app/services/service/base.rb:229:in `call'
# ./plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb:50:in `block (3 levels) in <main>'
# ./plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb:110:in `block (6 levels) in <main>'
# ./spec/rails_helper.rb:393:in `block (2 levels) in <top (required)>'
```
Prior to this fix `context.membership&.update!(last_viewed_at: Time.zone.now)` would generate an update statement from a GET request which is not permitted by default when in readonly mode.
The usual fix in this case is to check for readonly or rescue an error, however, this common pattern of updating "last seen" or similar can be better handled in a `Schedule::Defer` block, which won't raise the `ActiveRecord::ReadOnlyError` when in readonly and will also prevent the controller to wait for this operation.
What is the problem here?
In multiple controllers, we are accepting a `limit` params but do not
impose any upper bound on the values being accepted. Without an upper
bound, we may be allowing arbituary users from generating DB queries
which may end up exhausing the resources on the server.
What is the fix here?
A new `fetch_limit_from_params` helper method is introduced in
`ApplicationController` that can be used by controller actions to safely
get the limit from the params as a default limit and maximum limit has
to be set. When an invalid limit params is encountered, the server will
respond with the 400 response code.
This is extracted from #22390.
This patch introduces a scope to avoid duplication and a new method,
`Chat::Channel.find_by_id_or_slug` to allow finding a channel either by
its id or by its slug (or its category slug).
`Jobs::AutoJoinChannelBatch` was holding a lot of logic which should be in a service. Moreover, this refactoring is the opportunity to address a bug which could cause a duplicate key error.
From now when trying to insert a new membership it won't fail if a membership is already present.
Example error:
```
Job exception: ERROR: duplicate key value violates unique constraint "user_chat_channel_unique_memberships"
DETAIL: Key (user_id, chat_channel_id)=(1, 2) already exists.
Backtrace
rack-mini-profiler-3.1.0/lib/patches/db/pg.rb:110:in `exec'
rack-mini-profiler-3.1.0/lib/patches/db/pg.rb:110:in `async_exec'
(eval):29:in `async_exec'
mini_sql-1.4.0/lib/mini_sql/postgres/connection.rb:209:in `run'
mini_sql-1.4.0/lib/mini_sql/active_record_postgres/connection.rb:38:in `block in run'
mini_sql-1.4.0/lib/mini_sql/active_record_postgres/connection.rb:34:in `block in with_lock'
activesupport-7.0.5.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
activesupport-7.0.5.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
activesupport-7.0.5.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
activesupport-7.0.5.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
mini_sql-1.4.0/lib/mini_sql/active_record_postgres/connection.rb:34:in `with_lock'
mini_sql-1.4.0/lib/mini_sql/active_record_postgres/connection.rb:38:in `run'
mini_sql-1.4.0/lib/mini_sql/postgres/connection.rb:64:in `query_single'
/var/www/discourse/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb:38:in `execute'
```
Note this commit is also using main branch of `shoulda-matchers` as the gem has not been released yet.
Co-authored-by: Loïc Guitaut <5648+Flink@users.noreply.github.com>
Prior to this commit we were loading a large number of thread messages without any pagination. This commit attempts to fix this and also improves the following points:
- code sharing between channels and threads:
Attempts to reuse/share the code use in channels for threads. To make it possible part of this code has been extracted in dedicated helpers or has been improved to reduce the duplication needed.
Examples of extracted helpers:
- `stackingContextFix`: the ios hack for rendering bug when momentum scrolling is interrupted
- `scrollListToMessage`, `scrollListToTop`, `scrollListToBottom`: a series of helper to correctly scroll to a specific position in the list of messages
- better general performance of listing messages:
One of the main changes which has been made is to remove the computation of visible message during scroll, it will only happen when needed (update last read for example). This constant recomputation of `message.visible` on intersection observer event while scrolling was consuming a lot of CPU time.
This is extracted from #22390.
This patch simplifies a little how we handle uploads in chat, relying on
ActiveRecord mechanisms instead of calling custom methods.
This also makes `Chat::Message#validate_message` a “real” AR validation,
meaning it will run automatically when `#valid?` is called.
This only moves code around and doesn't change any behavior. This does two things:
1. Extracts the `channel.joined_by?` methods
2. Uses term "members" instead of "participants" for chat members
This is extracted from #22390.
This patch adds a new `optional` option to the `model` step. This
means if an optional model returns something blank (`nil` or an empty
collection) then the service won’t fail and will execute the next step.
However if a model is properly returned, the step will try to check if
it is valid or not (if it responds to `#invalid?`). If the model isn’t
valid, then the step will fail (so no change here).
Someone who cannot chat is also not able to join chat channels,
so we may not check all the time user.can_chat? && user.can_join_chat_channel?
and just call user.can_join_chat_channel? instead.
This commit makes it so that when the user has unread threads
for a channel we show a blue dot in the sidebar (or channel index
for mobile/drawer).
This blue dot is slightly different from the channel unread messages:
1. It will only show if the new thread messages were created since
the user last viewed the channel
2. It will be cleared when the user views the channel, but the threads
are still considered unread because we want the user to click into
the thread list to view them
This necessitates a change to the current user serializer to also
include the unread thread overview, which is all unread threads
across all channels and their last reply date + time.
Followup to d7ef7b9c03,
this adds a spec to test the case where old threads are
still unread for the user and should show at the top regardless
of pagination, and fixes some issues/makes some slight refactors.
This commit attempts to fix an issue where we are ending
up with bad created_at date formats for last messages, which
is breaking the DM sort order and sometimes causing DM channels
to fall off the list, or show "Invalid date" on mobile.
I have not been able to consistently reproduce these issues
locally, however the serialzier for the channels index uses
MultiJSON.dump() and the Chat::Publisher uses .to_json, both of
which format created_at differently for messages.
The former is `2023-07-05T06:53:25.977Z` (iso8601).
The latter is `2023-07-14 03:59:22 UTC` (.to_s default).
Since we are doing comparison and sorting of these dates on the UI
we need consistent formatting for the JS Date parsers (and moment)
to deal with.
If the issue still occurs after this we can investigate further.
It could only occur on message created by the user itself and deleted while the user was looking at the channel.
It more generally fix the trash service which was not correctly setting the author of the delete.
`SiteSetting.enable_public_channels` allows site admin to decide if public channels are available at all. There's no distinction between admins or not as we expect admins to create private category channels if they want to limit usage.
Initial migration and changes to models as well as
changing the following services to update last_message_id:
* Chat::MessageCreator
* Chat::RestoreMessage
* Chat::TrashMessage
The data migration will set the `last_message_id` for all existing
threads and channels in the database.
When we query the thread list as well as the channel,
we look at the last message ID for the following:
* Channel - Sorting DM channels, and channel metadata for the list of channels
* Thread - Last reply details for thread indicators and thread list
* FEATURE: Inline topic summary. Cached version accessible to everyone.
Anons and non-members of the `custom_summarization_allowed_groups_map` groups can see cached summaries for any accessible topic. After the first 12 hours and if the posts to summarize have changed, allowed users clicking on the button will automatically re-generate it.
* Ensure chat summaries work and prevent model hallucinations when there are no messages.
This implementation will need more work in the future. For simplification of tracking and other events (new thread, delete/restore OM...) we used the threads from `threadsManager` which makes pagination more complicated as we already have some results when we start.
Note this commit also simplify `Collection` to only have one `load` method which can be called repeatedly.
When a user sends their first message in a thread we
automatically track the thread in the backend, but we
don't reflect this in the UI until the user re-opens
the thread. This commit fixes that by showing the new
tracking level in the UI.
In previous changes we prevented creating a channel to also make users follow the channel. We were forcing recipients to follow the channel on message sent but this was not including the creator of the message itself.
This commit fixes it and also write an end-to-end system spec to cover these cases. The message creator service is currently being rewritten and should correctly test and ensure this logic is present.
This commit also makes changes on the frontend to instantly follow a DM when you open it, this change prevents a green dot to appear for a split second when you send a message in a channel you were previously not following. Only recipients will see the green dot.
Since we create threads in the background regardless of whether
threading is enabled for a channel, we get the unexpected behaviour
of everyone having a lot of unread threads when threading is enabled
for the channel.
To counteract this, when the admin enables threads for a channel
we can just run a high priority background job to mark all threads
as read in the channel for all users, so they are essentially
starting from a clean slate.
This commit includes several fixes and improvements to thread
original message handling:
1. When a thread's original message is deleted, the thread no longer
counts as unread for a user
2. When a thread original message is deleted and the user is looking
at the thread list, it will be removed from the list
3. When a thread original message is restored and the user is looking
at the thread list, it will be added back to the list if it was
previously loaded
This commit replaces two existing screens:
- draft
- channel selection modal
Main features compared to existing solutions
- features are now combined, meaning you can for example create multi users DM
- it will show users with chat disabled
- it shows unread state
- hopefully a better look/feel
- lots of small details and fixes...
Other noticeable fixes
- starting a DM with a user, even from the user card and clicking <kbd>Chat</kbd> will not show a green dot for the target user (or even the channel) until a message is actually sent
- it should almost never do a full page reload anymore
---------
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Jordan Vidrine <30537603+jordanvidrine@users.noreply.github.com>
Co-authored-by: chapoi <101828855+chapoi@users.noreply.github.com>
Co-authored-by: Mark VanLandingham <markvanlan@gmail.com>
Without this fix, the following error is raised:
```
ActiveRecord::StatementInvalid:
PG::SyntaxError: ERROR: syntax error at or near ")"
LINE 4: WHERE thread_id IN ()
```
This fixes a longstanding TODO to move the contents of the
UpdateUserCountsForChannels job to the ensure_consistency!
method of Chat::Channel, which runs every 15 mins as part of
periodical updates.
This commit also addresses the performance issue of the original,
where we would fetch all channels and do an individual query to
get the count and update the count of each one. Now we do it all
in one query, and only publish the changed channels to the UI.
This will be used when we move the channel creation for DMs
to happen when we first send a message in a DM channel to avoid
a double-request. For now we can just have a new API endpoint
for creating this that the existing frontend code can use,
that uses the new service pattern.
This also uses the new policy pattern for services where the policy
can be defined in a class so a more dynamic reason for the policy
failing can be sent to the controller.
Co-authored-by: Loïc Guitaut <loic@discourse.org>
Enabling/Disabling threading has been possible through command line until now. This commit introduces two new UIs:
- When creating a channel, it will be available once the category has been selected
- On the settings page of a channel for admins
Whenever a user opens a channel or marks it read, we now
update the last_viewed_at datetime for that channel membership
record. This is so we will be able to show thread unread indicators
in the channel sidebar that clear independently of the main thread
unread indicators. This unread functionality will follow in another
PR.
Followup to c6b43ce68b
We can just use the rich excerpt everywhere since we know
we don't need text_entities -- that introduced security issues
just to fix a spec.
Followup to 1526d1f97d
This commit fixes an N1 for mentions/user status
when querying chat threads. This only happened if
any of the thread OMs had mentions.
* FEATURE: Sort thread list by unread threads first
This commit changes the thread list to show the threads that
have unread messages at the top of the list sorted by the
last reply date + time, then all other threads sorted by
last reply date + time.
This also fixes some issues by removing the last_reply
relationship on the thread, which did not work for complex
querying scenarios because its order would be discarded.
* FIX: Various fixes for thread list loading
* Use the channel.threadsManager and find the channel first rather
than use activeChannel in the threads manager, otherwise we may
be looking at differenct channels.
* Look at threadsManager directly instead of storing result for threads
list otherwise it can get out of sync because of replace: true in
other places we are loading threads into the store.
* Fix sorting for thread.last_reply, needed a resort.
Updates the interface for implementing summarization strategies and adds a cache layer to summarize topics once.
The cache stores the final summary and each chunk used to build it, which will be useful when we have to extend or rebuild it.
To export chat messages, go to `/admin/plugins/chat` and click the Create export
button in the _Export chat messages_ section. You'll receive a direct message
when the export is finished.
Currently, this exports all messages from the last 6 months, but not more than
10000 messages.
This exports all chat messages, including messages from private channels and
users' direct conversations. This also exports messages that were deleted.
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 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.
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'
This fixes an issue where a user could send an empty
string as a chat message .e.g ' ' and the message would
be posted. We don't want this, we need to strip the message
first before validating for length etc.
We were calling reset without the proper params which was causing errors in the console. This commit does the following changes:
- ensures `composer.cancel()` is the only way to cancel editing/reply
- adds a `draftSaved` property to chat message to allow for better tests
- writes a spec to ensure the flow is correct
- adds more page objects for better tests
- homogenize the default state of objects on chat message
Co-authored-by: Martin Brennan <martin@discourse.org>
Followup 55ef2d0698.
In the cases where the user has no last_read_message_id for
a channel, we want to make sure that a page_size is set for
the ChannelViewBuilder + MessagesQuery, otherwise we end up
loading way more messages than needed (the additional message
loading was fixed in the last commit).
This commit attempts to fix the case where the messages loaded initially don't fill the screen. It would prevent user to scroll and as a result to load more.
There are multiple fixes in this commit:
- the main fix is removing this code which was preventing the actual fill:
```javascript
// prevents an edge case where user clicks bottom arrow
// just after scrolling to top
if (loadingPast && this.#isAtBottom()) {
return;
}
```
- ensures we always give a page site to the `chatApi.channel(...)` call if we have one, in the current state when `fetchFromLastRead` was `true` we would not set `args.page_size`
- ensures the `query_paginated_messages` is having a valid page size, which is not nil and not > `MAX_PAGE_SIZE`
- write a spec for the autofill, it was a challenging spec to write but it should give us the confidence we need here
Since 5cce829 and the new
channel view builder, we have no need of these obsolete
routes which have way too much logic in the controller, which
has been superseded by the view builder anyway.
Remove the routes and update the channel message loading to use it.
* Moved the settings cog from thread list to thread and
put it in a new header component
* Remove thread original message component, no longer needed
and the list item and thread indicator styles/content
will be quite different
* Start adding content (unread indicator etc.) to the thread
list item and changing structure to be more like designs
* Serialize the last thread reply when opening the thread index,
show in list and update with message bus
Followup to d4a5b79592,
this introduced an N1 because every message in the list
we had to query users for the mentions and then the user's
status too. Instead we can just include both in Chat::MessagesQuery.
#### FIX: Do not use client lastReadMessageId when fetching channel messages
We had an issue where the following happened:
1. User opened channel and saw the last message, and we set the
lastReadMessageId on the server and the client
2. User navigated to another channel
3. Another user deleted the message in the original channel
4. The first user navigated back to the original channel before
the MessageBus event for the deleted message arrived, and got
a 404 error because we were sending the deleted lastReadMessageId as
target_message_id to the channel controller.
Instead of this which is a bit flaky and is hard to cover all
the issues for, instead we can pass a fetch_from_last_read boolean
param to the channels controller, and just get the user's
last_read_message_id straight from the database to use for the
target_message_id. This gets rid of any sources of race conditions
or lack of updates from MessageBus.
#### FIX: Include missing memberships for thread tracking publish
When we publish the channel/message tracking state for a
user and that message was a thread reply the publisher
was erroring because we were not telling Chat::TrackingStateReportQuery
to return missing memberships (which have zeroed out unread counts)
as well, which is what we do for the channel tracking state here.
Also just make sure that the TrackingStateReport does not error
when passed an ID it doesn't have data for.
This commit follows up b6c5a2da08
by serializing the user's thread memberships in these cases:
1. When we do the initial channel fetch with messages, we get
all threads and all the user's thread memberships for those
messages.
2. When the thread list is fetched, we get all the user's memberships
in that list.
3. When the single thread is fetched, either from opening it from
the list, an OM indicator, or just from doing .find() on the
manager when a new MessageBus message comes in
This will let us track the lastReadMessageId on the client, and
will also let us fix an issue where the unread indicator in the
channel header was incrementing for every thread that got a
new message, regardless of whether the user was a member.
This patch introduces policy objects to chat services. It allows putting
more complex logic in a dedicated class, which will make services
thinner. It also allows providing a reason why the policy failed.
Some change has been made to the service runner too to use more easily
these new policy objects: when matching a failing policy (or any failing
step actually), the result object is now provided to the block. This
way, instead of having to access the reason why the policy failed by
doing `result["result.policy.policy_name"].reason` inside the block,
this one can be simply written like this:
```ruby
on_failed_policy(:policy_name) { |policy| policy.reason }
```
This commit adds the thread index and individual thread
in the index list unread indicators, and wires up the message
bus events to mark the threads as read/unread when:
1. People send a new message in the thread
2. The user marks a thread as read
There are several hacky parts and TODOs to cover before
this is more functional:
1. We need to flesh out the thread scrolling and message
visibility behaviour. Currently if you scroll to the end
of the thread it will just mark the whole thread read
unconditionally.
2. We need to send down the thread current user membership
along with the last read message ID to the client and
update that with read state.
3. We need to handle the sidebar unread dot for when threads
are unread in the channel and clear it based on when the
channel was last viewed.
4. We need to show some indicator of thread unreads on the
thread indicators on original messages.
5. UI improvements to make the experience nicer and more
like the actual design rather than just placeholders.
But, the basic premise around incrementing/decrementing the
thread overview count and showing which thread is unread
in the list is working as intended.
This PR adds status to mentions in chat and makes those mentions receive live updates.
There are known unfinished part in this implementation: when posting a message, status on mentions on that message appears immediately, but only if a user used autocomplete when typing the message. If user copy and paste a message with mentions into chat composer, those mentions won't have user status on them.
PRs with fixes for both problems are following soon.
Preparations for this PR that were made previously include:
- DEV: correct a relationship – a chat message may have several mentions 0dcfd7ddec
- DEV: extract the logic for extracting and expanding mentions from ChatNotifier 75b81b6854
- DEV: Always create chat mention records fa543cda06
- DEV: better split create_notification! and send_notifications logic e292c45924
- DEV: more tests for mentions when updating chat messages e7292e1682
- DEV: extract updating status on mentions into a lib function e49d338c21
- DEV: Create and update chat message mentions earlier 35a414bb38
- DEV: Create a chat_mention record when self mentioning 2703f2311a
- DEV: When deleting a chat message, do not delete mention records f4fde4e49b
In the ChannelViewBuilder, we introduced a check to see if
the target message exists, which errors if the message has
been trashed. However if the user is the creator of the message
or admin then they are able to see trashed messages, so
we need to take this into account.
Followup to c908eeacc9
Instead of using the latest message ID in the channel, which
could cause issues if you have an earlier last read message ID
that matches the deleted one, instead we use the first non-deleted
message that comes before the deleted message by ID.
Followup ae3231e140, when a
message is trashed we already update the lastReadMessageId of
all users in the channel to the latest non-deleted message on
the server side. However we didn't propagate this to the client,
so in some cases when we did the following:
1. Delete the last message in the channel
2. Switch to another channel
3. Switch back to the original
We would get a 404 error from the target message ID being looked
up still being the old lastReadMessageId (now deleted) for the
user's channel membership.
All we need to do is send the last not-deleted message ID for
the channel (or thread) to all the member users.
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>
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.
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).
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
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.
- `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.
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.
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
* 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.
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>
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.
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.
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.
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.
