feat(flags): add comment flags#1140
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements per-board flag/flair support end-to-end: flag metadata and lookups, selection UI in PostForm/ReplyModal, conversion to challenge answers and publish payloads, parsing of flair sources into renderable view models, UI rendering in posts, store/hook plumbing to persist flags through publishing, challenge modal sequencing, and i18n additions for the "flag" label. ChangesBoard Flags & Flairs System
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/__tests__/comment-flags.test.ts (1)
1-104:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRun
yarn testsuccessfully (use Yarn 4 via Corepack) and share the results
yarn testcurrently fails due topackageManager: yarn@4.13.0vs global Yarn1.22.22- Enable Corepack (
corepack enable) and rerunyarn test, then include the output/exit status in the PR🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/__tests__/comment-flags.test.ts` around lines 1 - 104, The test suite fails locally because global Yarn is v1 while the repo specifies yarn@4; enable Corepack and run the tests with the right Yarn, then paste results and exit code into the PR: run corepack enable (so Corepack can shim Yarn 4), then run yarn test (which will use the declared packageManager and execute the tests covering getAuthorFlagViewModels, getCommentFlagFlairs, and getAuthorFlagFlairs); copy the full terminal output and the final exit status into the PR so CI reviewers can verify the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/post-form/post-form.tsx`:
- Around line 302-307: The table header and aria-label currently hardcode "Flag"
instead of using the i18n helper; update the hardcoded strings to use the
translation function (e.g., t('flag') or an appropriate key) so the header text
and the select's aria-label go through i18n. Locate the fragment around the
PostForm component where the <td>Flag</td> and the select with aria-label='Flag'
(near symbols flagOptions and styles.flagSelect) are rendered and replace both
occurrences with the translated string via the existing t(...) call used
elsewhere in this component.
In `@src/components/reply-modal/reply-modal.tsx`:
- Line 522: The select element currently hardcodes aria-label='Flag'; replace
that literal with a localized string using your app's i18n utility (e.g.,
t('reply.flag') or intl.formatMessage({id:'reply.flag', defaultMessage:'Flag'}))
so the accessibility label is translated; update the select at the element where
key={flagOptions.map((option) => option.value).join('|')} ref={flagRef}
defaultValue={...} to use the localized value and add the corresponding
translation key/entry (reply.flag) to your translation files.
In `@src/lib/comment-flag-selection.ts`:
- Around line 38-40: The getDirectoryCode helper incorrectly uses the nullish
coalescing operator so an empty trimmed directoryCode ('') prevents falling back
to parsing the title; update getDirectoryCode to treat an empty/whitespace-only
directoryCode as absent by checking the trimmed value's truthiness (e.g.,
compute const code = directory?.directoryCode?.trim().toLowerCase(); return code
? code : directory?.title?.match(/^\/([^/]+)\//)?.[1]?.toLowerCase()) so the
title parsing is used when directoryCode is empty; reference getDirectoryCode,
directory?.directoryCode?.trim().toLowerCase(), and directory?.title?.match(...)
when locating the change.
In `@src/lib/comment-flags.ts`:
- Around line 117-121: Normalize the parsed token before doing literal
comparisons: call normalizeBoardFlagKind(kindValue) (or otherwise
lowercase/normalize kindValue) into a local variable (e.g., normalizedKind) and
use that when checking for 'country' or 'geo' and when returning the kind;
update both the block that currently checks kindValue === 'country' || kindValue
=== 'geo' (which returns { type: 'country', code }) and the later similar block
at lines 127-131 to use the normalizedKind variable instead of the raw kindValue
so mixed/upper-case inputs are handled correctly.
---
Outside diff comments:
In `@src/lib/__tests__/comment-flags.test.ts`:
- Around line 1-104: The test suite fails locally because global Yarn is v1
while the repo specifies yarn@4; enable Corepack and run the tests with the
right Yarn, then paste results and exit code into the PR: run corepack enable
(so Corepack can shim Yarn 4), then run yarn test (which will use the declared
packageManager and execute the tests covering getAuthorFlagViewModels,
getCommentFlagFlairs, and getAuthorFlagFlairs); copy the full terminal output
and the final exit status into the PR so CI reviewers can verify the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7782c241-099a-47bc-859f-0d3a5fe09436
⛔ Files ignored due to path filters (3)
public/assets/icons/flags-2.pngis excluded by!**/*.pngpublic/assets/icons/flags-pony.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (26)
package.jsonsrc/components/challenge-modal/__tests__/challenge-modal.test.tsxsrc/components/challenge-modal/challenge-modal.tsxsrc/components/post-author-flags.tsxsrc/components/post-desktop/post-desktop.tsxsrc/components/post-form/__tests__/post-form.test.tsxsrc/components/post-form/post-form.module.csssrc/components/post-form/post-form.tsxsrc/components/post-mobile/post-mobile.tsxsrc/components/reply-modal/__tests__/reply-modal.test.tsxsrc/components/reply-modal/reply-modal.module.csssrc/components/reply-modal/reply-modal.tsxsrc/hooks/__tests__/use-directories.test.tsxsrc/hooks/use-directories.tssrc/hooks/use-publish-post.tssrc/hooks/use-publish-reply.tssrc/lib/__tests__/board-flags.test.tssrc/lib/__tests__/comment-flag-selection.test.tssrc/lib/__tests__/comment-flags.test.tssrc/lib/board-flags.tssrc/lib/comment-flag-selection.tssrc/lib/comment-flags.tssrc/stores/__tests__/publish-stores.test.tssrc/stores/use-publish-post-store.tssrc/stores/use-publish-reply-store.tssrc/views/post/post.module.css
|
Addressed the valid review findings in commit acf28a8: removed the temporary flags test community fallback, localized the flag labels, fixed blank directoryCode fallback behavior, normalized text flag kind parsing, and removed the stale reply-modal CSS-module reference.\n\nLocal verification after the changes:\n- corepack yarn vitest run src/hooks/tests/use-directories.test.tsx src/lib/tests/comment-flag-selection.test.ts src/lib/tests/comment-flags.test.ts src/components/post-form/tests/post-form.test.tsx src/components/reply-modal/tests/reply-modal.test.tsx\n- corepack yarn test --run\n- corepack yarn lint\n- corepack yarn type-check\n- corepack yarn doctor\n- corepack yarn build\n\nReact Doctor still reports existing repo warnings outside this diff; I did not treat those as blockers for this PR. |
|
Addressed the latest valid review findings in d612dcd: non-flag publishes now explicitly clear stale flag flairs/challenge requests from one-shot publish options, and the unused standalone challenge-request helper was removed. Local verification: corepack yarn install --immutable, corepack yarn knip, corepack yarn lint, corepack yarn type-check, corepack yarn test --run, corepack yarn doctor, and corepack yarn build. One full test run briefly hit a settings-modal assertion that passed in isolation and on rerun; the final full-suite rerun passed. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d612dcd. Configure here.
| link: string | undefined; | ||
| flairs: Comment['flairs'] | undefined; | ||
| spoiler: boolean | undefined; | ||
| challengeRequest: Record<string, unknown> | undefined; |
There was a problem hiding this comment.
Redundant challengeRequest top-level state field never read
Low Severity
The challengeRequest field is added as a top-level state property in both use-publish-post-store and use-publish-reply-store, but no hook ever reads it from the top level. Both use-publish-post.ts and use-publish-reply.ts access the challenge request exclusively via publishCommentOptions.challengeRequest. This creates dead state that's written on every store update and reset but never consumed, unlike flairs which is read from both the top-level field and publishCommentOptions. The asymmetry risks confusing future contributors into thinking it's a meaningful separate field.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d612dcd. Configure here.
|
Latest Cursor Bugbot low-severity note about the top-level store challengeRequest field is valid cleanup, but not a merge blocker: publishing reads from publishCommentOptions.challengeRequest, and the shipped correctness bug was stale flag data leaking into non-flag publishes, which is fixed and covered by the new one-shot post/reply tests. I’m treating that store-shape cleanup as follow-up rather than reopening CI again for a non-behavioral simplification. |
1 participant
Summary
hasFlagsand forward selected flags into publish optionsVerification
corepack yarn vitest run src/lib/__tests__/comment-flags.test.ts src/lib/__tests__/comment-flag-selection.test.ts src/stores/__tests__/publish-stores.test.ts src/components/post-form/__tests__/post-form.test.tsx src/components/reply-modal/__tests__/reply-modal.test.tsx src/components/challenge-modal/__tests__/challenge-modal.test.tsxcorepack yarn lintcorepack yarn type-checkcorepack yarn buildcomment["5chan"].flagNotes
Note
Medium Risk
Changes the publish and challenge-completion paths plus a protocol dependency bump; mistakes could break posting or flag verification, but scope is feature/UI rather than core auth.
Overview
This PR adds 4chan-style author flags end to end: boards with
features.hasFlagsget a flag dropdown on the post form and reply modal, selections map to comment flairs andbitsocial-flags:5chan:…challenge answers, and desktop/mobile posts render sprite flags from signedcomment["5chan"].flag(with fallbacks). New helpers cover country / political / pony flag catalogs, parsing, and publish options; publish stores/hooks now carryflairsandchallengeRequestand clear stale flag fields when posting without a flag.The challenge modal no longer publishes after the first iframe—it steps through multiple iframe challenges before submitting the full answer array (e.g. spam blocker then flags host).
@pkcprotocol/pkc-jsis bumped to 0.0.37 (with yarn resolutions for gossipsub). Locale files gain aflagstring.findDirectoryByAddressreturnsundefinedwhen nothing matches (no silent fuzzy match after normalized lookup).Reviewed by Cursor Bugbot for commit d612dcd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Localization
Style
Tests
Chores