This PR is introducing glimmer usage in the chat-live-pane, for components but also for models. RestModel usage has been dropped in favor of native classes.
Other changes/additions in this PR:
- sticky dates, scrolling will now keep the date separator of the current section at the top of the screen
- better unread management, marking a channel as unread will correctly mark the correct message and not mark the whole channel as read. Tracking state will also now correctly return unread count and unread mentions.
- adds an animation on bottom arrow
- better scrolling behavior, we should now always correctly keep the scroll position while loading more
- reactions are now more reactive, and will update their tooltip without needed to close/reopen it
- skeleton has been improved with placeholder images and reactions
- when making a reaction on the desktop message actions, the menu won't move anymore
- simplify logic and stop maintaining a list of unloaded messages
* FIX: Use pluralized string
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Fix misuse of pluralized string
* DEV: Remove linting of `one` key in MessageFormat string, it doesn't work
* REFACTOR: Fix misuse of pluralized string
This also ensures that the URL works on subfolder and shows the site setting link only for admins instead of staff. The string is quite complicated, so the best option was to switch to MessageFormat.
* REFACTOR: Fix misuse of pluralized string
* FIX: Use pluralized string
This also ensures that the URL works on subfolder and shows the site setting link only for admins instead of staff.
* REFACTOR: Correctly pluralize reaction tooltips in chat
This also ensures that maximum 5 usernames are shown and fixes the number of "others" which was off by 1 if the current user reacted on a message.
* REFACTOR: Use translatable string as comma separator
* DEV: Add comment to translation to clarify the meaning of `%{identifier}`
* REFACTOR: Use translatable comma separator and use explicit interpolation keys
* REFACTOR: Don't interpolate lowercase channel status
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Don't interpolate channel status
* REFACTOR: Use %{count} interpolation key
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Correctly pluralize DM chat channel titles
Adds a new LookupThread class that handles finding the
thread based on thread + channel ID, checking permissions
and policy/contract checks.
Co-authored-by: Loïc Guitaut <loic@discourse.org>
This commit introduces the skeleton of the chat thread UI. The
structure of the components looks like this. Its done this way
so the side panel can be used for other things as well if we wish,
not just for threads:
```
.main-chat-outlet
<ChatLivePane />
<ChatSidePanel>
<-- rendered with {{outlet}} -->
<ChatThread />
</ChatSidePanel>
```
Later on the `ChatThreadList` will be rendered here as well.
Now, when you go to a channel you can open a thread by clicking
on either the Open Thread message action button or by clicking on
the reply indicator. This will take you to a route like `chat/c/:slug/:channelId/t/:threadId`.
This works on mobile as well.
This commit includes basic serializers and routes for threads,
as well as a new `ChatThreadsManager` service in JS that caches
threads for a channel the same way the channel threads manager does.
The chat messages inside the thread are intentionally left out
until a later PR.
**NOTE: These changes are gated behind the site setting enable_experimental_chat_threaded_discussions
and the threading_enabled boolean on a ChatChannel**
This is a combined work of Martin Brennan, Loïc Guitaut, and Joffrey Jaffeux.
---
This commit implements a base service object when working in chat. The documentation is available at https://discourse.github.io/discourse/chat/backend/Chat/Service.html
Generating documentation has been made as part of this commit with a bigger goal in mind of generally making it easier to dive into the chat project.
Working with services generally involves 3 parts:
- The service object itself, which is a series of steps where few of them are specialized (model, transaction, policy)
```ruby
class UpdateAge
include Chat::Service::Base
model :user, :fetch_user
policy :can_see_user
contract
step :update_age
class Contract
attribute :age, :integer
end
def fetch_user(user_id:, **)
User.find_by(id: user_id)
end
def can_see_user(guardian:, **)
guardian.can_see_user(user)
end
def update_age(age:, **)
user.update!(age: age)
end
end
```
- The `with_service` controller helper, handling success and failure of the service within a service and making easy to return proper response to it from the controller
```ruby
def update
with_service(UpdateAge) do
on_success { render_serialized(result.user, BasicUserSerializer, root: "user") }
end
end
```
- Rspec matchers and steps inspector, improving the dev experience while creating specs for a service
```ruby
RSpec.describe(UpdateAge) do
subject(:result) do
described_class.call(guardian: guardian, user_id: user.id, age: age)
end
fab!(:user) { Fabricate(:user) }
fab!(:current_user) { Fabricate(:admin) }
let(:guardian) { Guardian.new(current_user) }
let(:age) { 1 }
it { expect(user.reload.age).to eq(age) }
end
```
Note in case of unexpected failure in your spec, the output will give all the relevant information:
```
1) UpdateAge when no channel_id is given is expected to fail to find a model named 'user'
Failure/Error: it { is_expected.to fail_to_find_a_model(:user) }
Expected model 'foo' (key: 'result.model.user') was not found in the result object.
[1/4] [model] 'user' ❌
[2/4] [policy] 'can_see_user'
[3/4] [contract] 'default'
[4/4] [step] 'update_age'
/Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/update_age.rb:32:in `fetch_user': missing keyword: :user_id (ArgumentError)
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/base.rb:202:in `instance_exec'
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/base.rb:202:in `call'
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/base.rb:219:in `call'
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/base.rb:417:in `block in run!'
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/base.rb:417:in `each'
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/base.rb:417:in `run!'
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/base.rb:411:in `run'
from <internal:kernel>:90:in `tap'
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/app/services/base.rb:302:in `call'
from /Users/joffreyjaffeux/Code/pr-discourse/plugins/chat/spec/services/update_age_spec.rb:15:in `block (3 levels) in <main>'
```
Triggers a DiscourseEvent when a message is deleted, similar to
`:chat_message_created` and `:chat_message_edited`. This is not used
in this plugin, but can be used by other plugins to act when a message
is trashed.
Deleting a message with a mention doesn't clear the associated notification, confusing the mentioned user.
There are different chat notification types, but we only care about `chat_mentioned` since `chat_quoted` is associated with a post, and `chat_message` is only for push notifications.
Unfortunately, this change doesn't fix the chat bubble getting out of sync when a message gets deleted since we track unread/mentions count with an integer, making it a bit hard to manipulate. We can follow up later if we consider it necessary.
This commit introduces the ability to edit the channel
slug from the About tab for the chat channel when the user
is admin. Similar to the create channel modal functionality
introduced in 641e94f, if
the slug is left empty then we autogenerate a slug based
on the channel name, and if the user just changes the slug
manually we use that instead.
We do not do any link remapping or anything else of the
sort, when the category slug is changed that does not happen
either.
Only allow maximum of `50_000` characters for chat drafts. A hidden `max_chat_draft_length` setting can control this limit. A migration is also provided to delete any abusive draft in the database.
The number of drafts loaded on current user has also been limited and ordered by most recent update.
Note that spec files moved are not directly related to the fix.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Co-authored-by: Régis Hanol <regis@hanol.fr>
Prior to this fix trashed channels would still prevent a channel with the same slug to be created. This commit generates a new slug on trash and frees the slug for future usage.
The format used for the slug is: `YYYYMMDD-HHMM-OLD_SLUG-deleted` truncated to the max length of a channel name.
This commit allows us to set the channel slug when creating new chat
channels. As well as this, it introduces a new `SlugsController` which can
generate a slug using `Slug.for` and a name string for input. We call this
after the user finishes typing the channel name (debounced) and fill in
the autogenerated slug in the background, and update the slug input
placeholder.
This autogenerated slug is used by default, but if the user writes anything
else in the input it will be used instead.
There was an issue with channel archiving, where at times the topic
creation could fail which left the archive in a bad state, as read-only
instead of archived. This commit does several things:
* Changes the ChatChannelArchiveService to validate the topic being
created first and if it is not valid report the topic creation errors
in the PM we send to the user
* Changes the UI message in the channel with the archive status to reflect
that topic creation failed
* Validate the new topic when starting the archive process from the UI,
and show the validation errors to the user straight away instead of
creating the archive record and starting the process
This also fixes another issue in the discourse_dev config which was
failing because YAML parsing does not enable all classes by default now,
which was making the seeding rake task for chat fail.
* FIX: Channel archive N1 when serializing current user
The `ChatChannelSerializer` serializes the archive for the
channel if it is present, however this was causing an N1 for
the current user serializer in the case of DM channels, which
were not doing `includes(:chat_channel_archive)` in the
`ChatChannelFetcher`.
DM channels cannot be archived, so we can just never try to serialize
the archive for DM channels in `ChatChannelSerializer`, which
removes the N1.
* DEV: Add N1 performance spec for latest.html preloading
We modify current user serializer in chat, so it's a good
idea to have some N1 performance specs to avoid regressions
here.
Note this is a very large PR, and some of it could have been splited, but keeping it one chunk made it to merge conflicts and to revert if necessary. Actual new code logic is also not that much, as most of the changes are removing js tests, adding system specs or moving things around.
To make it possible this commit is doing the following changes:
- converting (and adding new) existing js acceptances tests into system tests. This change was necessary to ensure as little regressions as possible while changing paradigm
- moving away from store. Using glimmer and tracked properties requires to have class objects everywhere and as a result works well with models. However store/adapters are suffering from many bugs and limitations. As a workaround the `chat-api` and `chat-channels-manager` are an answer to this problem by encapsulating backend calls and frontend storage logic; while still using js models.
- dropping `appEvents` as much as possible. Using tracked properties and a better local storage of channel models, allows to be much more reactive and doesn’t require arbitrary manual updates everywhere in the app.
- while working on replacing store, the existing work of a chat api (backend) has been continued to support more cases.
- removing code from the `chat` service to separate concerns, `chat-subscriptions-manager` and `chat-channels-manager`, being the largest examples of where the code has been rewritten/moved.
Future wok:
- improve behavior when closing/deleting a channel, it's already slightly buggy on live, it's rare enough that it's not a big issue, but should be improved
- improve page objects used in chat
- move more endpoints to the API
- finish temporarily skipped tests
- extract more code from the `chat` service
- use glimmer for `chat-messages`
- separate concerns in `chat-live-pane`
- eventually add js tests for `chat-api`, `chat-channels-manager` and `chat-subscriptions-manager`, they are indirectly heavy tested through system tests but it would be nice to at least test the public API
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
Previously, restricted category chat channel was available for all groups - even `readonly`. From now on, only user who belong to group with `create_post` or `full` permissions can access that chat channel.
* FEATURE: Enforce mention limits for chat messages
The first part of these changes adds a new setting called `max_mentions_per_chat_message`, which skips notifications when the message contains too many mentions. It also respects the `max_users_notified_per_group_mention` setting
and skips notifications if expanding a group mention would exceed it.
We also include a new component to display JIT warning for these limits to the user while composing a message.
* Simplify ignoring/muting filter in chat_notifier
* Post-send warnings for unsent warnings
* Improve pluralization
* Address review feedback
* Fix test
* Address second feedback round
* Third round of feedback
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
The settings tab of each category channel should now present the option to allow or disallow channel wide mentions: @here and @all.
When disallowed, using these mentions in the channel should have no effect.
This commit adds variousMessageBus.last_ids to serializer payloads
for chat channels and the chat view (for chat live pane) so
we can use those IDs when subscribing to MessageBus channels
from chat.
This allows us to ensure that any messages created between the
server being hit and the UI loaded and subscribing end up being
delivered to the client, rather than just silently dropped.
This commit also fixes an issue where we were subscribing to
the new-messages and new-mentions MessageBus channels multiple
times when following/unfollowing a channel multiple times.
Only allow maximum of 6000 characters for chat messages when they
are created or edited. A hidden setting can control this limit,
6000 is the default.
There is also a migration here to truncate any existing messages to
6000 characters if the message is already over that and if the
chat_messages table exists. We also set cooked_version to NULL
for those messages so we can identify them for rebake.
Currently when a category is deleted, if it has an associated chat
channel, the latter won’t be deleted automatically.
The fix is quite simple as we were simply missing a `dependent:
:destroy` option on the existing relation.
This commit automatically ensures that category channels
have slugs when they are created or updated based on the
channel name, category name, or existing slug. The behaviour
has been copied from the Category model.
We also include a backfill here with a simplified version
of Slug.for with deduplication to fill the slugs for already
created Category chat channels.
The channel slug is also now used for chat notifications,
and for the UI and navigation for chat. `slugifyChannel`
is still used, but now does the following fallback:
* Uses channel.slug if it is present
* Uses channel.escapedTitle if it is present
* Uses channel.title if it is present
In future we may want to remove this altogether
and always rely on the slug being present, but this
is currently not possible because we are not generating
slugs for DM channels at this point.
Currently it’s not possible to delete a category if an associated chat
channel is present even if there are no messages in this channel.
This can lead to annoying situations for our users.
This patch addresses the issue by checking if the channel is empty
instead of just checking if there is a channel.
Follow up to 766bcbc684
This fixes a gaffe from that commit where I passed in the
guardian to ChatMessageUpdater but then forgot to remove
the old way of setting the guardian and user instance variables
from the chat_message that was passed in.
Also, it moves the ensure_can_edit_message! check from the
controller into ChatMessageUpdater so all the access
checks are in the same place.
This is a followup of the previous refactor where we created two new
models to handle all the dedicated logic that was present in the
`ChatChannel` model.
For the sake of consistency, `DMChannel` has been renamed to
`DirectMessageChannel` and the previous `DirectMessageChannel` model is
now named `DirectMessage`. This should help reasoning about direct
messages.
