-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FIX: Discourse automation reports with parameters #363
Conversation
@@ -35,26 +35,10 @@ def self.generate_post(query_id, query_params, opts = {}) | |||
build_report_post(query, table, attach_csv: opts[:attach_csv], result:) | |||
end | |||
|
|||
private |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scoping keywords put in the class body only affect instance methods. All methods in this class are class methods and therefore already public.
params_hash = {} | ||
|
||
if !params.blank? | ||
param_key, param_value = [], [] | ||
params.flatten.each.with_index do |data, i| | ||
if i % 2 == 0 | ||
param_key << data | ||
else | ||
param_value << data | ||
end | ||
end | ||
|
||
params_hash = Hash[param_key.zip(param_value)] | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chunk is the same as Array#to_h
.
451153d
to
27a17da
Compare
What is the problem?
When setting up an automation to create a DM or a post with a report on a recurring basis, and using a Data Explorer query that has parameters, we encounter an error.
Why is this happening?
The
.params_to_hash
currently expects an array of arrays for the parameters, e.g.:but in reality they seem to be arrays of hashes:
How come we only found out now?
We do have tests for the report generator, and we do pass query parameters to it, but 1) we hard-code the parameters as an array of arrays and 2) the parameters aren't in the query used as a fixture.
How does this fix it?
This change makes
ReportGenerator.params_to_hash
work with arrays of hashes. It also preserves the ability to work with nested arrays in case this is used somewhere else.