Repository navigation
Conversation
| # rubocop:disable Rails/UniqueValidationWithoutIndex | ||
| validates :translated_post_id, | ||
| uniqueness: { scope: :language_id, | ||
| message: :translation_already_exists }, | ||
| allow_nil: true | ||
| # rubocop:enable Rails/UniqueValidationWithoutIndex |
There was a problem hiding this comment.
Disabled this cop instead of adding a unique index on (translated_post_id, language_id). Production likely has duplicates already (that's what this issue is about), so the index would need a data cleanup first, and that felt out of scope here. The validation itself runs the same query either way and uses the existing index on translated_post_id.
| end | ||
| end | ||
|
|
||
| context "when the original post has a draft translation in the language" do |
There was a problem hiding this comment.
The issue only mentions published translations, but this implementation also blocks the case where the existing translation is still a draft. Drafts are shared between all admins, otherwise a second translation in the same language could be posted while the draft is pending.
There was a problem hiding this comment.
Yeah, the issue predates the existence of drafts, but this is the behavior I'd expect now that we have them. Thanks for including drafts in test coverage!
sarken
left a comment
sarken
left a comment
There was a problem hiding this comment.
Just a little nitpicks. If you want to wait to address it until we hear back from Translation about the error message, that's fine!
| end | ||
| end | ||
|
|
||
| context "when the original post has a draft translation in the language" do |
There was a problem hiding this comment.
Yeah, the issue predates the existence of drafts, but this is the behavior I'd expect now that we have them. Thanks for including drafts in test coverage!
| end | ||
| end | ||
|
|
||
| describe "translation language uniqueness" do |
There was a problem hiding this comment.
I think this can just be #translated_post_id to match the attribute we're validating. (If there are other validations on this attribute that we want to test at some future date, we can put them under here as well.)
| invalid_permissions: Comment permissions are invalid. | ||
| translated_post_id: | ||
| must_be_posted_first: cannot be posted before the original post | ||
| translation_already_exists: already has a translation in this language |
There was a problem hiding this comment.
I'm making sure this message is clear with Translation and will let you know what they say!
github-actions
Bot
added
Coder Has Actioned Review
and removed
Reviewed: Action Needed
labels
This branch has not been deployed
2 participants
Pull Request Checklist
AO3-1234 Fix thing)Issue
https://otwarchive.atlassian.net/browse/AO3-4122
Purpose
Adds a validation so an admin post can't be saved as a translation of another post when that post already has a translation in the same language. The error is reported on the translated post field, alongside the existing "cannot be same language as original post" and "cannot be posted before the original post" errors.
Testing Instructions
Credit
Pablo Monfort (he/him)