Draft: Suggestions in Zen mode #11187
Conversation
juliancoffee
changed the title
| @@ -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"> | |||
There was a problem hiding this comment.
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?
| # Handle accepting/deleting suggestions | ||
| _obj, unit_set, _context = parse_path_units( | ||
| request, path, (Translation, ProjectLanguage, CategoryLanguage) | ||
| ) |
There was a problem hiding this comment.
This is already done at top of this function.
| or "spam" in request.POST | ||
| or "upvote" in request.POST | ||
| or "downvote" in request.POST | ||
| ) |
There was a problem hiding this comment.
This is essentially a copy of code from translate function, it should be shared.
There was a problem hiding this comment.
Can I just put such a function in the same file, or it should go to some utils.py file?
|
|
||
| unit = checksum_form.cleaned_data["unit"] | ||
| # We are ignoring any redirects and responses here | ||
| _response = handle_suggestions(request, unit, "", "") |
There was a problem hiding this comment.
If the return value is not needed, there is no need to store it in a variable.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11187 +/- ##
==========================================
- Coverage 90.82% 90.74% -0.08%
==========================================
Files 554 559 +5
Lines 57306 57680 +374
Branches 9122 9212 +90
==========================================
+ Hits 52046 52341 +295
- Misses 3640 3704 +64
- Partials 1620 1635 +15
|
|
Thanks for the review! I want to continue working on it, just got interrupted somewhat. |
|
This pull request has been automatically marked as stale because there wasn’t any recent activity. It will be closed soon if no further action occurs. Thank you for your contributions! |
|
meow, bot |
|
This pull request has been automatically marked as stale because there wasn’t any recent activity. It will be closed soon if no further action occurs. Thank you for your contributions! |
github-actions
bot
added
wontfix
|
This pull request has been automatically marked as stale because there wasn’t any recent activity. It will be closed soon if no further action occurs. Thank you for your contributions! |
github-actions
bot
added
wontfix
|
This pull request has been automatically marked as stale because there wasn’t any recent activity. It will be closed soon if no further action occurs. Thank you for your contributions! |
github-actions
bot
added
wontfix
|
This pull request has been automatically marked as stale because there wasn’t any recent activity. It will be closed soon if no further action occurs. Thank you for your contributions! |
WalkthroughThe pull request introduces modifications to the HTML templates and backend logic related to user suggestions in the translation interface. Changes include the addition of permission checks and a suggestion submission form in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
User->>Frontend: Submit suggestion
Frontend->>Backend: POST request with suggestion data
Backend->>Backend: Check permissions
Backend->>Backend: Validate suggestion form
Backend-->>Frontend: Return success or error response
Frontend-->>User: Display result
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- weblate/templates/snippets/suggestions.html (1 hunks)
- weblate/templates/zen-units.html (1 hunks)
- weblate/trans/views/edit.py (1 hunks)
Files skipped from review due to trivial changes (1)
- weblate/templates/snippets/suggestions.html
Additional context used
Ruff
weblate/trans/views/edit.py
897-907: Too many Boolean expressions (9 > 8)
(PLR0916)
917-917:
returnis not documented in docstring(DOC201)
GitHub Check: codecov/patch
weblate/trans/views/edit.py
[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
Additional comments not posted (3)
weblate/templates/zen-units.html (3)
93-95: LGTM!The permission checks are correctly implemented and ensure that the user has the necessary permissions to perform actions on suggestions.
97-99: LGTM!The form is correctly implemented with the appropriate action URL and method.
101-102: LGTM!The template inclusion is correctly implemented, and the
suggestionsvariable is passed to the included template as expected.
| 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 | ||
| ) | ||
| ): | ||
| # Handle accepting/deleting suggestions | ||
| _obj, unit_set, _context = parse_path_units( | ||
| request, path, (Translation, ProjectLanguage, CategoryLanguage) | ||
| ) | ||
|
|
||
| checksum_form = ChecksumForm(unit_set, request.POST) | ||
| if not checksum_form.is_valid(): | ||
| show_form_errors(request, checksum_form) | ||
| return HttpResponseBadRequest("Invalid checksum") | ||
|
|
||
| unit = checksum_form.cleaned_data["unit"] | ||
| # We are ignoring any redirects and responses here | ||
| _response = handle_suggestions(request, unit, "", "") |
There was a problem hiding this comment.
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:
- The code duplicates logic from the
translatefunction. Consider refactoring the code to share common logic and avoid duplication. - 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.
- The code lacks test coverage, as indicated by the static analysis hints. Add appropriate tests to ensure the correctness of the new code paths.
- 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:
returnis 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
Proposed changes
Solves #671, kind of.
At the time of writing, it is functional, but probably need to move most events to javascript to avoid resending Zen each time.
Also, I went via always show suggestions route, but I think that probably may be configured.
Checklist
Other information
Summary by CodeRabbit