diff --git a/CHANGELOG.md b/CHANGELOG.md index 025aaefa..c1d0e6b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,8 +20,24 @@ written while it was being built. See [RELEASING.md](RELEASING.md). ### Changed +- Following an invitation link into a private hackathon now admits you straight + away, rather than putting you on a list for an organiser to approve. The + invitation is the decision. Public hackathons are unchanged. +- Manage Participants no longer shows a Waitlist tab on a private hackathon, + where nobody should ever be waiting. It reappears if somebody is. + ### Fixed +- Invitation links no longer fail with "This invitation is no longer valid" for + people who have never used Hackagon before. The link was always fine — their + account had simply never been created. +- After accepting an invitation to a private hackathon, the page told people + they were on a list and that organisers would confirm their place, when they + were already full members. It now says "You're in" and links into the event. +- An invitation that only half went through used to leave somebody holding a + place they could not see, with nothing they could do about it. The invitation + page now offers "Finish joining", which completes it. + ## [0.8.0](https://github.com/SwissDataScienceCenter/hackagon/releases/tag/v0.8.0) - 2026-09-08 The first tagged release, and the one currently in production. It predates this diff --git a/components/backend/cmd/seed/h3.go b/components/backend/cmd/seed/h3.go index 70cb5ee0..00c93e7c 100644 --- a/components/backend/cmd/seed/h3.go +++ b/components/backend/cmd/seed/h3.go @@ -128,9 +128,11 @@ func (h *harness) seedH3(now time.Time, admin, alice, dana *actor) error { return err } - // Private, so getting in takes an invitation. `Join` admits anyone who can - // already read the hackathon — which on the public fixtures is everybody — - // and refuses everyone else outright unless they carry a valid invite token. + // Private, so getting in takes an invitation — and an invitation is all it + // takes: `Join` confirms a private hackathon's joiners itself. It admits + // anyone who can already read the hackathon — which on the public fixtures is + // everybody — and refuses everyone else outright unless they carry a valid + // invite token. // Neither alice nor dana holds a role here before joining, so without this // they are turned away with "invalid or expired invitation". // diff --git a/components/backend/cmd/seed/h5.go b/components/backend/cmd/seed/h5.go index e6580347..6121c02c 100644 --- a/components/backend/cmd/seed/h5.go +++ b/components/backend/cmd/seed/h5.go @@ -65,8 +65,8 @@ func (h *harness) seedH5(now time.Time, alice, dana *actor) error { "Two days with the SDSC data partners, working on the datasets " + "nobody can publish yet. Attendance is by invitation: there is no " + "public sign-up page and this event is not listed anywhere.\n\n" + - "If you were sent a link, you are in the right place — request a " + - "place below and one of the organizers will confirm it.", + "If you were sent a link, you are in the right place — the " + + "invitation is your place, so accepting it puts you straight in.", ), StartsAt: timestamppb.New(startsAt), EndsAt: timestamppb.New(endsAt), @@ -113,14 +113,19 @@ func (h *harness) seedH5(now time.Time, alice, dana *actor) error { return err } - // dana joins on the live link and stays waitlisted: approval is a separate - // act, and somebody sitting in the queue is what gives the organizer's - // waitlist something to approve. + // dana follows the live link and is a confirmed member the moment she does: + // this hackathon is private, and `Join` treats the invitation as the decision + // rather than parking her on a waiting list nobody can see her on. So this + // call is the fixture for the whole invite-to-membership path, end to end. + // + // H5 therefore has an empty waitlist, deliberately. The organizer's + // waitlist-with-somebody-on-it lives in H1, which is public and where + // approval is still a separate act. // // Before the form below exists, which is the same order H1 uses and for the // same reason: `Join` refuses a signup that leaves a mandatory question - // unanswered, and `joinWithInvite` sends no answers. So dana is the fixture - // for somebody who got in before the form went up — waitlisted, with + // unanswered, and `joinWithInvite` sends no answers. So dana is also the + // fixture for somebody who got in before the form went up — a member with // nothing on file for an organizer to read. if err := h.joinWithInvite(dana, id, live.GetToken()); err != nil { return err diff --git a/components/backend/cmd/seed/steps.go b/components/backend/cmd/seed/steps.go index 8c249165..2cfcb532 100644 --- a/components/backend/cmd/seed/steps.go +++ b/components/backend/cmd/seed/steps.go @@ -146,8 +146,10 @@ func (h *harness) revokeInvite(owner *actor, inviteID string) error { return nil } -// join signs somebody up. Join always writes a waitlisted row — approval is a -// separate act — so this on its own is the fixture's waitlisted participant. +// join signs somebody up. In a **public** hackathon Join writes a waitlisted row +// and approval is a separate act, so this on its own is the fixture's waitlisted +// participant. In a private one Join confirms on the spot, so this leaves a full +// member and there is nothing left to approve. // // It sends no answers, which only works while the hackathon asks nothing // mandatory. Where the fixture wants both a registration form and somebody who @@ -162,7 +164,9 @@ func (h *harness) join(who *actor, hackathonID string) error { // An empty token means none, which is what every public hackathon sends: Join // only looks at the token when the hackathon is private, and admits anyone who // can read the hackathon regardless. Pass a real one and it is the token that -// gets somebody into a hackathon they cannot see. +// gets somebody into a hackathon they cannot see — and, in a private hackathon, +// straight into membership: the invitation is the decision, so Join confirms +// them itself rather than leaving them for an organizer. func (h *harness) joinWithInvite(who *actor, hackathonID, token string) error { // Absent rather than empty on the wire. The handler compares the token // against "" before parsing it as a uuid, so an empty string would take the @@ -196,6 +200,10 @@ func (h *harness) joinAndApprove(owner *actor, hackathonID string, who ...*actor // The same invitation admits everyone in `who`: an invite is a link rather than // a per-person ticket, and one link passed around is how a private hackathon // actually fills up. +// +// The approval half is redundant in a private hackathon, where Join confirms +// people itself, and kept anyway: `ApproveParticipant` is idempotent in both +// halves, and this helper is also how public fixtures are filled. func (h *harness) joinAndApproveWithInvite( owner *actor, hackathonID, token string, diff --git a/components/backend/internal/service/hackathon_service.go b/components/backend/internal/service/hackathon_service.go index 35ba857a..099f4550 100644 --- a/components/backend/internal/service/hackathon_service.go +++ b/components/backend/internal/service/hackathon_service.go @@ -487,7 +487,45 @@ func hackathonFinished(h *ent.Hackathon) bool { return h.EndsAt != nil && h.EndsAt.Before(time.Now()) } -//nolint:gocognit // Joining is pretty complex, no way around that. +// inviteAdmits says whether `token` is a valid invitation to this hackathon. +func (s *HackathonService) inviteAdmits( + ctx context.Context, + hackathonID uuid.UUID, + token string, +) (bool, error) { + if token == "" { + return false, nil + } + + inviteID, err := uuid.Parse(token) + if err != nil { + return false, status.Error(codes.InvalidArgument, "invalid invite token") + } + + invite, err := s.dbClient.HackathonInvite.Query(). + Where( + enthackathoninvite.Token(inviteID), + enthackathoninvite.HasHackathonWith(enthackathon.IDEQ(hackathonID)), + ).Only(ctx) + if err != nil { + if ent.IsNotFound(err) { + return false, status.Error(codes.NotFound, "invite not found") + } + slog.Error("query invite", "err", err) + + return false, status.Error(codes.Internal, "couldn't query database") + } + + if invite.RevokedAt != nil { + return false, status.Error(codes.FailedPrecondition, "this invite is not valid anymore") + } + if invite.ExpiresAt != nil && invite.ExpiresAt.Before(time.Now()) { + return false, status.Error(codes.FailedPrecondition, "this invite expired") + } + + return true, nil +} + func (s *HackathonService) Join( ctx context.Context, req *msgs.JoinRequest, @@ -522,43 +560,10 @@ func (s *HackathonService) Join( } inviteValid := false - //nolint:nestif // Complexity is ok here. if h.Visibility == enthackathon.VisibilityPrivate { - inviteToken := req.GetInviteToken() - if inviteToken != "" { - inviteID, parseErr := uuid.Parse(inviteToken) - if parseErr != nil { - return nil, status.Error(codes.InvalidArgument, "invalid invite token") - } - invite, err := s.dbClient.HackathonInvite.Query(). - Where( - enthackathoninvite.Token(inviteID), - enthackathoninvite.HasHackathonWith(enthackathon.IDEQ(id)), - ).Only(ctx) - if err != nil { - if ent.IsNotFound(err) { - return nil, status.Errorf( - codes.NotFound, - "invite not found", - ) - } - slog.Error("query hackathon", "err", err) - - return nil, status.Error(codes.Internal, "couldn't query database") - } - if invite.RevokedAt != nil { - return nil, status.Errorf( - codes.FailedPrecondition, - "this invite is not valid anymore", - ) - } - if invite.ExpiresAt != nil && invite.ExpiresAt.Before(time.Now()) { - return nil, status.Errorf( - codes.FailedPrecondition, - "this invite expired", - ) - } - inviteValid = true + inviteValid, err = s.inviteAdmits(ctx, id, req.GetInviteToken()) + if err != nil { + return nil, err } } // a hackathon need to have join permission enabled(== registration phase open), and @@ -655,9 +660,69 @@ func (s *HackathonService) Join( return nil, status.Error(codes.Internal, "couldn't commit transaction") } + // A private hackathon confirms the joiner right here. The invitation was + // already the organizer's decision, and asking for a second one left the + // invitee holding no role — so the event they had just joined was hidden + // from them. Public hackathons still waitlist. + // + // Keyed on the row still waiting, not on this call having created it. Both + // halves below are idempotent, so a confirmation that half-failed earlier is + // retried and healed the next time the invitee joins — the old guard skipped + // the whole block once a row existed, which meant nobody but an organizer + // could ever repair it. + // + // A failure is still logged rather than returned: the join above is already + // committed, and what is left is a waitlisted row that either the next Join + // or the organizer's Approve clears. + if h.Visibility == enthackathon.VisibilityPrivate && participant.IsWaiting { + if err := s.grantMembership(ctx, id, user); err != nil { + // grantMembership already logged the cause; this says who it hit. + slog.Error("private join not auto-approved", "hackathon", id, "user", user.ID) + } + } + return &msgs.JoinResponse{HackathonId: h.ID.String()}, nil } +// grantMembership confirms a participant: casbin `Member` role first, then +// `is_waiting` cleared. Both matter — the role is what makes the hackathon +// visible to them, the flag is what the rosters show. +// +// The order is deliberate. Casbin and the database cannot share one transaction, +// so if the second write fails the order decides what is left behind. Role first +// leaves somebody who can use the hackathon but still shows as waiting, and +// Approve — which calls this same function — repairs that. The reverse would +// show "Approved" over an account that can see nothing, with no control to fix +// it. +// +// Failures are logged here and returned as a status error, like the other +// helpers in this package. +func (s *HackathonService) grantMembership( + ctx context.Context, + hackathonID uuid.UUID, + user *ent.User, +) error { + if _, err := s.enforcer.AddRole(user.KeycloakID, mw.Member, hackathonID.String()); err != nil { + slog.Error("add hackathon member", "err", err) + + return status.Error(codes.Internal, "couldn't set hackathon member permission") + } + + if _, err := s.dbClient.Participant.Update(). + Where( + entparticipant.HackathonIDEQ(hackathonID), + entparticipant.UserID(user.ID), + ). + SetIsWaiting(false). + Save(ctx); err != nil { + slog.Error("clear is_waiting", "err", err) + + return status.Error(codes.Internal, "couldn't approve participant") + } + + return nil +} + func (s *HackathonService) ApproveParticipant( ctx context.Context, req *msgs.ApproveParticipantRequest, @@ -722,23 +787,9 @@ func (s *HackathonService) ApproveParticipant( return nil, status.Error(codes.Internal, "couldn't query database") } - // Update participant record to set is_waiting=false (approved) - _, err = s.dbClient.Participant.Update(). - Where( - entparticipant.HackathonIDEQ(id), - entparticipant.UserID(user.ID), - ). - SetIsWaiting(false). - Save(ctx) - if err != nil { - slog.Error("update participant", "err", err) - - return nil, status.Errorf(codes.Internal, "couldn't approve participant") - } - if _, err := s.enforcer.AddRole(user.KeycloakID, mw.Member, h.ID.String()); err != nil { - slog.Error("add hackathon member", "err", err) - - return nil, status.Errorf(codes.Internal, "couldn't set hackathon member permission") + // The same confirmation a private hackathon does for itself in `Join`. + if err := s.grantMembership(ctx, h.ID, user); err != nil { + return nil, err } return &msgs.ApproveParticipantResponse{}, nil diff --git a/components/backend/internal/service/hackathon_service_test.go b/components/backend/internal/service/hackathon_service_test.go index 8908f02a..affa8c63 100644 --- a/components/backend/internal/service/hackathon_service_test.go +++ b/components/backend/internal/service/hackathon_service_test.go @@ -299,7 +299,8 @@ var _ = Describe("HackathonService", func() { Expect(err).NotTo(HaveOccurred()) }) - It("allows authorized user to join hackathon", func() { + It("waitlists an authorized joiner on a public hackathon, "+ + "for an organizer to confirm", func() { // Create a non-admin test user nonAdminKeycloakID := "non-admin" token := testutils.CreateTestJWTToken(nonAdminKeycloakID) @@ -4607,7 +4608,8 @@ var _ = Describe("HackathonService", func() { inviteToken = resp.GetInvite().GetToken() }) - It("allows join with valid invite token on private hackathon", func() { + It("confirms the joiner outright on a private hackathon, "+ + "clearing is_waiting and granting the role that makes it readable", func() { nonAdminKeycloakID := "invite-join-user" token := testutils.CreateTestJWTToken(nonAdminKeycloakID) ctx := metadata.NewOutgoingContext( @@ -4639,7 +4641,55 @@ var _ = Describe("HackathonService", func() { WithUser(). Only(context.Background()) Expect(err).NotTo(HaveOccurred()) - Expect(participant.IsWaiting).To(BeTrue()) + Expect(participant.IsWaiting).To(BeFalse()) + + got, err := client.Get(ctx, &msgs.GetRequest{HackathonId: privateHackathonID}) + Expect(err).NotTo(HaveOccurred()) + Expect(got.GetHackathon().GetId()).To(Equal(privateHackathonID)) + }) + + It("confirms a joiner who is still waiting, so a half-failed "+ + "confirmation heals on the next join", func() { + waitingKeycloakID := "invite-rejoin-user" + waitingUser, err := dbClient.User.Create(). + SetKeycloakID(waitingKeycloakID). + SetUsername("invite-rejoin-user-username"). + Save(context.Background()) + Expect(err).NotTo(HaveOccurred()) + + // The state a half-failed confirmation leaves: a committed row + // that still says waiting, and no Member role. + _, err = dbClient.Participant.Create(). + SetHackathonID(uuid.MustParse(privateHackathonID)). + SetUserID(waitingUser.ID). + SetIsWaiting(true). + Save(context.Background()) + Expect(err).NotTo(HaveOccurred()) + + token := testutils.CreateTestJWTToken(waitingKeycloakID) + ctx := metadata.NewOutgoingContext( + context.Background(), + metadata.Pairs("authorization", "Bearer "+token), + ) + + _, err = client.Join(ctx, &msgs.JoinRequest{ + HackathonId: privateHackathonID, + InviteToken: &inviteToken, + }) + Expect(err).NotTo(HaveOccurred()) + + participant, err := dbClient.Participant.Query(). + Where( + entparticipant.HackathonIDEQ(uuid.MustParse(privateHackathonID)), + entparticipant.UserID(waitingUser.ID), + ). + Only(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(participant.IsWaiting).To(BeFalse()) + + got, err := client.Get(ctx, &msgs.GetRequest{HackathonId: privateHackathonID}) + Expect(err).NotTo(HaveOccurred()) + Expect(got.GetHackathon().GetId()).To(Equal(privateHackathonID)) }) It("rejects join without invite token on private hackathon", func() { diff --git a/components/frontend/src/lib/components/hackathon/ParticipantsManageTabs.svelte b/components/frontend/src/lib/components/hackathon/ParticipantsManageTabs.svelte index 48ff95e1..014d4f35 100644 --- a/components/frontend/src/lib/components/hackathon/ParticipantsManageTabs.svelte +++ b/components/frontend/src/lib/components/hackathon/ParticipantsManageTabs.svelte @@ -6,12 +6,16 @@ current, confirmedCount, waitingCount, + showWaitlist, }: { hackathonId: string; /** Which of the two pages is rendering this. */ current: 'roster' | 'waitlist'; confirmedCount: number; waitingCount: number; + /** Whether this hackathon has a waitlist worth a tab. False renders + * nothing at all — see the comment below for why not one lone chip. */ + showWaitlist: boolean; } = $props(); @@ -28,29 +32,37 @@ Counts sit in the tabs rather than under the heading: the number an organiser wants is usually the one on the tab they are *not* on. + + **The whole bar disappears with the waitlist**, rather than leaving a lone + "Participants" chip: a segmented control with one segment offers no choice and + reads as a broken one. The roster prints its own count under its heading, so + nothing is lost with it. `showWaitlist` is the caller's call — the waitlist + page always passes true, because a page must not hide its own tab. --> - +{#if showWaitlist} + +{/if} diff --git a/components/frontend/src/lib/components/hackathon/ParticipantsManageTabs.test.ts b/components/frontend/src/lib/components/hackathon/ParticipantsManageTabs.test.ts new file mode 100644 index 00000000..4d13f688 --- /dev/null +++ b/components/frontend/src/lib/components/hackathon/ParticipantsManageTabs.test.ts @@ -0,0 +1,74 @@ +import { render, screen } from "@testing-library/svelte" +import { describe, expect, it } from "vitest" + +import ParticipantsManageTabs from "./ParticipantsManageTabs.svelte" + +/* + * Whether there is a tab bar at all, which is the only branch in here. + * + * `showWaitlist` is the caller's decision, not this component's — the roster + * page composes it from the hackathon's visibility and the size of the queue + * (see `waitlistsJoiners` in its load), and the waitlist page passes true + * unconditionally so that a page reached by link never hides its own tab. What + * is asserted here is what either answer renders. + * + * The all-or-nothing part is the point: a false leaves no lone "Participants" + * chip behind, because a segmented control with one segment reads as a broken + * one rather than as a choice. + */ + +const props = { + hackathonId: "h1", + current: "roster" as const, + confirmedCount: 12, + waitingCount: 3, + showWaitlist: true, +} + +describe("ParticipantsManageTabs", () => { + it("renders both halves when there is a waitlist to show", () => { + render(ParticipantsManageTabs, props) + + expect( + screen.getByRole("navigation", { name: "Participants" }), + ).toBeTruthy() + expect(screen.getByRole("link", { name: /Participants 12/ })).toBeTruthy() + expect(screen.getByRole("link", { name: /Waitlist 3/ })).toBeTruthy() + }) + + it("renders nothing at all when the waitlist is not shown", () => { + render(ParticipantsManageTabs, { ...props, showWaitlist: false }) + + // Not "the waitlist chip is gone" — the roster's own chip goes with it, so + // there is no navigation left to find. + expect(screen.queryByRole("navigation")).toBeNull() + expect(screen.queryByRole("link")).toBeNull() + }) + + it("badges a non-empty queue and leaves an empty one plain", () => { + const { container, unmount } = render(ParticipantsManageTabs, props) + expect(container.querySelector(".badge-warning")).not.toBeNull() + unmount() + + // A public hackathon keeps this tab at zero: there the waitlist is the + // front door, so "0 waiting" is an answer. Nothing to chase, no warning. + const empty = render(ParticipantsManageTabs, { ...props, waitingCount: 0 }) + expect(empty.container.querySelector(".badge-warning")).toBeNull() + expect(screen.getByRole("link", { name: /Waitlist 0/ })).toBeTruthy() + }) + + it("marks the current half for assistive tech", () => { + render(ParticipantsManageTabs, { ...props, current: "waitlist" }) + + expect( + screen + .getByRole("link", { name: /Waitlist/ }) + .getAttribute("aria-current"), + ).toBe("page") + expect( + screen + .getByRole("link", { name: /Participants/ }) + .getAttribute("aria-current"), + ).toBeNull() + }) +}) diff --git a/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/+page.server.ts b/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/+page.server.ts index 571e7c2a..eba101e6 100644 --- a/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/+page.server.ts +++ b/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/+page.server.ts @@ -1,4 +1,5 @@ import type { PageServerLoad } from "./$types" +import { Visibility } from "$lib/server/grpc/generated/hackathon/entities/visibility" import { membershipBadgeLabel } from "$lib/utils/hackathonRole" import { mayManageParticipants } from "$lib/server/hackathon/capabilities" import { answeredParticipantIds } from "$lib/server/hackathon/registrationForm" @@ -45,6 +46,32 @@ async function answerStatus( } } +/** Whether this hackathon puts joiners on the waitlist at all. + * + * Only a public one does. `Join` confirms a private hackathon's joiners itself + * (`hackathon_service.go:693`), because the invitation already *is* the + * organizer's decision about who takes part — so a private waitlist is not the + * front door, it is an anomaly. + * + * Not the same question as "is the waitlist empty", and the difference is why + * the tab is not simply hidden whenever nothing is in it: + * + * - A **public** hackathon waitlists everybody, so an empty queue means "no + * requests yet" — real information, on the page an organizer checks for it. + * A tab that vanished as the last person was approved would take that away + * at exactly the moment they looked again. + * - A **private** one should never have a queue, so an empty one is worth no + * tab. A non-empty one is worth the tab badly: auto-approval failure is + * logged rather than returned (`hackathon_service.go:694`), which leaves a + * waitlisted row that Approve — behind this tab — is the repair for. Hiding + * it by visibility alone would strand that person with nothing on screen + * and no control that fixes them. `Update` can also flip a hackathon to + * private (`:961`) while people are queued in it. + */ +function waitlistsJoiners(visibility: Visibility): boolean { + return visibility === Visibility.VISIBILITY_PUBLIC +} + export const load: PageServerLoad = async (event) => { // No RPC of its own for the roster: the layout's `hackathon.get` already // returns every participant with their casbin role and waitlist flag. @@ -104,5 +131,8 @@ export const load: PageServerLoad = async (event) => { waitingCount: hackathon.members.filter( (m) => m.user !== undefined && m.isWaiting, ).length, + // A plain boolean, not the enum: this crosses to a `.svelte` file, which + // may not import from `$lib/server` — the generated `Visibility` included. + waitlistsJoiners: waitlistsJoiners(hackathon.visibility), } } diff --git a/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/+page.svelte b/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/+page.svelte index 639662bf..c13097ed 100644 --- a/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/+page.svelte +++ b/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/+page.svelte @@ -99,11 +99,18 @@ + 0} />
diff --git a/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/waitlist/+page.server.ts b/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/waitlist/+page.server.ts index cfe8d74a..80727675 100644 --- a/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/waitlist/+page.server.ts +++ b/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/waitlist/+page.server.ts @@ -1,4 +1,5 @@ import type { PageServerLoad } from "./$types" +import { Visibility } from "$lib/server/grpc/generated/hackathon/entities/visibility" import { mayManageParticipants } from "$lib/server/hackathon/capabilities" import { answeredParticipantIds } from "$lib/server/hackathon/registrationForm" import { requireGrpc } from "$lib/server/grpc/client" @@ -80,5 +81,11 @@ export const load: PageServerLoad = async (event) => { confirmedCount: hackathon.members.filter( (m) => m.user !== undefined && !m.isWaiting, ).length, + // Not for the tab — this page always shows its own — but for the copy: an + // empty queue means "no requests yet" in a public hackathon and "nothing + // went wrong" in a private one, and a queue that is *not* empty in a + // private one is the anomaly worth naming. See the roster's + // `waitlistsJoiners`, which decides the tab from the same fact. + waitlistsJoiners: hackathon.visibility === Visibility.VISIBILITY_PUBLIC, } } diff --git a/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/waitlist/+page.svelte b/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/waitlist/+page.svelte index 2d4138f4..0157b3a0 100644 --- a/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/waitlist/+page.svelte +++ b/components/frontend/src/routes/(app)/my/hackathon/[id]/participants/manage/waitlist/+page.svelte @@ -59,20 +59,42 @@ {countLabel} · nobody here is in the hackathon yet, and only organizers can see them + + {#if !data.waitlistsJoiners && data.waiting.length > 0} + + An invitation to this private hackathon admits its holder on the + spot, so nobody should be queued here. Approving these people is + what puts them in. + + {/if}
+
{#if data.waiting.length === 0}

- Nobody is waiting to join. Approved participants are on the - Participants tab. + {#if data.waitlistsJoiners} + Nobody is waiting to join. Approved participants are on the + Participants tab. + {:else} + Nobody is waiting, and in a private hackathon nobody should + be: an invitation admits its holder outright, so people join + straight onto the Participants tab. + {/if}

{:else} {#each data.waiting as person (person.id)} diff --git a/components/frontend/src/routes/(public)/invite/[token]/+page.server.ts b/components/frontend/src/routes/(public)/invite/[token]/+page.server.ts index b0016886..6d3bf2f0 100644 --- a/components/frontend/src/routes/(public)/invite/[token]/+page.server.ts +++ b/components/frontend/src/routes/(public)/invite/[token]/+page.server.ts @@ -4,7 +4,9 @@ import type { Actions, PageServerLoad } from "./$types" import { createAuthorizedGrpc, publicHackathonClient, + type AuthorizedGrpc, } from "$lib/server/grpc/client" +import { Visibility } from "$lib/server/grpc/generated/hackathon/entities/visibility" import { parseAnswers, questionRows, @@ -25,17 +27,27 @@ import type { CustomSession } from "../../../../auth.d" // permission check at all and serves anonymous callers — so demanding a session // first would add a login wall in front of information the link already grants. // -// **Redeeming grants visibility, not membership.** `Join` writes a waitlisted -// row and the organiser still confirms it, so a link forwarded beyond the people -// it was meant for cannot insert a stranger into the roster. +// **Redeeming a private hackathon's link grants membership outright.** `Join` +// confirms the joiner itself when the hackathon is private: the invitation is +// the organizer's decision about who takes part, and requiring a second +// confirmation left the invitee holding no role and therefore unable to see the +// event at all. The flip side is that the link *is* admission — a link forwarded +// beyond the people it was meant for lets a stranger in, and revoking it or +// removing the participant are the controls, both after the fact. // -// This page is also where somebody comes *back* to. A waitlisted participant in -// a private hackathon holds no `hackathon:read` — that arrives with the `Member` -// role on approval — so the event is filtered out of `List` -// (`hackathon_service.go:1473`) and appears nowhere on their dashboard. Until -// they are approved, this link is the only trace of what they asked for, which -// is why `alreadyParticipant` gets a real state on screen rather than a silent -// redirect somewhere emptier. +// A public hackathon reached through a link still waitlists, so `autoApproves` +// below decides which of the two the page describes. Invites are not restricted +// to private hackathons (`CreateInvite` performs no visibility check), so this +// cannot be assumed from the route. +// +// This page is also where somebody comes *back* to, which still matters for the +// public case: a waitlisted participant holds no `Member` role, so a private +// hackathon they are waiting in is filtered out of `List` +// (`hackathon_service.go:1473`) and appears nowhere on their dashboard. That is +// now only reachable for somebody waitlisted before auto-approval existed, or +// whose confirmation half-failed — and it is exactly why `alreadyParticipant` +// still gets a real state on screen rather than a silent redirect somewhere +// emptier. interface Preview { hackathonId: string @@ -44,6 +56,9 @@ interface Preview { startsAt?: Date endsAt?: Date status: number + /** Whether `Join` confirms on the spot here, which it does for a private + * hackathon. Decides whether this page offers a place or asks for one. */ + autoApproves: boolean questions: QuestionRow[] alreadyParticipant: boolean } @@ -60,11 +75,40 @@ function authorizedFor(session: CustomSession | null) { : undefined } +/** Ask `PreviewInvite` — as the caller when there is one, anonymously otherwise. + * + * Who asks decides one field. The RPC performs no permission check and serves + * anonymous callers, but it fills `already_participant` by looking the *caller* + * up (`hackathon_service.go:454`), so an anonymous preview always reports false + * — which is how somebody a private hackathon had just admitted was told + * "You're on the list" on the way back to this page. + * + * A dead access token must not cost a public page its content, so an auth + * refusal falls back to the anonymous call: everything but that one field is + * identical, and the page's whole point is being readable before signing in. + * `usableSession` already screens out the refusal Auth.js reports, but a token + * can also lapse between refreshes, and the backend answers that with INTERNAL + * rather than UNAUTHENTICATED — see `TODO(backend: jwt-error-codes)` below. + */ +function askPreview(token: string, grpc?: AuthorizedGrpc) { + if (!grpc) return publicHackathonClient().previewInvite({ token }) + + return grpc.hackathon.previewInvite({ token }).catch((e) => { + if ( + e instanceof ClientError && + (e.code === Status.UNAUTHENTICATED || e.code === Status.INTERNAL) + ) { + return publicHackathonClient().previewInvite({ token }) + } + throw e + }) +} + /** Exchange the token for what the page renders. */ -async function preview(token: string): Promise { +async function preview(token: string, grpc?: AuthorizedGrpc): Promise { let res try { - res = await publicHackathonClient().previewInvite({ token }) + res = await askPreview(token, grpc) } catch (e) { if (e instanceof ClientError) { // One answer for all four dead cases, because the backend gives one: @@ -97,18 +141,22 @@ async function preview(token: string): Promise { startsAt: res.hackathon.startsAt, endsAt: res.hackathon.endsAt, status: res.hackathon.status as number, + autoApproves: res.hackathon.visibility === Visibility.VISIBILITY_PRIVATE, questions: questionRows(res.questions), alreadyParticipant: res.alreadyParticipant, } } export const load: PageServerLoad = async (event) => { - const p = await preview(event.params.token) const session = (await event.locals.auth()) as CustomSession | null // A stale session counts as signed out here: the page then offers the sign-in // button, which is the one control that fixes it. Offering "Request a place" // to somebody holding a dead token is how this page produced a 500. const signedIn = usableSession(session) + // Before the preview, not after: the session is what decides who asks, and + // asking anonymously is what made `alreadyParticipant` below meaningless. + const grpc = authorizedFor(session) + const p = await preview(event.params.token, grpc) // Whether an existing participant has been approved yet, derived rather than // asked: `PreviewInvite` reports only *that* somebody holds a participant row, @@ -117,16 +165,13 @@ export const load: PageServerLoad = async (event) => { // exactly what approval grants — so its presence in their own list is the // answer, and it costs one call nobody else on this page makes. let approved = false - if (signedIn && p.alreadyParticipant) { - const grpc = authorizedFor(session) - if (grpc) { - approved = await grpc.hackathon - .list({ statusFilter: [] }) - .then((r) => r.hackathons.some((h) => h.id === p.hackathonId)) - // A failure here costs the link into the event, not the page: they are - // on the list either way, and that is the part they came to read. - .catch(() => false) - } + if (signedIn && p.alreadyParticipant && grpc) { + approved = await grpc.hackathon + .list({ statusFilter: [] }) + .then((r) => r.hackathons.some((h) => h.id === p.hackathonId)) + // A failure here costs the link into the event, not the page: they are + // on the list either way, and that is the part they came to read. + .catch(() => false) } return { @@ -141,6 +186,7 @@ export const load: PageServerLoad = async (event) => { }, questions: p.questions, alreadyParticipant: p.alreadyParticipant, + autoApproves: p.autoApproves, approved, signedIn, } @@ -164,10 +210,25 @@ export const actions: Actions = { // Re-read the questions rather than trusting the form: the answers are // parsed against them, and an organiser may have changed the form while this // page sat open in somebody's mail client for a week. - const p = await preview(event.params.token) + const p = await preview(event.params.token, grpc) const answers = parseAnswers(await event.request.formData(), p.questions) try { + // Provision the platform user before joining. `hooks.server.ts` does this + // (`:182`) for **protected** routes only, and this route is public on + // purpose — so somebody who signs in *from the invitation* and accepts it + // on the spot reaches `Join` holding a Keycloak account and no `users` + // row. `Join` answers that with NOT_FOUND (`hackathon_service.go:605`), + // which the branch below reports as an invalid invitation: exactly how a + // live link looked broken to the one person it was written for, somebody + // whose first ever visit to the platform is this page. + // + // `Register` is idempotent — it returns the existing user, syncing the + // profile fields Keycloak holds — so this is safe on every join rather + // than only a first one, and it needs no "have they registered?" call in + // front of it. + await grpc.user.register({}) + await grpc.hackathon.join({ hackathonId: p.hackathonId, answers, diff --git a/components/frontend/src/routes/(public)/invite/[token]/+page.svelte b/components/frontend/src/routes/(public)/invite/[token]/+page.svelte index 85c645eb..418acc84 100644 --- a/components/frontend/src/routes/(public)/invite/[token]/+page.svelte +++ b/components/frontend/src/routes/(public)/invite/[token]/+page.svelte @@ -13,6 +13,20 @@ // Either they just asked, or they had already asked before this visit. The // page reads the same both ways: what matters is that they are on the list. const onTheList = $derived(Boolean(form?.joined) || data.alreadyParticipant); + // A private hackathon confirms the joiner in `Join`, so following this link + // is joining rather than applying, and the copy has to say which. Not + // assumable from the route: an invite can be minted for a public hackathon + // too, and that one still goes to the waitlist. + const admitsOnJoin = $derived(data.autoApproves); + // A private hackathon confirms its joiners in `Join`, so this combination + // should not exist: on the list, yet not confirmed. It means the backend's + // auto-approval half-failed (it logs and lets the join stand). Joining again + // re-runs the confirmation, which is idempotent, so this is the retry. + // + // `data.approved` comes from a best-effort lookup that falls back to false, + // so a failed one shows this to somebody already in. Pressing Join then is + // harmless — the backend skips a participant who is not waiting. + const needsRetry = $derived(onTheList && !data.approved && admitsOnJoin); const hasMandatory = $derived(data.questions.some((q) => q.mandatory)); // Back to this very link after Keycloak, not to the dashboard: a private @@ -32,6 +46,30 @@ + +{#snippet joinForm(label: string)} +
+ {#if data.questions.length > 0} +
+ A few questions first + {#each data.questions as question (question.id)} + + {/each} + {#if hasMandatory} +

+ + Required. +

+ {/if} +
+ {/if} + +
+{/snippet} +
You've been invited @@ -58,10 +96,19 @@
{#if onTheList} -

You're on the list

+ redirect. A confirmed member gets the link into the event + itself; somebody still waiting holds no role, so the event is + filtered out of every list they can see and this link is their + only way back to it. --> +

+ {#if data.approved} + You're in + {:else if needsRetry} + Almost in + {:else} + You're on the list + {/if} +

{#if data.approved}

Your place is confirmed. The event is on your dashboard now. @@ -72,6 +119,12 @@ > Open {h.name} + {:else if needsRetry} +

+ Your place is held, but the last step did not finish — which is why + the event is still hidden from you. Joining again completes it. +

+ {@render joinForm('Finish joining')} {:else}

The organizers review each request and will confirm your place. Until @@ -80,30 +133,16 @@

{/if} {:else if data.signedIn} -

Ask for a place

+

+ {admitsOnJoin ? 'Take your place' : 'Ask for a place'} +

- This puts you on the organizers' list. They decide who takes part. + {admitsOnJoin + ? 'This invitation is your place — accepting it puts you straight in.' + : "This puts you on the organizers' list. They decide who takes part."}

-
- {#if data.questions.length > 0} -
- A few questions first - {#each data.questions as question (question.id)} - - {/each} - {#if hasMandatory} -

- - Required. -

- {/if} -
- {/if} - -
+ {@render joinForm(admitsOnJoin ? 'Join' : 'Request a place')} {:else}

Sign in to continue