jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

bans: change expires_at field to duration.

Browse Source 
Changes:

* Change the `expires_at` field to `duration`.
* Make moderators choose from a fixed set of standard ban lengths,
  instead of allowing arbitrary ban lengths.
* List `duration` in seconds in the /bans.json API.
* Dump bans to BigQuery.

Note that some old bans have a negative duration. This is because their
expiration date was before their creation date, which is because in 2013
bans were migrated to Danbooru 2 and the original ban creation dates
were lost.
This commit is contained in:
evazion
2021-03-10 17:20:52 -06:00
parent 791b8c61f6
commit 81fe68d392
16 changed files with 90 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/bans_controller.rb
View File

@@ -1,5 +1,5 @@
class BansController < ApplicationController
respond_to :html, :xml, :json
respond_to :html, :xml, :json, :js
def new
@ban = authorize Ban.new(permitted_attributes(Ban))

2
app/logical/bigquery_export_service.rb
View File

@@ -21,7 +21,7 @@ class BigqueryExportService
Rails.application.eager_load!
models = ApplicationRecord.descendants.sort_by(&:name)
models -= [Ban, Favorite, IpAddress, TagRelationship, ArtistVersion, ArtistCommentaryVersion, NoteVersion, PoolVersion, PostVersion, WikiPageVersion]
models -= [Favorite, IpAddress, TagRelationship, ArtistVersion, ArtistCommentaryVersion, NoteVersion, PoolVersion, PostVersion, WikiPageVersion]
models
end

4
app/logical/concerns/searchable.rb
View File

@@ -212,7 +212,7 @@ module Searchable
search_text_attribute(name, params)
when :boolean
search_boolean_attribute(name, params)
when :integer, :float, :datetime
when :integer, :float, :datetime, :interval
search_numeric_attribute(name, params, type: type)
when :inet
search_inet_attribute(name, params)
@@ -221,7 +221,7 @@ module Searchable
when :array
search_array_attribute(name, subtype, params)
else
raise NotImplementedError, "unhandled attribute type: #{name}"
raise NotImplementedError, "unhandled attribute type: #{name} (#{type})"
end
end

6
app/logical/post_query_builder.rb
View File

@@ -740,6 +740,9 @@ class PostQueryBuilder
when :age
DurationParser.parse(object).ago
when :interval
DurationParser.parse(object)
when :ratio
object =~ /\A(\d+(?:\.\d+)?):(\d+(?:\.\d+)?)\Z/i
@@ -765,6 +768,9 @@ class PostQueryBuilder
end
(size * conversion_factor).to_i
else
raise NotImplementedError, "unrecognized type #{type} for #{object}"
end
end

23
app/models/ban.rb
View File

@@ -1,4 +1,6 @@
class Ban < ApplicationRecord
attribute :duration, :interval
after_create :create_feedback
after_create :update_user_on_create
after_create :create_ban_mod_action
@@ -7,20 +9,14 @@ class Ban < ApplicationRecord
belongs_to :user
belongs_to :banner, :class_name => "User"
validates_presence_of :reason, :duration
validates :reason, presence: true
validate :user, :validate_user_is_bannable, on: :create
scope :unexpired, -> { where("bans.expires_at > ?", Time.now) }
scope :expired, -> { where("bans.expires_at <= ?", Time.now) }
attr_reader :duration
def self.is_banned?(user)
exists?(["user_id = ? AND expires_at > ?", user.id, Time.now])
end
scope :unexpired, -> { where("bans.created_at + bans.duration > ?", Time.now) }
scope :expired, -> { where("bans.created_at + bans.duration <= ?", Time.now) }
def self.search(params)
q = search_attributes(params, :id, :created_at, :updated_at, :expires_at, :reason, :user, :banner)
q = search_attributes(params, :id, :created_at, :updated_at, :duration, :reason, :user, :banner)
q = q.text_attribute_matches(:reason, params[:reason_matches])
q = q.expired if params[:expired].to_s.truthy?
@@ -28,7 +24,7 @@ class Ban < ApplicationRecord
case params[:order]
when "expires_at_desc"
q = q.order("bans.expires_at desc")
q = q.order(Arel.sql("bans.created_at + bans.duration DESC"))
else
q = q.apply_default_order(params)
end
@@ -62,9 +58,8 @@ class Ban < ApplicationRecord
self.user = User.find_by_name(username)
end
def duration=(dur)
self.expires_at = dur.to_i.days.from_now
@duration = dur
def expires_at
created_at + duration
end
def humanized_duration

