Skip to content

Hard problems features#2

Open
gaby-frei wants to merge 5 commits into
andotherstuff:mainfrom
gaby-frei:hard-problems-features
Open

Hard problems features#2
gaby-frei wants to merge 5 commits into
andotherstuff:mainfrom
gaby-frei:hard-problems-features

Conversation

@gaby-frei

Copy link
Copy Markdown

Add upvote feature and tag filtering to Hard Problems

  • Approved attendees can toggle-upvote problems; votes persist in the APPROVALS KV
    under upvote:count: and upvote:user:: keys
  • Tag filter bar above the cards with AND logic
  • Problem list redesigned as bordered cards with inline tag pills and upvote button
  • Two new authenticated Worker endpoints: GET /api/hard-problems/upvotes and POST /api/hard-problems/upvote

gaby-frei and others added 5 commits May 28, 2026 11:16
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The upvote endpoints use env.APPROVALS (production KV), not local wrangler
state. The comment was left over from early development and was incorrect.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@NotThatKindOfDrLiz NotThatKindOfDrLiz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes. The upvote feature is close, but this should not merge until the POST auth/body binding, slug validation, and vote storage correctness are fixed. Root npm test passes locally; worker tsc --noEmit currently fails on an existing nostr-tools MessageEvent type issue outside this diff.

mutationFn: async (slug: string) => {
if (!user) throw new Error('Not logged in');
const url = `${API_BASE}/api/hard-problems/upvote`;
const token = await createNip98Token(user, url, 'POST');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This POST auth token is not bound to the request body. createNip98Token only signs URL + method, and the worker verifier does not check a NIP-98 payload hash, so a captured token can be replayed within the 60s window with a different slug/body. Please add body hashing for POST/PUT/DELETE auth tokens and verify the payload tag server-side before shipping this endpoint.

Comment thread worker/src/index.ts
}

const { slug } = body;
if (!slug || typeof slug !== 'string') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please validate slug against the seeded Hard Problems list before constructing KV keys. As written, any approved attendee can write arbitrary upvote:user:* and upvote:count:* keys and grow the APPROVALS namespace. Load insights:hard-problems, derive the allowed slug set, and reject anything else. Stable explicit slugs in the seed would be even better than deriving from headings.

Comment thread worker/src/index.ts
const userVoteKey = `upvote:user:${npub}:${slug}`;
const countKey = `upvote:count:${slug}`;
const existing = await env.APPROVALS.get(userVoteKey);
const current = await env.APPROVALS.get(countKey);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This read-modify-write counter is not safe under concurrent votes. Two attendees upvoting at once can both read the same count and overwrite each other, and the separate count/user writes can desync on partial failure. For this event scale, consider storing only per-vote keys such as upvote:vote:<slug>:<npub> and deriving counts from the vote keys, or move the counter to an atomic primitive like a Durable Object.

{filteredSections.map((section) => {
const slug = slugify(section.heading);
const count = upvotesData?.counts[slug] ?? 0;
const hasVoted = upvotesData?.userVotes.includes(slug) ?? false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can still crash if the API returns { counts } without userVotes, or if an older/malformed cached value has a different shape. Use a normalized array check, e.g. Array.isArray(upvotesData?.userVotes) && upvotesData.userVotes.includes(slug), and apply the same defensive handling in the cache update in useUpvoteProblem.

if (!data) return [];
const seen = new Set<string>();
data.sections.forEach(s => s.tags?.forEach(t => seen.add(t)));
return Object.keys(TAG_COLORS).filter(t => seen.has(t));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drops any tag that exists in the data but is not already listed in TAG_COLORS, so those tags can appear on cards but never be filterable. Build the filter list from the data first, then use TAG_COLORS[tag] only as a color lookup fallback. Also note that the local worker/hard-problems-data.local.json currently has no tags, so this UI will render an empty filter bar unless the seeded production data has diverged.

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.

2 participants