refactor(forms): replace custom querystring logic with Django {% querystring %} tag#19672
Open
Deign86 wants to merge 2 commits into
Open
refactor(forms): replace custom querystring logic with Django {% querystring %} tag#19672Deign86 wants to merge 2 commits into
Deign86 wants to merge 2 commits into
Conversation
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
weblate/templates/paginator.html:45
- The paginator “jump to position” GET form no longer includes hidden inputs for the active filters/search parameters (previously provided via
search_items). Submitting the position form will now drop all existing query params exceptpage/limit, changing the result set unexpectedly. Preserve the current query parameters in the form submission (e.g., render hidden inputs fromrequest.GETexcludingpage/limit, or use a querystring-basedactionthat retains them).
<a class="position-input-editable" title="{% translate "Go to position" %}">
{# djlint:off #}
{% if not paginator_form %}<form method="get"{% if anchor %} action="#{{ anchor }}"{% endif %}>{% endif %}
{# djlint:on #}
<input {% if paginator_form %}form="{{ paginator_form }}"{% endif %}
type="hidden"
name="limit"
value="{{ page_obj.paginator.per_page }}"
aria-label="{{ page_obj.paginator.per_page }}" />
weblate/templates/trans/change_list.html:51
change_list.htmlnow ends inside{% block content %}without closing{% if form.is_valid %}, without rendering the search<form method="get">…</form>, and without{% endblock content %}. This is a template syntax error and will break the changes page rendering. Restore the missing closing tags/content (or adjust the structure) so the template is complete.
{% block content %}
{% if form.is_valid %}
{% include "paginator.html" %}
{% format_last_changes_content last_changes=object_list user=user %}
|
|
||
| # Some URLs we will most likely use | ||
| base_unit_url = f"{obj.get_translate_url()}?{search_result['url']}&offset=" | ||
| base_unit_url = f"{obj.get_translate_url()}?{request.GET.urlencode()}&offset=" |
|
|
||
| # Some URLs we will most likely use | ||
| base_unit_url = f"{obj.get_translate_url()}?{search_result['url']}&offset=" | ||
| base_unit_url = f"{obj.get_translate_url()}?{request.GET.urlencode()}&offset=" |
| ) | ||
|
|
||
| base_unit_url = f"{reverse('browse', kwargs={'path': obj.get_url_path()})}?{search_result['url']}&offset=" | ||
| base_unit_url = f"{reverse('browse', kwargs={'path': obj.get_url_path()})}?{request.GET.urlencode()}&offset=" |
Comment on lines
781
to
785
| initial={"scope": "global" if unit.is_source else "translation"}, | ||
| ), | ||
| "context_form": ContextForm(instance=unit.source_unit, user=user), | ||
| "search_form": search_result["form"].reset_offset(), | ||
| "search_form": search_result["form"], | ||
| "secondary": secondary, |
| </li> | ||
| <li class="page-item{% if page_obj.paginator.num_pages == page_obj.number %} disabled{% endif %}"> | ||
| <a href="?page={{ page_obj.paginator.num_pages }}&limit={{ page_obj.paginator.per_page }}{% if page_obj.paginator.sort_by %}&sort_by={{ page_obj.paginator.sort_by }}{% endif %}{% if query_string %}&{{ query_string }}{% endif %}{% if anchor %}#{{ anchor }}{% endif %}" | ||
| <a href="?{% querystring page=page_obj.paginator.num_pages limit=page_obj.paginator.per_page %}{% if page_obj.paginator.sort_by %}&sort_by={{ page_obj.paginator.sort_by }}{% endif %}{% if anchor %}#{{ anchor }}{% endif %}" |
Comment on lines
95
to
99
| <td> | ||
| {% if unit.context %} | ||
| <a href="{{ unit.get_absolute_url }}{% if include_search and search_url %}&{{ search_url }}{% elif sort_query %}&sort_by={{ sort_query }}{% endif %}"> | ||
| <a href="{{ unit.get_absolute_url }}{% if include_search %}{% querystring %}{% elif sort_query %}&sort_by={{ sort_query }}{% endif %}"> | ||
| {% format_source_string unit.context unit wrap=True search_match=search_query %} | ||
| </a> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the manual querystring building in SearchForm.items(), urlencode(), and reset_offset() (and ChangesForm equivalents) with Django's built-in {% querystring %} template tag introduced in Django 5.1.
Changes
Python
SearchForm.items(),SearchForm.urlencode(),SearchForm.reset_offset()(weblate/trans/forms.py)ChangesForm.items(),ChangesForm.urlencode()(weblate/trans/forms.py)from django.utils.http import urlencodeimportsearch()view in search.py to stop passingquery_string/search_url/search_itemsto templatesChangesViewin changes.py: replacedself.changes_form.urlencode()withself.request.GET.urlencode()edit.pyviews: removedsearch_url,search_items,reset_offset()referencesTemplates
paginator.html: pagination links now use{% querystring page=X limit=Y %}change_list.html: all?{{ query_string }}→?{% querystring %}translate.html,zen.html,zen-units.html:{{ search_url }}→{% querystring %}embed-units.html,last-changes-content.html: same patternThis simplifies the codebase by removing ~140 lines of manual querystring manipulation in favor of Django's built-in solution.
Closes #13638