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.
Introduces the concept of image thumbnails in chat, prior to this we uploaded and used full size chat images within channels and direct messages.
The following changes are covered:
- Post processing of image uploads to create the thumbnail within Chat::MessageProcessor
- Extract responsive image ratios into CookedProcessorMixin (used for creating upload variations)
- Add thumbnail to upload serializer from plugin.rb
- Convert chat upload template to glimmer component using .gjs format
- Use thumbnail image within chat upload component (stores full size img in orig-src data attribute)
- Old uploads which don't have thumbnails will fallback to full size images in channels/DMs
- Update Magnific lightbox to use full size image when clicked
- Update Glimmer lightbox to use full size image (enables zooming for chat images)
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.
This consistently fails on core now, see
https://github.com/discourse/discourse/actions/runs/7109919490/job/19355591619?pr=24738
Error: QUnit Test Failure: Browser Id 2 - Discourse Chat | Component | chat message collapser images: escapes link
not ok 444 Chrome 120.0 - [58 ms] - Browser Id 2 - Discourse Chat | Component | chat message collapser images: escapes link
---
actual: >
false
expected: >
true
stack: >
Expected value is %3Cscript%3Esomeeviltitle%3C/script%3E and actual value is
<script>someeviltitle</script>
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`.
We're seeing some deprecation warnings in production. This is because we're passing a raw Ruby timestamp, which gets stringified implicitly when written to Redis. As per #15842, this conversion needs to be done explicitly.
This outlet allows to redefine the button displayed when asking the user to join a channel.
The following outletArgs are sent to the outlet:
```
onJoinChannel
channel
icon
title
label
disabled
```
Prior to this fix direct message would always show "Chat in ..." when hovering the channel even if you were hovering a direct message channel with another user (or yourself).
We will now correctly show:
- `Chat in ...` for group channels
- `Chat with ...` for direct message channels
Previously the spec could be flakey as the long message could show on the screen while we await for processing. Now we will first check to have the error message on screen, at this point the erroneous message should never be visible.
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 PR introduces thread support for channel archives. Now, threaded messages are rendered inside a `details` HTML tag in posts.
The transcript markdown rules now support two new attributes: `threadId` and `threadTitle`.
- If `threadId` is present, all nested `chat` tags are rendered inside the first one.
- `threadTitle` (optional) defines the summary content.
```
[chat threadId=19 ... ]
thread OM
[chat ... ]
thread reply
[/chat]
[/chat]
```
If threads are split across multiple posts when archiving, the range of messages in each part will be displayed alongside the thread title. For example: `(message 1 to 16 of 20)` and `(message 17 to 20 of 20)`.
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.
Before this fix we would reset the input two times:
- right before sending message
- and after it's been sent
The second one is actually not necessary, and more over with the server delay the user could have started typing a new message and that would clear it.
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
Fixes a minor issue where "Invalid date" is shown briefly
when sending a message in a chat thread. Change to use
`new Date()` instead like the channel staged message which
does not have this issue.
This PR refactors the following:
* leaving all the CSS applied to the old `modal-body` classes in their respective files
* made new clean styling for `.d-modal` and refactored the template to use the new BEM classes
* `inner-`, `middle-`, `outer-` container classes are gone and replaced with simplified `wrapper` and `container` classes
* use standardised max-sizes with modifiers `-large` and `-max`
* lighter backdrop,
* min-width to prevent puny modals
* other styling changes regarding padding, close button,…
* pulled out all modal overrides into a general `modal-overrides` file + cleanup of outdated CSS
* pulled out login and create account modal styling into their own file, cause it's such a big override
* removed old general login.scss file for mobile & desktop
* only kept some remainders I don't want to touch in `app/assets/stylesheets/common/base/login.scss`
Some of these files are quite small, and if we rename them in the same
commit where we inlined the template, Git may choose to see them as
different files. This commit forces Git to recognize the rename, which
will preserve the lineage of the refactored files.
Follow-up to #24278 that is slightly less trivial.
* Some were "trivial" usages that were missed in the previous PR because the same file that had at least one other non-trivial usage.
* These involve extra arguments or inheritance but I have checked that they seem correct.
- Remove vendored copy
- Update Rails implementation to look for language definitions in node_modules
- Use webpack-based dynamic import for hljs core
- Use browser-native dynamic import for site-specific language bundle (and fallback to webpack-based dynamic import in tests)
- Simplify markdown implementation to allow all languages into the `lang-{blah}` className
- Now that all languages are passed through, resolve aliases at runtime to avoid the need for the pre-built `highlightjs-aliases` index
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
Fixes this problem that happens sometimes in specs:
> Mocha::StubbingError:
> #<Mock:0x135150> was instantiated in one test but it is receiving
invocations within another test. This can lead to unintended
interactions between tests and hence unexpected test failures. Ensure
that every test correctly cleans up any state that it introduces.
The most common thing that we do with fab! is:
fab!(:thing) { Fabricate(:thing) }
This commit adds a shorthand for this which is just simply:
fab!(:thing)
i.e. If you omit the block, then, by default, you'll get a `Fabricate`d object using the fabricator of the same name.
This adds the ability to collect stats without exposing them
among other stats via API.
The most important thing I wanted to achieve is to provide
an API where stats are not exposed by default, and a developer
has to explicitly specify that they should be
exposed (`expose_via_api: true`). Implementing an opposite
solution would be simpler, but that's less safe in terms of
potential security issues.
When working on this, I had to refactor the current solution.
I would go even further with the refactoring, but the next steps
seem to be going too far in changing the solution we have,
and that would also take more time. Two things that can be
improved in the future:
1. Data structures for holding stats can be further improved
2. Core stats are hard-coded in the About template (it's hard
to fix it without correcting data structures first, see point 1):
63a0700d45/app/views/about/index.html.erb (L61-L101)
The most significant refactorings are:
1. Introducing the `Stat` model
2. Aligning the way the core and the plugin stats' are registered
There is an edge case where the following occurs:
1. The user sets a bookmark reminder on a post/topic
2. The post/topic is changed to a PM before or after the reminder
fires, and the notification remains unread by the user
3. The user opens their bookmark reminder notification list
and they can still see the notification even though they cannot
access the topic anymore
There is a very low chance for information leaking here, since
the only thing that could be exposed is the topic title if it
changes to something sensitive.
This commit filters the bookmark unread notifications by using
the bookmarkable can_see? methods and also prevents sending
reminder notifications for bookmarks the user can no longer see.
When quoting a chat message in a post, if that message contains a mention,
that mention should be ignored. But we've been detecting them and sending
notifications to users. This PR fixes the problem. Since this fix is for
the chat plugin, I had to introduce a new API for plugins:
# We strip posts before detecting mentions, oneboxes, attachments etc.
# We strip those elements that shouldn't be detected. For example,
# a mention inside a quote should be ignored, so we strip it off.
# Using this API plugins can register their own post strippers.
def register_post_stripper(&block)
end
Subscriptions manager have been a pain since the beginning, one of the problem is that thread and channels behave mostly the same but with various small difference which I expect to increase over time.
Trying to use subclasses for this case has proven to be a mistake, this commit now uses a class for each case (channel, thread) which for now contains a lot of duplication, which might be reduced in the future but has the merit to make reasoning about each case very simple.
This refactor is fixing a bug introduced in 90efdd7f9d which was causing the wrong channel to be unsubscribed, this shouldn't be possible anymore. We had tests for this which were disabled due to flakeyness, I will consider re-enabling them in the future.
Other notes:
- notices had been added to the subscriptions manager service, they have been moved into their own dedicated service: `ChatChannelNoticesManager`
- the `(each model)` trick used in `<ChatChannel />` since 90efdd7f9d to ensure atomicity has been applied to `<ChatThread />` too
Chat redesign work to improve chat navigation:
- New header title with channel name (thread list on mobile)
- New header title without channel name (thread list on full page chat)
- Removes the close button on threads (mobile only)
- Updates to back button route within thread (mobile), taking user to:
- The thread index, if they accessed the thread from the thread index.
- The channel itself, if they accessed the thread directly from the channel.
- The channel itself, if they accessed the thread from a notification.
- Show thread title in chat drawer header
- Properly convert emoji in thread titles in chat header (all devices)
- Upgrades various templates to use gjs format.
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
When uploading images, they are assigned a dominant color which gets used in various places, such as Discourse Hub and the new lightbox. Previously in chat we didn't assign this attribute, so it was defaulting to a null value. We did however use it as an inline CSS style for the image background (which is visible while the image is downloaded).
This change adds data-dominant-color to the uploaded image in chat and uses it correctly within lightbox.
This is a follow-up to e6299a3. I additionally fixed these three things:
1. Since e6299a3 there's no need anymore to join the group_users table
when looking for users who were reached by a group mention, so
I removed that join in that commit. But turned out we were joining
the group_users table twice, so I removed the second join in this PR.
That drastically speeded up my test query, from 6 sec to 0.26 sec.
2. We also were joining twice the user_chat_channel_memebership table,
so I removed the second unnecessary join too.
3. We actually need to join the user_chat_channel_memebership table
only in certain cases, and we don't need to do that for group mentions,
so I fixed that too.
As a result of these changes, time of my test query fall down from
6 sec to 0.001 sec. And the resulting SQL query now contains only
one JOIN statement.
A follow-up to faac6773. This PR eliminates one more heavy join by forcing
Active Record to do two queries instead.
Also, along the way, I made this change:
```
# this generates two quries to the groups table
def groups_to_mention
@groups_to_mention = mentionable_groups - groups_with_too_many_members
end
# so I changed it to (this makes only one query to the groups table):
def groups_to_mention
@groups_to_mention ||= mentionable_groups.where("user_count <= ?", SiteSetting.max_users_notified_per_group_mention)
end
```
This one is kind of a premature optimization, because we don't have evidence that
this extra query is a problem, but it seems cleaner this way.
Commits history on this PR may help better understand the change.
Add new chat indicator preference within chat user preferences.
Enabling this option will mean that green notifications will only appear for mentions (within channels and DMs.
This change also enables mentions within direct messages.
- correctly respects min-width/height defined in css
- removes fixed width/height when resizing window
- reduces the min width of the side panel from 250px to 150px
