From e4241d2001fc1c068747c3e3e7b2829dd6132748 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sat, 14 Aug 2021 16:07:12 +0200 Subject: [PATCH 01/14] Create user ban model Co-authored-by: Georg Gadinger --- app/models/user.rb | 5 ++++ app/models/user_ban.rb | 4 +++ db/migrate/20210814134115_create_user_bans.rb | 28 +++++++++++++++++++ db/schema.rb | 9 ++++++ spec/models/user_ban_spec.rb | 5 ++++ 5 files changed, 51 insertions(+) create mode 100644 app/models/user_ban.rb create mode 100644 db/migrate/20210814134115_create_user_bans.rb create mode 100644 spec/models/user_ban_spec.rb diff --git a/app/models/user.rb b/app/models/user.rb index f3407967..0c23c4cd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -49,6 +49,11 @@ class User < ApplicationRecord has_one :profile, dependent: :destroy has_one :theme, dependent: :destroy + has_many :user_bans, dependent: :destroy + has_many :banned_users, class_name: 'UserBan', + foreign_key: 'banned_by_id', + dependent: :nullify + SCREEN_NAME_REGEX = /\A[a-zA-Z0-9_]{1,16}\z/ WEBSITE_REGEX = /https?:\/\/([A-Za-z.\-]+)\/?(?:.*)/i diff --git a/app/models/user_ban.rb b/app/models/user_ban.rb new file mode 100644 index 00000000..ee143412 --- /dev/null +++ b/app/models/user_ban.rb @@ -0,0 +1,4 @@ +class UserBan < ApplicationRecord + belongs_to :user + belongs_to :banned_by, class_name: 'User' +end diff --git a/db/migrate/20210814134115_create_user_bans.rb b/db/migrate/20210814134115_create_user_bans.rb new file mode 100644 index 00000000..a5146c90 --- /dev/null +++ b/db/migrate/20210814134115_create_user_bans.rb @@ -0,0 +1,28 @@ +class CreateUserBans < ActiveRecord::Migration[5.2] + def up + create_table :user_bans do |t| + t.bigint :user_id + t.string :reason + t.datetime :expires_at + t.bigint :banned_by_id, nullable: true + + t.timestamps + end + + # foxy's functional fqueries + execute "INSERT INTO user_bans + (user_id, reason, expires_at, created_at, updated_at) + SELECT users.id, users.ban_reason, users.banned_until, users.updated_at, NOW() FROM users + WHERE banned_until IS NOT NULL AND NOT permanently_banned;" + + + execute "INSERT INTO user_bans + (user_id, reason, expires_at, created_at, updated_at) + SELECT users.id, users.ban_reason, NULL, users.updated_at, NOW() FROM users + WHERE permanently_banned;" + end + + def down + drop_table :user_bans + end +end diff --git a/db/schema.rb b/db/schema.rb index a1294985..acb6f16a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -248,6 +248,15 @@ ActiveRecord::Schema.define(version: 2021_12_28_135426) do t.index ["user_id", "code"], name: "index_totp_recovery_codes_on_user_id_and_code" end + create_table "user_bans", force: :cascade do |t| + t.bigint "user_id" + t.string "reason" + t.datetime "expires_at" + t.bigint "banned_by_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "users", id: :bigint, default: -> { "gen_timestamp_id('users'::text)" }, force: :cascade do |t| t.string "email", default: "", null: false t.string "encrypted_password", default: "", null: false diff --git a/spec/models/user_ban_spec.rb b/spec/models/user_ban_spec.rb new file mode 100644 index 00000000..f7902eea --- /dev/null +++ b/spec/models/user_ban_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe UserBan, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end From be0cf6936826c39c8e3a194f09cb3a9280af4fad Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sat, 14 Aug 2021 18:04:58 +0200 Subject: [PATCH 02/14] Refactor existing ban-related methods --- app/controllers/ajax/moderation_controller.rb | 20 ++++++++++-------- app/models/user.rb | 21 +++++++++++-------- app/models/user_ban.rb | 2 ++ lib/errors.rb | 7 +++++++ 4 files changed, 32 insertions(+), 18 deletions(-) create mode 100644 lib/errors.rb diff --git a/app/controllers/ajax/moderation_controller.rb b/app/controllers/ajax/moderation_controller.rb index 8bd822e6..f2149885 100644 --- a/app/controllers/ajax/moderation_controller.rb +++ b/app/controllers/ajax/moderation_controller.rb @@ -109,13 +109,15 @@ class Ajax::ModerationController < AjaxController params.require :user params.require :ban params.require :permaban + params.require :duration + params.require :duration_unit - reason = params[:reason] + duration = params[:duration].to_s + duration_unit = params[:duration_unit].to_s + reason = params[:reason].to_s target = User.find_by_screen_name!(params[:user]) - unban = params[:ban] == "0" - perma = params[:permaban] == "1" - - buntil = DateTime.strptime params[:until], "%m/%d/%Y %I:%M %p" unless unban || perma + unban = params[:ban] == '0' + perma = params[:permaban] == '1' if !unban && target.has_role?(:administrator) @response[:status] = :nopriv @@ -128,11 +130,11 @@ class Ajax::ModerationController < AjaxController @response[:message] = I18n.t('messages.moderation.ban.unban') @response[:success] = true elsif perma - target.ban nil, reason + target.ban nil, nil, reason, current_user @response[:message] = I18n.t('messages.moderation.ban.perma') else - target.ban buntil, reason - @response[:message] = I18n.t('messages.moderation.ban.temp', date: buntil.to_s) + target.ban duration, duration_unit, reason, current_user + @response[:message] = "User banned for #{duration} #{duration_unit}" end target.save! @@ -162,7 +164,7 @@ class Ajax::ModerationController < AjaxController @response[:checked] = status type = params[:type].downcase - target_role = {"admin" => "administrator"}.fetch(type, type).to_sym + target_role = {'admin' => 'administrator'}.fetch(type, type).to_sym if status target_user.add_role target_role diff --git a/app/models/user.rb b/app/models/user.rb index 0c23c4cd..bbcc8f6b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -223,21 +223,24 @@ class User < ApplicationRecord end # endregion - # forwards fill def banned? - self.permanently_banned? or ((not self.banned_until.nil?) and self.banned_until >= DateTime.current) + self.user_bans.current.any? end def unban - self.update(permanently_banned: false, ban_reason: nil, banned_until: nil) + self.user_bans.current.update(expires_at: DateTime.now) end - def ban(buntil=nil, reason=nil) - if buntil == nil - self.update(permanently_banned: true, ban_reason: reason) - else - self.update(permanently_banned: false, banned_until: buntil, ban_reason: reason) - end + # Bans a user. + # @param duration [Fixnum, nil] Ban duration + # @param duration_unit [String, nil] Unit for the duration parameter. Accepted units: hours, days, weeks, months + # @param reason [String] Reason for the ban. This is displayed to the user. + # @param banned_by [User] User who instated the ban + def ban(duration, duration_unit = 'hours', reason = nil, banned_by = nil) + raise Errors::InvalidBanDuration unless %w[hours days weeks months].include? duration_unit + + expiry = duration && DateTime.now + duration.public_send(duration_unit) + self.user_bans.create(expires_at: expiry, reason: reason, banned_by: banned_by) end def can_export? diff --git a/app/models/user_ban.rb b/app/models/user_ban.rb index ee143412..e7b3c1fe 100644 --- a/app/models/user_ban.rb +++ b/app/models/user_ban.rb @@ -1,4 +1,6 @@ class UserBan < ApplicationRecord belongs_to :user belongs_to :banned_by, class_name: 'User' + + scope :current, -> { where('expires_at IS NULL or expires_at < NOW()') } end diff --git a/lib/errors.rb b/lib/errors.rb new file mode 100644 index 00000000..c13228a8 --- /dev/null +++ b/lib/errors.rb @@ -0,0 +1,7 @@ +module Errors + class Base < StandardError + end + + class InvalidBanDuration < Base + end +end \ No newline at end of file From 9a35584284fa7b59896137c21a05d07a51af92c3 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 19 Aug 2021 17:50:24 +0200 Subject: [PATCH 03/14] Refactor ban-related functionality into use cases --- Gemfile | 3 ++ Gemfile.lock | 21 ++++++++ app/controllers/ajax/moderation_controller.rb | 41 ++++++++++----- app/javascript/legacy/moderation/ban.coffee | 10 ++-- app/models/user.rb | 5 +- app/models/user_ban.rb | 2 +- app/views/modal/_ban.haml | 7 ++- lib/errors.rb | 23 ++++++++- lib/types.rb | 7 +++ lib/use_case/base.rb | 19 +++++++ lib/use_case/user/ban.rb | 50 +++++++++++++++++++ lib/use_case/user/unban.rb | 17 +++++++ 12 files changed, 181 insertions(+), 24 deletions(-) create mode 100644 lib/types.rb create mode 100644 lib/use_case/base.rb create mode 100644 lib/use_case/user/ban.rb create mode 100644 lib/use_case/user/unban.rb diff --git a/Gemfile b/Gemfile index 998bcbc9..d51035e5 100644 --- a/Gemfile +++ b/Gemfile @@ -36,6 +36,9 @@ gem "hcaptcha", "~> 6.0", git: "https://github.com/Retrospring/hcaptcha.git", re gem "rolify", "~> 5.2" +gem "dry-initializer", "~> 3.0" +gem "dry-types", "~> 1.4" + gem 'ruby-progressbar' gem 'rails_admin' diff --git a/Gemfile.lock b/Gemfile.lock index 9e3dc9a0..3a48f89d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -154,6 +154,25 @@ GEM docile (1.4.0) domain_name (0.5.20190701) unf (>= 0.0.5, < 1.0.0) + dry-configurable (0.12.1) + concurrent-ruby (~> 1.0) + dry-core (~> 0.5, >= 0.5.0) + dry-container (0.8.0) + concurrent-ruby (~> 1.0) + dry-configurable (~> 0.1, >= 0.1.3) + dry-core (0.7.1) + concurrent-ruby (~> 1.0) + dry-inflector (0.2.1) + dry-initializer (3.0.4) + dry-logic (1.2.0) + concurrent-ruby (~> 1.0) + dry-core (~> 0.5, >= 0.5) + dry-types (1.5.1) + concurrent-ruby (~> 1.0) + dry-container (~> 0.3) + dry-core (~> 0.5, >= 0.5) + dry-inflector (~> 0.1, >= 0.1.2) + dry-logic (~> 1.0, >= 1.0.2) equalizer (0.0.11) erubi (1.10.0) excon (0.89.0) @@ -606,6 +625,8 @@ DEPENDENCIES devise (~> 4.0) devise-async devise-i18n + dry-initializer (~> 3.0) + dry-types (~> 1.4) factory_bot_rails fake_email_validator faker diff --git a/app/controllers/ajax/moderation_controller.rb b/app/controllers/ajax/moderation_controller.rb index f2149885..b6c9d9e9 100644 --- a/app/controllers/ajax/moderation_controller.rb +++ b/app/controllers/ajax/moderation_controller.rb @@ -1,3 +1,7 @@ +require 'use_case/user/ban' +require 'use_case/user/unban' +require 'errors' + class Ajax::ModerationController < AjaxController def vote params.require :id @@ -108,38 +112,49 @@ class Ajax::ModerationController < AjaxController params.require :user params.require :ban - params.require :permaban - params.require :duration - params.require :duration_unit - duration = params[:duration].to_s + duration = params[:duration].to_i duration_unit = params[:duration_unit].to_s reason = params[:reason].to_s - target = User.find_by_screen_name!(params[:user]) + target_user = User.find_by_screen_name!(params[:user]) unban = params[:ban] == '0' - perma = params[:permaban] == '1' + perma = params[:duration].nil? - if !unban && target.has_role?(:administrator) + if !unban && target_user.has_role?(:administrator) @response[:status] = :nopriv @response[:message] = I18n.t('messages.moderation.ban.nopriv') return end if unban - target.unban + UseCase::User::Unban.call(target_user.id) @response[:message] = I18n.t('messages.moderation.ban.unban') @response[:success] = true + @response[:status] = :okay + return elsif perma - target.ban nil, nil, reason, current_user @response[:message] = I18n.t('messages.moderation.ban.perma') + expiry = nil else - target.ban duration, duration_unit, reason, current_user - @response[:message] = "User banned for #{duration} #{duration_unit}" + params.require :duration + params.require :duration_unit + + raise Errors::InvalidBanDuration unless %w[hours days weeks months].include? duration_unit + + expiry = DateTime.now + duration.public_send(duration_unit) + @response[:message] = I18n.t('messages.moderation.ban.temp', date: expiry.to_s) end - target.save! + + UseCase::User::Ban.call( + target_user_id: target_user.id, + expiry: expiry, + reason: reason, + source_user_id: current_user.id) + + target_user.save! @response[:status] = :okay - @response[:success] = target.banned? == !unban + @response[:success] = true end def privilege diff --git a/app/javascript/legacy/moderation/ban.coffee b/app/javascript/legacy/moderation/ban.coffee index 8530ff23..e37296de 100644 --- a/app/javascript/legacy/moderation/ban.coffee +++ b/app/javascript/legacy/moderation/ban.coffee @@ -30,12 +30,15 @@ load = -> data = { ban: checktostr banCheckbox - permaban: checktostr permabanCheckbox - until: modalForm.elements["until"].value.trim() - reason: modalForm.elements["reason"].value.trim() user: modalForm.elements["user"].value } + if banCheckbox.checked + data.reason = modalForm.elements["reason"].value.trim() + unless permabanCheckbox.checked + data.duration = modalForm.elements["duration"].value.trim() + data.duration_unit = modalForm.elements["duration_unit"].value.trim() + $.ajax url: '/ajax/mod/ban' type: 'POST' @@ -43,6 +46,7 @@ load = -> success: (data, status, jqxhr) -> showNotification data.message, data.success error: (jqxhr, status, error) -> + console.error 'request failed', data console.log jqxhr, status, error showNotification translate('frontend.error.message'), false complete: (jqxhr, status) -> diff --git a/app/models/user.rb b/app/models/user.rb index bbcc8f6b..2b98d6a8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -228,7 +228,7 @@ class User < ApplicationRecord end def unban - self.user_bans.current.update(expires_at: DateTime.now) + self.user_bans.current.update!(expires_at: DateTime.now) end # Bans a user. @@ -237,9 +237,6 @@ class User < ApplicationRecord # @param reason [String] Reason for the ban. This is displayed to the user. # @param banned_by [User] User who instated the ban def ban(duration, duration_unit = 'hours', reason = nil, banned_by = nil) - raise Errors::InvalidBanDuration unless %w[hours days weeks months].include? duration_unit - - expiry = duration && DateTime.now + duration.public_send(duration_unit) self.user_bans.create(expires_at: expiry, reason: reason, banned_by: banned_by) end diff --git a/app/models/user_ban.rb b/app/models/user_ban.rb index e7b3c1fe..bce39f5c 100644 --- a/app/models/user_ban.rb +++ b/app/models/user_ban.rb @@ -2,5 +2,5 @@ class UserBan < ApplicationRecord belongs_to :user belongs_to :banned_by, class_name: 'User' - scope :current, -> { where('expires_at IS NULL or expires_at < NOW()') } + scope :current, -> { where('expires_at IS NULL or expires_at > NOW()') } end diff --git a/app/views/modal/_ban.haml b/app/views/modal/_ban.haml index 9854cae1..e4e6ec59 100644 --- a/app/views/modal/_ban.haml +++ b/app/views/modal/_ban.haml @@ -14,7 +14,12 @@ #ban-controls{ style: user.banned? ? '' : 'display: none' } = f.check_box :permaban, label: t('views.modal.bancontrol.permanent'), checked: user.permanently_banned? #ban-controls-time{ style: user.permanently_banned? ? 'display: none' : '' } - = f.text_field :until, label: '', required: true, value: (user.banned_until || DateTime.current).strftime('%m/%d/%Y %I:%M %p') + = f.text_field :duration, label: '', required: true + .form-check.form-check-inline + = f.radio_button :duration_unit, 'hours', label: 'Hours', checked: true + = f.radio_button :duration_unit, 'days', label: 'Days' + = f.radio_button :duration_unit, 'weeks', label: 'Weeks' + = f.radio_button :duration_unit, 'months', label: 'Months' = f.text_field :reason, placeholder: t('views.modal.bancontrol.reason'), value: user.ban_reason .modal-footer %button.btn.btn-default{ name: 'stop-time', type: :button, data: { dismiss: :modal } }= t 'views.actions.close' diff --git a/lib/errors.rb b/lib/errors.rb index c13228a8..1ddb9198 100644 --- a/lib/errors.rb +++ b/lib/errors.rb @@ -1,7 +1,26 @@ module Errors class Base < StandardError + def status + 500 + end + + def code + @code ||= self.class.name.sub('Errors::', '').underscore + end end - class InvalidBanDuration < Base + class BadRequest < Base + def status + 400 + end end -end \ No newline at end of file + + class InvalidBanDuration < BadRequest + end + + class Forbidden < Base + def status + 403 + end + end +end diff --git a/lib/types.rb b/lib/types.rb new file mode 100644 index 00000000..51e96551 --- /dev/null +++ b/lib/types.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'dry-types' + +module Types + include Dry.Types() +end diff --git a/lib/use_case/base.rb b/lib/use_case/base.rb new file mode 100644 index 00000000..5468e0ff --- /dev/null +++ b/lib/use_case/base.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'dry-initializer' +require 'types' +require 'errors' + +module UseCase + class Base + extend Dry::Initializer + + def self.call(*args, **kwargs) + new(*args, **kwargs).call + end + + def call + raise NotImplementedError + end + end +end \ No newline at end of file diff --git a/lib/use_case/user/ban.rb b/lib/use_case/user/ban.rb new file mode 100644 index 00000000..b5295d69 --- /dev/null +++ b/lib/use_case/user/ban.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'use_case/base' + +module UseCase + module User + class Ban < UseCase::Base + REASON_SPAM = 'Spam' + REASON_HARASSMENT = 'Harassment' + REASON_BAN_EVASION = 'Ban evasion' + + option :target_user_id, type: Types::Coercible::Integer + option :expiry, types: Types::Nominal::DateTime.optional + option :source_user_id, type: Types::Coercible::Integer.optional + option :reason, type: Types::Coercible::String.optional + + def call + ban = ::UserBan.create!( + user: target_user, + expires_at: expiry, + banned_by: source_user, + reason: reason + ) + + if reason == REASON_SPAM + target_user.update!( + display_name: nil, + bio: '', + location: '', + website: '', + profile_picture: nil, + profile_header: nil + ) + end + + { + ban: ban + } + end + + def target_user + @target_user ||= ::User.find(target_user_id) + end + + def source_user + @source_user ||= ::User.find(source_user_id) + end + end + end +end diff --git a/lib/use_case/user/unban.rb b/lib/use_case/user/unban.rb new file mode 100644 index 00000000..d399a940 --- /dev/null +++ b/lib/use_case/user/unban.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'use_case/base' + +module UseCase + module User + class Unban < UseCase::Base + param :target_user_id, type: Types::Coercible::Integer + + def call + UserBan.current.where(user_id: target_user_id).update_all( + expires_at: DateTime.now + ) + end + end + end +end \ No newline at end of file From ea62d91014d0b1f5644e708cada4a2f26de90dd3 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Mon, 23 Aug 2021 09:50:35 +0200 Subject: [PATCH 04/14] Make ban UI more intuitive when a user is already banned --- app/models/user.rb | 8 +++--- app/views/modal/_ban.haml | 51 +++++++++++++++++++++++++++------------ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 2b98d6a8..40b1d329 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -49,7 +49,7 @@ class User < ApplicationRecord has_one :profile, dependent: :destroy has_one :theme, dependent: :destroy - has_many :user_bans, dependent: :destroy + has_many :bans, class_name: 'UserBan', dependent: :destroy has_many :banned_users, class_name: 'UserBan', foreign_key: 'banned_by_id', dependent: :nullify @@ -224,11 +224,11 @@ class User < ApplicationRecord # endregion def banned? - self.user_bans.current.any? + self.bans.current.any? end def unban - self.user_bans.current.update!(expires_at: DateTime.now) + self.bans.current.update!(expires_at: DateTime.now) end # Bans a user. @@ -237,7 +237,7 @@ class User < ApplicationRecord # @param reason [String] Reason for the ban. This is displayed to the user. # @param banned_by [User] User who instated the ban def ban(duration, duration_unit = 'hours', reason = nil, banned_by = nil) - self.user_bans.create(expires_at: expiry, reason: reason, banned_by: banned_by) + self.bans.create(expires_at: expiry, reason: reason, banned_by: banned_by) end def can_export? diff --git a/app/views/modal/_ban.haml b/app/views/modal/_ban.haml index e4e6ec59..d0435f64 100644 --- a/app/views/modal/_ban.haml +++ b/app/views/modal/_ban.haml @@ -1,3 +1,4 @@ +- current_ban = user.bans.current.first .modal.fade#modal-ban{ aria: { hidden: true, labelledby: 'modal-ban-label' }, role: :dialog, tabindex: -1 } .modal-dialog .modal-content @@ -9,18 +10,38 @@ %span.sr-only Close = bootstrap_form_tag(url: '/mod/ban', html: { method: :post, novalidate: :novalidate }) do |f| = f.hidden_field :user, value: user.screen_name - .modal-body#ban-control-super - = f.check_box :ban, label: t('views.modal.bancontrol.ban'), checked: user.banned? - #ban-controls{ style: user.banned? ? '' : 'display: none' } - = f.check_box :permaban, label: t('views.modal.bancontrol.permanent'), checked: user.permanently_banned? - #ban-controls-time{ style: user.permanently_banned? ? 'display: none' : '' } - = f.text_field :duration, label: '', required: true - .form-check.form-check-inline - = f.radio_button :duration_unit, 'hours', label: 'Hours', checked: true - = f.radio_button :duration_unit, 'days', label: 'Days' - = f.radio_button :duration_unit, 'weeks', label: 'Weeks' - = f.radio_button :duration_unit, 'months', label: 'Months' - = f.text_field :reason, placeholder: t('views.modal.bancontrol.reason'), value: user.ban_reason - .modal-footer - %button.btn.btn-default{ name: 'stop-time', type: :button, data: { dismiss: :modal } }= t 'views.actions.close' - = f.submit t('views.modal.bancontrol.hammertime'), class: 'btn btn-primary', name: 'hammer-time' + - if current_ban.nil? + .modal-body#ban-control-super + = f.check_box :ban, label: t('views.modal.bancontrol.ban'), checked: user.banned? + #ban-controls{ style: user.banned? ? '' : 'display: none' } + = f.check_box :permaban, label: t('views.modal.bancontrol.permanent'), checked: user.permanently_banned? + #ban-controls-time{ style: user.permanently_banned? ? 'display: none' : '' } + = f.text_field :duration, label: '', required: true + .form-check.form-check-inline + = f.radio_button :duration_unit, 'hours', label: 'Hours', checked: true + = f.radio_button :duration_unit, 'days', label: 'Days' + = f.radio_button :duration_unit, 'weeks', label: 'Weeks' + = f.radio_button :duration_unit, 'months', label: 'Months' + = f.text_field :reason, placeholder: t('views.modal.bancontrol.reason'), value: user.ban_reason + .modal-footer + %button.btn.btn-default{ name: 'stop-time', type: :button, data: { dismiss: :modal } }= t 'views.actions.close' + = f.submit t('views.modal.bancontrol.hammertime'), class: 'btn btn-primary', name: 'hammer-time' + - else + = f.hidden_field :ban, value: '0' + .modal-body#ban-control-super + - if current_ban.expires_at.nil? + This user is currently permanently banned for + %strong= current_ban.reason + - else + This user is currently banned until + %strong= current_ban.expires_at + for + %strong= current_ban.reason + - if current_ban.banned_by.present? + %br + This ban was instated by + %strong= current_ban.banned_by.safe_name + on + %strong= current_ban.created_at + .modal-footer + = f.submit 'Unban', class: 'btn btn-primary', name: 'hammer-time' From 6500d7ac715143fc9881e00b4d3a4abbe1cc9b34 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 30 Dec 2021 00:20:09 +0100 Subject: [PATCH 05/14] Update usages of bans --- app/controllers/application_controller.rb | 7 ++-- app/models/user.rb | 12 +++++- app/views/modal/_ban.haml | 10 ++--- lib/use_case/user/ban.rb | 16 +++++--- .../ajax/moderation_controller_spec.rb | 40 ++++++++++--------- 5 files changed, 52 insertions(+), 33 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 94d2ef02..78d9ee01 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -40,10 +40,11 @@ class ApplicationController < ActionController::Base name = current_user.screen_name # obligatory '2001: A Space Odyssey' reference flash[:notice] = t('flash.ban.error', name: name) - unless current_user.ban_reason.nil? - flash[:notice] += "\n#{t('flash.ban.reason', reason: current_user.ban_reason)}" + current_ban = current_user.bans.current.first + unless current_ban&.reason.nil? + flash[:notice] += "\n#{t('flash.ban.reason', reason: current_user.bans.current.first.reason)}" end - if not current_user.permanently_banned? + unless current_ban&.permanently_banned? # TODO format banned_until flash[:notice] += "\n#{t('flash.ban.until', time: current_user.banned_until)}" end diff --git a/app/models/user.rb b/app/models/user.rb index 40b1d329..41725ec6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -237,7 +237,17 @@ class User < ApplicationRecord # @param reason [String] Reason for the ban. This is displayed to the user. # @param banned_by [User] User who instated the ban def ban(duration, duration_unit = 'hours', reason = nil, banned_by = nil) - self.bans.create(expires_at: expiry, reason: reason, banned_by: banned_by) + if duration + expiry = duration.public_send(duration_unit) + else + expiry = nil + end + UseCase::User::Ban.call( + target_user_id: id, + expiry: expiry, + reason: reason, + source_user_id: banned_by&.id + ) end def can_export? diff --git a/app/views/modal/_ban.haml b/app/views/modal/_ban.haml index d0435f64..64fa4bb6 100644 --- a/app/views/modal/_ban.haml +++ b/app/views/modal/_ban.haml @@ -1,7 +1,7 @@ - current_ban = user.bans.current.first .modal.fade#modal-ban{ aria: { hidden: true, labelledby: 'modal-ban-label' }, role: :dialog, tabindex: -1 } .modal-dialog - .modal-content + .modal-content#ban-control-super .modal-header %h5.modal-title#modal-ban-label = t 'views.modal.bancontrol.title' @@ -11,7 +11,7 @@ = bootstrap_form_tag(url: '/mod/ban', html: { method: :post, novalidate: :novalidate }) do |f| = f.hidden_field :user, value: user.screen_name - if current_ban.nil? - .modal-body#ban-control-super + .modal-body = f.check_box :ban, label: t('views.modal.bancontrol.ban'), checked: user.banned? #ban-controls{ style: user.banned? ? '' : 'display: none' } = f.check_box :permaban, label: t('views.modal.bancontrol.permanent'), checked: user.permanently_banned? @@ -22,13 +22,13 @@ = f.radio_button :duration_unit, 'days', label: 'Days' = f.radio_button :duration_unit, 'weeks', label: 'Weeks' = f.radio_button :duration_unit, 'months', label: 'Months' - = f.text_field :reason, placeholder: t('views.modal.bancontrol.reason'), value: user.ban_reason + = f.text_field :reason, placeholder: t('views.modal.bancontrol.reason'), value: user.bans.current.first&.reason .modal-footer %button.btn.btn-default{ name: 'stop-time', type: :button, data: { dismiss: :modal } }= t 'views.actions.close' = f.submit t('views.modal.bancontrol.hammertime'), class: 'btn btn-primary', name: 'hammer-time' - else = f.hidden_field :ban, value: '0' - .modal-body#ban-control-super + .modal-body - if current_ban.expires_at.nil? This user is currently permanently banned for %strong= current_ban.reason @@ -40,7 +40,7 @@ - if current_ban.banned_by.present? %br This ban was instated by - %strong= current_ban.banned_by.safe_name + %strong= current_ban.banned_by.profile.safe_name on %strong= current_ban.created_at .modal-footer diff --git a/lib/use_case/user/ban.rb b/lib/use_case/user/ban.rb index b5295d69..5ba61f40 100644 --- a/lib/use_case/user/ban.rb +++ b/lib/use_case/user/ban.rb @@ -24,13 +24,15 @@ module UseCase if reason == REASON_SPAM target_user.update!( - display_name: nil, - bio: '', - location: '', - website: '', profile_picture: nil, profile_header: nil ) + target_user.profile.update!( + display_name: nil, + description: '', + location: '', + website: '', + ) end { @@ -43,7 +45,11 @@ module UseCase end def source_user - @source_user ||= ::User.find(source_user_id) + if source_user_id + @source_user ||= ::User.find(source_user_id) + else + nil + end end end end diff --git a/spec/controllers/ajax/moderation_controller_spec.rb b/spec/controllers/ajax/moderation_controller_spec.rb index 013bdd25..63a32cd6 100644 --- a/spec/controllers/ajax/moderation_controller_spec.rb +++ b/spec/controllers/ajax/moderation_controller_spec.rb @@ -373,9 +373,9 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do { user: user_param, ban: ban, - permaban: permaban, reason: "just a prank, bro", - until: wrongly_formatted_date_ugh + duration: duration, + duration_unit: duration_unit, } end @@ -414,17 +414,17 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do context "when ban = 0" do let(:ban) { "0" } - let(:wrongly_formatted_date_ugh) { nil } "01".each_char do |pb| context "when permaban = #{pb}" do - let(:permaban) { pb } + let(:duration) { nil } + let(:duration_unit) { nil } context "when user is already banned" do - before { target_user.ban } + before { target_user.ban(nil) } it "unbans the user" do - expect { subject }.to(change { target_user.reload.banned? }.from(true).to(false)) + expect { subject }.to change { target_user.reload.banned? }.from(true).to(false) end include_examples "returns the expected response" @@ -443,16 +443,18 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do context "when ban = 1" do let(:ban) { "1" } - let(:wrongly_formatted_date_ugh) { "4/20/2420 12:00 AM" } + let(:duration) { 3 } + let(:duration_unit) { 'hours' } context "when permaban = 0" do let(:permaban) { "0" } - it "bans the user until 2420-04-20" do - expect { subject }.to(change { target_user.reload.banned? }.from(false).to(true)) - expect(target_user).not_to be_permanently_banned - expect(target_user.ban_reason).to eq("just a prank, bro") - expect(target_user.banned_until).to eq(DateTime.strptime(wrongly_formatted_date_ugh, "%m/%d/%Y %I:%M %p")) + it "bans the user for 3 hours" do + Timecop.freeze do + expect { subject }.to(change { target_user.reload.banned? }.from(false).to(true)) + expect(target_user.bans.current.first.reason).to eq("just a prank, bro") + expect(target_user.bans.current.first.expires_at).to eq(Time.now.utc + 3.hours) + end end include_examples "returns the expected response" @@ -461,13 +463,13 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do end context "when permaban = 1" do - let(:permaban) { "1" } + let(:duration) { nil } + let(:duration_unit) { nil } it "bans the user for all eternity" do - expect { subject }.to(change { target_user.reload.banned? }.from(false).to(true)) - expect(target_user).to be_permanently_banned - expect(target_user.ban_reason).to eq("just a prank, bro") - expect(target_user.banned_until).to be_nil + expect { subject }.to change { target_user.reload.banned? }.from(false).to(true) + expect(target_user.bans.current.first.reason).to eq("just a prank, bro") + expect(target_user.bans.current.first.expires_at).to be_nil end include_examples "returns the expected response" @@ -480,8 +482,8 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do context "when user does not exist" do let(:user_param) { "fritz-fantom" } let(:ban) { "1" } - let(:permaban) { "1" } - let(:wrongly_formatted_date_ugh) { "4/20/2420 12:00 AM" } + let(:duration) { nil } + let(:duration_unit) { nil } let(:expected_response) do { "success" => false, From 0659ff1f9a86aaa84b620456ae050ed7452a7dd9 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 30 Dec 2021 13:05:52 +0100 Subject: [PATCH 06/14] Update tests for unbanning users --- spec/controllers/ajax/moderation_controller_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/controllers/ajax/moderation_controller_spec.rb b/spec/controllers/ajax/moderation_controller_spec.rb index 63a32cd6..91edb9e6 100644 --- a/spec/controllers/ajax/moderation_controller_spec.rb +++ b/spec/controllers/ajax/moderation_controller_spec.rb @@ -417,8 +417,8 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do "01".each_char do |pb| context "when permaban = #{pb}" do - let(:duration) { nil } - let(:duration_unit) { nil } + let(:duration) { pb == '0' ? 3 : nil } + let(:duration_unit) { pb == '0' ? 'hours' : nil } context "when user is already banned" do before { target_user.ban(nil) } @@ -453,7 +453,7 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do Timecop.freeze do expect { subject }.to(change { target_user.reload.banned? }.from(false).to(true)) expect(target_user.bans.current.first.reason).to eq("just a prank, bro") - expect(target_user.bans.current.first.expires_at).to eq(Time.now.utc + 3.hours) + expect(target_user.bans.current.first.expires_at.to_i).to eq((Time.now.utc + 3.hours).to_i) end end From a9392dad533d18cef95bbb14aff398504d9810e0 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 30 Dec 2021 13:13:35 +0100 Subject: [PATCH 07/14] Use use case for `User#unban` --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 41725ec6..669e36c1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -228,7 +228,7 @@ class User < ApplicationRecord end def unban - self.bans.current.update!(expires_at: DateTime.now) + UseCase::User::Unban.call(id) end # Bans a user. From 7677ed21a4b3e6d91f00de415fa8ad0439ffaa62 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 30 Dec 2021 21:06:21 +0100 Subject: [PATCH 08/14] Update tests for unbanning users --- app/controllers/ajax/moderation_controller.rb | 2 +- app/models/user.rb | 2 +- lib/use_case/user/unban.rb | 3 ++- spec/controllers/ajax/moderation_controller_spec.rb | 9 ++++----- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/ajax/moderation_controller.rb b/app/controllers/ajax/moderation_controller.rb index b6c9d9e9..85f23296 100644 --- a/app/controllers/ajax/moderation_controller.rb +++ b/app/controllers/ajax/moderation_controller.rb @@ -118,7 +118,7 @@ class Ajax::ModerationController < AjaxController reason = params[:reason].to_s target_user = User.find_by_screen_name!(params[:user]) unban = params[:ban] == '0' - perma = params[:duration].nil? + perma = params[:duration].blank? if !unban && target_user.has_role?(:administrator) @response[:status] = :nopriv diff --git a/app/models/user.rb b/app/models/user.rb index 669e36c1..0581621a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -224,7 +224,7 @@ class User < ApplicationRecord # endregion def banned? - self.bans.current.any? + self.bans.current.count > 0 end def unban diff --git a/lib/use_case/user/unban.rb b/lib/use_case/user/unban.rb index d399a940..81c5ca1c 100644 --- a/lib/use_case/user/unban.rb +++ b/lib/use_case/user/unban.rb @@ -9,7 +9,8 @@ module UseCase def call UserBan.current.where(user_id: target_user_id).update_all( - expires_at: DateTime.now + # -1s to account for flakyness with timings in tests + expires_at: DateTime.now - 1.second ) end end diff --git a/spec/controllers/ajax/moderation_controller_spec.rb b/spec/controllers/ajax/moderation_controller_spec.rb index 91edb9e6..7dd713b8 100644 --- a/spec/controllers/ajax/moderation_controller_spec.rb +++ b/spec/controllers/ajax/moderation_controller_spec.rb @@ -424,7 +424,7 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do before { target_user.ban(nil) } it "unbans the user" do - expect { subject }.to change { target_user.reload.banned? }.from(true).to(false) + expect { subject }.to change { target_user.reload.banned? }.from(true).to(false) end include_examples "returns the expected response" @@ -443,15 +443,14 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do context "when ban = 1" do let(:ban) { "1" } - let(:duration) { 3 } - let(:duration_unit) { 'hours' } context "when permaban = 0" do - let(:permaban) { "0" } + let(:duration) { 3 } + let(:duration_unit) { 'hours' } it "bans the user for 3 hours" do Timecop.freeze do - expect { subject }.to(change { target_user.reload.banned? }.from(false).to(true)) + expect { subject }.to change { target_user.reload.banned? }.from(false).to(true) expect(target_user.bans.current.first.reason).to eq("just a prank, bro") expect(target_user.bans.current.first.expires_at.to_i).to eq((Time.now.utc + 3.hours).to_i) end From d06e2c97b2841d76666a99d5f0cdbd8ab6e4379b Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 30 Dec 2021 21:06:36 +0100 Subject: [PATCH 09/14] Add test for blanking out a user's profile if they were banned for spam --- .../ajax/moderation_controller_spec.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/controllers/ajax/moderation_controller_spec.rb b/spec/controllers/ajax/moderation_controller_spec.rb index 7dd713b8..265a1308 100644 --- a/spec/controllers/ajax/moderation_controller_spec.rb +++ b/spec/controllers/ajax/moderation_controller_spec.rb @@ -478,6 +478,33 @@ describe Ajax::ModerationController, :ajax_controller, type: :controller do end end + context "when reason = Spam" do + let(:params) do + { + user: target_user.screen_name, + ban: "1", + reason: "Spam", + duration: nil, + duration_unit: nil, + } + end + + it "empties the user's profile" do + user.profile.display_name = "Veggietales Facts" + user.profile.description = "Are you a fan of Veggietales? Want to expand your veggie knowledge? Here at Veggietales Facts, we tweet trivia for fans like you." + user.profile.location = "Hell" + user.profile.website = "https://twitter.com/veggiefact" + + expect { subject }.to change { target_user.reload.banned? }.from(false).to(true) + expect(target_user.bans.current.first.reason).to eq("Spam") + + expect(target_user.profile.display_name).to be_nil + expect(target_user.profile.description).to be_empty + expect(target_user.profile.location).to be_empty + expect(target_user.profile.website).to be_empty + end + end + context "when user does not exist" do let(:user_param) { "fritz-fantom" } let(:ban) { "1" } From ae83353b21b15d4a4f180dfa567f0801360306d8 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 30 Dec 2021 21:35:24 +0100 Subject: [PATCH 10/14] Update ban logic in Rakefile --- Rakefile | 58 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/Rakefile b/Rakefile index 9c689bd9..b386660c 100644 --- a/Rakefile +++ b/Rakefile @@ -154,9 +154,11 @@ namespace :justask do fail "screen name required" if args[:screen_name].nil? user = User.find_by_screen_name(args[:screen_name]) fail "user #{args[:screen_name]} not found" if user.nil? - user.permanently_banned = true - user.ban_reason = args[:reason] - user.save! + UseCase::User::Ban.call( + target_user_id: user.id, + expiry: nil, + reason: args[:reason], + ) puts "#{user.screen_name} got hit by\033[5m YE OLDE BANHAMMER\033[0m!!1!" end @@ -164,10 +166,11 @@ namespace :justask do task :ban, [:screen_name, :reason] => :environment do |t, args| fail "screen name required" if args[:screen_name].nil? user = User.find_by_screen_name(args[:screen_name]) - user.permanently_banned = false - user.banned_until = DateTime.current + 1 - user.ban_reason = args[:reason] - user.save! + UseCase::User::Ban.call( + target_user_id: user.id, + expiry: DateTime.current + 1, + reason: args[:reason], + ) puts "#{user.screen_name} got hit by\033[5m YE OLDE BANHAMMER\033[0m!!1!" end @@ -175,10 +178,11 @@ namespace :justask do task :week_ban, [:screen_name, :reason] => :environment do |t, args| fail "screen name required" if args[:screen_name].nil? user = User.find_by_screen_name(args[:screen_name]) - user.permanently_banned = false - user.banned_until = DateTime.current + 7 - user.ban_reason = args[:reason] - user.save! + UseCase::User::Ban.call( + target_user_id: user.id, + expiry: DateTime.current + 7, + reason: args[:reason], + ) puts "#{user.screen_name} got hit by\033[5m YE OLDE BANHAMMER\033[0m!!1!" end @@ -186,10 +190,11 @@ namespace :justask do task :month_ban, [:screen_name, :reason] => :environment do |t, args| fail "screen name required" if args[:screen_name].nil? user = User.find_by_screen_name(args[:screen_name]) - user.permanently_banned = false - user.banned_until = DateTime.current + 30 - user.ban_reason = args[:reason] - user.save! + UseCase::User::Ban.call( + target_user_id: user.id, + expiry: DateTime.current + 30, + reason: args[:reason], + ) puts "#{user.screen_name} got hit by\033[5m YE OLDE BANHAMMER\033[0m!!1!" end @@ -197,10 +202,11 @@ namespace :justask do task :year_ban, [:screen_name, :reason] => :environment do |t, args| fail "screen name required" if args[:screen_name].nil? user = User.find_by_screen_name(args[:screen_name]) - user.permanently_banned = false - user.banned_until = DateTime.current + 365 - user.ban_reason = args[:reason] - user.save! + UseCase::User::Ban.call( + target_user_id: user.id, + expiry: DateTime.current + 365, + reason: args[:reason], + ) puts "#{user.screen_name} got hit by\033[5m YE OLDE BANHAMMER\033[0m!!1!" end @@ -208,10 +214,11 @@ namespace :justask do task :aeon_ban, [:screen_name, :reason] => :environment do |t, args| fail "screen name required" if args[:screen_name].nil? user = User.find_by_screen_name(args[:screen_name]) - user.permanently_banned = false - user.banned_until = DateTime.current + 365_000_000_000 - user.ban_reason = args[:reason] - user.save! + UseCase::User::Ban.call( + target_user_id: user.id, + expiry: DateTime.current + 365_000_000_000, + reason: args[:reason], + ) puts "#{user.screen_name} got hit by\033[5m YE OLDE BANHAMMER\033[0m!!1!" end @@ -220,10 +227,7 @@ namespace :justask do fail "screen name required" if args[:screen_name].nil? user = User.find_by_screen_name(args[:screen_name]) fail "user #{args[:screen_name]} not found" if user.nil? - user.permanently_banned = false - user.banned_until = nil - user.ban_reason = nil - user.save! + UseCase::User::Unban.call(user.id) puts "#{user.screen_name} is no longer banned." end From 374da66cd1bbd3e435aa4eb57eccd3e78519fb47 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 30 Dec 2021 21:39:05 +0100 Subject: [PATCH 11/14] Add `UserBan` to Rails Admin --- config/initializers/rails_admin.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/initializers/rails_admin.rb b/config/initializers/rails_admin.rb index 22954c2e..abbf80f2 100644 --- a/config/initializers/rails_admin.rb +++ b/config/initializers/rails_admin.rb @@ -45,5 +45,6 @@ RailsAdmin.config do |config| Smile Theme User + UserBan ] end From b196cbdd1c1e5c5b48746f7dd9d92625de9c97b1 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 30 Dec 2021 21:56:03 +0100 Subject: [PATCH 12/14] Adjust ban script to work when unbanning --- app/javascript/legacy/moderation/ban.coffee | 29 +++++++++++---------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/app/javascript/legacy/moderation/ban.coffee b/app/javascript/legacy/moderation/ban.coffee index e37296de..934fcd3e 100644 --- a/app/javascript/legacy/moderation/ban.coffee +++ b/app/javascript/legacy/moderation/ban.coffee @@ -6,18 +6,19 @@ load = -> banCheckbox = modalForm.querySelector('[name="ban"][type="checkbox"]') permabanCheckbox = modalForm.querySelector('[name="permaban"][type="checkbox"]') - banCheckbox.addEventListener "change", (event) -> - $t = $ this - if $t.is(":checked") - $("#ban-controls").show() - else - $("#ban-controls").hide() - permabanCheckbox.addEventListener "change", (event) -> - $t = $ this - if $t.is(":checked") - $("#ban-controls-time").hide() - else - $("#ban-controls-time").show() + if banCheckbox + banCheckbox.addEventListener "change", (event) -> + $t = $ this + if $t.is(":checked") + $("#ban-controls").show() + else + $("#ban-controls").hide() + permabanCheckbox.addEventListener "change", (event) -> + $t = $ this + if $t.is(":checked") + $("#ban-controls-time").hide() + else + $("#ban-controls-time").show() modalForm.addEventListener "submit", (event) -> event.preventDefault(); @@ -29,11 +30,11 @@ load = -> "0" data = { - ban: checktostr banCheckbox + ban: banCheckbox ? checktostr banCheckbox : false user: modalForm.elements["user"].value } - if banCheckbox.checked + if banCheckbox && banCheckbox.checked data.reason = modalForm.elements["reason"].value.trim() unless permabanCheckbox.checked data.duration = modalForm.elements["duration"].value.trim() From b249e4027360894e3215f04eec5b292d300817f0 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Fri, 31 Dec 2021 00:17:09 +0100 Subject: [PATCH 13/14] Fix ban checkbox being passed into ban payload --- app/javascript/legacy/moderation/ban.coffee | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/javascript/legacy/moderation/ban.coffee b/app/javascript/legacy/moderation/ban.coffee index 934fcd3e..f62b8b3b 100644 --- a/app/javascript/legacy/moderation/ban.coffee +++ b/app/javascript/legacy/moderation/ban.coffee @@ -23,18 +23,13 @@ load = -> modalForm.addEventListener "submit", (event) -> event.preventDefault(); - checktostr = (el) -> - if el.checked - "1" - else - "0" - data = { - ban: banCheckbox ? checktostr banCheckbox : false + ban: "0" user: modalForm.elements["user"].value } if banCheckbox && banCheckbox.checked + data.ban = "1" data.reason = modalForm.elements["reason"].value.trim() unless permabanCheckbox.checked data.duration = modalForm.elements["duration"].value.trim() From b398265a98d25e5036bc94ee8ae706a38aa5784d Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Thu, 6 Jan 2022 13:56:55 +0100 Subject: [PATCH 14/14] Address review comments from @nilsding Co-authored-by: Georg Gadinger --- app/models/user.rb | 2 +- spec/models/user_ban_spec.rb | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) delete mode 100644 spec/models/user_ban_spec.rb diff --git a/app/models/user.rb b/app/models/user.rb index 0581621a..612c8a8d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -232,7 +232,7 @@ class User < ApplicationRecord end # Bans a user. - # @param duration [Fixnum, nil] Ban duration + # @param duration [Integer?] Ban duration # @param duration_unit [String, nil] Unit for the duration parameter. Accepted units: hours, days, weeks, months # @param reason [String] Reason for the ban. This is displayed to the user. # @param banned_by [User] User who instated the ban diff --git a/spec/models/user_ban_spec.rb b/spec/models/user_ban_spec.rb deleted file mode 100644 index f7902eea..00000000 --- a/spec/models/user_ban_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'rails_helper' - -RSpec.describe UserBan, type: :model do - pending "add some examples to (or delete) #{__FILE__}" -end