From 142ccbbb2cb80b33999e7ee626b79fc3d4cc4643 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 9 Dec 2024 16:41:10 +0100 Subject: [PATCH 1/4] UX: clarify the need for authorized extension When checking `attach_csv` in the `Schedule a PM with Data Explorer results` automation script, `csv` has to be added to the list of authorized extensions in the site settings. --- config/locales/client.en.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index c445d906..08da4e55 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -118,6 +118,7 @@ en: label: Skip sending PM if there are no results attach_csv: label: Attach the CSV file to the PM + description: Requires to add `csv` to the list of authorized extensions in the site settings recurring_data_explorer_result_topic: fields: topic_id: From dd61664292ce025dd2c1005d18ca26a27d6d53fd Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 9 Dec 2024 17:14:33 +0100 Subject: [PATCH 2/4] use a validator --- config/locales/client.en.yml | 1 - config/locales/server.en.yml | 1 + plugin.rb | 10 +++++++++- .../recurring_data_explorer_result_pm_spec.rb | 11 +++++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 08da4e55..c445d906 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -118,7 +118,6 @@ en: label: Skip sending PM if there are no results attach_csv: label: Attach the CSV file to the PM - description: Requires to add `csv` to the list of authorized extensions in the site settings recurring_data_explorer_result_topic: fields: topic_id: diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 7c92a5e9..083ef16a 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -6,6 +6,7 @@ en: recurring_data_explorer_result_pm: title: Schedule a PM with Data Explorer results description: Get scheduled reports sent to your messages + no_csv_allowed: "When using `attach_csv` field, `csv` must be added to the list of authorized extensions in the site settings." recurring_data_explorer_result_topic: title: Schedule a post in a topic with Data Explorer results description: Get scheduled reports posted to a specific topic diff --git a/plugin.rb b/plugin.rb index 7f1ec0ad..bb71d143 100644 --- a/plugin.rb +++ b/plugin.rb @@ -82,7 +82,15 @@ module ::DiscourseDataExplorer field :query_id, component: :choices, required: true, extra: { content: queries } field :query_params, component: :"key-value", accepts_placeholders: true field :skip_empty, component: :boolean - field :attach_csv, component: :boolean + field :attach_csv, + component: :boolean, + validator: ->(attach_csv) do + if attach_csv && !SiteSetting.authorized_extensions.split("|").include?("csv") + I18n.t( + "discourse_automation.scriptables.recurring_data_explorer_result_pm.no_csv_allowed", + ) + end + end version 1 triggerables [:recurring] diff --git a/spec/automation/recurring_data_explorer_result_pm_spec.rb b/spec/automation/recurring_data_explorer_result_pm_spec.rb index 39aefff4..f27bd797 100644 --- a/spec/automation/recurring_data_explorer_result_pm_spec.rb +++ b/spec/automation/recurring_data_explorer_result_pm_spec.rb @@ -126,4 +126,15 @@ ) end end + + context "when using attach_csv" do + it "requires csv to be in authorized extensions" do + SiteSetting.authorized_extensions = "pdf|txt" + + expect { automation.upsert_field!("attach_csv", "boolean", { value: true }) }.to raise_error( + ActiveRecord::RecordInvalid, + /#{I18n.t("discourse_automation.scriptables.recurring_data_explorer_result_pm.no_csv_allowed")}/, + ) + end + end end From 7857dad028e6864b6e665e09da5ac8454b641dc4 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 9 Dec 2024 17:15:43 +0100 Subject: [PATCH 3/4] better test --- spec/automation/recurring_data_explorer_result_pm_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/automation/recurring_data_explorer_result_pm_spec.rb b/spec/automation/recurring_data_explorer_result_pm_spec.rb index f27bd797..7861e1c0 100644 --- a/spec/automation/recurring_data_explorer_result_pm_spec.rb +++ b/spec/automation/recurring_data_explorer_result_pm_spec.rb @@ -135,6 +135,12 @@ ActiveRecord::RecordInvalid, /#{I18n.t("discourse_automation.scriptables.recurring_data_explorer_result_pm.no_csv_allowed")}/, ) + + SiteSetting.authorized_extensions = "pdf|txt|csv" + + expect { + automation.upsert_field!("attach_csv", "boolean", { value: true }) + }.to_not raise_error end end end From f55d7ca74f6714a09cc71d431404e07bc99ad598 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 9 Dec 2024 22:06:22 +0100 Subject: [PATCH 4/4] handle * --- plugin.rb | 5 ++++- spec/automation/recurring_data_explorer_result_pm_spec.rb | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/plugin.rb b/plugin.rb index bb71d143..df790503 100644 --- a/plugin.rb +++ b/plugin.rb @@ -85,7 +85,10 @@ module ::DiscourseDataExplorer field :attach_csv, component: :boolean, validator: ->(attach_csv) do - if attach_csv && !SiteSetting.authorized_extensions.split("|").include?("csv") + return if !attach_csv + + extensions = SiteSetting.authorized_extensions.split("|") + if (extensions & %w[csv *]).empty? I18n.t( "discourse_automation.scriptables.recurring_data_explorer_result_pm.no_csv_allowed", ) diff --git a/spec/automation/recurring_data_explorer_result_pm_spec.rb b/spec/automation/recurring_data_explorer_result_pm_spec.rb index 7861e1c0..775d03eb 100644 --- a/spec/automation/recurring_data_explorer_result_pm_spec.rb +++ b/spec/automation/recurring_data_explorer_result_pm_spec.rb @@ -141,6 +141,12 @@ expect { automation.upsert_field!("attach_csv", "boolean", { value: true }) }.to_not raise_error + + SiteSetting.authorized_extensions = "*" + + expect { + automation.upsert_field!("attach_csv", "boolean", { value: true }) + }.to_not raise_error end end end