fix: URL-encode programming language filter values in resource list#2079
fix: URL-encode programming language filter values in resource list#2079tdruez merged 4 commits intoaboutcode-org:mainfrom
Conversation
Signed-off-by: uttam282005 <uttam282005@gmail.com>
Signed-off-by: uttam282005 <uttam282005@gmail.com>
tdruez
left a comment
There was a problem hiding this comment.
@uttam282005 Good start, the PR is small and well focused but a few changes are required before merging the code.
- Fix the JavaScript
- Add missing unit test for HTML rendering
- Remove the new lines you've added at the end of the template files
- Address the API test usefulness
| </td> | ||
| <td class="break-all"> | ||
| <a href="?programming_language={{ resource.programming_language }}" class="is-black-link">{{ resource.programming_language }}</a> | ||
| <a href="?programming_language={{ resource.programming_language|urlencode }}" class="is-black-link">{{ resource.programming_language }}</a> |
There was a problem hiding this comment.
Add a unit test in ScanPipeViewsTest to make sure the value is properly encoded in the rendered HTML.
| @@ -123,7 +123,7 @@ | |||
| <ul> | |||
| {% for entry in scan_summary.other_languages %} | |||
| {% if entry.value %} | |||
| <a href="{% url 'project_resources' project.slug %}?programming_language={{ entry.value }}" target="_blank"> | |||
| <a href="{% url 'project_resources' project.slug %}?programming_language={{ entry.value|urlencode }}" target="_blank"> | |||
There was a problem hiding this comment.
Add a unit test in ScanPipeViewsTest to make sure the values are properly encoded in the rendered HTML.
| response = self.csrf_client.get(url + "?slug=aaa") | ||
| self.assertEqual(2, response.data["count"]) | ||
|
|
||
| def test_scanpipe_api_project_action_resources_filterset_special_chars(self): |
There was a problem hiding this comment.
The test is unrelated to the PR changes. What's the reasoning behind testing the API in place of testing your code changes?
There was a problem hiding this comment.
i was just confirming whether or not special characters are correctly handled in the api. so, thought of adding a test for it.
on second thought django does handle this automatically.
should i remove this test?
There was a problem hiding this comment.
It's fine let's keep it. I was just wondering about your intent since the actual changes were not tested.
Signed-off-by: uttam282005 <uttam282005@gmail.com>
|
thanks for the review @tdruez i have done the requested changes. |
tdruez
left a comment
There was a problem hiding this comment.
Almost there, please revert those 2 unwanted leftover changes.
| {% endif %} | ||
| </script> | ||
| {% endblock %} No newline at end of file | ||
| {% endblock %} |
There was a problem hiding this comment.
Revert those type of unwanted changes.
| {% endif %} | ||
| </div> | ||
| {% endblock %} No newline at end of file | ||
| {% endblock %} |
There was a problem hiding this comment.
Revert those type of unwanted changes.
| response = self.csrf_client.get(url + "?slug=aaa") | ||
| self.assertEqual(2, response.data["count"]) | ||
|
|
||
| def test_scanpipe_api_project_action_resources_filterset_special_chars(self): |
There was a problem hiding this comment.
It's fine let's keep it. I was just wondering about your intent since the actual changes were not tested.
Signed-off-by: uttam282005 <uttam282005@gmail.com>
|
done. |
tdruez
left a comment
There was a problem hiding this comment.
@uttam282005 Looks good, thanks for the fix!

Issues
Changes
URL encoding improvements for programming language filters:
scan_summary_panel.htmlto use theurlencodefilter for programming language values.resource_list.htmlto use theurlencodefilter, preventing issues with special characters.project_charts.htmlto useencodeURIComponentfor programming language values in filter URLs.Testing for special character handling:
test_api.pyto verify that programming languages with special characters (e.g., "C#") are correctly filtered and returned by the API when URL-encoded.Checklist