fix rendering non-members on the dashboard (#238)

* fix rendering non-members on the dashboard

fixes #236

* remove alias or feedref code from template

doing this in the template was hard to read and inefficient.

also: rename OnlineMembers to OnlineUsers since it is a misnomer.
There are other connected peers in a room in certain privacy modes.

web/handlers/admin/dashboard.go

@@ -3,6 +3,7 @@
 package admin
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"time"
@@ -29,6 +30,7 @@ type dashboardHandler struct {
 
 func (h dashboardHandler) overview(w http.ResponseWriter, req *http.Request) (interface{}, error) {
 	var (
+		err     error
 		ctx     = req.Context()
 		roomRef = h.netInfo.RoomID.Ref()
 
@@ -54,13 +56,19 @@ func (h dashboardHandler) overview(w http.ResponseWriter, req *http.Request) (in
 	}
 
 	// in the timeout case, nothing will happen here since the onlineRefs slice is empty
-	onlineMembers := make([]roomdb.Member, len(onlineRefs))
+	onlineUsers := make([]connectedUser, len(onlineRefs))
 	for i, ref := range onlineRefs {
-		var err error
-		onlineMembers[i], err = h.dbs.Members.GetByFeed(ctx, ref)
+		// try to get the member
+		onlineUsers[i].Member, err = h.dbs.Members.GetByFeed(ctx, ref)
 		if err != nil {
-			// TODO: do we want to show "external users" (non-members) on the dashboard?
-			return nil, fmt.Errorf("failed to lookup online member: %w", err)
+			if !errors.Is(err, roomdb.ErrNotFound) { // any other error can't be handled here
+				return nil, fmt.Errorf("failed to lookup online member: %w", err)
+			}
+
+			// if there is no member for this ref present it as role unknown
+			onlineUsers[i].ID = -1
+			onlineUsers[i].PubKey = ref
+			onlineUsers[i].Role = roomdb.RoleUnknown
 		}
 	}
 
@@ -80,12 +88,12 @@ func (h dashboardHandler) overview(w http.ResponseWriter, req *http.Request) (in
 	}
 
 	pageData := map[string]interface{}{
-		"RoomRef":       roomRef,
-		"OnlineMembers": onlineMembers,
-		"OnlineCount":   onlineCount,
-		"MemberCount":   memberCount,
-		"InviteCount":   inviteCount,
-		"DeniedCount":   deniedCount,
+		"RoomRef":     roomRef,
+		"OnlineUsers": onlineUsers,
+		"OnlineCount": onlineCount,
+		"MemberCount": memberCount,
+		"InviteCount": inviteCount,
+		"DeniedCount": deniedCount,
 	}
 
 	pageData["Flashes"], err = h.flashes.GetAll(w, req)
@@ -95,3 +103,16 @@ func (h dashboardHandler) overview(w http.ResponseWriter, req *http.Request) (in
 
 	return pageData, nil
 }
+
+// connectedUser defines how we want to present a connected user
+type connectedUser struct {
+	roomdb.Member
+}
+
+// if the member has an alias, use the first one. Otherwise use the public key
+func (dm connectedUser) String() string {
+	if len(dm.Aliases) > 0 {
+		return dm.Aliases[0].Name
+	}
+	return dm.PubKey.Ref()
+}

web/handlers/admin/dashboard_test.go

@@ -0,0 +1,82 @@
+package admin
+
+import (
+	"bytes"
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/ssb-ngi-pointer/go-ssb-room/v2/roomdb"
+	"github.com/ssb-ngi-pointer/go-ssb-room/v2/web/router"
+	"github.com/ssb-ngi-pointer/go-ssb-room/v2/web/webassert"
+	"github.com/stretchr/testify/assert"
+	refs "go.mindeco.de/ssb-refs"
+)
+
+func TestDashboardSimple(t *testing.T) {
+	ts := newSession(t)
+	a := assert.New(t)
+
+	testRef := refs.FeedRef{Algo: "ed25519", ID: bytes.Repeat([]byte{0}, 32)}
+	ts.RoomState.AddEndpoint(testRef, nil) // 1 online
+	ts.MembersDB.CountReturns(4, nil)      // 4 members
+	ts.InvitesDB.CountReturns(3, nil)      // 3 invites
+	ts.DeniedKeysDB.CountReturns(2, nil)   // 2 banned
+
+	dashURL := ts.URLTo(router.AdminDashboard)
+
+	html, resp := ts.Client.GetHTML(dashURL)
+	a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code")
+
+	a.Equal("1", html.Find("#online-count").Text())
+	a.Equal("4", html.Find("#member-count").Text())
+	a.Equal("3", html.Find("#invite-count").Text())
+	a.Equal("2", html.Find("#denied-count").Text())
+
+	webassert.Localized(t, html, []webassert.LocalizedElement{
+		{"title", "AdminDashboardTitle"},
+	})
+}
+
+// make sure the dashboard renders when someone is connected that is not a member
+func TestDashboardWithVisitors(t *testing.T) {
+	ts := newSession(t)
+	a := assert.New(t)
+
+	visitorRef := refs.FeedRef{Algo: "ed25519", ID: bytes.Repeat([]byte{0}, 32)}
+	memberRef := refs.FeedRef{Algo: "ed25519", ID: bytes.Repeat([]byte{1}, 32)}
+	ts.RoomState.AddEndpoint(visitorRef, nil)
+	ts.RoomState.AddEndpoint(memberRef, nil)
+
+	ts.MembersDB.CountReturns(1, nil)
+	// return a member for the member but not for the visitor
+	ts.MembersDB.GetByFeedStub = func(ctx context.Context, r refs.FeedRef) (roomdb.Member, error) {
+		if r.Equal(&memberRef) {
+			return roomdb.Member{ID: 23, Role: roomdb.RoleMember, PubKey: r}, nil
+		}
+		return roomdb.Member{}, roomdb.ErrNotFound
+	}
+
+	dashURL := ts.URLTo(router.AdminDashboard)
+
+	html, resp := ts.Client.GetHTML(dashURL)
+	a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code")
+
+	a.Equal("2", html.Find("#online-count").Text())
+	a.Equal("1", html.Find("#member-count").Text())
+
+	memberList := html.Find("#connected-list a")
+	a.Equal(2, memberList.Length())
+
+	htmlVisitor := memberList.Eq(0)
+	a.Equal(visitorRef.Ref(), htmlVisitor.Text())
+	gotLink, has := htmlVisitor.Attr("href")
+	a.False(has, "visitor should not have a link to a details page: %v", gotLink)
+
+	htmlMember := memberList.Eq(1)
+	a.Equal(memberRef.Ref(), htmlMember.Text())
+	gotLink, has = htmlMember.Attr("href")
+	a.True(has, "member should  have a link to a details page")
+	wantLink := ts.URLTo(router.AdminMemberDetails, "id", 23)
+	a.Equal(wantLink.String(), gotLink)
+}

web/templates/admin/dashboard.tmpl

@@ -80,27 +80,21 @@
     </div>
   </div>
 
-  <div class="mb-8">
+  <div class="mb-8" id="connected-list">
     {{if gt .OnlineCount 0}}
     <div class="ml-11 h-8 w-0.5 bg-gray-200"></div>
     {{end}}
 
-    {{range .OnlineMembers}}
-      {{$member := .PubKey.Ref}}
-      {{$memberIsAlias := false}}
-      {{range $index, $alias := .Aliases}}
-        {{if eq $index 0}}
-          {{$member = $alias.Name}}
-          {{$memberIsAlias = true}}
-        {{end}}
-      {{end}}
+    {{range .OnlineUsers}}
     <div class="ml-11 h-8 w-0.5 bg-gray-200"></div>
     <div class="ml-11 relative h-3">
       <div class="absolute inline-flex w-3 h-3 bg-green-500 rounded-full -left-1 -ml-px"></div>
       <a
+        {{if gt .ID 0}}
         href="{{urlTo "admin:member:details" "id" .ID}}"
+        {{end}}
         class="absolute w-44 sm:w-auto -top-1.5 ml-5 pl-1 font-mono truncate flex-auto text-gray-700 hover:underline"
-        >{{$member}}</a>
+        >{{.String}}</a>
     </div>
     {{end}}
   </div>