bans: don't allow users to be double banned.

app/models/ban.rb

@@ -6,11 +6,15 @@ class Ban < ApplicationRecord
   after_destroy :create_unban_mod_action
   belongs_to :user
   belongs_to :banner, :class_name => "User"
+
   validates_presence_of :reason, :duration
+  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
@@ -48,6 +52,10 @@ class Ban < ApplicationRecord
     end
   end
 
+  def validate_user_is_bannable
+    self.errors[:user] << "is already banned" if user.is_banned?
+  end
+
   def update_user_on_create
     user.update!(is_banned: true)
   end
@@ -69,8 +77,6 @@ class Ban < ApplicationRecord
     @duration = dur
   end
 
-  attr_reader :duration
-
   def humanized_duration
     ApplicationController.helpers.distance_of_time_in_words(created_at, expires_at)
   end

test/factories/ban.rb

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

test/functional/bans_controller_test.rb

@@ -4,10 +4,7 @@ class BansControllerTest < ActionDispatch::IntegrationTest
   context "A bans controller" do
     setup do
       @mod = create(:moderator_user)
-      @user = create(:user)
+      @ban = create(:ban)
-      as(@mod) do
-        @ban = create(:ban, user: @user)
-      end
     end
 
     context "new action" do
@@ -40,7 +37,7 @@ class BansControllerTest < ActionDispatch::IntegrationTest
 
     context "search action" do
       should "render" do
-        get_auth bans_path(search: {user_name: @user.name}), @mod
+        get_auth bans_path(search: { user_name: @ban.user.name }), @mod
         assert_response :success
       end
     end
@@ -48,6 +45,7 @@ class BansControllerTest < ActionDispatch::IntegrationTest
     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 }}
 
           assert_redirected_to bans_path
@@ -77,12 +75,20 @@ 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 }}
 
           assert_response 403
           assert_equal(false, @mod.reload.is_banned?)
         end
       end
+
+      should "not allow users to be double banned" do
+        assert_difference("Ban.count", 0) do
+          post_auth bans_path, @mod, params: { ban: { duration: 60, reason: "xxx", user_id: @ban.user.id }}
+          assert_response :success
+        end
+      end
     end
 
     context "update action" do