deal with users who have judgements when you want to delete them. (#999)

app/controllers/admin/users_controller.rb

@@ -4,7 +4,7 @@
 
 module Admin
   class UsersController < Admin::AdminController
-    before_action :set_user, only: [ :show, :edit, :update, :destroy ]
+    before_action :set_user, only: [ :show, :edit, :update, :destroy, :assign_judgements_to_anonymous_user ]
 
     # GET /admin/users
     # GET /admin/users.json
@@ -101,13 +101,28 @@ def update
     # DELETE /admin/users/1
     # DELETE /admin/users/1.json
     def destroy
-      @user.destroy
-      respond_to do |format|
-        format.html { redirect_to admin_users_url, notice: 'User account was successfully deleted.' }
-        format.json { head :no_content }
+      if @user.destroy
+        respond_to do |format|
+          format.html { redirect_to admin_users_url, notice: 'User account was successfully deleted.' }
+          format.json { head :no_content }
+        end
+      else
+        respond_to do |format|
+          format.html { render :edit }
+          format.json { render json: @user.errors, status: :unprocessable_entity }
+        end
       end
     end
 
+    def assign_judgements_to_anonymous_user
+      @user.judgements.each do |j|
+        j.user = nil
+        j.save!
+      end
+
+      redirect_to admin_user_path @user, notice: 'All judgements assigned to anonymous user.'
+    end
+
     private
 
     # Use callbacks to share common setup or constraints between actions.

app/controllers/books_controller.rb

@@ -37,7 +37,12 @@ def show
     unique_judge_ids = @book.query_doc_pairs.joins(:judgements)
       .distinct.pluck(:user_id)
     unique_judge_ids.each do |judge_id|
-      judge = User.find(judge_id) unless judge_id.nil?
+      begin
+        judge = User.find(judge_id) unless judge_id.nil?
+      rescue ActiveRecord::RecordNotFound
+        puts 'got a nil'
+        judge = nil
+      end
       @leaderboard_data << { judge:      judge.nil? ? 'anonymous' : judge.fullname,
                              judgements: @book.judgements.where(user: judge).count }
       @stats_data << {

app/models/judgement.rb

@@ -17,7 +17,7 @@
 # Indexes
 #
 #  index_judgements_on_query_doc_pair_id              (query_doc_pair_id)
-#  index_judgements_on_user_id_and_query_doc_pair_id  (user_id,query_doc_pair_id)
+#  index_judgements_on_user_id_and_query_doc_pair_id  (user_id,query_doc_pair_id) UNIQUE
 #
 # Foreign Keys
 #
@@ -27,10 +27,7 @@ class Judgement < ApplicationRecord
   belongs_to :query_doc_pair
   belongs_to :user, optional: true
 
-  # rubocop:disable Rails/UniqueValidationWithoutIndex
-  validates :user_id, :uniqueness => { :scope => :query_doc_pair_id }
-  # rubocop:enable Rails/UniqueValidationWithoutIndex
-
+  validates :user_id, :uniqueness => { :scope => :query_doc_pair_id }, unless: -> { user_id.nil? }
   validates :rating,
             presence: true, unless: :rating_not_required?
 

app/models/user.rb

@@ -107,6 +107,9 @@ class User < ApplicationRecord
   has_many :scores,
            dependent: :destroy
 
+  has_many :judgements,
+           dependent: :restrict_with_error
+
   has_many :case_metadata,
            class_name: 'CaseMetadatum',
            dependent:  :destroy
@@ -146,6 +149,7 @@ def terms_and_conditions?
   before_create :set_defaults
   before_destroy :check_team_ownership_before_removing!, prepend: true
   before_destroy :check_scorer_ownership_before_removing!, prepend: true
+  before_destroy :check_judgements_before_removing!, prepend: true
 
   def check_team_ownership_before_removing!
     owned_teams.each do |team|
@@ -156,6 +160,13 @@ def check_team_ownership_before_removing!
     end
   end
 
+  def check_judgements_before_removing!
+    if judgements.count.positive?
+      errors.add(:base, "Please reassign ownership of the #{judgements.count} judgements." )
+      throw(:abort)
+    end
+  end
+
   def check_scorer_ownership_before_removing!
     owned_scorers.each do |scorer|
       if shared_scorers.include?(scorer)

app/views/admin/users/_form.html.erb

@@ -134,3 +134,13 @@
     </div>
   <% end %>
 <% end %>
+
+<hr/>
+
+<h3>Assign Judgements to Anonymous User</h3>
+<p class="form-text">
+  If you need to delete a user then you need to assign their judgements over to the anonymous user.  This user has <%=@user.judgements.count %> judgements total. 
+</p>
+<%= button_to 'Assign Judgements to Anonymous User', assign_judgements_to_anonymous_user_admin_user_path(@user), method: :post, class: 'btn btn-sm btn-danger' %>
+
+<p/>

app/views/profiles/show.html.erb

@@ -40,7 +40,7 @@
                     <p class="small text-muted mb-1">
                       Ratings
                     </p>
-                    <p class="mb-0">n/a</p>
+                    <p class="mb-0"><%= current_user.judgements.count %></p>
                   </div>
                 </div>
                 <div class="d-flex pt-1">

config/routes.rb

@@ -100,6 +100,9 @@
     resources :users do
       resource :lock, only: [ :update ], module: :users
       resource :pulse, only: [ :show ], module: :users
+      member do
+        post :assign_judgements_to_anonymous_user
+      end
     end
     resources :communal_scorers
     resources :announcements do

db/migrate/20240409185546_assign_judgements_for_deleted_users_to_anonymous.rb

@@ -0,0 +1,9 @@
+class AssignJudgementsForDeletedUsersToAnonymous < ActiveRecord::Migration[7.1]
+  def change
+    judgements = Judgement.left_outer_joins(:user).where(users: { id: nil })
+    judgements.each do |j|
+      j.user = nil
+      j.save!
+    end
+  end
+end

db/migrate/20240409185548_add_partial_unique_index_to_judgements.rb

@@ -0,0 +1,6 @@
+class AddPartialUniqueIndexToJudgements < ActiveRecord::Migration[7.1]
+  def change
+    remove_index :judgements, [:user_id, :query_doc_pair_id]
+    add_index :judgements, [:user_id, :query_doc_pair_id], unique: true, where: 'user_id IS NOT NULL'
+  end
+end

test/fixtures/judgements.yml

@@ -15,7 +15,7 @@
 # Indexes
 #
 #  index_judgements_on_query_doc_pair_id              (query_doc_pair_id)
-#  index_judgements_on_user_id_and_query_doc_pair_id  (user_id,query_doc_pair_id)
+#  index_judgements_on_user_id_and_query_doc_pair_id  (user_id,query_doc_pair_id) UNIQUE
 #
 # Foreign Keys
 #

test/models/judgement_test.rb

@@ -17,7 +17,7 @@
 # Indexes
 #
 #  index_judgements_on_query_doc_pair_id              (query_doc_pair_id)
-#  index_judgements_on_user_id_and_query_doc_pair_id  (user_id,query_doc_pair_id)
+#  index_judgements_on_user_id_and_query_doc_pair_id  (user_id,query_doc_pair_id) UNIQUE
 #
 # Foreign Keys
 #
@@ -42,6 +42,15 @@ class JudgementTest < ActiveSupport::TestCase
       judgement2 = Judgement.create(user: user2, query_doc_pair: query_doc_pair, rating: 1.0)
       assert judgement2.save
     end
+
+    test 'However multiple anonymous judgements is okay' do
+      judgement = Judgement.create(user: nil, query_doc_pair: query_doc_pair, rating: 4.4)
+      assert judgement.save
+
+      duplicate_judgement = Judgement.create(user: nil, query_doc_pair: query_doc_pair, rating: 1.0)
+      assert duplicate_judgement.save
+      assert_not duplicate_judgement.errors.include?(:user_id)
+    end
   end
   describe 'unrateable attribute behavior' do
     let(:query_doc_pair) { query_doc_pairs(:one) }

test/models/user_test.rb

@@ -246,6 +246,12 @@ class UserTest < ActiveSupport::TestCase
     end
 
     it 'prevents a user who owns a scorer shared with a team from being deleted' do
+      # need to null out the judgements first
+      random.judgements.each do |j|
+        j.user = nil
+        j.save!
+      end
+
       random.destroy
       assert_not random.destroyed?
       assert random.errors.full_messages_for(:base).include?('Please remove the scorer Scorer for sharing from the team before deleting this user.')
@@ -281,6 +287,14 @@ class UserTest < ActiveSupport::TestCase
     it 'prevents access to the books because the user is not part of a team' do
       assert_nil user_without_book_access.books_involved_with.where(id: book_of_comedy_films).first
     end
+
+    it 'prevents you from deleting a user that has judgements' do
+      assert user_with_book_access.judgements.size.positive?
+
+      assert_not user_with_book_access.destroy
+      assert user_with_book_access.errors.include?(:base)
+      assert_not user_with_book_access.destroyed?
+    end
   end
 
   describe 'User accessing Search Endpoints' do