Add community discussions feature#572
Open
arpit2006 wants to merge 3 commits into
Open
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Warning Review limit reached
More reviews will be available in 15 minutes and 29 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR implements a complete community discussion feature, adding file-backed discussion CRUD endpoints, authenticated user interactions (likes/comments), Mongoose schemas, form-based frontend UI, and non-MongoDB fallback authentication to enable deployment without a database. ChangesCommunity Discussion Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant CommunityUI as Community.tsx
participant BackendAPI as Discussion API
participant SessionAuth as Session/Auth
User->>CommunityUI: Load page / filter discussions
CommunityUI->>BackendAPI: GET / with params
BackendAPI->>CommunityUI: Return discussions + categories
User->>CommunityUI: Create/Edit discussion
CommunityUI->>BackendAPI: POST / or PUT /:id
BackendAPI->>CommunityUI: Return created/updated discussion
CommunityUI->>User: Refresh feed, show success
User->>CommunityUI: Like or comment
CommunityUI->>BackendAPI: POST /:id/likes or POST /:id/comments
BackendAPI->>CommunityUI: Return updated discussion
CommunityUI->>User: Update engagement UI
User->>CommunityUI: Attempt protected action (unauthorized)
BackendAPI->>CommunityUI: 401 response
CommunityUI->>User: Show sign-in message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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)
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
Author
|
@mehul-m-prajapati, Requesting review! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/server.js (1)
25-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing fallback or validation for
SESSION_SECRET.
express-sessionrequires a secret to sign session cookies. IfSESSION_SECRETis undefined, the server may fail to start or use an insecure empty string. Add validation or a fallback with a warning.Proposed fix
+if (!process.env.SESSION_SECRET) { + logger.warn('SESSION_SECRET is not set; using insecure default (development only)'); +} app.use(session({ - secret: process.env.SESSION_SECRET, + secret: process.env.SESSION_SECRET || 'insecure-dev-secret-change-me', resave: false, saveUninitialized: false, }));🤖 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 `@backend/server.js` around lines 25 - 29, The session middleware uses process.env.SESSION_SECRET directly which can be undefined; validate and provide a non-empty secret (or abort startup) before calling app.use(session(...)). Add code that reads SESSION_SECRET into a variable, checks it's a non-empty string, and if missing either throw an error/process.exit(1) or generate a secure fallback while logging a clear warning; then pass that validated secret into the session config used in app.use(session({ secret: secretVar, resave: false, saveUninitialized: false })). Ensure the check runs at startup so the session middleware never receives an undefined/empty secret.
🤖 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 `@backend/data/users.json`:
- Around line 3-20: The seeded user file backend/data/users.json contains
real-looking emails and bcrypt password hashes for entries with ids
"a95e96b8-c655-4122-b949-9a9a2e166e96", "7a565adc-7c6b-4254-bd38-874c22d88892",
and "a6775248-35ef-410c-b529-220e350210f2"; replace those with synthetic/demo
data by removing or replacing personal emails with clearly synthetic addresses
(e.g., user+N@example.invalid) and remove actual password hashes, either
replacing them with a null/placeholder password field or generate ephemeral
hashes at bootstrap time (documented in README); ensure usernames remain
non-identifying or anonymized and update any code that depends on hardcoded
hashes to derive credentials at setup instead.
In `@backend/models/Discussion.js`:
- Around line 46-68: The tags, likes, and comments fields on the Discussion
schema can be undefined downstream; update the schema definition for the array
fields (tags, likes, comments) to include defaults of empty arrays so consumers
can safely call .length/.includes/.map; specifically add default: [] to the tags
array element definition, the likes array definition, and the comments array
(which uses CommentSchema) in Discussion.js to guarantee a consistent shape.
In `@backend/routes/auth.js`:
- Around line 26-41: readUsers/writeUsers use a non-atomic read-modify-write on
usersFile causing lost concurrent signups; fix by gating writes with a file lock
or atomic write/rename: in writeUsers (and any signup flow that reads then
writes) acquire a lock on usersFile (e.g., proper-lockfile.lock with retries),
perform fs.writeFile to a temp file followed by fs.rename or write directly
while holding the lock, then always release the lock in a finally block; ensure
ensureUsersFile and any callers of readUsers/writeUsers honor the lock to
prevent concurrent read/modify/write races.
In `@backend/routes/discussions.js`:
- Around line 93-113: The route-level validatePayload is weaker than the shared
validator contract; update the discussions route to reuse or mirror the shared
validators instead of the current ad-hoc checks: replace/augment validatePayload
(and the create/update/comment handlers that call it) to enforce the same
constraints as the shared validator (max title length, max body length, max
category length, max tags count and per-tag length, and tag type checks), or
import and call the shared validation function directly (e.g., use the shared
validator module rather than the local validatePayload) so requests that exceed
counts/lengths are rejected consistently across createDiscussion,
updateDiscussion and addComment handlers. Ensure error objects match the shared
validator shape (field + message) so callers receive consistent responses.
- Around line 21-36: The file-store is vulnerable to lost updates because
multiple handlers call readStore() → mutate → writeStore() concurrently; add a
single-process async mutex to serialize all read-modify-write operations (or
provide an atomic update API) so only one mutation runs at a time. Implement a
locked helper (e.g., updateStore or wrap writeStore) that acquires the mutex,
calls ensureDataFile(), reads/parses dataFile, applies an updater function to
the discussions array, writes the new JSON back, and releases the mutex; update
all callers to use this new locked updater instead of calling
readStore()/writeStore() separately. Keep references: readStore, writeStore,
ensureDataFile, dataFile, and the new updateStore(lock) helper so future
reviewers can find the changes.
- Around line 44-64: getCommunityIdentity currently returns a guest identity
that allows unauthenticated users to perform mutating actions; to fix this, stop
treating guests as authorized for writes by requiring authentication in mutating
handlers. Add an auth check (middleware or inline) that returns 401/403 when
req.user is missing for all endpoints that create/edit/delete/like/comment (the
route handlers for createDiscussion, editDiscussion, deleteDiscussion,
likeDiscussion, addComment or similarly named handlers referenced in the diff),
and keep getCommunityIdentity only for read operations (listing/fetching
discussions). Update routes to use the new auth guard for POST/PUT/DELETE
actions and remove any logic that uses req.session.communityUserId to authorize
writes.
- Around line 25-30: The catch block that swallows JSON.parse errors and returns
[] is dangerous because it masks corruption; in the function that parses raw
(the block using JSON.parse(raw) and checking parsed.discussions) stop treating
parse failures as an empty store—catch the error and rethrow or propagate it
(e.g., throw a new Error including the original error and context) so callers
can handle the failure instead of silently returning an empty array; update any
callers to handle the thrown error or to fail-safe without overwriting existing
discussions.
In `@src/pages/Community/Community.tsx`:
- Around line 125-131: The fetch success path currently updates setDiscussions
and setCategories but doesn't restore auth state after a prior 401; after a
successful response (where setDiscussions(response.data.items || []) and
setCategories(...) are called) call setIsAuthenticated(true) and clear any
previous error (e.g., setError("") or null) so the component reflects restored
authentication; leave the existing catch branch with
axios.isAxiosError(requestError) && requestError.response?.status === 401 to
still setIsAuthenticated(false).
In `@src/pages/Login/Login.tsx`:
- Line 7: The current backendUrl constant in Login.tsx falls back to
"http://localhost:5000", which breaks production clients; update the
initialization of backendUrl so it does not default to a localhost HTTP
address—either require import.meta.env.VITE_BACKEND_URL to be set and throw or
log a clear error, or derive a secure default (e.g., same-origin via
window.location.origin) for production; locate the backendUrl symbol in
Login.tsx and replace the fallback logic accordingly so production builds won't
silently point to http://localhost:5000.
---
Outside diff comments:
In `@backend/server.js`:
- Around line 25-29: The session middleware uses process.env.SESSION_SECRET
directly which can be undefined; validate and provide a non-empty secret (or
abort startup) before calling app.use(session(...)). Add code that reads
SESSION_SECRET into a variable, checks it's a non-empty string, and if missing
either throw an error/process.exit(1) or generate a secure fallback while
logging a clear warning; then pass that validated secret into the session config
used in app.use(session({ secret: secretVar, resave: false, saveUninitialized:
false })). Ensure the check runs at startup so the session middleware never
receives an undefined/empty secret.
🪄 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: 5677e360-2280-4d2e-a9d0-9f4ad2c58bda
📒 Files selected for processing (13)
backend/data/discussions.jsonbackend/data/users.jsonbackend/models/Discussion.jsbackend/routes/auth.jsbackend/routes/discussions.jsbackend/server.jsbackend/validators/discussionValidator.jssrc/App.tsxsrc/Routes/Router.tsxsrc/components/Navbar.tsxsrc/pages/Community/Community.tsxsrc/pages/Login/Login.tsxsrc/pages/Signup/Signup.tsx
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Related Issue
Description
Added a dedicated community discussion section to GitHub Tracker so users can create posts, share ideas, ask questions, and interact with other developers. This includes a discussion feed, create/edit/delete actions, comments, likes, search, category filtering, trending discussions, responsive UI, and dark mode support.
I also fixed the login issue because it was necessary for this feature creation and for authenticated discussion actions to work properly.
How Has This Been Tested?
npm run buildScreenshots (if applicable)
Type of Change
Summary by CodeRabbit
Release Notes
New Features
Improvements