This commit adds some system specs to test uploads with
direct to S3 single and multipart uploads via uppy. This
is done with minio as a local S3 replacement. We are doing
this to catch regressions when uppy dependencies need to
be upgraded or we change uppy upload code, since before
this there was no way to know outside manual testing whether
these changes would cause regressions.
Minio's server lifecycle and the installed binaries are managed
by the https://github.com/discourse/minio_runner gem, though the
binaries are already installed on the discourse_test image we run
GitHub CI from.
These tests will only run in CI unless you specifically use the
CI=1 or RUN_S3_SYSTEM_SPECS=1 env vars.
For a history of experimentation here see https://github.com/discourse/discourse/pull/22381
Related PRs:
* https://github.com/discourse/minio_runner/pull/1
* https://github.com/discourse/minio_runner/pull/2
* https://github.com/discourse/minio_runner/pull/3
* Minor style adjustments
* Removes "all" count because it's redundant to the count on New
* Updates generic class names with -- modifier to follow BEM and help avoid class name collisions
* Hides the toggle when bulk select is enabled (the UI ends up being too busy)
This PR adds a new toggle to switch the (new) /new list between showing topics with new replies (a.k.a unread topics), new topics, or everything mixed together.
So we have to order by calling `find_each(order: :desc)`.
Note that that will order rows by Id, not by `last_match_at`
as we tried before (though that didn't work).
What is the problem here?
When transiting between `/filter` routes with different `q` query
params, the input field is not updating to include the values in the `q`
query param. This was because we were setting the value of the input
field in the constructor of the controller but controllers are actually
singletons in Ember so setting the value of the input field is only done
once when the controller is initialised.
What is the fix here?
Instead of setting the value of the input field in the controller, we
set the value in the `setupController` hook in the route file.
* scrub non-a html tags from tag descriptions on create, strips all tags from tag description when displayed in tag hover
* test for tag description links
* UX: basic render-tag test
* UX: fix linting
* UX: fix linting
* fix broken tests
* Update spec/models/tag_spec.rb
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
* UX: use has_sanitizable_fields instead of has_scrubbable_fields to ensafen tag.description
---------
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
When we receive the stream parameter, we'll queue a job that periodically publishes partial updates, and after the summarization finishes, a final one with the completed version, plus metadata.
`summary-box` listens to these updates via MessageBus, and updates state accordingly.
The OpenComposer mixin comes from a time before we had a composer service. As well as being a general cleanup/refactor, this commit aims to removes interlinking between composer APIs and the discovery-related controllers which are being removed as part of #22622.
In summary, this commit:
- Removes OpenComposer mixin
- Adds and updates composer service APIs to support everything that `openComposer` did
- Updates consumers to call the composer service directly, instead of relying on the mixin (either directly, or via a route-action which bubbled up to some parent)
- Deprecates composer-related methods on `DiscourseRoute` and on the application route
This commit moves the calendar date and time picker shown in
the local dates modal into a core component that can be reused
in other places. Also add system specs to make sure there isn't
any breakages with this feature, and a section to the styleguide.
There is no decorateCooked equivalent for small action posts,
so we need to manually call decorateHashtags when there is a custom
message for small action posts in order for the hashtags to get
their coloured icon/square.
This commit removes any logic in the app and in specs around
enable_experimental_hashtag_autocomplete and deletes some
old category hashtag code that is no longer necessary.
It also adds a `slug_ref` category instance method, which
will generate a reference like `parent:child` for a category,
with an optional depth, which hashtags use. Also refactors
PostRevisor which was using CategoryHashtagDataSource directly
which is a no-no.
Deletes the old hashtag markdown rule as well.
This fixes:
- a regression from 30c152c, where navigating to a topic's last reply
via keyboard would lose track of the topic when returning to the topic
list
- an issue where if a topic's last post is a small post, navigating to it
via keyboard would not focus the post
Co-authored-by: David Taylor <david@taylorhq.com>
Edit community section button is hidden in secondary/more section. However, when there are no secondary links, then more section is not shown. In that case, we should still display an edit button for admins, so they can edit the section.
FEATURE: Only approved flags for post counters
* Why was this change necessary?
The counters for flagged posts in the user's profile and user index from
the admin view include flags that were rejected, ignored or pending
review. This introduces unnecessary noise. Also the flagged posts
counter in the user's profile includes custom flags which add further
noise to this signal.
* How does it address the problem?
* Modifying User#flags_received_count to return posts with only approved
standard flags
* Refactoring User#number_of_flagged_posts to alias to
User#flags_received_count
* Updating the flagged post staff counter hyperlink to navigate to a
filtered view of that user's approved flagged posts to maintain
consistency with the counter
* Adding system tests for the profile page to cover the flagged posts
staff counter
By default, only 10 members are highlighted on group cards. However,
joining/leaving a big group via the buttons on the group card results in
up to 50 members being highlighted. For large groups, this causes the card
to move off-screen.
This happens because, while the initial render explicitly fetches only 10
members, we don't seem to apply the same limit as part of the member
reload performed when a user leaves/joins via the buttons on the card.
This PR fixes that by only making the first 10 users available for
highlight regardless of the number of members loaded in the store.
Why this change?
We were verifying that a url for a section link in a custom sidebar
section is valid by passing the url string to `Router#recognize`.
If a `rootURL` has been set on the router, the url string that is passed
to `Router#recognize` has to start with the `rootURL`.
This commit fixes the problem by ensuring that `RouteInfoHelper` adds
the application subfolder path before calling `Router#recognize` on the
url string.
Why this change?
When setting up the `IntersectionObserver`, we did not account for the
top margin and padding causing no intersection event to fire when the
last tag is load into view. This commits fixes the problem by setting a
bottom margin using the `rootMargin` option when setting up the
`IntersectionObserver`.
This commit also improves the test coverage surrounding the loading of
more tags.
Why this change?
We're already displaying a category's description as the title attribute
on the category section link. We should do the same for tags as well.
* Why was this change necessary?
The current logic in the user.hbs template file does not render the
trust level element for the user's info panel when the user is TL0,
because 0 is treated as falsey in the `if` conditional block.
Ref: https://meta.discourse.org/t/tl0-not-displayed-on-users-profile-pages/271779/10
* How does it address the problem?
This PR adds a predicate helper method local to the user controller that
includes an additional check which returns true if the trust_level of
the user is 0 on top of the existing logic. This allows TL0 users to
have their trust level rendered correctly in their profile's info panel.
Performing a `Delete User`/`Delete and Block User` reviewable actions for a
queued post reviewable from the `review.show` route results in an error
popup even if the action completes successfully.
This happens because unlike other reviewable types, a user delete action
on a queued post reviewable results in the deletion of the reviewable
itself. A subsequent attempt to reload the reviewable record results in
404. The deletion happens as part of the call to `UserDestroyer` which
includes a step for destroying reviewables created by the user being
destroyed. At the root of this is the creator of the queued post
being set as the creator of the reviewable as instead of the system
user.
This change assigns the creator of the reviewable to the system user and
uses the more approapriate `target_created_by` column for the creator of the
post being queued.
Why was the test flaky?
The test relied on the fact that visiting a topic would marked its
post as unread. However, we did not actually stay on the topic long
enough in some cases for it to be considered read based on the logic in
our client side code.
This commit fixes the flakiness by ensuring that the post has actually
been read before navigating away.
1) Bookmarking posts and topics topic level bookmarks clears all topic bookmarks from the topic bookmark button if more than one post is bookmarked
Failure/Error: expect(Bookmark.where(user: current_user).count).to eq(0)
expected: 0
got: 2
Using the lastViewedTopicId indiscriminately can cause strange scrolling behavior when navigating to a **different** topic list after viewing a topic. We only want to refocus the topic when going 'back' to the same topic list which originally triggered the navigation.
Previously we were implementing scroll reset/memorization on a per-page basis. Many of these approaches relied on the `didInsertElement` hook, which is no longer appropriate since Discourse changed to use the 'loading slider' strategy for page transitions.
This commit rips out all of our custom scroll resetting/memorizing, and implements those things in a generic service. There are two features:
1. After every route transition, scroll to the top of the page
2. When using browser back/forward buttons, restore the last known scroll position for those routes
To opt-out of the behaviour, individual routes can add a scrollOnTransition boolean to their RouteInfo metadata using Ember's `buildRouteInfoMetadata` hook.
Why this change?
Prior to this change, dismissing unreads posts did not publish the
changes across clients for the same user. As a result, users can end up
seeing an unread count being present but saw no topics being loaded when
visiting the `/unread` route.
Why this change?
Group mention notifications are currently placed in the "Others" tab
of the user menu which is odd considering that mentioned notifications
are in the reply tab. This commit changes it such that group mention
notifications are displayed in the reply tab as well.
This commit makes sure we don't load all data into memory when doing CSV exports.
The most important change here made to the recently introduced export of chat
messages (3ea31f4). We were loading all data into memory in the first version, with
this commit it's not the case anymore.
Speaking of old exports. Some of them already use find_each, and it worked as
expected, without loading all data into memory. And it will proceed working as
expected after this commit.
In general, I made sure this change didn't break other CSV exports, first manually, and
then by writing system specs for them. Sadly, I haven't managed yet to make those
specs stable, they work fine locally, but flaky in GitHub actions, so I've disabled them
for now.
I'll be making more changes to the CSV exports code soon, those system specs will be
very helpful. I'll be running them locally, and I hope I'll manage to make them stable
while doing that work.
* FEATURE: Inline topic summary. Cached version accessible to everyone.
Anons and non-members of the `custom_summarization_allowed_groups_map` groups can see cached summaries for any accessible topic. After the first 12 hours and if the posts to summarize have changed, allowed users clicking on the button will automatically re-generate it.
* Ensure chat summaries work and prevent model hallucinations when there are no messages.
Why is this change being made?
We've decided that the previous "community" section should look more
like a primary section that holds the most important navigation links
for the site and the word "community" doesn't quite fit that
description. Therefore, we've made the decision to drop the
section heading for the community section.
As part of removing the section heading, the following changes are made
as well:
1. Button to customize the section has been moved to the "footer" of the
"More..." section when `navigation_menu` site setting is set to `sidebar`.
When `navigation_menu` is set to `header dropdown`, a button to customize
the section is shown inline.
2. The section will no longer be collapsable.
3. The title of the section is no longer customisable as it is no longer
displayed. As a technical note, we have not dropped any previous
customisations of the section's title previously in case we have to
bring back the header in the future.
4. The new topic button that was previously present in the header has
been removed alongside the header. Admins can add a custom section
link to the `/new-topic` route if there would like to make it easier for
users to create a new topic in the sidebar.
1) Edit Category when editing a category with form templates set should have form templates enabled and showing the selected templates
Failure/Error: expect(category_page).to have_selected_template(selected_templates)
expected `#<PageObjects::Pages::Category:0x00007fdb278fbd30>.has_selected_template?("template_0,template_1")` to be truthy, got false
Wait for CSS rather than trying to compare attr directly
and also make sure the ids are always in order.
Previously , the test was flaky and failing with a selenium stale
element error because we were retrieving the tag nodes with `all` and
then calling `.map(&:text)` on it. However, there is a chance that a
re-render happens and those nodes will end up being stale resulting in
the selenium error.
Why this change?
By ensuring the reset happens in an `ensure` code block, we ensure that
the code will always be run even if code fails or an error is raised.
This helps to prevent leaking custom network condition states and
improves the stability of our system tests.
What is the problem?
Before this change, we were relying on the `/tags` endpoint which
returned all the tags that are visible to a give user on the site leading to potential performance problems.
The attribute keys of the response also changes based on the `tags_listed_by_group` site setting.
What is the fix?
This commit fixes the problems listed above by creating a dedicate `#list` action in the
`TagsController` to handle the listing of the tags in the edit
navigation menu tags modal. This is because the `TagsController#index`
action was created specifically for the `/tags` route and the response
body does not really map well to what we need. The `TagsController#list`
action added here is also much safer since the response is paginated and
we avoid loading a whole bunch of tags upfront.
What is the problem?
This regressed in fe294ab1a7 and we did
not have any tests on mobile to catch the regression. The problem was
that we were conditionally rendering the edit nav menu modals component
in the sidebar. However, the sidebar is collapsed on mobile when a
button is clicked. When the sidebar collapses, the edit nav menu modals
ended up being destroyed with it.
Why this change?
A new component based API for modals was introduced in
b3a23bd9d6. This commit moves the edit
navigation menu tags and categories modal to the new API.
What is the problem?
Before this change, the edit navigation menu tags modal was not
displaying tags that belonged to a tag_group when the tags_listed_by_group
site setting was set to true. This is because we are relying on the
/tags endpoint which returned tags in various keys depending on the
tags_listed_by_group site setting. When the site setting is set to
true, tags under belonging to tag groups were returned in the
extra.tag_groups attribute.
What is the fix?
This commit fixes it by pushing all tags in returned under the
`tag_groups` attribute into the list of tags to displayed. In a
following commit, we will move away from the `/tags` endpoint to a
dedicated route to handle the listing of tags in the modal.
Before this change, links which required full reload because they are not in ember routes like `/my/preferences` or links to docs like `/pub/*` were treated as real external links. Therefore, they were opening in self window or new tab based on user `external_links_in_new_tab` setting.
To be consistent with behavior when full reload links are in the post, they are treated as internal and always open in the same window.
Why this change?
We are currently not fully satisfied with the current way to edit the
categories and tags that appears in the sidebar where the user is
redirected to the tracking preferences tab in the user's profile causing
the user to lose context of the current page. In addition, the dropdown
to select categories or tags limits the amount of information we can
display.
Since editing or adding a custom categories section is already using a
modal, we have decided to switch editing the categories and tags that
appear in the sidebar to use a modal as well.
This commit removes the `new_edit_sidebar_categories_tags_interface_groups` site setting and
make the modals the default for all users.
Updates the interface for implementing summarization strategies and adds a cache layer to summarize topics once.
The cache stores the final summary and each chunk used to build it, which will be useful when we have to extend or rebuild it.
Why does this change do?
If the `fixed_category_positions` is `false`, we want to order the
categories in the edit navigation menu categories modal by name. This
makes it easier to filter through a large list of categories.
This commit also fixes a bug where we were unintentionally mutating the
`this.site.categories` array.
Why is this change required?
Previously, the tests in `viewing_sidebar_as_anonymous_user_spec.rb` was
flaky because the ordering of the tags changes depending on what the
auto generated tag names are. If a tag name is generated with the name
`tag10`, it would then be sorted before `tag9` which messes up the
ordering specified in our tests. This commit fixes the problem by
specifying the tag names instead of relying on the auto generated ones
by fabricator.
What does this change do?
This change adds a dropdown filter that allows a user to filter by
selected or unselected categories/tags in the edit navigation menu
modal.
For the categories modal, parent categories that do not match the
dropdown filter will be displayed as disabled since those parent
categories need to be displayed to maintain the hieracy of the child
child categories.
Why this change?
Predicate matchers are poor at providing good error messages when it
fails if all the predicate matcher does is to return a boolean. Prior to
this change, we were using `has_css? && all?` to assert for the tag
section links. There are two problems here. Firstly, when one of the matchers
fail, the error message does not provide any indication of which matcher
failed making it hard to debug failures. Secondly, the matchers were not
able to assert for the ordering of the tag section links which is an
important behaviour to assert for.
This commit changes `PageObjects::Components::Sidebar#has_tag_section_links?`
such that we make use of assertions to ensure ordering. The usage of
`all` will also provide a clear error message when things go wrong.
Why this change?
There was alot of duplication between the edit navigation menu tags/categories modal which
was making it hard to introduce new changes as the work had to be
duplicated into multiple places.
This commit mainly extracts the duplicated code into common components
such that it is easier to make styling changes across both modals.
This PR splits up the preference that controls the count vs dot and destination of sidebar links, which is really hard to understand, into 2 simpler checkboxes:
The new preferences/checkboxes are off by default, but there are database migrations to switch the old preference to the new ones so that existing users don't have to update their preferences to keep their preferred behavior of sidebar links when this changed is rolled out.
Internal topic: t/103529.
What this change?
When a user opens the modal to edit tags or categories for the
navigation menu, we want to input filter to have focus. This commit
fixes that by doing the following:
1. Changes <DModal> component such that it prioritises elements with the
autofocus attribute first.
2. Adds `autofocus` to the input elements on the edit tags/categories
modal form.
When a site does not have `default_navigation_menu_tags`
site setting set, anonymous users should be shown the site's top tags as
a default in the tags section. However, this regressed in 9fad71809c
and we ended up showing anonymous users a tags section with only the
`All Tags` section link.
As part of this commit, I have also refactored the QUnit acceptance
tests to system tests which are much easier to work with.
What does this change do?
This change adds the deselect all and reset to defaults buttons to the
edit navigation menu tags modal. The deselect all button when
clicked deselects all the selected tags in the modal. If the user
saves with no tags selected, the user's tags section in the
navigation menu will be set to the site's top tags.
The reset to defaults button is only shown when the
`default_navigation_menu_tags` site setting has been configured.
When clicked, the user's tags section in the navigation menu is
automatically set to the tags defined by the
`default_navigation_menu_tags` site setting.
There is a problem that unread and new count is not updated to reflecting topicTrackingState.
It is because discourseComputed on Category is not working properly with topicTrackingState. Moving it to component level is making counter reliable.
What does this change do?
This commit adds an input filter to filter through the tag checkboxes in the
modal to edit tags that are shown in the user's navigation menu. The
filtering is a simple matching of the given filter term against the
names of the tags.
What does this change do?
This change is a first pass for adding a modal used to edit tags that appears in
the navigation menu. As the feature is being worked on in phases, it is
currently hidden behind the `new_edit_sidebar_categories_tags_interface_groups` site setting.
The following features will be worked on in future commits:
1. Input filter to filter through the tgas
2. Button to reset tag selection to default navigation menu tags site
settings
3. Button to deselect all current selection
Why this change?
The comment consists of an output that was copied from RSpec's default
output. This has the potential to mess with systems that are parsing
RSpec's output to fetch the spec failures as those systems are usually
looking for the first occurence of `Failures:`
This commit adds an aria-label attribute to cooked hashtags using
the post/chat message decorateCooked functionality. I have just used
the inner content of the hashtag (the tag/category/channel name) for
the label -- we can reexamine at some point if we want something
different like "Link to dev category" or something, but from what I
can tell things like Twitter don't even have aria-labels for hashtags
so the text would be read out directly.
This commit also refactors any ruby specs checking the HTML of hashtags
to use rspec-html-matchers which is far clearer than having to maintain
the HTML structure in a HEREDOC for comparison, and gives better spec
failures.
c.f. https://meta.discourse.org/t/hashtags-are-getting-a-makeover/248866/23?u=martin
What does this change do?
This change adds the deselect all and reset to defaults buttons to the
edit navigation menu categories modal. The deselect all button when
click deselects all the selected categories in the modal. If the user
saves with no categories selected, the user's categories section in the
navigation menu will be set to the site's top categories.
The reset to defaults button is only shown when the
`default_navigation_menu_categories` site setting has been configured.
When clicked, the user's categories section in the navigation menu is
automatically set to the categories defined by the
`default_navigation_menu_categories` site setting.
1. `everything` was changed to `topics`
2. Path for my posts translation is `sidebar.sections.community.links.my_posts.content` not `sidebar.sections.community.links.my/posts.content`
Today I learnt that `has_link?("text of link")` by default does an
includes instead of looking for a link with the exact text. This is not
the behaviour I want so I'm changing
`PageObjects::Components::Sidebar.has_section_link?` to use the
`exact_text` option instead.
Communities can use sidebar or header dropdown, therefore navigation menu is a better name settings in 2 places:
- Old user sidebar preferences;
- Site setting about default tags and categories.
* FEATURE: Content custom summarization strategies.
This PR establishes a pattern for plugins to register alternative ways of summarizing content by extending a class that defines an interface.
Core controls which strategy we'll use and who has access to it through the `summarization_strategy` and `custom_summarization_allowed_groups`. It also defines the UI for summarizing topics.
Other plugins can access this summarization mechanism and implement their features, removing cross-plugin customizations, as it currently happens between chat and the discourse-ai plugin.
* Group membership validation and rate limiting
* Work with objects instead of classes
* Port summarization feature from discourse-ai to chat
* Rename available summaries to 'Top Replies' and 'Summary'
Clicking on TOC heading anchors in a subfolder setup was breaking the current URL for users.
Other than the fix this change introduces the ability to test the subfolder setup in system specs.
What does this change do?
This change is a continuation of
2191b879c6 and adds an input filter to the
edit sidebar categories modal which the user can use to filter through
the list of categories by the category's name.
Note that if a child category is being shown, all of its ancestors will
be shown even if the names of the ancestors do not match the given
filter. This is to ensure that we continue to display the hierarchy of a
child category even if the parent category does not match the filter.
Why does this commit do?
This commit adds support for sub-subcategories in the new edit sidebar
categories modal added in fc296b9a81. Note
that sub-subcategories are enabled when `max_category_nesting` is set to
`3`.
What this change?
We are currently not fully satisfied with the current way to edit the
categories and tags that appears in the sidebar where the user is
redirected to the tracking preferences tab in the user's profile causing
the user to lose context of the current page. In addition, the dropdown
to select categories or tags limits the amount of information we can
display.
Since editing or adding a custom categories section is already using a
modal, we have decided to switch editing the categories and tags that
appear in the sidebar to use a modal as well.
This commit ships a first pass of the edit categories modal such that we
can keep the commit small and reviewable. The incomplete nature of the
feature is also reflected in the fact that the feature is hidden behind
a new `new_edit_sidebar_categories_tags_interface_groups` site setting.
One user can create a post or chat message with a hashtag they
have permission to use, but then when other users look at that
post they will see an empty space next to the hashtag because they
do not have the permission to load the colors in CSS classes for
the related category.
This fixes the issue by adding a default color with a special
CSS class if the user doesn't have permission to see the linked
channel/category on the hashtag.
Display modal for combined new and unread view with options:
- [x] Dismiss new topics
- [x] Dismiss new posts
- [ ] Stop tracking these topics so they stop appearing in my new list
What is this change required?
We were seeing this error on CI
```
1) Fast edit when editing text that has strange characters saves when paragraph contains apostrophe
Failure/Error:
expect(find("#{topic_page.post_by_number_selector(2)} .cooked p")).to have_content(
"It ‘twas a great’ “day”!",
)
Selenium::WebDriver::Error::StaleElementReferenceError:
stale element reference: stale element not found
(Session info: chrome=114.0.5735.90)
```
I believe this is because the element that is "found" using `find` is
eventually re-rendered before the `have_content` matcher is called on
it.
Failure message on CI
```
Failures:
1) Network Disconnected Doesn't show the offline indicator when the site setting isn't present
Failure/Error: expect(page).to have_css("html.message-bus-offline")
expected to find css "html.message-bus-offline" but there were no matches
[Screenshot Image]: /__w/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_network_disconnected_doesn_t_show_the_offline_indicator_when_the_site_setting_isn_t_present_764.png
~~~~~~~ JS LOGS ~~~~~~~
http://localhost:31338/uploads/default/test_0/optimized/1X/_129430568242d1b7f853bb13ebea28b3f6af4e7_2_512x512.png - Failed to load resource: net::ERR_INTERNET_DISCONNECTED
http://localhost:31338/categories - Error while trying to use the following icon from the Manifest: http://localhost:31338/uploads/default/test_0/optimized/1X/_129430568242d1b7f853bb13ebea28b3f6af4e7_2_512x512.png (Download error or resource isn't a valid image)
~~~~~ END JS LOGS ~~~~~
# ./spec/system/network_disconnected_spec.rb:35:in `block (3 levels) in <main>'
# ./spec/system/network_disconnected_spec.rb:8:in `with_network_disconnected'
# ./spec/system/network_disconnected_spec.rb:34:in `block (2 levels) in <main>'
# ./spec/rails_helper.rb:380:in `block (3 levels) in <top (required)>'
# ./spec/rails_helper.rb:380:in `block (2 levels) in <top (required)>'
# ./spec/rails_helper.rb:372:in `block (3 levels) in <top (required)>'
# ./vendor/bundle/ruby/3.2.0/gems/timeout-0.3.2/lib/timeout.rb:189:in `block in timeout'
# ./vendor/bundle/ruby/3.2.0/gems/timeout-0.3.2/lib/timeout.rb:196:in `timeout'
# ./spec/rails_helper.rb:367:in `block (2 levels) in <top (required)>'
# ./spec/rails_helper.rb:356:in `block (2 levels) in <top (required)>'
# ./vendor/bundle/ruby/3.2.0/gems/webmock-3.18.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
```
