Followup to e113eff663
We previously sanitized input for integer site settings
on the server side only, which was a bit confusing when
users would enter e.g. 100.5 and end up with 1005, and
not see this reflected in the UI.
Now that we are using native number inputs for these settings,
we can improve the experience a bit by not allowing `.` or `,`
in the input, because it should be whole numbers only, and
add a step size of 1. All other characters are already prevented
in this native number input.
The old implementation used unnecessary `\r\n` and caused the table generator to incorrectly add extra empty lines.
This commit replaces it with `\n`, fixing the bug
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
* FIX: Table Builder editor eradicates column alignment specification
Currently, when you use the table builder to edit an existing table, the table builder does not observe the text-align property of the original table. This results in the original table alignment being lost after editing and reset to no alignment.
This commit fixed this issue
related meta topic: https://meta.discourse.org/t/table-builder-editor-eradicates-column-alignment-specification/299577
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.
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.
Adds warnings for:
- `discourse.plugin-outlet-tag-name`
- `discourse.plugin-outlet-parent-view`
Also updates the ID list to be strings rather than regex (so that `.` is not treated as a wildcard).
Firefox is having a lot of inconsistent issues with this
feature introduced in 30fdd7738e,
disabling it there for now until further investigation can
be done.
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
* PERF: Fix N+1 issue for javascript_cache
* FIX: missing upload fields should still appear in stylesheets
Sass is still expected to compile successfully even without uploads.
Revert a blank upload to have a blank URL
* DEV: remove unneeded test comment
---------
Co-authored-by: Jeff Wong <awole20@gmail.com>
Followup 30fdd7738e,
The issue with keyup is that it happens too late. maybeContinueList
itself runs in about 1 or 2 ms. But we show the linebreak in the
textarea on keydown and we handle it in keyup, which causes the “lag”.
The fix here is “hacking” itsatrap and textarea behavior to allow us to handle
it right away after the linebreak is inserted.
Full credit to Joffrey Jaffeux for this fix, I am making him
"co-author" below.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
* 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.
This improves the way we sanitize `iframe` and correctly strips them from the "raw" before cooking it.
Otherwise, we would show an empty iframe box.
Internal ref - t/131430
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 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
If the `enforce_second_factor_on_external_auth` setting
is disabled and a user logs in with an OAuth method,
they don't automatically get redirected to /preferences/second-factor
on login. However, they can get there manually, and once there
they cannot leave.
This commit fixes the issue and allows them to leave
and also does some refactors to indicate to the client
what login method is used as a followup to
0e1102b332
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.
We didn't escape the "user status" before inserting in in the title of the "user status badge" next to the current user avatar.
This only affects the current user.
Internal ref - t/130332
In 53b3d2f0dc we introduced a stricter BBCode Tag parser. It prevents having "values" with spaces when they're not surrounded by a valid pair of quotes.
The `[details=` BBCode Tag is popular enough that it's worth adding a special case for it (especially since it doesn't support other parameters).
This also adds the Finnish pair of quotes.
Context - https://meta.discourse.org/t/details-accepts-only-one-word-as-summary/313019
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
…to avoid re-evaulation right before destroying.
With `DeferredTrackedSet` we delay both adding and removing elements from the set. That means when you're transitioning between routes, and breadcrumbs change, both old and new breadcrumbs are rendered (briefly, in a first render pass)
And since the arguments for the old breadcrumbs can be (and often are) destroyed - it would blow up the renderer. By caching the template it will reuse it in that first pass.
---
No test because I couldn't figure out a synthetic test setup where you have breadcrumbs in a deeply nested route and where you navigate from that route to one of the parent routes.
* 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>
This commit introduces behaviour similar to sites
like GitHub, Notion, and others where, if you
are already typing a list and press enter in the composer,
we continue the list on the next line.
Then, if you press enter again on the next line with
an empty list item, we remove that item on that last line.
This works with the following list types:
* star bullet
- dash bullet
* [] star and dash bullet with checkbox
1. numbered
This also works if you are in the middle of a list, and
with indented sub-lists.
With the numbered lists, we continue with the next number
in the sequence, and if you start a new line in the middle
of the list, we renumber the rest of the list.
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>
We previously migrated field_type from a string to an integer backed enum. Part of this involved renaming a column in a post migration, swapping out field_type:string for field_type:integer. This borks the ActiveRecord cache since the application is already running. Rebooting fixes it, but we want to avoid having this happen in the first place.
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.
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.
* UX: Add new preview links to Popular Themes
Replace previews for 'Discourse' based ones
* prettier
---------
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
* 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.
There is currently only one scenario when both the composer and a user card would be present at the same time:
if you have the composer open and then you click on something outside it that triggers a card. Which implies intent to see the card (unobstructed by the composer 😉)
The reverse doesn't happen because opening the composer would close an existing user card.
In theory there's also displaying a user card by clicking on a mention in composer's preview but that functionality is currently broken (and this PR is a prerequisite 😉)
---
I changed `.user-card, .group-card` to `.fk-d-menu[data-identifier="card"]` because that regressed when we moved user cards to float-kit – they are nested inside `.fk-d-menu` so its `z-index` is now important (effectively the cards had `z-index: z("dropdown")` instead of `z("usercard")`)
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 allows modifyClass to be used like this:
```
api.modifyClass(
"model:topic",
(Superclass) =>
class extends Superclass {
static someStaticMethod() {
return `${super.someStaticMethod()} modified`;
}
someFunction() {
return `${super.someFunction()} modified`;
}
get someGetter() {
return `${super.someGetter} modified`;
}
}
);
```
One limitation, which is the same as the old object-literal syntax, is that native class fields and constructors cannot be modified.
`@tracked` properties can be overriden, because the decorator turns them into getters/setters.
There is no need to pass a `pluginId` any more. Changes are automatically rolled back as part of test cleanup 🎉
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.
* use `TrackedSet` instead of `@tracked []`
* correct return type annotations
* move code to outside Promise blocks where possible
* fix an outdated comment
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>
