-
-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add search filter for items with no photo #383
base: main
Are you sure you want to change the base?
feat: add search filter for items with no photo #383
Conversation
WalkthroughThis pull request introduces enhancements across multiple files to support filtering items based on their photo status. Key changes include the addition of a new Changes
Possibly related PRs
Suggested reviewers
Security Recommendations
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
5106eda
to
d65958d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
frontend/pages/items.vue (1)
403-405
: Consider adding ARIA attributes for better accessibility.The checkbox implementation could benefit from additional accessibility attributes.
<label class="label mr-auto cursor-pointer"> - <input v-model="onlyWithoutPhoto" type="checkbox" class="toggle toggle-primary toggle-sm" /> - <span class="label-text ml-4 text-right"> {{ $t("items.only_without_photo") }} </span> + <input + v-model="onlyWithoutPhoto" + type="checkbox" + class="toggle toggle-primary toggle-sm" + :aria-label="$t('items.only_without_photo')" + /> + <span class="label-text ml-4 text-right" aria-hidden="true"> + {{ $t("items.only_without_photo") }} + </span> </label>backend/internal/data/repo/repo_items.go (1)
383-392
: LGTM! Query implementation is efficient.The filtering logic for items without photos is properly implemented using ent predicates. The query will utilize appropriate indexes through the ent framework's query builder.
Consider monitoring query performance if this filter is frequently used on large datasets. If needed, you could optimize by adding a composite index on
(primary, type)
columns in the attachments table.
🛑 Comments failed to post (1)
frontend/lib/api/classes/items.ts (1)
27-27: 💡 Codebase verification
Security measures need enhancement for the
onlyWithoutPhoto
filterBased on the code analysis:
- Authentication is properly implemented through token-based middleware (
mwAuthToken
) with role-based access control (mwRoles
).- Parameter validation exists but is basic (simple boolean conversion).
- No rate limiting was found in the implementation.
Security recommendations:
- Add rate limiting middleware to protect against abuse of the filtering API
- Implement input sanitization for the boolean parameter using the existing validation framework
- Consider adding logging for filter usage to detect potential abuse patterns
- Add request size limits to prevent DoS attacks through query parameter manipulation
🔗 Analysis chain
Verify security measures for the new filter.
Please ensure that appropriate security measures are in place:
- Rate limiting for the filter API endpoint
- Proper validation of the boolean parameter
- Access control checks before filtering sensitive data
Let's verify the security measures:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for rate limiting middleware and access controls # Look for rate limiting configuration echo "Checking for rate limiting configuration..." rg -l "rate.*limit|throttle" --type go # Check for parameter validation echo "Checking for parameter validation..." ast-grep --pattern 'func $_(ItemsQuery|ItemRequest) { $$$ onlyWithoutPhoto $$ { $$$ } }' # Look for access control checks echo "Checking for access control middleware..." rg -l "auth.*middleware|authorize" --type goLength of output: 610
Script:
#!/bin/bash # Let's check the actual implementation of the API endpoint and middleware # Check the API implementation for this endpoint echo "Checking API implementation..." rg -A 10 "onlyWithoutPhoto" # Check middleware implementation echo "Checking middleware implementation..." cat backend/app/api/middleware.go # Check validation implementation echo "Checking validation implementation..." cat backend/internal/sys/validate/errors.go # Look for rate limiting in configuration files echo "Checking configuration files..." fd -e yaml -e json -e toml | xargs rg "rate|limit|throttle"Length of output: 16813
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
frontend/pages/items.vue (2)
254-254
: LGTM! Consider implementing rate limiting.The search parameter is correctly added to the API call. Consider implementing rate limiting for the search API to prevent potential DoS attacks through rapid search requests.
390-414
: Enhance accessibility for the filter options.While the implementation is correct, consider these accessibility improvements:
- Add
aria-label
attributes to the checkboxes- Ensure proper keyboard navigation through the dropdown
- Add
role
attributes to the dropdown elements<div tabindex="0" - class="dropdown-content mt-1 max-h-[80vh] w-72 -translate-x-24 overflow-auto rounded-md bg-base-100 p-4 shadow" + class="dropdown-content mt-1 max-h-[80vh] w-72 -translate-x-24 overflow-auto rounded-md bg-base-100 p-4 shadow" + role="menu" + aria-label="Search options" > <!-- ... --> <label class="label mr-auto cursor-pointer"> <input v-model="onlyWithoutPhoto" type="checkbox" + aria-label="Filter items without photos" class="toggle toggle-primary toggle-sm" /> <span class="label-text ml-4 text-right"> {{ $t("items.only_without_photo") }} </span> </label> <!-- ... --> </div>
🛑 Comments failed to post (1)
frontend/pages/items.vue (1)
225-225: 💡 Codebase verification
URL parameters should be properly encoded to prevent XSS
The router query parameters in
items.vue
are not consistently encoded. While some parts of the codebase useencodeURIComponent
(e.g., inlabel-generator.vue
andPageQRCode.vue
), the query parameters initems.vue
are passed directly without encoding. This could potentially lead to XSS vulnerabilities through URL manipulation.
- In
items.vue
, add URL encoding when setting query parameters:router.push({ query: { ... q: encodeURIComponent(query.value || ""), onlyWithoutPhoto: onlyWithoutPhoto.value ? "true" : "false", ... } });🔗 Analysis chain
LGTM! Consider URL sanitization.
The router query parameter is correctly added. For additional security, consider implementing URL sanitization for all query parameters to prevent XSS via URL manipulation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for URL sanitization implementations rg -A 5 "sanitize|escape|encodeURI"Length of output: 11459
Script:
#!/bin/bash # Check for URL sanitization in the items.vue file rg -A 5 "router|query|params" frontend/pages/items.vue # Check for any existing URL sanitization utilities fd -e ts -e js -e vue | xargs rg -l "sanitizeURL|sanitizeQuery|encodeURI"Length of output: 3696
What type of PR is this?
What this PR does / why we need it:
Adds a new search filter for items without photos. This can be useful to locate items from the collection whose file needs to be extended with a picture.
Which issue(s) this PR fixes:
No issue
Special notes for your reviewer:
This PR also fixes a small visual inconsistency where longer filter lables are split into different lines and aligned differently from the rest of the entries.
Testing
In the Search page toggle the new checkbox
Only items without photo
inside Options.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
These changes improve user experience by allowing more specific item searches and enhancing the interface's usability.