From 3a17b5a13ea98a16d6ea3fdb2cc73df0e45b0046 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 7 Aug 2020 18:53:25 -0500 Subject: [PATCH] flags/appeals: replace is_resolved flag with statuses. Replace references to the `is_resolved` field with the `status` field. Post flags were marked as resolved when a post was approved (but not when the post was deleted because it went unapproved). The status field supercedes the resolved field. --- app/javascript/src/styles/specific/posts.scss | 5 ----- app/logical/post_query_builder.rb | 2 +- app/models/post.rb | 4 +--- app/models/post_appeal.rb | 13 ------------- app/models/post_event.rb | 18 +++++++++++++----- app/models/post_flag.rb | 4 +--- app/views/post_appeals/_search.html.erb | 2 +- app/views/post_appeals/index.html.erb | 4 ++-- app/views/post_events/index.html.erb | 14 ++++++++------ app/views/post_flags/_reasons.html.erb | 4 ---- app/views/post_flags/_search.html.erb | 2 +- app/views/post_flags/index.html.erb | 4 ++-- test/factories/post_flag.rb | 1 - test/functional/post_events_controller_test.rb | 10 +++------- 14 files changed, 33 insertions(+), 54 deletions(-) diff --git a/app/javascript/src/styles/specific/posts.scss b/app/javascript/src/styles/specific/posts.scss index d4eeadc9f..90a90acc9 100644 --- a/app/javascript/src/styles/specific/posts.scss +++ b/app/javascript/src/styles/specific/posts.scss @@ -263,11 +263,6 @@ div#c-posts { margin-bottom: 0; } - .resolved { - margin-left: 0.5em; - font-weight: bold; - } - &.post-notice-parent { background: var(--post-parent-notice-background); } &.post-notice-child { background: var(--post-child-notice-background); } &.post-notice-pending { background: var(--post-pending-notice-background); } diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 9c6a03ba6..2d2835bcc 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -6,7 +6,7 @@ class PostQueryBuilder COUNT_METATAGS = %w[ comment_count deleted_comment_count active_comment_count note_count deleted_note_count active_note_count - flag_count resolved_flag_count unresolved_flag_count + flag_count child_count deleted_child_count active_child_count pool_count deleted_pool_count active_pool_count series_pool_count collection_pool_count appeal_count approval_count replacement_count diff --git a/app/models/post.rb b/app/models/post.rb index 4db165be3..3786dd2c8 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -303,7 +303,7 @@ class Post < ApplicationRecord end def flag!(reason, is_deletion: false) - flag = flags.create(reason: reason, is_resolved: false, is_deletion: is_deletion, creator: CurrentUser.user) + flag = flags.create(reason: reason, is_deletion: is_deletion, creator: CurrentUser.user) if flag.errors.any? raise PostFlag::Error.new(flag.errors.full_messages.join("; ")) @@ -1201,8 +1201,6 @@ class Post < ApplicationRecord def with_flag_stats relation = left_outer_joins(:flags).group(:id).select("posts.*") relation = relation.select("COUNT(post_flags.id) AS flag_count") - relation = relation.select("COUNT(post_flags.id) FILTER (WHERE post_flags.is_resolved = TRUE) AS resolved_flag_count") - relation = relation.select("COUNT(post_flags.id) FILTER (WHERE post_flags.is_resolved = FALSE) AS unresolved_flag_count") relation end diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb index 925fca5ae..5fe36eb5b 100644 --- a/app/models/post_appeal.rb +++ b/app/models/post_appeal.rb @@ -17,8 +17,6 @@ class PostAppeal < ApplicationRecord 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) } @@ -28,23 +26,12 @@ class PostAppeal < ApplicationRecord 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? - q = q.unresolved if params[:is_resolved].to_s.falsy? - q.apply_default_order(params) end end extend SearchMethods - def resolved? - post.present? && !post.is_deleted? && !post.is_flagged? - end - - def is_resolved - resolved? - end - def validate_creator_is_not_limited if appeal_count_for_creator >= MAX_APPEALS_PER_DAY errors[:creator] << "can appeal at most #{MAX_APPEALS_PER_DAY} post a day" diff --git a/app/models/post_event.rb b/app/models/post_event.rb index bbad886c6..074c0c2ae 100644 --- a/app/models/post_event.rb +++ b/app/models/post_event.rb @@ -30,10 +30,6 @@ class PostEvent event.try(:reason) || "" end - def is_resolved - event.try(:is_resolved) || false - end - def creator_id event.try(:creator_id) || event.try(:user_id) end @@ -42,6 +38,18 @@ class PostEvent event.try(:creator) || event.try(:user) end + def status + if event.is_a?(PostApproval) + "approved" + elsif (event.is_a?(PostAppeal) && event.succeeded?) || (event.is_a?(PostFlag) && event.rejected?) + "approved" + elsif (event.is_a?(PostAppeal) && event.rejected?) || (event.is_a?(PostFlag) && event.succeeded?) + "deleted" + else + "pending" + end + end + def is_creator_visible?(user = CurrentUser.user) case event when PostAppeal, PostApproval @@ -57,7 +65,7 @@ class PostEvent "creator_id": nil, "created_at": nil, "reason": nil, - "is_resolved": nil, + "status": nil, "type": nil } end diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 21046c68d..4dcbd86df 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -26,8 +26,6 @@ class PostFlag < ApplicationRecord 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 :expired, -> { pending.where("post_flags.created_at <= ?", 3.days.ago) } @@ -62,7 +60,7 @@ class PostFlag < ApplicationRecord def search(params) q = super - q = q.search_attributes(params, :post, :is_resolved, :reason, :status) + q = q.search_attributes(params, :post, :reason, :status) q = q.text_attribute_matches(:reason, params[:reason_matches]) if params[:creator_id].present? diff --git a/app/views/post_appeals/_search.html.erb b/app/views/post_appeals/_search.html.erb index 2e8f24bac..15fa4e243 100644 --- a/app/views/post_appeals/_search.html.erb +++ b/app/views/post_appeals/_search.html.erb @@ -3,6 +3,6 @@ <%= f.input :post_tags_match, label: "Tags", input_html: { value: params[:search][:post_tags_match], data: { autocomplete: "tag-query" } } %> <%= f.input :post_id, label: "Post ID", input_html: { value: params[:search][:post_id] } %> <%= f.input :creator_name, label: "Creator", input_html: { value: params[:search][:creator_name], data: { autocomplete: "user" } } %> - <%= f.input :is_resolved, label: "Resolved?", collection: [["Yes", true], ["No", false]], include_blank: true, selected: params[:search][:is_resolved] %> + <%= f.input :status, collection: PostAppeal.statuses, include_blank: true, selected: params[:search][:status] %> <%= f.submit "Search" %> <% end %> diff --git a/app/views/post_appeals/index.html.erb b/app/views/post_appeals/index.html.erb index 4b59adf8d..48d16c180 100644 --- a/app/views/post_appeals/index.html.erb +++ b/app/views/post_appeals/index.html.erb @@ -16,8 +16,8 @@ <% t.column "Appeals", width: "1%" do |post_appeal| %> <%= link_to post_appeal.post.appeals.size, post_appeals_path(search: { post_id: post_appeal.post_id }) %> <% end %> - <% t.column "Resolved?", width: "5%" do |post_appeal| %> - <%= link_to post_appeal.is_resolved.to_s, post_appeals_path(search: params[:search].merge(is_resolved: post_appeal.is_resolved)) %> + <% t.column "Status", width: "5%" do |post_appeal| %> + <%= link_to post_appeal.status, post_appeals_path(search: { status: post_appeal.status }) %> <% end %> <% t.column "Uploaded", width: "15%" do |post_appeal| %> <%= compact_time post_appeal.post.created_at %> diff --git a/app/views/post_events/index.html.erb b/app/views/post_events/index.html.erb index 5f27ee763..02c9ae080 100644 --- a/app/views/post_events/index.html.erb +++ b/app/views/post_events/index.html.erb @@ -4,6 +4,14 @@ <%= table_for @events, class: "striped autofit", width: "100%" do |t| %> <% t.column :type_name, name: "Type" %> + <% t.column "Description", td: { class: "col-expand" } do |event| %> +
+ <%= format_text event.reason %> +
+ <% end %> + <% t.column "Status" do |event| %> + <%= event.status %> + <% end %> <% t.column "User" do |event| %> <% if event.is_creator_visible? %> <%= link_to_user event.creator %> @@ -12,12 +20,6 @@ <% end %>
<%= time_ago_in_words_tagged event.created_at %> <% end %> - <% t.column "Description", td: { class: "col-expand" } do |event| %> -
- <%= format_text event.reason %> -
- <% end %> - <% t.column :is_resolved, name: "Resolved" %> <% end %> diff --git a/app/views/post_flags/_reasons.html.erb b/app/views/post_flags/_reasons.html.erb index 7395e182a..e0664076d 100644 --- a/app/views/post_flags/_reasons.html.erb +++ b/app/views/post_flags/_reasons.html.erb @@ -8,10 +8,6 @@ <% end %> - <%= time_ago_in_words_tagged(flag.created_at) %> - - <% if flag.is_resolved? %> - RESOLVED - <% end %> <% end %> diff --git a/app/views/post_flags/_search.html.erb b/app/views/post_flags/_search.html.erb index d16a5dff8..fe1ecedc7 100644 --- a/app/views/post_flags/_search.html.erb +++ b/app/views/post_flags/_search.html.erb @@ -5,7 +5,7 @@ <% if policy(PostFlag).can_search_flagger? %> <%= f.input :creator_name, label: "Creator", input_html: { value: params[:search][:creator_name], data: { autocomplete: "user" } } %> <% end %> - <%= f.input :is_resolved, label: "Resolved?", collection: [["Yes", true], ["No", false]], include_blank: true, selected: params[:search][:is_resolved] %> <%= f.input :category, label: "Category", collection: ["normal", "unapproved", "rejected", "deleted"], include_blank: true, selected: params[:search][:category] %> + <%= f.input :status, collection: PostFlag.statuses, include_blank: true, selected: params[:search][:status] %> <%= f.submit "Search" %> <% end %> diff --git a/app/views/post_flags/index.html.erb b/app/views/post_flags/index.html.erb index 2151799c7..876e9ad7e 100644 --- a/app/views/post_flags/index.html.erb +++ b/app/views/post_flags/index.html.erb @@ -19,8 +19,8 @@ <% t.column "Category", width: "1%" do |post_flag| %> <%= link_to post_flag.category.to_s, post_flags_path(search: params[:search].merge(category: post_flag.category)) %> <% end %> - <% t.column "Resolved?", width: "1%" do |post_flag| %> - <%= link_to post_flag.is_resolved?.to_s, post_flags_path(search: params[:search].merge(is_resolved: post_flag.is_resolved?)) %> + <% t.column "Status", width: "5%" do |post_flag| %> + <%= link_to post_flag.status, post_flags_path(search: { status: post_flag.status }) %> <% end %> <% t.column "Uploaded", width: "15%" do |post_flag| %> <%= compact_time post_flag.post.created_at %> diff --git a/test/factories/post_flag.rb b/test/factories/post_flag.rb index baf373052..634d9bb4d 100644 --- a/test/factories/post_flag.rb +++ b/test/factories/post_flag.rb @@ -3,6 +3,5 @@ FactoryBot.define do creator post { build(:post, is_flagged: true) } reason {"xxx"} - is_resolved {false} end end diff --git a/test/functional/post_events_controller_test.rb b/test/functional/post_events_controller_test.rb index 623b9d2c1..79c5c3f7d 100644 --- a/test/functional/post_events_controller_test.rb +++ b/test/functional/post_events_controller_test.rb @@ -8,10 +8,10 @@ class PostEventsControllerTest < ActionDispatch::IntegrationTest end as(@user) do - @post = create(:post) - @post.flag!("aaa") + @post = create(:post, is_flagged: true) + create(:post_flag, post: @post, status: :rejected) @post.update(is_deleted: true) - create(:post_appeal, post: @post) + create(:post_appeal, post: @post, status: :succeeded) @post.approve!(@mod) end end @@ -38,9 +38,5 @@ class PostEventsControllerTest < ActionDispatch::IntegrationTest should "render" do assert_not_nil(@appeal) end - - should "return is_resolved correctly" do - assert_equal(false, @appeal["is_resolved"]) - end end end