diff --git a/app/controllers/modqueue_controller.rb b/app/controllers/modqueue_controller.rb index 773b2cae7..44b63fb73 100644 --- a/app/controllers/modqueue_controller.rb +++ b/app/controllers/modqueue_controller.rb @@ -4,14 +4,14 @@ class ModqueueController < ApplicationController def index authorize :modqueue - @posts = Post.includes(:appeals, :disapprovals, :uploader, flags: [:creator]).pending_or_flagged.available_for_moderation(CurrentUser.user, hidden: search_params[:hidden]) + @posts = Post.includes(:appeals, :disapprovals, :uploader, flags: [:creator]).in_modqueue.available_for_moderation(CurrentUser.user, hidden: search_params[:hidden]) @posts = @posts.paginated_search(params, order: "modqueue", count_pages: true) - @modqueue_posts = @posts.except(:offset, :limit, :order) - @pending_post_count = @modqueue_posts.pending.count - @flagged_post_count = @modqueue_posts.flagged.count - @disapproval_reasons = PostDisapproval.where(post: @modqueue_posts).where.not(reason: "disinterest").group(:reason).order(count: :desc).distinct.count(:post_id) - @uploaders = @modqueue_posts.group(:uploader).order(count: :desc).limit(20).count + @modqueue_posts = @posts.reselect(nil).reorder(nil).offset(nil).limit(nil) + @pending_post_count = @modqueue_posts.select(&:is_pending?).count + @flagged_post_count = @modqueue_posts.select(&:is_flagged?).count + @disapproval_reasons = PostDisapproval.where(post: @modqueue_posts.reselect(:id)).where.not(reason: "disinterest").group(:reason).order(count: :desc).distinct.count(:post_id) + @uploaders = @modqueue_posts.map(&:uploader).tally.sort_by(&:last).reverse.take(20).to_h @tags = RelatedTagCalculator.frequent_tags_for_post_relation(@modqueue_posts) @artist_tags = @tags.select(&:artist?).sort_by(&:overlap_count).reverse.take(10) diff --git a/app/logical/post_pruner.rb b/app/logical/post_pruner.rb index 02eef81de..924fc7152 100644 --- a/app/logical/post_pruner.rb +++ b/app/logical/post_pruner.rb @@ -1,7 +1,10 @@ -class PostPruner +module PostPruner + module_function + def prune! prune_pending! prune_flagged! + prune_appealed! end def prune_pending! @@ -11,10 +14,14 @@ class PostPruner end def prune_flagged! - Post.flagged.each do |post| - if post.flags.unresolved.old.any? - post.delete!("Unapproved in three days after returning to moderation queue", user: User.system) - end + PostFlag.expired.each do |flag| + flag.post.delete!("Unapproved in three days after returning to moderation queue", user: User.system) + end + end + + def prune_appealed! + PostAppeal.expired.each do |appeal| + appeal.post.delete!("Unapproved in three days after returning to moderation queue", user: User.system) end end end diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 0829ce3ba..9c6a03ba6 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -274,8 +274,10 @@ class PostQueryBuilder Post.pending when "flagged" Post.flagged + when "appealed" + Post.appealed when "modqueue" - Post.pending_or_flagged + Post.in_modqueue when "deleted" Post.deleted when "banned" @@ -283,7 +285,7 @@ class PostQueryBuilder when "active" Post.active when "unmoderated" - Post.pending_or_flagged.available_for_moderation(current_user, hidden: false) + Post.in_modqueue.available_for_moderation(current_user, hidden: false) when "all", "any" Post.all else @@ -307,7 +309,7 @@ class PostQueryBuilder Post.where(parent: nil) when "any" Post.where.not(parent: nil) - when /pending|flagged|modqueue|deleted|banned|active|unmoderated/ + when "pending", "flagged", "appealed", "modqueue", "deleted", "banned", "active", "unmoderated" Post.where.not(parent: nil).where(parent: status_matches(parent)) when /\A\d+\z/ Post.where(id: parent).or(Post.where(parent: parent)) @@ -322,7 +324,7 @@ class PostQueryBuilder Post.where(has_children: false) when "any" Post.where(has_children: true) - when /pending|flagged|modqueue|deleted|banned|active|unmoderated/ + when "pending", "flagged", "appealed", "modqueue", "deleted", "banned", "active", "unmoderated" Post.where(has_children: true).where(children: status_matches(child)) else Post.none @@ -606,10 +608,10 @@ class PostQueryBuilder .order("contributor_fav_count DESC, posts.fav_count DESC, posts.id DESC") when "modqueue", "modqueue_desc" - relation = relation.left_outer_joins(:flags).order(Arel.sql("GREATEST(posts.created_at, post_flags.created_at) DESC, posts.id DESC")) + relation = relation.with_queued_at.order("queued_at DESC, posts.id DESC") when "modqueue_asc" - relation = relation.left_outer_joins(:flags).order(Arel.sql("GREATEST(posts.created_at, post_flags.created_at) ASC, posts.id ASC")) + relation = relation.with_queued_at.order("queued_at ASC, posts.id ASC") when "none" relation = relation.reorder(nil) diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index 80052c527..7d5d2a47f 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -149,7 +149,7 @@ module PostSets end def not_shown(post) - post.is_deleted? && tag_string !~ /status:(?:all|any|deleted|banned)/ + post.is_deleted? && tag_string !~ /status:(?:all|any|deleted|banned|modqueue)/ end def none_shown diff --git a/app/models/application_record.rb b/app/models/application_record.rb index ccd96df91..bd366d214 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -111,6 +111,10 @@ class ApplicationRecord < ActiveRecord::Base ensure connection.execute("SET STATEMENT_TIMEOUT = #{CurrentUser.user.try(:statement_timeout) || 3_000}") unless Rails.env == "test" end + + def update!(*args) + all.each { |record| record.update!(*args) } + end end end diff --git a/app/models/post.rb b/app/models/post.rb index 5e307b5ee..a826a71ff 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -61,8 +61,9 @@ class Post < ApplicationRecord scope :pending, -> { where(is_pending: true) } scope :flagged, -> { where(is_flagged: true) } scope :banned, -> { where(is_banned: true) } - scope :active, -> { where(is_pending: false, is_deleted: false, is_flagged: false) } - scope :pending_or_flagged, -> { pending.or(flagged) } + scope :active, -> { where(is_pending: false, is_deleted: false, is_flagged: false).where.not(id: PostAppeal.pending) } + scope :appealed, -> { where(id: PostAppeal.pending.select(:post_id)) } + scope :in_modqueue, -> { pending.or(flagged).or(appealed) } scope :expired, -> { where("posts.created_at < ?", 3.days.ago) } scope :unflagged, -> { where(is_flagged: false) } @@ -281,8 +282,24 @@ class Post < ApplicationRecord end module ApprovalMethods + def in_modqueue? + is_pending? || is_flagged? || is_appealed? + end + + def is_active? + !is_deleted? && !in_modqueue? + end + + def is_appealed? + is_deleted? && appeals.any?(&:pending?) + end + + def is_appealable? + is_deleted? && !is_appealed? + end + def is_approvable?(user = CurrentUser.user) - !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && uploader != user + !is_status_locked? && !is_active? && uploader != user end def flag!(reason, is_deletion: false) @@ -991,7 +1008,10 @@ class Post < ApplicationRecord transaction do automated = (user == User.system) - flags.create!(reason: reason, is_deletion: true, creator: user) + flags.pending.update!(status: :succeeded) + appeals.pending.update!(status: :rejected) + + flags.create!(reason: reason, is_deletion: true, creator: user, status: :succeeded) update!(is_deleted: true, is_pending: false, is_flagged: false) # XXX This must happen *after* the `is_deleted` flag is set to true (issue #3419). @@ -1224,6 +1244,14 @@ class Post < ApplicationRecord relation end + def with_queued_at + relation = group(:id) + relation = relation.left_outer_joins(:flags, :appeals) + relation = relation.select("posts.*") + relation = relation.select(Arel.sql("MAX(GREATEST(posts.created_at, post_flags.created_at, post_appeals.created_at)) AS queued_at")) + relation + end + def with_stats(tables) return all if tables.empty? diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb index d758f945f..925fca5ae 100644 --- a/app/models/post_appeal.rb +++ b/app/models/post_appeal.rb @@ -5,20 +5,27 @@ class PostAppeal < ApplicationRecord belongs_to :creator, :class_name => "User" belongs_to :post - validates_presence_of :reason + validates :reason, presence: true, length: { in: 1..140 } - validate :validate_post_is_inactive - validate :validate_creator_is_not_limited - validates_uniqueness_of :creator_id, :scope => :post_id, :message => "have already appealed this post" + validate :validate_post_is_appealable, on: :create + validate :validate_creator_is_not_limited, on: :create + validates :creator, uniqueness: { scope: :post, message: "have already appealed this post" }, on: :create + + enum status: { + pending: 0, + succeeded: 1, + rejected: 2 + } scope :resolved, -> { where(post: Post.undeleted.unflagged) } scope :unresolved, -> { where(post: Post.deleted.or(Post.flagged)) } scope :recent, -> { where("post_appeals.created_at >= ?", 1.day.ago) } + scope :expired, -> { pending.where("post_appeals.created_at <= ?", 3.days.ago) } module SearchMethods def search(params) q = super - q = q.search_attributes(params, :creator, :post, :reason) + q = q.search_attributes(params, :creator, :post, :reason, :status) q = q.text_attribute_matches(:reason, params[:reason_matches]) q = q.resolved if params[:is_resolved].to_s.truthy? @@ -44,10 +51,8 @@ class PostAppeal < ApplicationRecord end end - def validate_post_is_inactive - if resolved? - errors[:post] << "is active" - end + def validate_post_is_appealable + errors[:post] << "cannot be appealed" if post.is_status_locked? || !post.is_appealable? end def appeal_count_for_creator diff --git a/app/models/post_approval.rb b/app/models/post_approval.rb index 2981143d2..b2efa6437 100644 --- a/app/models/post_approval.rb +++ b/app/models/post_approval.rb @@ -12,7 +12,7 @@ class PostApproval < ApplicationRecord errors.add(:post, "is locked and cannot be approved") end - if post.status == "active" + if post.is_active? errors.add(:post, "is already active and cannot be approved") end @@ -28,7 +28,9 @@ class PostApproval < ApplicationRecord def approve_post is_undeletion = post.is_deleted - post.flags.each(&:resolve!) + post.flags.pending.update!(status: :rejected) + post.appeals.pending.update!(status: :succeeded) + post.update(approver: user, is_flagged: false, is_pending: false, is_deleted: false) ModAction.log("undeleted post ##{post_id}", :post_undelete) if is_undeletion diff --git a/app/models/post_disapproval.rb b/app/models/post_disapproval.rb index bf00344fe..fc1076d6b 100644 --- a/app/models/post_disapproval.rb +++ b/app/models/post_disapproval.rb @@ -46,7 +46,7 @@ class PostDisapproval < ApplicationRecord end def validate_disapproval - if post.status == "active" + if post.is_active? errors[:post] << "is already active and cannot be disapproved" end end diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 1a2febf0b..4f8542723 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -17,13 +17,19 @@ class PostFlag < ApplicationRecord before_save :update_post attr_accessor :is_deletion + enum status: { + pending: 0, + succeeded: 1, + rejected: 2 + } + scope :by_users, -> { where.not(creator: User.system) } scope :by_system, -> { where(creator: User.system) } scope :in_cooldown, -> { by_users.where("created_at >= ?", COOLDOWN_PERIOD.ago) } scope :resolved, -> { where(is_resolved: true) } scope :unresolved, -> { where(is_resolved: false) } scope :recent, -> { where("post_flags.created_at >= ?", 1.day.ago) } - scope :old, -> { where("post_flags.created_at <= ?", 3.days.ago) } + scope :expired, -> { pending.where("post_flags.created_at <= ?", 3.days.ago) } module SearchMethods def creator_matches(creator, searcher) @@ -56,7 +62,7 @@ class PostFlag < ApplicationRecord def search(params) q = super - q = q.search_attributes(params, :post, :is_resolved, :reason) + q = q.search_attributes(params, :post, :is_resolved, :reason, :status) q = q.text_attribute_matches(:reason, params[:reason_matches]) if params[:creator_id].present? @@ -113,12 +119,8 @@ class PostFlag < ApplicationRecord def validate_post errors[:post] << "is pending and cannot be flagged" if post.is_pending? && !is_deletion + errors[:post] << "is deleted and cannot be flagged" if post.is_deleted? && !is_deletion errors[:post] << "is locked and cannot be flagged" if post.is_status_locked? - errors[:post] << "is deleted" if post.is_deleted? - end - - def resolve! - update_column(:is_resolved, true) end def flag_count_for_creator diff --git a/app/presenters/post_presenter.rb b/app/presenters/post_presenter.rb index aed0843b5..2f956e469 100644 --- a/app/presenters/post_presenter.rb +++ b/app/presenters/post_presenter.rb @@ -7,7 +7,7 @@ class PostPresenter return "none".html_safe end - if !options[:show_deleted] && post.is_deleted? && options[:tags] !~ /status:(?:all|any|deleted|banned)/ + if !options[:show_deleted] && post.is_deleted? && options[:tags] !~ /status:(?:all|any|deleted|banned|modqueue)/ return "" end diff --git a/app/views/modqueue/_post.html.erb b/app/views/modqueue/_post.html.erb index dfa6d9865..ce523b029 100644 --- a/app/views/modqueue/_post.html.erb +++ b/app/views/modqueue/_post.html.erb @@ -1,6 +1,6 @@ <%= content_tag(:div, { id: "post-#{post.id}", class: ["post", "mod-queue-preview", "column-container", *PostPresenter.preview_class(post)].join(" ") }.merge(PostPresenter.data_attributes(post))) do %>
diff --git a/app/views/posts/partials/show/_information.html.erb b/app/views/posts/partials/show/_information.html.erb index 4e6dae192..dcfc2b11c 100644 --- a/app/views/posts/partials/show/_information.html.erb +++ b/app/views/posts/partials/show/_information.html.erb @@ -43,22 +43,18 @@ Status: <% if post.is_pending? %> Pending - <% end %> - - <% if post.is_deleted? %> - Deleted - <% end %> - - <% if post.is_flagged? %> + <% elsif post.is_flagged? %> Flagged + <% elsif post.is_appealed? %> + Appealed + <% elsif post.is_deleted? %> + Deleted + <% else %> + Active <% end %> <% if post.is_banned? %> Banned <% end %> - - <% if !post.is_pending? && !post.is_deleted? && !post.is_banned? %> - Active - <% end %> diff --git a/app/views/posts/partials/show/_notices.html.erb b/app/views/posts/partials/show/_notices.html.erb index d3119ea8f..256d33109 100644 --- a/app/views/posts/partials/show/_notices.html.erb +++ b/app/views/posts/partials/show/_notices.html.erb @@ -1,4 +1,4 @@ -<% if post.is_flagged? && !post.is_deleted? && post.flags.any? %> +<% if post.is_flagged? %>

