Discource-C
/
discourse-data-explorer
mirror of https://github.com/discourse/discourse-data-explorer.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Optionally linkify URL columns server-side (#330) 

Followup da1c99ec2d

We already had the functionality to convert results like
this:

```
|blah_url|
|--------|
|3,https://test.com|
```

To a link clientside, so in the UI it looks like this:

```
<a href="https://test.com">3</a>
```

With the addition of the recurring report to post automation,
and the existing report to PM automation, we also need to be
able to do this server-side, so reports don't come out malformed
if they have these type of count + URL columns.
This commit is contained in:
Martin Brennan 2024-10-14 16:33:13 +10:00 committed by GitHub
parent b43d82d5d6
commit 8d19a33250
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 73 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
assets/javascripts/discourse/components/query-row-content.js
View File

@ -8,7 +8,7 @@ import TextViewComponent from "./result-types/text";
export default class QueryRowContent extends Component {
@cached
get results() {
return this.args.columnComponents.map((t, idx) => {
return this.args.columnComponents.map((componentDefinition, idx) => {
const value = this.args.row[idx],
id = parseInt(value, 10);
@ -23,19 +23,20 @@ export default class QueryRowContent extends Component {
component: TextViewComponent,
textValue: "NULL",
};
} else if (t.name === "text") {
} else if (componentDefinition.name === "text") {
return {
component: TextViewComponent,
textValue: escapeExpression(this.args.row[idx].toString()),
};
}
const lookupFunc = this.args[`lookup${capitalize(t.name)}`];
const lookupFunc =
this.args[`lookup${capitalize(componentDefinition.name)}`];
if (lookupFunc) {
ctx[t.name] = lookupFunc.call(this.args, id);
ctx[componentDefinition.name] = lookupFunc.call(this.args, id);
}
if (t.name === "url") {
if (componentDefinition.name === "url") {
let [url, name] = guessUrl(value);
ctx["href"] = url;
ctx["target"] = name;
@ -43,7 +44,7 @@ export default class QueryRowContent extends Component {
try {
return {
component: t.component || TextViewComponent,
component: componentDefinition.component || TextViewComponent,
ctx,
};
} catch (e) {
@ -53,10 +54,10 @@ export default class QueryRowContent extends Component {
}
}
function guessUrl(t) {
let [dest, name] = [t, t];
function guessUrl(columnValue) {
let [dest, name] = [columnValue, columnValue];
const split = t.split(/,(.+)/);
const split = columnValue.split(/,(.+)/);
if (split.length > 1) {
name = split[0];

6
lib/discourse_data_explorer/report_generator.rb
View File

@ -13,7 +13,8 @@ module ::DiscourseDataExplorer
query.update!(last_run_at: Time.now)
return [] if opts[:skip_empty] && result[:pg_result].values.empty?
table = ResultToMarkdown.convert(result[:pg_result])
table =
ResultToMarkdown.convert(result[:pg_result], render_url_columns: opts[:render_url_columns])
build_report_pms(query, table, recipients, attach_csv: opts[:attach_csv], result:)
end
@ -28,7 +29,8 @@ module ::DiscourseDataExplorer
query.update!(last_run_at: Time.now)
return {} if opts[:skip_empty] && result[:pg_result].values.empty?
table = ResultToMarkdown.convert(result[:pg_result])
table =
ResultToMarkdown.convert(result[:pg_result], render_url_columns: opts[:render_url_columns])
build_report_post(query, table, attach_csv: opts[:attach_csv], result:)
end

14
lib/discourse_data_explorer/result_to_markdown.rb
View File

@ -4,7 +4,7 @@ include HasSanitizableFields
module ::DiscourseDataExplorer
class ResultToMarkdown
def self.convert(pg_result)
def self.convert(pg_result, render_url_columns = false)
relations, colrender = DataExplorer.add_extra_data(pg_result)
result_data = []
@ -17,7 +17,8 @@ module ::DiscourseDataExplorer
row.each_with_index do |col, col_index|
col_name = pg_result.fields[col_index]
related = relations.dig(colrender[col_index].to_sym) unless colrender[col_index].nil?
col_render = colrender[col_index]
related = relations.dig(col_render.to_sym) if col_render.present?
if related.is_a?(ActiveModel::ArraySerializer)
related_row = related.object.find_by(id: col)
@ -32,6 +33,9 @@ module ::DiscourseDataExplorer
else
row_data[col_index] = "#{related_row[column]} (#{col})"
end
elsif col_render == "url" && render_url_columns
url, text = guess_url(col)
row_data[col_index] = "[#{text}](#{url})"
else
row_data[col_index] = col
end
@ -45,5 +49,11 @@ module ::DiscourseDataExplorer
"|#{table_headers}\n|#{table_body}\n#{result_data.join}"
end
def self.guess_url(column_value)
text, url = column_value.split(/,(.+)/)
[url || column_value, text || column_value]
end
end
end

9
plugin.rb
View File

@ -105,7 +105,12 @@ after_initialize do
end
DiscourseDataExplorer::ReportGenerator
.generate(query_id, query_params, recipients, { skip_empty:, attach_csv: })
.generate(
query_id,
query_params,
recipients,
{ skip_empty:, attach_csv:, render_url_columns: true },
)
.each do |pm|
begin
utils.send_pm(pm, automation_id: automation.id, prefers_encrypt: false)
@ -153,7 +158,7 @@ after_initialize do
DiscourseDataExplorer::ReportGenerator.generate_post(
query_id,
query_params,
{ skip_empty:, attach_csv: },
{ skip_empty:, attach_csv:, render_url_columns: true },
)
next if post.empty?

22
spec/automation/recurring_data_explorer_result_pm_spec.rb
View File

@ -18,7 +18,7 @@ describe "RecurringDataExplorerResultPM" do
end
fab!(:query) { Fabricate(:query, sql: "SELECT 'testabcd' AS data") }
fab!(:query_group) { Fabricate(:query_group, query: query, group: group) }
fab!(:query_group) { Fabricate(:query_group, query: query, group: another_group) }
fab!(:query_group_2) { Fabricate(:query_group, query: query, group: another_group) }
let!(:recipients) do
[user.username, not_allowed_user.username, another_user.username, another_group.name]
@ -105,5 +105,25 @@ describe "RecurringDataExplorerResultPM" do
expect { automation.trigger! }.to_not change { Post.count }
end
it "works with a query that returns URLs in a number,url format" do
freeze_time
query.update!(sql: "SELECT 3 || ',https://test.com' AS some_url")
automation.update(last_updated_by_id: admin.id)
automation.trigger!
expect(Post.last.raw).to eq(
I18n.t(
"data_explorer.report_generator.private_message.body",
recipient_name: another_group.name,
query_name: query.name,
table: "| some_url |\n| :----- |\n| [3](https://test.com) |\n",
base_url: Discourse.base_url,
query_id: query.id,
created_at: Time.zone.now.strftime("%Y-%m-%d at %H:%M:%S"),
timezone: Time.zone.name,
).chomp,
)
end
end
end

21
spec/automation/recurring_data_explorer_result_topic_spec.rb
View File

@ -12,14 +12,12 @@ describe "RecurringDataExplorerResultTopic" do
fab!(:topic)
fab!(:group) { Fabricate(:group, users: [user, another_user]) }
fab!(:another_group) { Fabricate(:group, users: [group_user]) }
fab!(:automation) do
Fabricate(:automation, script: "recurring_data_explorer_result_topic", trigger: "recurring")
end
fab!(:query) { Fabricate(:query, sql: "SELECT 'testabcd' AS data") }
fab!(:query_group) { Fabricate(:query_group, query: query, group: group) }
fab!(:query_group) { Fabricate(:query_group, query: query, group: another_group) }
before do
SiteSetting.data_explorer_enabled = true
@ -88,5 +86,24 @@ describe "RecurringDataExplorerResultTopic" do
expect { automation.trigger! }.to_not change { Post.count }
end
it "works with a query that returns URLs in a number,url format" do
freeze_time
query.update!(sql: "SELECT 3 || ',https://test.com' AS some_url")
automation.update(last_updated_by_id: admin.id)
automation.trigger!
expect(topic.posts.last.raw).to eq(
I18n.t(
"data_explorer.report_generator.post.body",
query_name: query.name,
table: "| some_url |\n| :----- |\n| [3](https://test.com) |\n",
base_url: Discourse.base_url,
query_id: query.id,
created_at: Time.zone.now.strftime("%Y-%m-%d at %H:%M:%S"),
timezone: Time.zone.name,
).chomp,
)
end
end
end