Merge pull request #189 from lukaszkorecki/tweak-login-prompt-copy

Make it more clear how to use /login command and provide clear instructions

Author: ericdallo

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Updated instructions for `/login` command and invalid input handling.
+
 ## 0.75.4
 
 - Fix 0.75.3 regression on custom openai-chat providers.

src/eca/features/login.clj

@@ -14,16 +14,18 @@
 ;; No provider selected
 (defmethod login-step [nil :login/start] [{:keys [db* chat-id input send-msg!] :as ctx}]
   (let [provider (string/trim input)
-        providers (->> @db* :auth keys sort)]
+        providers (->> @db* :auth keys sort)
+        provider-list-str (reduce (fn [s p] (str s "- " p "\n")) "" providers)]
     (if (get-in @db* [:auth provider])
-      (do (swap! db* assoc-in [:chats chat-id :login-provider] provider)
-          (swap! db* assoc-in [:auth provider] {:step :login/start})
-          (login-step (assoc ctx :provider provider)))
-      (send-msg! (reduce
-                  (fn [s provider]
-                    (str s "- " provider "\n"))
-                  "Choose a provider:\n"
-                  providers)))))
+      (do
+        (swap! db* assoc-in [:chats chat-id :login-provider] provider)
+        (swap! db* assoc-in [:auth provider] {:step :login/start})
+        (login-step (assoc ctx :provider provider)))
+      (if (string/blank? provider)
+        (send-msg! (str "Please type the name of your chosen provider and press Enter:\n" provider-list-str))
+        (send-msg! (str "Sorry, \"" provider "\" is not a valid provider.\n"
+                        "Please type the name of your chosen provider and press Enter:\n"
+                        provider-list-str))))))
 
 (defn handle-step [{:keys [message chat-id]} db* messenger config metrics]
   (let [provider (get-in @db* [:chats chat-id :login-provider])

test/eca/features/login_test.clj

@@ -0,0 +1,64 @@
+(ns eca.features.login-test
+  (:require [clojure.test :refer [deftest is testing]]
+            [eca.features.login :as login]
+            [matcher-combinators.test :refer [match?]]))
+
+(deftest test-login-step
+  (let [msg-log (atom [])
+        send-msg! (fn [msg] (swap! msg-log conj msg))
+
+        db* (atom {:auth {"google" {}
+                          "github-copilot" {}}
+                   :chats {1 {}}})]
+
+    (testing "user just typed /login for the first time"
+      (login/login-step {:provider nil
+                         :step :login/start
+                         :chat-id 0
+                         :input ""
+                         :db* db*
+                         :send-msg! send-msg!})
+
+      (testing "should ask to choose a provider"
+        (is (= "Please type the name of your chosen provider and press Enter:\n- github-copilot\n- google\n"
+               (last @msg-log)))))
+
+    (testing "user is confused"
+      (login/login-step {:provider nil
+                         :step :login/start
+                         :chat-id 0
+                         :input "/login github"
+                         :db* db*
+                         :send-msg! send-msg!})
+
+      (testing "should ask to choose a provider and provide instructions"
+        (is (= "Sorry, \"/login github\" is not a valid provider.\nPlease type the name of your chosen provider and press Enter:\n- github-copilot\n- google\n"
+               (last @msg-log)))
+
+        (testing "state didn't change"
+          (is (= {:auth {"github-copilot" {}
+                         "google" {}}
+                  :chats {1 {}}}
+                 @db*)))))
+
+    (testing "valid input is provided"
+
+      (login/login-step {:provider nil
+                         :step :login/start
+                         :chat-id 0
+                         :input "github-copilot"
+                         :db* db*
+                         :send-msg! send-msg!})
+
+      (testing "should proceed to the next step"
+        (is (re-find #"(?m)Open your browser at `https://github.com/login/device` and authenticate using the code: `.+`\nThen type anything in the chat and send it to continue the authentication."
+                     (last @msg-log)))
+
+        (testing "state is update to reflect in-progress login"
+          (is (match? {:auth {"github-copilot" {;; skipping :device-code attr
+                                                :step :login/waiting-user-confirmation}
+                              "google" {}}
+                       :chats {0 {:login-provider "github-copilot"}
+                               1 {}}}
+
+                      @db*)))))))