Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Draft: Suggestions in Zen mode #11187

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Draft: Suggestions in Zen mode #11187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion weblate/templates/snippets/suggestions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
{% else %}
{% perm 'suggestion.delete' suggestion as user_can_delete_suggestion %}
{% if user_can_vote_suggestion or user_can_accept_suggestion or user_can_delete_suggestion %}
<div class="btn-float pull-right flip">
<div class="btn-float pull-right flip" style="top: -20px">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid inline styles.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I don't like it too, I wonder what should I do instead? Changing the original class doesn't sound like a good idea either, I guess I could create new one?

{% if user_can_vote_suggestion %}
<button type="submit" class="btn btn-link green" name="upvote" value="{{ suggestion.id }}" title="{% trans "Vote for" %}">{% icon "thumb-up.svg" %}</button>
<button type="submit" class="btn btn-link red" name="downvote" value="{{ suggestion.id }}" title="{% trans "Vote against" %}">{% icon "thumb-down.svg" %}</button>
Expand Down
11 changes: 11 additions & 0 deletions weblate/templates/zen-units.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,17 @@
{% endif %}
<td {% if user.profile.zen_mode != user.profile.ZEN_HORIZONTAL %}colspan="2"{% endif %} class="translator">
{% crispy item.form %}

{% perm 'suggestion.add' item.unit as user_can_suggest %}
{% perm 'suggestion.accept' item.unit as user_can_accept_suggestion %}
{% perm 'suggestion.vote' item.unit.translation as user_can_vote_suggestion %}

<form action="{{ this_unit_url }}" method="post">
{% csrf_token %}
<input type="hidden" name="checksum" value="{{ item.unit.checksum }}" />

{% include "snippets/suggestions.html" with suggestions=item.unit.suggestions %}
</form>
</td>
</tr>

Expand Down
26 changes: 26 additions & 0 deletions weblate/trans/views/edit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,32 @@

search_result, unitdata = get_zen_unitdata(obj, project, unit_set, request)
sort = get_sort_name(request, obj)
if (
request.method == "POST"
and "merge" not in request.POST
and (
"accept" in request.POST
or "accept_edit" in request.POST
or "accept_approve" in request.POST
or "delete" in request.POST
or "spam" in request.POST
or "upvote" in request.POST
or "downvote" in request.POST
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is essentially a copy of code from translate function, it should be shared.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I just put such a function in the same file, or it should go to some utils.py file?

):
# Handle accepting/deleting suggestions
_obj, unit_set, _context = parse_path_units(

Check warning on line 910 in weblate/trans/views/edit.py

View check run for this annotation

Codecov / codecov/patch

weblate/trans/views/edit.py#L910 

Added line #L910 was not covered by tests
request, path, (Translation, ProjectLanguage, CategoryLanguage)
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already done at top of this function.


checksum_form = ChecksumForm(unit_set, request.POST)

Check warning on line 914 in weblate/trans/views/edit.py

View check run for this annotation

Codecov / codecov/patch

weblate/trans/views/edit.py#L914 

Added line #L914 was not covered by tests
if not checksum_form.is_valid():
show_form_errors(request, checksum_form)
return HttpResponseBadRequest("Invalid checksum")

Check warning on line 917 in weblate/trans/views/edit.py

View check run for this annotation

Codecov / codecov/patch

weblate/trans/views/edit.py#L916-L917 

Added lines #L916 - L917 were not covered by tests

unit = checksum_form.cleaned_data["unit"]

Check warning on line 919 in weblate/trans/views/edit.py

View check run for this annotation

Codecov / codecov/patch

weblate/trans/views/edit.py#L919 

Added line #L919 was not covered by tests
# We are ignoring any redirects and responses here
_response = handle_suggestions(request, unit, "", "")

Check warning on line 921 in weblate/trans/views/edit.py

View check run for this annotation

Codecov / codecov/patch

weblate/trans/views/edit.py#L921 

Added line #L921 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the return value is not needed, there is no need to store it in a variable.

Comment on lines +896 to +921
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor the code and add tests.

The code segment introduces new functionality to handle accepting or deleting suggestions within the zen function. While the code is functional, there are a few areas for improvement:

  1. The code duplicates logic from the translate function. Consider refactoring the code to share common logic and avoid duplication.
  2. The code has a high number of Boolean expressions, which can be simplified for better readability. Consider breaking down the complex condition into smaller, more manageable conditions.
  3. The code lacks test coverage, as indicated by the static analysis hints. Add appropriate tests to ensure the correctness of the new code paths.
  4. The return statements should be documented in the function's docstring to improve the code's maintainability and readability.

Do you want me to refactor the code to address these issues or open a GitHub issue to track these improvements?

Tools
Ruff

897-907: Too many Boolean expressions (9 > 8)

(PLR0916)

917-917: return is not documented in docstring

(DOC201)

GitHub Check: codecov/patch

[warning] 910-910: weblate/trans/views/edit.py#L910
Added line #L910 was not covered by tests

[warning] 914-914: weblate/trans/views/edit.py#L914
Added line #L914 was not covered by tests

[warning] 916-917: weblate/trans/views/edit.py#L916-L917
Added lines #L916 - L917 were not covered by tests

[warning] 919-919: weblate/trans/views/edit.py#L919
Added line #L919 was not covered by tests

[warning] 921-921: weblate/trans/views/edit.py#L921
Added line #L921 was not covered by tests


# Handle redirects
if isinstance(search_result, HttpResponse):
Expand Down
Loading