fix. convert all fields to String before matching#1032
fix. convert all fields to String before matching#1032
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/resources/templates/overview4.html (1)
232-239:⚠️ Potential issue | 🟠 Major
fuzzyFilterremains broken for entities with fuzzy search enabled.The issue is confirmed:
EventEntity,GalleryEntity,ErrorLogEntity,AdmissionEntryEntity, andEmailTemplateEntityall havefuzzy = true. ThefuzzyFilterfunction at lines 232-239 calls.trim()directly on potentially non-string or null values, which will throw aTypeErrorand abort the search.Formatted values from
OverviewBuilder.getTableDataandGenerateOverview.formatValuecan beBoolean, HTML strings, ornull— values that don't have a.trim()method. This is especially problematic forEventEntity, which is mentioned in the linked issue#998.Apply the same
String(...)normalization used elsewhere in the PR:Proposed fix
function fuzzyFilter(data, searchTerm) { let result = false; const fields = /*[[${searchSettings.rows}]]*/ []; for (let field in fields) { - result = result || isFuzzyMatch(data[fields[field]].trim(), searchTerm.trim()); + const raw = data[fields[field]]; + const value = raw == null ? "" : String(raw); + result = result || isFuzzyMatch(value.trim(), searchTerm.trim()); } return result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/resources/templates/overview4.html` around lines 232 - 239, The fuzzyFilter function currently calls .trim() directly on data values and the searchTerm which can be null/non-string and causes TypeError; update fuzzyFilter (the function named fuzzyFilter and its loop over fields) to normalize both the field value and searchTerm with String(...) (or String(value ?? '')) before calling .trim(), e.g. convert data[fields[field]] to String(data[fields[field]] ?? '') and convert searchTerm to String(searchTerm ?? '') so isFuzzyMatch always receives strings and does not throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/main/resources/templates/overview4.html`:
- Around line 185-193: In normalSearch, converting field values with
String(data[fields[field]]) turns null/undefined into "null"/"undefined" which
bypasses the value guard; update the implementation to treat nullish values as
empty strings before doing the contains check (e.g., read the raw value from
data using data[fields[field]] and normalize null/undefined to '' before
converting/casing and comparing with searchTerm) so that missing fields don't
match searches for "null"/"undefined"; adjust references in the function
(normalSearch, fields, data[fields[field]], searchTerm) accordingly.
---
Outside diff comments:
In `@backend/src/main/resources/templates/overview4.html`:
- Around line 232-239: The fuzzyFilter function currently calls .trim() directly
on data values and the searchTerm which can be null/non-string and causes
TypeError; update fuzzyFilter (the function named fuzzyFilter and its loop over
fields) to normalize both the field value and searchTerm with String(...) (or
String(value ?? '')) before calling .trim(), e.g. convert data[fields[field]] to
String(data[fields[field]] ?? '') and convert searchTerm to String(searchTerm ??
'') so isFuzzyMatch always receives strings and does not throw.
🪄 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: 62b5b425-5e3c-49e9-8e4a-d262bcebc50c
📒 Files selected for processing (1)
backend/src/main/resources/templates/overview4.html
| function normalSearch(data, searchTerm) { | ||
| let result = false; | ||
| const fields = /*[[${searchSettings.rows}]]*/ []; | ||
| for (let field in fields) { | ||
| const value = String(data[fields[field]]); | ||
| result = result || (value && value.toLowerCase().includes(searchTerm.toLowerCase())); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Edge case: null/undefined field values become matchable strings.
String(null) and String(undefined) produce the literal strings "null" / "undefined", which are truthy and will pass the value && ... guard. As a result, a user searching for the substring null or undefined will match rows where the corresponding field is actually missing. The pre-existing value && short-circuit is now effectively dead code, since String(...) always returns a non-empty string for these cases.
Consider normalizing nullish values to an empty string instead:
♻️ Proposed refinement
function normalSearch(data, searchTerm) {
let result = false;
const fields = /*[[${searchSettings.rows}]]*/ [];
for (let field in fields) {
- const value = String(data[fields[field]]);
- result = result || (value && value.toLowerCase().includes(searchTerm.toLowerCase()));
+ const raw = data[fields[field]];
+ const value = raw == null ? "" : String(raw);
+ result = result || (value !== "" && value.toLowerCase().includes(searchTerm.toLowerCase()));
}
return result;
}📝 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.
| function normalSearch(data, searchTerm) { | |
| let result = false; | |
| const fields = /*[[${searchSettings.rows}]]*/ []; | |
| for (let field in fields) { | |
| const value = String(data[fields[field]]); | |
| result = result || (value && value.toLowerCase().includes(searchTerm.toLowerCase())); | |
| } | |
| return result; | |
| } | |
| function normalSearch(data, searchTerm) { | |
| let result = false; | |
| const fields = /*[[${searchSettings.rows}]]*/ []; | |
| for (let field in fields) { | |
| const raw = data[fields[field]]; | |
| const value = raw == null ? "" : String(raw); | |
| result = result || (value !== "" && value.toLowerCase().includes(searchTerm.toLowerCase())); | |
| } | |
| return result; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/main/resources/templates/overview4.html` around lines 185 - 193,
In normalSearch, converting field values with String(data[fields[field]]) turns
null/undefined into "null"/"undefined" which bypasses the value guard; update
the implementation to treat nullish values as empty strings before doing the
contains check (e.g., read the raw value from data using data[fields[field]] and
normalize null/undefined to '' before converting/casing and comparing with
searchTerm) so that missing fields don't match searches for "null"/"undefined";
adjust references in the function (normalSearch, fields, data[fields[field]],
searchTerm) accordingly.
1 participant
Closes #998
Summary by CodeRabbit