This post was flagged for review (<%= link_to_wiki "learn more", "howto:flag" %>):

@@ -30,13 +30,14 @@
<% end %> -<% if post.is_pending? || post.is_flagged? %> +<% if post.in_modqueue? %>
<% if post.is_pending? %> - This post is pending approval. - (<%= link_to_wiki "learn more", "about:mod_queue" %>) - <% else %> + This post is pending approval. (<%= link_to_wiki "learn more", "about:mod_queue" %>) + <% elsif post.is_flagged? %> This post was flagged and is pending approval (<%= link_to_wiki "learn more", "about:mod_queue" %>) + <% elsif post.is_appealed? %> + This post was appealed and is pending approval (<%= link_to_wiki "learn more", "about:mod_queue" %>) <% end %> <%= render "post_disapprovals/counts", :disapprovals => post.disapprovals, :post => post %> @@ -48,7 +49,8 @@
<% end %> -<% if (post.is_flagged? || post.is_deleted?) && post.appeals.any? %> +<% #XXX %> +<% if post.is_deleted? && post.appeals.any? %>

This post was appealed:

<%= render "post_appeals/reasons", appeals: post.appeals %> diff --git a/app/views/posts/partials/show/_options.html.erb b/app/views/posts/partials/show/_options.html.erb index e55116e2f..d071f7bb6 100644 --- a/app/views/posts/partials/show/_options.html.erb +++ b/app/views/posts/partials/show/_options.html.erb @@ -51,25 +51,24 @@ <% if post.is_status_locked? %>
  • Status locked
  • <% else %> - <% if (!post.is_deleted? && !post.is_pending? && !post.is_flagged?) && policy(PostFlag).create? %> + <% if post.is_active? && policy(PostFlag).create? %>
  • <%= link_to "Flag", new_post_flag_path(post_flag: { post_id: post.id }), remote: true %>
  • - <% elsif (post.is_flagged? || post.is_deleted?) && policy(PostAppeal).create? %> + <% elsif post.is_appealable? && policy(PostAppeal).create? %>
  • <%= link_to "Appeal", new_post_appeal_path(post_appeal: { post_id: post.id }), remote: true %>
  • <% end %> <% if policy(PostApproval).create? %> - <% if post.is_deleted? %> -
  • <%= link_to "Undelete", post_approvals_path(post_id: post.id), remote: true, method: :post, "data-confirm": "Are you sure you want to undelete this post?" %>
  • - <% if policy(post).move_favorites? %> -
  • <%= link_to "Move favorites", confirm_move_favorites_moderator_post_post_path(post_id: post.id) %>
  • - <% end %> - <% elsif policy(post).delete? %> -
  • <%= link_to "Delete", post, method: :delete, remote: true %>
  • + <% if post.is_approvable? %> +
  • <%= link_to (post.is_deleted? ? "Undelete" : "Approve"), post_approvals_path(post_id: post.id), remote: true, method: :post, "data-shortcut": "shift+o", "data-confirm": "Are you sure you want to approve this post?" %>
  • +
  • <%= link_to "Hide from queue", post_disapprovals_path(post_disapproval: { post_id: post.id, reason: "disinterest" }), remote: true, method: :post %>
  • <% end %> - <% if post.is_approvable? && !post.is_deleted? %> -
  • <%= link_to "Approve", post_approvals_path(post_id: post.id), remote: true, method: :post, id: "approve", "data-shortcut": "shift+o", "data-confirm": "Are you sure you want to approve this post?" %>
  • -
  • <%= link_to "Hide from queue", post_disapprovals_path(post_disapproval: { post_id: post.id, reason: "disinterest" }), remote: true, method: :post, id: "disapprove" %>
  • + <% if post.is_deleted? && policy(post).move_favorites? %> +
  • <%= link_to "Move favorites", confirm_move_favorites_moderator_post_post_path(post_id: post.id) %>
  • + <% end %> + + <% if !post.is_deleted? && policy(post).delete? %> +
  • <%= link_to "Delete", post, method: :delete, remote: true %>
  • <% end %> <% if policy(post).unban? %> diff --git a/db/migrate/20200803022359_add_status_to_post_appeals_and_flags.rb b/db/migrate/20200803022359_add_status_to_post_appeals_and_flags.rb new file mode 100644 index 000000000..fd2420b67 --- /dev/null +++ b/db/migrate/20200803022359_add_status_to_post_appeals_and_flags.rb @@ -0,0 +1,9 @@ +class AddStatusToPostAppealsAndFlags < ActiveRecord::Migration[6.0] + def change + add_column :post_appeals, :status, :integer, default: 0, null: false + add_index :post_appeals, :status + + add_column :post_flags, :status, :integer, default: 0, null: false + add_index :post_flags, :status + end +end diff --git a/db/structure.sql b/db/structure.sql index c8e7ff934..e600fba4c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2584,7 +2584,8 @@ CREATE TABLE public.post_appeals ( creator_id integer NOT NULL, reason text, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + status integer DEFAULT 0 NOT NULL ); @@ -2684,7 +2685,8 @@ CREATE TABLE public.post_flags ( reason text, is_resolved boolean DEFAULT false NOT NULL, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + status integer DEFAULT 0 NOT NULL ); @@ -6622,6 +6624,13 @@ CREATE INDEX index_post_appeals_on_post_id ON public.post_appeals USING btree (p CREATE INDEX index_post_appeals_on_reason_tsvector ON public.post_appeals USING gin (to_tsvector('english'::regconfig, reason)); +-- +-- Name: index_post_appeals_on_status; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_post_appeals_on_status ON public.post_appeals USING btree (status); + + -- -- Name: index_post_approvals_on_post_id; Type: INDEX; Schema: public; Owner: - -- @@ -6671,6 +6680,13 @@ CREATE INDEX index_post_flags_on_post_id ON public.post_flags USING btree (post_ CREATE INDEX index_post_flags_on_reason_tsvector ON public.post_flags USING gin (to_tsvector('english'::regconfig, reason)); +-- +-- Name: index_post_flags_on_status; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_post_flags_on_status ON public.post_flags USING btree (status); + + -- -- Name: index_post_replacements_on_creator_id; Type: INDEX; Schema: public; Owner: - -- @@ -7393,6 +7409,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200325074859'), ('20200403210353'), ('20200406054838'), -('20200427190519'); +('20200427190519'), +('20200803022359'); diff --git a/script/fixes/066_initialize_appeal_and_flag_statuses.rb b/script/fixes/066_initialize_appeal_and_flag_statuses.rb new file mode 100755 index 000000000..c48e7079a --- /dev/null +++ b/script/fixes/066_initialize_appeal_and_flag_statuses.rb @@ -0,0 +1,26 @@ +#!/usr/bin/env ruby + +require_relative "../../config/environment" + +PostFlag.transaction do + # Mark all old flags and appeals as succeeded or rejected. Recent flags and + # appeals are left as pending. This is not strictly correct for posts that + # may have been flagged or appealed multiple times. + PostAppeal.expired.where(post: Post.undeleted).update_all(status: "succeeded") + PostAppeal.expired.where(post: Post.deleted).update_all(status: "rejected") + PostFlag.where(post: Post.undeleted).update_all(status: "rejected") + PostFlag.where(post: Post.deleted).update_all(status: "succeeded") + + # Mark all unapproved in three days flags as successful. + PostFlag.category_matches("deleted").update_all(status: "succeeded") + + # Mark all currently flagged posts as pending. + PostFlag.where(post: Post.flagged).update_all(status: "pending") + + puts "Appeals pending: #{PostAppeal.pending.count}" + puts "Appeals succeeded: #{PostAppeal.succeeded.count}" + puts "Appeals rejected: #{PostAppeal.rejected.count}" + puts "Flags pending: #{PostFlag.pending.count}" + puts "Flags succeeded: #{PostFlag.succeeded.count}" + puts "Flags rejected: #{PostFlag.rejected.count}" +end diff --git a/test/factories/post_flag.rb b/test/factories/post_flag.rb index ed7c3d942..baf373052 100644 --- a/test/factories/post_flag.rb +++ b/test/factories/post_flag.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory(:post_flag) do creator - post + post { build(:post, is_flagged: true) } reason {"xxx"} is_resolved {false} end diff --git a/test/functional/modqueue_controller_test.rb b/test/functional/modqueue_controller_test.rb index 9ab23dc88..8312ea051 100644 --- a/test/functional/modqueue_controller_test.rb +++ b/test/functional/modqueue_controller_test.rb @@ -13,6 +13,14 @@ class ModqueueControllerTest < ActionDispatch::IntegrationTest get_auth modqueue_index_path, @admin assert_response :success end + + should "include appealed posts in the modqueue" do + @appeal = create(:post_appeal) + get_auth modqueue_index_path, @admin + + assert_response :success + assert_select "#post-#{@appeal.post_id}" + end end end end diff --git a/test/functional/post_appeals_controller_test.rb b/test/functional/post_appeals_controller_test.rb index faa5982de..a8dcc1607 100644 --- a/test/functional/post_appeals_controller_test.rb +++ b/test/functional/post_appeals_controller_test.rb @@ -48,14 +48,54 @@ class PostAppealsControllerTest < ActionDispatch::IntegrationTest end context "create action" do - setup do - @post = as(@user) { create(:post, is_deleted: true) } + context "appealing a deleted post" do + should "create a new appeal" do + @post = create(:post, is_deleted: true) + + assert_difference("PostAppeal.count", 1) do + post_auth post_appeals_path, @user, params: { post_appeal: { post_id: @post.id, reason: "xxx" }}, as: :json + end + + assert_response :success + end end - should "create a new appeal" do - assert_difference("PostAppeal.count", 1) do - post_auth post_appeals_path, @user, params: {:format => "js", :post_appeal => {:post_id => @post.id, :reason => "xxx"}} - assert_response :success + context "appealing a flagged post" do + should "fail" do + @flag = create(:post_flag) + + assert_no_difference("PostAppeal.count") do + post_auth post_appeals_path, @user, params: { post_appeal: { post_id: @flag.post.id, reason: "xxx" }}, as: :json + end + + assert_response 422 + assert_equal(["cannot be appealed"], response.parsed_body.dig("errors", "post")) + end + end + + context "appealing a pending post" do + should "fail" do + @post = create(:post, is_pending: true) + + assert_no_difference("PostAppeal.count") do + post_auth post_appeals_path, @user, params: { post_appeal: { post_id: @post.id, reason: "xxx" }}, as: :json + end + + assert_response 422 + assert_equal(["cannot be appealed"], response.parsed_body.dig("errors", "post")) + end + end + + context "appealing an already appealed post" do + should "fail" do + @appeal = create(:post_appeal) + + assert_no_difference("PostAppeal.count") do + post_auth post_appeals_path, @user, params: { post_appeal: { post_id: @appeal.post.id, reason: "xxx" }}, as: :json + end + + assert_response 422 + assert_equal(["cannot be appealed"], response.parsed_body.dig("errors", "post")) end end end diff --git a/test/functional/post_approvals_controller_test.rb b/test/functional/post_approvals_controller_test.rb index e4b93c65f..813cf62ef 100644 --- a/test/functional/post_approvals_controller_test.rb +++ b/test/functional/post_approvals_controller_test.rb @@ -27,6 +27,29 @@ class PostApprovalsControllerTest < ActionDispatch::IntegrationTest end end + context "for an appealed post" do + should "undelete the post and mark the appeal as successful" do + @appeal = create(:post_appeal) + post_auth post_approvals_path(post_id: @appeal.post_id, format: :js), @approver + + assert_response :success + assert_equal(false, @appeal.reload.post.is_deleted?) + assert_equal(true, @appeal.succeeded?) + end + end + + context "for a flagged post" do + should "approve the post and mark the flag as rejected" do + @flag = create(:post_flag) + post_auth post_approvals_path(post_id: @flag.post_id, format: :js), @approver + + assert_response :success + assert_equal(false, @flag.reload.post.is_deleted?) + assert_equal(false, @flag.post.is_flagged?) + assert_equal(true, @flag.rejected?) + end + end + should "not allow non-approvers to approve posts" do @post = create(:post, is_pending: true) post_auth post_approvals_path(post_id: @post.id, format: :js), create(:user) diff --git a/test/functional/post_events_controller_test.rb b/test/functional/post_events_controller_test.rb index af3f42b9b..623b9d2c1 100644 --- a/test/functional/post_events_controller_test.rb +++ b/test/functional/post_events_controller_test.rb @@ -10,6 +10,7 @@ class PostEventsControllerTest < ActionDispatch::IntegrationTest as(@user) do @post = create(:post) @post.flag!("aaa") + @post.update(is_deleted: true) create(:post_appeal, post: @post) @post.approve!(@mod) end diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index f3aeb2826..e70c2d358 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -483,7 +483,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest create(:note, post: @post) create(:artist_commentary, post: @post) create(:post_flag, post: @post, creator: @user) - create(:post_appeal, post: @post, creator: @user) + #create(:post_appeal, post: @post, creator: @user) create(:post_vote, post: @post, user: @user) create(:favorite, post: @post, user: @user) create(:moderation_report, model: @comment, creator: @builder) @@ -667,7 +667,6 @@ class PostsControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_equal(false, @post.is_deleted?) end - end context "revert action" do diff --git a/test/unit/post_appeal_test.rb b/test/unit/post_appeal_test.rb index ef0b9c27f..a37906ce2 100644 --- a/test/unit/post_appeal_test.rb +++ b/test/unit/post_appeal_test.rb @@ -32,7 +32,7 @@ class PostAppealTest < ActiveSupport::TestCase @post_appeal = build(:post_appeal, post: @post, creator: @alice) assert_equal(false, @post_appeal.valid?) - assert_equal(["Post is active"], @post_appeal.errors.full_messages) + assert_equal(["Post cannot be appealed"], @post_appeal.errors.full_messages) end end end diff --git a/test/unit/post_event_test.rb b/test/unit/post_event_test.rb index 460414504..46072a443 100644 --- a/test/unit/post_event_test.rb +++ b/test/unit/post_event_test.rb @@ -5,6 +5,7 @@ class PostEventTest < ActiveSupport::TestCase @user = create(:user, created_at: 2.weeks.ago) @post = create(:post) @post_flag = create(:post_flag, creator: @user, post: @post) + @post.update(is_deleted: true) @post_appeal = create(:post_appeal, creator: @user, post: @post) end diff --git a/test/unit/post_flag_test.rb b/test/unit/post_flag_test.rb index a59a0eb49..ced0cbc36 100644 --- a/test/unit/post_flag_test.rb +++ b/test/unit/post_flag_test.rb @@ -53,7 +53,7 @@ class PostFlagTest < ActiveSupport::TestCase @post_flag = build(:post_flag, post: @post, creator: @bob) @post_flag.save - assert_equal(["Post is deleted"], @post_flag.errors.full_messages) + assert_equal(["Post is deleted and cannot be flagged"], @post_flag.errors.full_messages) end should "not be able to flag a pending post" do diff --git a/test/unit/post_pruner_test.rb b/test/unit/post_pruner_test.rb index 09e491fa7..dd288eca0 100644 --- a/test/unit/post_pruner_test.rb +++ b/test/unit/post_pruner_test.rb @@ -1,30 +1,108 @@ require 'test_helper' class PostPrunerTest < ActiveSupport::TestCase - def setup - @old_post = FactoryBot.create(:post, :created_at => 5.days.ago, :is_pending => true) - @unresolved_flagged_post = FactoryBot.create(:post, :is_flagged => true) - @resolved_flagged_post = FactoryBot.create(:post, :is_flagged => true) + context "PostPruner" do + context "for a pending post" do + should "prune expired posts" do + @post = create(:post, created_at: 5.days.ago, is_pending: true) + PostPruner.prune! - @flagger = create(:gold_user, created_at: 2.weeks.ago) - @unresolved_post_flag = create(:post_flag, creator: @flagger, created_at: 5.days.ago, is_resolved: false, post: @unresolved_flagged_post) - @resolved_post_flag = create(:post_flag, creator: @flagger, created_at: 5.days.ago, is_resolved: true, post: @resolved_flagged_post) + assert_equal(true, @post.reload.is_deleted?) + assert_equal(false, @post.is_pending?) - PostPruner.new.prune! - end + assert_equal(1, @post.flags.size) + assert_equal("Unapproved in three days", @post.flags.last.reason) + end + end - should "prune old pending posts" do - @old_post.reload - assert(@old_post.is_deleted?) - end + context "for a flagged post" do + should "prune expired flags" do + @post = create(:post, created_at: 4.weeks.ago, is_flagged: true) + @flag = create(:post_flag, post: @post, created_at: 5.days.ago) + PostPruner.prune! - should "prune old flagged posts that are still unresolved" do - @unresolved_flagged_post.reload - assert(@unresolved_flagged_post.is_deleted?) - end + assert_equal(true, @post.reload.is_deleted?) + assert_equal(false, @post.is_pending?) + assert_equal(false, @post.is_flagged?) + assert_equal(true, @flag.reload.succeeded?) - should "not prune old flagged posts that are resolved" do - @resolved_flagged_post.reload - assert(!@resolved_flagged_post.is_deleted?) + assert_equal(2, @post.flags.size) + assert_equal("Unapproved in three days after returning to moderation queue", @post.flags.last.reason) + end + + should "not prune unexpired flags" do + @post = create(:post, created_at: 4.weeks.ago, is_flagged: true) + @flag = create(:post_flag, post: @post, created_at: 1.day.ago) + PostPruner.prune! + + assert_equal(false, @post.reload.is_deleted?) + assert_equal(false, @post.is_pending?) + assert_equal(true, @post.is_flagged?) + assert_equal(true, @flag.reload.pending?) + + assert_equal(1, @post.flags.size) + end + + should "leave the status of old flags unchanged" do + @post = create(:post, created_at: 4.weeks.ago, is_flagged: true) + @flag1 = create(:post_flag, post: @post, created_at: 3.weeks.ago, status: :succeeded) + @flag2 = create(:post_flag, post: @post, created_at: 2.weeks.ago, status: :rejected) + @flag3 = create(:post_flag, post: @post, created_at: 1.weeks.ago, status: :pending) + PostPruner.prune! + + assert_equal(true, @post.reload.is_deleted?) + assert_equal(false, @post.is_pending?) + assert_equal(false, @post.is_flagged?) + + assert_equal(true, @flag1.reload.succeeded?) + assert_equal(true, @flag2.reload.rejected?) + assert_equal(true, @flag3.reload.succeeded?) + end + end + + context "for an appealed post" do + should "prune expired appeals" do + @post = create(:post, created_at: 4.weeks.ago, is_deleted: true) + @appeal = create(:post_appeal, post: @post, created_at: 5.days.ago) + PostPruner.prune! + + assert_equal(false, @post.reload.is_pending?) + assert_equal(false, @post.is_flagged?) + assert_equal(true, @post.is_deleted?) + assert_equal(true, @appeal.reload.rejected?) + + assert_equal(1, @post.flags.size) + assert_equal("Unapproved in three days after returning to moderation queue", @post.flags.last.reason) + end + + should "not prune unexpired appeals" do + @post = create(:post, created_at: 4.weeks.ago, is_deleted: true) + @appeal = create(:post_appeal, post: @post, created_at: 1.day.ago) + PostPruner.prune! + + assert_equal(false, @post.reload.is_pending?) + assert_equal(false, @post.is_flagged?) + assert_equal(true, @post.is_deleted?) + assert_equal(true, @appeal.reload.pending?) + + assert_equal(0, @post.flags.size) + end + + should "leave the status of old appeals unchanged" do + @post = create(:post, created_at: 4.weeks.ago, is_deleted: true) + @appeal1 = create(:post_appeal, post: @post, created_at: 3.weeks.ago, status: :succeeded) + @appeal2 = create(:post_appeal, post: @post, created_at: 2.weeks.ago, status: :rejected) + @appeal3 = create(:post_appeal, post: @post, created_at: 1.weeks.ago, status: :pending) + PostPruner.prune! + + assert_equal(true, @post.reload.is_deleted?) + assert_equal(false, @post.is_pending?) + assert_equal(false, @post.is_flagged?) + + assert_equal(true, @appeal1.reload.succeeded?) + assert_equal(true, @appeal2.reload.rejected?) + assert_equal(false, @appeal3.reload.pending?) + end + end end end diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index 54b8ca27f..5810a9b92 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -584,22 +584,26 @@ class PostQueryBuilderTest < ActiveSupport::TestCase flagged = create(:post, is_flagged: true) deleted = create(:post, is_deleted: true) banned = create(:post, is_banned: true) - all = [banned, deleted, flagged, pending] + appealed = create(:post, is_deleted: true) + appeal = create(:post_appeal, post: appealed) + all = [appealed, banned, deleted, flagged, pending] - assert_tag_match([flagged, pending], "status:modqueue") + assert_tag_match([appealed, flagged, pending], "status:modqueue") assert_tag_match([pending], "status:pending") assert_tag_match([flagged], "status:flagged") - assert_tag_match([deleted], "status:deleted") + assert_tag_match([appealed], "status:appealed") + assert_tag_match([appealed, deleted], "status:deleted") assert_tag_match([banned], "status:banned") assert_tag_match([banned], "status:active") assert_tag_match([banned], "status:active status:banned") assert_tag_match(all, "status:any") assert_tag_match(all, "status:all") - assert_tag_match(all - [flagged, pending], "-status:modqueue") + assert_tag_match(all - [flagged, pending, appealed], "-status:modqueue") assert_tag_match(all - [pending], "-status:pending") assert_tag_match(all - [flagged], "-status:flagged") - assert_tag_match(all - [deleted], "-status:deleted") + assert_tag_match(all - [appealed], "-status:appealed") + assert_tag_match(all - [deleted, appealed], "-status:deleted") assert_tag_match(all - [banned], "-status:banned") assert_tag_match(all - [banned], "-status:active") @@ -611,11 +615,13 @@ class PostQueryBuilderTest < ActiveSupport::TestCase flagged = create(:post, is_flagged: true) pending = create(:post, is_pending: true) disapproved = create(:post, is_pending: true) + appealed = create(:post, is_deleted: true) create(:post_flag, post: flagged, creator: create(:user, created_at: 2.weeks.ago)) + create(:post_appeal, post: appealed) create(:post_disapproval, user: CurrentUser.user, post: disapproved, reason: "disinterest") - assert_tag_match([pending, flagged], "status:unmoderated") + assert_tag_match([appealed, pending, flagged], "status:unmoderated") assert_tag_match([disapproved], "-status:unmoderated") end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index c65956a32..c45d83bc3 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -513,24 +513,23 @@ class PostTest < ActiveSupport::TestCase end context "A status locked post" do - setup do - @post = FactoryBot.create(:post, is_status_locked: true) - end - should "not allow new flags" do assert_raises(PostFlag::Error) do + @post = create(:post, is_status_locked: true) @post.flag!("wrong") end end should "not allow new appeals" do + @post = create(:post, is_status_locked: true, is_deleted: true) @appeal = build(:post_appeal, post: @post) assert_equal(false, @appeal.valid?) - assert_equal(["Post is active"], @appeal.errors.full_messages) + assert_equal(["Post cannot be appealed"], @appeal.errors.full_messages) end should "not allow approval" do + @post = create(:post, is_status_locked: true, is_pending: true) approval = @post.approve! assert_includes(approval.errors.full_messages, "Post is locked and cannot be approved") end diff --git a/test/unit/upload_limit_test.rb b/test/unit/upload_limit_test.rb index f58c8c95a..06127b4fb 100644 --- a/test/unit/upload_limit_test.rb +++ b/test/unit/upload_limit_test.rb @@ -12,7 +12,7 @@ class UploadLimitTest < ActiveSupport::TestCase @post = create(:post, uploader: @user, is_pending: true, created_at: 7.days.ago) assert_equal(1000, @user.reload.upload_points) - PostPruner.new.prune! + PostPruner.prune! assert_equal(967, @user.reload.upload_points) end end