Skip to content

feat: serve Vote and Subscription - #26

Open
mattdjenkinson wants to merge 1 commit into
feat/postqueryfrom
feat/votes
Open

mattdjenkinson wants to merge 1 commit into
feat/postqueryfrom
feat/votes

Conversation

@mattdjenkinson

Copy link
Copy Markdown
Contributor

Serves Vote and Subscription. Closes #7.

Stacked on #25 (PostQuery), which is on #23, #22 and #16. GitHub retargets it as those merge.

What it does

  • Ownership: votes and subscriptions belong to a user and are named after the post, so there's one of each per user per post. Users create, read and delete their own in the user context. Staff list everyone's in the platform context. The store now sets a read-only feedback.miloapis.com/owner annotation on user-owned objects so staff can see whose is whose; kubectl get votes -o wide shows it. It is never stored and clients can't set it.
  • Rules:
    • voting needs a verified email, using the iam.miloapis.com/emailVerified claim Milo puts on human identities. As in Milo's admission plugin, identities without the claim pass.
    • The post must exist, be visible, and not be merged.
    • Spec is immutable.
    • Staff (platform context) and organization contexts can't vote.
  • Side effects, in one transaction:
    • creating a vote subscribes the voter (reason: voted) unless they already follow the post;
    • creating or deleting a vote recomputes the post's votes and bumps its resourceVersion;
    • unvoting keeps the subscription.
    • Users who follow a post themselves always get reason: manual; the other reasons are set by the server.
  • Voter lookups: organizations (OrganizationMembership by spec.userRef.name) and the linked GitHub username (UserIdentity by status.userUID) come from Milo through --milo-kubeconfig. They're best effort with a 3s timeout: if Milo can't answer, the vote is still recorded. Neither is public.
  • Dedupe: votes.linked counts voters whose linked GitHub username is among the 👍 reactions sync reports in the new votes.githubReactions, and total = datum + github - linked. This follows the product decision that matching is by login only. linked never exceeds the reaction count, in case the list lags the count.
  • Trending: Σ 0.5^(age_days / 7) over votes and 👍 reactions that have a time, counting linked people once. It's recomputed on every vote and post write and refreshed hourly as votes age, rounded so the hourly refresh doesn't rewrite posts for float noise.
  • Merges: when sync sets the new status.mergedInto, the post's votes and subscriptions move to the target. A user with both keeps the target's, and both posts are re-tallied, all in the same transaction.

API changes

  • Post:
    • new status.votes.linked (server-owned);
    • new status.votes.githubReactions (sync-owned, login and at);
    • new status.mergedInto (sync-owned, validated as a post name other than itself).
  • Vote: new status.githubLogin.
  • Flags: --milo-kubeconfig replaces --search-kubeconfig from feat: serve PostQuery #25, since search and the voter lookups both talk to Milo.

Bug found while testing

Aggregated APIs receive user extras as request headers, and the apiserver lowercases their keys (headerrequest/requestheader.go). So the claim arrives as iam.miloapis.com/emailverified, and an exact-match lookup let unverified users vote. The kind run caught this, and a test now covers the lowercase key. The parent-type and parent-name keys are already lowercase, so they're unaffected.

Not in this PR

  • Sync writing reactions and mergedInto: sync isn't built yet. It needs to send 👍 reactors with times when the count changes, and set mergedInto when an issue is closed as a duplicate of a tracked one.
  • Milo IAM for votes and subscriptions, and permission for the apiserver to list memberships and identities across users: Identity, contexts and authorization #10.
  • Public voter avatars (showPublicVotes): not part of Vote and Subscription #7.

Testing

  • Postgres integration tests:
    • counts, subscribe-on-vote and resourceVersion bump;
    • every rejection: name mismatch, bad source, missing, hidden or merged post, platform and organization contexts, unverified, empty or lowercase claim, spec change;
    • lookups recorded, forged status dropped, lookup failure tolerated;
    • user versus staff lists, owner annotation and field selectors;
    • dedupe and trending, including sync being unable to write server-owned fields, hourly decay, and a no-op second refresh;
    • merge with overlapping voters;
    • subscriptions.
  • Unit tests: tally edge cases and the Milo client.
  • Kind cluster:
    • votes and the voted subscription;
    • duplicate vote refused;
    • unverified refused, after the fix above;
    • staff -o wide shows owners;
    • staff vote refused;
    • the merge moved Bob's vote, dropped Alice's duplicate, moved both subscriptions and re-tallied both posts;
    • the trending sort in PostQuery reflected the votes.

Votes and subscriptions belong to a user and are named after the post, so
there is one of each per user per post. Staff list everyone's in the
platform context, with the owner in a read-only annotation the store sets.

- Voting needs a verified email (Milo's emailVerified claim, matched
  case-insensitively because aggregated APIs get extras through headers)
  and a visible, unmerged post.
- A vote subscribes the voter and recomputes the post's votes in the same
  transaction. Votes record the voter's organizations and linked GitHub
  username from Milo, best effort.
- votes.total subtracts voters whose linked GitHub username is among the
  thumbs-up reactions sync reports; trendingScore is the 7-day half-life
  sum over votes and timed reactions. Both are recomputed on every vote and
  post write, and trending hourly.
- When sync sets status.mergedInto, votes and subscriptions move to the
  target post.
- New fields: PostVotes.linked and githubReactions, PostStatus.mergedInto,
  VoteStatus.githubLogin.
- --milo-kubeconfig replaces --search-kubeconfig and is also used for the
  voter lookups.

Part of datum-cloud/enhancements#943. Closes #7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant