Commit Graph

23 Commits

Author SHA1 Message Date
Martin Brennan 9174716737
DEV: Remove Discourse.redis.delete_prefixed (#22103)
This method is a huge footgun in production, since it calls
the Redis KEYS command. From the Redis documentation at
https://redis.io/commands/keys/:

> Warning: consider KEYS as a command that should only be used in
production environments with extreme care. It may ruin performance when
it is executed against large databases. This command is intended for
debugging and special operations, such as changing your keyspace layout.
Don't use KEYS in your regular application code.

Since we were only using `delete_prefixed` in specs (now that we
removed the usage in production in 24ec06ff85)
we can remove this and instead rely on `use_redis_snapshotting` on the
particular tests that need this kind of clearing functionality.
2023-06-16 12:44:35 +10:00
Selase Krakani f462347e12
DEV: `PostRevisor` helper methods to track topic title and raw revisions (#21918)
Not all revisions involve changes to the actual post/topic content. We
may want to know if a revisions includes the topic title or post raw.

Specifically introducing these for use in the Akismet plugin to
conditionally queue checks.
2023-06-05 18:02:46 +00:00
Sérgio Saquetim 21ec70b509
FIX: Miscellaneous tagging errors (#21490)
* FIX: Displaying the wrong number of minimum tags in the composer

When the minimum number of tags set for the category is larger than the minimum number of tags
set in the category tag-groups, the composer was displaying the wrong value.

This commit fixes the value displayed in the composer to show the max value between the required
for the category and the tag-groups set for the category.

This bug was reported on Meta in https://meta.discourse.org/t/tags-from-multiple-tag-groups-required-only-suggest-select-at-least-one-tag/263817

* FIX: Limiting tags in categories not working as expected

When a category was restricted to a tag group A, which was set to only allow
one tag from the group per topic, selecting a tag belonging only to A returned
other tags from A that also belonged to other group/s (if any).

Example:

Tag group A: alpha, beta, gamma, epsilon, delta
Tag group B: alpha, beta, gamma

Both tag groups set to only allow one tag from the group per topic.

If Category 1 was set to only allow tags from the tag group A, and the first tag
selected was epsilon, then, because they also belonged to tag group B, the tags
alpha, beta, and gamma were still returned as valid options when they should not be.

This commit ensures that once a tag from a tag group that restricts its tags to
one per topic is selected, no other tag from this group is returned.

This bug was reported on Meta in https://meta.discourse.org/t/limiting-tags-to-categories-not-working-as-expected/263143.

* FIX: Moving topics does not prompt to add required tag for new category

When a topic moved from a category to another, the tag requirements
of the new category were not being checked.

This allowed a topic to be created and moved to a category:

- that limited the tags to a tag group, with the topic containing tags
not allowed.
- that required N tags from a tag group, with the topic not containing
the required tags.

This bug was reported on Meta in https://meta.discourse.org/t/moving-tagged-topics-does-not-prompt-to-add-required-tag-for-new-category/264138.

* FIX: Editing topics with tag groups from parents allows incorrect tagging

When there was a combination between parent tags defined in a tag group
set to allow only one tag from the group per topic, and other tag groups
relying on this restriction to combine the children tag types with the
parent tag, editing a topic could allow the user to insert an invalid
combination of these tags.

Example:

Automakers tag group: landhover, toyota
  - group set to limit one tag from the group per topic

Toyota models group: land-cruiser, hilux, corolla

Landhover models group: evoque, defender, discovery

If a topic was initially set up with the tags toyota, land-cruiser it was
possible to edit it by removing the tag toyota and adding the tag landhover
and other landhover model tags like evoque for example.

In this case, the topic would end up with the tags toyota, land-cruiser,
landhover, evoque because Discourse will automatically insert the
missing parent tag toyota when it detects the tag land-cruiser.

This combination of tags would violate the restriction specified in
the Automakers tag group resulting in an invalid combination of tags.

This commit enforces that the "one tag from the group per topic"
restriction is verified before updating the topic tags and also
make sure the verification checks the compatibility of parent tags that
would be automatically inserted.

After the changes, the user will receive an error similar to:
The tags land-cruiser, landhover cannot be used simultaneously.
Please include only one of them.
2023-05-15 17:19:41 -03:00
Bianca Nenciu b73a9a1faa
UX: Various improvements to welcome topic CTA (#21010)
- Update welcome topic copy
- Edit the welcome topic automatically when the title or description changes
- Remove “Create your Welcome Topic” banner/CTA
- Add "edit welcome topic" user tip
2023-05-12 17:09:40 +03:00
Ted Johansson da6295e3d1
FIX: Allow re-flagging of ninja-edited posts (#21360)
What is the problem?

Consider the following timeline:

1. OP starts a topic.
2. Troll responds snarkily.
3. Flagger flags the post as “inappropriate”.
4. Admin agrees and hides the post.
5. Troll ninja-edits the post within the grace period, but still snarky.
6. Flagger flags the post as inappropriate again.

The current behaviour is that the flagger is met with an error saying the post has been reviewed and can't be flagged again for the same reason.

The desired behaviour is after someone has edited a post, it should be flaggable again.

Why is this happening?

This is related to the ninja-edit feature, where within a set grace period no new revision is created, but a new revision is required to flag the same post for the same reason.

So essentially there is a window between the naughty corner cooldown where a flagged post can't be edited, and the ninja-edit grace period, where an edit can be made without a new revision. Posts that are edited within this window can't be re-flagged by the same user.

|-----------------|-------------------------------|
^ Flag accepted   | ~~~~~~~~~~~~~ 🥷🏻 ~~~~~~~~~~~~ |
                  |                               ^ Editing grace period over
                  ^ Naughty corner cooldown over

How does this fix it?

We already create a new revision when ninja-editing a post with a pending flag. The issue above happens only in the case where the flag is already accepted.

This change extends the existing behaviour so that a new revision is created when ninja-editing any flagged post, regardless of the status of the flag. (Deleted flags excluded.)

This should also help with posterity, avoiding situations where a successfully flagged post looks innocuous in the history because it was ninja-edited, and vice versa.
2023-05-04 10:22:07 +08:00
Isaac Janzen 599979902e
FIX: Error when trying to bump a topic with no category (#21207)
When revising a post, if the topic that post belonged to did not have a category attached it would error with 

> NoMethodError (undefined method `read_restricted' for nil:NilClass)
2023-04-24 09:28:10 -05:00
Mark VanLandingham 65f35e1ef2
FEATURE: SiteSetting for creation of small action on tag change (#20812)
This adds a SiteSetting, which when enabled, creates a small_action post for tag/category changes to the topic. It uses `topic.add_moderator_post, and passes raw text in, to describe the change.
2023-04-05 13:31:31 -05:00
David Taylor cb932d6ee1
DEV: Apply syntax_tree formatting to `spec/*` 2023-01-09 11:49:28 +00:00
Rafael dos Santos Silva e901403621
FEATURE: API to customize server side composer errors handling in the client side (#19107)
This will be used by plugins to handle the client side of their custom
post validations without having to overwrite the whole composer save
action as it was done in other plugins.

Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
2022-11-21 13:11:29 -03:00
Bianca Nenciu 6b788d7329
FIX: Skip all post validations if necessary (#18625)
When PostRevisor is called with 'skip_validations: true' it can save
the post twice and one of the calls passes the correct 'validate: false'
argument, but the other one does not.
2022-10-19 18:54:32 +03:00
Martin Brennan 8ebd5edd1e
DEV: Rename secure_media to secure_uploads (#18376)
This commit renames all secure_media related settings to secure_uploads_* along with the associated functionality.

This is being done because "media" does not really cover it, we aren't just doing this for images and videos etc. but for all uploads in the site.

Additionally, in future we want to secure more types of uploads, and enable a kind of "mixed mode" where some uploads are secure and some are not, so keeping media in the name is just confusing.

This also keeps compatibility with the `secure-media-uploads` path, and changes new
secure URLs to be `secure-uploads`.

Deprecated settings:

* secure_media -> secure_uploads
* secure_media_allow_embed_images_in_emails -> secure_uploads_allow_embed_images_in_emails
* secure_media_max_email_embed_image_size_kb -> secure_uploads_max_email_embed_image_size_kb
2022-09-29 09:24:33 +10:00
Arpit Jalan 3aaf4dcfd0
FIX: hide welcome topic banner as soon as the welcome topic is edited (#18125)
* FIX: hide welcome topic banner as soon as the welcome topic is edited

This commit adds a message bus listener on client to hide the welcome
topic banner as soon as the welcome topic is edited.

* update test

* only subscribe when show_welcome_topic_banner is true

* Do not lookup for messageBus service if it's not required

* Remove unneeded code

* Cache result for Site.show_welcome_topic_banner

* Update tests per latest changes

* Changes per PR review
2022-09-01 09:44:21 +05:30
Loïc Guitaut 3eaac56797 DEV: Use proper wording for contexts in specs 2022-08-04 11:05:02 +02:00
Phil Pirozhkov 493d437e79
Add RSpec 4 compatibility (#17652)
* Remove outdated option

04078317ba

* Use the non-globally exposed RSpec syntax

https://github.com/rspec/rspec-core/pull/2803

* Use the non-globally exposed RSpec syntax, cont

https://github.com/rspec/rspec-core/pull/2803

* Comply to strict predicate matchers

See:
 - https://github.com/rspec/rspec-expectations/pull/1195
 - https://github.com/rspec/rspec-expectations/pull/1196
 - https://github.com/rspec/rspec-expectations/pull/1277
2022-07-28 10:27:38 +08:00
Loïc Guitaut 296aad430a DEV: Use `describe` for methods in specs 2022-07-27 16:35:27 +02:00
Loïc Guitaut 91b6b5eee7 DEV: Don’t use `change { … }.by(0)` in specs 2022-07-26 10:34:15 +02:00
Jarek Radosz 02a96a53fb
DEV: Be specific about time units (#17629)
All other tests that are setting grade_period use either unitless `0`, `1.minute` or `5.minutes` so it wasn't clear if `5` was meant to be seconds (it was)
2022-07-25 02:32:15 +02:00
Bianca Nenciu e8d802eb86
FIX: Check if tags edit notifications are disabled (#17499)
Tag edit notifications are either created by PostActionNotifier or
PostRevisor. PostActionNotifier already checks if the site setting is
enabled. but PostRevisor scheduled a NotifyTagChange job without
checking disable_tags_edit_notifications.
2022-07-15 16:14:46 +10:00
Bianca Nenciu 9db8f00b3d
FEATURE: Create upload_references table (#16146)
This table holds associations between uploads and other models. This can be used to prevent removing uploads that are still in use.

* DEV: Create upload_references
* DEV: Use UploadReference instead of PostUpload
* DEV: Use UploadReference for SiteSetting
* DEV: Use UploadReference for Badge
* DEV: Use UploadReference for Category
* DEV: Use UploadReference for CustomEmoji
* DEV: Use UploadReference for Group
* DEV: Use UploadReference for ThemeField
* DEV: Use UploadReference for ThemeSetting
* DEV: Use UploadReference for User
* DEV: Use UploadReference for UserAvatar
* DEV: Use UploadReference for UserExport
* DEV: Use UploadReference for UserProfile
* DEV: Add method to extract uploads from raw text
* DEV: Use UploadReference for Draft
* DEV: Use UploadReference for ReviewableQueuedPost
* DEV: Use UploadReference for UserProfile's bio_raw
* DEV: Do not copy user uploads to upload references
* DEV: Copy post uploads again after deploy
* DEV: Use created_at and updated_at from uploads table
* FIX: Check if upload site setting is empty
* DEV: Copy user uploads to upload references
* DEV: Make upload extraction less strict
2022-06-09 09:24:30 +10:00
David Taylor 68c74e9b93
FEATURE: Allow multiple required tag groups for a category (#16381)
Previously we only supported a single 'required tag group' for a category. This commit allows admins to specify multiple required tag groups, each with their own minimum tag count.

A new category_required_tag_groups database table replaces the existing columns on the categories table. Data is automatically migrated.
2022-04-06 14:08:06 +01:00
David Taylor c9dab6fd08
DEV: Automatically require 'rails_helper' in all specs (#16077)
It's very easy to forget to add `require 'rails_helper'` at the top of every core/plugin spec file, and omissions can cause some very confusing/sporadic errors.

By setting this flag in `.rspec`, we can remove the need for `require 'rails_helper'` entirely.
2022-03-01 17:50:50 +00:00
Osama Sayegh 586d572e05
FIX: Don't advance draft sequence when editing topic title (#16002)
This commit handles the edge case where a draft is lost with no warnings if the user edits the title (or category/tags) of a topic while they're replying.to the same topic. Repro steps are as follows:

1. Start replying to a topic and type enough to get a draft saved.
2. Scroll up to the topic title and click the pencil icon next to the topic title, change the title, category and/or tags, and then save the changes.
3. Reload the page and you'll see that the draft is gone.

This happens because we only allow 1 draft per topic per user and when you edit the title of a topic that you're replying to, from the server perspective it'll look like as if you've submitted your reply so it will advance the draft sequence for the topic and delete the draft.

The fix in this commit makes `PostRevisor` skip advancing the draft sequence when a topic's title is edited using the pencil button next to the title.

Internal ticket: t60854.

Co-authored-by: Robin Ward <robin.ward@gmail.com>
2022-02-23 10:39:54 +03:00
Jarek Radosz 45cc16098d
DEV: Move spec/components to spec/lib (#15987)
Lib specs were inexplicably split into two directories (`lib` and `components`)

This moves them all into `lib`.
2022-02-18 19:41:54 +01:00