Fix Rendelhető items url - #190
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughURL path construction for item filters is standardized across JavaScript and HTML template code. The JavaScript Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/static/js/item.js`:
- Line 691: Remove the leftover debug statement that logs URL_BASE and path on
every GET: delete the console.log("URL_BASE: " + URL_BASE + "\tpath: " + path)
call in src/main/resources/static/js/item.js (the line referencing URL_BASE and
path) or replace it with a conditional debug logger controlled by a debug flag
so it does not log in production.
- Around line 58-64: getFilter currently returns values with a leading '/' which
breaks updateUrl because the slash ends up in the query string, losing the
filter on reload; fix by making getFilter return the bare token ('now' or
'tomorrow' or '') and prepend '/' only where used as a path (e.g., where you
build API paths or call fetch items), or alternatively, keep getFilter as-is but
change updateUrl to strip a leading '/' when composing the query string; update
the getFilter and updateUrl usages accordingly so path-building uses '/' +
getFilter() while query-building uses getFilter().replace(/^\/+/,'').
🪄 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: e8bd0fa1-935f-4e4a-8ee4-02c661af9e2b
📒 Files selected for processing (2)
src/main/resources/static/js/item.jssrc/main/resources/templates/items.html
| function getFilter(separator) { | ||
| if (location.search.includes('?now')) | ||
| return 'now' + separator; | ||
| return '/now' + separator; | ||
| if (location.search.includes('?tomorrow')) | ||
| return 'tomorrow' + separator; | ||
| return '/tomorrow' + separator; | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
updateUrl() is now broken — the leading / ends up inside the query string.
getFilter('') is also consumed by updateUrl() (lines 90–95). With the new return value '/now' / '/tomorrow', the produced history URLs become:
'/items' + '?' + '/now'→/items?/now'/items?' + '/now' + '&' + 'q=...'→/items?/now&q=...
After this pushState, location.search is ?/now…, and location.search.includes('?now') returns false, so on the very next interaction getFilter() returns '' and the now/tomorrow filter is silently dropped from api/items calls (and from any reload/share of that URL). This regresses the same filter behavior the PR is meant to fix.
Either keep getFilter() returning the bare token and prepend / only at the API call sites, or normalize in updateUrl(). Suggested minimal fix:
🔧 Proposed fix
function getFilter(separator) {
if (location.search.includes('?now'))
- return '/now' + separator;
+ return 'now' + separator;
if (location.search.includes('?tomorrow'))
- return '/tomorrow' + separator;
+ return '/tomorrow' + separator.replace(/^/, '');
return '';
}…and prepend the slash where the value is used as a path:
- getForJsonObject('api/items' + getFilter('') + (profile !== parseInt(0) ? '?circle=' + profile : ''))
+ getForJsonObject('api/items' + (getFilter('') ? '/' + getFilter('') : '') + (profile !== parseInt(0) ? '?circle=' + profile : ''))- getForJsonObject('api/items' + getFilter(''))
+ getForJsonObject('api/items' + (getFilter('') ? '/' + getFilter('') : ''))Alternatively, leave getFilter() as-is in this PR and patch updateUrl() to strip the leading / when composing the query string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/static/js/item.js` around lines 58 - 64, getFilter
currently returns values with a leading '/' which breaks updateUrl because the
slash ends up in the query string, losing the filter on reload; fix by making
getFilter return the bare token ('now' or 'tomorrow' or '') and prepend '/' only
where used as a path (e.g., where you build API paths or call fetch items), or
alternatively, keep getFilter as-is but change updateUrl to strip a leading '/'
when composing the query string; update the getFilter and updateUrl usages
accordingly so path-building uses '/' + getFilter() while query-building uses
getFilter().replace(/^\/+/,'').
1 participant
Summary by CodeRabbit
Bug Fixes
Refactor