chore(dashboard): fix bugs found in dashboard review

services/dashboard/main.go

@@ -68,6 +68,7 @@ func Run(cleanup cleanupper.Cleanupper) error {
 	jwks := auth.NewJWKSHttpClient(AUTH_JWKS_URL)
 	router.Use(
 		middleware.Logger,
+		middleware.Recoverer,
 		auth.ForwardRequestAuthentication(),
 		auth.Authenticate(jwks),
 		auth.Protect(),
@@ -107,8 +108,10 @@ func Run(cleanup cleanupper.Cleanupper) error {
 	))
 	csrfWrappedHandler := nosurf.New(router)
 	csrfWrappedHandler.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		fmt.Printf("nosurf.Reason(r): %v\n", nosurf.Reason(r))
+		// nosurf bypasses the chi middleware chain, so log here or the rejection is invisible
+		log.Printf("CSRF failure on %s %s: %v\n", r.Method, r.URL.Path, nosurf.Reason(r))
 		layout.WithSnackbarError(w, "CSRF Token was invalid, try reloading the page")
+		w.WriteHeader(http.StatusBadRequest)
 		//nolint
 		w.Write([]byte("A CSRF error occured. Reload the previous page and try again"))
 	}))

services/dashboard/routes/ingress.go

@@ -74,23 +74,24 @@ func (h *TracesPageHandler) listPartial() http.HandlerFunc {
 }
 
 func formatSince(t time.Time) string {
-	d := time.Since(t)
-	if d.Hours() > 24 {
+	d := max(time.Since(t), 0)
+	switch {
+	case d.Hours() > 24:
 		return "More than a day ago"
+	case int(d.Hours()) >= 1:
+		return about(int(d.Hours()), "hour")
+	case int(d.Minutes()) >= 1:
+		return about(int(d.Minutes()), "minute")
+	default:
+		return about(int(d.Seconds()), "second")
 	}
-	if int(d.Hours()) > 1 {
-		return fmt.Sprintf("About %d hours ago", int(d.Hours()))
-	}
-	if int(d.Hours()) > 0 {
-		return fmt.Sprintf("About %d hours ago", int(d.Hours()))
-	}
-	if int(d.Minutes()) > 1 {
-		return fmt.Sprintf("About %d minutes ago", int(d.Minutes()))
-	}
-	if int(d.Minutes()) > 0 {
-		return fmt.Sprintf("About %d minute ago", int(d.Minutes()))
+}
+
+func about(n int, unit string) string {
+	if n == 1 {
+		return fmt.Sprintf("About 1 %s ago", unit)
 	}
-	return fmt.Sprintf("About %d seconds ago", int(d.Seconds()))
+	return fmt.Sprintf("About %d %ss ago", n, unit)
 }
 
 func (h *TracesPageHandler) createViewData(ctx context.Context, traces []api.Trace) ([]views.Trace, error) {
@@ -138,7 +139,7 @@ func (h *TracesPageHandler) createViewData(ctx context.Context, traces []api.Tra
 			viewModels[i].Steps = append(viewModels[i].Steps, step)
 
 			// update last worker with duration
-			if j > 0 {
+			if j > 0 && j < len(trace.WorkerTimes) {
 				viewModels[i].Steps[j-1].Label = trace.WorkerTimes[j].Sub(trace.WorkerTimes[j-1]).String()
 			}
 

services/dashboard/routes/overview.go

@@ -98,7 +98,7 @@
 		w.Header().Set("hx-push-url", views.U("/overview?%s", r.URL.Query().Encode()))
 		w.Header().Set("hx-trigger-after-settle", "newDeviceList")
 		views.WriteRenderFilters(w, sg, true)
-		views.WriteRenderDeviceTable(w, res.Data, getCursor(res.Links.GetNext()))
+		views.WriteRenderDeviceTable(w, res.Data, devicesTableNextPage(res.Links.GetNext(), sgIDStr))
 	}
 }
 
@@ -112,7 +112,7 @@
 		w.Header().Set("hx-push-url", views.U("/overview?%s", r.URL.Query().Encode()))
 		w.Header().Set("hx-trigger-after-settle", "newDeviceList")
 		views.WriteRenderFilters(w, nil, true)
-		views.WriteRenderDeviceTable(w, res.Data, getCursor(res.Links.GetNext()))
+		views.WriteRenderDeviceTable(w, res.Data, devicesTableNextPage(res.Links.GetNext(), ""))
 	}
 }
 
@@ -136,11 +136,7 @@
 			return
 		}
 
