There was an issue where if hashtag-cooked HTML was sent
to the ExcerptParser without the keep_svg option, we would
end up with empty </use> and </svg> tags on the parts of the
excerpt where the hashtag was, in this case when a post
push notification was sent.
Fixed this, and also added a way to only display a plaintext
version of the hashtag for cases like this via PrettyText#excerpt.
When checking whether an existing upload should be secure
based on upload references, do not count deleted posts, since
there is still a reference attached to them. This can lead to
issues where e.g. an upload is used for a post then later on
a custom emoji.
Currently, `Tag#topic_count` is a count of all regular topics regardless of whether the topic is in a read restricted category or not. As a result, any users can technically poll a sensitive tag to determine if a new topic is created in a category which the user has not excess to. We classify this as a minor leak in sensitive information.
The following changes are introduced in this commit:
1. Introduce `Tag#public_topic_count` which only count topics which have been tagged with a given tag in public categories.
2. Rename `Tag#topic_count` to `Tag#staff_topic_count` which counts the same way as `Tag#topic_count`. In other words, it counts all topics tagged with a given tag regardless of the category the topic is in. The rename is also done so that we indicate that this column contains sensitive information.
3. Change all previous spots which relied on `Topic#topic_count` to rely on `Tag.topic_column_count(guardian)` which will return the right "topic count" column to use based on the current scope.
4. Introduce `SiteSetting.include_secure_categories_in_tag_counts` site setting to allow site administrators to always display the tag topics count using `Tag#staff_topic_count` instead.
This fixes a longstanding issue for sites with the
secure_uploads setting enabled. What would happen is a scenario
like this, since we did not check all places an upload could be
linked to whenever we used UploadSecurity to check whether an
upload should be secure:
* Upload is created and used for site setting, set to secure: false
since site setting uploads should not be secure. Let's say favicon
* Favicon for the site is used inside a post in a private category,
e.g. via a Onebox
* We changed the secure status for the upload to true, since it's been
used in a private category and we don't check if it's originator
was a public place
* The site favicon breaks :'(
This was a source of constant consternation. Now, when an upload is _not_
being created, and we are checking if an existing upload should be
secure, we now check to see what the first record in the UploadReference
table is for that upload. If it's something public like a site setting,
then we will never change the upload to `secure`.
Prior to this change, we were parsing `Post#cooked` every time we
serialize a post to extract the usernames of mentioned users in the
post. However, the only reason we have to do this is to support
displaying a user's status beside each mention in a post on the client side when
the `enable_user_status` site setting is enabled. When
`enable_user_status` is disabled, we should avoid having to parse
`Post#cooked` since there is no point in doing so.
Since the new hashtag format has been added, we want site
admins to be able to rebake old posts with the old hashtag
format. This can now be done with `rake hashtags:mark_old_format_for_rebake`
which goes and marks posts with the old cooked version of hashtags
in this format for rebake:
```
<a class=\"hashtag\" href=\"/c/ux/14\">#<span>ux</span></a>
```
c.f. https://meta.discourse.org/t/what-rebake-is-required-for-the-new-autocomplete-styling/249642/12
This commit fixes the following issue:
* User creates a post
* Akismet or some other thing like requiring posts to be approved puts
the post in the review queue, deleting it
* Admin approves the post
* Email is never sent to mailing list mode subscribers
We intentionally do not enqueue this for every single post when
recovering a topic (i.e. recovering the first post) since the topics
could have a lot of posts with emails already sent, and we don't want
to clog sidekiq with thousands of notify jobs.
TL4 users can already list and unlist topics, but they can't see
the unlisted topics. This change brings this to par by allowing
TL4 users to also see unlisted topics.
So it can easily be overwritten in a plugin for example.
### Added more tests to provide better coverage
We previously only had `u.silenced_till IS NULL` but I made it consistent with pretty much every other places where we check for "active" users.
These two new lines do change the query a tiny bit though.
**Before**
- You could not get the badge if you were currently silenced (no matter what period is being checked)
- You could get the badge if you were suspended 😬
**After**
- You can't get the badge if you were silenced during the past year
- You can't get the badge if you were suspended during the past year
### Improved the performance of the query by using `NOT EXISTS` instead of `LEFT JOIN / COUNT() = 0`
There is no difference in behaviour between
```sql
LEFT JOIN user_badges AS ub ON ub.user_id = u.id AND ...
[...]
HAVING COUNT(ub.*) = 0
```
and
```sql
NOT EXISTS (SELECT 1 FROM user_badges AS ub WHERE ub.user_id = u.id AND ...)
```
The only difference is performance-wise. The `NOT EXISTS` is 10-30% faster on very large databases (aka. posts and users in X millions). I checked on 3 of the largest datasets I could find.
When the thread is aborted, an exception is raised before the `start` of a job is set, and therefore raises an exception in the `ensure` block. This commit checks that `start` exists, and also adds `abort_on_exception=true` so that this issue would have caused test failures.
The `enable_new_notifications_menu` site setting allows sites that have
`navigation_menu` set to `legacy` to use the redesigned notifications
menu before switching to the new sidebar navigation menu.
This is a very subtle one. Setting the redirect URL is done by passing
a hash through a Discourse event. This is broken on Ruby 2 since the
support for keyword arguments in events was added.
In Ruby 2 the last argument is cast to keyword arguments if it is a
hash. The key point here is that creates a new copy of the hash, so
what the plugin is modifying is not the hash that was passed.
This patch introduces a cookies rotator as indicated in the Rails
upgrade guide. This allows to migrate from the old SHA1 digest to the
new SHA256 digest.
This will give us some aggregate stats on the defer queue performance.
It is limited to 100 entries (for safety) which is stored in an LRU cache.
Scheduler::Defer.stats can then be used to get an array that denotes:
- number of runs and completions (queued, finished)
- error count (errors)
- total duration (duration)
We can look later at exposing these metrics to gain visibility on the reason
the defer queue is clogged.
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
Currently when generating a onebox for Discourse topics, some important
context is missing such as categories and tags.
This patch addresses this issue by introducing a new onebox engine
dedicated to display this information when available. Indeed to get this
new information, categories and tags are exposed in the topic metadata
as opengraph tags.
If unaccent is called with quote-like Unicode characters then it can
generate invalid queries because some of the transformed quotes by
unaccent are not escaped and to_tsquery fails because of bad input.
This commits replaces more quote-like Unicode characters before
unaccent is called.
Fixes the support for kwargs in `DiscourseEvent.trigger()` on Ruby 3, e.g.
```rb
DiscourseEvent.trigger(:before_system_message_sent, message_type: type, recipient: @recipient, post_creator_args: post_creator_args, params: method_params)
```
Fixes https://github.com/discourse/discourse-local-site-contacts
If a secure upload's access_control_post was trashed, and an anon user
tried to look at that upload, they would get a 500 error rather than
the correct 403 because of an error inside the PostGuardian logic.
This commit does a couple of things:
1. Changes the limit of tags to include a subject for a
notification email to the `max_tags_per_topic` setting
instead of the arbitrary 3 limit
2. Adds both an X-Discourse-Tags and X-Discourse-Category
custom header to outbound emails containing the tags
and category from the subject, so people on mail clients
that allow advanced filtering (i.e. not Gmail) can filter
mail by tags and category, which is useful for mailing
list mode users
c.f. https://meta.discourse.org/t/headers-for-email-notifications-so-that-gmail-users-can-filter-on-tags/249982/17
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.
When sending emails out via group SMTP, if we
are sending them to non-staged users we want
to mask those emails with BCC, just so we don't
expose them to anyone we shouldn't. Staged users
are ones that have likely only interacted with
support via email, and will likely include other
people who were CC'd on the original email to the
group.
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit changes the default return value of `Auth::ManagedAuthenticator#primary_email_verified?` to false. We're changing the default to force developers to think about email verification when building a new authentication method. All existing authenticators (in core and official plugins) have been updated to explicitly define the `primary_email_verified?` method in their subclass of `Auth::ManagedAuthenticator` (example commit 65f57a4d05).
Internal topic: t/82084.
The `TopicView#bookmarks` method is called by `TopicViewSerializer` and `PostSerializer`
so we want to avoid running a meaningless query when user is not
present.
When loading posts in a topic, the topic level guardian
checks are run multiple times even though all the posts belong to the
same topic. Profiling in production revealed that this accounted for a
significant amount of request time for a user that is not staff or anon.
Therefore, we're optimizing this by adding memoizing the topic level
calls in `PostGuardian`. Speficifally, the result of
`TopicGuardian#can_see_topic?` and `PostGuardian#can_create_post?`
method calls are memoized per topic.
Locally profiling shows a significant improvement for normal users
loading a topic with 100 posts.
Benchmark script command: `ruby script/bench.rb --unicorn --skip-bundle-assets --iterations 100`
Before:
```
topic user:
50: 114
75: 117
90: 122
99: 209
topic.json user:
50: 67
75: 69
90: 72
99: 162
```
After:
```
topic user:
50: 101
75: 104
90: 107
99: 184
topic.json user:
50: 53
75: 53
90: 56
99: 138
```
This commit removes 3 redundant DB queries when loading posts.
1. `@posts` will eventually have to be loaded so we can avoid two
additional queries.
2. No need to preload topic association of posts as we're already
dealing with a fixed topic in `TopicView`.
In some situations (e.g. disaster recovery), it may make sense to spin up a temporary readonly version of a cluster. In that situation, the s3 `expire_missing_assets` job would delete assets which are still in use by the canonical read-write version of the cluster.
To avoid that, this commit will skip deletion if the site is currently in readonly mode.
1. Fix bug where we were not waiting for all unicorn workers to start up
before running benchmarks.
2. Fix a bug where headers were not used when benchmarking. Admin
benchmarks were basically running as anon user.
3. Disable rate limits when in profile env. We're pretty much going to
hit the rate limit every time as a normal user.
4. Benchmark against topic with a fixed posts count of 100. Previously profiling script was just randomly creating posts
and we would benchmark against a topic with a fixed posts count of 30.
Sometimes, the script fails because no topics with a posts count of 30
exists.
5. Benchmarks are not run against a normal user on top of anon and
admin.
6. Add script option to select tests that should be run.
# Context
When a topic is reviewable by a group we give those group moderators some admin abilities including the ability to delete a topic.
# Problem
There are two main problems:
1. Currently when a group moderator deletes a topic they are redirected to root (not the same for staff)
2. Viewing the categories deleted topics (`c/foo/1/?status=deleted`) does not display the deleted topic to the group moderator (not the same for staff).
# Fix
If the `deleted_by` user is part a group that matches the `reviewable_by_group` on a topic then don't redirect. This is the default interaction for staff to give them the ability to do things like restore the topic in case it was accidentally deleted.
To render the deleted topics as expected for the group moderator I am utilizing [the guardian scope of `guardian.can_see_deleted_topics?` for said category](https://github.com/discourse/discourse/pull/19618/files#diff-288e61b8bacdb29d9c2e05b42da6837b0036dcf1867332d977ca7c5e74a44297R802-R803)
Previously, browser logs would be printed to STDOUT halfway through the test run. This commit changes the behaviour so that the logs are included in the failure summary along with other rspec failure information.
There is an issue where chat message processing breaks due to
unhandles `SocketError` exceptions originating in the SSRF check,
specifically in `FinalDestination::Resolver`.
This change gives `FinalDestination::SSRFDetector` a new error class
to wrap the `SocketError` in, and haves the `RetrieveTitle` class
handle that error gracefully.
A few specs in `dashboard_controller_spec.rb` set some state in redis but don't clean it up afterwards which causes other specs to fail when they're ran after `dashboard_controller_spec.rb`.
Related commit: 18467d4.
At the time of writing, this is how the `TopicPosterSerializer` looks
like:
```
class TopicPosterSerializer < ApplicationSerializer
attributes :extras, :description
has_one :user, serializer: PosterSerializer
has_one :primary_group, serializer: PrimaryGroupSerializer
has_one :flair_group, serializer: FlairGroupSerializer
end
```
Within `PosterSerializer`, the `primary_group` and `flair_group`
association is requested on the `user` object. However, the associations
have not been loaded on the `user` object at this point leading to the
N+1 queries problem. One may wonder
why the associations have not been loaded when the `TopicPosterSerializer`
has `has_one :primary_group` and `has_one :flair_group`. It turns out that `TopicPoster`
is just a struct containing the `user`, `primary_group` and
`flair_group` objects. The `primary_group` and `flair_group`
ActiveRecord objects are loaded seperately in `UserLookup` and not preloaded when querying for
the users. This is done for performance reason so that we are able to
load the `primary_group` and `flair_group` records in a single query
without duplication.
We were adding to the resolver's work queue before setting up the `@lookup` and `@parent` information. That could lead to the lookup being performed on the wrong (or `nil`) hostname. This also lead to some flakiness in specs.
This task sometimes fails in CI due to temporary network issues. Retrying twice should help resolve those situations without needing to manually restart the job.
* UX: Wizard Step Enhancements
- Remove illustrations
- Add Emoji graphic to top of steps
- Add description below step title
- Move point of contact to last step
* Move step count to header, plus some button navigation tweaks
* add remaining emoji to step headers
* fix button logic on steps
* Update Point of Contact
* remove automated messages field
* adjust styling for counter, title, and emoji
* Update wording for logos
* Fix tests
* fix prettier
* fix specs
* set same with for steps except for styling screen
* use sentence case; remove duplicate copy under your organization fields
* fix missing buttons on small screens
* add spacing to buttons; adjust font weight to labels
* adjust styling for community logo step; use sentence case for button
* update copy for point of contact text helper
* use sentence case for field labels
* fix ui tests
* use btn-back class to fix ui tests
* reduce bottom margin for toggle fields
* clean up
Co-authored-by: Ella <ella.estigoy@gmail.com>
Follow up to a review in #18937, this commit changes the HashtagAutocompleteService to no longer use class variables to register hashtag data sources or types in context priority order. This is to address multisite concerns, where one site could e.g. have chat disabled and another might not. The filtered plugin registers I added will not be included if the plugin is disabled.
In both ChatMessage#rebake! and in ChatMessageProcessor
when we were calling ChatMessage.cook we were missing the
user_id to cook with, which causes missed hashtag cooks
because of missing permissions.
Previously, restricted category chat channel was available for all groups - even `readonly`. From now on, only user who belong to group with `create_post` or `full` permissions can access that chat channel.
* DEV: Remove enable_whispers site setting
Whispers are enabled as long as there is at least one group allowed to
whisper, see whispers_allowed_groups site setting.
* DEV: Always enable whispers for admins if at least one group is allowed.
We were changing the user's user_option.bookmark_auto_delete_preference
to whatever they changed it to in the bookmark modal to use as default
for future bookmarks. However this was leading to a lot of confusion
since if you wanted to set it for one bookmark you had to remember to
change it back on the next one.
This commit removes that automatic functionality, and instead moves
the bookmark auto delete preference to User Preferences > Interface
in an explicit dropdown.
This commit adds a new notification that gets sent to admins when the site gets new features after an upgrade/deploy. Clicking on the notification takes the admin to the admin dashboard at `/admin` where they can see the new features under the "New Features" section.
Internal topic: t/87166.
This introduces another "section" of queries to the
hashtag autocomplete search, which returns results for
each type that start with the search term. So now results
will be in this order, and within these sections ordered
by the types in priority order:
1. Exact matches sorted by type
2. "starts with" sorted by type
3. Everything else sorted by type then name within type
Depending on the current state of things, sometimes the homepage style
wouldn't update because we were incorrectly blocking updates the
`desktop_category_page_style` site setting if the first item in the top
menu was 'categories'.
Added a test case to handle this situation.
See https://meta.discourse.org/t/248354
When originally introduced, `attempts` was only used in the read-only check
context.
With the introduction of the exponential backoff in cda370db, `attempts` was
also used to count loop iterations, but was left inside the if block instead of
being incremented every loop, meaning the exponential backoff was only
happening when the site was recently readonly.
Co-authored-by: jbrw <jamie.wilson@discourse.org>
* FEATURE: Add chat and sidebar toggles to the setup wizard
- Fix css alighnment
- Add Enable Chat Toggle
- Add Enable Sidebar Toggle
* Check for the chat plugin
* Account for new sidebar step
* update chat and sidebar description
* UI: add checkmark as a visual indicator that it is enabled
* use new navigation_memu site setting for enabling the sidebar
* fix tests
* Add tests
* Update lib/wizard/step_updater.rb
Use HEADER_DROPDOWN instead of LEGACY
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
* Fix spec. Use HEADER_DROPDOWN instead of LEGACY
Co-authored-by: Ella <ella.estigoy@gmail.com>
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
The tsquery used for searching is generated using both functions from
Ruby and Postgresql (for example, unaccent function). Depending on the
term used, it generated an invalid tsquery. For example "can’t"
generated "''can''t''" instead of "''can''''t''".
* FIX: Use Category.secured(guardian) for hashtag datasource
Follow up to comments in #19219, changing the category
hashtag datasource to use Category.secured(guardian) instead
of Site.new(guardian).categories here since the latter does
more work for not much benefit, and the query time is the
same. Also eliminates some Hash -> Model back and forth
busywork. Add some more specs too.
* FIX: Server-side hashtag lookup cooking user loading
When we were using the PrettyText.options.currentUser
and parsing back and forth with JSON for the hashtag
lookups server-side, we had a bug where the user's
secure categories were not loaded since we never actually
loaded a User model from the database, only parsed it
from JSON.
This commit fixes the issue by instead using the
PretyText.options.userId and looking up the user directly
from the database when calling hashtag_lookup via the
PrettyText::Helpers code when cooking server-side. Added
the missing spec to check for this as well.
If configured, this will be used for static JS assets which are stored on S3. This can be useful if you want to use different CDN providers/configuration for Uploads and JS
This commit allows us to type # in the UI and present autocomplete
results immediately with the following logic for the topic composer,
and reversed for the chat composer:
* Categories the user can access and has not muted sorted by `topic_count`
* Tags the user can access and has not muted sorted by `topic_count`
* Chat channels the user is a member of sorted by `messages_count`
So in effect, we allow searching for hashtags without a search term.
To do this we add a new `search_without_term` to each data source so
each one can define how it wants to handle this logic.
This new site setting replaces the
`enable_experimental_sidebar_hamburger` and `enable_sidebar` site
settings as the sidebar feature exits the experimental phase.
Note that we're replacing this without depreciation since the previous
site setting was considered experimental.
Internal Ref: /t/86563
When under extremely high load, it has been observed that updating the categories table when creating a new topic can become a bottleneck.
This change will reduce the two updates to one when a new topic is created within a category, and therefore should help with performance when under extremely high load.
Note that we don't have a database table and a model for post mentions yet, and I decided to implement it without adding one to avoid heavy data migrations. Still, we may want to add such a model later, that would be convenient, we have such a model for mentions in chat.
Note that status appears on all mentions on all posts in a topic except of the case when you just posted a new post, and it appeared on the bottom of the topic. On such posts, status won't be shown immediately for now (you'll need to reload the page to see the status). I'll take care of it in one of the following PRs.
In some cases (e.g. user notification emails) we
are passing an excerpted/stripped version of the
post HTML to Email::Styles, at which point the
<span> elements surrounding the hashtag text have
been stripped. This caused an error when trying to
remove that element to replace the text.
Instead we can just remove all elements inside
a.hashtag-cooked and replace with the raw #hashtag
text which will work in more cases.
Bianca Nenciu
David Taylor
Martin Brennan
Vinoth Kannan
Jan Cernik
Krzysztof Kotlarek
Daniel Waterworth
Alan Guo Xiang Tan
Isaac Janzen
Ted Johansson
Sérgio Saquetim
Régis Hanol
Selase Krakani
Loïc Guitaut
Sam
Osama Sayegh
Justin René Back
Jarek Radosz
Roman Rizzi
Penar Musaraj
Blake Erickson
Rafael dos Santos Silva
Ryan Vandersmith
Gerhard Schlager
Michael Brown
jbrw
Andrei Prigorshnev
