discourse/spec/lib/post_revisor_spec.rb

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

1655 lines
55 KiB
Ruby
Raw Normal View History

# frozen_string_literal: true
require "post_revisor"
RSpec.describe PostRevisor do
fab!(:topic)
fab!(:newuser) { Fabricate(:newuser, last_seen_at: Date.today) }
fab!(:user)
fab!(:coding_horror)
fab!(:admin)
fab!(:moderator)
2015-05-30 02:08:39 +08:00
let(:post_args) { { user: newuser, topic: topic } }
describe "TopicChanges" do
let(:tc) do
topic.reload
PostRevisor::TopicChanges.new(topic, topic.user)
end
it "provides a guardian" do
expect(tc.guardian).to be_an_instance_of Guardian
end
it "tracks changes properly" do
expect(tc.diff).to eq({})
# it remembers changes we tell it to
tc.record_change("height", "180cm", "170cm")
expect(tc.diff["height"]).to eq(%w[180cm 170cm])
# it works with arrays of values
tc.record_change("colors", nil, %w[red blue])
expect(tc.diff["colors"]).to eq([nil, %w[red blue]])
# it does not record changes to the same val
tc.record_change("wat", "js", "js")
expect(tc.diff["wat"]).to be_nil
tc.record_change("tags", %w[a b], %w[a b])
expect(tc.diff["tags"]).to be_nil
end
end
describe "editing category" do
it "triggers the :post_edited event with topic_changed?" do
category = Fabricate(:category)
category.set_permissions(everyone: :full)
category.save!
post = create_post
events = DiscourseEvent.track_events { post.revise(post.user, category_id: category.id) }
event = events.find { |e| e[:event_name] == :post_edited }
expect(event[:params].first).to eq(post)
expect(event[:params].second).to eq(true)
expect(event[:params].third).to be_kind_of(PostRevisor)
expect(event[:params].third.topic_diff).to eq(
{ "category_id" => [SiteSetting.uncategorized_category_id, category.id] },
)
end
it "does not revise category when no permission to create a topic in category" do
category = Fabricate(:category)
category.set_permissions(staff: :full)
category.save!
post = create_post
old_id = post.topic.category_id
post.revise(post.user, category_id: category.id)
post.reload
expect(post.topic.category_id).to eq(old_id)
category.set_permissions(everyone: :full)
category.save!
post.revise(post.user, category_id: category.id)
post.reload
expect(post.topic.category_id).to eq(category.id)
end
it "does not revise category when the destination category requires topic approval" do
new_category = Fabricate(:category)
new_category.require_topic_approval = true
new_category.save!
post = create_post
old_category_id = post.topic.category_id
post.revise(post.user, category_id: new_category.id)
expect(post.reload.topic.category_id).to eq(old_category_id)
new_category.require_topic_approval = false
new_category.save!
post.revise(post.user, category_id: new_category.id)
expect(post.reload.topic.category_id).to eq(new_category.id)
end
it "does not revise category if incorrect amount of tags" do
SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0
new_category = Fabricate(:category, minimum_required_tags: 1)
post = create_post
old_category_id = post.topic.category_id
post.revise(post.user, category_id: new_category.id)
expect(post.reload.topic.category_id).to eq(old_category_id)
tag = Fabricate(:tag)
topic_tag = Fabricate(:topic_tag, topic: post.topic, tag: tag)
post.revise(post.user, category_id: new_category.id)
expect(post.reload.topic.category_id).to eq(new_category.id)
topic_tag.destroy
post.revise(post.user, category_id: new_category.id, tags: ["test_tag"])
expect(post.reload.topic.category_id).to eq(new_category.id)
end
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-16 04:19:41 +08:00
it "returns an error if the topic does not have minimum amount of tags that the new category requires" do
SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0
old_category = Fabricate(:category, minimum_required_tags: 0)
new_category = Fabricate(:category, minimum_required_tags: 1)
post = create_post(category: old_category)
topic = post.topic
post.revise(post.user, category_id: new_category.id)
expect(topic.errors.full_messages).to eq([I18n.t("tags.minimum_required_tags", count: 1)])
end
it "returns an error if the topic has tags not allowed in the new category" do
SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0
tag1 = Fabricate(:tag)
tag2 = Fabricate(:tag)
tag_group = Fabricate(:tag_group, tags: [tag1])
tag_group2 = Fabricate(:tag_group, tags: [tag2])
old_category = Fabricate(:category, tag_groups: [tag_group])
new_category = Fabricate(:category, tag_groups: [tag_group2])
post = create_post(category: old_category, tags: [tag1.name])
topic = post.topic
post.revise(post.user, category_id: new_category.id)
expect(topic.errors.full_messages).to eq(
[
I18n.t(
"tags.forbidden.restricted_tags_cannot_be_used_in_category",
count: 1,
tags: tag1.name,
category: new_category.name,
),
],
)
end
it "returns an error if the topic is missing tags required from a tag group in the new category" do
SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0
tag1 = Fabricate(:tag)
tag_group = Fabricate(:tag_group, tags: [tag1])
old_category = Fabricate(:category)
new_category =
Fabricate(
:category,
category_required_tag_groups: [
CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1),
],
)
post = create_post(category: old_category)
topic = post.topic
post.revise(post.user, category_id: new_category.id)
expect(topic.errors.full_messages).to eq(
[
I18n.t(
"tags.required_tags_from_group",
count: 1,
tag_group_name: tag_group.name,
tags: tag1.name,
),
],
)
end
end
describe "editing tags" do
subject(:post_revisor) { PostRevisor.new(post) }
fab!(:post)
before do
Jobs.run_immediately!
TopicUser.change(
newuser.id,
post.topic_id,
notification_level: TopicUser.notification_levels[:watching],
)
end
it "creates notifications" do
expect { post_revisor.revise!(admin, tags: ["new-tag"]) }.to change { Notification.count }.by(
1,
)
end
it "skips notifications if disable_tags_edit_notifications" do
SiteSetting.disable_tags_edit_notifications = true
expect { post_revisor.revise!(admin, tags: ["new-tag"]) }.not_to change { Notification.count }
end
it "doesn't create a small_action post when create_post_for_category_and_tag_changes is false" do
SiteSetting.create_post_for_category_and_tag_changes = false
expect { post_revisor.revise!(admin, tags: ["new-tag"]) }.not_to change { Post.count }
end
describe "when `create_post_for_category_and_tag_changes` site setting is enabled" do
fab!(:tag1) { Fabricate(:tag, name: "First tag") }
fab!(:tag2) { Fabricate(:tag, name: "Second tag") }
before { SiteSetting.create_post_for_category_and_tag_changes = true }
it "Creates a small_action post with correct translation when both adding and removing tags" do
post.topic.update!(tags: [tag1])
expect { post_revisor.revise!(admin, tags: [tag2.name]) }.to change {
Post.where(topic_id: post.topic_id, action_code: "tags_changed").count
}.by(1)
expect(post.topic.ordered_posts.last.raw).to eq(
I18n.t(
"topic_tag_changed.added_and_removed",
added: "##{tag2.name}",
removed: "##{tag1.name}",
),
)
end
it "Creates a small_action post with correct translation when adding tags" do
post.topic.update!(tags: [])
expect { post_revisor.revise!(admin, tags: [tag1.name]) }.to change {
Post.where(topic_id: post.topic_id, action_code: "tags_changed").count
}.by(1)
expect(post.topic.ordered_posts.last.raw).to eq(
I18n.t("topic_tag_changed.added", added: "##{tag1.name}"),
)
end
it "Creates a small_action post with correct translation when removing tags" do
post.topic.update!(tags: [tag1, tag2])
expect { post_revisor.revise!(admin, tags: []) }.to change {
Post.where(topic_id: post.topic_id, action_code: "tags_changed").count
}.by(1)
expect(post.topic.ordered_posts.last.raw).to eq(
I18n.t("topic_tag_changed.removed", removed: "##{tag1.name}, ##{tag2.name}"),
)
end
it "Creates a small_action post when category is changed" do
current_category = post.topic.category
category = Fabricate(:category)
expect { post_revisor.revise!(admin, category_id: category.id) }.to change {
Post.where(topic_id: post.topic_id, action_code: "category_changed").count
}.by(1)
expect(post.topic.ordered_posts.last.raw).to eq(
I18n.t(
"topic_category_changed",
to: "##{category.slug}",
from: "##{current_category.slug}",
),
)
end
end
end
describe "revise wiki" do
# There used to be a bug where wiki changes were considered posting "too similar"
# so this is enabled and checked
use_redis_snapshotting
before { SiteSetting.unique_posts_mins = 10 }
it "allows the user to change it to a wiki" do
pc =
PostCreator.new(newuser, topic_id: topic.id, raw: "this is a post that will become a wiki")
post = pc.create
expect(post.revise(post.user, wiki: true)).to be_truthy
post.reload
expect(post.wiki).to be_truthy
end
end
describe "revise" do
subject(:post_revisor) { PostRevisor.new(post) }
let(:post) { Fabricate(:post, post_args) }
let(:first_version_at) { post.last_version_at }
it "destroys last revision if edit is undone" do
old_raw = post.raw
post_revisor.revise!(admin, raw: "new post body", tags: ["new-tag"])
expect(post.topic.reload.tags.map(&:name)).to contain_exactly("new-tag")
expect(post.post_revisions.reload.size).to eq(1)
expect(post_revisor.raw_changed?).to eq(true)
post_revisor.revise!(admin, raw: old_raw, tags: [])
expect(post.topic.reload.tags.map(&:name)).to be_empty
expect(post.post_revisions.reload.size).to eq(0)
post_revisor.revise!(admin, raw: "next post body", tags: ["new-tag"])
expect(post.topic.reload.tags.map(&:name)).to contain_exactly("new-tag")
expect(post.post_revisions.reload.size).to eq(1)
end
describe "with the same body" do
2013-12-12 10:41:34 +08:00
it "doesn't change version" do
2015-01-10 00:34:37 +08:00
expect {
expect(post_revisor.revise!(post.user, raw: post.raw)).to eq(false)
post.reload
2015-01-10 00:34:37 +08:00
}.not_to change(post, :version)
end
end
describe "with nil raw contents" do
it "doesn't change version" do
expect {
expect(post_revisor.revise!(post.user, raw: nil)).to eq(false)
post.reload
}.not_to change(post, :version)
end
end
describe "topic is in slow mode" do
before { topic.update!(slow_mode_seconds: 1000) }
it "regular edits are not allowed by default" do
post_revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + 1000.minutes,
)
post.reload
expect(post.errors.present?).to eq(true)
expect(post.errors.messages[:base].first).to be I18n.t("cannot_edit_on_slow_mode")
end
it "grace period editing is allowed" do
SiteSetting.editing_grace_period = 1.minute
post_revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + 10.seconds,
)
post.reload
expect(post.errors).to be_empty
end
it "regular edits are allowed if it was turned on in settings" do
SiteSetting.slow_mode_prevents_editing = false
post_revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + 10.minutes,
)
post.reload
expect(post.errors).to be_empty
end
it "staff is allowed to edit posts even if the topic is in slow mode" do
admin = Fabricate(:admin)
post_revisor.revise!(
admin,
{ raw: "updated body" },
revised_at: post.updated_at + 10.minutes,
)
post.reload
expect(post.errors).to be_empty
end
end
describe "grace period editing" do
it "correctly applies edits" do
SiteSetting.editing_grace_period = 1.minute
2015-05-30 02:08:39 +08:00
post_revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + 10.seconds,
)
post.reload
2015-01-10 00:34:37 +08:00
expect(post.version).to eq(1)
expect(post.public_version).to eq(1)
expect(post.revisions.size).to eq(0)
expect(post.last_version_at).to eq_time(first_version_at)
expect(post_revisor.category_changed).to be_blank
end
2015-05-30 02:08:39 +08:00
it "does create a new version if a large diff happens" do
SiteSetting.editing_grace_period_max_diff = 10
post = Fabricate(:post, raw: "hello world")
revisor = PostRevisor.new(post)
revisor.revise!(
post.user,
{ raw: "hello world123456789" },
revised_at: post.updated_at + 1.second,
)
post.reload
expect(post.version).to eq(1)
revisor = PostRevisor.new(post)
revisor.revise!(
post.user,
{ raw: "hello world12345678901" },
revised_at: post.updated_at + 1.second,
)
post.reload
expect(post.version).to eq(2)
expect(post.revisions.first.modifications["raw"][0]).to eq("hello world")
expect(post.revisions.first.modifications["cooked"][0]).to eq("<p>hello world</p>")
SiteSetting.editing_grace_period_max_diff_high_trust = 100
post.user.update_columns(trust_level: 2)
revisor = PostRevisor.new(post)
revisor.revise!(
post.user,
{ raw: "hello world12345678901 123456789012" },
revised_at: post.updated_at + 1.second,
)
post.reload
expect(post.version).to eq(2)
expect(post.revisions.count).to eq(1)
end
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
it "creates a new version when the post is flagged" do
SiteSetting.editing_grace_period = 1.minute
post = Fabricate(:post, raw: "hello world")
Fabricate(:flag, post: post, user: user)
revisor = PostRevisor.new(post)
revisor.revise!(
post.user,
{ raw: "hello world, JK" },
revised_at: post.updated_at + 1.second,
)
post.reload
expect(post.version).to eq(2)
expect(post.revisions.count).to eq(1)
end
2015-05-30 02:08:39 +08:00
it "doesn't create a new version" do
SiteSetting.editing_grace_period = 1.minute
SiteSetting.editing_grace_period_max_diff = 100
2015-05-30 02:08:39 +08:00
# making a revision
post_revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + SiteSetting.editing_grace_period + 1.seconds,
)
2015-05-30 02:08:39 +08:00
# "roll back"
post_revisor.revise!(
post.user,
{ raw: "Hello world" },
revised_at: post.updated_at + SiteSetting.editing_grace_period + 2.seconds,
)
2015-05-30 02:08:39 +08:00
post.reload
expect(post.version).to eq(1)
expect(post.public_version).to eq(1)
expect(post.revisions.size).to eq(0)
end
it "should bump the topic" do
expect {
post_revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + SiteSetting.editing_grace_period + 1.seconds,
)
}.to change { post.topic.bumped_at }
end
it "should bump topic when no topic category" do
topic_with_no_category = Fabricate(:topic, category_id: nil)
post_from_topic_with_no_category = Fabricate(:post, topic: topic_with_no_category)
expect {
result =
post_revisor.revise!(
Fabricate(:admin),
raw: post_from_topic_with_no_category.raw,
tags: ["foo"],
)
expect(result).to eq(true)
}.to change { topic.reload.bumped_at }
end
it "should send muted and latest message" do
TopicUser.create!(topic: post.topic, user: post.user, notification_level: 0)
messages =
MessageBus.track_publish("/latest") do
post_revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + SiteSetting.editing_grace_period + 1.seconds,
)
end
muted_message = messages.find { |message| message.data["message_type"] == "muted" }
latest_message = messages.find { |message| message.data["message_type"] == "latest" }
expect(muted_message.data["topic_id"]).to eq(topic.id)
expect(latest_message.data["topic_id"]).to eq(topic.id)
end
end
describe "edit reasons" do
it "does create a new version if an edit reason is provided" do
post = Fabricate(:post, raw: "hello world")
revisor = PostRevisor.new(post)
revisor.revise!(
post.user,
{ raw: "hello world123456789", edit_reason: "this is my reason" },
revised_at: post.updated_at + 1.second,
)
post.reload
expect(post.version).to eq(2)
expect(post.revisions.count).to eq(1)
end
it "resets the edit_reason attribute in post model" do
freeze_time
SiteSetting.editing_grace_period = 5.seconds
post = Fabricate(:post, raw: "hello world")
revisor = PostRevisor.new(post)
revisor.revise!(
post.user,
{ raw: "hello world123456789", edit_reason: "this is my reason" },
revised_at: post.updated_at + 1.second,
)
post.reload
expect(post.edit_reason).to eq("this is my reason")
revisor.revise!(
post.user,
{ raw: "hello world4321" },
revised_at: post.updated_at + 7.seconds,
)
post.reload
expect(post.edit_reason).not_to be_present
end
it "does not create a new version if an edit reason is provided and its the same as the current edit reason" do
post = Fabricate(:post, raw: "hello world", edit_reason: "this is my reason")
revisor = PostRevisor.new(post)
revisor.revise!(
post.user,
{ raw: "hello world123456789", edit_reason: "this is my reason" },
revised_at: post.updated_at + 1.second,
)
post.reload
expect(post.version).to eq(1)
expect(post.revisions.count).to eq(0)
end
it "does not clobber the existing edit reason for a revision if it is not provided in a subsequent revision" do
post = Fabricate(:post, raw: "hello world")
revisor = PostRevisor.new(post)
revisor.revise!(
post.user,
{ raw: "hello world123456789", edit_reason: "this is my reason" },
revised_at: post.updated_at + 1.second,
)
post.reload
revisor.revise!(
post.user,
{ raw: "hello some other thing" },
revised_at: post.updated_at + 1.second,
)
expect(post.revisions.first.modifications[:edit_reason]).to eq([nil, "this is my reason"])
end
end
describe "hidden post" do
it "correctly stores the modification value" do
post.update(hidden: true, hidden_reason_id: Post.hidden_reasons[:flag_threshold_reached])
revisor = PostRevisor.new(post)
revisor.revise!(post.user, { raw: "hello world" }, revised_at: post.updated_at + 11.minutes)
expect(post.revisions.first.modifications.symbolize_keys).to eq(
cooked: ["<p>Hello world</p>", "<p>hello world</p>"],
raw: ["Hello world", "hello world"],
)
end
end
describe "revision much later" do
let!(:revised_at) { post.updated_at + 2.minutes }
before do
SiteSetting.editing_grace_period = 1.minute
post_revisor.revise!(post.user, { raw: "updated body" }, revised_at: revised_at)
post.reload
end
it "doesn't update a category" do
expect(post_revisor.category_changed).to be_blank
end
it "updates the versions" do
2015-01-10 00:34:37 +08:00
expect(post.version).to eq(2)
expect(post.public_version).to eq(2)
end
it "creates a new revision" do
2015-01-10 00:34:37 +08:00
expect(post.revisions.size).to eq(1)
end
it "updates the last_version_at" do
2015-01-10 00:34:37 +08:00
expect(post.last_version_at.to_i).to eq(revised_at.to_i)
end
describe "new edit window" do
before do
post_revisor.revise!(
post.user,
{ raw: "yet another updated body" },
revised_at: revised_at,
)
post.reload
end
it "doesn't create a new version if you do another" do
2015-01-10 00:34:37 +08:00
expect(post.version).to eq(2)
expect(post.public_version).to eq(2)
end
it "doesn't change last_version_at" do
2015-01-10 00:34:37 +08:00
expect(post.last_version_at.to_i).to eq(revised_at.to_i)
end
it "doesn't update a category" do
expect(post_revisor.category_changed).to be_blank
end
context "after second window" do
let!(:new_revised_at) { revised_at + 2.minutes }
before do
post_revisor.revise!(
post.user,
{ raw: "yet another, another updated body" },
revised_at: new_revised_at,
)
post.reload
end
it "does create a new version after the edit window" do
2015-01-10 00:34:37 +08:00
expect(post.version).to eq(3)
expect(post.public_version).to eq(3)
end
it "does create a new version after the edit window" do
2015-01-10 00:34:37 +08:00
expect(post.last_version_at.to_i).to eq(new_revised_at.to_i)
end
end
end
end
describe "category topic" do
2013-02-26 00:42:20 +08:00
let!(:category) do
category = Fabricate(:category)
category.update_column(:topic_id, topic.id)
category
end
let(:new_description) { "this is my new description." }
it "should have no description by default" do
2015-01-10 00:34:37 +08:00
expect(category.description).to be_blank
end
context "with one paragraph description" do
before do
post_revisor.revise!(post.user, raw: new_description)
category.reload
end
it "returns the changed category info" do
expect(post_revisor.category_changed).to eq(category)
end
it "updates the description of the category" do
2015-01-10 00:34:37 +08:00
expect(category.description).to eq(new_description)
end
end
context "with multiple paragraph description" do
before do
post_revisor.revise!(post.user, raw: "#{new_description}\n\nOther content goes here.")
category.reload
end
it "returns the changed category info" do
expect(post_revisor.category_changed).to eq(category)
end
it "updates the description of the category" do
2015-01-10 00:34:37 +08:00
expect(category.description).to eq(new_description)
2013-02-26 00:42:20 +08:00
end
end
context "with invalid description without paragraphs" do
before do
post_revisor.revise!(post.user, raw: "# This is a title")
category.reload
end
it "returns a error for the user" do
expect(post.errors.present?).to eq(true)
expect(post.errors.messages[:base].first).to be I18n.t(
"category.errors.description_incomplete",
)
end
it "doesn't update the description of the category" do
expect(category.description).to eq(nil)
end
end
context "when updating back to the original paragraph" do
before do
category.update_column(:description, "this is my description")
post_revisor.revise!(post.user, raw: Category.post_template)
category.reload
end
it "puts the description back to nothing" do
2015-01-10 00:34:37 +08:00
expect(category.description).to be_blank
end
it "returns the changed category info" do
expect(post_revisor.category_changed).to eq(category)
end
end
end
describe "rate limiter" do
fab!(:changed_by) { coding_horror }
before do
RateLimiter.enable
SiteSetting.editing_grace_period = 0
end
use_redis_snapshotting
it "triggers a rate limiter" do
EditRateLimiter.any_instance.expects(:performed!)
post_revisor.revise!(changed_by, raw: "updated body")
end
it "raises error when a user gets rate limited" do
SiteSetting.max_edits_per_day = 1
user = Fabricate(:user, trust_level: 1)
post_revisor.revise!(user, raw: "body (edited)")
expect do post_revisor.revise!(user, raw: "body (edited twice) ") end.to raise_error(
RateLimiter::LimitExceeded,
)
end
it "edit limits scale up depending on user's trust level" do
SiteSetting.max_edits_per_day = 1
SiteSetting.tl2_additional_edits_per_day_multiplier = 2
SiteSetting.tl3_additional_edits_per_day_multiplier = 3
SiteSetting.tl4_additional_edits_per_day_multiplier = 4
user = Fabricate(:user, trust_level: 2)
expect { post_revisor.revise!(user, raw: "body (edited)") }.to_not raise_error
expect { post_revisor.revise!(user, raw: "body (edited twice)") }.to_not raise_error
expect do post_revisor.revise!(user, raw: "body (edited three times) ") end.to raise_error(
RateLimiter::LimitExceeded,
)
user = Fabricate(:user, trust_level: 3)
expect { post_revisor.revise!(user, raw: "body (edited)") }.to_not raise_error
expect { post_revisor.revise!(user, raw: "body (edited twice)") }.to_not raise_error
expect { post_revisor.revise!(user, raw: "body (edited three times)") }.to_not raise_error
expect do post_revisor.revise!(user, raw: "body (edited four times) ") end.to raise_error(
RateLimiter::LimitExceeded,
)
user = Fabricate(:user, trust_level: 4)
expect { post_revisor.revise!(user, raw: "body (edited)") }.to_not raise_error
expect { post_revisor.revise!(user, raw: "body (edited twice)") }.to_not raise_error
expect { post_revisor.revise!(user, raw: "body (edited three times)") }.to_not raise_error
expect { post_revisor.revise!(user, raw: "body (edited four times)") }.to_not raise_error
expect do post_revisor.revise!(user, raw: "body (edited five times) ") end.to raise_error(
RateLimiter::LimitExceeded,
)
end
end
describe "admin editing a new user's post" do
fab!(:changed_by) { Fabricate(:admin) }
before do
SiteSetting.newuser_max_embedded_media = 0
url = "http://i.imgur.com/wfn7rgU.jpg"
Oneboxer.stubs(:onebox).with(url, anything).returns("<img src='#{url}'>")
post_revisor.revise!(changed_by, raw: "So, post them here!\n#{url}")
end
it "allows an admin to insert images into a new user's post" do
2015-01-10 00:34:37 +08:00
expect(post.errors).to be_blank
end
it "marks the admin as the last updater" do
2015-01-10 00:34:37 +08:00
expect(post.last_editor_id).to eq(changed_by.id)
end
end
describe "new user editing their own post" do
before do
SiteSetting.newuser_max_embedded_media = 0
url = "http://i.imgur.com/FGg7Vzu.gif"
Oneboxer.stubs(:cached_onebox).with(url, anything).returns("<img src='#{url}'>")
post_revisor.revise!(post.user, raw: "So, post them here!\n#{url}")
end
it "doesn't allow images to be inserted" do
2015-01-10 00:34:37 +08:00
expect(post.errors).to be_present
end
end
describe "with a new body" do
before { SiteSetting.editing_grace_period_max_diff = 1000 }
fab!(:changed_by) { coding_horror }
let!(:result) { post_revisor.revise!(changed_by, raw: "lets update the body. Здравствуйте") }
it "correctly updates raw" do
2015-01-10 00:34:37 +08:00
expect(result).to eq(true)
expect(post.raw).to eq("lets update the body. Здравствуйте")
2015-01-10 00:34:37 +08:00
expect(post.invalidate_oneboxes).to eq(true)
expect(post.version).to eq(2)
expect(post.public_version).to eq(2)
expect(post.revisions.size).to eq(1)
expect(post.revisions.first.user_id).to eq(changed_by.id)
# updates word count
expect(post.word_count).to eq(5)
post.topic.reload
expect(post.topic.word_count).to eq(5)
end
it "increases the post_edits stat count" do
expect do post_revisor.revise!(post.user, { raw: "This is a new revision" }) end.to change {
post.user.user_stat.post_edits_count.to_i
}.by(1)
end
context "when second poster posts again quickly" do
it "is a grace period edit, because the second poster posted again quickly" do
SiteSetting.editing_grace_period = 1.minute
post_revisor.revise!(
changed_by,
{ raw: "yet another updated body" },
revised_at: post.updated_at + 10.seconds,
)
post.reload
2015-01-10 00:34:37 +08:00
expect(post.version).to eq(2)
expect(post.public_version).to eq(2)
expect(post.revisions.size).to eq(1)
end
end
context "when passing skip_revision as true" do
before do
SiteSetting.editing_grace_period = 1.minute
post_revisor.revise!(
changed_by,
{ raw: "yet another updated body" },
revised_at: post.updated_at + 10.hours,
skip_revision: true,
)
post.reload
end
it "does not create new revision " do
expect(post.version).to eq(2)
expect(post.public_version).to eq(2)
expect(post.revisions.size).to eq(1)
end
end
context "when editing the before_edit_post event signature" do
it "contains post and params" do
params = { raw: "body (edited)" }
events = DiscourseEvent.track_events { post_revisor.revise!(user, params) }
expect(events).to include(event_name: :before_edit_post, params: [post, params])
end
end
end
describe "topic excerpt" do
it "topic excerpt is updated only if first post is revised" do
revisor = PostRevisor.new(post)
first_post = topic.first_post
expect {
revisor.revise!(
first_post.user,
{ raw: "Edit the first post" },
revised_at: first_post.updated_at + 10.seconds,
)
topic.reload
}.to change { topic.excerpt }
second_post = Fabricate(:post, post_args.merge(post_number: 2, topic_id: topic.id))
expect {
PostRevisor.new(second_post).revise!(second_post.user, raw: "Edit the 2nd post")
topic.reload
}.to_not change { topic.excerpt }
end
end
it "doesn't strip starting whitespaces" do
post_revisor.revise!(post.user, raw: " <-- whitespaces --> ")
post.reload
2015-01-10 00:34:37 +08:00
expect(post.raw).to eq(" <-- whitespaces -->")
end
it "revises and tracks changes of topic titles" do
new_title = "New topic title"
result =
post_revisor.revise!(
post.user,
{ title: new_title },
revised_at: post.updated_at + 10.minutes,
)
expect(result).to eq(true)
post.reload
expect(post.topic.title).to eq(new_title)
expect(post.revisions.first.modifications["title"][1]).to eq(new_title)
expect(post_revisor.topic_title_changed?).to eq(true)
expect(post_revisor.raw_changed?).to eq(false)
end
it "revises and tracks changes of topic archetypes" do
new_archetype = Archetype.banner
result =
post_revisor.revise!(
post.user,
{ archetype: new_archetype },
revised_at: post.updated_at + 10.minutes,
)
expect(result).to eq(true)
post.reload
expect(post.topic.archetype).to eq(new_archetype)
expect(post.revisions.first.modifications["archetype"][1]).to eq(new_archetype)
expect(post_revisor.raw_changed?).to eq(false)
end
it "revises and tracks changes of topic tags" do
post_revisor.revise!(admin, tags: ["new-tag"])
expect(post.post_revisions.last.modifications).to eq("tags" => [[], ["new-tag"]])
expect(post_revisor.raw_changed?).to eq(false)
post_revisor.revise!(admin, tags: %w[new-tag new-tag-2])
expect(post.post_revisions.last.modifications).to eq("tags" => [[], %w[new-tag new-tag-2]])
expect(post_revisor.raw_changed?).to eq(false)
post_revisor.revise!(admin, tags: ["new-tag-3"])
expect(post.post_revisions.last.modifications).to eq("tags" => [[], ["new-tag-3"]])
expect(post_revisor.raw_changed?).to eq(false)
end
describe "#publish_changes" do
let!(:post) { Fabricate(:post, topic: topic) }
it "should publish topic changes to clients" do
revisor = PostRevisor.new(topic.ordered_posts.first, topic)
message =
MessageBus
.track_publish("/topic/#{topic.id}") do
revisor.revise!(newuser, title: "this is a test topic")
end
.first
payload = message.data
expect(payload[:reload_topic]).to eq(true)
end
end
context "when logging staff edits" do
it "doesn't log when a regular user revises a post" do
post_revisor.revise!(post.user, raw: "lets totally update the body")
log =
UserHistory.where(acting_user_id: post.user.id, action: UserHistory.actions[:post_edit])
expect(log).to be_blank
end
it "logs an edit when a staff member revises a post" do
post_revisor.revise!(moderator, raw: "lets totally update the body")
log =
UserHistory.where(
acting_user_id: moderator.id,
action: UserHistory.actions[:post_edit],
).first
expect(log).to be_present
expect(log.details).to eq("Hello world\n\n---\n\nlets totally update the body")
end
it "doesn't log an edit when skip_staff_log is true" do
post_revisor.revise!(
moderator,
{ raw: "lets totally update the body" },
skip_staff_log: true,
)
log =
UserHistory.where(
acting_user_id: moderator.id,
action: UserHistory.actions[:post_edit],
).first
expect(log).to be_blank
end
it "doesn't log an edit when a staff member edits their own post" do
revisor = PostRevisor.new(Fabricate(:post, user: moderator))
revisor.revise!(moderator, raw: "my own edit to my own thing")
log =
UserHistory.where(acting_user_id: moderator.id, action: UserHistory.actions[:post_edit])
expect(log).to be_blank
end
end
context "when logging group moderator edits" do
fab!(:group_user)
fab!(:category) do
Fabricate(:category, reviewable_by_group_id: group_user.group.id, topic: topic)
end
before do
SiteSetting.enable_category_group_moderation = true
topic.update!(category: category)
post.update!(topic: topic)
end
it "logs an edit when a group moderator revises the category description" do
PostRevisor.new(post).revise!(
group_user.user,
raw: "a group moderator can update the description",
)
log =
UserHistory.where(
acting_user_id: group_user.user.id,
action: UserHistory.actions[:post_edit],
).first
expect(log).to be_present
expect(log.details).to eq(
"Hello world\n\n---\n\na group moderator can update the description",
)
end
end
context "with staff_edit_locks_post" do
context "when disabled" do
before { SiteSetting.staff_edit_locks_post = false }
it "does not lock the post when revised" do
result = post_revisor.revise!(moderator, raw: "lets totally update the body")
expect(result).to eq(true)
post.reload
expect(post).not_to be_locked
end
end
context "when enabled" do
before { SiteSetting.staff_edit_locks_post = true }
it "locks the post when revised by staff" do
result = post_revisor.revise!(moderator, raw: "lets totally update the body")
expect(result).to eq(true)
post.reload
expect(post).to be_locked
end
it "doesn't lock the wiki posts" do
post.wiki = true
result = post_revisor.revise!(moderator, raw: "some new raw content")
expect(result).to eq(true)
post.reload
expect(post).not_to be_locked
end
it "doesn't lock the post when the raw did not change" do
result = post_revisor.revise!(moderator, title: "New topic title, cool!")
expect(result).to eq(true)
post.reload
expect(post.topic.title).to eq("New topic title, cool!")
expect(post).not_to be_locked
end
it "doesn't lock the post when revised by a regular user" do
result = post_revisor.revise!(user, raw: "lets totally update the body")
expect(result).to eq(true)
post.reload
expect(post).not_to be_locked
end
it "doesn't lock the post when revised by system user" do
result =
post_revisor.revise!(Discourse.system_user, raw: "I usually replace hotlinked images")
expect(result).to eq(true)
post.reload
expect(post).not_to be_locked
end
it "doesn't lock a staff member's post" do
staff_post = Fabricate(:post, user: moderator)
revisor = PostRevisor.new(staff_post)
result = revisor.revise!(moderator, raw: "lets totally update the body")
expect(result).to eq(true)
staff_post.reload
expect(staff_post).not_to be_locked
end
end
end
context "with alerts" do
fab!(:mentioned_user) { Fabricate(:user) }
before { Jobs.run_immediately! }
it "generates a notification for a mention" do
expect {
post_revisor.revise!(
user,
raw: "Random user is mentioning @#{mentioned_user.username_lower}",
)
}.to change { Notification.where(notification_type: Notification.types[:mentioned]).count }
end
it "never generates a notification for a mention when the System user revise a post" do
expect {
post_revisor.revise!(
Discourse.system_user,
raw: "System user is mentioning @#{mentioned_user.username_lower}",
)
}.not_to change {
Notification.where(notification_type: Notification.types[:mentioned]).count
}
end
end
context "with tagging" do
context "with tagging disabled" do
before { SiteSetting.tagging_enabled = false }
it "doesn't add the tags" do
result =
post_revisor.revise!(
user,
raw: "lets totally update the body",
tags: %w[totally update],
)
expect(result).to eq(true)
post.reload
expect(post.topic.tags.size).to eq(0)
end
end
context "with tagging enabled" do
before { SiteSetting.tagging_enabled = true }
context "when can create tags" do
before do
SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0
end
it "can create all tags if none exist" do
expect {
@result =
post_revisor.revise!(
user,
raw: "lets totally update the body",
tags: %w[totally update],
)
}.to change { Tag.count }.by(2)
expect(@result).to eq(true)
post.reload
expect(post.topic.tags.map(&:name).sort).to eq(%w[totally update])
end
it "creates missing tags if some exist" do
Fabricate(:tag, name: "totally")
expect {
@result =
post_revisor.revise!(
user,
raw: "lets totally update the body",
tags: %w[totally update],
)
}.to change { Tag.count }.by(1)
expect(@result).to eq(true)
post.reload
expect(post.topic.tags.map(&:name).sort).to eq(%w[totally update])
end
it "can remove all tags" do
topic.tags = [Fabricate(:tag, name: "super"), Fabricate(:tag, name: "stuff")]
result = post_revisor.revise!(user, raw: "lets totally update the body", tags: [])
expect(result).to eq(true)
post.reload
expect(post.topic.tags.size).to eq(0)
end
it "can't add staff-only tags" do
create_staff_only_tags(["important"])
result =
post_revisor.revise!(
user,
raw: "lets totally update the body",
tags: %w[important stuff],
)
expect(result).to eq(false)
expect(post.topic.errors.present?).to eq(true)
end
it "staff can add staff-only tags" do
create_staff_only_tags(["important"])
result =
post_revisor.revise!(
admin,
raw: "lets totally update the body",
tags: %w[important stuff],
)
expect(result).to eq(true)
post.reload
expect(post.topic.tags.map(&:name).sort).to eq(%w[important stuff])
end
it "triggers the :post_edited event with topic_changed?" do
topic.tags = [Fabricate(:tag, name: "super"), Fabricate(:tag, name: "stuff")]
events =
DiscourseEvent.track_events do
post_revisor.revise!(user, raw: "lets totally update the body", tags: [])
end
event = events.find { |e| e[:event_name] == :post_edited }
expect(event[:params].first).to eq(post)
expect(event[:params].second).to eq(true)
expect(event[:params].third).to be_kind_of(PostRevisor)
expect(event[:params].third.topic_diff).to eq({ "tags" => [%w[super stuff], []] })
end
context "with staff-only tags" do
before do
create_staff_only_tags(["important"])
topic = post.topic
topic.tags = [
Fabricate(:tag, name: "super"),
Tag.where(name: "important").first,
Fabricate(:tag, name: "stuff"),
]
end
it "staff-only tags can't be removed" do
result =
post_revisor.revise!(user, raw: "lets totally update the body", tags: ["stuff"])
expect(result).to eq(false)
expect(post.topic.errors.present?).to eq(true)
post.reload
expect(post.topic.tags.map(&:name).sort).to eq(%w[important stuff super])
end
it "can't remove all tags if some are staff-only" do
result = post_revisor.revise!(user, raw: "lets totally update the body", tags: [])
expect(result).to eq(false)
expect(post.topic.errors.present?).to eq(true)
post.reload
expect(post.topic.tags.map(&:name).sort).to eq(%w[important stuff super])
end
it "staff-only tags can be removed by staff" do
result =
post_revisor.revise!(admin, raw: "lets totally update the body", tags: ["stuff"])
expect(result).to eq(true)
post.reload
expect(post.topic.tags.map(&:name)).to eq(["stuff"])
end
it "staff can remove all tags" do
result = post_revisor.revise!(admin, raw: "lets totally update the body", tags: [])
expect(result).to eq(true)
post.reload
expect(post.topic.tags.size).to eq(0)
end
end
context "with hidden tags" do
let(:bumped_at) { 1.day.ago }
before do
topic.update!(bumped_at: bumped_at)
create_hidden_tags(%w[important secret])
topic = post.topic
topic.tags = [
Fabricate(:tag, name: "super"),
Tag.where(name: "important").first,
Fabricate(:tag, name: "stuff"),
]
end
it "doesn't bump topic if only staff-only tags are added" do
expect {
result =
post_revisor.revise!(
Fabricate(:admin),
raw: post.raw,
tags: topic.tags.map(&:name) + ["secret"],
)
expect(result).to eq(true)
}.to_not change { topic.reload.bumped_at }
end
it "doesn't bump topic if only staff-only tags are removed" do
expect {
result =
post_revisor.revise!(
Fabricate(:admin),
raw: post.raw,
tags: topic.tags.map(&:name) - %w[important secret],
)
expect(result).to eq(true)
}.to_not change { topic.reload.bumped_at }
end
it "doesn't bump topic if only staff-only tags are removed and there are no tags left" do
topic.tags = Tag.where(name: %w[important secret]).to_a
expect {
result = post_revisor.revise!(Fabricate(:admin), raw: post.raw, tags: [])
expect(result).to eq(true)
}.to_not change { topic.reload.bumped_at }
end
it "doesn't bump topic if empty string is given" do
topic.tags = Tag.where(name: %w[important secret]).to_a
expect {
result = post_revisor.revise!(Fabricate(:admin), raw: post.raw, tags: [""])
expect(result).to eq(true)
}.to_not change { topic.reload.bumped_at }
end
it "should bump topic if non staff-only tags are added" do
expect {
result =
post_revisor.revise!(
Fabricate(:admin),
raw: post.raw,
tags: topic.tags.map(&:name) + [Fabricate(:tag).name],
)
expect(result).to eq(true)
}.to change { topic.reload.bumped_at }
end
it "creates a hidden revision" do
post_revisor.revise!(
Fabricate(:admin),
raw: post.raw,
tags: topic.tags.map(&:name) + ["secret"],
)
expect(post.reload.revisions.first.hidden).to eq(true)
end
it "doesn't notify topic owner about hidden tags" do
PostActionNotifier.enable
Jobs.run_immediately!
expect {
post_revisor.revise!(
Fabricate(:admin),
raw: post.raw,
tags: topic.tags.map(&:name) + ["secret"],
)
}.not_to change {
Notification.where(notification_type: Notification.types[:edited]).count
}
end
end
context "with required tag group" do
fab!(:tag1) { Fabricate(:tag) }
fab!(:tag2) { Fabricate(:tag) }
fab!(:tag3) { Fabricate(:tag) }
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) }
fab!(:category) do
Fabricate(
:category,
name: "beta",
category_required_tag_groups: [
CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1),
],
)
end
before { post.topic.update(category: category) }
it "doesn't allow removing all tags from the group" do
post.topic.tags = [tag1, tag2]
result = post_revisor.revise!(user, raw: "lets totally update the body", tags: [])
expect(result).to eq(false)
end
it "allows removing some tags" do
post.topic.tags = [tag1, tag2, tag3]
result =
post_revisor.revise!(user, raw: "lets totally update the body", tags: [tag1.name])
expect(result).to eq(true)
expect(post.reload.topic.tags.map(&:name)).to eq([tag1.name])
end
it "allows admins to remove the tags" do
post.topic.tags = [tag1, tag2, tag3]
result = post_revisor.revise!(admin, raw: "lets totally update the body", tags: [])
expect(result).to eq(true)
expect(post.reload.topic.tags.size).to eq(0)
end
end
end
context "when cannot create tags" do
before do
SiteSetting.min_trust_to_create_tag = 4
SiteSetting.min_trust_level_to_tag_topics = 0
end
it "only uses existing tags" do
Fabricate(:tag, name: "totally")
expect {
@result =
post_revisor.revise!(
user,
raw: "lets totally update the body",
tags: %w[totally update],
)
}.to_not change { Tag.count }
expect(@result).to eq(true)
post.reload
expect(post.topic.tags.map(&:name)).to eq(["totally"])
end
end
end
end
context "with uploads" do
let(:image1) { Fabricate(:upload) }
let(:image2) { Fabricate(:upload) }
let(:image3) { Fabricate(:upload) }
let(:image4) { Fabricate(:upload) }
let(:post_args) { { user: user, topic: topic, raw: <<~RAW } }
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-16 04:19:41 +08:00
This is a post with multiple uploads
![image1](#{image1.short_url})
![image2](#{image2.short_url})
RAW
it "updates linked post uploads" do
post.link_post_uploads
expect(post.upload_references.pluck(:upload_id)).to contain_exactly(image1.id, image2.id)
post_revisor.revise!(user, raw: <<~RAW)
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-16 04:19:41 +08:00
This is a post with multiple uploads
![image2](#{image2.short_url})
![image3](#{image3.short_url})
![image4](#{image4.short_url})
RAW
expect(post.reload.upload_references.pluck(:upload_id)).to contain_exactly(
image2.id,
image3.id,
image4.id,
)
end
context "with secure uploads uploads" do
let!(:image5) { Fabricate(:secure_upload) }
before do
Jobs.run_immediately!
setup_s3
SiteSetting.authorized_extensions = "png|jpg|gif|mp4"
SiteSetting.secure_uploads = true
stub_upload(image5)
end
it "updates the upload secure status, which is secure by default from the composer. set to false for a public topic" do
stub_image_size
post_revisor.revise!(user, raw: <<~RAW)
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-16 04:19:41 +08:00
This is a post with a secure upload
![image5](#{image5.short_url})
RAW
expect(image5.reload.secure).to eq(false)
expect(image5.security_last_changed_reason).to eq(
FIX: Secure upload post processing race condition (#23968) * FIX: Secure upload post processing race condition This commit fixes a couple of issues. A little background -- when uploads are created in the composer for posts, regardless of whether the upload will eventually be marked secure or not, if secure_uploads is enabled we always mark the upload secure at first. This is so the upload is by default protected, regardless of post type (regular or PM) or category. This was causing issues in some rare occasions though because of the order of operations of our post creation and processing pipeline. When creating a post, we enqueue a sidekiq job to post-process the post which does various things including converting images to lightboxes. We were also enqueuing a job to update the secure status for all uploads in that post. Sometimes the secure status job would run before the post process job, marking uploads as _not secure_ in the background and changing their ACL before the post processor ran, which meant the users would see a broken image in their posts. This commit fixes that issue by always running the upload security changes inline _within_ the cooked_post_processor job. The other issue was that the lightbox wrapper link for images in the post would end up with a URL like this: ``` href="/secure-uploads/original/2X/4/4e1f00a40b6c952198bbdacae383ba77932fc542.jpeg" ``` Since we weren't actually using the `upload.url` to pass to `UrlHelper.cook_url` here, we weren't converting this href to the CDN URL if the post was not in a secure context (the UrlHelper does not know how to convert a secure-uploads URL to a CDN one). Now we always end up with the correct lightbox href. This was less of an issue than the other one, since the secure-uploads URL works even when the upload has become non-secure, but it was a good inconsistency to fix anyway.
2023-10-19 07:48:01 +08:00
"access control post dictates security | source: post processor",
)
end
it "does not update the upload secure status, which is secure by default from the composer for a private" do
post.topic.update(category: Fabricate(:private_category, group: Fabricate(:group)))
stub_image_size
post_revisor.revise!(user, raw: <<~RAW)
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-16 04:19:41 +08:00
This is a post with a secure upload
![image5](#{image5.short_url})
RAW
expect(image5.reload.secure).to eq(true)
expect(image5.security_last_changed_reason).to eq(
FIX: Secure upload post processing race condition (#23968) * FIX: Secure upload post processing race condition This commit fixes a couple of issues. A little background -- when uploads are created in the composer for posts, regardless of whether the upload will eventually be marked secure or not, if secure_uploads is enabled we always mark the upload secure at first. This is so the upload is by default protected, regardless of post type (regular or PM) or category. This was causing issues in some rare occasions though because of the order of operations of our post creation and processing pipeline. When creating a post, we enqueue a sidekiq job to post-process the post which does various things including converting images to lightboxes. We were also enqueuing a job to update the secure status for all uploads in that post. Sometimes the secure status job would run before the post process job, marking uploads as _not secure_ in the background and changing their ACL before the post processor ran, which meant the users would see a broken image in their posts. This commit fixes that issue by always running the upload security changes inline _within_ the cooked_post_processor job. The other issue was that the lightbox wrapper link for images in the post would end up with a URL like this: ``` href="/secure-uploads/original/2X/4/4e1f00a40b6c952198bbdacae383ba77932fc542.jpeg" ``` Since we weren't actually using the `upload.url` to pass to `UrlHelper.cook_url` here, we weren't converting this href to the CDN URL if the post was not in a secure context (the UrlHelper does not know how to convert a secure-uploads URL to a CDN one). Now we always end up with the correct lightbox href. This was less of an issue than the other one, since the secure-uploads URL works even when the upload has become non-secure, but it was a good inconsistency to fix anyway.
2023-10-19 07:48:01 +08:00
"access control post dictates security | source: post processor",
)
end
end
end
context "with drafts" do
it "does not advance draft sequence if keep_existing_draft option is true" do
post = Fabricate(:post, user: user)
topic = post.topic
draft_key = "topic_#{topic.id}"
data = { reply: "test 12222" }.to_json
Draft.set(user, draft_key, 0, data)
Draft.set(user, draft_key, 0, data)
expect {
PostRevisor.new(post).revise!(
post.user,
{ title: "updated title for my topic" },
keep_existing_draft: true,
)
}.to not_change {
Draft.where(user: user, draft_key: draft_key).first.sequence
}.and not_change {
DraftSequence.where(user_id: user.id, draft_key: draft_key).first.sequence
}
expect {
PostRevisor.new(post).revise!(post.user, { title: "updated title for my topic" })
}.to change { Draft.where(user: user, draft_key: draft_key).count }.from(1).to(
0,
).and change {
DraftSequence.where(user_id: user.id, draft_key: draft_key).first.sequence
}.by(1)
end
end
context "when skipping validations" do
fab!(:post) { Fabricate(:post, raw: "aaa", skip_validation: true) }
it "can revise multiple times and remove unnecessary revisions" do
post_revisor.revise!(admin, { raw: "bbb" }, skip_validations: true)
expect(post.errors).to be_empty
# Revert to old version which was invalid to destroy previously created
# post revision and trigger another post save.
post_revisor.revise!(admin, { raw: "aaa" }, skip_validations: true)
expect(post.errors).to be_empty
end
end
end
context "when the review_every_post setting is enabled" do
let(:post) { Fabricate(:post, post_args) }
let(:revisor) { PostRevisor.new(post) }
before { SiteSetting.review_every_post = true }
it "queues the post when a regular user edits it" do
expect {
revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + 10.minutes,
)
}.to change(ReviewablePost, :count).by(1)
end
it "does nothing when a staff member edits a post" do
admin = Fabricate(:admin)
expect { revisor.revise!(admin, { raw: "updated body" }) }.not_to change(
ReviewablePost,
:count,
)
end
it "skips grace period edits" do
SiteSetting.editing_grace_period = 1.minute
expect {
revisor.revise!(
post.user,
{ raw: "updated body" },
revised_at: post.updated_at + 10.seconds,
)
}.not_to change(ReviewablePost, :count)
end
end
end