Commit Graph

267 Commits

Author SHA1 Message Date
Joffrey JAFFEUX 64971e5972
DEV: attempts to fix flakey spec (#21547)
The failure screenshot shows the message is on screen while the error is:

```
     Failure/Error: example.run
       expected to find text "My favorite message" in "Community\nEverything\nMy Posts\nMore\nMessages\nInbox\nChannels\nPolitics 1\nPersonal chat\nPolitics 1". (However, it was found 1 time including non-visible text.)
```

I expect the arrow element might e slightly hiding the link, but not 100% sure of this.
2023-05-15 07:52:02 +02:00
Daniel Waterworth 53d99d1a26
DEV: Skip flaky tests (#21534) 2023-05-15 07:51:25 +02:00
Andrei Prigorshnev b85d057df4
FIX: ensures `all_mentioned_user_ids` is not used as identifier (#21491)
A follow-up to 54b2a85b. That commit didn't fix the issue because the to_notify hash that we return from the notify_edit method isn't used anywhere apart from tests (that's confusing, we're going to fix that soon).
2023-05-12 17:47:48 +04:00
Joffrey JAFFEUX e10b262eb9
DEV: fix flakey spec (#21515)
Similar fix to the one made in aab6fb13a0

Instead of checking last object, check against all modified objects in no specific order.
2023-05-11 23:27:26 +02:00
Joffrey JAFFEUX f20be4b092
FIX: prevents subscribing with an old id (#21509)
This issue was for example possibly causing the last visit indicator to be reset by `sent` messages events.

The following was happening:
- a user (bob) had a last message bus ID of 1 on a channel (id:1) subscription
- bob then go to another channel (id:2), unsubscribing from updates of channel (id:1)
- another user (laura) then send messages to channel (id:1)
- bob goes back to channel (id:1)

At this point we we doing in the same sequence:
- loading channel with messages, getting a new last message bus id
- subscribing to updates using the last known message bus id

Most of the times we were lucky enough for this to work (no events while away, or just got the new id in time...) but it was also very likely to do a double fetch of messages as MessageBus would think we were late.
2023-05-11 22:27:48 +02:00
Andrei Prigorshnev f4fde4e49b
DEV: When deleting a chat message, do not delete mention records (#21486)
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).
2023-05-11 20:05:59 +04:00
Joffrey JAFFEUX 92bb845db2
FIX: messages selection with shift + click (#21506)
This commit fixes the shift+click multi selection in threads. We were not correctly using the manager of the message and would attempt to find messages in the channel instead of the thread.

The `activeThread` was also not correctly set sometimes.

Also adds tests for message selection in threads.
2023-05-11 17:52:53 +02:00
Andrei Prigorshnev 2703f2311a
DEV: Create a chat_mention record when self mentioning (#21438)
In the past, we create a `chat_mention` records only when we wanted to notify a user about a mention. Since we don't send notifications when a user mentioning himself, we didn't create a `chat_mention` records in those cases.

Now we use `chat_mentions` records in other scenarios too, so when a user is mentioning himself we want to:
1. Create a `chat_mention` record for that mention
2. Do not create a notification for that mention
2023-05-11 19:30:26 +04:00
Martin Brennan 26f9ccd8bb
FEATURE: Create and update thread memberships (#21501)
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
2023-05-11 14:35:26 +02:00
Joffrey JAFFEUX aab6fb13a0
DEV: fix flakey spec in handle_category_udpated (#21488) 2023-05-11 08:01:02 +02:00
Martin Brennan cc2570fce3
DEV: Create UserChatThreadMembership table and model (#21481)
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>
2023-05-10 17:19:48 +02:00
Martin Brennan 91c5658e9b
FIX: Handle deleted original message for thread index (#21470)
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.
2023-05-10 13:58:15 +02:00
Jan Cernik cbbaeb55b5
FIX: Don't autojoin users when they have ready-only permissions (#20213)
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>
2023-05-10 08:45:13 -03:00
Joffrey JAFFEUX c6b43ce68b
FEATURE: Thread list initial UI (#21412)
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.
2023-05-10 11:42:32 +02:00
Joffrey JAFFEUX 7a84fc3d9d
DEV: more resilient scroll to bottom spec (#21467)
The spec now checks we are in the state we expect to be before clicking bottom button. The bottom button could show while it's still loading and on slow systems cause failures.
2023-05-10 10:54:02 +02:00
Joffrey JAFFEUX dacb605e98
DEV: more resilient upload spec (#21462)
Checking "uploading" string is tricky because it both takes time before showing, and when it will show it will show for a short period of time. I prefer to reduce the surface tested here while still getting some confidence out of it and making the test more reliable.
2023-05-10 00:33:40 +02:00
Joffrey JAFFEUX 54b2a85b27
FIX: ensures `all_mentioned_user_ids` is not used as identifier (#21452)
When making the list of users to notify we set `all_mentioned_user_ids` key on the `to_notify` Hash.

This hash will be passed around until the actual moment where we send the notifications:

```ruby
identifier_text =
  case identifier_type
  when :here_mentions
    "@here"
  when :global_mentions
    "@all"
  when :direct_mentions
    ""
  else
    "@#{identifier_type}"
  end
```

As not found `all_mentioned_user_ids` would end up being sent as `@all_mentioned_user_ids` which is obviously incorrect.

This commit is a direct fix to the issue and will remove the key as soon as we have used it sooner up in the chain.

This bug was reproducible when doing this sequence of events:

- create a message with a direct mention: `@bob hi`
- edit this message into a global mention `@all hi`
2023-05-09 13:00:19 +02:00
Joffrey JAFFEUX 8f27913ec1
FIX: no event when threading is disabled (#21439)
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.
2023-05-09 10:11:29 +02:00
Joffrey JAFFEUX 24cc3ac7dc
DEV: more resilient spec (#21436)
Allows more time for the members to be appended in tests
2023-05-08 21:10:57 +02:00
Andrei Prigorshnev c45fff9405
DEV: group mentions related specs into context (#21434) 2023-05-08 21:18:11 +04:00
Joffrey JAFFEUX 22521d3428
DEV: converts models to native classes (#21418)
- `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.
2023-05-08 18:24:41 +02:00
Joffrey JAFFEUX 5c9efea480
FIX: correct link on original message (#21415)
Ensures that using copy link on the the original message of a thread in a channel, copies the link to the channel and not the thread.
2023-05-08 09:10:10 +02:00
Joffrey JAFFEUX cb5e5f3e5b
UX: groups deleted messages (#21411)
Any continuous series of deleted messages will now be grouped into one single expand button.
2023-05-05 17:08:33 +02:00
Martin Brennan ae3231e140
FIX: Incorrect unread count shown in channel when message deleted (#21410)
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.
2023-05-05 15:28:48 +02:00
Andrei Prigorshnev 35a414bb38
DEV: Create and update chat message mentions earlier (#21388)
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.
2023-05-05 15:47:07 +04:00
Joffrey JAFFEUX e88b997153
Revert "UX: preloads a thread when hovering thread indicator (#21406)" (#21407)
This reverts commit 028dba144d.
2023-05-05 12:06:30 +02:00
Joffrey JAFFEUX 028dba144d
UX: preloads a thread when hovering thread indicator (#21406)
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>
2023-05-05 11:18:03 +02:00
Joffrey JAFFEUX 187b59d376
UX: implements draft threads (#21361)
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
2023-05-05 08:55:55 +02:00
Alan Guo Xiang Tan 7ff8e5580f
DEV: Speed up chat plugin system tests (#21399)
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.
2023-05-05 08:16:23 +08:00
Martin Brennan b2a727336e
FIX: Thread mention read state and notification links (#21385)
* 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.
2023-05-04 17:28:51 +02:00
Joffrey JAFFEUX ed077cf3f1
DEV: after has been proven problematic in other tests (#21358)
It seems more reliable to revert state at the end of the it block. In another PR I noticed that the network state was leaking in other tests when I was reverting in the after block.

Also trashes a suspicious spec.
2023-05-03 11:03:38 +02:00
Joffrey JAFFEUX 616f4a1118
FIX: disables send btn while uploads in progress (#21298)
Before this fix you could press send while upload was in progress and lose it as it was not yet uploaded.
2023-05-02 18:11:40 +02:00
Joffrey JAFFEUX 69a72015f0
UX: resets active message when scrolling (#21305)
This will avoid the messages actions floating around while scrolling. Note it's not testing the thread counterpart yet as I have a plan in mind to tests channels and threads in a clean way in the near future.
2023-05-02 14:21:30 +02:00
Joffrey JAFFEUX 6c90747dea
FIX: correctly use types for reviewables type (#21333)
Before this fix if the underlying model of a reviewable was changed, the filter wouldn't work anymore as it was expecting a 1:1 relation between filter type and model name.

This commit also relies on the `Reviewable.types` array to check against valid types instead of a regex not checking much.

Finally this commit adds a spec to ensure chat reviewables are listable from the review index page.
2023-05-02 14:21:14 +02:00
Joffrey JAFFEUX 0239e88809
UX: cook message before sending edit (#21303)
This allows for instant feedback in the UI.
2023-04-28 20:11:57 +02:00
chapoi f29f131387
UX: composer use BEM + small tweaks to enabled/disabled styling (#21246)
- using BEM notation
- making animation linear instead of default ease
- small tweaks to composer state (disabled/send-disabled/send-enabled)
- fixing bug with disabled composer on mobile
2023-04-28 10:24:49 +02:00
Joffrey JAFFEUX e5ec0b84a9
DEV: fix a spec and skips another one (#21277)
- It seems that `window_opened_by/within_window` it not reliable in our current setup/test
- System specs should avoid at all cost to rely on backend state, any change should be visible one way or another on the front to be properly tested
2023-04-27 09:43:26 +02:00
Joffrey JAFFEUX 7f803a0335
FIX: ensures thread is cleared when closing it (#21264) 2023-04-26 20:37:58 +02:00
Joffrey JAFFEUX 6487c8ef24
FIX: properly respects chat_minimum_message_length (#21256)
Before this fix we were only considering `>` and not `>=`, this also adds two tests.
2023-04-26 17:35:35 +02:00
Joffrey JAFFEUX 731282c2ec
FIX: attempts to make cooking less order dependent (#21253)
It's very hard to repro but under specific circumstances I suspect it was possible for this sequence to happen:

- set message TEXT
- cooking starts
- set message COOKED through another mean (like a message bus)
- the cooking started sooner finished and erases the cooked set at the step before causing the message to have the incorrect cooked
2023-04-26 16:50:38 +02:00
Joffrey JAFFEUX e495a2fc3f
DEV: Enable parallel system specs in GitHub actions CI (#21251)
Also skips/improves few flakey specs
2023-04-26 13:02:19 +02:00
Joffrey JAFFEUX 427fa36edd
FIX: generates markdown from pasting link (#21241)
After removing `TextareaTextManipulation` from `ChatComposer` and using `TextareaInteractor` as a proxy, one function has been forgotten: `paste(event)` which is not available in glimmer components anymore, and even less avaiable now that the mixin is not tied to a component anymore but a real DOM node. As a solution we now add a manual paste event listener which will call `paste(event)`.
2023-04-26 10:05:48 +02:00
Joffrey JAFFEUX 6ea4854c51
UX: displays channel title in document title (#21242) 2023-04-26 09:09:07 +02:00
Joffrey JAFFEUX bf886662df
UX: improves composer and thread panel (#21210)
This pull request is a full overhaul of the chat-composer and contains various improvements to the thread panel. They have been grouped in the same PR as lots of improvements/fixes to the thread panel needed an improved composer. This is meant as a first step.

### New features included in this PR

- A resizable side panel
- A clear dropzone area for uploads
- A simplified design for image uploads, this is only a first step towards more redesign of this area in the future

### Notable fixes in this PR

- Correct placeholder in thread panel
- Allows to edit the last message of a thread with arrow up
- Correctly focus composer when replying to a message
- The reply indicator is added instantly in the channel when starting a thread
- Prevents a large variety of bug where the composer could bug and prevent sending message or would clear your input while it has content

### Technical notes

To achieve this PR, three important changes have been made:

- `<ChatComposer>` has been fully rewritten and is now a glimmer component
- The chat composer now takes a `ChatMessage` as input which can directly be used in other operations, it simplifies a lot of logic as we are always working a with a `ChatMessage`
- `TextareaInteractor` has been created to wrap the existing `TextareaTextManipulation` mixin, it will make future migrations easier and allow us to have a less polluted `<ChatComposer>`

Note ".chat-live-pane" has been renamed ".chat-channel"

Design for upload dropzone is from @chapoi
2023-04-25 10:23:03 +02:00
Martin Brennan 896707c7ea
FIX: Do not delete old chat messages if chat disabled (#21214)
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.
2023-04-24 14:42:50 +10:00
Martin Brennan 24ec06ff85
FEATURE: Reintroduce better thread reply counter cache (#21197)
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.
2023-04-24 09:32:04 +10:00
Martin Brennan 21f93731a3
DEV: Move channel creation for category into service (#21167)
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>
2023-04-24 09:15:16 +10:00
Andrei Prigorshnev e7292e1682
DEV: more tests for mentions when updating chat messages (#21185)
This adds these two new test cases:

    context "when updating a mentioned user" do
      it "updates the mention record" do

    # and
    context "when there are duplicate mentions" do
        it "creates a single mention record per mention" do

Apart from that, this groups mention related tests into a context, renames one test, and moves setup of another test into the test case itself from the before block (to make it more clear, that test is the only one that uses that setup). See the PR's commit history.
2023-04-21 16:58:01 +04:00
Daniel Waterworth 38cebd3ed5
Revert "FEATURE: Better thread reply counter cache (#21108)" (#21192)
This reverts commit 180e3e11d1.

Per internal discussions, this is a temporary revert, to investigate if this is causing a performance regression.
2023-04-20 15:09:47 -05:00
Andrei Prigorshnev 8b438767e5
FIX: send notifications after a chat message was updated with new mentions (#21173)
Steps to reproduce the bug:
1. Send a chat message
2. Edit the message and add a mention to it
3. The mentioned user won't receive a notification

This PR fixes the problem.

Also:
1. There's no need anymore to have a code for removing notifications in the `notify_edit` method, because a call to `@chat_message.update_mentions` in the first line of the `notify_edit` method does that job:
    ff56f403a2/plugins/chat/lib/chat/notifier.rb (L90)

2. There's no need to load mention records from database, it's enough to pluck user ids
2023-04-20 19:05:17 +04:00