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.
* UX: handle long userstatus in menupanel
* UX: remove margin on userstatus emoji
* UX: change emoji sise of user status in DM creator
* FIX: user status overflow on chat index
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.
Before that change, footer of the sidebar was not visible.
Footer is very important, especially now, when add custom section button is located there.
Also, distance between chat input and keyboard were increased
This commit changes the ChatThreadsManager into a native
class instead of an ember service, and initializes it
for every ChatChannel model. This way each channel has its
own thread manager and cache that we can load/unload as
needed, and we also move activeThread to the channel since
it makes more sense to keep it there, not inside the chat service.
The pattern of calling setOwner with the passed in owner
from ChatChannel is adapted from the latest ember docs,
and is needed to avoid the error below when calling services
from the native class:
> Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container
It works well _only_ if we use our own getOwner wrapper
from addon/lib/get-owner, which is for backwards compat.
c.f. https://guides.emberjs.com/release/in-depth-topics/native-classes-in-depth/
We were calling the job with a symbol as one of the values:
```ruby
Jobs.enqueue(
:send_message_notifications,
chat_message_id: 1,
timestamp: Time.now.iso8601(6),
reason: :new,
)
```
Which is a bad pattern as when the job serialisation will happen, `:new` will become `"new"` and you have to deal with a string in your job and not a symbol, which can be confusing and lead to bugs.
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.
Follow up to 82b4a53d29
On mobile, we just need to add `min-width: 0` to
`chat-live-pane` so it will not overflow the grid
defined in `main-chat-outlet.chat-view`.
The overflow could be triggered by:
1. Replying on mobile to a really long chat message
2. Uploading > 2 files
Both of these situations are fixed.
This css was causing the view on mobile to take more space than the available width. This was particularly visible with uploads due to a bug preventing the overflow, this is also fixed.
This commit is expanding on previous work making everything chat working through an URL.
Improves drawer templates to be all URLs
Implements some kind of router for the drawer
Removes few remaining actions for opening channels
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.
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>'
```
This change will ensure we enter and subscribe to presence channels on start and will use the correct "change" events from presence channel to update state.
This change only makes the model reflect correctly what's
already happening in the database. Note that there are no calls
to chat_message.chat_mention in Core and plugins so this
change should be safe.
Also note, that at the moment we use the chat_mentions db
table only to support notifications about mentions, but
we're going to start using it for other cases. This commit is
the first step in that direction.
Whenever we create a chat message that is `in_reply_to` another
message, we want to lazily populate the thread record for the
message chain.
If there is no thread yet for the root message in the reply chain,
we create a new thread with the appropriate details, and use that
thread ID for every message in the chain that does not yet have
a thread ID.
* Root message (ID 1) - no thread ID
* Message (ID 2, in_reply_to 1) - no thread ID
* When I as a user create a message in reply to ID 2, we create a thread and apply it to ID 1, ID 2, and the new message
If there is a thread for the root message in the reply chain, we
do not create one, and use the thread ID for the newly created chat
message.
* Root message (ID 1) - thread ID 700
* Message (ID 2, in_reply_to 1) - thread ID 700
* When I as a user create a message in reply to ID 2, we use the existing thread ID 700 for the new message
We also support passing in the `thread_id` to `ChatMessageCreator`,
which will be used when replying to a message that is already part of
a thread, and we validate whether that `thread_id` is okay in the context
of the channel and also the reply chain.
This work is always done, regardless of channel `thread_enabled` settings
or the `enable_experimental_chat_threaded_discussions` site setting.
This commit does not include a large data migration to backfill threads for
all existing reply chains, its unnecessary to do this so early in the project,
we can do this later if necessary.
This commit also includes thread considerations in the `MessageMover` class:
* If the original message and N other messages of a thread is moved,
the remaining messages in the thread have a new thread created in
the old channel and are moved to it.
* The reply chain is not preserved for moved messages, so new threads are
not created in the destination channel.
In addition to this, I added a fix to also clear the `in_reply_to_id` of messages
in the old channel which are moved out of that channel for data cleanliness.
Whenever we create a chat message that is `in_reply_to` another
message, we want to lazily populate the thread record for the
message chain.
If there is no thread yet for the root message in the reply chain,
we create a new thread with the appropriate details, and use that
thread ID for every message in the chain that does not yet have
a thread ID.
* Root message (ID 1) - no thread ID
* Message (ID 2, in_reply_to 1) - no thread ID
* When I as a user create a message in reply to ID 2, we create a thread and apply it to ID 1, ID 2, and the new message
If there is a thread for the root message in the reply chain, we
do not create one, and use the thread ID for the newly created chat
message.
* Root message (ID 1) - thread ID 700
* Message (ID 2, in_reply_to 1) - thread ID 700
* When I as a user create a message in reply to ID 2, we use the existing thread ID 700 for the new message
We also support passing in the `thread_id` to `ChatMessageCreator`,
which will be used when replying to a message that is already part of
a thread, and we validate whether that `thread_id` is okay in the context
of the channel and also the reply chain.
This work is always done, regardless of channel `thread_enabled` settings
or the `enable_experimental_chat_threaded_discussions` site setting.
This commit does not include a large data migration to backfill threads for
all existing reply chains, its unnecessary to do this so early in the project,
we can do this later if necessary.
This commit also includes thread considerations in the `MessageMover` class:
* If the original message and N other messages of a thread is moved,
the remaining messages in the thread have a new thread created in
the old channel and are moved to it.
* The reply chain is not preserved for moved messages, so new threads are
not created in the destination channel.
In addition to this, I added a fix to also clear the `in_reply_to_id` of messages
in the old channel which are moved out of that channel for data cleanliness.
UI is not modified much besides removing the border-bottom, and using only message body.
However instead of having a fix template, this is all automatically generated and random, resulting in a more natural experience.
Only the header's height and 15px spacing are removed from the height of the viewport.
Previously it was limited to 90vh and there was also a useless property on a child node limiting it to 85vh. We now use only one property.
Public channels were previously sorted by name, however, channels with a leading emoji in the name would always appear first in the list. By using slug we avoid this issue.
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.
First follow-up to the feature introduced in #19034. We don't want to pollute main components like `chat-live-pane`, so we'll use a service to track and manage the state needed to display before-send warnings while composing a chat message.
We also move from acceptance tests to system specs.
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 implements a requested feature: resizing the chat drawer.
The user can now adjust the drawer size to their liking, and the new size will be stored in localstorage so that it persists across refreshes. In addition to this feature, a bug was fixed where the --composer-right margin was not being correctly computed. This bug could have resulted in incorrectly positioned drawer when the composer was expanded.
Note that it includes support for RTL.
* FIX: Emoji autocomplete “more” button not working
* Rely on setting an intial value on the filter input
This commit removes custom logic applied on initial filter and instead gives a param to use as value for the input, automatically triggering the existing filtering handler.
Other notes:
- Slightly changes the API to be able to set a filter and open the composer in one go
- Adds a very simple service spec
- Adds a system spec to fully validate the behavior
---------
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This change was made to prevent composer input to be reset randomly during tests but this is causing more issues for now: most notably composer state not being reset when changing channel. This will need a better solution.
* DEV: Rnemae channel path to just c
Also swap the channel id and channel slug params to be consistent with core.
* linting
* channel_path
* Drop slugify helper and channel route without slug
* Request slug and route models through the channel model if possible
* DEV: Pass messageId as a dynamic segment instead of a query param
* Ensure change is backwards-compatible
* drop query param from oneboxes
* Correctly extract channelId from routes
* Better route organization using siblings for regular and near-message
* Ensures sessions are unique even when using parallelism
* prevents didReceiveAttrs to clear input mid test
* we disable animations in capybara so sometimes the message was barely showing
* adds wait
* ensures finished loading
* is it causing more harm than good?
* this check is slowing things for no reason
* actually target the button
* more resilient select chat message
* apply similar fix to bookmark
* fix
---------
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This new table will be used to automatically group replies
for messages into one place. In future additional functionality
will be built around the thread, like pinning messages, changing
the title, etc., the columns are just the main ones needed at first.
The columns are not prefixed with `chat_*` e.g. `chat_channel` since
this is redundant and just adds duplication everywhere, we want to
move away from this generally within chat.
Adds hidden `enable_experimental_chat_threaded_discussions`
setting which will control whether threads show in the UI,
alongside the `ChatChannel.threading_enabled` boolean column,
which does the same. The former is a global switch for this
feature, while the latter can be used to allow single channels
to show this new functionality if the site setting is true.
Neither setting impacts whether `ChatThread` records (which will
be added in a future PR) will be created, they will always be
made regardless.
`last_message_sent_at` could be equal and as a result the order would be random causing random spec failures in plugins/chat/spec/mailers/user_notifications_spec.rb:182
Using a shared channel with per-message permissions means that every client is updated with the channel's 'last_id', even if there are no messages available to them. Per-user channel names avoid this problem - the last_id will only be incremented when there is a message for the given user.
* FIX: correct various mistakes in chat-notification-manager
- The code was still handling global chat through its own variable instead of relying on `ChatStateManager`
- There was a typo were the code was using `this` instead of `opts`
Note notifications are a future big work of this year and this should be heavily reworked and tested.
* linting
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.
When expanding the "Advanced mode" of the date modal, the Timezones
picker was at the bottom of the modal, and expanding it would often
overflow the viewport. This PR moves the elment higher up, therefore
avoiding the overflow.
There's a small width change as well, for better consistency.
* DEV: Rnemae channel path to just c
Also swap the channel id and channel slug params to be consistent with core.
* linting
* channel_path
* params in wrong order
* Drop slugify helper and channel route without slug
* Request slug and route models through the channel model if possible
* Add client side redirection for backwards-compatibility
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
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>
We've had the UploadReference table for some time now in core,
but it was added after ChatUpload was and chat was just never
moved over to this new system.
This commit changes all chat code dealing with uploads to create/
update/delete/query UploadReference records instead of ChatUpload
records for consistency. At a later date we will drop the ChatUpload
table, but for now keeping it for data backup.
The migration + post migration are the same, we need both in case
any chat uploads are added/removed during deploy.
Learn more about skidding here: https://popper.js.org/docs/v2/modifiers/offset/#skidding-1
This change has two goals:
- Fixes an issue when the user had zoomed the viewport and the popper would position on the opposite side
- Makes msg actions arguably more pleasant to the eye by preventing it to be right aligned with the message container
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.
Note this commit also slightly changes internal API: channel instead of getChannel and updateCurrentUserChannelNotificationsSettings instead of updateCurrentUserChatChannelNotificationsSettings.
Also destroyChannel takes a second param which is the name confirmation instead of an optional object containing this confirmation. This is to enforce the fact that it's required.
In the future a top level jsdoc config file could be used instead of the hack tempfile, but while it's only an experiment for chat, it's probably good enough.
We refer to the channel name rather than title elsewhere
(including the new channel modal), so we should be consistent.
Title is an internal abstraction, since DM channels cannot have
names (currently).
Also change the name field on channel edit to a input type="text"
rather than a textarea, since we don't want a huge input here.
Added in c2013865d7,
this migration was supposed to only turn off the hashtag
setting for existing sites (since that was the old default)
but its doing it for new ones too because we run all migrations
on new sites.
Instead, we should only run this if the first migration was
only just created, meaning its a new site.
If the enable_experimental_hashtag_autocomplete setting is
enabled, then we should autolink hashtag references to the
archived channels (e.g. #blah::channel) for a nicer UX, and
just show the channel name if not (since doing #channelName
can lead to weird inconsistent results).
Since the poll post handler runs very early in the post creation
process, it's possible to run the handler on an obiviously invalid post.
This change ensures the post's `raw` value is present before
proceeding.
The spec was flaky because it was dependent on order,
when usernames got high enough sequence numbers in them
we would get this error:
> expected to find text "bruce99, bruce100" in "bruce100, bruce99"
Also move selectors into page object and use them in the
spec 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.
* Firefox now finally returns PerformanceMeasure from performance.measure
* Some TODOs were really more NOTE or FIXME material or no longer relevant
* retain_hours is not needed in ExternalUploadsManager, it doesn't seem like anywhere in the UI sends this as a param for uploads
* https://github.com/discourse/discourse/pull/18413 was merged so we can remove JS test workaround for settings
* 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.
In certain cases, like when `SiteSetting.slug_generation_method`
is set to `none` with certain locales, the autogenerated chat
channel slugs will end up blank. This was causing errors in
unrelated jobs calling `update!` on the channel. Instead, we
should just copy Category behaviour, which does not error
if the autogenerated slug ends up blank. We already allow
for this with chat channel URLs, using `-` in place of the
missing slug.
The problem here was that if your input has an Enter
listener (such as the chat message input) and the
`fill_in(with: str)` string has a `\n` at the end, this
is treated as an Enter keypress, so this `fill_in` was
submitting the chat message.
This commit fixes an issue where the chat message bookmarks
did not respect the user's `bookmark_auto_delete_preference`
which they select in their user preference page.
Also, it changes the default for that value to "keep bookmark and clear reminder"
rather than "never", which ends up leaving a lot of expired bookmark
reminders around which are a pain to clean up.
`last_message_sent_at` has a `NOT_NULL` constraint in the DB so it should be safe to use for sorting.
This was causing two flakeys:
```
1) UserNotifications.chat_summary with public channel email subject with regular mentions includes both channel titles when there are exactly two with unread mentions
Failure/Error: example.run
expected: "[Discourse] New message in Random 62 and Test channel"
got: "[Discourse] New message in Test channel and Random 62"
(compared using ==)
# ./plugins/chat/spec/mailers/user_notifications_spec.rb:203:in `block (6 levels) in <main>'
# ./spec/rails_helper.rb:356:in `block (2 levels) in <top (required)>'
# ./vendor/bundle/ruby/3.1.0/gems/webmock-3.18.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
2) UserNotifications.chat_summary with public channel email subject with regular mentions displays a count when there are more than two channels with unread mentions
Failure/Error: example.run
expected: "[Discourse] New message in Random 62 and 2 others"
got: "[Discourse] New message in Test channel 0 and 2 others"
(compared using ==)
# ./plugins/chat/spec/mailers/user_notifications_spec.rb:236:in `block (6 levels) in <main>'
# ./spec/rails_helper.rb:356:in `block (2 levels) in <top (required)>'
# ./vendor/bundle/ruby/3.1.0/gems/webmock-3.18.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
```
This commit is a series of fixes to improve stability of system tests following the use of threadsafe:
* Jobs.run_immediately in before block was causing issues
* During test a js error could be caused by an undefined this.details in chat-live-pane
* Apply the chat composer click trick everywhere when sending a message, it ensures we are not hiding anything with autocomplete
* There was another case not using send_message yet
This PR removes the limit added to max_users_notified_per_group_mention during #19034 and improve the performance when expanding mentions for large channel or groups by removing some N+1 queries and making the whole process async.
* Fully async chat message notifications
* Remove mention setting limit and get rid of N+1 queries
It should fix flakeys we have due to using_session. This commit is also fixing tests which were failing constantly with treadsafe enabled.
A test has also bene skipped as the issue couldn't be found so far.
More info: https://github.com/teamcapybara/capybara#threadsafe-mode
- improves UI by displaying channel status on it's own line
- ensures channel status is correctly updated right after the request on frontend
- adds status on info page
Recent changes surfaced the various issues with this codepath:
- we were not correctly reseting `messageLookup` leading to us trying to scroll to a non existing message in the view
- we were calling markAsRead which would scroll to the bottom, even if we had a target message
- we were not debouncing fetchMessages, which could cause multiple reload of the messages when loading it with a targetMessageId: first fetch from last read and then immediately fetch from targetMessageId
- other naming inconsistencies
- not handling drawer
This commit also adds tests for classic scenarios related to this use case.
I could repro the same failure by doing: `page.driver.browser.network_conditions = { offline: false, latency: 3000, throughput: 0 }`
Wait shouldn't be needed as we wait for selector, but I couldn't find a better solution on this case for now.
We are all in on system specs, so this commit moves all the chat quoting acceptance tests (some of which have been skipped for a while) into system specs.
The way our markdown raw_html hoisting worked, we only
supported one level of hoisting the HTML content. However
when nesting [chat] transcript BBCode we need to allow
for multiple levels of it. This commit changes opts.discourse.hoisted
to be more constant, and the GUID keys that have the hoisted
content are only deleted by unhoistForCooked rather than
the cook function itself, which prematurely deletes them
when they are needed further down the line.
Honestly seems like it's being in some weird loop for
discourse/hashtag_autocomplete_spec.rb for this:
```ruby
within topic_page.post_by_number(2) do
cooked_hashtags = page.all(".hashtag-cooked", count: 2)
expect(cooked_hashtags[0]["outerHTML"]).to eq(<<~HTML.chomp)
<a class=\"hashtag-cooked\" href=\"#{category.url}\" data-type=\"category\" data-slug=\"cool-cat\"><svg class=\"fa d-icon d-icon-folder svg-icon svg-node\"><use href=\"#folder\"></use></svg><span>Cool Category</span></a>
HTML
expect(cooked_hashtags[1]["outerHTML"]).to eq(<<~HTML.chomp)
<a class=\"hashtag-cooked\" href=\"#{tag.url}\" data-type=\"tag\" data-slug=\"cooltag\"><svg class=\"fa d-icon d-icon-tag svg-icon svg-node\"><use href=\"#tag\"></use></svg><span>cooltag</span></a>
HTML
end
```
I see this many times in the full logs with `SELENIUM_VERBOSE_DRIVER_LOGS=1`:
```
COMMAND FindElements {
"using": "css selector",
"value": "#post_2"
}
Followed by:
COMMAND FindChildElements {
"id": "26dfe542-659b-46cc-ac8c-a6c2d9cbdf0a",
"using": "css selector",
"value": ".hashtag-cooked"
}
```
Over and over and over, there are 58 such occurrences. I am beginning to
think `within` is just poison that should be avoided.
Joffrey JAFFEUX
Jan Cernik
chapoi
Martin Brennan
Andrei Prigorshnev
Krzysztof Kotlarek
Loïc Guitaut
Discourse Translator Bot
Gerhard Schlager
Kris
Keegan George
Isaac Janzen
Dan Ungureanu
Roman Rizzi
David Taylor
Penar Musaraj
Michael Brown
Natalie Tay
Jarek Radosz
Selase Krakani
Alan Guo Xiang Tan
