Address second-pass PR review feedback

- activity.go: URL-encode flag values when building the query string
  so a board/creator ID containing & or = can't inject phantom params.
- board_test.go: guard mock.GetCalls[0] indexing with a length check
  in the new board accesses tests so a missing call surfaces as a
  clear assertion instead of an array-bounds panic.
- e2e activity_test.go: keep retrying until the just-created card
  appears in the activity feed, then fail hard if it doesn't. The
  previous soft Logf masked regressions where cards drop out of the
  activity stream.

Author: robzolkos

e2e/cli_tests/activity_test.go

@@ -14,39 +14,37 @@ func TestActivityList(t *testing.T) {
 	cardNum := createCard(t, h, boardID)
 	creatorID := currentUserID(t, h)
 
+	cardNumStr := strconv.Itoa(cardNum)
 	var result *harness.Result
+	foundCard := false
 	for attempt := 0; attempt < 10; attempt++ {
 		r := h.Run("activity", "list", "--board", boardID)
-		if r.ExitCode == harness.ExitSuccess && len(r.GetDataArray()) > 0 {
+		if r.ExitCode == harness.ExitSuccess {
 			result = r
-			break
+			for _, item := range r.GetDataArray() {
+				m := asMap(item)
+				if m == nil {
+					continue
+				}
+				if eventable := asMap(m["eventable"]); eventable != nil {
+					if mapValueString(eventable, "number") == cardNumStr {
+						foundCard = true
+						break
+					}
+				}
+			}
+			if foundCard {
+				break
+			}
 		}
 		time.Sleep(200 * time.Millisecond)
 	}
 	if result == nil {
-		t.Fatal("expected at least one activity for throwaway board")
+		t.Fatal("expected at least one successful activity list call")
 	}
-
 	assertOK(t, result)
-	if len(result.GetDataArray()) == 0 {
-		t.Fatal("expected activity list to return at least one item")
-	}
-
-	foundCard := false
-	for _, item := range result.GetDataArray() {
-		m := asMap(item)
-		if m == nil {
-			continue
-		}
-		if eventable := asMap(m["eventable"]); eventable != nil {
-			if got := mapValueString(eventable, "number"); got == strconv.Itoa(cardNum) {
-				foundCard = true
-				break
-			}
-		}
-	}
 	if !foundCard {
-		t.Logf("activity list did not expose created card number %d; continuing because board activity was non-empty", cardNum)
+		t.Fatalf("activity list did not expose created card number %d after retries", cardNum)
 	}
 
 	creatorResult := h.Run("activity", "list", "--board", boardID, "--creator", creatorID)

internal/commands/activity.go

@@ -2,6 +2,7 @@ package commands
 
 import (
 	"fmt"
+	"net/url"
 	"strconv"
 	"strings"
 
@@ -36,10 +37,10 @@ var activityListCmd = &cobra.Command{
 
 		var params []string
 		if activityListBoard != "" {
-			params = append(params, "board_ids[]="+activityListBoard)
+			params = append(params, "board_ids[]="+url.QueryEscape(activityListBoard))
 		}
 		if activityListCreator != "" {
-			params = append(params, "creator_ids[]="+activityListCreator)
+			params = append(params, "creator_ids[]="+url.QueryEscape(activityListCreator))
 		}
 		if activityListPage > 0 {
 			params = append(params, "page="+strconv.Itoa(activityListPage))

internal/commands/board_test.go

@@ -688,6 +688,9 @@ func TestBoardAccesses(t *testing.T) {
 		boardAccessesPage = 0
 
 		assertExitCode(t, err, 0)
+		if len(mock.GetCalls) != 1 {
+			t.Fatalf("expected 1 GET call, got %d", len(mock.GetCalls))
+		}
 		if mock.GetCalls[0].Path != "/boards/123/accesses.json" {
 			t.Errorf("expected path '/boards/123/accesses.json', got '%s'", mock.GetCalls[0].Path)
 		}
@@ -711,6 +714,9 @@ func TestBoardAccesses(t *testing.T) {
 		boardAccessesPage = 0
 
 		assertExitCode(t, err, 0)
+		if len(mock.GetCalls) != 1 {
+			t.Fatalf("expected 1 GET call, got %d", len(mock.GetCalls))
+		}
 		if mock.GetCalls[0].Path != "/boards/123/accesses.json?page=2" {
 			t.Errorf("expected path '/boards/123/accesses.json?page=2', got '%s'", mock.GetCalls[0].Path)
 		}