Slack tweaks for orgs with large numbers of channels/users (#13105)

* Slack tweaks for orgs with large numbers of channels/users

* Test fix 

src/metabase/integrations/slack.clj

@@ -54,8 +54,10 @@
             request (merge-with merge
                       {:query-params   {:token token}
                        :as             :stream
-                       :conn-timeout   1000
-                       :socket-timeout 1000}
+                       ;; use a relatively long connection timeout (10 seconds) in cases where we're fetching big
+                       ;; amounts of data -- see #11735
+                       :conn-timeout   10000
+                       :socket-timeout 10000}
                       request)]
         (try
           (handle-response (request-fn url request))
@@ -78,15 +80,19 @@
   (not-empty (get-in response [:response_metadata :next_cursor])))
 
 (def ^:private max-list-results
-  "Absolute maximum number of results to fetch from Slack API list endpoints. To prevent unbounded pagination of results."
-  1000)
+  "Absolute maximum number of results to fetch from Slack API list endpoints. To prevent unbounded pagination of
+  results. Don't set this too low -- some orgs have many thousands of channels (see #12978)"
+  10000)
 
 (defn- paged-list-request
   "Make a GET request to a Slack API list `endpoint`, returning a sequence of objects returned by the top level
   `results-key` in the response. If additional pages of results exist, fetches those lazily, up to a total of
   `max-list-results`."
   [endpoint results-key params]
-  (let [response (m/mapply GET endpoint params)]
+  ;; use default limit (page size) of 1000 instead of 100 so we don't end up making a hundred API requests for orgs
+  ;; with a huge number of channels or users.
+  (let [default-params {:limit 1000}
+        response       (m/mapply GET endpoint (merge default-params params))]
     (when (seq response)
       (take
        max-list-results
@@ -109,7 +115,7 @@
   (some (fn [channel]
           (when (= (:name channel) channel-name)
             channel))
-        (conversations-list :exclude_archived false)))
+        (conversations-list)))
 
 (s/defn valid-token?
   "Check whether a Slack token is valid by checking whether we can call `conversations.list` with it."
@@ -124,13 +130,17 @@
 (defn users-list
   "Calls Slack API `users.list` endpoint and returns the list of available users."
   [& {:as query-parameters}]
-  (paged-list-request "users.list" :members query-parameters))
+  (->> (paged-list-request "users.list" :members query-parameters)
+       ;; filter out deleted users and bots. At the time of this writing there's no way to do this in the Slack API
+       ;; itself so we need to do it after the fact.
+       (filter (complement :deleted))
+       (filter (complement :is_bot))))
 
 (defn- files-channel* []
   (or (channel-with-name files-channel-name)
       (let [message (str (tru "Slack channel named `metabase_files` is missing!")
                          " "
-                         (tru "Please create the channel in order to complete the Slack integration.")
+                         (tru "Please create or unarchive the channel in order to complete the Slack integration.")
                          " "
                          (tru "The channel is used for storing graphs that are included in Pulses and MetaBot answers."))]
         (log/error (u/format-color 'red message))
@@ -151,7 +161,7 @@
 (def ^:private NonEmptyByteArray
   (s/constrained
    (Class/forName "[B")
-   #(pos? (count %))
+   not-empty
    "Non-empty byte array"))
 
 (s/defn upload-file!

test/metabase/integrations/slack_test.clj

@@ -49,7 +49,7 @@
     (testing "should return nil if no Slack token has been configured"
       (tu/with-temporary-setting-values [slack-token nil]
         (is (= nil
-               (thunk)))))))
+               (not-empty (thunk))))))))
 
 (defn- test-invalid-auth-token
   "Test that a Slack API endpoint function throws an Exception if an invalid Slack API token is set."