4
app/policies/ban_policy.rb
View File

@@ -9,11 +9,11 @@ class BanPolicy < ApplicationPolicy
alias_method :destroy?, :bannable?
def permitted_attributes_for_create
[:reason, :duration, :expires_at, :user_id, :user_name]
[:reason, :duration, :user_id, :user_name]
end
def permitted_attributes_for_update
[:reason, :duration, :expires_at]
[:reason, :duration]
end
def html_data_attributes

9
app/views/bans/_form.html.erb
View File

@@ -1,6 +1,9 @@
<%= edit_form_for(ban) do |f| %>
<%= f.input :user_name, :as => :string, :input_html => { data: { autocomplete: "user" } } %>
<%= f.input :duration, :hint => "in days" %>
<% if ban.new_record? %>
<%= f.input :user_name, as: :string, input_html: { "data-autocomplete": "user" } %>
<% end %>
<%= f.input :duration, collection: [["1 day", 1.day.iso8601], ["3 days", 3.days.iso8601], ["1 week", 1.week.iso8601], ["1 month", 1.month.iso8601], ["3 months", 3.months.iso8601], ["6 months", 6.months.iso8601], ["1 year", 1.year.iso8601], ["forever", 100.years.iso8601]] %>
<%= f.input :reason %>
<%= f.button :submit, :value => "Ban" %>
<%= f.button :submit %>
<% end %>

2
app/views/bans/destroy.js.erb
View File

@@ -1 +1 @@
$("#ban-<%= @ban.id %>").remove();
location.reload();

6
app/views/bans/edit.html.erb
View File

@@ -2,11 +2,7 @@
<div id="a-edit">
<h1>Edit Ban</h1>
<%= edit_form_for(@ban) do |f| %>
<%= f.input :duration, :hint => "in days" %>
<%= f.input :reason %>
<%= f.button :submit, :value => "Ban" %>
<% end %>
<%= render "form", ban: @ban %>
</div>
</div>

6
app/views/bans/index.html.erb
View File

@@ -9,14 +9,14 @@
<%= link_to_user(ban.user) %>
<%= link_to "»", bans_path(search: search_params.merge(user_name: ban.user.name)) %>
<% end %>
<% t.column "Duration" do |ban| %>
<%= humanized_duration(ban.created_at, ban.expires_at) %>
<% end %>
<% t.column "Reason", td: {class: "col-expand"} do |ban| %>
<div class="prose">
<%= format_text ban.reason %>
</div>
<% end %>
<% t.column "Duration" do |ban| %>
<%= humanized_duration(ban.created_at, ban.expires_at) %>
<% end %>
<% t.column "Banner" do |ban| %>
<%= link_to_user ban.banner %>
<%= link_to "»", bans_path(search: { banner_name: ban.banner.name }) %>

6
app/views/bans/show.html.erb
View File

@@ -5,10 +5,10 @@
<div id="a-show">
<h1>Show Ban</h1>
<ul style="margin-bottom: 1em;">
<li><strong>User</strong>: <%= link_to_user(@ban.user) %></li>
<li><strong>Expires</strong>: <%= compact_time @ban.expires_at %></li>
<li><strong>User</strong> <%= link_to_user(@ban.user) %></li>
<li><strong>Duration</strong> <%= humanized_duration(@ban.created_at, @ban.expires_at) %></li>
<li>
<strong>Reason</strong>:
<strong>Reason</strong>
<div class="prose">
<%= format_text @ban.reason %>
</div>

11
db/migrate/20210310221248_change_expires_at_to_duration_on_bans.rb Normal file
View File

@@ -0,0 +1,11 @@
class ChangeExpiresAtToDurationOnBans < ActiveRecord::Migration[6.1]
def up
change_column :bans, :expires_at, :interval, using: "expires_at - created_at"
rename_column :bans, :expires_at, :duration
end
def down
change_column :bans, :duration, :timestamp, using: "created_at + duration"
rename_column :bans, :duration, :expires_at
end
end

