Option to filter Vulnerable and Non Vulnerable Packages #1760
Conversation
Rishi-source
force-pushed
the
Vuln
branch
2 times, most recently
from
c8d7dea to
8fc0661
Compare
Signed-off-by: Rishi Garg <[email protected]>
|
Hi @TG1999 , please review this Pull request. |
|
@Rishi-source your code change and PR description do not add up, these things are nowhere to be found in the diff.
|
|
the tests are not added because first I want to get the implementation to get reviewed .previously they where added but Apologies for not removing the test part from the description after I have removed them from the code changes. |
|
I’ll update the description I have changed the code in the PR but forgotten to change the previous description |
There was a problem hiding this comment.
Thanks @Rishi-source, see suggestions below. Also, add tests for these changes.
vulnerabilities/api_v2.py
Outdated
| is_vulnerable = is_vulnerable.lower() == "true" | ||
| queryset = queryset.filter(is_vulnerable=is_vulnerable) | ||
|
|
||
| queryset = queryset.exclude(version="") |
There was a problem hiding this comment.
In the UI, (and likely also in the API), I would welcome an option to filter only vulnerable or non vulnerable packages when doing a search for packages.
the issue description quotes if the same thing can be implemented in the API so I have added it in the API as well.
There was a problem hiding this comment.
Yeah, I get that we need filter in the API, but what I'm trying to understand is whether we need this queryset = queryset.exclude(version="").
| <form method="get" style="display: inline;"> | ||
| {% if search %}<input type="hidden" name="search" value="{{ search }}">{% endif %} | ||
| <select name="vulnerable_only" class="select" id="vulnerable-select" onchange="this.form.submit()"> | ||
| <option value="">All Packages</option> | ||
| <option value="true" {% if request.GET.vulnerable_only == 'true' %}selected{% endif %}>Vulnerable Only</option> | ||
| <option value="false" {% if request.GET.vulnerable_only == 'false' %}selected{% endif %}>Non-Vulnerable Only</option> | ||
| </select> | ||
| </form> |
There was a problem hiding this comment.
I'm not sure if this is how we want to have filters, how about a filter icon to the left with drop-down with All, Non-vulnerable, Vulnerable options.
There was a problem hiding this comment.
Sure would add that as well.
vulnerabilities/views.py
Outdated
| if hasattr(self, "request"): | ||
| vulnerable_only = self.request.GET.get("vulnerable_only", "").lower() |
There was a problem hiding this comment.
Wouldn't it be much cleaner to use a form instead?
There was a problem hiding this comment.
Yes It would be clean as well as safer from XSS attacks.. would handle it from form.
Signed-off-by: RISHI GARG <[email protected]>
Signed-off-by: RISHI GARG <[email protected]>
Signed-off-by: RISHI GARG <[email protected]>
Signed-off-by: RISHI GARG <[email protected]>
Signed-off-by: RISHI GARG <[email protected]>
|
@keshav-space I have addressed the review points and I have added tests can you please review the changes again and let me know if any other thing is needed. |
Implement vulnerable package filtering functionality in package search
Fixes : #1759
Feature Added:
Changes Made:
Feature Added:
Tests Added:
Signed-off-by: Rishi Garg [email protected]