feat: support anonymous replies (storage + masking + tests) #37#39
feat: support anonymous replies (storage + masking + tests) #37#39JoshuaNam wants to merge 6 commits intoCMU-313:mainfrom
Conversation
Pull Request Test Coverage Report for Build 22021919621Details
💛 - Coveralls |
|
hmm the auto-deployment doesn't seem to be working... maybe u need to make a branch in this repo instead of in your fork? |
|
Hmm alright, I'll make the changes and let you know. |
src/posts/summary.js
Outdated
| if (post.anonymous === 1) { | ||
| if (isAdmin) { | ||
| post.isAnonymous = true; |
There was a problem hiding this comment.
why add another anonymous field here?
There was a problem hiding this comment.
Good catch... you're right, isAnonymous is redundant. Admins already have post.anonymous === 1 available directly and no frontend template or client-side JS actually reads isAnonymous. I've removed it from summary.js, topics/posts.js, and api/posts.js and updated the tests to assert on post.anonymous instead.
| anonymous: | ||
| type: number | ||
| description: Whether this post was made anonymously (1 for anonymous, 0 for not) |
There was a problem hiding this comment.
code changes make sense, pretty good
why is anonymous of type number instead of boolean here?
There was a problem hiding this comment.
this is the convention that the other components use
There was a problem hiding this comment.
This follows the existing NodeBB convention for DB-persisted boolean flags. Specifically: deleted, edited, and uid in PostDataObject (same file, lines 215/227), locked/pinned/scheduled in TopicObject.yaml, and banned/email:confirmed in UserObject.yaml all use type: number with 0/1 values. Since anonymous is stored as 0/1 in the DB and returned as-is (not converted to a boolean in processing code), type: number is consistent here.
src/api/posts.js
Outdated
| uid: 0, | ||
| username: 'Anonymous', | ||
| displayname: 'Anonymous', | ||
| userslug: '', | ||
| picture: '', | ||
| signature: '', | ||
| status: 'offline', | ||
| 'icon:text': '?', | ||
| 'icon:bgColor': '#aaa', | ||
| }; | ||
| } |
There was a problem hiding this comment.
where is this code being used in the frontend btw? I see it's being used for the /api/v3/posts/:pid endpoint (see src/routes/write/posts.js), but I can't find the frontend code that calls this endpoint
There was a problem hiding this comment.
You're right that the frontend doesn't call /api/v3/posts/:pid directly for this. The masking in postsAPI.get() works for single-post fetches, but I found a gap: postsAPI.getReplies() (used by the "Show Replies" button via public/src/client/topic/replies.js:17) doesn't apply any anonymous masking. I've fixed this by extracting a shared Posts.anonymizePost() helper and applying it in getReplies() as well, so anonymous authors are masked consistently across all paths.
3 participants
Changes (Issue #29 + #6)
anonymousanonymousflag (0/1) on posts in the databaseisAnonymous=trueflagTesting
Run the full backend test suite:
npm run lint
npm run test
Verify:
anonymous=1uid=0, username"Anonymous")isAnonymous=trueRelated Issues
Resolves #6
Resolves #29