Frontend crashes with special character '#' filter text box#5203
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates several API query utilities and a component to enable URL encoding by setting "encode: true" in "qs.stringify" calls. The reviewer feedback indicates that explicitly setting "encode: true" is redundant as it is the default behavior of the library. It is recommended to remove this property for cleaner code and to consider centralizing the stringify configuration options into a shared constant to improve maintainability and ensure consistency across the application.
|
|
||
| export function getProvidersQuery(query: ProvidersQuery) { | ||
| return stringify(query, { encode: false, indices: false }); | ||
| return stringify(query, { encode: true, indices: false }); |
There was a problem hiding this comment.
| // filter_by props are converted | ||
| export function getQuery(query: Query) { | ||
| return stringify(convertFilterBy(query), { encode: false, indices: false, sort: alphabeticalSort }); | ||
| return stringify(convertFilterBy(query), { encode: true, indices: false, sort: alphabeticalSort }); |
There was a problem hiding this comment.
Explicitly setting encode: true is redundant as it is the default behavior for qs.stringify. More importantly, the repetition of these configuration options (encode, indices, sort) across multiple functions and files is a maintainability risk. Consider centralizing the qs configuration into a shared constant to ensure consistent behavior (e.g., always applying the same sorting and encoding rules) and to simplify future updates.
| return stringify(convertFilterBy(query), { encode: true, indices: false, sort: alphabeticalSort }); | |
| return stringify(convertFilterBy(query), { indices: false, sort: alphabeticalSort }); |
| // filter_by props are not converted | ||
| export function getQueryRoute(query: Query) { | ||
| return stringify(query, { encode: false, indices: false, sort: alphabeticalSort }); | ||
| return stringify(query, { encode: true, indices: false, sort: alphabeticalSort }); |
There was a problem hiding this comment.
|
|
||
| export function getUserAccessQuery(query: UserAccessQuery) { | ||
| return stringify(query, { encode: false, indices: false }); | ||
| return stringify(query, { encode: true, indices: false }); |
| }; | ||
| return { | ||
| search: stringify(searchQuery, { encode: false, indices: false }), | ||
| search: stringify(searchQuery, { encode: true, indices: false }), |
|
|
||
| export function getProvidersQuery(query: ProvidersQuery) { | ||
| return stringify(query, { encode: false, indices: false }); | ||
| return stringify(query, { encode: true, indices: false }); |
| // filter_by props are converted | ||
| export function getQuery(query: Query) { | ||
| return stringify(convertFilterBy(query), { encode: false, indices: false, sort: alphabeticalSort }); | ||
| return stringify(convertFilterBy(query), { encode: true, indices: false, sort: alphabeticalSort }); |
There was a problem hiding this comment.
Explicitly setting encode: true is redundant as it is the default behavior for qs.stringify. Consider centralizing these options to improve maintainability and consistency across the application.
| return stringify(convertFilterBy(query), { encode: true, indices: false, sort: alphabeticalSort }); | |
| return stringify(convertFilterBy(query), { indices: false, sort: alphabeticalSort }); |
| // filter_by props are not converted | ||
| export function getQueryRoute(query: Query) { | ||
| return stringify(query, { encode: false, indices: false, sort: alphabeticalSort }); | ||
| return stringify(query, { encode: true, indices: false, sort: alphabeticalSort }); |
There was a problem hiding this comment.
|
|
||
| export function getUserAccessQuery(query: UserAccessQuery) { | ||
| return stringify(query, { encode: false, indices: false }); | ||
| return stringify(query, { encode: true, indices: false }); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5203 +/- ##
=======================================
Coverage 78.93% 78.93%
=======================================
Files 742 742
Lines 16830 16830
Branches 4050 4046 -4
=======================================
Hits 13284 13284
- Misses 3351 3354 +3
+ Partials 195 192 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Frontend crashes with special character '#' filter text box
https://redhat.atlassian.net/browse/COST-7507