9
db/structure.sql
View File

@@ -690,7 +690,7 @@ CREATE TABLE public.bans (
user_id integer,
reason text NOT NULL,
banner_id integer NOT NULL,
expires_at timestamp without time zone NOT NULL,
expires_at interval NOT NULL,
created_at timestamp without time zone NOT NULL,
updated_at timestamp without time zone NOT NULL
);
@@ -5066,10 +5066,10 @@ CREATE INDEX index_bans_on_banner_id ON public.bans USING btree (banner_id);
--
-- Name: index_bans_on_expires_at; Type: INDEX; Schema: public; Owner: -
-- Name: index_bans_on_duration; Type: INDEX; Schema: public; Owner: -
--
CREATE INDEX index_bans_on_expires_at ON public.bans USING btree (expires_at);
CREATE INDEX index_bans_on_duration ON public.bans USING btree (duration);
--
@@ -8004,6 +8004,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20210127012303'),
('20210214095121'),
('20210214101614'),
('20210303195217');
('20210303195217'),
('20210310221248');

2
test/factories/ban.rb
View File

@@ -3,6 +3,6 @@ FactoryBot.define do
banner :factory => :admin_user
user
reason {FFaker::Lorem.words.join(" ")}
duration {60}
duration { 1.week }
end
end

64
test/functional/bans_controller_test.rb
View File

