Uh oh!
There was an error while loading. Please reload this page.
feat: anonymous ereferences - #927
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds anonymous eReference support: new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Frontend as UI
participant Controller as ReferenceController
participant Service as ReferenceService
participant DB as Database
Note over Client,DB: Create anonymous reference flow
Client->>Frontend: user submits reference (content, anonymous=true)
Frontend->>Controller: POST /api/references {..., anonymous: true}
Controller->>Service: createReference(data)
Service->>DB: INSERT reference (anonymous=true, authorId)
DB-->>Service: inserted Reference
Service-->>Controller: Reference (author omitted/null)
Controller-->>Frontend: 201 Created (anonymous=true, author=null)
Frontend-->>Client: show confirmation
Note over Client,DB: Retrieve references (anonymous redaction)
Client->>Frontend: GET /api/references
Frontend->>Controller: GET /api/references
Controller->>Service: getAllReferences()
Service->>DB: SELECT references
DB-->>Service: references (with anonymous flags)
Service-->>Controller: mapped references (author null when anonymous)
Controller-->>Frontend: 200 OK (forFrom="Anonymous" where applicable)
Frontend-->>Client: render list with Anonymous badges
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platforms/ereputation/api/src/controllers/ReferenceController.ts (1)
17-37:⚠️ Potential issue | 🟡 MinorValidate
anonymousas a boolean.
req.body.anonymousis forwarded as-is. Non-boolean payloads can silently change or break the stored privacy flag depending on downstream coercion, so this should reject anything excepttrue/false.🛡️ Suggested fix
if (!targetType || !targetId || !targetName || !content) { return res.status(400).json({ error: "Missing required fields" }); } + if (anonymous !== undefined && typeof anonymous !== "boolean") {+ return res.status(400).json({ error: "anonymous must be a boolean" });+ }+ if (numericScore && (numericScore < 1 || numericScore > 5)) { return res.status(400).json({ error: "Numeric score must be between 1 and 5" }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/api/src/controllers/ReferenceController.ts` around lines 17 - 37, The request handler in ReferenceController reads req.body.anonymous and passes it into createReference without type-checking, so validate that anonymous is strictly a boolean (true or false) before using it: check req.body.anonymous is either === true or === false (or absent/undefined) and if present but not a boolean return res.status(400).json({ error: "anonymous must be a boolean" }); then pass anonymous: anonymous ?? false into this.referenceService.createReference; reference the req.body.anonymous parameter, the anonymous local used in the createReference call, and the createReference method to locate where to add the check.
🧹 Nitpick comments (3)
platforms/ereputation/api/package.json (1)
11-11: Quote the variable to handle edge cases.The
$namevariable is unquoted, which could cause issues if someone enters a migration name with spaces (though uncommon). Additionally, the script relies on bash-specific features (read -p), which won't work on Windows without WSL or Git Bash.🛠️ Suggested fix
-"migration:generate": "bash -c 'read -p \"Migration name: \" name && npx typeorm-ts-node-commonjs migration:generate src/database/migrations/$name -d src/database/data-source.ts'",+"migration:generate": "bash -c 'read -p \"Migration name: \" name && npx typeorm-ts-node-commonjs migration:generate \"src/database/migrations/$name\" -d src/database/data-source.ts'",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/api/package.json` at line 11, The migration script in package.json ("migration:generate") uses an unquoted $name and a bash-only read -p, which breaks on Windows and when names contain spaces; update the script to quote the variable (use "$name") and replace the bash prompt with a cross-platform input method (for example a short node one-liner or an npm CLI prompt) that reads the migration name and then calls npx typeorm-ts-node-commonjs migration:generate with the quoted name; reference the "migration:generate" script and the $name variable when making the change.platforms/ereputation/api/src/controllers/DashboardController.ts (1)
87-94: Prefer an allow-list DTO for anonymous activities.This works today, but redacting
authorandauthorIdafter spreading the whole entity makes this endpoint easy to regress the next timeReferencegrows another identity-bearing field. For a privacy-sensitive response, it would be safer to serialize only the dashboard fields you intend to expose.💡 Example direction
- const refData = ref.anonymous- ? { ...ref, author: undefined, authorId: undefined }- : ref;+ const refData = {+ id: ref.id,+ targetType: ref.targetType,+ targetId: ref.targetId,+ targetName: ref.targetName,+ content: ref.content,+ referenceType: ref.referenceType,+ numericScore: ref.numericScore,+ status: ref.status,+ createdAt: ref.createdAt,+ anonymous: ref.anonymous ?? false,+ author: ref.anonymous ? undefined : ref.author,+ }; ... - data: { ...refData, anonymous: ref.anonymous ?? false }+ data: refDataAlso applies to: 103-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/api/src/controllers/DashboardController.ts` around lines 87 - 94, The current logic in DashboardController that builds authorName and refData by spreading the full Reference (symbols: ref, authorName, refData) risks leaking future identity fields; replace the spread-based response with an explicit allow-list DTO: construct a new object containing only the dashboard-safe fields you intend to expose (e.g., id, type, content, createdAt, authorName, any non-identifying metadata) and, when ref.anonymous is true, omit or null out all identity properties (author, authorId, handle, ename, etc.); update both places where refData is created so the endpoint serializes only those listed fields instead of {...ref}.platforms/ereputation/client/client/src/pages/dashboard.tsx (1)
122-165: Remove the dead second reference branch.Lines 124-142 already return for every reference activity, so the later reference check can never execute. Keeping the anonymous modal mapping duplicated in unreachable code makes future changes easy to drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/client/client/src/pages/dashboard.tsx` around lines 122 - 165, The second duplicated reference-check branch inside handleViewActivity is dead code because the first reference branch always returns; remove the entire redundant if-block (the repeated "if (activity.type === 'reference' || activity.activity === 'Reference Provided' || activity.activity === 'Reference Received')" block along with its inner mapping and setReferenceViewModal call) so only the first reference-handling logic remains, leaving the rest of handleViewActivity (calculation/activity handling) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/ereputation/api/src/utils/jwt.ts`:
- Around line 3-5: Update the stale comment above the environment check to
reference the actual env var name used in code: change the comment that
currently says "Fail fast if JWT_SECRET is missing" so it matches the checked
symbol process.env.EREPUTATION_JWT_SECRET and the thrown error message; ensure
the comment explicitly names EREPUTATION_JWT_SECRET to avoid mismatch with the
code in jwt.ts.
In
`@platforms/ereputation/client/client/src/components/modals/reference-modal.tsx`:
- Line 65: The modal's anonymous state (anonymous / setAnonymous) is only reset
inside resetForm(), but the normal close code path(s) do not invoke resetForm(),
so canceling and reopening preserves the previous privacy choice; update the
modal close handlers referenced in this file (the normal close path and the
other close/cancel path around the second block noted) to call resetForm() (or
at minimum call setAnonymous(false) and any other cleanup done by resetForm())
before closing so the anonymous toggle is cleared on close.
---
Outside diff comments:
In `@platforms/ereputation/api/src/controllers/ReferenceController.ts`:
- Around line 17-37: The request handler in ReferenceController reads
req.body.anonymous and passes it into createReference without type-checking, so
validate that anonymous is strictly a boolean (true or false) before using it:
check req.body.anonymous is either === true or === false (or absent/undefined)
and if present but not a boolean return res.status(400).json({ error: "anonymous
must be a boolean" }); then pass anonymous: anonymous ?? false into
this.referenceService.createReference; reference the req.body.anonymous
parameter, the anonymous local used in the createReference call, and the
createReference method to locate where to add the check.
---
Nitpick comments:
In `@platforms/ereputation/api/package.json`:
- Line 11: The migration script in package.json ("migration:generate") uses an
unquoted $name and a bash-only read -p, which breaks on Windows and when names
contain spaces; update the script to quote the variable (use "$name") and
replace the bash prompt with a cross-platform input method (for example a short
node one-liner or an npm CLI prompt) that reads the migration name and then
calls npx typeorm-ts-node-commonjs migration:generate with the quoted name;
reference the "migration:generate" script and the $name variable when making the
change.
In `@platforms/ereputation/api/src/controllers/DashboardController.ts`:
- Around line 87-94: The current logic in DashboardController that builds
authorName and refData by spreading the full Reference (symbols: ref,
authorName, refData) risks leaking future identity fields; replace the
spread-based response with an explicit allow-list DTO: construct a new object
containing only the dashboard-safe fields you intend to expose (e.g., id, type,
content, createdAt, authorName, any non-identifying metadata) and, when
ref.anonymous is true, omit or null out all identity properties (author,
authorId, handle, ename, etc.); update both places where refData is created so
the endpoint serializes only those listed fields instead of {...ref}.
In `@platforms/ereputation/client/client/src/pages/dashboard.tsx`:
- Around line 122-165: The second duplicated reference-check branch inside
handleViewActivity is dead code because the first reference branch always
returns; remove the entire redundant if-block (the repeated "if (activity.type
=== 'reference' || activity.activity === 'Reference Provided' ||
activity.activity === 'Reference Received')" block along with its inner mapping
and setReferenceViewModal call) so only the first reference-handling logic
remains, leaving the rest of handleViewActivity (calculation/activity handling)
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6479698-4373-4b84-a011-b7f2d7ccc141
📒 Files selected for processing (13)
platforms/ereputation/api/package.jsonplatforms/ereputation/api/src/controllers/DashboardController.tsplatforms/ereputation/api/src/controllers/ReferenceController.tsplatforms/ereputation/api/src/database/entities/Reference.tsplatforms/ereputation/api/src/database/migrations/1773718262631-anon-erefs.tsplatforms/ereputation/api/src/services/CalculationService.tsplatforms/ereputation/api/src/services/ReferenceService.tsplatforms/ereputation/api/src/services/VotingReputationService.tsplatforms/ereputation/api/src/utils/jwt.tsplatforms/ereputation/client/client/src/components/modals/reference-modal.tsxplatforms/ereputation/client/client/src/components/modals/reference-view-modal.tsxplatforms/ereputation/client/client/src/pages/dashboard.tsxplatforms/ereputation/client/client/src/pages/references.tsx
Comment on lines
+3
to
+5
| // Fail fast if JWT_SECRET is missing | ||
| if (!process.env.EREPUTATION_JWT_SECRET) { | ||
| throw new Error("JWT_SECRET environment variable is required but was not provided. Please set JWT_SECRET in your environment configuration."); | ||
| throw new Error("EREPUTATION_JWT_SECRET environment variable is required but was not provided. Please set EREPUTATION_JWT_SECRET in your environment configuration."); |
Contributor
There was a problem hiding this comment.
Comment on line 3 is stale.
The comment references JWT_SECRET but the actual check and error message reference EREPUTATION_JWT_SECRET. Consider updating the comment to match:
-// Fail fast if JWT_SECRET is missing+// Fail fast if EREPUTATION_JWT_SECRET is missing📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Fail fast if JWT_SECRET is missing | |
| if(!process.env.EREPUTATION_JWT_SECRET){ | |
| thrownewError("JWT_SECRET environment variable is required but was not provided. Please set JWT_SECRET in your environment configuration."); | |
| thrownewError("EREPUTATION_JWT_SECRET environment variable is required but was not provided. Please set EREPUTATION_JWT_SECRET in your environment configuration."); | |
| // Fail fast if EREPUTATION_JWT_SECRET is missing | |
| if(!process.env.EREPUTATION_JWT_SECRET){ | |
| thrownewError("EREPUTATION_JWT_SECRET environment variable is required but was not provided. Please set EREPUTATION_JWT_SECRET in your environment configuration."); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/ereputation/api/src/utils/jwt.ts` around lines 3 - 5, Update the
stale comment above the environment check to reference the actual env var name
used in code: change the comment that currently says "Fail fast if JWT_SECRET is
missing" so it matches the checked symbol process.env.EREPUTATION_JWT_SECRET and
the thrown error message; ensure the comment explicitly names
EREPUTATION_JWT_SECRET to avoid mismatch with the code in jwt.ts.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
platforms/ereputation/client/client/src/components/modals/reference-modal.tsx (1)
239-244:⚠️ Potential issue | 🟠 MajorAdd
eventSourceto the close-reset effect dependencies to prevent stale SSE connections.If a form submission completes after the modal closes, the
onSuccesshandler will create a new EventSource viastartSSEConnection()after the close-reset effect has already run. Since the effect depends only onopen, it won't re-execute to clean up the newly created EventSource, leaving an active SSE connection while the modal is closed.🔧 Proposed fix
useEffect(() => { if (!open) { resetForm(); } - }, [open]);+ }, [open, eventSource]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/client/client/src/components/modals/reference-modal.tsx` around lines 239 - 244, The effect that resets the form when the modal closes currently depends only on open and can miss cleaning up EventSource instances created after close; update the useEffect that calls resetForm() to also list eventSource in its dependency array and ensure it closes any active SSE by checking and calling eventSource.close() (or invoking the existing SSE cleanup helper) before or when resetForm() runs; reference the useEffect surrounding resetForm(), the eventSource variable/state, and startSSEConnection()/onSuccess handler so the effect will re-run and properly tear down stale SSE connections.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@platforms/ereputation/client/client/src/components/modals/reference-modal.tsx`:
- Around line 239-244: The effect that resets the form when the modal closes
currently depends only on open and can miss cleaning up EventSource instances
created after close; update the useEffect that calls resetForm() to also list
eventSource in its dependency array and ensure it closes any active SSE by
checking and calling eventSource.close() (or invoking the existing SSE cleanup
helper) before or when resetForm() runs; reference the useEffect surrounding
resetForm(), the eventSource variable/state, and startSSEConnection()/onSuccess
handler so the effect will re-run and properly tear down stale SSE connections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b0c6adb-6826-4f13-a889-f09394ea7214
📒 Files selected for processing (2)
platforms/ereputation/api/src/utils/jwt.tsplatforms/ereputation/client/client/src/components/modals/reference-modal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- platforms/ereputation/api/src/utils/jwt.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
allows users to make anonymous ereferences
we still know that the references are being provided by real people so it has no affect on score calculation
we only obfuscate the details on the transport from backend to client, the backend still knows who provided the reference to maintain data integrity
Issue Number
closes#873
Type of change
How the change has been tested
manual
Change checklist
SS
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores