From 48a4038809d926c33f76083f89cf6a63fb9aa184 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 16 Jan 2024 15:09:05 +1000 Subject: [PATCH] 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. --- .../discourse/components/param-input.js | 8 +- lib/discourse_data_explorer/parameter.rb | 110 +++++++++--------- spec/lib/parameter_spec.rb | 62 ++++++++++ 3 files changed, 127 insertions(+), 53 deletions(-) create mode 100644 spec/lib/parameter_spec.rb diff --git a/assets/javascripts/discourse/components/param-input.js b/assets/javascripts/discourse/components/param-input.js index b6bbe9c..7c75f1a 100644 --- a/assets/javascripts/discourse/components/param-input.js +++ b/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); diff --git a/lib/discourse_data_explorer/parameter.rb b/lib/discourse_data_explorer/parameter.rb index 04d1464..b244726 100644 --- a/lib/discourse_data_explorer/parameter.rb +++ b/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 = {}) - in_params = false - ret_params = [] - sql.lines.find do |line| - line.chomp! + private - 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 + 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 - ret_params end end end diff --git a/spec/lib/parameter_spec.rb b/spec/lib/parameter_spec.rb new file mode 100644 index 0000000..70752ec --- /dev/null +++ b/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