@@ -2,16 +2,10 @@ require 'test_helper'
class BansControllerTest < ActionDispatch::IntegrationTest
context "A bans controller" do
setup do
@mod = create(:moderator_user, name: "danbo")
@admin = create(:admin_user)
@user = create(:member_user, id: 999, name: "cirno")
as(@mod) { @ban = create(:ban, reason: "blah", user: @user, banner: @mod) }
end
context "new action" do
should "render" do
@mod = create(:mod_user)
get_auth new_ban_path, @mod
assert_response :success
end
@@ -19,6 +13,9 @@ class BansControllerTest < ActionDispatch::IntegrationTest
context "edit action" do
should "render" do
@mod = create(:mod_user)
@ban = create(:ban)
get_auth edit_ban_path(@ban.id), @mod
assert_response :success
end
@@ -26,14 +23,18 @@ class BansControllerTest < ActionDispatch::IntegrationTest
context "show action" do
should "render" do
get_auth ban_path(@ban.id), @mod
@ban = create(:ban)
get ban_path(@ban)
assert_response :success
end
end
context "index action" do
setup do
as(@admin) { @admin_ban = create(:ban, user: build(:builder_user), banner: @admin, expires_at: 1.day.ago ) }
@mod = create(:mod_user, name: "mod")
@ban1 = create(:ban, created_at: 1.week.ago, duration: 1.day)
@ban2 = create(:ban, user: build(:builder_user), reason: "blah", banner: @mod, duration: 100.years)
end
should "render" do
@@ -41,24 +42,21 @@ class BansControllerTest < ActionDispatch::IntegrationTest
assert_response :success
end
should respond_to_search({}).with { [@admin_ban, @ban] }
should respond_to_search(reason_matches: "blah").with { @ban }
should respond_to_search(expired: "true").with { @admin_ban }
should respond_to_search({}).with { [@ban2, @ban1] }
should respond_to_search(reason_matches: "blah").with { @ban2 }
should respond_to_search(expired: "false").with { @ban2 }
should respond_to_search(duration: "<1w").with { @ban1 }
context "using includes" do
should respond_to_search(banner_name: "danbo").with { @ban }
should respond_to_search(banner: {level: User::Levels::ADMIN}).with { @admin_ban }
should respond_to_search(user_id: 999).with { @ban }
should respond_to_search(user: {name: "cirno"}).with { @ban }
should respond_to_search(user: {level: User::Levels::BUILDER}).with { @admin_ban }
end
should respond_to_search(banner_name: "mod").with { @ban2 }
should respond_to_search(banner: { level: User::Levels::MODERATOR }).with { @ban2 }
end
context "create action" do
should "allow mods to ban members" do
assert_difference("Ban.count", 1) do
@user = create(:user)
post_auth bans_path, @mod, params: { ban: { duration: 60, reason: "xxx", user_id: @user.id }}
@mod = create(:mod_user)
post_auth bans_path, @mod, params: { ban: { duration: 1.day.iso8601, reason: "xxx", user_id: @user.id }}
assert_redirected_to bans_path
assert_equal(true, @user.reload.is_banned?)
@@ -67,7 +65,9 @@ class BansControllerTest < ActionDispatch::IntegrationTest
should "not allow mods to ban admins" do
assert_difference("Ban.count", 0) do
post_auth bans_path, @mod, params: { ban: { duration: 60, reason: "xxx", user_id: @admin.id }}
@admin = create(:admin_user)
@mod = create(:mod_user)
post_auth bans_path, @mod, params: { ban: { duration: 1.day.iso8601, reason: "xxx", user_id: @admin.id }}
assert_response 403
assert_equal(false, @admin.reload.is_banned?)
@@ -76,8 +76,9 @@ class BansControllerTest < ActionDispatch::IntegrationTest
should "not allow mods to ban other mods" do
assert_difference("Ban.count", 0) do
@mod = create(:mod_user)
@mod2 = create(:mod_user)
post_auth bans_path, @mod, params: { ban: { duration: 60, reason: "xxx", user_id: @mod2.id }}
post_auth bans_path, @mod, params: { ban: { duration: 1.day.iso8601, reason: "xxx", user_id: @mod2.id }}
assert_response 403
assert_equal(false, @mod2.reload.is_banned?)
@@ -87,7 +88,8 @@ class BansControllerTest < ActionDispatch::IntegrationTest
should "not allow regular users to ban anyone" do
assert_difference("Ban.count", 0) do
@user = create(:user)
post_auth bans_path, @user, params: { ban: { duration: 60, reason: "xxx", user_id: @mod.id }}
@mod = create(:mod_user)
post_auth bans_path, @user, params: { ban: { duration: 1.day.iso8601, reason: "xxx", user_id: @mod.id }}
assert_response 403
assert_equal(false, @mod.reload.is_banned?)
@@ -95,13 +97,17 @@ class BansControllerTest < ActionDispatch::IntegrationTest
end
should "not allow users to be double banned" do
@ban = create(:ban, duration: 1.week)
@mod = create(:mod_user)
assert_difference("Ban.count", 0) do
post_auth bans_path, @mod, params: { ban: { duration: 60, reason: "xxx", user_id: @ban.user.id }}
post_auth bans_path, @mod, params: { ban: { duration: 1.day.iso8601, reason: "xxx", user_id: @ban.user.id }}
assert_response :success
end
end
should "not raise an exception on a blank username" do
@mod = create(:mod_user)
post_auth bans_path, @mod, params: {}
assert_response :success
end
@@ -109,7 +115,10 @@ class BansControllerTest < ActionDispatch::IntegrationTest
context "update action" do
should "update a ban" do
put_auth ban_path(@ban.id), @mod, params: {ban: {reason: "xxx", duration: 60}}
@ban = create(:ban)
@mod = create(:mod_user)
put_auth ban_path(@ban.id), @mod, params: {ban: {reason: "xxx", duration: 1.day.iso8601}}
assert_equal("xxx", @ban.reload.reason)
assert_redirected_to(ban_path(@ban))
end
@@ -117,6 +126,9 @@ class BansControllerTest < ActionDispatch::IntegrationTest
context "destroy action" do
should "destroy a ban" do
@ban = create(:ban)
@mod = create(:mod_user)
assert_difference("Ban.count", -1) do
delete_auth ban_path(@ban.id), @mod
assert_redirected_to bans_path

20
test/unit/ban_test.rb
View File

@@ -84,26 +84,6 @@ class BanTest < ActiveSupport::TestCase
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
context "when only expired bans exist" do
setup do
@ban = FactoryBot.create(:ban, :user => @user, :banner => @admin, :duration => -1)
end
should "not return expired bans" do
assert(!Ban.is_banned?(@user))
end
end
context "when active bans still exist" do
setup do
@ban = FactoryBot.create(:ban, :user => @user, :banner => @admin, :duration => 1)
end
should "return active bans" do
assert(Ban.is_banned?(@user))
end
end
end
end
end