This ensures that elasticsearch doesn't parse it as an object. There are
too many combination of job opts so we don't want elasticsearch to be
parsing and indexing this field as an object.
This improves the `TextSentinel` so that we don't consider CJK text as being uppercase and thus failing the validator.
It also optimizes the entropy computation by using native ruby `.bytes` to get all the bytes from the text.
It also tweaks the `seems_pronounceable?` and `seems_unpretentious?` check to use the `\p{Alnum}` unicode regexp group to account for non-latin languages.
Reference - https://meta.discourse.org/t/body-seems-unclear-error-when-users-are-typing-in-chinese/88715
Inspired by https://github.com/discourse/discourse/pull/27900
Co-authored-by: Paulo Magalhaes <mentalstring@gmail.com>
* FEATURE: Add logging for CustomEmoji
We didn't provide any logs for CustomEmoji before, nor did we record the
person who added any emoji in the database. As a result, the staff had
no way to trace back who added a certain emoji.
This commit adds a new column `user_id` to `custom_emojis` to record the
creator of an emoji. At the same time, a log is added for staff logs to
record who added or deleted a custom emoji.
If a user has a required action, e.g. adding a 2FA method or filling in new required fields, we disable client-side routing except to allowed pages.
This led to a situation where a user might navigate away from e.g. the profile page to look at the new ToS, and then being "stuck" due to not knowing how to get back to accept the new terms.
This PR makes it so that if you click any restricted link, instead of doing nothing we transition the user back to the page where they can take the required action.
User actions can trigger functions that render changes to the screen within the same cycle (e.g. pressing the reply button will cause the login modal to pop up), potentially impacting performance and causing some jank on slower devices.
This change inserts runAfterFramePaint where certain actions are triggered. Below are some screenshots indicating an improved INP for some of the buttons affected on controls with the highest INPs. The two places where this is added help with several actions, e.g. user + group cards, generic button action usage.
When tags contain an underscore we should allow filtering in the same way, previously due to the regex those with underscores were not being found when filtering.
This commit ensures that we reset the `missing_s3_uploads` status count
if there are no inventory files which are at least 2 days older than the
site's restored date.
Otherwise, a site with missing uploads but was subsequntly restored will
be continue to report missing uploads for 2 days.
Followup 560e8aff75
The linked commit allowed oneboxing private GitHub PRs,
issues, commits, and so on, but it didn't actually allow
oneboxing the root repo e.g https://github.com/discourse/discourse-reactions
We didn't have an engine for this, we were relying on OpenGraph
tags on the HTML rendering of the page like we do with other
oneboxes.
To fix this, we needed a new github engine for repos specifically.
Also, this commit adds a `data-github-private-repo` attribute to
PR, issue, and repo onebox HTML so we have an indicator of
whether the repo was private, which can be used for theme components
and so on.
Our old group SMTP SSL option was a checkbox,
but this was not ideal because there are actually
3 different ways SSL can be used when sending
SMTP:
* None
* SSL/TLS
* STARTTLS
We got around this before with specific overrides
for Gmail, but it's not flexible enough and now people
want to use other providers. It's best to be clear,
though it is a technical detail. We provide a way
to test the SMTP settings before saving them so there
should be little chance of messing this up.
This commit also converts GroupEmailSettings to a glimmer
component.
Allow admin to create custom flag which requires an additional message.
I decided to rename the old `custom_flag` into `require_message` as it is more descriptive.
# Context
Currently there is no way to add a custom filter to the experimental `/filter` endpoint. While you can implement a custom `status:` there is no way to include the user's input in a custom query.
# PR
This PR adds the ability to implement a custom filter. eg. `CUSTOM_FILTER:foo`
- Add `add_filter_custom_filter` for extension
- Add specs
This PR introduces FormKit, a component-based form library designed to simplify form creation and management. This library provides a single `Form` component, various field components, controls, validation mechanisms, and customization options. Additionally, it includes helpers to facilitate testing and writing specifications for forms.
1. **Form Component**:
- The main component that encapsulates form logic and structure.
- Yields various utilities like `Field`, `Submit`, `Alert`, etc.
**Example Usage**:
```gjs
import Form from "discourse/form";
<template>
<Form as |form|>
<form.Field
@name="username"
@title="Username"
@validation="required"
as |field|
>
<field.Input />
</form.Field>
<form.Field @name="age" @title="Age" as |field|>
<field.Input @type="number" />
</form.Field>
<form.Submit />
</Form>
</template>
```
2. **Validation**:
- Built-in validation rules such as `required`, `number`, `length`, and `url`.
- Custom validation callbacks for more complex validation logic.
**Example Usage**:
```javascript
validateUsername(name, value, data, { addError }) {
if (data.bar / 2 === value) {
addError(name, "That's not how maths work.");
}
}
```
```hbs
<form.Field @name="username" @validate={{this.validateUsername}} />
```
3. **Customization**:
- Plugin outlets for extending form functionality.
- Styling capabilities through propagated attributes.
- Custom controls with properties provided by `form` and `field`.
**Example Usage**:
```hbs
<Form class="my-form" as |form|>
<form.Field class="my-field" as |field|>
<MyCustomControl id={{field.id}} @onChange={{field.set}} />
</form.Field>
</Form>
```
4. **Helpers for Testing**:
- Test assertions for form and field validation.
**Example usage**:
```javascript
assert.form().hasErrors("the form shows errors");
assert.form().field("foo").hasValue("bar", "user has set the value");
```
- Helper for interacting with he form
**Example usage**:
```javascript
await formKit().field("foo").fillIn("bar");
```
5. **Page Object for System Specs**:
- Page objects for interacting with forms in system specs.
- Methods for submitting forms, checking alerts, and interacting with fields.
**Example Usage**:
```ruby
form = PageObjects::Components::FormKit.new(".my-form")
form.submit
expect(form).to have_an_alert("message")
```
**Field Interactions**:
```ruby
field = form.field("foo")
expect(field).to have_value("bar")
field.fill_in("bar")
```
6. **Collections handling**:
- A specific component to handle array of objects
**Example Usage**:
```gjs
<Form @data={{hash foo=(array (hash bar=1) (hash bar=2))}} as |form|>
<form.Collection @name="foo" as |collection|>
<collection.Field @name="bar" @title="Bar" as |field|>
<field.Input />
</collection.Field>
</form.Collection>
</Form>
```
Previously, we did not log any topic slow mode changes. This allowed
some malicious (or just careless) TL4 users to delete slow modes created
by moderators at will. Administrators could not see who changed the slow
mode unless they had SQL knowledge and used Data Explorer.
This commit enables logging who turns slow mode on, off, or changes it.
Related meta topic: https://meta.discourse.org/t/why-is-there-no-record-of-who-added-or-removed-slow-mode/316354
Followup 7b627dc14b
In this other commit, I changed the email settings validator
to always use the `login` authentication method for
office365 and outlook, but I didn't change the actual
group SMTP mailer to do this.
This commit fixes that issue and does some minor refactoring.
This commit updates `TopicQuery.validators` to cover all of the
public options listed in `TopicQuery.public_valid_options`. This is done
to fix the app returning a 500 response code when an invalid value, such
as a hash, is passed as a query param when accessing the various topic
list routes.
Originally in 964da21817
we hid the SMTPAuthenticationError message except in
very specific cases. However this message often contains
helpful information from the mail provider, for example
here is a response from Office365:
> 535 5.7.139 Authentication unsuccessful, user is locked by your
organization's security defaults policy. Contact your administrator.
So, we will show the error message in the modal UI instead
of supressing it with a generic message to be more helpful.
The watch words controller creation function, create_or_update_word(), doesn’t validate the size of the replacement parameter, unlike the word parameter, when creating a replace watched word. So anyone with moderator privileges can create watched words with almost unlimited characters.
This commit updates `StaticController#enter` to not redirect to invalid
paths when the `redirect` param is set. Instead it should redirect to `/` when the
`redirect` param is invalid.
Followup 560e8aff75
GitHub auth tokens cannot be made with permissions to
access multiple organisations. This is quite limiting.
This commit changes the site setting to be a "secret list"
type, which allows for a key/value mapping where the value
is treated like a password in the UI.
Now when a GitHub URL is requested for oneboxing, the
org name from the URL is used to determine which token
to use for the request.
Just in case anyone used the old site setting already,
there is a migration to create a `default` entry
with that token in the new list setting, and for
a period of time we will consider that token valid to
use for all GitHub oneboxes as well.
### What is the problem?
We have recently added a new option to add user fields required for existing users. This is in contrast to requiring fields only on sign-up.
This revealed an existing problem. Consider the following:
1. User A signs up.
2. Admin adds a new user field required on sign-up. (Should not apply to User A since they already signed up.)
3. User A tries to update their profile.
**Expected behaviour:**
No problem.
**Actual behaviour:**
User A receives an error saying they didn't fill up all required fields.
### How does this fix it?
When updating profile, we only check that required fields that are "for all users" are filled. Additionally, we check that fields that were required on sign-up and have previously been filled are not blanked out.
Allow admin to create custom flag which requires an additional message.
I decided to rename the old `custom_flag` into `require_message` as it is more descriptive.
Currently, when a plugin registers a new reviewable type or extends a
list method (through `register_reviewble_type` and `extend_list_method`
respectively), the new array is statically computed and always returns
the same value. It will continue to return the same value even if the
plugin is disabled (it can be a problem in a multisite env too).
To address this issue, this patch changes how `extend_list_method`
works. It’s now using `DiscoursePluginRegistry.define_filtered_register`
to create a register on the fly and store the extra values from various
plugins. It then combines the original values with the ones from the
registry. The registry is already aware of disabled plugins, so when a
plugin is disabled, its registered values won’t be returned.
This commit improves the logging of Sidekiq errors when
`ENABLE_LOGSTASH_LOGGER` is set to 1. Prior to this change, we would
only log the message and the backtrace. After this change, useful
information like `job.class`, `job.opts`, `job.problem_db`,
`exception.class` and `exception.message` are included in the log line
as well.
Both office365 and outlook SMTP servers need LOGIN
SMTP authentication instead of PLAIN (which is what
we are using by default). This commit uses that
unconditionally for these servers, and also makes
sure to use STARTTLS for them too.
Before Rails 7.1, the `config.i18n.raise_on_missing_translations` option
was raising only in controllers and views, now it’s anywhere in the app.
It means it raises each time `#description` is called for a setting that
is missing a proper description (and we have a ton of them). Most of the
time it’s fine, as those are usually settings that aren’t shown to the
user.
We can’t just let the code blow up every time there’s a setting with a
missing description, that’s why it’s currently returning an empty
string when the translation is missing.
However, this silently broke our I18n integrity spec that was relying on
the old “Translation missing” message to detect missing translations.
This patch addresses this issue by checking the description isn’t an
empty string. It caught a missing translation by the way.
This change how we present attachments from incoming emails to now be "hidden" in a "[details]" so they don't "hang" at the end of the post.
This is especially useful when using Discourse as a support tool where email is the main communication channel. For various reasons, images are often duplicated by email user agents, and hiding them behind the details block help keep the conversation focused on the isssue at hand.
Internal ref t/122333
This patch upgrades the MessageFormat library to version 3.3.0 from
0.1.5.
Our `I18n.messageFormat` method signature is unchanged, and now uses the
new API under the hood.
We don’t need dedicated locale files for handling pluralization rules
anymore as everything is now included by the library itself.
The compilation of the messages now happens through our
`messageformat-wrapper` gem. It then outputs an ES module that includes
all its needed dependencies.
Most of the changes happen in `JsLocaleHelper` and in the `ExtraLocales`
controller.
A new method called `.output_MF` has been introduced in
`JsLocaleHelper`. It handles all the fetching, compiling and
transpiling to generate the proper MF messages in JS. Overrides and
fallbacks are also handled directly in this method.
The other main change is that now the MF translations are served through
the `ExtraLocales` controller instead of being statically compiled in a
JS file, then having to patch the messages using overrides and
fallbacks. Now the MF translations are just another bundle that is
created on the fly and cached by the client.
Drafts used to be deleted instead of being destroyed. The callbacks that
clean up the upload references were not being called. As a result, the
upload references were not cleaned up and uploads were not deleted
either. This has been partially fixed in 9655bf3e.
This commit updates `DiscourseLogstashLogger#add_with_opts` to avoid
logging messages that matches regexp patterns configured in
`Logster.store.ignore`. Those error logs are mostly triggered by clients
and do not serve any useful purpose.
This commit updates `DiscourseLogstashlogger` to add the
`exception_class` and `exception_message` field to the log line when the
`progname` of the log message is `web-exception` which is Logster's
logging of exceptions during a web request.
The `exception_class` and `exception_message` fields allows consumers of
the logs to easily group logs together.
This commit adds the ability to onebox private GitHub
commits, pull requests, issues, blobs, and actions using
a new `github_onebox_access_token` site setting. The token
must be set up in correctly to have access to the repos needed.
To do this successfully with the Oneboxer, we need to skip
redirects on the github.com host, otherwise we get a 404
on the URL before it is translated into a GitHub API URL
and has the appropriate headers added.
Handles the cases where the sections titles are Unicode only strings, allowing them to be expanded separately if the Unicode string contains letters.
Also prevents a sidebar section with the header hidden to be displayed collapsed.
Followup 3ff7ce78e7
Basing this setting on referrer was too brittle --
the referrer header can easily be ommitted or changed.
Instead, for the small amount of use cases that this
site setting serves, we can use a group-based setting
instead, changing it to `cross_origin_opener_unsafe_none_groups`
instead.
When a crawler visits a topic that has a deleted author, it would error because the `show.html.erb` view was expecting a user to be always present.
This ensure we don't render the "author" meta data when the author of the topic has been deleted.
Internal ref t/132508
This change eliminates a couple of instances where subfolder urls are badly formatted, in most cases we can use Discourse.base_url_no_prefix to prevent adding the subfolder to the base url.
Adds a report to show the top 100 most viewed topics in a date range,
combining logged in and anonymous views. Can be filtered by category.
This is a followup to 527f02e99f
and d1191b7f5f. We are also going to
be able to see this data in a new topic map, but this admin report
helps to see an overview across the forum for a date range.
This commit adds a hidden `s3_inventory_bucket_region` site setting to
specify the region of the `s3_inventory_bucket` when the `S3Inventory`
class initializes an instance of the `S3Helper`. By default, the
`S3Helper` class uses the value of the `s3_region` site setting but the
region of the `s3_inventory_bucket` is not always the same as the
`s3_region` configured.
Background:
In order to redrive failed webhook events, an operator has to go through and click on each. This PR is adding a mechanism to retry all failed events to help resolve issues quickly once the underlying failure has been resolved.
What is the change?:
Previously, we had to redeliver each webhook event. This merge is adding a 'Redeliver Failed' button next to the webhook event filter to redeliver all failed events. If there is no failed webhook events to redeliver, 'Redeliver Failed' gets disabled. If you click it, a window pops up to confirm the operator. Failed webhook events will be added to the queue and webhook event list will show the redelivering progress. Every minute, a job will be ran to go through 20 events to redeliver. Every hour, a job will cleanup the redelivering events which have been stored more than 8 hours.
This is a follow up to 005f623c42 where
we want to truncate the user agent string instead of nulling out the
column when the user agent string is too low. By truncating, we still
get to retain information that can still be useful.
Adds experimental_flags_admin_page_enabled_groups (default "")
to remove the Moderation Flags link from the admin sidebar for now,
there are still a few bugfixes that need to be done before we
are comfortable with turning this on more widely. This is
a _temporary_ flag, we will be removing this once the feature
is more stable.
Add a new column - `user_agent` - to the `SearchLog` table.
This column can be null as we are only allowing a the user-agent string to have a max length of 2000 characters. In the case the user-agent string surpasses the max characters allowed, we simply nullify the value, and save/write the log as normal.
The user serializer groups method previously relied on the members_visible_groups to determine groups that the user should be able to see, however this setting was intended for visibility of group members (which is entirely different).
The result of this could be seen when choosing a primary group from user preferences -> account, due to the serializer the group name was not visible when members_visible_groups was set to owners.
In our official Docker image, running git commands results in the
following error:
```
fatal: detected dubious ownership in repository at '/var/www/discourse'
To add an exception for this directory, call:
git config --global --add safe.directory /var/www/discourse
```
This commit rewrites `DiscourseLogstashLogger` to not be an instance
of `LogstashLogger`. The reason we don't want it to be an instance of
`LogstashLogger` is because we want the new logger to be chained to
Logster's logger which can then pass down useful information like the
request's env and error backtraces which Logster has already gathered.
Note that this commit does not bother to maintain backwards
compatibility and drops the `LOGSTASH_URI` and `UNICORN_LOGSTASH_URI`
ENV variables which were previously used to configure the destination in
which `logstash-logger` would send the logs to. Instead, we introduce
the `ENABLE_LOGSTASH_LOGGER` ENV variable to replace both ENV and remove
the need for the log paths to be specified. Note that the previous
feature was considered experimental as stated in d888d3c54c
and the new feature should be considered experimental as well. The code
may be moved into a plugin in the future.
Instead of the deprecated 'min trust level to allow ignore' in order to reduce the number of deprecation notices in the logs.
This tweaks a few serializers so that the 'can_ignore_users?` property is always coming from the server and properly used on the client-side.
SiteSetting.hide_user_profiles_from_public raises a Forbidden, which disallows our after_action: add no index header from triggering.
This fix makes sure that the no index header gets added via before_action instead
When tag preference in group and site settings are both used with same default notification level it will break new users signups because it tries to create duplicate records in the tag_users table which can’t happen because we have a unique index set.
If an existing user (John) accepts an invite created by Kenny to a group, John may be seen as invited by Kenny, despite already having an account on the site.
This fix removes the bug by excluding invites that determine the invited_by after the user's creation date. The delay buffer in the query accounts for invites that also create the user at the same time.
Badges can have their associated image uploads deleted. When this happens, any user who has that badge will have their profile page error out.
After this fix, when deleting an upload that's associated with a badge, we nullify the foreign key ID on the badge. This makes the existing safeguard work correctly.
There's currently a race condition in the following spec:
65be7a7880/spec/system/admin_about_config_area_spec.rb (L70-L95)
where the form can be saved before the image uploader field has finished uploading the selected image and causing the assertion at line 94 to fail with the following error:
```
Failure/Error: expect(SiteSetting.about_banner_image.sha1).to eq(Upload.generate_digest(image_file))
NoMethodError:
undefined method `sha1' for nil
[Screenshot Image]: /__w/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_admin_about_config_area_page_the_general_settings_card_can_saves_its_fields_to_their_corresponding_site_settings_312.png
~~~~~~~ JS LOGS ~~~~~~~
http://localhost:31338/assets/vendor.js 15902:14 "WARNING: uppy needs a unique id, pass one in to the component implementing this mixin"
~~~~~ END JS LOGS ~~~~~
./spec/system/admin_about_config_area_spec.rb:94:in `block (3 levels) in <main>'
./spec/rails_helper.rb:552:in `block (3 levels) in <top (required)>'
./spec/rails_helper.rb:552:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:513:in `block (3 levels) in <top (required)>'
./spec/rails_helper.rb:503:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:460:in `block (2 levels) in <top (required)>'
./vendor/bundle/ruby/3.3.0/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
This PR fixes the problem by making the system test wait for the image to finish uploading (with 10 seconds timeout) before carrying out the rest of the system test.
Followup 2f2da72747
When the "Consolidated Pageviews with Browser Detection (Experimental)"
report was introduced, we started counting the original
"page_view_logged_in" and "page_view_anon" ApplicationRequest
data as "Other Pageviews", subtracting
"page_view_anon_browser" and "page_view_logged_in_browser" from
this number.
However we unknowingly automatically started counting these
browser-based page views, which are a subset of the total
"page_view_logged_in" and "page_view_anon" counts, in the
original "Pageviews" report, leading to double counting
which meant that when you looked at the data for each
report side-by-side the data didn't add up.
This commit fixes the issue by not counting the "browser"
pageviews in the Pageviews report, and making the code where
we were only counting certain types of requests for this
report more plain, explicitly stating which types of requests
we want.
When a topic embed is run with either no tags argument or a nil tag argument
this should not affect any existing tags.
Only update topic tags when tags argument is explicitly empty.
Before checking if flags were reordered on the topic page, we need to ensure that the reorder action was finished. To achieve it "saving" CSS is added and removed when AJAX call is completed.
Followup 2f2da72747
This commit moves topic view tracking from happening
every time a Topic is requested, which is susceptible
to inflating numbers of views from web crawlers, to
our request tracker middleware.
In this new location, topic views are only tracked when
the following headers are sent:
* HTTP_DISCOURSE_TRACK_VIEW - This is sent on every page navigation when
clicking around the ember app. We count these as browser page views
because we know it comes from the AJAX call in our app. The topic ID
is extracted from HTTP_DISCOURSE_TRACK_VIEW_TOPIC_ID
* HTTP_DISCOURSE_DEFERRED_TRACK_VIEW - Sent when MessageBus initializes
after first loading the page to count the initial page load view. The
topic ID is extracted from HTTP_DISCOURSE_DEFERRED_TRACK_VIEW.
This will bring topic views more in line with the change we
made to page views in the referenced commit and result in
more realistic topic view counts.
By default, secure sessions expire after 1 hour.
For OAuth authentication it should expire at the same time when the authentication cookie expires - `SiteSetting.maximum_session_age.hours`.
It is possible that the forum will not have persistent sessions, based on `persistent_sessions` site setting. In that case, with next username and password authentication we need to reset information about OAuth.
Bug introduced in this PR - https://github.com/discourse/discourse/pull/27547
* FIX: Division by zero error on WebHookEventsDailyAggregate
* DEV: Update implementation of WebHookEventsDailyAggregate to handle division by zero error
I am changing many of these to notes or resolving them as is,
most of these I have not actively worked on in years so someone
else can work on them when we get to these areas again.
This commit continues work laid out by ffec8163b0 for the admin config page for the /about page. The last commit set up the user interface, and this one sets up all the wiring needed to make the input fields and save buttons actually work.
Internal topic: t/128544.
While creating a new category if the user didn't specify a value for `minimum_required_tags` input but clicked it then it returned the "PG::NotNullViolation: null value in column 'minimum_required_tags'" error.
When bad data is provided in the URI for redirecting to a category,
Rails raises an `ActionController::Redirecting::UnsafeRedirectError`
error, leading to a 500 error.
This patch catches the exception to render a 404 instead.
Currently redirecting to an external URL through a permalink doesn’t
work because Rails raises a
`ActionController::Redirecting::UnsafeRedirectError` error.
This wasn’t the case before we upgraded to Rails 7.0.
This patch fixes the issue by using `allow_other_host: true` on the
redirect.
If, for whatever reasons, the user's locale is "blank" and an admin is accepting their group membership request, there will be an error because we're generating posts with the locale of recipient.
In order to fix this, we now use the `user.effective_locale` which takes care of multiple things, including returning the default locale when the user's locale is blank.
Internal ref - t/132347
When using the full page search and filtering down to a specific topic, the sort order was overwritten to by by "post_number".
This was confusing because we allow different type of sort order in the full search page.
This fixes it by only sorting by post_number when there's no "global" sort order defined.
Since the "new topic map" uses the search endpoint behind the scene, this also fixes the "most likes" popup.
Context - https://meta.discourse.org/t/searching-order-seems-to-be-broken-when-searching-in-topic/312303
* DEV: add db migration to filter out invalid csp script source values
* DEV: insert UserHistory row during data migration to track old value for content_security_policy_script_src site setting
When visiting a user profile, and then opening the search, there's an option to filter down by posts made by that user.
When clicking that option, it used to pre-fill the "search bar" with "@<username>" to filter down the search.
This restore this behaviour and add a system spec to ensure it doesn't regress.
Context - https://meta.discourse.org/t/in-posts-by-search-option-does-not-work-when-clicked/312916
We want to allow admins to make new required fields apply to existing users. In order for this to work we need to have a way to make those users fill up the fields on their next page load. This is very similar to how adding a 2FA requirement post-fact works. Users will be redirected to a page where they can fill up the remaining required fields, and until they do that they won't be able to do anything else.
Followup 96a0781bc1
When sending emails where secure uploads is enabled
and secure_uploads_allow_embed_images_in_emails is
true, we attach the images to the email, and we
do some munging with the final email so the structure
of the MIME parts looks like this:
```
multipart/mixed
multipart/alternative
text/plain
text/html
image/jpeg
image/jpeg
image/png
```
However, we were not specifying the `boundary` of the
`multipart/mixed` main content-type of the email, so
sometimes the email would come through appearing to
have an empty body with the entire thing attached as
one attachment, and some mail parsers considered the
entire email as the "epilogue" and/or "preamble".
This commit fixes the issue by specifying the boundary
in the content-type header per https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html
Adds a checkbox to filter untranslated text strings in the admin UI, behind a hidden and default `false` site setting `admin_allow_filter_untranslated_text`.
Followup to 0e1102b332
Minor followup, makes the condition check against the
boolean val, see the difference here:
```ruby
!SiteSetting.enforce_second_factor_on_external_auth && "true"
=> "true"
```
vs:
```ruby
!SiteSetting.enforce_second_factor_on_external_auth && "true" == "true"
=> true
```
* Removed the link from the title, so the settings can only be accessed via the settings button on the right
* Added an icon to the "Learn more" link to indicate that it opens a new window
* Made various styling adjustments
* DEV: Upgrade Rails to 7.1
* FIX: Remove references to `Rails.logger.chained`
`Rails.logger.chained` was provided by Logster before Rails 7.1
introduced their broadcast logger. Now all the loggers are added to
`Rails.logger.broadcasts`.
Some code in our initializers was still using `chained` instead of
`broadcasts`.
* DEV: Make parameters optional to all FakeLogger methods
* FIX: Set `override_level` on Logster loggers (#27519)
A followup to f595d599dd
* FIX: Don’t duplicate Rack response
---------
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
In some instances, the `modifications` of `tags` hasn't been properly serialized as a Ruby array but rather as a string (I've seen `""`, `"[]"`, and `"[\"\"]"`).
This generates an error when we try to `filter_tags` and remove `hidden_tags` (which is an array) from `tags` which might be a string.
Internal ref - t/131126
I wasn't able to figure out the root cause of this so I reverted the behavior that was introduced ~6 years ago in f2c060bdf2
This ensures that the theme id is resolved as early as possible in the
request cycle. This is necessary for the custom homepage to skip
preloading the wrong data.
Previously filter route was not setting topic list, this meant that
keyboard navigation using "G" "J" was not functioning.
This amends it by ensuring the list is set after looking up the model.
* DEV: Upgrade Rails to 7.1
* FIX: Remove references to `Rails.logger.chained`
`Rails.logger.chained` was provided by Logster before Rails 7.1
introduced their broadcast logger. Now all the loggers are added to
`Rails.logger.broadcasts`.
Some code in our initializers was still using `chained` instead of
`broadcasts`.
* DEV: Make parameters optional to all FakeLogger methods
* FIX: Set `override_level` on Logster loggers (#27519)
A followup to f595d599dd
* FIX: Don’t duplicate Rack response
---------
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
In this PR we introduced `enforce_second_factor_on_external_auth` setting https://github.com/discourse/discourse/pull/27506
When it is set to false and the user is authenticated via OAuth, then we should not enforce the 2fa configuration.
We recently fixed a problem where secure upload images weren't re-attached when sending the activity summary e-mail.
This fix contained a bug that would lead to n copies of the e-mail body being included, n being the number of duplicates. This is because #fix_parts_after_attachments! was called once per attachment, and adding more parts to the multipart e-mail.
This PR fixes that by:
Adding a failing test case for the above.
Moving the looping over multiple posts into #fix_parts_after_attachments! itself.
When a post is cooked the links are extracted and `TopicLink` instances
are created for each of them. These links are used in various places,
including the topic view, user summary page, etc.
In previous commit 48e5d1a, hotlinked images from Oneboxes have been
ignored from the texts, but hotlinked images turned into Lightboxes
were still extracted.
The test that checks that securely uploaded images are re-attached to the digest e-mail wasn't rendering the actual digest e-mail template. This change fixes that.
Many site settings can be distructive or have huge side-effects
for a site that the admin may not be aware of when changing it.
This commit introduces a `requires_confirmation` attribute that
can be added to any site setting. When it is true, a confirmation
dialog will open if that setting is changed in the admin UI,
optionally with a custom message that is defined in client.en.yml.
If the admin does not confirm, we reset the setting to its previous
clean value and do not save the new value.
Some tooling may rely on an unsafe-none cross origin opener policy to work. This change adds a hidden site setting that can be used to list referrers where we add this header instead of the default one configured in cross_origin_opener_policy_header.
For Topic Embeds, we would prefer <article> to be the main article in a topic, rather than a table cell <td> with potentially a lot of data. However, in an example URL like here, the table cell (the very large code snippet) is seen as the Topic Embed's article due to the determined content weight by the Readability library we use.
In the newly released 0.7.1 cantino/ruby-readability#94, the library has a new option to exclude the library's default <td> element into content weighting. This is more in line with the original library where they only weighted <p>. So this PR excludes the td, as seen in the tests, to allow the actual article to be seen as the article. This PR also adds the details tag into the allow-list.
Followup 6b872c4c53
Even though we were showing a validation error for a reject
reason that was too long, we were still sending an email and
doing other operations on the user which we are rejecting.
This commit fixes this by validating the reviewable model
before attempting to do anything else after the reason is set.
A new admin setting called `enforce_second_factor_on_external_auth`. It allows users to authenticate using external providers even when 2FA is forced with `enforce_second_factor` site setting.
* Revert "FIX: Set `override_level` on Logster loggers (#27519)"
This reverts commit c1b0488c54.
* Revert "DEV: Make parameters optional to all FakeLogger methods"
This reverts commit 3318dad7b4.
* Revert "FIX: Remove references to `Rails.logger.chained`"
This reverts commit f595d599dd.
* Revert "DEV: Upgrade Rails to 7.1"
This reverts commit 081b00391e.
`Rails.logger.chained` was provided by Logster before Rails 7.1
introduced their broadcast logger. Now all the loggers are added to
`Rails.logger.broadcasts`.
Some code in our initializers was still using `chained` instead of
`broadcasts`.
Currently when a cache entry is corrupt, we log the event without doing
anything else. It means the cache is still corrupt, and the proper value
isn’t computed again.
Normally, it’s very rare the cache becomes corrupt, but it can happen
when upgrading Rails for example and the cache format changes. This is
normally handled automatically by Rails but since we’re using a custom
cache class, we have to do it ourselves.
This patch takes the same approach the Rails team did, when a cache
entry is corrupt, we treat it as a miss, recomputing the proper value
and caching it in the new format.
Wasn't quite handling the cases where a closing bracket `]` was used in the value of one of the attributes.
```markdown
[chat quote=user channel="[broken]"]
```
Would not be correctly parsed because we would _greedily_ use the first `]` as the end of the tag even though it might be a valid character when inside proper quotes.
c39a4de139/app/assets/javascripts/discourse-markdown-it/src/features/bbcode-block.js (L62)
Re-wrote the `parseBBCodeTag` to properly handle the following cases
- A closing tag (aka `[/name]`) which are easy since they don't have any attributes
- An old `[quote=...]` format we used that doesn't uses quotes but still has various attributes of the form `key:value`
- All three valid BBCode opening tag formats we support
- `[name]` without any attributes
- `[name=foo]` with a default value
- `[name foo=bar]` with some attributes
Ended up having to fix/rewrite the few bbcode rules that were using the `parseBBCodeTag` function, namely `d-wrap` and `discourse-local-dates`.
While working on this, I think I also found a way to get rid the of shims we had in place so that plugins could use the `parseBBCodeTag` function.
Reference - https://meta.discourse.org/t/having-a-right-bracket-in-a-channel-name-breaks-all-quotes-from-that-channel/308439
* Load search results in displayed order so that when more categories are loaded on scroll, they appear at the end,
* Limit the number of subcategories that are shown per category and display 'show more' links,
This commit adds ability to fetch a subset of site settings from the `/admin/site_settings` endpoint so that it can be used in all places where the client app needs access to a subset of the site settings.
Additionally, this commit also introduces a new service class called `UpdateSiteSetting` that encapsulates all the logic that surrounds updating a site setting so that it can be used to update site setting(s) anywhere in the backend. This service comes in handy with, for example, the controller for the flags admin config area which may need to update some site settings related to flags.
Internal topic: t/130713.
This commits introduces the `sidekiq_report_long_running_jobs_minutes`
global setting which allows a site administrator to log a warning in the
Rails log when a Sidekiq job has been running for too long.
The warning is logged with the backtrace of the thread that is
processing the Sidekiq job to make it easier to figure out what a
sidekiq job is stuck on.
When we turn on settings automatically for customers,
we sometimes use `.set_and_log` which will make a staff
action log for the site setting change. This is fine, but
there is no context for customers.
This change allows setting a message with `.set_and_log`, which
will be stored in the `details` column of the staff action log
created, which will show up on `/admin/logs/staff_action_logs`
---------
Co-authored-by: Kelv <kelv@discourse.org>
In #26642 we introduced a change that re-attaches securely uploaded images in the digest e-mail. However, this change assumed that the type argument to the Email::Sender constructor would be a symbol, but when it is coming from the UserEmail job it is a string. This PR fixes that.
This introduces the syntax of
`category:a,b,c` which will search across multiple categories.
Previously there was no way to allow search across a wide selection of
categories.
After working on the Webhook events filter by Status, I noticed that the 'Delivered' and 'Failed' options do not take the status param when loading more than fifty Webhook events. It causes to load all Webhook events regardless of its status after the first load.
This PR is adding webhook events status for the filter to the param when loading more than fifty Webhook events.
This commit tries another work around for the `Socket::ResolutionError: getaddrinfo: Temporary failure in name resolution`
error we are seeing on CI.
The problem with the previous workaround is that `Capybara.using_session` will attempt to resolve `localhost`
before yielding the block which means our retry code is not hit.
This problem may be related to https://bugs.ruby-lang.org/issues/20172
which hints at us potentially not being able to spin up threads on CI so
I'm adding a debugging statement when stuff fails.
decorator-transforms (https://github.com/ef4/decorator-transforms) is a modern replacement for babel's plugin-proposal-decorators. It provides a decorator implementation using modern browser features, without needing to enable babel's full suite of class feature transformations. This improves the developer experience and performance.
In local testing with Google's 'tachometer' tool, this reduces Discourse's 'init-to-render' time by around 3-4% (230ms -> 222ms).
It reduces our initial gzip'd JS payloads by 3.2% (2.43MB -> 2.35MB), or 7.5% (14.5MB -> 13.4MB) uncompressed.
This was previously reverted in 97847f6. This version includes a babel transformation which works around the bug in Safari <= 15.
For Cloudflare compatibility issues, check https://meta.discourse.org/t/311390
Not all HTML elements are converted into Markdown. Some are kept as HTML.
Without this fix XML/HTML entities that are formatted as text instead of code are swallowed by Discourse.
This also fixes quotes in the `title` attribute of the `<abbr>` tag.
Previously `HtmlToMarkdown` always converted HTML tables into Markdown tables. That lead to some badly formatted Markdown tables, e.g. when the table contained `rowspan` or `colspan`. This solves the issue by using very basic HTML tables in those cases.
When running ` rspec spec/services/external_upload_manager_spec.rb`
in the development environment, tests were failing with the following
error:
```
NameError:
uninitialized constant FakeS3::Aws
```
This commit introduces a hidden `s3_inventory_bucket` site setting which
replaces the `enable_s3_inventory` and `s3_configure_inventory_policy`
site setting.
The reason `enable_s3_inventory` and `s3_configure_inventory_policy`
site settings are removed is because this feature has technically been
broken since it was introduced. When the `enable_s3_inventory` feature
is turned on, the app will because configure a daily inventory policy for the
`s3_upload_bucket` bucket and store the inventories under a prefix in
the bucket. The problem here is that once the inventories are created,
there is nothing cleaning up all these inventories so whoever that has
enabled this feature would have been paying the cost of storing a whole
bunch of inventory files which are never used. Given that we have not
received any complains about inventory files inflating S3 storage costs,
we think that it is very likely that this feature is no longer being
used and we are looking to drop support for this feature in the not too
distance future.
For now, we will still support a hidden `s3_inventory_bucket` site
setting which site administrators can configure via the
`DISCOURSE_S3_INVENTORY_BUCKET` env.
This is a follow up to 9ff0805a1d. We
noticed that `localhost` can fail to resolve in other spots of the app
and not just in selenium-webdriver.
From the failing tests we have seen, the `getaddrinfo: Temporary failure in name resolution` error is only
seen from within the `Capybara.using_session` block. This commit aims to
ensure that `localhost` can be resolve after the new session is started.
* FEATURE: Add Filter for Webhook Events by Status
* Fixing multiple issues
* Lint
* Fixing multiple issues
* Change the range of the status for webhook events
We want to get rid of the old topic bulk actions modal
and use the new dropdown (currently gated behind
experimental_topic_bulk_actions_enabled_groups). To do
this we need to use the new dropdown in all places in the
UI.
This commit changes the full page search UI to use the new
topic bulk actions dropdown if experimental_topic_bulk_actions_enabled_groups
is enabled, and makes some minor refactors to make this work.
Also add a spec for both the old and new functionality.
This commit fixes a problem where the user will not be able to reset
their password when they only have security keys and backup codes
configured.
This commit also makes the following changes/fixes:
1. Splits password reset system tests to
`spec/system/forgot_password_spec.rb` instead of missing the system
tests in `spec/system/login_spec.rb` which is mainly used to test
the login flow.
2. Fixes a UX issue where the `Use backup codes` or `Use authenticator
app` text is shown on the reset password form when the user does
not have either backup codes or an authenticator app configured.
The mistake was made when flags were moved to the database. The `notify_moderators` (something else) flag should be the last position on the list.
This commit contains 3 changes:
- update fixtures order;
- remove position and enable from fixtures (they can be overridden by admin and we don't want seed to restore them);
- migration to fix data if the order was not changed by admin.
This commit updates the `UserPasswordExpirer.expire_user_password`
method to update `UserPassword#password_expired_at` when an existing
`UserPassword` record exists with the same `password_salt`,
`password_hash` and `password_algorithm`. This is to prevent the unique
validation error on `UserPassword#user_id` and
`UserPassword#password_hash` from being raised when the method is called
twice for a user that has not changed its password.
This commit includes various UX improvements to the reset
password page:
* Introduce a `hide-application-header-buttons` helper to do the following:
* Hide Sign Up and Log In buttons, they are not necessary on this flow
* Hide the sidebar, it is a distraction on this flow
* Improve messaging when a 2FA confirmation is required first
* Improve display of server-side ActiveRecord model validation errors
in password form, e.g. instead of "is the same as your current password"
we do "The password is the same as your current password"
* Move password tip to next line below input and move caps lock hint
inline with Show/Hide password toggle
* Add system specs for 2FA flow on reset password page
* Fixes a computed property conflict issue on the password reset
page when toggling 2FA methods
Continued work on moderate flags UI.
In this PR admins are allowed to change the order of flags. The notify user flag is always on top but all other flags can be moved.
On Github Actions, system tests which uses `Capybara#using_session` are
failing intermittently with the error "Socket::ResolutionError: getaddrinfo: Temporary failure in name resolution"
when `Selenium::WebDriver::Platform.localhost` tries to resolve
`localhost`.
Too much time has been spent trying to figure out why so we are giving
up here and just retrying the resolution of `localhost` on Github
Actions.
This makes it more obvious what's happening, and makes it much less likely that users will send repeated reset emails (and thereby hit the rate limit)
Followup to e97ef7e9af
This commit adds the ability for site administrators to mark users'
passwords as expired. Note that this commit does not add any client side
interface to mark a user's password as expired.
The following changes are introduced in this commit:
1. Adds a `user_passwords` table and `UserPassword` model. While the
`user_passwords` table is currently used to only store expired
passwords, it will be used in the future to store a user's current
password as well.
2. Adds a `UserPasswordExpirer.expire_user_password` method which can
be used from the Rails console to mark a user's password as expired.
3. Updates `SessionsController#create` to check that the user's current
password has not been marked as expired after confirming the
password. If the password is determined to be expired based on the
existence of a `UserPassword` record with the `password_expired_at`
column set, we will not log the user in and will display a password
expired notice. A forgot password email is automatically send out to
the user as well.
Even when the admin sidebar sections are collapsed, they should expand while filtering. When the filter is removed, sections should go back to the previous state.
In addition, trim whitespace from the filter section.
This commit re-introduces the "Move to Inbox" and "Move to Archive"
bulk topic actions, which we had in the old modal but had not yet added
to the new "experimental" dropdown, which isn't really experimental at
this point.
Once this is merged we can remove the old modal and only
rely on the new dropdown.
Prior to this fix we were opening a modal before closing the `DMenu` modal, given `DModal` expects only one modal at a time it was closing the latest modal and instantly closing the one we just opened.
This commit introduces the following changes which allows a site
administrator to mark `Upload` records with the `s3_file_missing`
verification status which will result in the `Upload` record being ignored when
`Discourse.store.list_missing_uploads` is ran on a site where S3 uploads
are enabled and `SiteSetting.enable_s3_inventory` is set to `true`.
1. Introduce `s3_file_missing` to `Upload.verification_statuses`
2. Introduce `Upload.mark_invalid_s3_uploads_as_missing` which updates
`Upload#verification_status` of all `Upload` records from `invalid_etag` to `s3_file_missing`.
3. Introduce `rake uploads:mark_invalid_s3_uploads_as_missing` Rake task
which allows a site administrator to change `Upload` records with
`invalid_etag` verification status to the `s3_file_missing`
verificaton_status.
4. Update `S3Inventory` to ignore `Upload` records with the
`s3_file_missing` verification status.
When uploading a video, the composer will now show a thumbnail image in
the composer preview instead of just the video placeholder image.
If `enable_diffhtml_preview` is enabled the video will be rendered in
the composer preview and is playable.
Introduced back in 2022 in
e3d495850d,
our new more specific message-id format for inbound and
outbound emails has now been in use for a very long time,
we can remove the support for the old formats:
`topic/:topic_id/:post_id.:random@:host`
`topic/:topic_id@:host`
`topic/:topic_id.:random@:host`
We are seeing a weird resolution error on Github actions with the
following backtrace:
```
Failure/Error:
visit File.join(
GlobalSetting.relative_url_root || "",
"/session/#{user.encoded_username}/become.json?redirect=false",
)
Socket::ResolutionError:
getaddrinfo: Temporary failure in name resolution
```
Switch to use `127.0.0.1` instead of forcing a name resolution.
In 958437e7dd we ensured that the email summaries are properly sent based on 'digest_attempted_at' for people who barely/never visit the forum.
This fixed the "frequency" of the email summaries but introduced a bug where the digest would be sent even though there wasn't anything new since for some users.
The logic we use to compute the threshold date for the content to be included in the digest was
```ruby
@since = opts[:since] || user.last_seen_at || user.user_stat&.digest_attempted_at || 1.month.ago
```
It was working as expected for users who haven never been seen but for users who have connected at least once, we would use their "last_seen_at" date as the "threshold date" for the content to be sent in a summary 😬
This fix changes the logic to be the most recent date amongst the `last_seen_at`, `digest_attempted_at` and `1.month.ago` so it's correctly handling cases where
- user has never been seen nor emailed a summary
- user has been seen in a while but has recently been sent a summary
- user has been sent a summary recently but hasn't been seen in a while.
This commit moves the logic for crawler rate limits out of the application controller and into the request tracker middleware. The reason for this move is to apply rate limits to all crawler requests instead of just the requests that make it to the application controller. Some requests are served early from the middleware stack without reaching the Rails app for performance reasons (e.g. `AnonymousCache`) which results in crawlers getting 200 responses even though they've reached their limits and should be getting 429 responses.
Internal topic: t/128810.
This gives us daily fidelity of topic view stats
New table stores a row per topic viewed per day tracking
anonymous and logged on views
We also have a new endpoint `/t/ID/views-stats.json` to get the statistics for the topic.
When selected some text inside a post, we offer the ability to "fast edit" the selected text without opening the composer.
However, there are certain cases where this isn't working quite a expected, due to the fact that we have some text in the "cooked" version of the post that isn't literally in the "raw" version of the post.
This ensures that whenever someone selects the within
- a quote
- a onebox
- an encrypted message
- a "cooked" date
we directly show the composer instead of showing the fast edit modal and then leaving the user with an invisible error.
Internal ref. t/128400
This commit updates `S3Inventory#files` to ignore S3 inventory files
which have a `last_modified` timestamp which are not at least 2 days
older than `BackupMetadata.last_restore_date` timestamp.
This check was previously only in `Jobs::EnsureS3UploadsExistence` but
`S3Inventory` can also be used via Rake tasks so this protection needs
to be in `S3Inventory` and not in the scheduled job.
After flags were moved to the database, with each save they are changing available PostActionTypes. Therefore, flag specs should clear the state before and after each example not just before.
In addition, we need to clear `nil` counts for dynamically created flags from serializer.
* FEATURE: add agree and edit
adds agree and edit - an alias for agree and keep -- but with a client action to
edit the post in the composer before the flag is agreed with
---------
Co-authored-by: Juan David Martinez <juan@discourse.org>
We're planning to implement a feature that allows adding required fields for existing users. This PR does some preparatory refactoring to make that possible. There should be no changes to existing behaviour. Just a small update to the admin UI.
This commit updates `Post#each_upload_url` to reject URLs that do not
have a host which matches `Discourse.current_hostname` but follows the
`/uploads/short-url` uploads URL format. This situation most commonly
happen when users copy upload URL link between different Discourse
sites.
For plugins with only an "enabled" site setting, it doesn't
make sense to take them to the site settings page, since the
toggle switch in the list can be used to change enabled/disabled.
This will not be the case for plugins that have their own custom
config page (like Automation), but we will deal with this when
we actually overhaul this plugin to use the new show page.
Also adds another rspec fixture of a test plugin.
This PR introduces a basic AdminNotice model to store these notices. Admin notices are categorized by their source/type (currently only notices from problem check.) They also have a priority.
* DEV: allow reply_by_email, visit_link_to_respond strings to be modified by plugins
* DEV: separate visit_link_to_respond and reply_by_email modifiers out
