FEATURE: Allow topic URL without post number for post_id param (#275)

This commit allows base topic URLs (e.g. https://meta.discourse.org/t/hide-text-in-text-select-popup-menu-feedback/287656)
to be used by the post_id parameter type. We just assume in this case
that the post_id for the param is 1.

assets/javascripts/discourse/components/param-input.js

@@ -124,7 +124,13 @@ export default class ParamInput extends Component {
       case "int_list":
         return value.split(",").every((i) => /^(-?\d+|null)$/.test(i.trim()));
       case "post_id":
-        return isPositiveInt || /\d+\/\d+(\?u=.*)?$/.test(value);
+        return (
+          isPositiveInt ||
+          /\d+\/\d+(\?u=.*)?$/.test(value) ||
+          /\/t\/[^/]+\/(\d+)(\?u=.*)?/.test(value)
+        );
+      case "topic_id":
+        return isPositiveInt || /\/t\/[^/]+\/(\d+)/.test(value);
       case "category_id":
         if (isPositiveInt) {
           return !!this.site.categories.find((c) => c.id === intVal);

lib/discourse_data_explorer/parameter.rb

@@ -67,6 +67,46 @@ module ::DiscourseDataExplorer
       @type_aliases ||= { integer: :int, text: :string, timestamp: :datetime }
     end
 
+    def self.create_from_sql(sql, opts = {})
+      in_params = false
+      ret_params = []
+      sql.lines.find do |line|
+        line.chomp!
+
+        if in_params
+          # -- (ident) :(ident) (= (ident))?
+
+          if line =~ /^\s*--\s*([a-zA-Z_ ]+)\s*:([a-z_]+)\s*(?:=\s+(.*)\s*)?$/
+            type = $1
+            ident = $2
+            default = $3
+            nullable = false
+            if type =~ /^(null)?(.*?)(null)?$/i
+              nullable = true if $1 || $3
+              type = $2
+            end
+            type = type.strip
+
+            begin
+              ret_params << Parameter.new(ident, type, default, nullable)
+            rescue StandardError
+              raise if opts[:strict]
+            end
+
+            false
+          elsif line =~ /^\s+$/
+            false
+          else
+            true
+          end
+        else
+          in_params = true if line =~ /^\s*--\s*\[params\]\s*$/
+          false
+        end
+      end
+      ret_params
+    end
+
     def cast_to_ruby(string)
       string = @default unless string
 
@@ -79,14 +119,6 @@ module ::DiscourseDataExplorer
       end
       return nil if string.downcase == "#null"
 
-      def invalid_format(string, msg = nil)
-        if msg
-          raise ValidationError.new("'#{string}' is an invalid #{type} - #{msg}")
-        else
-          raise ValidationError.new("'#{string}' is an invalid value for #{type}")
-        end
-      end
-
       value = nil
 
       case @type
@@ -144,7 +176,7 @@ module ::DiscourseDataExplorer
           parent = Category.query_parent_category(parent_name)
           invalid_format string, "Could not find category named #{parent_name}" unless parent
           object = Category.query_category(child_name, parent)
-          unless object
+          if object.blank?
             invalid_format string,
                            "Could not find subcategory of #{parent_name} named #{child_name}"
           end
@@ -152,18 +184,18 @@ module ::DiscourseDataExplorer
           object =
             Category.where(id: string.to_i).first || Category.where(slug: string).first ||
               Category.where(name: string).first
-          invalid_format string, "Could not find category named #{string}" unless object
+          invalid_format string, "Could not find category named #{string}" if object.blank?
         end
 
         value = object.id
       when :user_id, :post_id, :topic_id, :group_id, :badge_id
         if string.gsub(/[ _]/, "") =~ /^-?\d+$/
-          clazz_name = (/^(.*)_id$/.match(type.to_s)[1].classify.to_sym)
+          klass_name = (/^(.*)_id$/.match(type.to_s)[1].classify.to_sym)
           begin
-            object = Object.const_get(clazz_name).with_deleted.find(string.gsub(/[ _]/, "").to_i)
+            object = Object.const_get(klass_name).with_deleted.find(string.gsub(/[ _]/, "").to_i)
             value = object.id
           rescue ActiveRecord::RecordNotFound
-            invalid_format string, "The specified #{clazz_name} was not found"
+            invalid_format string, "The specified #{klass_name} was not found"
           end
         elsif type == :user_id
           begin
@@ -173,9 +205,13 @@ module ::DiscourseDataExplorer
             invalid_format string, "The user named #{string} was not found"
           end
         elsif type == :post_id
-          if string =~ %r{(\d+)/(\d+)(\?u=.*)?$}
+          if string =~ %r{/t/[^/]+/(\d+)(\?u=.*)?$}
+            object = Post.with_deleted.find_by(topic_id: $1, post_number: 1)
+            invalid_format string, "The first post for topic:#{$1} was not found" if object.blank?
+            value = object.id
+          elsif string =~ %r{(\d+)/(\d+)(\?u=.*)?$}
             object = Post.with_deleted.find_by(topic_id: $1, post_number: $2)
-            unless object
+            if object.blank?
               invalid_format string, "The post at topic:#{$1} post_number:#{$2} was not found"
             end
             value = object.id
@@ -191,7 +227,7 @@ module ::DiscourseDataExplorer
           end
         elsif type == :group_id
           object = Group.where(name: string).first
-          invalid_format string, "The group named #{string} was not found" unless object
+          invalid_format string, "The group named #{string} was not found" if object.blank?
           value = object.id
         else
           invalid_format string
@@ -212,44 +248,14 @@ module ::DiscourseDataExplorer
       value
     end
 
-    def self.create_from_sql(sql, opts = {})
+    private
-      in_params = false
-      ret_params = []
-      sql.lines.find do |line|
-        line.chomp!
 
-        if in_params
+    def invalid_format(string, msg = nil)
-          # -- (ident) :(ident) (= (ident))?
+      if msg
-
+        raise ValidationError.new("'#{string}' is an invalid #{type} - #{msg}")
-          if line =~ /^\s*--\s*([a-zA-Z_ ]+)\s*:([a-z_]+)\s*(?:=\s+(.*)\s*)?$/
+      else
-            type = $1
+        raise ValidationError.new("'#{string}' is an invalid value for #{type}")
-            ident = $2
-            default = $3
-            nullable = false
-            if type =~ /^(null)?(.*?)(null)?$/i
-              nullable = true if $1 || $3
-              type = $2
-            end
-            type = type.strip
-
-            begin
-              ret_params << Parameter.new(ident, type, default, nullable)
-            rescue StandardError
-              raise if opts[:strict]
-            end
-
-            false
-          elsif line =~ /^\s+$/
-            false
-          else
-            true
-          end
-        else
-          in_params = true if line =~ /^\s*--\s*\[params\]\s*$/
-          false
-        end
       end
-      ret_params
     end
   end
 end

spec/lib/parameter_spec.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+RSpec.describe DiscourseDataExplorer::Parameter do
+  def param(identifier, type, default, nullable)
+    described_class.new(identifier, type, default, nullable)
+  end
+
+  describe ".cast_to_ruby" do
+    it "returns nil for nullable blank string" do
+      expect(param("param123", :string, nil, true).cast_to_ruby("")).to eq(nil)
+    end
+
+    it "raises error for not-nullable blank string" do
+      expect { param("param123", :string, nil, false).cast_to_ruby("") }.to raise_error(
+        ::DiscourseDataExplorer::ValidationError,
+      )
+    end
+
+    describe "post_id type" do
+      fab!(:post)
+
+      context "when the value provided is a post share URL" do
+        it "returns the found post id" do
+          expect(param("post_id", :post_id, nil, false).cast_to_ruby(post.url)).to eq(post.id)
+        end
+
+        it "returns the found post id when there is a share user param" do
+          expect(
+            param("post_id", :post_id, nil, false).cast_to_ruby(
+              "#{post.url}?u=#{post.user.username}",
+            ),
+          ).to eq(post.id)
+        end
+
+        it "returns the found post id when no post number is provided" do
+          expect(
+            param("post_id", :post_id, nil, false).cast_to_ruby("#{post.url(share_url: true)}"),
+          ).to eq(post.id)
+        end
+
+        it "raises an error if no such post exists" do
+          post.destroy
+          expect { param("post_id", :post_id, nil, false).cast_to_ruby(post.url) }.to raise_error(
+            ::DiscourseDataExplorer::ValidationError,
+          )
+        end
+      end
+
+      context "when the value provided is an integer" do
+        it "raises an error if no such post exists" do
+          expect { param("post_id", :post_id, nil, false).cast_to_ruby("-999") }.to raise_error(
+            ::DiscourseDataExplorer::ValidationError,
+          )
+        end
+
+        it "returns the post id if the post exists" do
+          expect(param("post_id", :post_id, nil, false).cast_to_ruby(post.id.to_s)).to eq(post.id)
+        end
+      end
+    end
+  end
+end