feat: Automatically get explained response if URL can be provided

[nikromen]

See fedora-copr/copr#3583

[FrostyX]

Did you try this with an actual https://github.com/fedora-copr/logdetective ? I haven't yet, but my suspicion is that this will show the "Getting a response from the server" line forever. The query-url will be defined even after we receive the data from the server, so we will go back here, instead of going to the else branch (the whole cond expression).

[FrostyX]

Still haven't tried with the actual logdetective service, but I think I am right. The send function defines these two handlers:

logdetective-website/frontend/src/app/prompt/core.cljs

Lines 68 to 82 in 13a7827

	:error-handler
	(fn [error]
	(handle-backend-error
	(:error (:response error))
	(:description (:response error))))
	:handler
	(fn [data]
	(if (m/validate InputSchema data)
	(do
	(reset! status "ok")
	(reset! form data))
	(handle-backend-error
	"Invalid data"
	"Got invalid data from the backend. This is likely a bug.")))))

If you put (js/console.log "Hello") or (js/alert "Hello") in them, you can see that the response handled almost immediately (in my case) but the page still shows "Getting a response from the server"

[FrostyX]

@nikromen I think you wanted something like this:

From 8fd9d633e6ae0cf455559c438dc45cc654d0d3cf Mon Sep 17 00:00:00 2001
From: Jakub Kadlcik <[email protected]>
Date: Fri, 14 Feb 2025 11:24:20 +0100
Subject: [PATCH] frontend: fixups for the nikromen PR
 
---
 frontend/src/app/helpers.cljs     |  7 +++++
 frontend/src/app/prompt/core.cljs | 43 +++++++++++++++----------------
 2 files changed, 28 insertions(+), 22 deletions(-)
 
diff --git a/frontend/src/app/helpers.cljs b/frontend/src/app/helpers.cljs
index 8cd6bef..f557383 100644
--- a/frontend/src/app/helpers.cljs
+++ b/frontend/src/app/helpers.cljs
@@ -27,6 +27,13 @@
   (when (local-storage-enabled)
     (.getItem js/localStorage name)))
 
+(defn query-params-get [name]
+  (-> js/window
+      .-location
+      .-search
+      js/URLSearchParams.
+      (.get name)))
+
 (defn local-storage-error []
   {:title "Local storage is blocked by the browser"
    :description
diff --git a/frontend/src/app/prompt/core.cljs b/frontend/src/app/prompt/core.cljs
index 4916dd2..ea44391 100644
--- a/frontend/src/app/prompt/core.cljs
+++ b/frontend/src/app/prompt/core.cljs
@@ -4,6 +4,7 @@
    [malli.core :as m]
    [clojure.string :as str]
    [ajax.core :refer [POST]]
+   [app.helpers :refer [query-params-get]]
    [app.components.jumbotron :refer
     [render-error
      loading-screen]]))
@@ -51,11 +52,8 @@
   [:map {:closed true}
    [:prompt [:maybe :string]]])
 
