Every replies creates a thread, even when threading is disabled. This is how we ensure we can go back and forth. However, a message bus event should only be published when threading is enabled, otherwise frontend will attempt to display a thread which is not possible when disabled.
This fixes a silent background 404 when doing a reply in a direct message channel or a non threading enabled category channel.
This commit also adds a component test for it and fixes a bug in `chat-channel-archive-status` `#getTopicURL` property which was incorrectly called as a function.
What is the problem?
Previously, this was the query used to move change messages into another
channel.
```
INSERT INTO chat_messages(
chat_channel_id, user_id, last_editor_id, message, cooked, cooked_version, created_at, updated_at
)
SELECT :destination_channel_id,
user_id,
last_editor_id,
message,
cooked,
cooked_version,
CLOCK_TIMESTAMP(),
CLOCK_TIMESTAMP()
FROM chat_messages
WHERE id IN (:message_ids)
RETURNING id
```
The problem is that this incorrectly assumes that the insertion will be based on the order of `message_ids`. However, that
is not the case as PostgreSQL provides no such guarantee. Instead we need to explicitly order the messages to ensure
the right order of insertion.
This problem was discovered by a flaky test which exposed the non-guarantee order of insertion.
A category's slug can be encoded when
`SiteSetting.slug_generation_method` has been set to "encoded". As a
result, we have to support non ASCII characters as well.
- `ChatChannel`
- `UserChatChannelMembership`
Also creates a new `chat-direct-message` model used as the object for the`chatable` property of the `ChatChannel` when the `ChatChannel` is a direct message channel. When the chatable is a category a real `Category` object will now be returned.
Archive state of a `ChatChannel` is now hold in a `ChatChannelArchive` object.
After a long time with no activity or hidden browser (2.5 minutes), the app will re-sync the chat user-tracking-state to ensure unreads are synced.
We might also need to couple this later with more recovering logic.
This commit adds support for excluding categories when using the
`category:` filter with the `-` prefix. For example,
`-category:category-slug` will exclude all topics that belong to the
category with slug "category-slug" and all of its sub-categories.
To only exclude a particular category and not all of its sub-categories,
the `-` prefix can be used with the `=` prefix. For example,
`-=category:category-slug` will only exclude topics that belong to the
category with slug "category-slug". Topics in the sub-categories of
"category-slug" will still be included.
Change mechanism handling `more` button for sidebar.
Before it was using HTML details tag.
To make tests more reliable, we are switching to use ember runloop.
What is the problem?
The TopicTrackingState is a service on the client side that is used to store
state of topics which is new or has unread posts for a given user. The state
is updated via various means and the one in concern here is whenever we load
a new topic list from the server. When a topic list is loaded from the server,
we sync this new topic list with the states in TopicTrackingState. There is also
a hard limit on the number of states that is stored by TopicTrackingState for
performance reasons and the limit is currently set to 4000. It was noticed that
once this limit has been reached, syncing a topic list with TopicTrackingState can
result in the registered state change callbacks to be called unnecessarily. This
is because during `TopicTrackingState#sync` we call `TopicTrackingState#removeTopic`
if the topic in question is neither new or unread to a user. However, `TopicTrackingState#removeTopic`
would call `TopicTrackingState#_afterStateChange` even if nothing was removed.
What is the fix?
This commit fixes the problem by checking that `TopicTrackingState#_afterStateChange` is only
called in `TopicTrackingState#removeTopic` when a topic is actually removed.
This amends it so our cached counting reliant specs run in synchronize mode
When running async there are situations where data is left over in the table
after a transactional test. This means that repeat runs of the test suite
fail.
- Ensure changing timezones are reflected immediately in the date-time-input (the computed property was missing a dependent key)
- Ensure date-input doesn't lose timezone information (calling `toDate()` causes moment timestamps to lose timezone information)
This was created to resolve issues in the discourse-calendar plugin (https://github.com/discourse/discourse-calendar/pull/399)
When we were deleting messages in chat, we would find all of
the UserChatChannelMembership records that had a matching
last_read_message_id and set that column to NULL.
This became an issue when multiple users had that deleted message
set to their last_read_message_id. When we called ChannelUnreadsQuery
to get the unread count for each of the user's channels, we were
COALESCing the last_read_message_id and returning 0 if it was NULL,
which meant that the unread count for the channel would be the total
count of the messages not sent by the user in that channel.
This was particularly noticeable for DM channels since we show
the count with the indicator in the header. This issue would disappear
as soon as the user opened the problem channel, because we would then
set the last_read_message_id to an actual ID.
To circumvent this, instead of NULLifying the last_read_message_id in
most cases, it makes more sense to just set it to the most recent
non-deleted chat message ID for the channel. The only time it will
be set to NULL now is when there are no more other messages in the
channel.
We need to create and update `chat_mentions` records for messages earlier. They should be created or updated before we call `Chat::Publisher.publish_new!` `Chat::Publisher.publish_edit!` to send the message to message bus subscribers).
This logic is covered with tests in `message_creator_spec.rb`, `message_updater_spec.rb`, `notifier_spec.rb` and `notify_mentioned_spec.rb`.
See the commits history for steps of refactoring.
Since our recent change of inverting thread scrolling direction it feels more responsive to scroll down in thread panel as soon as message is staged and not after it's actually persisted.
When hovering a thread indicator in a channel we will now append two `<link rel="preload" ...>` to the `<head>` of the document. Clicking on it should be significantly faster.
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit implements all the necessary logic to create thread seamlessly. For this it relies on the same logic used for messages and generates a `staged-id`(using the format: `staged-thread-CHANNEL_ID-MESSAGE_ID` which is used to re-conciliate state client sides once the thread has been persisted on the backend.
Part of this change the client side is now always using real thread and channel objects instead of sometimes relying on a flat `threadId` or `channelId`.
This PR also brings three UX changes:
- thread starts from top
- number of buttons on message actions is dependent of the width of the enclosing container
- <kbd>shift + ArrowUp</kbd> will reply to the last message
See e323628d8a for more details.
This commit speeds up the tests by roughly 10 seconds locally where the
default wait time is 2 seconds. On CI, this speeds up the tests by 20
seconds where the default wait time is 4 seconds.
What is the problem?
We are relying on RSpec custom matchers in system tests by defining
predicates in page objects. The problem is that this can result in a
system test unnecessarily waiting up till the full duration of
Capybara's default wait time when the RSpec custom matcher is used with
`not_to`. Considering this topic page object where we have a `has_post?`
predicate defined.
```
class Topic < PageObject
def has_post?
has_css?('something')
end
end
```
The assertion `expect(Topic.new).not_to have_post` will end up waiting
the full Capybara's default wait time since the RSpec custom matcher is
calling Capybara's `has_css?` method which will wait until the selector
appear. If the selector has already disappeared by the time the
assertion is called, we end up waiting for something that will never
exists.
This commit fixes such cases by introducing new predicates that uses
the `has_no_*` versions of Capybara's node matchers.
For future reference, `to have_css` and `not_to have_css` is safe to sue
because the RSpec matcher defined by Capbyara is smart enough to call
`has_css?` or `has_no_css?` based on the expectation of the assertion.
* 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.
