Pagination Updated - Items per page option added #1625
Conversation
|
Hi @pombredanne @TG1999 @keshav-space @Hritik14 , Can you please review this PR It has passed all workflow tests and was also working fine on my local server. Thank you |
Rishi-source
force-pushed
the
Page-Updated
branch
4 times, most recently
from
d8068aa
to
c28765a
Compare
Rishi-source
force-pushed
the
Page-Updated
branch
2 times, most recently
from
e4a1a18
to
8d177c8
Compare
Signed-off-by: Rishi Garg <[email protected]>
Rishi-source
force-pushed
the
Page-Updated
branch
from
8d177c8
to
6ca25ee
Compare
|
@tdruez please see the changes now workflow tests are also successful . |
|
@tdruez I would request you to please review the PR I have made changes In the code after you requested some changes after first review. |
| @@ -3,7 +3,7 @@ | |||
| # VulnerableCode is a trademark of nexB Inc. | |||
| # SPDX-License-Identifier: Apache-2.0 | |||
| # See http://www.apache.org/licenses/LICENSE-2.0 for the license text. | |||
| # See https://github.com/aboutcode-org/vulnerablecode for support or download. | |||
| @@ -12,26 +12,87 @@ | |||
|
|
|||
| from vulnerabilities.models import ApiUser | |||
|
|
|||
| from .models import * | |||
| widget=forms.Select( | ||
| attrs={ | ||
| "class": "select is-small", | ||
| "onchange": "handlePageSizeChange(this.value)", |
| def clean_search(self): | ||
| return self.cleaned_data.get("search", "") |
There was a problem hiding this comment.
What is the purpose of this?
| def get_queryset(self, query=None): | ||
| """ | ||
| Get queryset with search/filter/ordering applied. | ||
| Args: | ||
| query (str, optional): Direct query for testing | ||
| """ | ||
| if query is not None: | ||
| return self._search(query) | ||
|
|
||
| if not self.is_valid(): | ||
| return self.model.objects.none() | ||
|
|
||
| return self._search(self.clean_search()) |
There was a problem hiding this comment.
What's the need for this logic?
| def _search(self, query): | ||
| """Execute package-specific search logic.""" | ||
| return ( | ||
| self.model.objects.search(query) | ||
| .with_vulnerability_counts() | ||
| .prefetch_related() | ||
| .order_by("package_url") | ||
| ) |
There was a problem hiding this comment.
This is not following the Django forms conventions.
| def _search(self, query): | ||
| """Execute vulnerability-specific search logic.""" | ||
| return self.model.objects.search(query=query).with_package_counts() |
| <a href="?page={{ page_obj.previous_page_number }}&search={{ search|urlencode }}&page_size={{ page_obj.paginator.per_page }}" | ||
| class="pagination-previous">Previous</a> |
There was a problem hiding this comment.
What happens if other filters are active, those are lost when using the pagination?
|
|
There was a problem hiding this comment.
All the complexity of those templates should likely be handled in Python.
| model = models.Package | ||
| template_name = "packages.html" | ||
| ordering = ["type", "namespace", "name", "version"] | ||
| class BaseSearchView(ListView): |
There was a problem hiding this comment.
Could we use the django_filters.views.FilterView instead of re-implementing it?
@tdruez what do you mean by "reusable" here. I would code the implementation again to make it more simpler and satisfy the required needs. |
@Rishi-source Let's say I want to add this pagination on another Django view, could you document the few steps I need to do with this current implementation? This would help to figure out where we can improve the "reusability". |
Add pagination to Package Search and Vulnerability Search #1616 #1617
This change aligns Package Search functionality and Vulnerability Search, improving consistency across the application and enhancing user control over result display.
For Package Search:
For Vulnerability Search:
Signed-off-by: Rishi Garg [email protected]