Feature/per translator review - #20370
Conversation
nijel
left a comment
nijel
left a comment
There was a problem hiding this comment.
I think this does something different from what #2699 asked. My understanding there is that the motivation there is for each translator to have it's own review state.
While this pull request implements secondary tracking of who did the last state change to approved (effectively duplicating what can be figured out from the Change model already).
|
|
||
| class Meta: | ||
| app_label = "trans" | ||
| unique_together = [("unit", "user")] |
There was a problem hiding this comment.
The ruff is failing here, but you claim that you've run it successfully.
| def is_reviewed_by(self, user: User) -> bool: | ||
| if not user.is_authenticated: | ||
| return False | ||
| return self.reviews.filter(user=user).exists() |
There was a problem hiding this comment.
This doesn't seem to be used anywhere?
| if self.state == STATE_APPROVED: | ||
| self.reviews.get_or_create(user=user) | ||
| else: | ||
| self.reviews.filter(user=user).delete() |
There was a problem hiding this comment.
This will store at most one user in the reviews model. If there is no change, reviews are not updated (so if the string is already approved, no additional approvals will be added).
| # Test reviewed_by | ||
| self.unit = self.get_unit() | ||
| self.unit.reviews.create(user=self.user) | ||
| self.do_search({"q": "reviewed_by:me"}, "Hello, world") |
There was a problem hiding this comment.
The tests fail here; have you run the test suite?
|
@nijel You're completely right—my current implementation just mirrors the global state change rather than tracking independent per-user states. To properly implement the per-translator review state as requested in #2699, I'd like to align on the architecture before rewriting the logic. My proposed approach would be: Expand the new UnitReview model to store not just the user, but also the state (e.g., Approved, Needs Editing) that the specific user selected. Decouple the UI's "Review state" radio buttons from the global Unit.state, so that submitting the form updates the current user's UnitReview record instead. My main architectural question before proceeding: How should this interact with the global Unit.state? If User A marks their personal state as "Approved", but User B marks theirs as "Needs editing", how should we calculate the global Unit.state? Should we introduce a threshold setting (e.g., requires 2 approvals to globally approve), or should the global state just be removed entirely in favor of an aggregated query? Let me know your thoughts on the best way to integrate this into the core engine! |
|
Hey @nijel any suggestions? |
|
soft bump. This is a needed feature. |
3 participants
This PR implements per-translator review tracking, fulfilling the request to explicitly list which users have approved a translation, rather than relying on a binary anonymous "Approved" toggle.
A new
UnitReviewmodel has been introduced to track individual approvals. When a user approves a string, a badge reading Reviewed by: [username] will be displayed above the source string. If the translation is modified, all existing reviews for that unit are automatically cleared so it can be re-reviewed.This also adds support for the
reviewed_by:meandreviewed_by:[username]search queries.Changes Made
UnitReviewmodel and corresponding migration to track user reviews per translation unit.Unit.translate()to save aUnitReviewrecord whenSTATE_APPROVEDis selected, and to clear reviews when the translation text changes.translate.htmltemplate to render the Reviewed by: [username] badge.reviewed_by:search keyword support and tests.docs/changes.rstwith the new feature.UI Changes
Before:
Previously, the review process was completely anonymous. If translation reviews were turned on, clicking "Approved" would simply set the state to Approved, but there was no visual indicator of who actually reviewed the translation on the UI, nor was there a way to track multiple reviewers.
After:
Testing Performed
STATE_APPROVEDproperly records the review.UnitReviewrecords.reviewed_by:meto successfully filter approved strings.