Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,23 @@ and the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

## [1.2.0] - 2026-07-11

### Added

- **Self-nomination** (#41) — a member can request a 360 round on themselves
from **My Feedback → Request feedback**: they pick their reviewers, and the
round is created as a draft **owned by a manager (their team admin, else a
global admin) — never the subject**, so the requester can never de-anonymize
their reviewers. One open request at a time. The owner reviews and starts it.

### Changed

- Non-admin **Rounds** now also lists the rounds you own, so a team admin sees
and can manage the rounds they created (including self-nominated ones).
- Performance (#33 follow-up): paginated round lists resolve only the users on
the current page via a batch lookup, instead of loading the whole user table.

## [1.1.0] - 2026-07-11

### Added
Expand Down
78 changes: 78 additions & 0 deletions internal/handlers/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"net/http"
"net/http/cookiejar"
"net/http/httptest"
"net/url"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -64,6 +66,82 @@ func get(t *testing.T, c *http.Client, url string) (int, string) {
return resp.StatusCode, string(body)
}

// csrfToken pulls the per-session CSRF token from a rendered page's meta tag.
func csrfToken(t *testing.T, c *http.Client, base string) string {
t.Helper()
_, body := get(t, c, base+"/my-feedback")
m := regexp.MustCompile(`name="csrf-token" content="([^"]*)"`).FindStringSubmatch(body)
if m == nil {
t.Fatal("no csrf-token meta on page")
}
return m[1]
}

// postForm submits a form with the CSRF token in the header (as htmx does).
func postForm(t *testing.T, c *http.Client, base, path, token string, form url.Values) (int, string) {
t.Helper()
req, _ := http.NewRequest(http.MethodPost, base+path, strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("X-CSRF-Token", token)
resp, err := c.Do(req)
if err != nil {
t.Fatalf("POST %s: %v", path, err)
}
defer resp.Body.Close()
body, _ := io.ReadAll(resp.Body)
return resp.StatusCode, string(body)
}

func TestSelfNominationOwnedByManagerNotSubject(t *testing.T) {
srv, admin, repos := newTestServer(t)
ctx := t.Context()
// admin@example.com matches ADMIN_EMAIL → the eligible owner.
_, _ = get(t, admin, srv.URL+"/auth/dev-login?email=admin@example.com")
adminUser, _ := repos.Users.FindByEmail(ctx, "admin@example.com")

// A member requests feedback on themselves.
jar, _ := cookiejar.New(nil)
member := &http.Client{Jar: jar}
member.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }
_, _ = get(t, member, srv.URL+"/auth/dev-login?email=mia@example.com")
memberUser, _ := repos.Users.FindByEmail(ctx, "mia@example.com")
token := csrfToken(t, member, srv.URL)

code, _ := postForm(t, member, srv.URL, "/request-feedback", token, url.Values{"reviewer_ids": {adminUser.ID}})
if code != http.StatusSeeOther {
t.Fatalf("expected 303 after request, got %d", code)
}

rounds, _ := repos.Rounds.FindBySubjectID(ctx, memberUser.ID)
if len(rounds) != 1 {
t.Fatalf("expected exactly one requested round, got %d", len(rounds))
}
rd := rounds[0]
if rd.SubjectID != memberUser.ID {
t.Fatalf("subject should be the member")
}
// The invariant: the owner (creator, who gets raw-submission access) is NOT
// the subject — otherwise the member could de-anonymize their reviewers.
if rd.CreatedByID == memberUser.ID {
t.Fatal("SECURITY: self-nominated round must not be owned by its subject")
}
if rd.CreatedByID != adminUser.ID {
t.Fatalf("expected the admin to own the round, got %q", rd.CreatedByID)
}
if rd.Status != models.RoundDraft {
t.Fatalf("expected draft (awaiting owner approval), got %q", rd.Status)
}

// A second request is blocked while one is pending.
code, _ = postForm(t, member, srv.URL, "/request-feedback", token, url.Values{"reviewer_ids": {adminUser.ID}})
if code != http.StatusSeeOther {
t.Fatalf("expected redirect on duplicate request, got %d", code)
}
if again, _ := repos.Rounds.FindBySubjectID(ctx, memberUser.ID); len(again) != 1 {
t.Fatalf("duplicate request should not create a second round; got %d", len(again))
}
}

func TestUnauthenticatedRedirectsToLogin(t *testing.T) {
srv, client, _ := newTestServer(t)
// Don't follow redirects, so we can assert the 303 → /login.
Expand Down
9 changes: 8 additions & 1 deletion internal/handlers/myfeedback.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,14 @@ func (h *Handlers) MyFeedback(w http.ResponseWriter, r *http.Request) {
"Radar": radar,
"CanCompare": len(cons) >= 2,
}
h.View.Page(w, http.StatusOK, h.page(r, "My feedback", "my-feedback", "my_feedback_content", data))
pd := h.page(r, "My feedback", "my-feedback", "my_feedback_content", data)
switch {
case r.URL.Query().Get("requested") == "1":
pd.Flash = "Your feedback request was sent — a manager will review and start it."
case r.URL.Query().Get("pending") == "1":
pd.Flash = "You already have a feedback request awaiting a manager's approval."
}
h.View.Page(w, http.StatusOK, pd)
}

func deltaLen(d *models.SelfVsOthersDelta) int {
Expand Down
50 changes: 43 additions & 7 deletions internal/handlers/presenters.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,31 @@ func (h *Handlers) roundsForMe(ctx context.Context, userID string) ([]models.Fee
if err != nil {
return nil, err
}
asOwner, err := h.Repos.Rounds.FindByCreatedByID(ctx, userID)
if err != nil {
return nil, err
}
asSubject, err := h.Repos.Rounds.FindBySubjectID(ctx, userID)
if err != nil {
return nil, err
}
seen := make(map[string]bool, len(asReviewer))
out := make([]models.FeedbackRound, 0, len(asReviewer)+len(asSubject))
for _, r := range asReviewer {
if !seen[r.ID] {
seen[r.ID] = true
out = append(out, r)
seen := make(map[string]bool, len(asReviewer)+len(asOwner))
out := make([]models.FeedbackRound, 0, len(asReviewer)+len(asOwner)+len(asSubject))
add := func(rounds []models.FeedbackRound) {
for _, r := range rounds {
if !seen[r.ID] {
seen[r.ID] = true
out = append(out, r)
}
}
}
// Rounds I review, plus every round I own (a team admin manages their own
// rounds, including ones members self-nominated to them).
add(asReviewer)
add(asOwner)
// As the subject, I only participate while the round is collecting feedback
// (self-assessment); I never gain owner access to my own rounds.
for _, r := range asSubject {
// The subject participates only while the round is collecting feedback.
if r.Status == models.RoundActive && !seen[r.ID] {
seen[r.ID] = true
out = append(out, r)
Expand Down Expand Up @@ -74,6 +85,31 @@ func (h *Handlers) allUsersIndex(ctx context.Context) (map[string]models.User, e
return userMap(users), nil
}

// usersForRounds resolves just the users a set of rounds reference (subject +
// creator) in a single batch query, rather than loading the whole user table.
func (h *Handlers) usersForRounds(ctx context.Context, rounds []models.FeedbackRound) (map[string]models.User, error) {
seen := make(map[string]struct{}, len(rounds)*2)
ids := make([]string, 0, len(rounds)*2)
add := func(id string) {
if id == "" {
return
}
if _, ok := seen[id]; !ok {
seen[id] = struct{}{}
ids = append(ids, id)
}
}
for _, rd := range rounds {
add(rd.SubjectID)
add(rd.CreatedByID)
}
users, err := h.Repos.Users.FindByIDs(ctx, ids)
if err != nil {
return nil, err
}
return userMap(users), nil
}

// canSeeManagerOnlyChannel reports whether the caller may see the private
// manager-only synthesis: global admins and the round creator, never the subject.
func canSeeManagerOnlyChannel(user *models.User, round models.FeedbackRound) bool {
Expand Down
15 changes: 9 additions & 6 deletions internal/handlers/rounds.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,9 @@ func (h *Handlers) RoundsList(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
u := h.user(r)

users, err := h.allUsersIndex(ctx)
if err != nil {
serverError(w, err)
return
}

var rounds []models.FeedbackRound
var nav pageNav // zero value renders no controls (both HasPrev/HasNext false)
var err error
if u.Role == models.RoleAdmin {
page := pageParam(r)
paged, perr := h.Repos.Rounds.FindPaged(ctx, pageSize+1, (page-1)*pageSize)
Expand All @@ -92,6 +87,14 @@ func (h *Handlers) RoundsList(w http.ResponseWriter, r *http.Request) {
return
}

// Resolve only the users referenced by this page (subject + creator), not
// the whole table — keeps the per-page cost bounded as the org grows.
users, err := h.usersForRounds(ctx, rounds)
if err != nil {
serverError(w, err)
return
}

cards := make([]RoundCard, 0, len(rounds))
for _, rd := range rounds {
cards = append(cards, h.toCard(ctx, rd, u.ID, users))
Expand Down
4 changes: 4 additions & 0 deletions internal/handlers/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ func (h *Handlers) MountAppRoutes(r chi.Router, submitLimit func(http.Handler) h
// Onboarding
r.Post("/onboarding/complete", h.CompleteOnboarding)

// Self-nomination: any member can request a feedback round on themselves.
r.Get("/request-feedback", h.RequestFeedbackForm)
r.Post("/request-feedback", h.CreateFeedbackRequest)

// Rounds
r.Get("/rounds", h.RoundsList)
r.With(h.Auth.RequireTeamAdminOrAdmin).Get("/rounds/new", h.NewRoundForm)
Expand Down
141 changes: 141 additions & 0 deletions internal/handlers/selfnominate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package handlers

import (
"context"
"errors"
"net/http"
"strings"

"github.com/mondial7/smart-360/internal/models"
)

// RequestFeedbackForm lets a member ask for a feedback round on themselves.
func (h *Handlers) RequestFeedbackForm(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
u := h.user(r)

// One open request at a time: if the member already has a draft round as the
// subject, send them to it rather than letting requests pile up.
if pending, _ := h.pendingRequestFor(ctx, u.ID); pending != nil {
http.Redirect(w, r, "/my-feedback?pending=1", http.StatusSeeOther)
return
}

// Everyone except the member can be a reviewer.
all, err := h.Repos.Users.FindAll(ctx)
if err != nil {
serverError(w, err)
return
}
reviewers := make([]models.User, 0, len(all))
for _, usr := range all {
if usr.ID != u.ID {
reviewers = append(reviewers, usr)
}
}
templates, err := h.Repos.Templates.FindAll(ctx)
if err != nil {
serverError(w, err)
return
}
data := map[string]any{"Reviewers": reviewers, "Templates": templates}
h.View.Page(w, http.StatusOK, h.page(r, "Request feedback", "my-feedback", "request_feedback_content", data))
}

// CreateFeedbackRequest creates a self-nominated round. The round is owned by a
// manager who is NOT the subject (the member's team admin, else a global admin),
// so the subject never gains the owner's raw-submission access. It starts as a
// draft for the owner to review and start.
func (h *Handlers) CreateFeedbackRequest(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
u := h.user(r)

if pending, _ := h.pendingRequestFor(ctx, u.ID); pending != nil {
http.Redirect(w, r, "/my-feedback?pending=1", http.StatusSeeOther)
return
}

owner, err := h.resolveOwnerFor(ctx, u)
if err != nil {
http.Error(w, "No manager is available to receive your request. Ask an admin to run a round for you.", http.StatusConflict)
return
}
if err := r.ParseForm(); err != nil {
http.Error(w, "bad form", http.StatusBadRequest)
return
}

var templateID *string
if t := r.FormValue("template_id"); t != "" {
templateID = &t
}
round := &models.FeedbackRound{
SubjectID: u.ID,
CreatedByID: owner.ID, // owner ≠ subject: preserves reviewer anonymity
TemplateID: templateID,
Deadline: parseDate(r.FormValue("deadline")),
Status: models.RoundDraft,
}
if err := h.Repos.Rounds.Create(ctx, round); err != nil {
serverError(w, err)
return
}
for _, reviewerID := range r.Form["reviewer_ids"] {
if reviewerID != "" && reviewerID != u.ID {
_ = h.Repos.Rounds.AddReviewer(ctx, round.ID, models.RoundReviewer{ReviewerID: reviewerID})
}
}
// Actor is the requester; the round is attributed to them in the trail.
h.audit(ctx, auditParams{Action: models.AuditRoundRequested, Actor: u, RoundID: round.ID,
RoundSubject: u.Name, Description: "Requested a feedback round (owner: " + owner.Name + ")"})

http.Redirect(w, r, "/my-feedback?requested=1", http.StatusSeeOther)
}

// pendingRequestFor returns the member's existing draft round-as-subject, if any.
func (h *Handlers) pendingRequestFor(ctx context.Context, memberID string) (*models.FeedbackRound, error) {
rounds, err := h.Repos.Rounds.FindBySubjectID(ctx, memberID)
if err != nil {
return nil, err
}
for i := range rounds {
if rounds[i].Status == models.RoundDraft {
return &rounds[i], nil
}
}
return nil, nil
}

// resolveOwnerFor picks a manager to own a member's self-nominated round —
// never the member themselves. Prefers the member's team admin, then the
// configured ADMIN_EMAIL admin, then any other admin.
func (h *Handlers) resolveOwnerFor(ctx context.Context, member *models.User) (*models.User, error) {
if member.TeamID != nil {
if team, err := h.Repos.Teams.FindByID(ctx, *member.TeamID); err == nil &&
team.TeamAdminID != "" && team.TeamAdminID != member.ID {
if admin, err := h.Repos.Users.FindByID(ctx, team.TeamAdminID); err == nil {
return admin, nil
}
}
}
users, err := h.Repos.Users.FindAll(ctx)
if err != nil {
return nil, err
}
var fallback *models.User
for i := range users {
if users[i].Role != models.RoleAdmin || users[i].ID == member.ID {
continue
}
if h.Cfg.AdminEmail != "" && strings.EqualFold(users[i].Email, h.Cfg.AdminEmail) {
return &users[i], nil // prefer the bootstrap owner
}
if fallback == nil {
fallback = &users[i]
}
}
if fallback != nil {
return fallback, nil
}
return nil, errors.New("no eligible manager")
}
1 change: 1 addition & 0 deletions internal/models/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ type AuditAction string
const (
// Round lifecycle
AuditRoundCreated AuditAction = "round.created"
AuditRoundRequested AuditAction = "round.requested"
AuditRoundStatusChanged AuditAction = "round.status_changed"
AuditRoundSubjectChanged AuditAction = "round.subject_changed"
AuditRoundDeadlineChanged AuditAction = "round.deadline_changed"
Expand Down
Loading
Loading