From 6bf3ac74269ff5c6a3c6ad7e0b4c7543fa621255 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E9=94=A6=E5=BF=83?=
 <41134017+Lhcfl@users.noreply.github.com>
Date: Wed, 21 Aug 2024 12:03:29 +0800
Subject: [PATCH] FIX: Fix `user_id` validation (#312)

Validation of `user_id` parameter will throw a 500 error because
`User.find_by_username_or_email` does not throw
`ActiveRecord::RecordNotFound`, but silently returns `nil`.
This results in a `NoMethodError` in `object.id` on the next line
---
 lib/discourse_data_explorer/parameter.rb |  9 +++------
 spec/lib/parameter_spec.rb               | 17 +++++++++++++++++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/lib/discourse_data_explorer/parameter.rb b/lib/discourse_data_explorer/parameter.rb
index da9a260..a49de14 100644
--- a/lib/discourse_data_explorer/parameter.rb
+++ b/lib/discourse_data_explorer/parameter.rb
@@ -205,12 +205,9 @@ module ::DiscourseDataExplorer
             invalid_format string, "The specified #{klass_name} was not found"
           end
         elsif type == :user_id
-          begin
-            object = User.find_by_username_or_email(string)
-            value = object.id
-          rescue ActiveRecord::RecordNotFound
-            invalid_format string, "The user named #{string} was not found"
-          end
+          object = User.find_by_username_or_email(string)
+          invalid_format string, "The user named #{string} was not found" if object.blank?
+          value = object.id
         elsif type == :post_id
           if string =~ %r{/t/[^/]+/(\d+)(\?u=.*)?$}
             object = Post.with_deleted.find_by(topic_id: $1, post_number: 1)
diff --git a/spec/lib/parameter_spec.rb b/spec/lib/parameter_spec.rb
index 20cd170..eeea570 100644
--- a/spec/lib/parameter_spec.rb
+++ b/spec/lib/parameter_spec.rb
@@ -76,6 +76,23 @@ RSpec.describe DiscourseDataExplorer::Parameter do
         end
       end
     end
+
+    describe "user_id type" do
+      fab!(:user)
+      it "raises an error if no such user exists" do
+        expect {
+          param("user_id", :user_id, nil, false).cast_to_ruby("user_not_exist")
+        }.to raise_error(::DiscourseDataExplorer::ValidationError)
+        expect {
+          param("user_id", :user_id, nil, false).cast_to_ruby("user_not_exist@fake.email")
+        }.to raise_error(::DiscourseDataExplorer::ValidationError)
+      end
+
+      it "returns the user id if the user exists" do
+        expect(param("user_id", :user_id, nil, false).cast_to_ruby(user.username)).to eq(user.id)
+        expect(param("user_id", :user_id, nil, false).cast_to_ruby(user.email)).to eq(user.id)
+      end
+    end
   end
 
   describe ".create_from_sql" do