Merge pull request #4666 from metabase/missing-metabase-files-slack-error

Throw a helpful error if #metabase_files channel is missing when sending message to Slack

src/metabase/integrations/slack.clj

@@ -37,14 +37,6 @@
 (def ^{:arglists '([endpoint & {:as params}]), :style/indent 1} GET  "Make a GET request to the Slack API."  (partial do-slack-request http/get  :query-params))
 (def ^{:arglists '([endpoint & {:as params}]), :style/indent 1} POST "Make a POST request to the Slack API." (partial do-slack-request http/post :form-params))
 
-(def ^:private ^{:arglists '([channel-id & {:as args}])} create-channel!
-  "Calls Slack api `channels.create` for CHANNEL."
-  (partial POST :channels.create, :name))
-
-(def ^:private ^{:arglists '([channel-id & {:as args}])} archive-channel!
-  "Calls Slack api `channels.archive` for CHANNEL."
-  (partial POST :channels.archive, :channel))
-
 (def ^{:arglists '([& {:as args}])} channels-list
   "Calls Slack api `channels.list` function and returns the list of available channels."
   (comp :channels (partial GET :channels.list, :exclude_archived 1)))
@@ -53,29 +45,26 @@
   "Calls Slack api `users.list` function and returns the list of available users."
   (comp :members (partial GET :users.list)))
 
-(defn- create-files-channel!
-  "Convenience function for creating our Metabase files channel to store file uploads."
-  []
-  (when-let [{files-channel :channel, :as response} (create-channel! files-channel-name)]
-    (when-not files-channel
-      (log/error (u/pprint-to-str 'red response))
-      (throw (ex-info "Error creating Slack channel for Metabase file uploads" response)))
-    ;; Right after creating our files channel, archive it. This is because we don't need users to see it.
-    (u/prog1 files-channel
-      (archive-channel! (:id <>)))))
-
-(defn- files-channel
-  "Return the `metabase_files` channel (as a map) if it exists."
+(def ^:private ^:const ^String channel-missing-msg
+  (str "Slack channel named `metabase_files` is missing! Please create the channel in order to complete "
+       "the Slack integration. The channel is used for storing graphs that are included in pulses and "
+       "MetaBot answers."))
+
+(defn- maybe-get-files-channel
+  "Return the `metabase_files channel (as a map) if it exists."
   []
   (some (fn [channel] (when (= (:name channel) files-channel-name)
                         channel))
         (channels-list :exclude_archived 0)))
 
-(defn get-or-create-files-channel!
-  "Calls Slack api `channels.info` and `channels.create` function as needed to ensure that a #metabase_files channel exists."
+(defn files-channel
+  "Calls Slack api `channels.info` to check whether a channel named #metabase_files exists. If it doesn't,
+   throws an error that advices an admin to create it."
   []
-  (or (files-channel)
-      (create-files-channel!)))
+  (or (maybe-get-files-channel)
+      (do (log/error (u/format-color 'red channel-missing-msg))
+          (throw (ex-info channel-missing-msg {:status-code 400})))))
+
 
 (defn upload-file!
   "Calls Slack api `files.upload` function and returns the body of the uploaded file."

src/metabase/pulse.clj

@@ -46,7 +46,7 @@
 (defn create-and-upload-slack-attachments!
   "Create an attachment in Slack for a given Card by rendering its result into an image and uploading it."
   [card-results]
-  (when-let [{channel-id :id} (slack/get-or-create-files-channel!)]
+  (let [{channel-id :id} (slack/files-channel)]
     (doall (for [{{card-id :id, card-name :name, :as card} :card, result :result} card-results]
              (let [image-byte-array (render/render-pulse-card-to-png card result)
                    slack-file-url   (slack/upload-file! image-byte-array "image.png" channel-id)]