-(defn send []
-  (let [query-url (-> js/window .-location .-search js/URLSearchParams. (.get "url"))
-        data (if query-url
-               {:prompt query-url}
-               {:prompt @prompt-value})]
+(defn send [url]
+  (let [data {:prompt url}]
     (if (m/validate OutputSchema data)
       (do
         (reset! status "waiting")
@@ -110,7 +108,7 @@
        :on-change on-change-prompt}]
      [:span
       {:class "input-group-addon btn btn-primary"
-       :on-click send}
+       :on-click #(send @prompt-value)}
       [:i {:class "fa-solid fa-play prompt-icon"}]]]]])
 
 (defn certainty-icon [percent]
@@ -200,7 +198,7 @@
   (let [target (.-target event)
         value (.-prompt (.-dataset target))]
     (reset! prompt-value value)
-    (send)))
+    (send value)))
 
 (defn disclaimer []
   [:div {:class "alert alert-warning text-left" :role "alert"}
@@ -274,22 +272,23 @@
                "it. You won't need to open the logs at all."))]])
 
 (defn prompt []
-  (let [query-url (-> js/window .-location .-search js/URLSearchParams. (.get "url"))]
-    (if query-url
+  (let [query-url (query-params-get "url")]
+    (cond
+      (= @status "error")
+      (render-error @error-title @error-description)
+
+      ;; TODO If we are already in a two-column-layout, we should print only
+      ;; a small loading icon somewhere, not a jumbotron
+      (= @status "waiting")
+      (loading-screen "Getting a response from the server")
+
+      (and query-url (not= @status "ok"))
       (do
-        (send)
+        (send query-url)
         (loading-screen "Getting a response from the server"))
-      (cond
-        (= @status "error")
-        (render-error @error-title @error-description)
 
-        ;; TODO If we are already in a two-column-layout, we should print only
-        ;; a small loading icon somewhere, not a jumbotron
-        (= @status "waiting")
-        (loading-screen "Getting a response from the server")
+      @form
+      (two-column-layout)
 
-        @form
-        (two-column-layout)
-
-        :else
-        (prompt-only)))))
+      :else
+      (prompt-only))))
-- 
2.48.1

It looks like a lot of changes but once you squash it with your previous commit, it should be shorter.

[FrostyX]

I just realized I didn't test if it works for URLs with & and other special characters.

[FrostyX]

I just realized I didn't test if it works for URLs with & and other special characters.

It doesn't work. When you open

http://127.0.0.1:5020/explain?url=http://foo.bar/baz?a1=v1&a2=v2

The log_url variable in frontend_explain_post is http://foo.bar/baz?a1=v1. How do you want to solve this @nikromen? Should we base64 the URLs?

Or maybe just HTTP encoding?

[FrostyX]

One more patch btw

From 97628f30c295810d982613ef370c89b168879712 Mon Sep 17 00:00:00 2001
From: Jakub Kadlcik <[email protected]>
Date: Fri, 21 Feb 2025 19:30:18 +0100
Subject: [PATCH] don't use decodeURIComponent

---
 frontend/src/app/helpers.cljs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/frontend/src/app/helpers.cljs b/frontend/src/app/helpers.cljs
index 9c06376..f557383 100644
--- a/frontend/src/app/helpers.cljs
+++ b/frontend/src/app/helpers.cljs
@@ -32,8 +32,7 @@
       .-location
       .-search
       js/URLSearchParams.
-      (.get name)
-      js/decodeURIComponent))
+      (.get name)))

 (defn local-storage-error []
   {:title "Local storage is blocked by the browser"
--
2.48.1

The js/decodeURIComponent was used in your original code so used it as is. But it causes problems because if the query parameter doesn't exist, it converts nil to "null" string.

[nikromen]

The log_url variable in frontend_explain_post is http://foo.bar/baz?a1=v1. How do you want to solve this @nikromen? Should we base64 the URLs?

thank you for looking into this. I already tested that and resolved the issue in https://github.com/fedora-copr/copr/pull/3608/files#diff-e03ca2d9d634f331b81cbff5b16910a6e1a34e742d899aeda14b884c53f7e40cR145 - I am using urlencode. I am sorry for not commenting on updates to inform you about this.

The js/decodeURIComponent was used in your original code so used it as is. But it causes problems because if the query parameter doesn't exist, it converts nil to "null" string.

I added that to your original patch to decode the url. With this null problem, I guess I could do something like:

let some_variable = ...
URLSearchParams.
.get name;

if some_variable is not nil:
    js/decodeURIComponent

return variable

[FrostyX]

I added that to your original patch to decode the url. With this null problem, I guess I could do something like:

IMHO it is not needed. Even without the decodeURIComponent, using the patch I am proposing, I can go to
http://127.0.0.1:5020/explain?url=http://foo.bar/baz%3Fa1%3Dv1%26a2%3Dv2
and both frontend and backend have this URL available as http://foo.bar/baz?a1=v1&a2=v2. So the decoding somehow happens anyway.