feat: add search & filter + tags system for notes - #147
Conversation
📝 WalkthroughWalkthroughNotes now support normalized tags across storage, CRUD APIs, creation, editing, display, search, and filtering. The frontend adds reusable tag badges, tag selection, debounced search, filtered empty states, and an authenticated endpoint for aggregated user tag counts. ChangesNotes tagging and filtering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant HomePage
participant SearchBar
participant TagFilter
participant notesController
participant NoteModel
User->>SearchBar: Enter search text
SearchBar->>HomePage: Update searchQuery
User->>TagFilter: Select a tag
TagFilter->>HomePage: Update selectedTag
HomePage->>notesController: Request filtered notes
notesController->>NoteModel: Query user-scoped notes
NoteModel-->>notesController: Return matching notes
notesController-->>HomePage: Return filtered notes
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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)
frontend/src/pages/HomePage.jsx (1)
52-84: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPrevent race conditions in search results.
When typing quickly, network responses may arrive out of order, causing older search queries to overwrite newer ones. Pass an
AbortSignaltofetchNotesto cancel stale requests and prevent data tearing.🛡️ Proposed fix
- const fetchNotes = useCallback(async () => { + const fetchNotes = useCallback(async (signal) => { try { const params = {}; if (debouncedSearch.trim()) { params.search = debouncedSearch.trim(); } if (selectedTag) { params.tag = selectedTag; } - const res = await api.get("/notes", { params }); + const res = await api.get("/notes", { params, signal }); setNotes(res.data); setIsRateLimited(false); } catch (error) { + if (signal?.aborted) return; console.log("Error fetching notes", error.response); if (error.response?.status === 429) { setIsRateLimited(true); } else { toast.error("Failed to load notes"); } } finally { - setLoading(false); + if (!signal?.aborted) { + setLoading(false); + } } }, [debouncedSearch, selectedTag]); useEffect(() => { if (!authLoading && !user) { navigate("/login"); return; } + const controller = new AbortController(); if (user) { - fetchNotes(); + fetchNotes(controller.signal); } + return () => controller.abort(); }, [user, authLoading, navigate, fetchNotes]);🤖 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 `@frontend/src/pages/HomePage.jsx` around lines 52 - 84, Update fetchNotes to accept an AbortSignal and pass it through the api.get("/notes") request, then have the useEffect create an AbortController for each fetch and abort it during cleanup before starting or leaving a newer request. Ignore cancellation errors while preserving existing error handling for genuine failures, preventing stale responses from updating notes or related state.
🧹 Nitpick comments (3)
backend/src/controllers/notesController.js (1)
23-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNormalize query tags to lowercase to match persistence logic.
Tags are lowercased when saved to the database (in
createNoteandupdateNote). However, the query tags here are only trimmed. If a user manually passes mixed-case tags in the query string, they will fail to match. Ensure query tags are also lowercased.♻️ Proposed fix
if (tag && typeof tag === "string" && tag.trim()) { - const tags = tag.split(",").map((t) => t.trim()).filter(Boolean); + const tags = tag.split(",").map((t) => t.trim().toLowerCase()).filter(Boolean); if (tags.length > 0) { filter.tags = { $in: tags }; } }🤖 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/src/controllers/notesController.js` around lines 23 - 28, Normalize each query tag to lowercase in the tags transformation within the tag-filter block before assigning filter.tags, matching the persistence behavior used by createNote and updateNote while preserving trimming and empty-tag removal.frontend/src/components/TagBadge.jsx (1)
34-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImprove accessibility for interactive tags.
When
onClickis provided, this<span>acts as an interactive element. To make it accessible to keyboard and screen-reader users, consider rendering a<button>when it's clickable, or add appropriate ARIA roles and keyboard event handlers.♻️ Proposed refactor
return ( - <span + <button + type="button" className={`inline-flex items-center gap-1 rounded-full font-medium ${sizeClasses} ${getTagColor(tag)} ${activeClasses} ${onClick ? "cursor-pointer hover:opacity-80 transition-opacity" : ""}`} onClick={onClick ? (e) => { e.stopPropagation(); onClick(tag); } : undefined} + disabled={!onClick && !onRemove} > #{tag}Note: Depending on how
TagBadgeis used, you may prefer to conditionally render a<button>vs<span>to avoid nesting buttons ifonRemoveis also present.🤖 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 `@frontend/src/components/TagBadge.jsx` around lines 34 - 37, Update the interactive rendering in TagBadge so a tag with onClick is keyboard- and screen-reader-accessible, preferably by rendering a button while retaining span for non-interactive tags. Account for onRemove usage to avoid nested buttons, and preserve the existing stopPropagation and onClick(tag) behavior.frontend/src/pages/HomePage.jsx (1)
24-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify debounce logic.
The
useRefapproach to managing the debounce timer is unnecessarily complex. A standarduseEffectcleanup function inherently clears the timeout from the previous render.♻️ Proposed refactor
const [searchQuery, setSearchQuery] = useState(""); const [selectedTag, setSelectedTag] = useState(null); const [debouncedSearch, setDebouncedSearch] = useState(""); - const debounceTimer = useRef(null); // Drag & drop state const [activeDragId, setActiveDragId] = useState(null); // Group creation modal state const [showNamingModal, setShowNamingModal] = useState(false); const [modalSourceId, setModalSourceId] = useState(null); const [modalTargetId, setModalTargetId] = useState(null); const [groupTitle, setGroupTitle] = useState(""); // Debounce search input - useEffect(() => { - if (debounceTimer.current) { - clearTimeout(debounceTimer.current); - } - debounceTimer.current = setTimeout(() => { - setDebouncedSearch(searchQuery); - }, 300); - return () => { - if (debounceTimer.current) clearTimeout(debounceTimer.current); - }; - }, [searchQuery]); + useEffect(() => { + const timer = setTimeout(() => { + setDebouncedSearch(searchQuery); + }, 300); + return () => clearTimeout(timer); + }, [searchQuery]);🤖 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 `@frontend/src/pages/HomePage.jsx` around lines 24 - 47, In the HomePage debounce useEffect, remove the debounceTimer useRef and its manual clearTimeout checks. Create the timeout directly within the effect, update debouncedSearch after 300ms, and return cleanup that clears that effect’s timeout; keep searchQuery as the dependency.
🤖 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/src/controllers/notesController.js`:
- Around line 89-98: Update the tag parsing in createNote at
backend/src/controllers/notesController.js lines 89-98 to normalize tags, remove
duplicates, and reject requests with more than 10 tags using a 400 response;
apply the same uniqueness and maximum-length validation in updateNote at lines
147-155, preserving the existing array and string validation behavior.
- Around line 14-20: Escape the trimmed user input before constructing the
RegExp in the search filter block of notesController. Add or reuse a
regex-escaping helper, then pass the escaped value to new RegExp while
preserving case-insensitive title and content matching.
In `@frontend/src/pages/CreatePage.jsx`:
- Around line 32-40: Update handleTagKeyDown in
frontend/src/pages/CreatePage.jsx at lines 32-40 and the corresponding handler
in frontend/src/pages/NoteDetailPage.jsx at lines 84-89 so Tab only calls
preventDefault and handleAddTag when tagInput contains text; otherwise allow
normal focus navigation. Preserve the existing Enter and comma behavior.
In `@frontend/src/pages/HomePage.jsx`:
- Around line 196-206: Update the “Clear all filters” handler in HomePage to
immediately reset debouncedSearch alongside searchQuery and selectedTag before
triggering loading, preventing fetchNotes from using the stale search value
during the debounce window.
---
Outside diff comments:
In `@frontend/src/pages/HomePage.jsx`:
- Around line 52-84: Update fetchNotes to accept an AbortSignal and pass it
through the api.get("/notes") request, then have the useEffect create an
AbortController for each fetch and abort it during cleanup before starting or
leaving a newer request. Ignore cancellation errors while preserving existing
error handling for genuine failures, preventing stale responses from updating
notes or related state.
---
Nitpick comments:
In `@backend/src/controllers/notesController.js`:
- Around line 23-28: Normalize each query tag to lowercase in the tags
transformation within the tag-filter block before assigning filter.tags,
matching the persistence behavior used by createNote and updateNote while
preserving trimming and empty-tag removal.
In `@frontend/src/components/TagBadge.jsx`:
- Around line 34-37: Update the interactive rendering in TagBadge so a tag with
onClick is keyboard- and screen-reader-accessible, preferably by rendering a
button while retaining span for non-interactive tags. Account for onRemove usage
to avoid nested buttons, and preserve the existing stopPropagation and
onClick(tag) behavior.
In `@frontend/src/pages/HomePage.jsx`:
- Around line 24-47: In the HomePage debounce useEffect, remove the
debounceTimer useRef and its manual clearTimeout checks. Create the timeout
directly within the effect, update debouncedSearch after 300ms, and return
cleanup that clears that effect’s timeout; keep searchQuery as the dependency.
🪄 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: aea44da6-1c52-465a-86bc-cc14a61ee528
📒 Files selected for processing (11)
backend/src/controllers/notesController.jsbackend/src/models/Note.jsbackend/src/routes/notesRoutes.jsfrontend/src/components/NoteCard.jsxfrontend/src/components/NotesNotFound.jsxfrontend/src/components/SearchBar.jsxfrontend/src/components/TagBadge.jsxfrontend/src/components/TagFilter.jsxfrontend/src/pages/CreatePage.jsxfrontend/src/pages/HomePage.jsxfrontend/src/pages/NoteDetailPage.jsx
| if (search && typeof search === "string" && search.trim()) { | ||
| const regex = new RegExp(search.trim(), "i"); | ||
| filter.$or = [ | ||
| { title: regex }, | ||
| { content: regex }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Escape special characters in the search string to prevent ReDoS and application errors.
Passing user input directly to new RegExp without escaping special characters can lead to Regular Expression Denial of Service (ReDoS) or cause 500 errors if the user searches for characters like [, *, or ?.
Please escape the search string before using it in the regular expression.
🛡️ Proposed fix to escape regex characters
if (search && typeof search === "string" && search.trim()) {
- const regex = new RegExp(search.trim(), "i");
+ const escapedSearch = search.trim().replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+ const regex = new RegExp(escapedSearch, "i");
filter.$or = [
{ title: regex },
{ content: regex },
];
}📝 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.
| if (search && typeof search === "string" && search.trim()) { | |
| const regex = new RegExp(search.trim(), "i"); | |
| filter.$or = [ | |
| { title: regex }, | |
| { content: regex }, | |
| ]; | |
| } | |
| if (search && typeof search === "string" && search.trim()) { | |
| const escapedSearch = search.trim().replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | |
| const regex = new RegExp(escapedSearch, "i"); | |
| filter.$or = [ | |
| { title: regex }, | |
| { content: regex }, | |
| ]; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 14-14: Detects non-literal values in regular expressions
Context: new RegExp(search.trim(), "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
🤖 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/src/controllers/notesController.js` around lines 14 - 20, Escape the
trimmed user input before constructing the RegExp in the search filter block of
notesController. Add or reuse a regex-escaping helper, then pass the escaped
value to new RegExp while preserving case-insensitive title and content
matching.
Source: Linters/SAST tools
| // Validate tags if provided | ||
| let parsedTags = []; | ||
| if (tags !== undefined) { | ||
| if (!Array.isArray(tags)) { | ||
| return res.status(400).json({ message: "Tags must be an array of strings" }); | ||
| } | ||
| parsedTags = tags | ||
| .map((t) => (typeof t === "string" ? t.trim().toLowerCase() : "")) | ||
| .filter((t) => t.length > 0); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce tag limits and uniqueness on the backend.
Although the frontend limits users to 10 tags and prevents duplicates, this validation should also be enforced in the backend controllers to maintain data integrity and prevent malformed API requests from bypassing frontend constraints.
backend/src/controllers/notesController.js#L89-L98: Update thecreateNotetag parsing logic to remove duplicates and reject requests exceeding 10 tags.backend/src/controllers/notesController.js#L147-L155: Apply the same uniqueness and length constraints to theupdateNotelogic.
📍 Affects 1 file
backend/src/controllers/notesController.js#L89-L98(this comment)backend/src/controllers/notesController.js#L147-L155
🤖 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/src/controllers/notesController.js` around lines 89 - 98, Update the
tag parsing in createNote at backend/src/controllers/notesController.js lines
89-98 to normalize tags, remove duplicates, and reject requests with more than
10 tags using a 400 response; apply the same uniqueness and maximum-length
validation in updateNote at lines 147-155, preserving the existing array and
string validation behavior.
| const handleTagKeyDown = (e) => { | ||
| if (e.key === "Enter") { | ||
| e.preventDefault(); | ||
| handleAddTag(); | ||
| } else if (e.key === "," || e.key === "Tab") { | ||
| e.preventDefault(); | ||
| handleAddTag(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix keyboard trap on Tab key navigation.
Unconditionally calling e.preventDefault() when the Tab key is pressed prevents users from tabbing out of the input when it is empty. This breaks keyboard navigation and causes an accessibility blocker. You should only prevent default on Tab if there is text in tagInput to add as a tag.
frontend/src/pages/CreatePage.jsx#L32-L40: Adjust logic to only prevent default on Tab whentagInputis not empty.frontend/src/pages/NoteDetailPage.jsx#L84-L89: Adjust logic to only prevent default on Tab whentagInputis not empty.
♿ Proposed fixes
frontend/src/pages/CreatePage.jsx
const handleTagKeyDown = (e) => {
- if (e.key === "Enter") {
- e.preventDefault();
- handleAddTag();
- } else if (e.key === "," || e.key === "Tab") {
- e.preventDefault();
- handleAddTag();
- }
+ if (e.key === "Enter" || e.key === ",") {
+ e.preventDefault();
+ handleAddTag();
+ } else if (e.key === "Tab" && tagInput.trim()) {
+ e.preventDefault();
+ handleAddTag();
+ }
};frontend/src/pages/NoteDetailPage.jsx
const handleTagKeyDown = (e) => {
- if (e.key === "Enter" || e.key === "," || e.key === "Tab") {
- e.preventDefault();
- handleAddTag();
- }
+ if (e.key === "Enter" || e.key === ",") {
+ e.preventDefault();
+ handleAddTag();
+ } else if (e.key === "Tab" && tagInput.trim()) {
+ e.preventDefault();
+ handleAddTag();
+ }
};📝 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.
| const handleTagKeyDown = (e) => { | |
| if (e.key === "Enter") { | |
| e.preventDefault(); | |
| handleAddTag(); | |
| } else if (e.key === "," || e.key === "Tab") { | |
| e.preventDefault(); | |
| handleAddTag(); | |
| } | |
| }; | |
| const handleTagKeyDown = (e) => { | |
| if (e.key === "Enter" || e.key === ",") { | |
| e.preventDefault(); | |
| handleAddTag(); | |
| } else if (e.key === "Tab" && tagInput.trim()) { | |
| e.preventDefault(); | |
| handleAddTag(); | |
| } | |
| }; |
| const handleTagKeyDown = (e) => { | |
| if (e.key === "Enter") { | |
| e.preventDefault(); | |
| handleAddTag(); | |
| } else if (e.key === "," || e.key === "Tab") { | |
| e.preventDefault(); | |
| handleAddTag(); | |
| } | |
| }; | |
| const handleTagKeyDown = (e) => { | |
| if (e.key === "Enter" || e.key === ",") { | |
| e.preventDefault(); | |
| handleAddTag(); | |
| } else if (e.key === "Tab" && tagInput.trim()) { | |
| e.preventDefault(); | |
| handleAddTag(); | |
| } | |
| }; |
📍 Affects 2 files
frontend/src/pages/CreatePage.jsx#L32-L40(this comment)frontend/src/pages/NoteDetailPage.jsx#L84-L89
🤖 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 `@frontend/src/pages/CreatePage.jsx` around lines 32 - 40, Update
handleTagKeyDown in frontend/src/pages/CreatePage.jsx at lines 32-40 and the
corresponding handler in frontend/src/pages/NoteDetailPage.jsx at lines 84-89 so
Tab only calls preventDefault and handleAddTag when tagInput contains text;
otherwise allow normal focus navigation. Preserve the existing Enter and comma
behavior.
| <button | ||
| onClick={() => { | ||
| setSearchQuery(""); | ||
| setSelectedTag(null); | ||
| setLoading(true); | ||
| }} | ||
| className="ml-2 text-blue-600 dark:text-blue-400 hover:underline text-xs" | ||
| > | ||
| Clear all filters | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear the debounced search state immediately to prevent redundant requests.
When clearing all filters, selectedTag updates immediately, causing fetchNotes to fire before the 300ms debounce timer has a chance to clear debouncedSearch. This results in an unnecessary API call with the old search query. Setting debouncedSearch immediately aligns both states.
🐛 Proposed fix
<button
onClick={() => {
setSearchQuery("");
+ setDebouncedSearch("");
setSelectedTag(null);
setLoading(true);
}}
className="ml-2 text-blue-600 dark:text-blue-400 hover:underline text-xs"
>
Clear all filters
</button>📝 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.
| <button | |
| onClick={() => { | |
| setSearchQuery(""); | |
| setSelectedTag(null); | |
| setLoading(true); | |
| }} | |
| className="ml-2 text-blue-600 dark:text-blue-400 hover:underline text-xs" | |
| > | |
| Clear all filters | |
| </button> | |
| </div> | |
| <button | |
| onClick={() => { | |
| setSearchQuery(""); | |
| setDebouncedSearch(""); | |
| setSelectedTag(null); | |
| setLoading(true); | |
| }} | |
| className="ml-2 text-blue-600 dark:text-blue-400 hover:underline text-xs" | |
| > | |
| Clear all filters | |
| </button> | |
| </div> |
🤖 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 `@frontend/src/pages/HomePage.jsx` around lines 196 - 206, Update the “Clear
all filters” handler in HomePage to immediately reset debouncedSearch alongside
searchQuery and selectedTag before triggering loading, preventing fetchNotes
from using the stale search value during the debounce window.
1 participant
Summary
Adds a search bar and tags system for organizing and filtering notes in ThinkBoard.
Changes
Backend
tagsfield to Note model?search=(searches title & content) and?tag=(filter by tag) query paramsGET /notes/tagsendpoint returns all unique tags with countsFrontend
Files Changed
Build Status
✅ Vite build passes with zero errors
Testing
Summary by CodeRabbit