AstonJ
(AstonJ)
April 2, 2020, 8:31am
1
Not sure if this is a bug or whether I’m doing something wrong, but it seems the first element in an int_list
array is always replaced with 0 in a data explorer query.
Eg:
-- int_list :categories = "3, 5, 6"
gets changed to [0, 5, 6]
looking at:
# frozen_string_literal: true
# name: discourse-data-explorer
# about: Allows you to make SQL queries against your live database, allowing for up-to-the-minute stats reporting.
# meta_topic_id: 32566
# version: 0.3
# authors: Riking
# url: https://github.com/discourse/discourse-data-explorer
enabled_site_setting :data_explorer_enabled
register_asset "stylesheets/explorer.scss"
register_svg_icon "caret-down"
register_svg_icon "caret-right"
register_svg_icon "chevron-left"
register_svg_icon "exclamation-circle"
register_svg_icon "info"
register_svg_icon "pencil-alt"
register_svg_icon "upload"
add_admin_route "explorer.title", "explorer"
module ::DiscourseDataExplorer
PLUGIN_NAME = "discourse-data-explorer"
# This should always match the max value for the
# data_explorer_query_result_limit site setting
QUERY_RESULT_MAX_LIMIT = 10_000
end
require_relative "lib/discourse_data_explorer/engine"
after_initialize do
require_relative "app/jobs/scheduled/delete_hidden_queries"
require_relative "lib/discourse_data_explorer/data_explorer"
require_relative "lib/discourse_data_explorer/parameter"
require_relative "lib/discourse_data_explorer/queries"
require_relative "lib/discourse_data_explorer/query_group_bookmarkable"
GlobalSetting.add_default(:max_data_explorer_api_reqs_per_10_seconds, 2)
# Available options:
# - warn
# - warn+block
# - block
GlobalSetting.add_default(:max_data_explorer_api_req_mode, "warn")
add_to_class(:guardian, :user_is_a_member_of_group?) do |group|
return false if !current_user
return true if current_user.admin?
current_user.group_ids.include?(group.id)
end
add_to_class(:guardian, :user_can_access_query?) do |query|
return false if !current_user
return true if current_user.admin?
query.groups.blank? || query.groups.any? { |group| user_is_a_member_of_group?(group) }
end
add_to_class(:guardian, :group_and_user_can_access_query?) do |group, query|
return false if !current_user
return true if current_user.admin?
user_is_a_member_of_group?(group) && query.groups.exists?(id: group.id)
end
add_to_serializer(
:group_show,
:has_visible_data_explorer_queries,
include_condition: -> { scope.user_is_a_member_of_group?(object) },
) { DiscourseDataExplorer::Query.for_group(object).exists? }
register_bookmarkable(DiscourseDataExplorer::QueryGroupBookmarkable)
add_api_key_scope(
:discourse_data_explorer,
{ run_queries: { actions: %w[discourse_data_explorer/query#run], params: %i[id] } },
)
require_relative "lib/report_generator"
require_relative "lib/result_to_markdown"
reloadable_patch do
if defined?(DiscourseAutomation)
add_automation_scriptable("recurring_data_explorer_result_pm") do
queries =
DiscourseDataExplorer::Query
.where(hidden: false)
.map { |q| { id: q.id, translated_name: q.name } }
field :recipients, component: :email_group_user, required: true
field :query_id, component: :choices, required: true, extra: { content: queries }
field :query_params, component: :"key-value", accepts_placeholders: true
field :skip_empty, component: :boolean
version 1
triggerables [:recurring]
script do |_, fields, automation|
recipients = Array(fields.dig("recipients", "value")).uniq
query_id = fields.dig("query_id", "value")
query_params = fields.dig("query_params", "value") || {}
skip_empty = fields.dig("skip_empty", "value") || false
unless SiteSetting.data_explorer_enabled
Rails.logger.warn "#{DiscourseDataExplorer::PLUGIN_NAME} - plugin must be enabled to run automation #{automation.id}"
next
end
if recipients.blank?
Rails.logger.warn "#{DiscourseDataExplorer::PLUGIN_NAME} - couldn't find any recipients for automation #{automation.id}"
next
end
DiscourseDataExplorer::ReportGenerator
.generate(query_id, query_params, recipients, { skip_empty: })
.each do |pm|
begin
utils.send_pm(pm, automation_id: automation.id, prefers_encrypt: false)
rescue ActiveRecord::RecordNotSaved => e
Rails.logger.warn "#{DiscourseDataExplorer::PLUGIN_NAME} - couldn't send PM for automation #{automation.id}: #{e.message}"
end
end
end
end
end
end
end
This file has been truncated. show original
If I do that in console I get the expected output:
string = "3,5,6"
value = string.split(',').map { |s| s.downcase == '#null' ? nil : s.to_i }
=> [3, 5, 6]
You can reproduce with this query:
-- [params]
-- int_list :categories = "3, 5, 6"
SELECT *
FROM topics
WHERE topics.category_id = ANY (ARRAY [ :categories ] )
To break it (so you can view the error) simply remove the closing bracket, eg:
-- [params]
-- int_list :categories = "3, 5, 6"
SELECT *
FROM topics
WHERE topics.category_id = ANY (ARRAY [ :categories )
PG::SyntaxError: ERROR: syntax error at or near ")"
LINE 12: WHERE topics.category_id = ANY (ARRAY [ 0,5,6 )
^
(Look at the array)
1 Like
simon
April 2, 2020, 5:22pm
2
It looks like what is happening when you set a default value for an int_list
parameter in this way:
-- int_list :categories = "3, 5, 6"
is that the following gets run:
'"1'.downcase.to_i
That will return 0
.
You can get around the issue by leaving the quotes off the default parameter:
-- int_list :categories = 3, 5, 6
That value still gets interpreted as a string before being split into an array. Maybe the plugin should strip outer quotation marks if they get added to a string input.
6 Likes
AstonJ
(AstonJ)
April 2, 2020, 5:42pm
3
Thanks Simon!
I was sure I had tried that… and looking again now I see that changing default parameters in the body and clicking on save and run
has no effect - you either have to refresh the page after saving or change the values in the box for the field as well (which I totally forgot about).
2 Likes
sam
(Sam Saffron)
April 3, 2020, 1:36am
4
FWIW I find params really fiddly to reason about, I wonder if we should shift this to full UX vs magic comments @riking
5 Likes
riking
(Kane York)
April 3, 2020, 2:00am
5
Yup, totally makes sense to do that, the magic comments were optimizing for minimum # of plugin-sourced DB migrations, which are much less of a problem these days.
5 Likes
system
(system)
Closed
June 4, 2024, 12:33pm
7
This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.