Prior to this change `registered_bookmarkable` would return `nil` as `type` in `Bookmark.registered_bookmarkable_from_type(type)` would be `ChatMessage` and we registered a `Chat::Message` class.
This commit will now properly rely on each model `polymorphic_class_for(name)` to help us infer the proper type from a a `bookmarkable_type`.
Tests have also been added to ensure that creating/destroying chat message bookmarks is working correctly.
---
Longer explanation
Currently when you save a bookmark in the database, it's associated to another object through a polymorphic relationship, which will is represented by two columns: `bookmarkable_id` and `bookmarkable_type`. The `bookmarkable_id` contains the id of the relationship (a post ID for example) and the `bookmarkable_type` contains the type of the object as a string by default, (`"Post"` for example).
Chat plugin just started namespacing objects, as a result a model named `ChatMessage` is now named `Chat::Message`, to avoid complex and risky migrations we rely on methods provided by rails to alter the `bookmarkable_type` when we save it: we want to still save it as `"ChatMessage"` and not `"Chat::Message"`. And, to retrieve the correct model when we load the bookmark from the database: we want `"ChatMessage"` to load the `Chat::Message` model and not the `ChatMessage`model which doesn't exist anymore.
On top of this the bookmark codepath is allowing plugins to register types and will check against these types, so we alter this code path to be able to do a similar ChatMessage <-> Chat::Message dance and allow to check the type is valid. In the specific case of this commit, we were retrieving a `"ChatMessage"` bookmarkable_type from the DB and looking for it in the registered bookmarkable types which contain `Chat::Message` and not `ChatMessage`.
This commit main goal was to comply with Zeitwerk and properly rely on autoloading. To achieve this, most resources have been namespaced under the `Chat` module.
- Given all models are now namespaced with `Chat::` and would change the stored types in DB when using polymorphism or STI (single table inheritance), this commit uses various Rails methods to ensure proper class is loaded and the stored name in DB is unchanged, eg: `Chat::Message` model will be stored as `"ChatMessage"`, and `"ChatMessage"` will correctly load `Chat::Message` model.
- Jobs are now using constants only, eg: `Jobs::Chat::Foo` and should only be enqueued this way
Notes:
- This commit also used this opportunity to limit the number of registered css files in plugin.rb
- `discourse_dev` support has been removed within this commit and will be reintroduced later
<!-- 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. -->
Mentions are now displayed as using the non-cooked message which fixes
the problem. This is not ideal. I think we might want to rework how
these excerpts are created and rendered in the near future.
Co-authored-by: Jan Cernik <jancernik12@gmail.com>
Non-markdown tags weren't being escaped in chat excerpts. This could be
triggered by editing a chat message containing a tag (self XSS), or by
replying to a chat message with a tag (XSS).
Co-authored-by: Jan Cernik <jancernik12@gmail.com>
Usage:
```javascript
api.addChatDrawerStateCallback(({ isDrawerActive, isDrawerExpanded }) => {
// do something
});
```
Note this commit also uses this new API to add a css class (chat-drawer-active) on the body when the drawer is active.
This is just a little clean-up in tests. In the past, when creating a `chat_mention`
record, we always created a related notification. Starting from fa543cda
notifications and chat_mentions are fully decoupled from each other. So if we're
testing just chat mentions there is no need to fabricate notifications for them.
* DEV: Change sidebar header dropdown to use wait_for_animation
Introduced in 54351e1b8a, this
helper should remove the need to have to add the .animated
CSS class in JS for the sidebar.
* DEV: Revert spec change
- Will consider a message read only one the bottom of the message has been read
- Will allow to mark a message bigger than the view port as read
- Code should be more performant as the scroll is doing less (albeit more often)
- Gives us a very precise scroll state. Problem with throttling scroll is that you could end up never getting the even where scrollTop is at 0, opening a whole range of edge cases to handle
When we introduced `existingUploads` as an arg to the
ChatComposerUploads component, we also introduced a bug where
if multiple uploads were being done at once, and the draft
was saved, then because of didReceiveAttrs we would cancel
the currently uploading files because the draft uploads became
the existingUploads.
To work around this, since we do want to keep this on didReceiveAttrs
for cases when the user opens a draft or edits another message,
the easiest thing to do is to just not save uploads into the chat
draft if there are still uploads in progress. That way only when
all uploads are complete do we make them a part of the draft.
There is a small risk that the user could do something to lose
their uploads in the draft, but it's a better gamble to have
that happen rather than in progress uploads to be cancelled
while the user is waiting for them to be done because of the
draft.
Also changes the uploads system spec back to the old way of
attaching multiple files since that is why it was failing.
This is used when calling click_message_action_mobile to wait
for the message actions menu to finish animating up before
attempting to click on it using capybara. Without this, in
the time between capybara getting the x,y position of a menu
item to click on and the click being fired, the animating menu
can move that item out of the way.
With the new helper, we constantly compare x,y client rect positions
for the animating element and wait for them to stabilise. Once they
do, it means the animation is done, and it is safe to click on
anything within the element.
Re-enables mobile system specs for chat that were ignored because
of this.
Similar spirit to e195e6f614,
this moves the Bookmarkable registration to DiscoursePluginRegistry
so plugins which are not enabled do not register additional
bookmarkable classes.
Before this commit, we created a chat mention record only in case we wanted to send a notification about that mention to the user. Notifications were the only use case for the chat_mention db table. Now we want to use that table for other features, so we have to always create a chat_mention record.
- group writes when computing separators positions
- shows skeleton only on initial load
- forces date separator to be pinned when first message to prevent a pinned - not pinned - pinned sequence when loading more in past
- relies on `message.visible` property instead of checking `isElementInViewport`
- attempts to load next/prev messages earlier
- do not scroll to on fetch more
- hides `last visit` text while pinned
Note this test my prove to be flakey, so I might have to remove it or find a different solution. It's extremely complicated to test for something which shouldn't appear in a period of time and is not a present at T=0
This was possible due to specific events which are hard to represent in a test. The provided test is as close as possible to what was happening in production: a message bus event was played on a channel which has just loaded its state with the existing reaction.
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
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
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
This commit allows the user to set their preference vis-a-vis
the chat icon in the header of the page. There are three options:
- All New (default) - This maintains the existing behaviour where
all new messages in the channel show a blue dot on the icon
- Direct Messages and Mentions - Only show the green dot on the
icon when you are directly messaged or mentioned, the blue dot
is never shown
- Never - Never show any dot on the chat icon, for those who
want tractor-beam-laser-focus
Initially, the chat_mention db table was created to support notifications. So when creating
a `chat_mention` record we were always creating a related `notification` record. So did the
ChatMention fabricator.
Now we want to use the chat_mention db table in other scenarios. So we started decoupling
mentions from notification in 75b81b68.
This removes fabrication of Notifications from the ChatMention fabricator. We need to be able
to fabricate a ChatMention without a Notification.
This patch introduces a new `ServiceJob` class allowing the use of
`with_service` in jobs.
This way, it’s easier to use the chat service objects in jobs and
provides the same level of functionality than the one we have in
controllers.
Fixes issue introduced in 7ef482a292
where the correct warning message was not shown when enabling auto-join
for public categories when creating a channel. Adds more system specs
as well to avoid regressions.
* 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
Previous commit 479c0a3051 was done with the assumption that this info was defined on user serializer but it was actually defined on post serializer in core. This commit extends the user serializer for messages to add this data to the user.
Also correctly adds serializer test to ensure we actually have this data.
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>
Initially, the ChatMention model / db table was introduced to better support notifications (see discourse/discourse-chat@0801d10). That means that currently, we create a new chat_mention record only if a user will be notified about the mention.
Now we plan to start using the ChatMention model in other scenarios (for example for implementing user status on mentions) so we need to always create a new record in the chat_mention table. This PR does the first step into that direction by decoupling the logic for extracting and expanding mentions from the code related to notifications.
This doesn't change any behavior, only extracts code from ChatNotifier.
This commit fixes the UpdateUserLastRead spec which was checking
for a message ID that did not exist -- this could fail at times
since message ID 2 could exist. Better to create + destroy a message
since then it's guaranteed we have a unique ID.
This also attempts to clarify a step that we expect to fail which
succeeds instead by adding another emoji next to the success tick and
an explanation text.
Also removes some uses of unless in Services::Base, we generally prefer
to use alternatives, since unless can be hard to parse in a lot of
cases.
Co-authored-by: Loïc Guitaut <loic@discourse.org>
Followup to b94fa3b87a,
which broke the functionality to click on a message
checkbox, hold shift, then click another one, and have
the messages inbetween selected. Add system spec to
catch this.
A typo was preventing a click on channel title when in drawer mode to correctly open the channel info in full page.
This commit fixes the typo and adds a test.
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**
We’re now using `contract` as the first step and validations for
mandatory parameters have been added.
To simplify specs a bit, we only assert the service contract is run as
expected without testing each validation case. We’re now testing the
contract itself in isolation.
