diff --git a/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs b/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs index 5a676c9e..c7035402 100644 --- a/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs +++ b/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs @@ -19,7 +19,8 @@ public class CurationDeskPayloadTests CurationDeskWrites.RosterList, CurationDeskWrites.RosterSet, CurationDeskWrites.RosterRetire, CurationDeskWrites.ApplicationApply, CurationDeskWrites.ApplicationMine, CurationDeskWrites.ApplicationWithdraw, CurationDeskWrites.ApplicationList, - CurationDeskWrites.ApplicationDecide, CurationDeskWrites.ApplicationWindow, + CurationDeskWrites.ApplicationDecide, CurationDeskWrites.ApplicationVote, + CurationDeskWrites.ApplicationWindow, }; private const string IngestBody = @@ -66,6 +67,8 @@ private static string ValidBodyFor(CurationDeskWrites.Route route) return "{" + forged + "\"motivation\":\"why\",\"availability\":\"evenings\",\"pick\":\"a post\"}"; if (ReferenceEquals(route, CurationDeskWrites.ApplicationDecide)) return "{" + forged + "\"applicant\":\"bob\",\"state\":\"declined\"}"; + if (ReferenceEquals(route, CurationDeskWrites.ApplicationVote)) + return "{" + forged + "\"applicant\":\"bob\",\"vote\":\"endorse\"}"; if (ReferenceEquals(route, CurationDeskWrites.ApplicationWindow)) return "{" + forged + "\"open\":true}"; return "{" + forged + "\"limit\":5}"; @@ -714,7 +717,7 @@ public void TheApplicantsOwnRoutesCarryNothingTheCallerSent() public void ADecisionNamesTheApplicantInItsOwnField() { var payload = Ok(CurationDeskWrites.ApplicationDecide, - "{\"applicant\":\"bob\",\"state\":\"accepted\",\"role\":\"trial\",\"note\":\"guest curator\"}"); + "{\"applicant\":\"bob\",\"state\":\"accepted\",\"role\":\"curator\",\"note\":\"guest curator\"}"); Assert.Equal(new[] { "username", "applicant", "state", "role", "note" }, payload.Select(kv => kv.Key).ToArray()); Assert.Equal("alice", payload["username"]!.GetValue()); @@ -738,9 +741,127 @@ public void AMalformedDecisionIsRefusedRatherThanTrimmed(string json, string exp [Fact] public void ANeverGrantedRoleCannotArriveThroughAnAcceptance() { - // The seat an acceptance grants is a trial by default and a curator or mod at most. - // Admin runs the desk, so it is not something a form hands out. + // The seat an acceptance grants is a curator, or a mod when an admin says so. + // Admin runs the desk, so it is not something a form hands out. `trial` is gone + // for a different reason: a guest seat is TRAILED and bounded by its term, and an + // untrailed month would be a month of work nothing follows. Assert.DoesNotContain("admin", CurationDeskWrites.ApplicationRoles); + Assert.DoesNotContain("trial", CurationDeskWrites.ApplicationRoles); + } + + // ---- electing a guest curator ------------------------------------------ + + [Fact] + public void AVoteNamesTheApplicantInItsOwnField() + { + var payload = Ok(CurationDeskWrites.ApplicationVote, + "{\"applicant\":\"bob\",\"vote\":\"object\",\"note\":\"farms comments\"}"); + Assert.Equal(new[] { "username", "applicant", "vote", "note" }, + payload.Select(kv => kv.Key).ToArray()); + Assert.Equal("alice", payload["username"]!.GetValue()); + Assert.Equal("bob", payload["applicant"]!.GetValue()); + } + + [Theory] + [InlineData("{\"vote\":\"endorse\"}", "applicant required")] + [InlineData("{\"applicant\":\"Bob\",\"vote\":\"endorse\"}", "applicant required")] + [InlineData("{\"applicant\":\"bob\"}", "invalid vote")] + [InlineData("{\"applicant\":\"bob\",\"vote\":\"yes\"}", "invalid vote")] + [InlineData("{\"applicant\":\"bob\",\"vote\":\"withdraw\"}", "invalid vote")] + [InlineData("{\"applicant\":\"bob\",\"vote\":\"ENDORSE\"}", "invalid vote")] + [InlineData("{\"applicant\":\"bob\",\"vote\":null}", "invalid vote")] + [InlineData("{\"applicant\":\"bob\",\"vote\":true}", "invalid vote")] + public void AMalformedVoteIsRefusedRatherThanTrimmed(string json, string expected) + { + Assert.Equal(expected, Rejected(CurationDeskWrites.ApplicationVote, json)); + } + + [Fact] + public void AStopCanBeLiftedWithoutBecomingAnEndorsement() + { + // Without a third value the only way out of an objection is to endorse, which + // makes somebody who merely stopped objecting add a vote toward the quorum. + Assert.Contains("abstain", CurationDeskWrites.ApplicationVotes); + Assert.Equal(new[] { "username", "applicant", "vote" }, + Ok(CurationDeskWrites.ApplicationVote, "{\"applicant\":\"bob\",\"vote\":\"abstain\"}") + .Select(kv => kv.Key).ToArray()); + } + + [Fact] + public void AVoteNoteIsCappedInRunesNotUtf16Units() + { + // The column counts characters and so does Python's len(). An emoji is two UTF-16 + // units and one rune, so counting the wrong one refuses 250 valid characters. + var emoji = string.Concat(Enumerable.Repeat("\U0001F600", 500)); + Assert.True(Ok(CurationDeskWrites.ApplicationVote, + "{\"applicant\":\"bob\",\"vote\":\"endorse\",\"note\":\"" + emoji + "\"}") + .ContainsKey("note")); + Assert.Equal("invalid note", Rejected(CurationDeskWrites.ApplicationVote, + "{\"applicant\":\"bob\",\"vote\":\"endorse\",\"note\":\"" + emoji + "\U0001F600\"}")); + } + + [Theory] + [InlineData("{\"open\":true,\"quorum\":0}", "quorum must be a whole number from 1 to 50")] + [InlineData("{\"open\":true,\"quorum\":51}", "quorum must be a whole number from 1 to 50")] + [InlineData("{\"open\":true,\"quorum\":true}", "quorum must be a whole number from 1 to 50")] + [InlineData("{\"open\":true,\"quorum\":1.5}", "quorum must be a whole number from 1 to 50")] + [InlineData("{\"open\":true,\"quorum\":\"3\"}", "quorum must be a whole number from 1 to 50")] + // A present null would be copied through the allowlist and read as absent upstream, + // quietly doing nothing while this fence claimed it had checked the number. + [InlineData("{\"open\":true,\"quorum\":null}", "quorum must be a whole number from 1 to 50")] + [InlineData("{\"open\":true,\"term_days\":0}", "term_days must be a whole number from 1 to 365")] + [InlineData("{\"open\":true,\"term_days\":366}", "term_days must be a whole number from 1 to 365")] + public void AMalformedElectionKnobIsRefused(string json, string expected) + { + Assert.Equal(expected, Rejected(CurationDeskWrites.ApplicationWindow, json)); + } + + [Fact] + public void TheKnobsAreOptionalAndTravelWhenTheyAreSent() + { + Assert.Equal(new[] { "username", "open" }, + Ok(CurationDeskWrites.ApplicationWindow, "{\"open\":true}").Select(kv => kv.Key).ToArray()); + Assert.Equal(new[] { "username", "open", "quorum", "term_days" }, + Ok(CurationDeskWrites.ApplicationWindow, "{\"open\":true,\"quorum\":4,\"term_days\":14}") + .Select(kv => kv.Key).ToArray()); + } + + [Theory] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":-1}")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":366}")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":true}")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":null}")] + public void AMalformedSeatTermIsRefused(string json) + { + Assert.Equal("term_days must be a whole number from 0 to 365", + Rejected(CurationDeskWrites.RosterSet, json)); + } + + [Fact] + public void ATermCannotBeAskedForOnASeatThatDoesNotExpire() + { + // The backend refuses this too. The fence exists so the round trip never leaves. + Assert.Equal("a mod seat does not expire", Rejected(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"mod\",\"term_days\":30}")); + Assert.Equal("a admin seat does not expire", Rejected(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"admin\",\"term_days\":1}")); + // 0 says "no term", which is what a permanent role already is, so it stays legal. + Assert.True(Ok(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"mod\",\"term_days\":0}").ContainsKey("term_days")); + Assert.True(Ok(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"trial\",\"term_days\":30}").ContainsKey("term_days")); + } + + [Fact] + public void AZeroTermIsHowASeatIsMadePermanent() + { + // 0 is not "no term given": absent means keep the term the seat has, and 0 is the + // one way to say the seat should stop expiring. So it has to reach the backend. + var payload = Ok(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":0}"); + Assert.Equal(new[] { "username", "curator", "role", "term_days" }, + payload.Select(kv => kv.Key).ToArray()); + Assert.Equal(0, payload["term_days"]!.GetValue()); } [Theory] diff --git a/dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs b/dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs index cc1aab61..195cac54 100644 --- a/dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs +++ b/dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs @@ -268,7 +268,9 @@ public static TestClock UseTestClock() yield return ("application-withdraw", PrivateApi.CurationDeskApplicationWithdraw, "{" + code + "}"); yield return ("application-list", PrivateApi.CurationDeskApplicationList, "{" + code + ",\"state\":\"open\"}"); yield return ("application-decide", PrivateApi.CurationDeskApplicationDecide, - "{" + code + ",\"applicant\":\"bob\",\"state\":\"accepted\",\"role\":\"trial\"}"); + "{" + code + ",\"applicant\":\"bob\",\"state\":\"accepted\",\"role\":\"curator\"}"); + yield return ("application-vote", PrivateApi.CurationDeskApplicationVote, + "{" + code + ",\"applicant\":\"bob\",\"vote\":\"endorse\"}"); yield return ("application-window", PrivateApi.CurationDeskApplicationWindow, "{" + code + ",\"open\":false,\"message\":\"back soon\"}"); } diff --git a/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs b/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs index 4a3f1c4d..ed7bf078 100644 --- a/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs +++ b/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs @@ -477,6 +477,10 @@ public static Task CurationDeskApplicationList(HttpContext ctx) => public static Task CurationDeskApplicationDecide(HttpContext ctx) => ServeDeskWrite(ctx, CurationDeskWrites.ApplicationDecide); + // POST /private-api/curation-desk/application-vote + public static Task CurationDeskApplicationVote(HttpContext ctx) => + ServeDeskWrite(ctx, CurationDeskWrites.ApplicationVote); + // POST /private-api/curation-desk/application-window public static Task CurationDeskApplicationWindow(HttpContext ctx) => ServeDeskWrite(ctx, CurationDeskWrites.ApplicationWindow); @@ -815,12 +819,26 @@ public static readonly (string Key, int Max)[] ApplicationAnswers = public static readonly IReadOnlySet ApplicationDecisions = new HashSet { "shortlisted", "accepted", "declined" }; /// Roles an acceptance may grant. `admin` is deliberately absent: desk keys are - /// not handed out by a form, and the backend refuses it too. + /// not handed out by a form, and the backend refuses it too. `trial` is absent as well: + /// a guest seat is trailed and bounded by its TERM, not by an untrailed role, and the + /// backend's APPLICATION_ROLES no longer carries it either. public static readonly IReadOnlySet ApplicationRoles = - new HashSet { "trial", "curator", "mod" }; + new HashSet { "curator", "mod" }; + /// How a reviewer may answer an application. An objection is a stop, not a + /// veto: upstream it keeps the automatic grant shut and leaves the case to an admin. + /// `abstain` is how a stop is lifted, so that stopping objecting does not force + /// somebody to endorse instead. + public static readonly IReadOnlySet ApplicationVotes = + new HashSet { "endorse", "object", "abstain" }; public const int MaxApplicationNoteLength = 500; public const int MaxApplicationMessageLength = 200; public const int MaxApplicationListLimit = 200; + public const int MaxApplicationQuorum = 50; + public const int MaxApplicationTermDays = 365; + /// Roles a seat term may expire. A term on an admin or a mod means nothing, + /// and the backend's expiry reads the role as well as the clock for the same reason. + public static readonly IReadOnlySet TermRoles = + new HashSet { "curator", "trial" }; /// /// Views the roster feed takes: the public ones plus `excluded`, which is @@ -877,8 +895,13 @@ public static readonly (string Key, int Max)[] ApplicationAnswers = /// public static readonly Route RosterList = new("curation/desk/roster/list", Array.Empty()); + /// + /// `term_days` is tri-state upstream: absent keeps the seat's term, 0 makes it + /// permanent, n restarts the clock. Absent is not "make permanent", so a note being + /// corrected here cannot quietly turn a guest seat into a standing one. + /// public static readonly Route RosterSet = new("curation/desk/roster/set", - new[] { "curator", "role", "rules", "note" }); + new[] { "curator", "role", "rules", "note", "term_days" }); public static readonly Route RosterRetire = new("curation/desk/roster/retire", new[] { "curator" }); @@ -922,8 +945,20 @@ public static readonly (string Key, int Max)[] ApplicationAnswers = public static readonly Route ApplicationDecide = new("curation/desk/applications/decide", new[] { "applicant", "state", "role", "note" }); + /// + /// One reviewer's line on one application. `applicant` names the account being voted + /// on for the same reason `decide` does: `username` carries the validated caller and + /// Build() refuses a client's version of it. + /// + public static readonly Route ApplicationVote = new("curation/desk/applications/vote", + new[] { "applicant", "vote", "note" }); + + /// + /// `quorum` and `term_days` are optional here and absent means "leave as they are" + /// upstream, not "reset": this route is called every time the message is reworded. + /// public static readonly Route ApplicationWindow = new("curation/desk/applications/window", - new[] { "open", "message" }); + new[] { "open", "message", "quorum", "term_days" }); /// /// The upstream body: the validated username plus the route's whitelisted @@ -1230,6 +1265,22 @@ private static void Truncate(JsonObject payload, string key, int max) } return null; } + if (ReferenceEquals(route, ApplicationVote)) + { + if (body.Str("applicant") is not { } voted || !HiveNames.IsAccountName(voted)) + { + return "applicant required"; + } + var ballot = RequireOneOf(body, "vote", ApplicationVotes); + if (ballot != null) return ballot; + if (body.TryGetPropertyValue("note", out var voteNote) && voteNote is not null + && (body.Str("note") is not { } voteText + || voteText.EnumerateRunes().Count() > MaxApplicationNoteLength)) + { + return "invalid note"; + } + return null; + } if (ReferenceEquals(route, ApplicationWindow)) { if (body.Field("open")?.GetValueKind() is not (JsonValueKind.True or JsonValueKind.False)) @@ -1242,7 +1293,12 @@ private static void Truncate(JsonObject payload, string key, int max) { return "invalid message"; } - return null; + // Both knobs are checked when PRESENT, null included: CopyIfPresent forwards a + // null through the allowlist, and a `"quorum": null` reaching the desk would + // read as absent there, silently doing nothing while this fence claimed every + // quorum it passes is a number in range. + return RequireWholeNumber(body, "quorum", 1, MaxApplicationQuorum) + ?? RequireWholeNumber(body, "term_days", 1, MaxApplicationTermDays); } if (ReferenceEquals(route, RosterSet)) { @@ -1261,6 +1317,19 @@ private static void Truncate(JsonObject payload, string key, int max) { return "invalid note"; } + // 0 is allowed and MEANS something here: it is how a guest seat is made + // permanent. The backend reads absent as "keep the term it has". + var termError = RequireWholeNumber(body, "term_days", 0, MaxApplicationTermDays); + if (termError != null) return termError; + // A term only expires a seat that may carry one, so asking for a term on a + // permanent role is a value the backend would only reject. 0 stays legal on + // any role: it says "no term", which is what a permanent role already is. + if (body.Field("term_days")?.GetValueKind() is JsonValueKind.Number + && body.Field("term_days")!.GetValue() > 0 + && body.Str("role") is { } seat && !TermRoles.Contains(seat)) + { + return $"a {seat} seat does not expire"; + } // A PRESENT `rules` must be an object, null included. `CopyIfPresent` forwards // a null through the allowlist, and the fence should say what the contract says. if (body.TryGetPropertyValue("rules", out var rules)) @@ -1347,6 +1416,23 @@ private static void CopyIfPresent(JsonObject target, JsonObject body, string key private static string? RequireOneOf(JsonObject body, string key, IReadOnlySet allowed) => body.Str(key) is { } value && allowed.Contains(value) ? null : $"invalid {key}"; + + /// + /// A PRESENT whole number in range, null included. Absent is fine and means the caller + /// is not setting it. A fractional number fails here because TryGetValue<int> + /// refuses one, and `true` fails because it is not JsonValueKind.Number, which matters: + /// every one of these knobs decides how a roster seat is granted or how long it lasts. + /// + private static string? RequireWholeNumber(JsonObject body, string key, int min, int max) + { + if (!body.TryGetPropertyValue(key, out var node)) return null; + if (node is not JsonValue value || value.GetValueKind() is not JsonValueKind.Number + || !value.TryGetValue(out var number) || number < min || number > max) + { + return $"{key} must be a whole number from {min} to {max}"; + } + return null; + } } /// diff --git a/dotnet/EcencyApi/Handlers/Routes.cs b/dotnet/EcencyApi/Handlers/Routes.cs index 2f98a532..e954644b 100644 --- a/dotnet/EcencyApi/Handlers/Routes.cs +++ b/dotnet/EcencyApi/Handlers/Routes.cs @@ -199,6 +199,7 @@ public static void Map(WebApplication app) app.MapPost("/private-api/curation-desk/application-withdraw", PrivateApi.CurationDeskApplicationWithdraw); app.MapPost("/private-api/curation-desk/application-list", PrivateApi.CurationDeskApplicationList); app.MapPost("/private-api/curation-desk/application-decide", PrivateApi.CurationDeskApplicationDecide); + app.MapPost("/private-api/curation-desk/application-vote", PrivateApi.CurationDeskApplicationVote); app.MapPost("/private-api/curation-desk/application-window", PrivateApi.CurationDeskApplicationWindow); // ---- SSR RPC cache (internal, header-gated; see SsrRpc.cs) ---- diff --git a/dotnet/parity/driver.py b/dotnet/parity/driver.py index 4faea61e..5c6836e4 100644 --- a/dotnet/parity/driver.py +++ b/dotnet/parity/driver.py @@ -263,7 +263,9 @@ def norm_body(text): "recommend-meta", "recommendation-dismiss", "ingest", # Guest curator applications, added with the desk's apply page. "application-apply", "application-mine", "application-withdraw", - "application-list", "application-decide", "application-window") + "application-list", "application-decide", "application-window", + # Electing them: the roster votes, a quorum grants the seat. + "application-vote") for case in ("min", "pop", "badcode") ]