From d147d0642a3752fe98eb628156eb6d717928e9fb Mon Sep 17 00:00:00 2001 From: Henry <111202+cryptix@users.noreply.github.com> Date: Tue, 1 Jun 2021 14:23:42 +0200 Subject: [PATCH] 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 | 43 +++++++++++---- web/handlers/admin/dashboard_test.go | 82 ++++++++++++++++++++++++++++ web/handlers/admin/handler_test.go | 37 ------------- web/templates/admin/dashboard.tmpl | 16 ++---- 4 files changed, 119 insertions(+), 59 deletions(-) create mode 100644 web/handlers/admin/dashboard_test.go delete mode 100644 web/handlers/admin/handler_test.go diff --git a/web/handlers/admin/dashboard.go b/web/handlers/admin/dashboard.go index 9c8423c..bea165e 100644 --- a/web/handlers/admin/dashboard.go +++ b/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() +} diff --git a/web/handlers/admin/dashboard_test.go b/web/handlers/admin/dashboard_test.go new file mode 100644 index 0000000..2d08d58 --- /dev/null +++ b/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) +} diff --git a/web/handlers/admin/handler_test.go b/web/handlers/admin/handler_test.go deleted file mode 100644 index 70a5b2d..0000000 --- a/web/handlers/admin/handler_test.go +++ /dev/null @@ -1,37 +0,0 @@ -package admin - -import ( - "bytes" - "net/http" - "testing" - - "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 TestDashoard(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"}, - }) -} diff --git a/web/templates/admin/dashboard.tmpl b/web/templates/admin/dashboard.tmpl index 2d7e13c..40a491e 100644 --- a/web/templates/admin/dashboard.tmpl +++ b/web/templates/admin/dashboard.tmpl @@ -80,27 +80,21 @@ -
+
{{if gt .OnlineCount 0}}
{{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}}
{{$member}} + >{{.String}}
{{end}}