added site toggle functionality through site settings
added tests to implemented feature
Introduced suggested correction
renamed find_new_topic method and deleted click_new_topic_button method
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.
This was failing quite often with the following error:
```
1) Emoji deny list when using composer should remove denied emojis from emoji picker
Failure/Error: find("#{COMPOSER_ID} .emoji-picker")
Capybara::ElementNotFound:
Unable to find css "#reply-control .emoji-picker"
```
This was because our `click_toolbar_button` call on the Composer
page object used a number for the position of the toolbar button,
which can be flaky since there are things that hide/show toolbar
buttons or change their position.
Each toolbar button in the composer has a CSS class, so it is
more reliable to use that instead. Also fixed an instance of
calling `has_X?` method directly instead of using the
`have_x` rspec matcher.
Responding to negative behaviour tends to solicit more of the same. Common wisdom states: "don't feed the trolls".
This change codifies that advice by introducing a new nudge when hitting the reply button on a flagged post. It will be shown if either the current user, or two other users (configurable via a site setting) have flagged the post.
Following a change in e9f7262813 which prevents the notification level to be returned from the update endpoint, the model couldn't update itself. This commit makes the update manually and adds a test to prevent future regressions.
Note we could also change the backend endpoint, but this should work correctly with minimum risk.
This commit introduces a ChatChannelPaneSubscriptionsManager
and a ChatChannelThreadPaneSubscriptionsManager that inherits
from the first service that handle MessageBus subscriptions
for the main channel and the thread panel respectively.
This necessitated a change to Chat::Publisher to be able to
send MessageBus messages to multiple channels based on whether
a message was an OM for a thread, a thread reply, or a regular
channel message.
An initial change to update the thread indicator with new replies
has been done too, but that will be improved in future as we have
more data to update on the indicators.
Still remaining is to fully move over the handleSentMessage
functionality which includes scrolling and new message indicator
things.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This feature will allow sites to define which emoji are not allowed. Emoji in this list should be excluded from the set we show in the core emoji picker used in the composer for posts when emoji are enabled. And they should not be allowed to be chosen to be added to messages or as reactions in chat.
This feature prevents denied emoji from appearing in the following scenarios:
- topic title and page title
- private messages (topic title and body)
- inserting emojis into a chat
- reacting to chat messages
- using the emoji picker (composer, user status etc)
- using search within emoji picker
It also takes into account the various ways that emojis can be accessed, such as:
- emoji autocomplete suggestions
- emoji favourites (auto populates when adding to emoji deny list for example)
- emoji inline translations
- emoji skintones (ie. for certain hand gestures)
Previously, public custom sections were only visible to logged-in users. In this PR, we are making them visible to anonymous as well.
The reason is that Community Section will be moved into custom section model to be easily editable by admins.
Change styling of filter input & remove button.
This follows the same pattern of design we use for search. In the search dropdown we do not have a button to search. We rely on pressing enter. I've also provided an example of Github's PR filter UI at the bottom of this comment.
We also do not have buttons like this on any other topic-list header. On tag and category dropdowns, we also rely on pressing enter to filter the topic list by chosen categories & tags.
Co-authored-by: Jordan Vidrine <jordan@jordanvidrine.com>
When a category has default_list_filter=none, there were a number of issues which this commit resolves:
1. When using the breadcrumbs to navigate a `default_list_filter=none` category, adding a tag filter would not apply the no-subcategories filter, but the subcategories dropdown would still say 'none'. This commit adjusts `getCategoryAndTagUrl` so that `/none` is added to the URL
2. When landing on `/tags/c/{slug}/{id}/{tag}`, for a default_list_filter=none category, it would include subcategories. This commit introduces a client-side redirect to match the behavior of `/c/{slug}/{id}`
3. When directly navigating to `/c/{slug}/{id}`, it was correctly redirecting to `/c/{slug}/{id}/none`, BUT it was still using the preloaded data for the old route. This has been happening since e7a84948. Prior to that, the preloaded data was discarded and a new JSON request was made to the server. This commit restores that discarding behavior. In future we may want to look into making this more efficient.
System specs are introduced to provide end-end testing of this functionality
* 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
Feature to allow adding new tags from the edit tag synonyms tag search field.
Previously new tags had to be created from the topic composer, and then added via the edit tag synonyms page.
/t/92741
What is the problem?
We have a hidden site setting `show_category_definitions_in_topic_lists`
which is set to false by default. What this means is that category
definition topics are not shown in the topic list by default. Only the
category definition topic for the category being viewed will be shown.
However, we have a bug where we would show that a category has new
topics when a new child category along with its category definition
topic is created even though the topic list does not list the child
category's category definition topic.
What is the fix here?
This commit fixes the problem by shipping down an additional
`is_category_topic` attribute in `TopicTrackingStateItemSerializer` when
the `show_category_definitions_in_topic_lists` site setting has been set
to false. With the new attribute, we can then exclude counting child
categories' category definition topics when counting new and unread
counts for a category.
Small js fix for fast edit to allow posts to save changes when the post contains apostrophes and quotation marks. Replaces unicode characters in text prior to saving the edit.
Includes system tests for fast edit and introduces a new system spec component for fast edit usage.
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.
When the `navigation_menu` site setting has been set to `sidebar` or
`header dropdown`, overriding the site setting via the `navigation_menu`
query params did not work.
Follow-up to c47015b861
With the introduction of the sidebar navigation menu, the design team at
Discourse redesigned the user profile navigation to better coexist with
the sidebar.
What does this change do?
This commit the client to override the navigation menu setting
configured by the site temporarily based on the value of the
`navigation_menu` query param. The new query param replaces the old
`enable_sidebar` query param.
Why do we need this change?
The motivation here is to allow theme maintainers to quickly preview
what the site will look like with the various navigation menu site
setting.
Allows users to configure their own custom sidebar sections with links withing Discourse instance. Links can be passed as relative path, for example "/tags" or full URL.
Only path is saved in DB, so when Discourse domain is changed, links will be still valid.
Feature is hidden behind SiteSetting.enable_custom_sidebar_sections. This hidden setting determines the group which members have access to this new feature.
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.
The latter can be called directly from the Topic page object,
so we can remove some duplication between the two. There are
levels of page objects (e.g. entire page, component, complete flow)
and its perfectly valid to call one from another.
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.
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.
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.
Fixes an issue on mobile where navigating away from search and returning
results in confusing UI where there are no results but headings says "N
results found".
Fixes broken behaviour of arrow buttons for certain users as the interval to scroll menu can be cancelled before the scrolling actually happens.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This commit fleshes out and adds functionality for the new `#hashtag` search and
lookup system, still hidden behind the `enable_experimental_hashtag_autocomplete`
feature flag.
**Serverside**
We have two plugin API registration methods that are used to define data sources
(`register_hashtag_data_source`) and hashtag result type priorities depending on
the context (`register_hashtag_type_in_context`). Reading the comments in plugin.rb
should make it clear what these are doing. Reading the `HashtagAutocompleteService`
in full will likely help a lot as well.
Each data source is responsible for providing its own **lookup** and **search**
method that returns hashtag results based on the arguments provided. For example,
the category hashtag data source has to take into account parent categories and
how they relate, and each data source has to define their own icon to use for the
hashtag, and so on.
The `Site` serializer has two new attributes that source data from `HashtagAutocompleteService`.
There is `hashtag_icons` that is just a simple array of all the different icons that
can be used for allowlisting in our markdown pipeline, and there is `hashtag_context_configurations`
that is used to store the type priority orders for each registered context.
When sending emails, we cannot render the SVG icons for hashtags, so
we need to change the HTML hashtags to the normal `#hashtag` text.
**Markdown**
The `hashtag-autocomplete.js` file is where I have added the new `hashtag-autocomplete`
markdown rule, and like all of our rules this is used to cook the raw text on both the clientside
and on the serverside using MiniRacer. Only on the server side do we actually reach out to
the database with the `hashtagLookup` function, on the clientside we just render a plainer
version of the hashtag HTML. Only in the composer preview do we do further lookups based
on this.
This rule is the first one (that I can find) that uses the `currentUser` based on a passed
in `user_id` for guardian checks in markdown rendering code. This is the `last_editor_id`
for both the post and chat message. In some cases we need to cook without a user present,
so the `Discourse.system_user` is used in this case.
**Chat Channels**
This also contains the changes required for chat so that chat channels can be used
as a data source for hashtag searches and lookups. This data source will only be
used when `enable_experimental_hashtag_autocomplete` is `true`, so we don't have
to worry about channel results suddenly turning up.
------
**Known Rough Edges**
- Onebox excerpts will not render the icon svg/use tags, I plan to address that in a follow up PR
- Selecting a hashtag + pressing the Quote button will result in weird behaviour, I plan to address that in a follow up PR
- Mixed hashtag contexts for hashtags without a type suffix will not work correctly, e.g. #ux which is both a category and a channel slug will resolve to a category when used inside a post or within a [chat] transcript in that post. Users can get around this manually by adding the correct suffix, for example ::channel. We may get to this at some point in future
- Icons will not show for the hashtags in emails since SVG support is so terrible in email (this is not likely to be resolved, but still noting for posterity)
- Additional refinements and review fixes wil
This commit introduces rails system tests run with chromedriver, selenium,
and headless chrome to our testing toolbox.
We use the `webdrivers` gem and `selenium-webdriver` which is what
the latest Rails uses so the tests run locally and in CI out of the box.
You can use `SELENIUM_VERBOSE_DRIVER_LOGS=1` to show extra
verbose logs of what selenium is doing to communicate with the system
tests.
By default JS logs are verbose so errors from JS are shown when
running system tests, you can disable this with
`SELENIUM_DISABLE_VERBOSE_JS_LOGS=1`
You can use `SELENIUM_HEADLESS=0` to run the system
tests inside a chrome browser instead of headless, which can be useful to debug things
and see what the spec sees. See note above about `bin/ember-cli` to avoid
surprises.
I have modified `bin/turbo_rspec` to exclude `spec/system` by default,
support for parallel system specs is a little shaky right now and we don't
want them slowing down the turbo by default either.
### PageObjects and System Tests
To make querying and inspecting parts of the page easier
and more reusable inbetween system tests, we are using the
concept of [PageObjects](https://www.selenium.dev/documentation/test_practices/encouraged/page_object_models/) in
our system tests. A "Page" here is generally corresponds to
an overarching ember route, e.g. "Topic" for `/t/324345/some-topic`,
and this contains logic for querying components within the topic
such as "Posts".
I have also split "Modals" into their own entity. Further down the
line we may want to explore creating independent "Component"
contexts.
Capybara DSL should be included in each PageObject class,
reference for this can be found at https://rubydoc.info/github/teamcapybara/capybara/master#the-dsl
For system tests, since they are so slow, we want to focus on
the "happy path" and not do every different possible context
and branch check using them. They are meant to be overarching
tests that check a number of things are correct using the full stack
from JS and ember to rails to ruby and then the database.
### CI Setup
Whenever a system spec fails, a screenshot
is taken and a build artifact is produced _after the entire CI run is complete_,
which can be downloaded from the Actions UI in the repo.
Most importantly, a step to build the Ember app using Ember CLI
is needed, otherwise the JS assets cannot be found by capybara:
```
- name: Build Ember CLI
run: bin/ember-cli --build
```
A new `--build` argument has been added to `bin/ember-cli` for this
case, which is not needed locally if you already have the discourse
rails server running via `bin/ember-cli -u` since the whole server is built and
set up by default.
Co-authored-by: David Taylor <david@taylorhq.com>
