cleanup redirect mess for denied removal

updates #205

web/handlers/admin/denied_keys.go

@@ -3,7 +3,6 @@
 package admin
 
 import (
-	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -27,14 +26,18 @@ type deniedKeysHandler struct {
 const redirectToDeniedKeys = "/admin/denied"
 
 func (h deniedKeysHandler) add(w http.ResponseWriter, req *http.Request) {
+	// always redirect
+	defer http.Redirect(w, req, redirectToDeniedKeys, http.StatusSeeOther)
+
 	if req.Method != "POST" {
 		err := weberrors.ErrBadRequest{Where: "HTTP Method", Details: fmt.Errorf("expected POST not %s", req.Method)}
-		h.r.Error(w, req, http.StatusBadRequest, err)
+		h.flashes.AddError(w, req, err)
 		return
 	}
+
 	if err := req.ParseForm(); err != nil {
 		err = weberrors.ErrBadRequest{Where: "Form data", Details: err}
-		h.r.Error(w, req, http.StatusBadRequest, err)
+		h.flashes.AddError(w, req, err)
 		return
 	}
 
@@ -43,7 +46,6 @@ func (h deniedKeysHandler) add(w http.ResponseWriter, req *http.Request) {
 	if err != nil {
 		err = weberrors.ErrBadRequest{Where: "Public Key", Details: err}
 		h.flashes.AddError(w, req, err)
-		http.Redirect(w, req, redirectToDeniedKeys, http.StatusTemporaryRedirect)
 		return
 	}
 
@@ -56,8 +58,6 @@ func (h deniedKeysHandler) add(w http.ResponseWriter, req *http.Request) {
 	} else {
 		h.flashes.AddMessage(w, req, "AdminDeniedKeysAdded")
 	}
-
-	http.Redirect(w, req, redirectToDeniedKeys, http.StatusTemporaryRedirect)
 }
 
 func (h deniedKeysHandler) overview(rw http.ResponseWriter, req *http.Request) (interface{}, error) {
@@ -106,32 +106,27 @@ func (h deniedKeysHandler) removeConfirm(rw http.ResponseWriter, req *http.Reque
 }
 
 func (h deniedKeysHandler) remove(rw http.ResponseWriter, req *http.Request) {
+	// always redirect
+	defer http.Redirect(rw, req, redirectToDeniedKeys, http.StatusSeeOther)
+
 	err := req.ParseForm()
 	if err != nil {
 		err = weberrors.ErrBadRequest{Where: "Form data", Details: err}
-		// TODO "flash" errors
+		h.flashes.AddError(rw, req, err)
-		http.Redirect(rw, req, redirectToDeniedKeys, http.StatusTemporaryRedirect)
 		return
 	}
 
 	id, err := strconv.ParseInt(req.FormValue("id"), 10, 64)
 	if err != nil {
 		err = weberrors.ErrBadRequest{Where: "ID", Details: err}
-		// TODO "flash" errors
+		h.flashes.AddError(rw, req, err)
-		http.Redirect(rw, req, redirectToDeniedKeys, http.StatusTemporaryRedirect)
 		return
 	}
 
-	status := http.StatusFound
 	err = h.db.RemoveID(req.Context(), id)
 	if err != nil {
-		if !errors.Is(err, roomdb.ErrNotFound) {
+		h.flashes.AddError(rw, req, err)
-			// TODO "flash" errors
+	} else {
-			h.r.Error(rw, req, http.StatusInternalServerError, err)
+		h.flashes.AddMessage(rw, req, "AdminDeniedKeysRemoved")
-			return
-		}
-		status = http.StatusNotFound
 	}
-
-	http.Redirect(rw, req, redirectToDeniedKeys, status)
 }

web/handlers/admin/denied_keys_test.go

@@ -72,7 +72,11 @@ func TestDeniedKeysDisabledInterface(t *testing.T) {
 		"pub_key": []string{newKey},
 	}
 	rec := ts.Client.PostForm(addURL, addVals)
-	a.Equal(http.StatusTemporaryRedirect, rec.Code)
+	a.Equal(http.StatusSeeOther, rec.Code)
+
+	overview := ts.URLTo(router.AdminDeniedKeysOverview)
+	a.Equal(overview.Path, rec.Header().Get("Location"))
+	webassert.HasFlashMessages(t, ts.Client, overview, "AdminDeniedKeysAdded")
 
 	a.Equal(1, ts.DeniedKeysDB.AddCallCount())
 	_, addedKey, addedComment := ts.DeniedKeysDB.AddArgsForCall(0)
@@ -146,7 +150,11 @@ func TestDeniedKeysAdd(t *testing.T) {
 		"pub_key": []string{newKey},
 	}
 	rec := ts.Client.PostForm(addURL, addVals)
-	a.Equal(http.StatusTemporaryRedirect, rec.Code)
+	a.Equal(http.StatusSeeOther, rec.Code)
+
+	overview := ts.URLTo(router.AdminDeniedKeysOverview)
+	a.Equal(overview.Path, rec.Header().Get("Location"))
+	webassert.HasFlashMessages(t, ts.Client, overview, "AdminDeniedKeysAdded")
 
 	a.Equal(1, ts.DeniedKeysDB.AddCallCount())
 	_, addedKey, addedComment := ts.DeniedKeysDB.AddArgsForCall(0)
@@ -166,7 +174,7 @@ func TestDeniedKeysDontAddInvalid(t *testing.T) {
 		"pub_key": []string{newKey},
 	}
 	rec := ts.Client.PostForm(addURL, addVals)
-	a.Equal(http.StatusTemporaryRedirect, rec.Code)
+	a.Equal(http.StatusSeeOther, rec.Code)
 
 	a.Equal(0, ts.DeniedKeysDB.AddCallCount(), "did not call add")
 
@@ -269,7 +277,11 @@ func TestDeniedKeysRemove(t *testing.T) {
 
 	addVals := url.Values{"id": []string{"666"}}
 	rec := ts.Client.PostForm(urlRemove, addVals)
-	a.Equal(http.StatusFound, rec.Code)
+	a.Equal(http.StatusSeeOther, rec.Code)
+
+	overview := ts.URLTo(router.AdminDeniedKeysOverview)
+	a.Equal(overview.Path, rec.Header().Get("Location"))
+	webassert.HasFlashMessages(t, ts.Client, overview, "AdminDeniedKeysRemoved")
 
 	a.Equal(1, ts.DeniedKeysDB.RemoveIDCallCount())
 	_, theID := ts.DeniedKeysDB.RemoveIDArgsForCall(0)
@@ -279,6 +291,7 @@ func TestDeniedKeysRemove(t *testing.T) {
 	ts.DeniedKeysDB.RemoveIDReturns(roomdb.ErrNotFound)
 	addVals = url.Values{"id": []string{"667"}}
 	rec = ts.Client.PostForm(urlRemove, addVals)
-	a.Equal(http.StatusNotFound, rec.Code)
+	a.Equal(http.StatusSeeOther, rec.Code)
-	//TODO: update redirect code with flash errors
+	a.Equal(overview.Path, rec.Header().Get("Location"))
+	webassert.HasFlashMessages(t, ts.Client, overview, "ErrorNotFound")
 }

web/i18n/defaults/active.en.toml

@@ -101,6 +101,7 @@ AdminDeniedKeysComment = "Comment"
 AdminDeniedKeysCommentDescription = "The person who added this ban, added the following comment"
 AdminDeniedKeysRemoveConfirmWelcome = "Are you sure you want to remove this ban? They will will be able to access the room again."
 AdminDeniedKeysRemoveConfirmTitle = "Confirm member removal"
+AdminDeniedKeysRemoved = "The key was removed from the list and is thus no longer banned."
 
 # members dashboard
 ###################