-		nextCursor := ""
-		if res.Links.GetNext() != "" {
-			nextCursor = views.U("/overview/devices/table?cursor=%s", getCursor(res.Links.GetNext()))
-		}
-		views.WriteRenderDeviceTableRows(w, res.Data, nextCursor)
+		views.WriteRenderDeviceTableRows(w, res.Data, devicesTableNextPage(res.Links.GetNext(), r.URL.Query().Get("sensor_group")))
 	}
 }
 
@@ -187,12 +183,7 @@
 		}
 		page.Devices = res.Data
 
-		if res.Links.GetNext() != "" {
-			u, err := url.Parse(res.Links.GetNext())
-			if err == nil {
-				page.DevicesNextPage = views.U("/overview/devices/table?cursor=%s", u.Query().Get("cursor"))
-			}
-		}
+		page.DevicesNextPage = devicesTableNextPage(res.Links.GetNext(), sensorGroupIDStr)
 
 		if isHX(r) {
 			page.WriteBody(w)
@@ -271,7 +262,7 @@
 		wsCTX = context.WithValue(wsCTX, api.ContextAccessToken, r.Context().Value(api.ContextAccessToken))
 		go func(ctx context.Context) {
 			defer cancel()
 			defer ws.Close()
 			var nextCursor string
 			for {
 				// Start fetching pages of devices and stream them to the client
@@ -290,12 +281,12 @@
 						log.Printf("cannot open writer for ws: %v\n", err)
 						return
 					}
-					defer writer.Close()
 					frame := fmt.Sprintf(`{"device_id": %d, "device_code": "%s", "coordinates": [%f,%f]}`, dev.Id, dev.Code, dev.GetLatitude(), dev.GetLongitude())
 					if _, err := writer.Write([]byte(frame)); err != nil {
 						log.Printf("Failed to write to websocket: %v\n", err)
 						return
 					}
+					writer.Close()
 				}
 				nextCursor = getCursor(res.Links.GetNext())
 				if nextCursor == "" {
@@ -369,10 +360,12 @@
 		start, err := time.Parse(time.RFC3339, r.URL.Query().Get("start"))
 		if err != nil {
 			web.HTTPError(w, web.NewError(http.StatusBadRequest, "Start parameter is not ISO8601/RFC3339", ""))
+			return
 		}
 		end, err := time.Parse(time.RFC3339, r.URL.Query().Get("end"))
 		if err != nil {
 			web.HTTPError(w, web.NewError(http.StatusBadRequest, "End parameter is not ISO8601/RFC3339", ""))
+			return
 		}
 
 		ws, err := upgrader.Upgrade(w, r, nil)
@@ -388,7 +381,7 @@
 			defer cancel()
 
 			var nextCursor string
 			defer ws.Close()
 			for {
 				// Stop if the context is canceled
 				select {
@@ -450,6 +443,20 @@
 	return u.Query().Get("cursor")
 }
 
+// devicesTableNextPage builds the URL for the next page of the overview device
+// table from the API's next link, preserving the sensor_group filter. It
+// returns an empty string when there is no next page.
+func devicesTableNextPage(next, sensorGroupID string) string {
+	cursor := getCursor(next)
+	if cursor == "" {
+		return ""
+	}
+	if sensorGroupID != "" {
+		return views.U("/overview/devices/table?sensor_group=%s&cursor=%s", sensorGroupID, cursor)
+	}
+	return views.U("/overview/devices/table?cursor=%s", cursor)
+}
+
 func (t *OverviewRoute) resolveDevice(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		deviceID, err := URLParamInt(r, "device_id")

services/dashboard/routes/pipelines.go

@@ -2,9 +2,9 @@ package routes
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"log"
 	"net/http"
 
@@ -109,18 +109,7 @@ func (h *PipelinePageHandler) createPipeline() http.HandlerFunc {
 
 		_, resp, err := h.coreClient.PipelinesApi.CreatePipeline(r.Context()).CreatePipelineRequest(dto).Execute()
 		if err != nil {
-			web.HTTPError(w, fmt.Errorf("could not create pipeline: %w", err))
-			return
-		}
-
-		if resp.StatusCode != http.StatusCreated {
-			responseBody, err := io.ReadAll(resp.Body)
-			if err != nil {
-				log.Printf("in createPipeline, err reading response body: %s\n", err)
-			} else {
-				log.Printf("in createPipeline, err: %s\n", string(responseBody))
-			}
-			layout.SnackbarSomethingWentWrong(w)
+			handleAPIClientError(w, "createPipeline", resp, err)
 			return
 		}
 
@@ -240,34 +229,7 @@ func (h *PipelinePageHandler) updatePipeline(next http.Handler) http.Handler {
 
 		_, resp, err := h.coreClient.PipelinesApi.UpdatePipeline(r.Context(), pipelineId).UpdatePipelineRequest(updateDto).Execute()
 		if err != nil {
-			responseBody, err := io.ReadAll(resp.Body)
-			if err != nil {
-				log.Printf("in createPipeline, err reading response body: %s\n", err)
-			} else {
-				log.Printf("in createPipeline, err: %s\n", string(responseBody))
-			}
-			layout.SnackbarSomethingWentWrong(w)
-			return
-		}
-
-		// TODO: API returns status created instead of found for some reason
-		if resp.StatusCode != http.StatusCreated {
-			if resp.StatusCode == http.StatusInternalServerError {
-				responseBody, err := io.ReadAll(resp.Body)
-				if err != nil {
-					log.Printf("in createPipeline, err reading response body: %s\n", err)
-				} else {
-					log.Printf("in createPipeline, err: %s\n", string(responseBody))
-				}
-				layout.SnackbarSomethingWentWrong(w)
-			} else {
-				var apierror *web.APIError
-				if errors.As(err, &apierror) {
-					layout.WithSnackbarError(w, apierror.Message)
-					w.WriteHeader(apierror.HTTPStatus)
-					return
-				}
-			}
+			handleAPIClientError(w, "updatePipeline", resp, err)
 			return
 		}
 
@@ -491,8 +453,6 @@ func (h *PipelinePageHandler) getWorkersForSteps(r *http.Request, steps []string
 	workers := res.GetData()
 	workers = append(workers, createPlaceholderWorkers(missingWorkers)...)
 
-	fmt.Printf("workers: %v\n", workers)
-	fmt.Printf("steps: %v\n", steps)
 	if len(workers) != len(steps) {
 		return nil, fmt.Errorf("some pipeline workers not found")
 	}
@@ -516,6 +476,29 @@ func createPlaceholderWorkers(steps []string) []api.UserWorker {
 	})
 }
 
+// handleAPIClientError surfaces an API client error as a snackbar. If the API
+// returned a structured error body its message is shown to the user, otherwise
+// a generic snackbar is set. The error is always logged.
+func handleAPIClientError(w http.ResponseWriter, op string, resp *http.Response, err error) {
+	log.Printf("in %s, api client error: %v\n", op, err)
+	var genErr *api.GenericOpenAPIError
+	if errors.As(err, &genErr) && len(genErr.Body()) > 0 {
+		log.Printf("in %s, api response body: %s\n", op, genErr.Body())
+		var apiErr web.APIError
+		if json.Unmarshal(genErr.Body(), &apiErr) == nil && apiErr.Message != "" {
+			layout.WithSnackbarError(w, apiErr.Message)
+			if resp != nil && resp.StatusCode >= http.StatusBadRequest {
+				w.WriteHeader(resp.StatusCode)
+			} else {
+				w.WriteHeader(http.StatusBadRequest)
+			}
+			return
+		}
+	}
+	layout.SnackbarSomethingWentWrong(w)
+	w.WriteHeader(http.StatusInternalServerError)
+}
+
 func placeholderWorker(name string, description string) api.UserWorker {
 	return api.UserWorker{
 		Id:          name,

services/dashboard/routes/workers.go

@@ -56,7 +56,6 @@ func (h *WorkerPageHandler) listWorkers() http.HandlerFunc {
 			page.WorkersNextPage = views.U("/workers/table?cursor=%s", getCursor(res.Links.GetNext()))
 		}
 
-		fmt.Println("Cursor", res.Links.GetNext())
 		if isHX(r) {
 			page.WriteBody(w)
 			return
@@ -67,7 +66,6 @@ func (h *WorkerPageHandler) listWorkers() http.HandlerFunc {
 
 func (h *WorkerPageHandler) workersTable() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		fmt.Println("get table")
 		req := h.workersClient.WorkersApi.ListWorkers(r.Context())
 		if r.URL.Query().Has("cursor") {
 			req = req.Cursor(r.URL.Query().Get("cursor"))
@@ -129,15 +127,12 @@ func (h *WorkerPageHandler) updateWorker() http.HandlerFunc {
 		if name := r.FormValue("name"); name != "" {
 			dto.Name = &name
 		}
-		if desc := r.FormValue("description"); desc != "" {
-			dto.Description = &desc
-		}
-		switch r.FormValue("state") {
-		case "on":
-			dto.SetState("enabled")
-		default:
-			dto.SetState("disabled")
+		// The server keeps the stored description when the field is nil and
+		// clears it when it is empty, so only send it when the form contains it
+		if desc, ok := r.Form["description"]; ok {
+			dto.Description = &desc[0]
 		}
+		dto.SetState(formState(r))
 		if userCode := r.FormValue("userCode"); userCode != "" {
 			dto.UserCode = &userCode
 		}
@@ -152,6 +147,14 @@ func (h *WorkerPageHandler) updateWorker() http.HandlerFunc {
 	}
 }
 
+// formState maps the state checkbox value to the worker state accepted by the API
+func formState(r *http.Request) string {
+	if r.FormValue("state") == "on" {
+		return "enabled"
+	}
+	return "disabled"
+}
+
 func (h *WorkerPageHandler) createWorkerPage() http.HandlerFunc {
 	const defaultUserCode = `
 def process(msg):
@@ -181,6 +184,7 @@ func (h *WorkerPageHandler) createWorker() http.HandlerFunc {
 		dto.SetName(r.FormValue("name"))
 		dto.SetUserCode(r.FormValue("userCode"))
 		dto.SetDescription(r.FormValue("description"))
+		dto.SetState(formState(r))
 
 		_, _, err := h.workersClient.WorkersApi.CreateWorker(r.Context()).CreateUserWorkerRequest(dto).Execute()
 		if err != nil {

services/dashboard/views/pipelineEditPage.qtpl

@@ -84,7 +84,8 @@
             </div>
         </div>
     </template>
-    <div 
+    {% if p.Pipeline != nil %}
+    <div
         class="bg-white border rounded-md lg:col-span-3 xl:col-span-3"
     >
         <header class="flex border-b py-2 px-4 text-sm text-slate-700">
@@ -94,6 +95,7 @@
         <div>
         </div>
     </div>
+    {% endif %}
 </div>
 {% endfunc %}